From 1f0e14ade7965deef23ef2241eadbb97785d2deb Mon Sep 17 00:00:00 2001 From: Oliver King Date: Fri, 3 Nov 2023 22:28:32 -0500 Subject: [PATCH] add default nic controller --- pkg/controller/controller.go | 34 ++++++-- pkg/controller/nginxingress/default.go | 77 +++++++++++++++++++ .../nginxingress/nginx_ingress_controller.go | 40 ++++++++-- pkg/webhook/nginxingress.go | 8 +- 4 files changed, 141 insertions(+), 18 deletions(-) create mode 100644 pkg/controller/nginxingress/default.go diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index dc5f4847..761aaeaa 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -115,9 +115,12 @@ func NewManagerForRestConfig(conf *config.Config, rc *rest.Config) (ctrl.Manager return nil, fmt.Errorf("loading CRDs: %w", err) } - if err := setupControllers(m, conf); err != nil { - return nil, fmt.Errorf("setting up controllers: %w", err) - } + go func() { + if err := setupControllers(m, conf, webhooksReady, setupLog); err != nil { + setupLog.Error(err, "unable to setup controllers") + os.Exit(1) + } + }() webhookCfg, err := webhook.New(conf) if err != nil { @@ -147,8 +150,8 @@ func NewManagerForRestConfig(conf *config.Config, rc *rest.Config) (ctrl.Manager setupLog.Error(err, "failed to setup webhooks") os.Exit(1) } - setupLog.Info("webhooks are ready") + setupLog.Info("webhooks are ready") close(webhooksReady) }() @@ -163,18 +166,27 @@ func setupWebhooks(mgr ctrl.Manager, addWebhooksFn func(mgr ctrl.Manager) error) return nil } -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 - +func setupControllers(mgr ctrl.Manager, conf *config.Config, webhooksReady <-chan struct{}, lgr logr.Logger) error { + lgr.Info("settup up ExternalDNS controller") if err := dns.NewExternalDns(mgr, conf); err != nil { return fmt.Errorf("setting up external dns controller: %w", err) } + lgr.Info("setting up Nginx Ingress Controller reconciler") 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) + lgr.Info("waiting for webhooks to be ready") + <-webhooksReady // some controllers need to wait for webhooks to be ready because they interact directly with our CRDs + lgr.Info("finished waiting for webhooks to be ready") + + lgr.Info("setting up default Nginx Ingress Controller reconciler") + if err := nginxingress.NewDefaultReconciler(mgr); err != nil { + return fmt.Errorf("setting up nginx ingress default controller reconciler: %w", err) + } + + nginxConfigs, err := nginx.New(mgr, conf, nil) if err != nil { return fmt.Errorf("getting nginx configs: %w", err) } @@ -187,6 +199,7 @@ func setupControllers(mgr ctrl.Manager, conf *config.Config) error { }) } + lgr.Info("setting up ingress concurrency watchdog") if err := ingress.NewConcurrencyWatchdog(mgr, conf, watchdogTargets); err != nil { return fmt.Errorf("setting up ingress concurrency watchdog: %w", err) } @@ -201,20 +214,25 @@ func setupControllers(mgr ctrl.Manager, conf *config.Config) error { } ingressManager := keyvault.NewIngressManager(ics) + lgr.Info("setting up keyvault secret provider class reconciler") if err := keyvault.NewIngressSecretProviderClassReconciler(mgr, conf, ingressManager); err != nil { return fmt.Errorf("setting up ingress secret provider class reconciler: %w", err) } + lgr.Info("setting up keyvault placeholder pod controller") if err := keyvault.NewPlaceholderPodController(mgr, conf, ingressManager); err != nil { return fmt.Errorf("setting up placeholder pod controller: %w", err) } + lgr.Info("setting up keyvault event mirror") if err = keyvault.NewEventMirror(mgr, conf); err != nil { return fmt.Errorf("setting up event mirror: %w", err) } ingressControllerNamer := osm.NewIngressControllerNamer(icToController) + lgr.Info("setting up ingress backend reconciler") if err := osm.NewIngressBackendReconciler(mgr, conf, ingressControllerNamer); err != nil { return fmt.Errorf("setting up ingress backend reconciler: %w", err) } + lgr.Info("setting up ingress cert config reconciler") if err = osm.NewIngressCertConfigReconciler(mgr, conf); err != nil { return fmt.Errorf("setting up ingress cert config reconciler: %w", err) } diff --git a/pkg/controller/nginxingress/default.go b/pkg/controller/nginxingress/default.go new file mode 100644 index 00000000..eb2628a2 --- /dev/null +++ b/pkg/controller/nginxingress/default.go @@ -0,0 +1,77 @@ +package nginxingress + +import ( + "context" + "fmt" + "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" + netv1 "k8s.io/api/networking/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + // DefaultcName is the default Ingress class name + DefaultIcName = "webapprouting.kubernetes.azure.com" + // DefaultNicName is the default Nginx Ingress Controller resource name + DefaultNicName = "default" + DefaultNicResourceName = "nginx" + reconcileInterval = time.Minute * 3 +) + +func NewDefaultReconciler(mgr ctrl.Manager) error { + nic := &approutingv1alpha1.NginxIngressController{ + TypeMeta: metav1.TypeMeta{ + APIVersion: approutingv1alpha1.GroupVersion.String(), + Kind: "NginxIngressController", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultNicName, + }, + Spec: approutingv1alpha1.NginxIngressControllerSpec{ + ControllerNamePrefix: "nginx", + IngressClassName: DefaultIcName, + }, + 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) + } + + return nil +} + +func getDefaultIngressClassControllerClass(cl client.Client) (string, error) { + defaultNicCc := "webapprouting.kubernetes.azure.com/nginx" + + defaultIc := &netv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultIcName, + }, + } + err := cl.Get(context.Background(), types.NamespacedName{Name: DefaultIcName}, defaultIc) + if err == nil { + defaultNicCc = defaultIc.Spec.Controller + } + if err != nil && !k8serrors.IsNotFound(err) { + return "", fmt.Errorf("getting default ingress class: %w", err) + } + + return defaultNicCc, nil +} + +// IsDefaultNic returns true if the given NginxIngressController is the default one +func IsDefaultNic(nic *approutingv1alpha1.NginxIngressController) bool { + if nic == nil { + return false + } + + return nic.Name == DefaultNicName && nic.Spec.IngressClassName == DefaultIcName +} diff --git a/pkg/controller/nginxingress/nginx_ingress_controller.go b/pkg/controller/nginxingress/nginx_ingress_controller.go index a5166ef4..3462703e 100644 --- a/pkg/controller/nginxingress/nginx_ingress_controller.go +++ b/pkg/controller/nginxingress/nginx_ingress_controller.go @@ -47,19 +47,32 @@ var ( // nginxIngressControllerReconciler reconciles a NginxIngressController object type nginxIngressControllerReconciler struct { - client client.Client - conf *config.Config - events record.EventRecorder + client client.Client + conf *config.Config + events record.EventRecorder + defaultNicControllerClass string } // NewReconciler sets up the controller with the Manager. func NewReconciler(conf *config.Config, mgr ctrl.Manager) error { metrics.InitControllerMetrics(nginxIngressControllerReconcilerName) + // use non caching client since it's before manager is started + nonCachingCl, err := client.New(mgr.GetConfig(), client.Options{}) + if err != nil { + return fmt.Errorf("creating non caching client: %w", err) + } + + defaultNicCc, err := getDefaultIngressClassControllerClass(nonCachingCl) + if err != nil { + return fmt.Errorf("getting default ingress class controller class: %w", err) + } + reconciler := &nginxIngressControllerReconciler{ - client: mgr.GetClient(), - conf: conf, - events: mgr.GetEventRecorderFor("aks-app-routing-operator"), + client: mgr.GetClient(), + conf: conf, + events: mgr.GetEventRecorderFor("aks-app-routing-operator"), + defaultNicControllerClass: defaultNicCc, } if err := nginxIngressControllerReconcilerName.AddToController( @@ -198,15 +211,21 @@ func (n *nginxIngressControllerReconciler) ManagedResources(nic *approutingv1alp } cc := "webapprouting.kubernetes.azure.com/nginx/" + url.PathEscape(nic.Name) - // it's impossible for this to happen because we enforce nic.Name to be less than 101 characters in validating webhooks if len(cc) > controllerClassMaxLen { cc = cc[:controllerClassMaxLen] } + resourceName := fmt.Sprintf("%s-%d", nic.Spec.ControllerNamePrefix, nic.Status.CollisionCount) + + if IsDefaultNic(nic) { + cc = n.defaultNicControllerClass + resourceName = DefaultNicResourceName + } + nginxIngressCfg := &manifests.NginxIngressConfig{ ControllerClass: cc, - ResourceName: fmt.Sprintf("%s-%d", nic.Spec.ControllerNamePrefix, nic.Status.CollisionCount), + ResourceName: resourceName, IcName: nic.Spec.IngressClassName, ServiceConfig: &manifests.ServiceConfig{ Annotations: nic.Spec.LoadBalancerAnnotations, @@ -267,6 +286,11 @@ func (n *nginxIngressControllerReconciler) GetCollisionCount(ctx context.Context func (n *nginxIngressControllerReconciler) collides(ctx context.Context, nic *approutingv1alpha1.NginxIngressController) (collision, error) { lgr := log.FromContext(ctx) + // ignore any collisions on default nic for migration story. Need to acquire ownership of old app routing resources + if IsDefaultNic(nic) { + return collisionNone, nil + } + res := n.ManagedResources(nic) if res == nil { return collisionNone, fmt.Errorf("getting managed objects") diff --git a/pkg/webhook/nginxingress.go b/pkg/webhook/nginxingress.go index 66ecd2eb..7ab35ca0 100644 --- a/pkg/webhook/nginxingress.go +++ b/pkg/webhook/nginxingress.go @@ -9,6 +9,7 @@ import ( approutingv1alpha1 "github.com/Azure/aks-app-routing-operator/api/v1alpha1" "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/controller/nginxingress" "github.com/Azure/aks-app-routing-operator/pkg/manifests" "github.com/Azure/aks-app-routing-operator/pkg/util" "github.com/go-logr/logr" @@ -202,9 +203,12 @@ func (n *nginxIngressResourceValidator) Handle(ctx context.Context, req admissio return admission.Denied(invalidReason) } - // TODO: record metrics, will add later - if req.Operation == admissionv1.Create { + if nginxingress.IsDefaultNic(&nginxIngressController) { + // need to allow for ic to exist already for the default migration case (migrating from non-crd versions of app routing to crd versions) + return admission.Allowed("") + } + lgr.Info("checking if IngressClass already exists") ic := &netv1.IngressClass{ ObjectMeta: metav1.ObjectMeta{