add crd reconciler

api/v1alpha1/nginxingresscontroller_types.go

@@ -13,7 +13,9 @@ func init() {
 }
 
 const (
-	maxNameLength           = 100
+	maxNameLength = 100
+	// MaxCollisions is the maximum number of collisions allowed when generating a name for a managed resource. This corresponds to the status.CollisionCount
+	MaxCollisions           = 5
 	maxControllerNamePrefix = 253 - 10 // 253 is the max length of resource names - 10 to account for the length of the suffix https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names
 )
 
@@ -73,6 +75,7 @@ type NginxIngressControllerStatus struct {
 	// Count of hash collisions for the managed resources. The App Routing Operator uses this field
 	// as a collision avoidance mechanism when it needs to create the name for the managed resources.
 	// +optional
+	// +kubebuilder:validation:Maximum=5
 	CollisionCount int32 `json:"collisionCount"`
 
 	// ManagedResourceRefs is a list of references to the managed resources

config/crd/bases/approuting.kubernetes.azure.com_nginxingresscontrollers.yaml

@@ -80,6 +80,7 @@ spec:
                   App Routing Operator uses this field as a collision avoidance mechanism
                   when it needs to create the name for the managed resources.
                 format: int32
+                maximum: 5
                 type: integer
               conditions:
                 description: Conditions is an array of current observed conditions

pkg/controller/controller.go

@@ -10,6 +10,7 @@ import (
 	"os"
 
 	approutingv1alpha1 "github.com/Azure/aks-app-routing-operator/api/v1alpha1"
+	"github.com/Azure/aks-app-routing-operator/pkg/controller/nginxingress"
 	"github.com/Azure/aks-app-routing-operator/pkg/webhook"
 	"github.com/go-logr/logr"
 	cfgv1alpha2 "github.com/openservicemesh/osm/pkg/apis/config/v1alpha2"
@@ -165,10 +166,14 @@ func setupWebhooks(mgr ctrl.Manager, addWebhooksFn func(mgr ctrl.Manager) error)
 func setupControllers(mgr ctrl.Manager, conf *config.Config) error {
 	var selfDeploy *appsv1.Deployment = nil // self deploy doesn't work because operator isn't in same resources as child resources
 
-	if err := dns.NewExternalDns(mgr, conf, selfDeploy); err != nil {
+	if err := dns.NewExternalDns(mgr, conf); err != nil {
 		return fmt.Errorf("setting up external dns controller: %w", err)
 	}
 
+	if err := nginxingress.NewReconciler(conf, mgr); err != nil {
+		return fmt.Errorf("setting up nginx ingress controller reconciler: %w", err)
+	}
+
 	nginxConfigs, err := nginx.New(mgr, conf, selfDeploy)
 	if err != nil {
 		return fmt.Errorf("getting nginx configs: %w", err)

pkg/controller/dns/external_dns.go

@@ -8,7 +8,6 @@ import (
 	"github.com/Azure/aks-app-routing-operator/pkg/controller/controllername"
 	"github.com/Azure/aks-app-routing-operator/pkg/manifests"
 	"github.com/Azure/aks-app-routing-operator/pkg/util"
-	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -50,8 +49,8 @@ func addExternalDnsCleaner(manager ctrl.Manager, objs []cleanObj) error {
 }
 
 // NewExternalDns starts all resources required for external dns
-func NewExternalDns(manager ctrl.Manager, conf *config.Config, self *appsv1.Deployment) error {
-	instances := instances(conf, self)
+func NewExternalDns(manager ctrl.Manager, conf *config.Config) error {
+	instances := instances(conf)
 
 	deployInstances := filterAction(instances, deploy)
 	deployRes := getResources(deployInstances)
@@ -67,16 +66,16 @@ func NewExternalDns(manager ctrl.Manager, conf *config.Config, self *appsv1.Depl
 	return nil
 }
 
-func instances(conf *config.Config, self *appsv1.Deployment) []instance {
+func instances(conf *config.Config) []instance {
 	// public
 	publicCfg := publicConfig(conf)
 	publicAction := actionFromConfig(publicCfg)
-	publicResources := manifests.ExternalDnsResources(conf, self, []*manifests.ExternalDnsConfig{publicCfg})
+	publicResources := manifests.ExternalDnsResources(conf, []*manifests.ExternalDnsConfig{publicCfg})
 
 	// private
 	privateCfg := privateConfig(conf)
 	privateAction := actionFromConfig(privateCfg)
-	privateResources := manifests.ExternalDnsResources(conf, self, []*manifests.ExternalDnsConfig{privateCfg})
+	privateResources := manifests.ExternalDnsResources(conf, []*manifests.ExternalDnsConfig{privateCfg})
 
 	return []instance{
 		{

pkg/controller/dns/external_dns_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/Azure/aks-app-routing-operator/pkg/util"
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
-	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/rest"
@@ -25,9 +24,8 @@ var (
 	env        *envtest.Environment
 	restConfig *rest.Config
 	err        error
-	self       *appsv1.Deployment = nil
-	uid                           = uuid.New().String()
-	noZones                       = config.Config{
+	uid        = uuid.New().String()
+	noZones    = config.Config{
 		ClusterUid:        uid,
 		PrivateZoneConfig: config.DnsZoneConfig{},
 		PublicZoneConfig:  config.DnsZoneConfig{},
@@ -182,12 +180,12 @@ func TestInstances(t *testing.T) {
 			expected: []instance{
 				{
 					config:    publicConfig(&noZones),
-					resources: manifests.ExternalDnsResources(&noZones, self, []*manifests.ExternalDnsConfig{publicConfig(&noZones)}),
+					resources: manifests.ExternalDnsResources(&noZones, []*manifests.ExternalDnsConfig{publicConfig(&noZones)}),
 					action:    clean,
 				},
 				{
 					config:    privateConfig(&noZones),
-					resources: manifests.ExternalDnsResources(&noZones, self, []*manifests.ExternalDnsConfig{privateConfig(&noZones)}),
+					resources: manifests.ExternalDnsResources(&noZones, []*manifests.ExternalDnsConfig{privateConfig(&noZones)}),
 					action:    clean,
 				},
 			},
@@ -198,12 +196,12 @@ func TestInstances(t *testing.T) {
 			expected: []instance{
 				{
 					config:    publicConfig(&onlyPrivZones),
-					resources: manifests.ExternalDnsResources(&onlyPrivZones, self, []*manifests.ExternalDnsConfig{publicConfig(&onlyPrivZones)}),
+					resources: manifests.ExternalDnsResources(&onlyPrivZones, []*manifests.ExternalDnsConfig{publicConfig(&onlyPrivZones)}),
 					action:    clean,
 				},
 				{
 					config:    privateConfig(&onlyPrivZones),
-					resources: manifests.ExternalDnsResources(&onlyPrivZones, self, []*manifests.ExternalDnsConfig{privateConfig(&onlyPrivZones)}),
+					resources: manifests.ExternalDnsResources(&onlyPrivZones, []*manifests.ExternalDnsConfig{privateConfig(&onlyPrivZones)}),
 					action:    deploy,
 				},
 			},
@@ -214,12 +212,12 @@ func TestInstances(t *testing.T) {
 			expected: []instance{
 				{
 					config:    publicConfig(&onlyPubZones),
-					resources: manifests.ExternalDnsResources(&onlyPubZones, self, []*manifests.ExternalDnsConfig{publicConfig(&onlyPubZones)}),
+					resources: manifests.ExternalDnsResources(&onlyPubZones, []*manifests.ExternalDnsConfig{publicConfig(&onlyPubZones)}),
 					action:    deploy,
 				},
 				{
 					config:    privateConfig(&onlyPubZones),
-					resources: manifests.ExternalDnsResources(&onlyPubZones, self, []*manifests.ExternalDnsConfig{privateConfig(&onlyPubZones)}),
+					resources: manifests.ExternalDnsResources(&onlyPubZones, []*manifests.ExternalDnsConfig{privateConfig(&onlyPubZones)}),
 					action:    clean,
 				},
 			},
@@ -230,20 +228,20 @@ func TestInstances(t *testing.T) {
 			expected: []instance{
 				{
 					config:    publicConfig(&allZones),
-					resources: manifests.ExternalDnsResources(&allZones, self, []*manifests.ExternalDnsConfig{publicConfig(&allZones)}),
+					resources: manifests.ExternalDnsResources(&allZones, []*manifests.ExternalDnsConfig{publicConfig(&allZones)}),
 					action:    deploy,
 				},
 				{
 					config:    privateConfig(&allZones),
-					resources: manifests.ExternalDnsResources(&allZones, self, []*manifests.ExternalDnsConfig{privateConfig(&allZones)}),
+					resources: manifests.ExternalDnsResources(&allZones, []*manifests.ExternalDnsConfig{privateConfig(&allZones)}),
 					action:    deploy,
 				},
 			},
 		},
 	}
 
 	for _, test := range tests {
-		instances := instances(test.conf, self)
+		instances := instances(test.conf)
 		if !reflect.DeepEqual(instances, test.expected) {
 			t.Error(
 				"For", test.name,
@@ -255,9 +253,9 @@ func TestInstances(t *testing.T) {
 }
 
 func TestFilterAction(t *testing.T) {
-	allClean := instances(&noZones, self)
-	allDeploy := instances(&allZones, self)
-	oneDeployOneClean := instances(&onlyPrivZones, self)
+	allClean := instances(&noZones)
+	allDeploy := instances(&allZones)
+	oneDeployOneClean := instances(&onlyPrivZones)
 
 	tests := []struct {
 		name      string
@@ -318,7 +316,7 @@ func TestFilterAction(t *testing.T) {
 }
 
 func TestGetResources(t *testing.T) {
-	instances := instances(&noZones, self)
+	instances := instances(&noZones)
 	got := getResources(instances)
 	var expected []client.Object
 	for _, instance := range instances {
@@ -345,17 +343,17 @@ func TestGetLabels(t *testing.T) {
 		},
 		{
 			name:      "top level and private",
-			instances: filterAction(instances(&onlyPrivZones, self), deploy),
+			instances: filterAction(instances(&onlyPrivZones), deploy),
 			expected:  util.MergeMaps(manifests.GetTopLevelLabels(), manifests.PrivateProvider.Labels()),
 		},
 		{
 			name:      "top level and public",
-			instances: filterAction(instances(&onlyPubZones, self), deploy),
+			instances: filterAction(instances(&onlyPubZones), deploy),
 			expected:  util.MergeMaps(manifests.GetTopLevelLabels(), manifests.PublicProvider.Labels()),
 		},
 		{
 			name:      "all labels",
-			instances: instances(&allZones, self),
+			instances: instances(&allZones),
 			expected:  util.MergeMaps(manifests.GetTopLevelLabels(), manifests.PublicProvider.Labels(), manifests.PrivateProvider.Labels()),
 		},
 	}
@@ -374,36 +372,36 @@ func TestCleanObjs(t *testing.T) {
 	}{
 		{
 			name:      "private dns clean",
-			instances: instances(&onlyPubZones, self),
+			instances: instances(&onlyPubZones),
 			expected: []cleanObj{{
-				resources: instances(&onlyPubZones, self)[1].resources,
+				resources: instances(&onlyPubZones)[1].resources,
 				labels:    util.MergeMaps(manifests.GetTopLevelLabels(), manifests.PrivateProvider.Labels()),
 			}},
 		},
 		{
 			name:      "public dns clean",
-			instances: instances(&onlyPrivZones, self),
+			instances: instances(&onlyPrivZones),
 			expected: []cleanObj{{
-				resources: instances(&onlyPrivZones, self)[0].resources,
+				resources: instances(&onlyPrivZones)[0].resources,
 				labels:    util.MergeMaps(manifests.GetTopLevelLabels(), manifests.PublicProvider.Labels()),
 			}},
 		},
 		{
 			name:      "all dns clean",
-			instances: instances(&noZones, self),
+			instances: instances(&noZones),
 			expected: []cleanObj{
 				{
-					resources: instances(&noZones, self)[0].resources,
+					resources: instances(&noZones)[0].resources,
 					labels:    util.MergeMaps(manifests.GetTopLevelLabels(), manifests.PublicProvider.Labels()),
 				},
 				{
-					resources: instances(&noZones, self)[1].resources,
+					resources: instances(&noZones)[1].resources,
 					labels:    util.MergeMaps(manifests.GetTopLevelLabels(), manifests.PrivateProvider.Labels()),
 				}},
 		},
 		{
 			name:      "no dns clean",
-			instances: instances(&allZones, self),
+			instances: instances(&allZones),
 			expected:  []cleanObj(nil),
 		},
 	}
@@ -462,7 +460,7 @@ func TestAddExternalDnsCleaner(t *testing.T) {
 
 	err = addExternalDnsCleaner(m, []cleanObj{
 		{
-			resources: instances(&noZones, self)[0].resources,
+			resources: instances(&noZones)[0].resources,
 			labels:    util.MergeMaps(manifests.GetTopLevelLabels(), manifests.PublicProvider.Labels()),
 		}})
 	require.NoError(t, err)
@@ -473,7 +471,7 @@ func TestNewExternalDns(t *testing.T) {
 	require.NoError(t, err)
 
 	conf := &config.Config{NS: "app-routing-system", OperatorDeployment: "operator"}
-	err = NewExternalDns(m, conf, self)
+	err = NewExternalDns(m, conf)
 	require.NoError(t, err)
 }
 

pkg/controller/nginx/nginx.go

@@ -4,14 +4,12 @@ import (
 	"context"
 
 	"github.com/Azure/aks-app-routing-operator/pkg/config"
-	"github.com/Azure/aks-app-routing-operator/pkg/controller/ingress"
 	"github.com/Azure/aks-app-routing-operator/pkg/controller/service"
 	"github.com/Azure/aks-app-routing-operator/pkg/manifests"
 	appsv1 "k8s.io/api/apps/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 )
 
@@ -68,39 +66,13 @@ func New(m manager.Manager, conf *config.Config, self *appsv1.Deployment) ([]*ma
 		defaultIngConfig: defaultIngConfig,
 	}
 
-	if err := n.addIngressClassReconciler(); err != nil {
-		return nil, err
-	}
-
-	if err := n.addIngressControllerReconciler(); err != nil {
-		return nil, err
-	}
-
 	if err := n.addIngressReconciler(); err != nil {
 		return nil, err
 	}
 
 	return n.ingConfigs, nil
 }
 
-func (n *nginx) addIngressClassReconciler() error {
-	objs := []client.Object{}
-	for _, config := range n.ingConfigs {
-		objs = append(objs, manifests.NginxIngressClass(n.conf, n.self, config)...)
-	}
-
-	return ingress.NewIngressClassReconciler(n.manager, objs, n.name)
-}
-
-func (n *nginx) addIngressControllerReconciler() error {
-	objs := []client.Object{}
-	for _, config := range n.ingConfigs {
-		objs = append(objs, manifests.NginxIngressControllerResources(n.conf, n.self, config)...)
-	}
-
-	return ingress.NewIngressControllerReconciler(n.manager, objs, n.name)
-}
-
 func (n *nginx) addIngressReconciler() error {
 	return service.NewNginxIngressReconciler(n.manager, n.defaultIngConfig)
 }