From 68bb0c2129a761f53e2ec872479b605da30ab8e1 Mon Sep 17 00:00:00 2001 From: MichalK Date: Tue, 6 Sep 2022 18:13:06 +0200 Subject: [PATCH] Refactoring SetupManager (#948) * Refactoring SetupManager related to #932 The setup manager is quite complex. We are currently solving a bug related to the fact that changes in ingress also change the GSLB and can cause cycles #932. I am doing the necessary refactoring (code is better structured, but the functionality remains the same), to make it easier to make the necessary changes.. - split SetupWithManager and Reconciliation into two files - encapsulate createGslbFromIngress - simplify ingressHandler - extracting parse strategy func Signed-off-by: kuritka * Update controllers/gslb_controller_setup.go Co-authored-by: Jirka Kremser <535866+jkremser@users.noreply.github.com> Signed-off-by: MichalK Signed-off-by: kuritka Signed-off-by: MichalK Co-authored-by: Jirka Kremser <535866+jkremser@users.noreply.github.com> --- ...r.go => gslb_controller_reconciliation.go} | 165 --------------- ...=> gslb_controller_reconciliation_test.go} | 0 controllers/gslb_controller_setup.go | 197 ++++++++++++++++++ controllers/gslb_controller_setup_test.go | 19 ++ 4 files changed, 216 insertions(+), 165 deletions(-) rename controllers/{gslb_controller.go => gslb_controller_reconciliation.go} (53%) rename controllers/{gslb_controller_test.go => gslb_controller_reconciliation_test.go} (100%) create mode 100644 controllers/gslb_controller_setup.go create mode 100644 controllers/gslb_controller_setup_test.go diff --git a/controllers/gslb_controller.go b/controllers/gslb_controller_reconciliation.go similarity index 53% rename from controllers/gslb_controller.go rename to controllers/gslb_controller_reconciliation.go index 24af69f06b..d869048351 100644 --- a/controllers/gslb_controller.go +++ b/controllers/gslb_controller_reconciliation.go @@ -21,7 +21,6 @@ Generated by GoLic, for more details see: https://github.com/AbsaOSS/golic import ( "context" "fmt" - "strconv" "github.com/k8gb-io/k8gb/controllers/providers/metrics" @@ -32,19 +31,10 @@ import ( "github.com/k8gb-io/k8gb/controllers/providers/dns" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" - corev1 "k8s.io/api/core/v1" - netv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - types "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sigs.k8s.io/controller-runtime/pkg/source" - externaldns "sigs.k8s.io/external-dns/endpoint" ) // GslbReconciler reconciles a Gslb object @@ -198,158 +188,3 @@ func (r *GslbReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. m.IncrementReconciliation(gslb) return result.Requeue() } - -// SetupWithManager configures controller manager -func (r *GslbReconciler) SetupWithManager(mgr ctrl.Manager) error { - // Figure out Gslb resource name to Reconcile when non controlled Name is updated - - endpointMapHandler := handler.EnqueueRequestsFromMapFunc( - func(a client.Object) []reconcile.Request { - gslbList := &k8gbv1beta1.GslbList{} - opts := []client.ListOption{ - client.InNamespace(a.GetNamespace()), - } - c := mgr.GetClient() - err := c.List(context.TODO(), gslbList, opts...) - if err != nil { - log.Info().Msg("Can't fetch gslb objects") - return nil - } - gslbName := "" - for _, gslb := range gslbList.Items { - for _, rule := range gslb.Spec.Ingress.Rules { - for _, path := range rule.HTTP.Paths { - if path.Backend.Service != nil && path.Backend.Service.Name == a.GetName() { - gslbName = gslb.Name - } - } - } - } - if len(gslbName) > 0 { - return []reconcile.Request{ - {NamespacedName: types.NamespacedName{ - Name: gslbName, - Namespace: a.GetNamespace(), - }}, - } - } - return nil - }) - - createGslbFromIngress := func(annotationKey string, annotationValue string, a client.Object, strategy string) { - log.Info(). - Str("annotation", fmt.Sprintf("(%s:%s)", annotationKey, annotationValue)). - Str("ingress", a.GetName()). - Msg("Detected strategy annotation on ingress") - c := mgr.GetClient() - ingressToReuse := &netv1.Ingress{} - err := c.Get(context.Background(), client.ObjectKey{ - Namespace: a.GetNamespace(), - Name: a.GetName(), - }, ingressToReuse) - if err != nil { - log.Info(). - Str("ingress", a.GetName()). - Msg("Ingress does not exist anymore. Skipping Glsb creation...") - return - } - gslbExist := &k8gbv1beta1.Gslb{} - err = c.Get(context.Background(), client.ObjectKey{ - Namespace: a.GetNamespace(), - Name: a.GetName(), - }, gslbExist) - if err == nil { - log.Info(). - Str("gslb", gslbExist.Name). - Msg("Gslb already exists. Skipping Gslb creation...") - return - } - gslb := &k8gbv1beta1.Gslb{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: a.GetNamespace(), - Name: a.GetName(), - Annotations: a.GetAnnotations(), - }, - Spec: k8gbv1beta1.GslbSpec{ - Ingress: k8gbv1beta1.FromV1IngressSpec(ingressToReuse.Spec), - Strategy: k8gbv1beta1.Strategy{ - Type: strategy, - }, - }, - } - - annotationToInt := func(k string, v string) int { - intValue, err := strconv.Atoi(v) - if err != nil { - log.Err(err). - Str("annotationKey", k). - Str("annotationValue", v). - Msg("Can't parse annotation value to int") - } - return intValue - } - - for annotationKey, annotationValue := range a.GetAnnotations() { - switch annotationKey { - case dnsTTLSecondsAnnotation: - gslb.Spec.Strategy.DNSTtlSeconds = annotationToInt(annotationKey, annotationValue) - case splitBrainThresholdSecondsAnnotation: - gslb.Spec.Strategy.SplitBrainThresholdSeconds = annotationToInt(annotationKey, annotationValue) - } - } - - if strategy == depresolver.FailoverStrategy { - for annotationKey, annotationValue := range a.GetAnnotations() { - if annotationKey == primaryGeoTagAnnotation { - gslb.Spec.Strategy.PrimaryGeoTag = annotationValue - } - } - if gslb.Spec.Strategy.PrimaryGeoTag == "" { - log.Info(). - Str("annotation", primaryGeoTagAnnotation). - Str("gslb", gslb.Name). - Msg("Annotation is missing, skipping Gslb creation...") - return - } - } - - err = controllerutil.SetControllerReference(ingressToReuse, gslb, r.Scheme) - if err != nil { - log.Err(err). - Str("ingress", ingressToReuse.Name). - Str("gslb", gslb.Name). - Msg("Cannot set the Ingress as the owner of the Gslb") - } - - log.Info(). - Str("gslb", gslb.Name). - Msg("Creating new Gslb out of Ingress annotation") - err = c.Create(context.Background(), gslb) - if err != nil { - log.Err(err).Msg("Glsb creation failed") - } - } - - ingressMapHandler := handler.EnqueueRequestsFromMapFunc( - func(a client.Object) []reconcile.Request { - for annotationKey, annotationValue := range a.GetAnnotations() { - if annotationKey == strategyAnnotation { - switch annotationValue { - case depresolver.RoundRobinStrategy: - createGslbFromIngress(annotationKey, annotationValue, a, depresolver.RoundRobinStrategy) - case depresolver.FailoverStrategy: - createGslbFromIngress(annotationKey, annotationValue, a, depresolver.FailoverStrategy) - } - } - } - return nil - }) - - return ctrl.NewControllerManagedBy(mgr). - For(&k8gbv1beta1.Gslb{}). - Owns(&netv1.Ingress{}). - Owns(&externaldns.DNSEndpoint{}). - Watches(&source.Kind{Type: &corev1.Endpoints{}}, endpointMapHandler). - Watches(&source.Kind{Type: &netv1.Ingress{}}, ingressMapHandler). - Complete(r) -} diff --git a/controllers/gslb_controller_test.go b/controllers/gslb_controller_reconciliation_test.go similarity index 100% rename from controllers/gslb_controller_test.go rename to controllers/gslb_controller_reconciliation_test.go diff --git a/controllers/gslb_controller_setup.go b/controllers/gslb_controller_setup.go new file mode 100644 index 0000000000..6f58c7206e --- /dev/null +++ b/controllers/gslb_controller_setup.go @@ -0,0 +1,197 @@ +package controllers + +/* +Copyright 2022 The k8gb Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. + +Generated by GoLic, for more details see: https://github.com/AbsaOSS/golic +*/ + +import ( + "context" + "fmt" + "strconv" + + "github.com/k8gb-io/k8gb/controllers/depresolver" + + k8gbv1beta1 "github.com/k8gb-io/k8gb/api/v1beta1" + corev1 "k8s.io/api/core/v1" + netv1 "k8s.io/api/networking/v1" + 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" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" + externaldns "sigs.k8s.io/external-dns/endpoint" +) + +// SetupWithManager configures controller manager +func (r *GslbReconciler) SetupWithManager(mgr ctrl.Manager) error { + // Figure out Gslb resource name to Reconcile when non controlled Name is updated + + endpointMapHandler := handler.EnqueueRequestsFromMapFunc( + func(a client.Object) []reconcile.Request { + gslbList := &k8gbv1beta1.GslbList{} + opts := []client.ListOption{ + client.InNamespace(a.GetNamespace()), + } + c := mgr.GetClient() + err := c.List(context.TODO(), gslbList, opts...) + if err != nil { + log.Info().Msg("Can't fetch gslb objects") + return nil + } + gslbName := "" + for _, gslb := range gslbList.Items { + for _, rule := range gslb.Spec.Ingress.Rules { + for _, path := range rule.HTTP.Paths { + if path.Backend.Service != nil && path.Backend.Service.Name == a.GetName() { + gslbName = gslb.Name + } + } + } + } + if len(gslbName) > 0 { + return []reconcile.Request{ + {NamespacedName: types.NamespacedName{ + Name: gslbName, + Namespace: a.GetNamespace(), + }}, + } + } + return nil + }) + + ingressMapHandler := handler.EnqueueRequestsFromMapFunc( + func(a client.Object) []reconcile.Request { + annotations := a.GetAnnotations() + if annotationValue, found := annotations[strategyAnnotation]; found { + c := mgr.GetClient() + r.createGSLBFromIngress(c, a, annotationValue) + } + return nil + }) + + return ctrl.NewControllerManagedBy(mgr). + For(&k8gbv1beta1.Gslb{}). + Owns(&netv1.Ingress{}). + Owns(&externaldns.DNSEndpoint{}). + Watches(&source.Kind{Type: &corev1.Endpoints{}}, endpointMapHandler). + Watches(&source.Kind{Type: &netv1.Ingress{}}, ingressMapHandler). + Complete(r) +} + +func (r *GslbReconciler) createGSLBFromIngress(c client.Client, a client.Object, strategy string) { + log.Info(). + Str("annotation", fmt.Sprintf("(%s:%s)", strategyAnnotation, strategy)). + Str("ingress", a.GetName()). + Msg("Detected strategy annotation on ingress") + + ingressToReuse := &netv1.Ingress{} + err := c.Get(context.Background(), client.ObjectKey{ + Namespace: a.GetNamespace(), + Name: a.GetName(), + }, ingressToReuse) + if err != nil { + log.Info(). + Str("ingress", a.GetName()). + Msg("Ingress does not exist anymore. Skipping Glsb creation...") + return + } + gslbExist := &k8gbv1beta1.Gslb{} + err = c.Get(context.Background(), client.ObjectKey{ + Namespace: a.GetNamespace(), + Name: a.GetName(), + }, gslbExist) + if err == nil { + log.Info(). + Str("gslb", gslbExist.Name). + Msg("Gslb already exists. Skipping Gslb creation...") + return + } + gslb := &k8gbv1beta1.Gslb{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: a.GetNamespace(), + Name: a.GetName(), + Annotations: a.GetAnnotations(), + }, + Spec: k8gbv1beta1.GslbSpec{ + Ingress: k8gbv1beta1.FromV1IngressSpec(ingressToReuse.Spec), + }, + } + + gslb.Spec.Strategy, err = r.parseStrategy(a.GetAnnotations(), strategy) + if err != nil { + log.Err(err). + Str("gslb", gslbExist.Name). + Msg("can't parse Gslb strategy") + return + } + + err = controllerutil.SetControllerReference(ingressToReuse, gslb, r.Scheme) + if err != nil { + log.Err(err). + Str("ingress", ingressToReuse.Name). + Str("gslb", gslb.Name). + Msg("Cannot set the Ingress as the owner of the Gslb") + } + + log.Info(). + Str("gslb", gslb.Name). + Msg(fmt.Sprintf("Creating a new Gslb out of Ingress with '%s' annotation", strategyAnnotation)) + err = c.Create(context.Background(), gslb) + if err != nil { + log.Err(err).Msg("Glsb creation failed") + } +} + +func (r *GslbReconciler) parseStrategy(annotations map[string]string, strategy string) (result k8gbv1beta1.Strategy, err error) { + toInt := func(k string, v string) (int, error) { + intValue, err := strconv.Atoi(v) + if err != nil { + return -1, fmt.Errorf("can't parse annotation value %s to int for key %s", v, k) + } + return intValue, nil + } + + result = k8gbv1beta1.Strategy{ + Type: strategy, + } + + for annotationKey, annotationValue := range annotations { + switch annotationKey { + case dnsTTLSecondsAnnotation: + if result.DNSTtlSeconds, err = toInt(annotationKey, annotationValue); err != nil { + return result, err + } + case splitBrainThresholdSecondsAnnotation: + if result.SplitBrainThresholdSeconds, err = toInt(annotationKey, annotationValue); err != nil { + return result, err + } + case primaryGeoTagAnnotation: + result.PrimaryGeoTag = annotationValue + } + } + + if strategy == depresolver.FailoverStrategy { + if len(result.PrimaryGeoTag) == 0 { + return result, fmt.Errorf("%s strategy requires annotation %s", depresolver.FailoverStrategy, primaryGeoTagAnnotation) + } + } + + return result, nil +} diff --git a/controllers/gslb_controller_setup_test.go b/controllers/gslb_controller_setup_test.go new file mode 100644 index 0000000000..b8d82a0c7b --- /dev/null +++ b/controllers/gslb_controller_setup_test.go @@ -0,0 +1,19 @@ +package controllers + +/* +Copyright 2022 The k8gb Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. + +Generated by GoLic, for more details see: https://github.com/AbsaOSS/golic +*/