From fdd607244060ba4d34ba2166a64156e848696143 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Fri, 5 Apr 2024 13:27:06 +0200 Subject: [PATCH] Gateway "kuadrant.io/namespace" annotation owned by a single controller --- controllers/gateway_kuadrant_controller.go | 95 ++++++++++++++-------- controllers/kuadrant_controller.go | 69 ---------------- pkg/library/kuadrant/gatewayapi_utils.go | 19 ++--- pkg/library/kuadrant/kuadrant.go | 4 +- pkg/library/mappers/event_mapper.go | 9 ++ pkg/library/mappers/kuadrant_to_gateway.go | 41 ++++++++++ 6 files changed, 120 insertions(+), 117 deletions(-) create mode 100644 pkg/library/mappers/kuadrant_to_gateway.go diff --git a/controllers/gateway_kuadrant_controller.go b/controllers/gateway_kuadrant_controller.go index b0d25e87d..ed1cb3e17 100644 --- a/controllers/gateway_kuadrant_controller.go +++ b/controllers/gateway_kuadrant_controller.go @@ -26,15 +26,22 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" + "github.com/kuadrant/kuadrant-operator/pkg/library/mappers" "github.com/kuadrant/kuadrant-operator/pkg/reconcilers" ) -// GatewayKuadrantReconciler reconciles Gateway object with kuadrant metadata +// GatewayKuadrantReconciler is responsible of assiging gateways to a kuadrant instances +// Currently only one kuadrant instance is allowed per cluster +// This controller will annotate every gateway in the cluster +// with the namespace of the kuadrant instance +// TODO: After the RFC defined, we might want to get the gw to label/annotate from Kuadrant.Spec or manual labeling/annotation type GatewayKuadrantReconciler struct { *reconcilers.BaseReconciler } @@ -67,7 +74,7 @@ func (r *GatewayKuadrantReconciler) Reconcile(eventCtx context.Context, req ctrl logger.V(1).Info(string(jsonData)) } - err := r.reconcileGatewayKuadrantMetadata(ctx, gw) + err := r.reconcileGatewayWithKuadrantMetadata(ctx, gw) if err != nil { return ctrl.Result{}, err @@ -77,39 +84,15 @@ func (r *GatewayKuadrantReconciler) Reconcile(eventCtx context.Context, req ctrl return ctrl.Result{}, nil } -func (r *GatewayKuadrantReconciler) reconcileGatewayKuadrantMetadata(ctx context.Context, gw *gatewayapiv1.Gateway) error { - updated, err := r.reconcileKuadrantNamespaceAnnotation(ctx, gw) - if err != nil { - return err - } - - if updated { - if err := r.Client().Update(ctx, gw); err != nil { - return err - } - } - - return nil -} - -func (r *GatewayKuadrantReconciler) reconcileKuadrantNamespaceAnnotation(ctx context.Context, gw *gatewayapiv1.Gateway) (bool, error) { +func (r *GatewayKuadrantReconciler) reconcileGatewayWithKuadrantMetadata(ctx context.Context, gw *gatewayapiv1.Gateway) error { logger, err := logr.FromContext(ctx) if err != nil { - return false, err - } - - if kuadrant.IsKuadrantManaged(gw) { - return false, nil + return err } kuadrantList := &kuadrantv1beta1.KuadrantList{} if err := r.Client().List(ctx, kuadrantList); err != nil { - return false, err - } - if len(kuadrantList.Items) == 0 { - // Kuadrant was not found - logger.Info("Kuadrant instance not found in the cluster") - return false, nil + return err } if len(kuadrantList.Items) > 1 { @@ -119,18 +102,66 @@ func (r *GatewayKuadrantReconciler) reconcileKuadrantNamespaceAnnotation(ctx con keys[idx] = client.ObjectKeyFromObject(&kuadrantList.Items[idx]).String() } logger.Info("Multiple kuadrant instances found", "num", len(kuadrantList.Items), "keys", strings.Join(keys[:], ",")) - return false, nil + return nil } - kuadrant.AnnotateObject(gw, kuadrantList.Items[0].Namespace) + if len(kuadrantList.Items) == 0 { + logger.Info("Kuadrant instance not found in the cluster") + return r.removeKuadrantNamespaceAnnotation(ctx, gw) + } + + val, ok := gw.GetAnnotations()[kuadrant.KuadrantNamespaceAnnotation] + if !ok || val != kuadrantList.Items[0].Namespace { + // either the does not exist or is different, hence update + annotations := gw.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + annotations[kuadrant.KuadrantNamespaceAnnotation] = kuadrantList.Items[0].Namespace + gw.SetAnnotations(annotations) + logger.Info("annotate gateway with kuadrant namespace", "namespace", kuadrantList.Items[0].Namespace) + return r.UpdateResource(ctx, gw) + } - return true, nil + return nil +} + +func (r *GatewayKuadrantReconciler) removeKuadrantNamespaceAnnotation(ctx context.Context, gw *gatewayapiv1.Gateway) error { + logger, err := logr.FromContext(ctx) + if err != nil { + return err + } + + if _, ok := gw.GetAnnotations()[kuadrant.KuadrantNamespaceAnnotation]; ok { + delete(gw.Annotations, kuadrant.KuadrantNamespaceAnnotation) + logger.Info("remove gateway annotation with kuadrant namespace") + return r.UpdateResource(ctx, gw) + } + + return nil } // SetupWithManager sets up the controller with the Manager. func (r *GatewayKuadrantReconciler) SetupWithManager(mgr ctrl.Manager) error { + // maps any kuadrant event to gateway event + // on any kuadrant event, one reconciliation request for every gateway in the cluster is created + kuadrantToGatewayEventMapper := mappers.NewKuadrantToGatewayEventMapper( + mappers.WithLogger(r.Logger().WithName("kuadrantToGatewayEventMapper")), + ) + return ctrl.NewControllerManagedBy(mgr). // Gateway Kuadrant controller only cares about the annotations For(&gatewayapiv1.Gateway{}, builder.WithPredicates(predicate.AnnotationChangedPredicate{})). + // Watch for any kuadrant CR being created or deleted + Watches( + &kuadrantv1beta1.Kuadrant{}, + handler.EnqueueRequestsFromMapFunc(kuadrantToGatewayEventMapper.Map), + builder.WithPredicates(predicate.Funcs{ + UpdateFunc: func(event.UpdateEvent) bool { + // The reconciler only cares about creation/deletion events + return false + }, + }), + ). Complete(r) } diff --git a/controllers/kuadrant_controller.go b/controllers/kuadrant_controller.go index 9ba1e01c0..6826b4ccb 100644 --- a/controllers/kuadrant_controller.go +++ b/controllers/kuadrant_controller.go @@ -23,7 +23,6 @@ import ( "github.com/go-logr/logr" authorinov1beta1 "github.com/kuadrant/authorino-operator/api/v1beta1" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" - "golang.org/x/sync/errgroup" iopv1alpha1 "istio.io/istio/operator/pkg/apis/istio/v1alpha1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -36,7 +35,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" maistrav1 "github.com/kuadrant/kuadrant-operator/api/external/maistra/v1" maistrav2 "github.com/kuadrant/kuadrant-operator/api/external/maistra/v2" @@ -44,7 +42,6 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/common" "github.com/kuadrant/kuadrant-operator/pkg/istio" "github.com/kuadrant/kuadrant-operator/pkg/kuadranttools" - "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/log" "github.com/kuadrant/kuadrant-operator/pkg/reconcilers" ) @@ -112,10 +109,6 @@ func (r *KuadrantReconciler) Reconcile(eventCtx context.Context, req ctrl.Reques return ctrl.Result{}, err } - if err := r.removeAnnotationFromGateways(ctx, kObj); err != nil { - return ctrl.Result{}, err - } - logger.Info("removing finalizer") controllerutil.RemoveFinalizer(kObj, kuadrantFinalizer) if err := r.Client().Update(ctx, kObj); client.IgnoreNotFound(err) != nil { @@ -137,10 +130,6 @@ func (r *KuadrantReconciler) Reconcile(eventCtx context.Context, req ctrl.Reques } } - if gwErr := r.reconcileClusterGateways(ctx, kObj); gwErr != nil { - logger.V(1).Error(gwErr, "Reconciling cluster gateways failed") - } - specErr := r.reconcileSpec(ctx, kObj) statusResult, statusErr := r.reconcileStatus(ctx, kObj, specErr) @@ -411,64 +400,6 @@ func buildServiceMeshMember(kObj *kuadrantv1beta1.Kuadrant) *maistrav1.ServiceMe } } -func (r *KuadrantReconciler) reconcileClusterGateways(ctx context.Context, kObj *kuadrantv1beta1.Kuadrant) error { - // TODO: After the RFC defined, we might want to get the gw to label/annotate from Kuadrant.Spec or manual labeling/annotation - gwList := &gatewayapiv1.GatewayList{} - if err := r.Client().List(ctx, gwList); err != nil { - return err - } - errGroup, gctx := errgroup.WithContext(ctx) - - for i := range gwList.Items { - gw := &gwList.Items[i] - if !kuadrant.IsKuadrantManaged(gw) { - kuadrant.AnnotateObject(gw, kObj.Namespace) - errGroup.Go(func() error { - select { - case <-gctx.Done(): - // context cancelled - return nil - default: - if err := r.Client().Update(ctx, gw); err != nil { - return err - } - return nil - } - }) - } - } - - return errGroup.Wait() -} - -func (r *KuadrantReconciler) removeAnnotationFromGateways(ctx context.Context, kObj *kuadrantv1beta1.Kuadrant) error { - gwList := &gatewayapiv1.GatewayList{} - if err := r.Client().List(ctx, gwList); err != nil { - return err - } - errGroup, gctx := errgroup.WithContext(ctx) - - for i := range gwList.Items { - gw := &gwList.Items[i] - errGroup.Go(func() error { - select { - case <-gctx.Done(): - // context cancelled - return nil - default: - // When RFC defined, we could indicate which gateways based on a specific annotation/label - kuadrant.DeleteKuadrantAnnotationFromGateway(gw, kObj.Namespace) - if err := r.Client().Update(ctx, gw); err != nil { - return err - } - return nil - } - }) - } - - return errGroup.Wait() -} - func (r *KuadrantReconciler) reconcileLimitador(ctx context.Context, kObj *kuadrantv1beta1.Kuadrant) error { limitadorKey := client.ObjectKey{Name: common.LimitadorName, Namespace: kObj.Namespace} limitador := &limitadorv1alpha1.Limitador{} diff --git a/pkg/library/kuadrant/gatewayapi_utils.go b/pkg/library/kuadrant/gatewayapi_utils.go index fcaaa17e0..a63759128 100644 --- a/pkg/library/kuadrant/gatewayapi_utils.go +++ b/pkg/library/kuadrant/gatewayapi_utils.go @@ -226,7 +226,7 @@ func GetKuadrantNamespaceFromPolicyTargetRef(ctx context.Context, cli client.Cli } func GetKuadrantNamespaceFromPolicy(p Policy) (string, bool) { - if kuadrantNamespace, isSet := p.GetAnnotations()[KuadrantNamespaceLabel]; isSet { + if kuadrantNamespace, isSet := p.GetAnnotations()[KuadrantNamespaceAnnotation]; isSet { return kuadrantNamespace, true } return "", false @@ -236,7 +236,7 @@ func GetKuadrantNamespace(obj client.Object) (string, error) { if !IsKuadrantManaged(obj) { return "", errors.NewInternalError(fmt.Errorf("object %T is not Kuadrant managed", obj)) } - return obj.GetAnnotations()[KuadrantNamespaceLabel], nil + return obj.GetAnnotations()[KuadrantNamespaceAnnotation], nil } func AnnotateObject(obj client.Object, namespace string) { @@ -244,21 +244,12 @@ func AnnotateObject(obj client.Object, namespace string) { if len(annotations) == 0 { obj.SetAnnotations( map[string]string{ - KuadrantNamespaceLabel: namespace, + KuadrantNamespaceAnnotation: namespace, }, ) } else { - if !IsKuadrantManaged(obj) { - annotations[KuadrantNamespaceLabel] = namespace - obj.SetAnnotations(annotations) - } - } -} - -func DeleteKuadrantAnnotationFromGateway(gw *gatewayapiv1.Gateway, namespace string) { - annotations := gw.GetAnnotations() - if IsKuadrantManaged(gw) && annotations[KuadrantNamespaceLabel] == namespace { - delete(gw.Annotations, KuadrantNamespaceLabel) + annotations[KuadrantNamespaceAnnotation] = namespace + obj.SetAnnotations(annotations) } } diff --git a/pkg/library/kuadrant/kuadrant.go b/pkg/library/kuadrant/kuadrant.go index b01f71d94..f1800067e 100644 --- a/pkg/library/kuadrant/kuadrant.go +++ b/pkg/library/kuadrant/kuadrant.go @@ -7,7 +7,7 @@ import ( ) const ( - KuadrantNamespaceLabel = "kuadrant.io/namespace" + KuadrantNamespaceAnnotation = "kuadrant.io/namespace" ) type Policy interface { @@ -23,6 +23,6 @@ type PolicyList interface { } func IsKuadrantManaged(obj client.Object) bool { - _, isSet := obj.GetAnnotations()[KuadrantNamespaceLabel] + _, isSet := obj.GetAnnotations()[KuadrantNamespaceAnnotation] return isSet } diff --git a/pkg/library/mappers/event_mapper.go b/pkg/library/mappers/event_mapper.go index 0fc8a2a9f..df62f0357 100644 --- a/pkg/library/mappers/event_mapper.go +++ b/pkg/library/mappers/event_mapper.go @@ -3,6 +3,7 @@ package mappers import ( "github.com/go-logr/logr" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" @@ -21,16 +22,24 @@ func WithLogger(logger logr.Logger) MapperOption { }) } +func WithClient(cl client.Client) MapperOption { + return newFuncMapperOption(func(o *MapperOptions) { + o.Client = cl + }) +} + type MapperOption interface { apply(*MapperOptions) } type MapperOptions struct { Logger logr.Logger + Client client.Client } var defaultMapperOptions = MapperOptions{ Logger: logr.Discard(), + Client: fake.NewClientBuilder().Build(), } func newFuncMapperOption(f func(*MapperOptions)) *funcMapperOption { diff --git a/pkg/library/mappers/kuadrant_to_gateway.go b/pkg/library/mappers/kuadrant_to_gateway.go new file mode 100644 index 000000000..4173cf05f --- /dev/null +++ b/pkg/library/mappers/kuadrant_to_gateway.go @@ -0,0 +1,41 @@ +package mappers + +import ( + "context" + "fmt" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" + + kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" + "github.com/kuadrant/kuadrant-operator/pkg/library/utils" +) + +func NewKuadrantToGatewayEventMapper(o ...MapperOption) *KuadrantToGatewayEventMapper { + return &KuadrantToGatewayEventMapper{opts: Apply(o...)} +} + +type KuadrantToGatewayEventMapper struct { + opts MapperOptions +} + +func (k *KuadrantToGatewayEventMapper) Map(ctx context.Context, obj client.Object) []reconcile.Request { + logger := k.opts.Logger.WithValues("object", client.ObjectKeyFromObject(obj)) + + _, ok := obj.(*kuadrantv1beta1.Kuadrant) + if !ok { + logger.Error(fmt.Errorf("%T is not a kuadrant instance", obj), "cannot map") + return []reconcile.Request{} + } + + gwList := &gatewayapiv1.GatewayList{} + if err := k.opts.Client.List(ctx, gwList); err != nil { + logger.Error(err, "failed to list gateways") + return []reconcile.Request{} + } + + return utils.Map(gwList.Items, func(gw gatewayapiv1.Gateway) reconcile.Request { + return reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&gw)} + }) +}