swap default strategy to allow for default nic cleanup

go.mod

@@ -16,11 +16,13 @@ require (
 	github.com/prometheus/common v0.44.0
 	github.com/stretchr/testify v1.8.4
 	go.uber.org/zap v1.25.0
+	gomodules.xyz/jsonpatch/v2 v2.4.0
 	k8s.io/api v0.28.1
 	k8s.io/apiextensions-apiserver v0.28.1
 	k8s.io/apimachinery v0.28.1
 	k8s.io/client-go v0.28.1
 	k8s.io/klog/v2 v2.100.1
+	k8s.io/utils v0.0.0-20230726121419-3b25d923346b
 	sigs.k8s.io/controller-runtime v0.16.1
 	sigs.k8s.io/secrets-store-csi-driver v1.3.4
 	sigs.k8s.io/yaml v1.3.0
@@ -73,15 +75,13 @@ require (
 	golang.org/x/term v0.11.0 // indirect
 	golang.org/x/text v0.12.0 // indirect
 	golang.org/x/time v0.3.0 // indirect
-	gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
 	google.golang.org/protobuf v1.31.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/component-base v0.28.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20230816210353-14e408962443 // indirect
-	k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.3.0 // indirect
 )

pkg/controller/keyvault/placeholder_pod.go

@@ -138,13 +138,11 @@ func (p *PlaceholderPodController) Reconcile(ctx context.Context, req ctrl.Reque
 }
 
 func (p *PlaceholderPodController) buildDeployment(dep *appsv1.Deployment, spc *secv1.SecretProviderClass, ing *netv1.Ingress) {
-	selectorLabels := map[string]string{"app": spc.Name}
-	labels := util.MergeMaps(selectorLabels, manifests.GetTopLevelLabels())
-
+	labels := map[string]string{"app": spc.Name}
 	dep.Spec = appsv1.DeploymentSpec{
 		Replicas:             util.Int32Ptr(1),
 		RevisionHistoryLimit: util.Int32Ptr(2),
-		Selector:             &metav1.LabelSelector{MatchLabels: selectorLabels},
+		Selector:             &metav1.LabelSelector{MatchLabels: labels},
 		Template: corev1.PodTemplateSpec{
 			ObjectMeta: metav1.ObjectMeta{
 				Labels: labels,

pkg/controller/keyvault/placeholder_pod_test.go

@@ -5,9 +5,10 @@ package keyvault
 
 import (
 	"context"
-	"k8s.io/apimachinery/pkg/api/errors"
 	"testing"
 
+	"k8s.io/apimachinery/pkg/api/errors"
+
 	"github.com/go-logr/logr"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -92,7 +93,7 @@ func TestPlaceholderPodControllerIntegration(t *testing.T) {
 	replicas := int32(1)
 	historyLimit := int32(2)
 
-	expectedLabels := util.MergeMaps(spc.Labels, map[string]string{"app": spc.Name})
+	expectedLabels := map[string]string{"app": spc.Name}
 	expected := appsv1.DeploymentSpec{
 		Replicas:             &replicas,
 		RevisionHistoryLimit: &historyLimit,
@@ -217,7 +218,7 @@ func TestPlaceholderPodControllerNoManagedByLabels(t *testing.T) {
 	replicas := int32(1)
 	historyLimit := int32(2)
 
-	expectedLabels := util.MergeMaps(map[string]string{"app": spc.Name}, manifests.GetTopLevelLabels())
+	expectedLabels := .map[string]string{"app": spc.Name}
 	expected := appsv1.DeploymentSpec{
 		Replicas:             &replicas,
 		RevisionHistoryLimit: &historyLimit,

pkg/controller/nginxingress/default.go

@@ -6,8 +6,10 @@ import (
 	"time"
 
 	approutingv1alpha1 "github.com/Azure/aks-app-routing-operator/api/v1alpha1"
-	"github.com/Azure/aks-app-routing-operator/pkg/controller/common"
 	"github.com/Azure/aks-app-routing-operator/pkg/controller/controllername"
+	"github.com/Azure/aks-app-routing-operator/pkg/controller/metrics"
+	"github.com/Azure/aks-app-routing-operator/pkg/util"
+	"github.com/go-logr/logr"
 	netv1 "k8s.io/api/networking/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -22,10 +24,35 @@ const (
 	// DefaultNicName is the default Nginx Ingress Controller resource name
 	DefaultNicName         = "default"
 	DefaultNicResourceName = "nginx"
-	reconcileInterval      = time.Minute * 3
 )
 
 func NewDefaultReconciler(mgr ctrl.Manager) error {
+	name := controllername.New("default", "nginx", "ingress", "controller", "reconciler")
+	if err := mgr.Add(&defaultNicReconciler{
+		name:   name,
+		lgr:    name.AddToLogger(mgr.GetLogger()),
+		client: mgr.GetClient(),
+	}); err != nil {
+		return fmt.Errorf("adding default nginx ingress controller: %w", err)
+	}
+
+	return nil
+}
+
+type defaultNicReconciler struct {
+	name   controllername.ControllerNamer
+	client client.Client
+	lgr    logr.Logger
+}
+
+func (d *defaultNicReconciler) Start(ctx context.Context) (err error) {
+	start := time.Now()
+	d.lgr.Info("starting to reconcile default nginx ingress controller")
+	defer func() {
+		d.lgr.Info("finished reconciling default nginx ingress controller", "latencySec", time.Since(start).Seconds())
+		metrics.HandleControllerReconcileMetrics(d.name, ctrl.Result{}, err)
+	}()
+
 	nic := &approutingv1alpha1.NginxIngressController{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: approutingv1alpha1.GroupVersion.String(),
@@ -41,11 +68,48 @@ func NewDefaultReconciler(mgr ctrl.Manager) error {
 		Status: approutingv1alpha1.NginxIngressControllerStatus{},
 	}
 
-	if err := common.NewResourceReconciler(mgr, controllername.New("default", "nginx", "ingress", "controller", "reconciler"), []client.Object{nic}, reconcileInterval); err != nil {
-		return fmt.Errorf("creating default nginx ingress controller: %w", err)
+	d.lgr.Info("determining if default nginx ingress controller should be created")
+	shouldCreate, err := shouldCreateDefaultNic(d.client)
+	if err != nil {
+		d.lgr.Error(err, "checking if default nginx ingress controller should be created")
+		return fmt.Errorf("checking if default nginx ingress controller should be created: %w", err)
+	}
+
+	if !shouldCreate {
+		d.lgr.Info("default nginx ingress controller should not be created")
+		return nil
+	}
+
+	d.lgr.Info("upserting default nginx ingress controller")
+	if err := util.Upsert(ctx, d.client, nic); err != nil {
+		d.lgr.Error(err, "upserting default nginx ingress controller")
+		return fmt.Errorf("upserting default nginx ingress controller: %w", err)
 	}
 
 	return nil
+
+}
+
+func shouldCreateDefaultNic(cl client.Client) (bool, error) {
+	defaultIc := &netv1.IngressClass{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: DefaultIcName,
+		},
+	}
+
+	err := cl.Get(context.Background(), types.NamespacedName{Name: DefaultIcName}, defaultIc)
+	switch {
+	case err == nil: // default IngressClass exists, we must create default nic for upgrade story
+		return true, nil
+	case k8serrors.IsNotFound(err):
+		// default IngressClass does not exist, we don't need to create default nic. We aren't upgrading from older App Routing versions for the first time.
+		// this is either a new user or an existing user that deleted their default nic
+		return false, nil
+	case err != nil:
+		return false, fmt.Errorf("getting default ingress class: %w", err)
+	}
+
+	return false, nil
 }
 
 func getDefaultIngressClassControllerClass(cl client.Client) (string, error) {