From e6617bddd749dec3e4b2ce76e80b120f1607cd2b Mon Sep 17 00:00:00 2001 From: Akos Eros Date: Mon, 4 May 2026 15:17:07 +0200 Subject: [PATCH] fix: Add configmap watch instead of owns Remove ownership of configmap, and watch the patterns operator configmap Trigger reconcile on configmap changes. Create configmap during operator setup to help use cases where it needs modification, also create it during reconcile loop if it does not exist. --- cmd/main.go | 13 ++- internal/controller/console/catalog.go | 21 ++-- internal/controller/console/catalog_test.go | 31 +++-- internal/controller/pattern_controller.go | 109 +++++++++--------- .../controller/patterns_operator_config.go | 40 +++++++ 5 files changed, 137 insertions(+), 77 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 0dec97172..b8e12b11b 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -27,6 +27,7 @@ import ( // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. + _ "k8s.io/client-go/plugin/pkg/client/auth" k8sruntime "k8s.io/apimachinery/pkg/runtime" @@ -105,7 +106,17 @@ func main() { if err := console.EnablePlugin(ctx, mgr.GetClient()); err != nil { setupLog.Error(err, "unable to enable console plugin") } - if err := console.CreateOrUpdateCatalog(ctx, mgr.GetClient(), mgr.GetAPIReader()); err != nil { + cm, err := controllers.GetPatternsOperatorConfigMap(ctx, mgr.GetClient()) + if err != nil { + setupLog.Error(err, "unable to get operator configmap") + } + if cm == nil { + cm, err = controllers.CreatePatternsOperatorConfigMap(ctx, mgr.GetClient()) + if err != nil { + setupLog.Error(err, "unable to create operator configmap") + } + } + if err := console.CreateOrUpdateCatalog(ctx, mgr.GetClient(), cm); err != nil { setupLog.Error(err, "unable to create/update catalog deployment") } return nil diff --git a/internal/controller/console/catalog.go b/internal/controller/console/catalog.go index 23f27651a..72ff0ee07 100644 --- a/internal/controller/console/catalog.go +++ b/internal/controller/console/catalog.go @@ -55,11 +55,12 @@ const ( // CreateOrUpdateCatalog creates or updates the pattern-ui-catalog ConfigMap, Service, // and Deployment. If the operator ConfigMap contains a "catalog.image" key, that // image is used instead of the built-in default. -func CreateOrUpdateCatalog(ctx context.Context, cl client.Client, reader client.Reader) error { +// func CreateOrUpdateCatalog(ctx context.Context, cl client.Client, reader client.Reader) error { +func CreateOrUpdateCatalog(ctx context.Context, cl client.Client, operatorConfigMap *corev1.ConfigMap) error { logger := log.FromContext(ctx).WithName("catalog") ns := getDeploymentNamespace() - image := getCatalogImage(ctx, reader) + image := getCatalogImage(operatorConfigMap) logger.Info("using catalog image", "image", image) if err := createOrUpdateCatalogConfigMap(ctx, cl, ns); err != nil { @@ -77,17 +78,13 @@ func CreateOrUpdateCatalog(ctx context.Context, cl client.Client, reader client. // getCatalogImage reads the optional "catalog.image" override from the operator // ConfigMap. Returns the default image when the key is absent or empty. -func getCatalogImage(ctx context.Context, reader client.Reader) string { - var cm corev1.ConfigMap - if err := reader.Get(ctx, client.ObjectKey{ - Namespace: getDeploymentNamespace(), - Name: operatorConfigMap, - }, &cm); err != nil { - return CatalogDefaultImage - } - if image := cm.Data["catalog.image"]; image != "" { - return image +func getCatalogImage(operatorConfigMap *corev1.ConfigMap) string { + if operatorConfigMap != nil { + if image, ok := operatorConfigMap.Data["catalog.image"]; ok && image != "" { + return image + } } + return CatalogDefaultImage } diff --git a/internal/controller/console/catalog_test.go b/internal/controller/console/catalog_test.go index ed0f9911c..cc2d3303a 100644 --- a/internal/controller/console/catalog_test.go +++ b/internal/controller/console/catalog_test.go @@ -53,7 +53,7 @@ var _ = Describe("CreateOrUpdateCatalog", func() { }) It("should create a deployment with the default image", func() { - Expect(CreateOrUpdateCatalog(ctx, cl, cl)).To(Succeed()) + Expect(CreateOrUpdateCatalog(ctx, cl, cm)).To(Succeed()) deploy := &appsv1.Deployment{} Expect(cl.Get(ctx, client.ObjectKey{Namespace: defaultNamespace, Name: CatalogDeploymentName}, deploy)).To(Succeed()) @@ -68,7 +68,7 @@ var _ = Describe("CreateOrUpdateCatalog", func() { }) It("should create a deployment with the overridden image", func() { - Expect(CreateOrUpdateCatalog(ctx, cl, cl)).To(Succeed()) + Expect(CreateOrUpdateCatalog(ctx, cl, cm)).To(Succeed()) deploy := &appsv1.Deployment{} Expect(cl.Get(ctx, client.ObjectKey{Namespace: defaultNamespace, Name: CatalogDeploymentName}, deploy)).To(Succeed()) @@ -83,14 +83,14 @@ var _ = Describe("CreateOrUpdateCatalog", func() { }) It("should update the deployment image", func() { - Expect(CreateOrUpdateCatalog(ctx, cl, cl)).To(Succeed()) + Expect(CreateOrUpdateCatalog(ctx, cl, cm)).To(Succeed()) // Change the override cm.Data["catalog.image"] = "custom-catalog:v4" Expect(cl.Update(ctx, cm)).To(Succeed()) // Second call updates - Expect(CreateOrUpdateCatalog(ctx, cl, cl)).To(Succeed()) + Expect(CreateOrUpdateCatalog(ctx, cl, cm)).To(Succeed()) deploy := &appsv1.Deployment{} Expect(cl.Get(ctx, client.ObjectKey{Namespace: defaultNamespace, Name: CatalogDeploymentName}, deploy)).To(Succeed()) @@ -105,7 +105,7 @@ var _ = Describe("CreateOrUpdateCatalog", func() { }) It("should create the nginx ConfigMap", func() { - Expect(CreateOrUpdateCatalog(ctx, cl, cl)).To(Succeed()) + Expect(CreateOrUpdateCatalog(ctx, cl, cm)).To(Succeed()) catalogCM := &corev1.ConfigMap{} Expect(cl.Get(ctx, client.ObjectKey{Namespace: defaultNamespace, Name: CatalogConfigMapName}, catalogCM)).To(Succeed()) @@ -113,7 +113,7 @@ var _ = Describe("CreateOrUpdateCatalog", func() { }) It("should create the Service with the correct port", func() { - Expect(CreateOrUpdateCatalog(ctx, cl, cl)).To(Succeed()) + Expect(CreateOrUpdateCatalog(ctx, cl, cm)).To(Succeed()) svc := &corev1.Service{} Expect(cl.Get(ctx, client.ObjectKey{Namespace: defaultNamespace, Name: CatalogServiceName}, svc)).To(Succeed()) @@ -127,7 +127,24 @@ var _ = Describe("CreateOrUpdateCatalog", func() { }) It("should fall back to the default image", func() { - Expect(CreateOrUpdateCatalog(ctx, cl, cl)).To(Succeed()) + Expect(CreateOrUpdateCatalog(ctx, cl, nil)).To(Succeed()) + + deploy := &appsv1.Deployment{} + Expect(cl.Get(ctx, client.ObjectKey{Namespace: defaultNamespace, Name: CatalogDeploymentName}, deploy)).To(Succeed()) + Expect(deploy.Spec.Template.Spec.Containers[0].Image).To(Equal(CatalogDefaultImage)) + }) + }) + + Context("when the operator ConfigMap is empty (no data)", func() { + BeforeEach(func() { + cl = newFakeClient() + + }) + + It("should fall back to the default image", func() { + configmap := &corev1.ConfigMap{} + + Expect(CreateOrUpdateCatalog(ctx, cl, configmap)).To(Succeed()) deploy := &appsv1.Deployment{} Expect(cl.Get(ctx, client.ObjectKey{Namespace: defaultNamespace, Name: CatalogDeploymentName}, deploy)).To(Succeed()) diff --git a/internal/controller/pattern_controller.go b/internal/controller/pattern_controller.go index dba817819..da1b20d5a 100644 --- a/internal/controller/pattern_controller.go +++ b/internal/controller/pattern_controller.go @@ -44,11 +44,13 @@ import ( "k8s.io/client-go/dynamic" "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" crcontroller "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" klog "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" @@ -153,20 +155,22 @@ func (r *PatternReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct var patternsOperatorConfig PatternsOperatorConfig - // We try to get the configuration ConfigMap. The method getPatternsOperatorConfigMap returns nil, nil if the ConfigMap doesn't exist - configCM, err := r.getPatternsOperatorConfigMap() + // We try to get the configuration ConfigMap. The method GetPatternsOperatorConfigMap returns nil, nil if the ConfigMap doesn't exist + operatorConfigMap, err := GetPatternsOperatorConfigMap(ctx, r.Client) if err != nil { return r.actionPerformed(instance, "failed to get the configuration ConfigMap", err) } - if configCM == nil { // If the ConfigMap doesn't exist, we create it - if err = r.createPatternsOperatorConfigMap(instance); err != nil { + if operatorConfigMap == nil { // If the ConfigMap doesn't exist, we create it + operatorConfigMap, err = CreatePatternsOperatorConfigMap(ctx, r.Client) + if err != nil { return r.actionPerformed(instance, "failed to create the configuration ConfigMap", err) } } else { // If the ConfigMap exists, we set the ownership and get the configuration from Data - if err = r.setPatternsOperatorConfigMapOwnership(configCM, instance); err != nil { - return r.actionPerformed(instance, "failed to set ownership of configuration ConfigMap", err) - } - patternsOperatorConfig = configCM.Data + patternsOperatorConfig = operatorConfigMap.Data + } + + if err := console.CreateOrUpdateCatalog(ctx, r.Client, operatorConfigMap); err != nil { + return r.actionPerformed(instance, "unable to create/update catalog deployment", err) } // Remove the ArgoCD application on deletion @@ -407,7 +411,7 @@ func (r *PatternReconciler) reconcileGitOpsSubscription(qualifiedInstance *api.P if err := controllerutil.RemoveOwnerReference(qualifiedInstance, currentSub, r.Scheme); err == nil { changed = true } - operatorConfigMap, cmErr := r.getPatternsOperatorConfigMap() + operatorConfigMap, cmErr := GetPatternsOperatorConfigMap(context.Background(), r.Client) if cmErr == nil && operatorConfigMap != nil { if err := controllerutil.RemoveOwnerReference(operatorConfigMap, currentSub, r.Scheme); err == nil { changed = true @@ -916,11 +920,49 @@ func (r *PatternReconciler) SetupWithManager(mgr ctrl.Manager) error { var ctrlErr error r.ctrl, ctrlErr = ctrl.NewControllerManagedBy(mgr). For(&api.Pattern{}). - Owns(&corev1.ConfigMap{}). + // Use Watches instead of Owns: EnqueueRequestForOwner runs RESTMapping on the owner ref; failures + // there enqueue nothing and can be hard to spot. We only care about the operator config ConfigMap + // and a singleton Pattern in the operator namespace (enforced by webhook), so map directly. + Watches( + &corev1.ConfigMap{}, + handler.EnqueueRequestsFromMapFunc(r.enqueuePatternForOperatorConfigMap), + builder.WithPredicates(predicate.NewPredicateFuncs(isPatternsOperatorConfigMap)), + ). Build(r) return ctrlErr } +func isPatternsOperatorConfigMap(obj client.Object) bool { + return obj != nil && obj.GetName() == OperatorConfigMap && obj.GetNamespace() == DetectOperatorNamespace() +} + +// enqueuePatternForOperatorConfigMap enqueues reconcile for Pattern(s) in the operator namespace when +// patterns-operator-config changes. Webhook ensures a single Pattern; List is used to avoid owner-ref + RESTMapping. +func (r *PatternReconciler) enqueuePatternForOperatorConfigMap(ctx context.Context, obj client.Object) []reconcile.Request { + if !isPatternsOperatorConfigMap(obj) { + return nil + } + var list api.PatternList + ns := DetectOperatorNamespace() + if err := r.List(ctx, &list, client.InNamespace(ns)); err != nil { + ctrl.Log.Error(err, "failed to list Patterns after operator ConfigMap change", "namespace", ns) + return nil + } + if len(list.Items) == 0 { + return nil + } + out := make([]reconcile.Request, 0, len(list.Items)) + for i := range list.Items { + out = append(out, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: list.Items[i].Namespace, + Name: list.Items[i].Name, + }, + }) + } + return out +} + // startArgoCDWatch dynamically adds a watch on ArgoCD instances so that if // the ArgoCD CR is deleted (e.g. during an upgrade that changes the GitOps // Subscription), the Pattern controller reconciles immediately and recreates it. @@ -1335,53 +1377,6 @@ func (r *PatternReconciler) getLocalGit(p *api.Pattern) (string, error) { return "", nil } -func (r *PatternReconciler) createPatternsOperatorConfigMap(p *api.Pattern) error { - configMap := corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - Kind: "ConfigMap", - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: OperatorConfigMap, - Namespace: DetectOperatorNamespace(), - }, - } - if err := controllerutil.SetControllerReference(p, &configMap, r.Scheme); err != nil { - return err - } - if _, err := r.fullClient.CoreV1().ConfigMaps(DetectOperatorNamespace()).Create(context.Background(), &configMap, metav1.CreateOptions{}); err != nil { - return err - } - return nil -} - -func (r *PatternReconciler) getPatternsOperatorConfigMap() (*corev1.ConfigMap, error) { - cm, err := r.fullClient.CoreV1().ConfigMaps(DetectOperatorNamespace()).Get(context.Background(), OperatorConfigMap, metav1.GetOptions{}) - if err != nil { - if kerrors.IsNotFound(err) { - return nil, nil - } else { - return nil, err - } - } - return cm, nil -} - -func (r *PatternReconciler) setPatternsOperatorConfigMapOwnership(cm *corev1.ConfigMap, p *api.Pattern) error { - controllerRef := metav1.GetControllerOf(cm) - if controllerRef != nil && controllerRef.UID == p.GetUID() && controllerRef.Kind == p.Kind && controllerRef.APIVersion == p.APIVersion { - return nil - } - if err := controllerutil.SetControllerReference(p, cm, r.Scheme); err != nil { - return err - } - _, err := r.fullClient.CoreV1().ConfigMaps(DetectOperatorNamespace()).Update(context.Background(), cm, metav1.UpdateOptions{}) - if err != nil { - return err - } - return nil -} - func DropLocalGitPaths() error { // If there is a completely new local folder, let's remove the old one // User changed the target repo diff --git a/internal/controller/patterns_operator_config.go b/internal/controller/patterns_operator_config.go index 2adf05202..09f1be048 100644 --- a/internal/controller/patterns_operator_config.go +++ b/internal/controller/patterns_operator_config.go @@ -1,7 +1,13 @@ package controllers import ( + "context" "strings" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" ) type PatternsOperatorConfig map[string]string @@ -35,3 +41,37 @@ func (g PatternsOperatorConfig) getBoolValue(k string) bool { return strings.EqualFold(DefaultPatternsOperatorConfig[k], "true") } } + +// Creates the patterns operator configmap +// This will include configuration parameters that +// will allow operator configuration operatorConfigMap corev1.ConfigMap +func CreatePatternsOperatorConfigMap(ctx context.Context, cl client.Client) (*corev1.ConfigMap, error) { + configMap := corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + Kind: "ConfigMap", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: OperatorConfigMap, + Namespace: DetectOperatorNamespace(), + }, + } + if err := cl.Create(ctx, &configMap, &client.CreateOptions{}); err != nil { + return nil, err + } + return &configMap, nil +} + +func GetPatternsOperatorConfigMap(ctx context.Context, cl client.Client) (*corev1.ConfigMap, error) { + configMap := corev1.ConfigMap{} + + err := cl.Get(ctx, client.ObjectKey{Namespace: DetectOperatorNamespace(), Name: OperatorConfigMap}, &configMap, &client.GetOptions{}) + if err != nil { + if errors.IsNotFound(err) { + return nil, nil + } else { + return nil, err + } + } + return &configMap, nil +}