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 +}