🌱 set cma managed by addon-manager if not configured

manifests/cluster-manager/hub/cluster-manager-addon-manager-clusterrole.yaml

@@ -35,7 +35,7 @@ rules:
   verbs: ["update", "patch"]
 - apiGroups: ["addon.open-cluster-management.io"]
   resources: ["clustermanagementaddons"]
-  verbs: ["get", "list", "watch"]
+  verbs: ["patch", "get", "list", "watch"]
 - apiGroups: ["addon.open-cluster-management.io"]
   resources: ["managedclusteraddons"]
   verbs: ["get", "list", "watch", "create", "update", "delete"]

pkg/addon/controllers/addonconfiguration/mgmt_addon_progressing_reconciler.go → pkg/addon/controllers/addonconfiguration/cma_progressing_reconciler.go

@@ -13,12 +13,12 @@ import (
 	"open-cluster-management.io/sdk-go/pkg/patcher"
 )
 
-type clusterManagementAddonProgressingReconciler struct {
+type cmaProgressingReconciler struct {
 	patcher patcher.Patcher[
 		*addonv1alpha1.ClusterManagementAddOn, addonv1alpha1.ClusterManagementAddOnSpec, addonv1alpha1.ClusterManagementAddOnStatus]
 }
 
-func (d *clusterManagementAddonProgressingReconciler) reconcile(
+func (d *cmaProgressingReconciler) reconcile(
 	ctx context.Context, cma *addonv1alpha1.ClusterManagementAddOn, graph *configurationGraph) (*addonv1alpha1.ClusterManagementAddOn, reconcileState, error) {
 	var errs []error
 	cmaCopy := cma.DeepCopy()

pkg/addon/controllers/addonconfiguration/mgmt_addon_progressing_reconciler_test.go → pkg/addon/controllers/addonconfiguration/cma_progressing_reconciler_test.go

@@ -653,7 +653,7 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) {
 				managedClusterAddonIndexer:   addonInformers.Addon().V1alpha1().ManagedClusterAddOns().Informer().GetIndexer(),
 			}
 
-			reconcile := &clusterManagementAddonProgressingReconciler{
+			reconcile := &cmaProgressingReconciler{
 				patcher.NewPatcher[
 					*addonv1alpha1.ClusterManagementAddOn, addonv1alpha1.ClusterManagementAddOnSpec, addonv1alpha1.ClusterManagementAddOnStatus](
 					fakeAddonClient.AddonV1alpha1().ClusterManagementAddOns()),

pkg/addon/controllers/addonconfiguration/controller.go

@@ -79,7 +79,7 @@ func NewAddonConfigurationController(
 		&managedClusterAddonConfigurationReconciler{
 			addonClient: addonClient,
 		},
-		&clusterManagementAddonProgressingReconciler{
+		&cmaProgressingReconciler{
 			patcher: patcher.NewPatcher[
 				*addonv1alpha1.ClusterManagementAddOn, addonv1alpha1.ClusterManagementAddOnSpec, addonv1alpha1.ClusterManagementAddOnStatus](
 				addonClient.AddonV1alpha1().ClusterManagementAddOns()),

pkg/addon/controllers/managementaddoninstallprogression/controller.go → pkg/addon/controllers/cmainstallprogression/controller.go

@@ -1,4 +1,4 @@
-package managementaddoninstallprogression
+package cmainstallprogression
 
 import (
 	"context"
@@ -17,40 +17,38 @@ import (
 	"open-cluster-management.io/ocm/pkg/common/queue"
 )
 
-// managementAddonInstallProgressionController reconciles instances of clustermanagementaddon the hub
+// cmaInstallProgressionController reconciles instances of clustermanagementaddon the hub
 // based to update related object and status condition.
-type managementAddonInstallProgressionController struct {
+type cmaInstallProgressionController struct {
 	patcher patcher.Patcher[
 		*addonv1alpha1.ClusterManagementAddOn, addonv1alpha1.ClusterManagementAddOnSpec, addonv1alpha1.ClusterManagementAddOnStatus]
-	managedClusterAddonLister    addonlisterv1alpha1.ManagedClusterAddOnLister
 	clusterManagementAddonLister addonlisterv1alpha1.ClusterManagementAddOnLister
 	addonFilterFunc              factory.EventFilterFunc
 }
 
-func NewManagementAddonInstallProgressionController(
+func NewCMAInstallProgressionController(
 	addonClient addonv1alpha1client.Interface,
 	addonInformers addoninformerv1alpha1.ManagedClusterAddOnInformer,
 	clusterManagementAddonInformers addoninformerv1alpha1.ClusterManagementAddOnInformer,
 	addonFilterFunc factory.EventFilterFunc,
 	recorder events.Recorder,
 ) factory.Controller {
-	c := &managementAddonInstallProgressionController{
+	c := &cmaInstallProgressionController{
 		patcher: patcher.NewPatcher[
 			*addonv1alpha1.ClusterManagementAddOn, addonv1alpha1.ClusterManagementAddOnSpec, addonv1alpha1.ClusterManagementAddOnStatus](
 			addonClient.AddonV1alpha1().ClusterManagementAddOns()),
-		managedClusterAddonLister:    addonInformers.Lister(),
 		clusterManagementAddonLister: clusterManagementAddonInformers.Lister(),
 		addonFilterFunc:              addonFilterFunc,
 	}
 
 	return factory.New().WithInformersQueueKeysFunc(
 		queue.QueueKeyByMetaName,
 		addonInformers.Informer(), clusterManagementAddonInformers.Informer()).
-		WithSync(c.sync).ToController("management-addon-status-controller", recorder)
+		WithSync(c.sync).ToController("cma-install-progression-controller", recorder)
 
 }
 
-func (c *managementAddonInstallProgressionController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
+func (c *cmaInstallProgressionController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
 	addonName := syncCtx.QueueKey()
 	logger := klog.FromContext(ctx)
 	logger.V(4).Info("Reconciling addon", "addonName", addonName)
@@ -64,15 +62,6 @@ func (c *managementAddonInstallProgressionController) sync(ctx context.Context,
 
 	mgmtAddonCopy := mgmtAddon.DeepCopy()
 
-	clusterManagementAddon, err := c.clusterManagementAddonLister.Get(addonName)
-	if errors.IsNotFound(err) {
-		return nil
-	}
-
-	if err != nil {
-		return err
-	}
-
 	// set default config reference
 	mgmtAddonCopy.Status.DefaultConfigReferences = setDefaultConfigReference(mgmtAddonCopy.Spec.SupportedConfigs, mgmtAddonCopy.Status.DefaultConfigReferences)
 
@@ -84,7 +73,7 @@ func (c *managementAddonInstallProgressionController) sync(ctx context.Context,
 	}
 
 	// only update default config references and skip updating install progression for self-managed addon
-	if !c.addonFilterFunc(clusterManagementAddon) {
+	if !c.addonFilterFunc(mgmtAddon) {
 		_, err = c.patcher.PatchStatus(ctx, mgmtAddonCopy, mgmtAddonCopy.Status, mgmtAddon.Status)
 		return err
 	}

pkg/addon/controllers/managementaddoninstallprogression/controller_test.go → pkg/addon/controllers/cmainstallprogression/controller_test.go

@@ -1,4 +1,4 @@
-package managementaddoninstallprogression
+package cmainstallprogression
 
 import (
 	"context"
@@ -241,7 +241,7 @@ func TestReconcile(t *testing.T) {
 			syncContext := testingcommon.NewFakeSyncContext(t, c.syncKey)
 			recorder := syncContext.Recorder()
 
-			controller := NewManagementAddonInstallProgressionController(
+			controller := NewCMAInstallProgressionController(
 				fakeAddonClient,
 				addonInformers.Addon().V1alpha1().ManagedClusterAddOns(),
 				addonInformers.Addon().V1alpha1().ClusterManagementAddOns(),

pkg/addon/controllers/cmamanagedby/controller.go

@@ -0,0 +1,75 @@
+package cmamanagedby
+
+import (
+	"context"
+
+	"github.com/openshift/library-go/pkg/controller/factory"
+	"github.com/openshift/library-go/pkg/operator/events"
+	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/klog/v2"
+
+	addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
+	addonv1alpha1client "open-cluster-management.io/api/client/addon/clientset/versioned"
+	addoninformerv1alpha1 "open-cluster-management.io/api/client/addon/informers/externalversions/addon/v1alpha1"
+	addonlisterv1alpha1 "open-cluster-management.io/api/client/addon/listers/addon/v1alpha1"
+	"open-cluster-management.io/sdk-go/pkg/patcher"
+
+	"open-cluster-management.io/ocm/pkg/common/queue"
+)
+
+// cmaManagedByController reconciles clustermanagementaddon on the hub
+// to update the annotation "addon.open-cluster-management.io/lifecycle" value.
+// It sets the value to "addon-manager" if "self" is not set, which indicate the
+// the installation and upgrade of addon should be handled by the general addon manager.
+type cmaManagedByController struct {
+	patcher patcher.Patcher[
+		*addonv1alpha1.ClusterManagementAddOn, addonv1alpha1.ClusterManagementAddOnSpec, addonv1alpha1.ClusterManagementAddOnStatus]
+	clusterManagementAddonLister addonlisterv1alpha1.ClusterManagementAddOnLister
+}
+
+func NewCMAManagedByController(
+	addonClient addonv1alpha1client.Interface,
+	clusterManagementAddonInformers addoninformerv1alpha1.ClusterManagementAddOnInformer,
+	recorder events.Recorder,
+) factory.Controller {
+	c := &cmaManagedByController{
+		patcher: patcher.NewPatcher[
+			*addonv1alpha1.ClusterManagementAddOn, addonv1alpha1.ClusterManagementAddOnSpec, addonv1alpha1.ClusterManagementAddOnStatus](
+			addonClient.AddonV1alpha1().ClusterManagementAddOns()),
+		clusterManagementAddonLister: clusterManagementAddonInformers.Lister(),
+	}
+
+	return factory.New().WithInformersQueueKeysFunc(
+		queue.QueueKeyByMetaName,
+		clusterManagementAddonInformers.Informer()).
+		WithSync(c.sync).ToController("cma-managed-by-controller", recorder)
+
+}
+
+func (c *cmaManagedByController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
+	addonName := syncCtx.QueueKey()
+	logger := klog.FromContext(ctx)
+	logger.V(4).Info("Reconciling addon", "addonName", addonName)
+
+	cma, err := c.clusterManagementAddonLister.Get(addonName)
+	switch {
+	case errors.IsNotFound(err):
+		return nil
+	case err != nil:
+		return err
+	}
+
+	// If cma annotation "addon.open-cluster-management.io/lifecycle: self" is not set,
+	// force add "addon.open-cluster-management.io/lifecycle: addon-manager" .
+	// The migration plan refer to https://github.com/open-cluster-management-io/ocm/issues/355.
+	cmaCopy := cma.DeepCopy()
+	if cmaCopy.Annotations == nil {
+		cmaCopy.Annotations = map[string]string{}
+	}
+	if cmaCopy.Annotations[addonv1alpha1.AddonLifecycleAnnotationKey] != addonv1alpha1.AddonLifecycleSelfManageAnnotationValue {
+		cmaCopy.Annotations[addonv1alpha1.AddonLifecycleAnnotationKey] = addonv1alpha1.AddonLifecycleAddonManagerAnnotationValue
+	}
+
+	_, err = c.patcher.PatchLabelAnnotations(ctx, cmaCopy, cmaCopy.ObjectMeta, cma.ObjectMeta)
+	return err
+}

pkg/addon/controllers/cmamanagedby/controller_test.go

@@ -0,0 +1,120 @@
+package cmamanagedby
+
+import (
+	"context"
+	"encoding/json"
+	"testing"
+	"time"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	clienttesting "k8s.io/client-go/testing"
+
+	"open-cluster-management.io/addon-framework/pkg/addonmanager/addontesting"
+	addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
+	fakeaddon "open-cluster-management.io/api/client/addon/clientset/versioned/fake"
+	addoninformers "open-cluster-management.io/api/client/addon/informers/externalversions"
+
+	testingcommon "open-cluster-management.io/ocm/pkg/common/testing"
+)
+
+func newClusterManagementAddonWithAnnotation(name string, annotations map[string]string) *addonv1alpha1.ClusterManagementAddOn {
+	cma := addontesting.NewClusterManagementAddon(name, "", "").Build()
+	cma.Annotations = annotations
+	return cma
+}
+
+func TestReconcile(t *testing.T) {
+	cases := []struct {
+		name                 string
+		syncKey              string
+		cma                  []runtime.Object
+		validateAddonActions func(t *testing.T, actions []clienttesting.Action)
+	}{
+		{
+			name:    "add annotation if no annotation",
+			syncKey: "test",
+			cma:     []runtime.Object{newClusterManagementAddonWithAnnotation("test", nil)},
+			validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
+				addontesting.AssertActions(t, actions, "patch")
+				patch := actions[0].(clienttesting.PatchActionImpl).Patch
+				cma := &addonv1alpha1.ClusterManagementAddOn{}
+				err := json.Unmarshal(patch, cma)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				if len(cma.Annotations) != 1 || cma.Annotations[addonv1alpha1.AddonLifecycleAnnotationKey] != addonv1alpha1.AddonLifecycleAddonManagerAnnotationValue {
+					t.Errorf("cma annotation is not correct, expected addon-manager but got %s", cma.Annotations[addonv1alpha1.AddonLifecycleAnnotationKey])
+				}
+			},
+		},
+		{
+			name:    "add annotation if addon.open-cluster-management.io/lifecycle is empty",
+			syncKey: "test",
+			cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
+				"test": "test",
+				addonv1alpha1.AddonLifecycleAnnotationKey: "",
+			})},
+			validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
+				addontesting.AssertActions(t, actions, "patch")
+				patch := actions[0].(clienttesting.PatchActionImpl).Patch
+				cma := &addonv1alpha1.ClusterManagementAddOn{}
+				err := json.Unmarshal(patch, cma)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				if len(cma.Annotations) != 1 || cma.Annotations[addonv1alpha1.AddonLifecycleAnnotationKey] != addonv1alpha1.AddonLifecycleAddonManagerAnnotationValue {
+					t.Errorf("cma annotation is not correct, expected addon-manager but got %s", cma.Annotations[addonv1alpha1.AddonLifecycleAnnotationKey])
+				}
+			},
+		},
+		{
+			name:    "no patch annotation if managed by self",
+			syncKey: "test",
+			cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
+				"test": "test",
+				addonv1alpha1.AddonLifecycleAnnotationKey: addonv1alpha1.AddonLifecycleSelfManageAnnotationValue,
+			})},
+			validateAddonActions: addontesting.AssertNoActions,
+		},
+		{
+			name:    "no patch annotation if managed by addon-manager",
+			syncKey: "test",
+			cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
+				"test": "test",
+				addonv1alpha1.AddonLifecycleAnnotationKey: addonv1alpha1.AddonLifecycleAddonManagerAnnotationValue,
+			})},
+			validateAddonActions: addontesting.AssertNoActions,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			fakeAddonClient := fakeaddon.NewSimpleClientset(c.cma...)
+			addonInformers := addoninformers.NewSharedInformerFactory(fakeAddonClient, 10*time.Minute)
+
+			for _, obj := range c.cma {
+				if err := addonInformers.Addon().V1alpha1().ClusterManagementAddOns().Informer().GetStore().Add(obj); err != nil {
+					t.Fatal(err)
+				}
+			}
+
+			syncContext := testingcommon.NewFakeSyncContext(t, c.syncKey)
+			recorder := syncContext.Recorder()
+
+			controller := NewCMAManagedByController(
+				fakeAddonClient,
+				addonInformers.Addon().V1alpha1().ClusterManagementAddOns(),
+				recorder,
+			)
+
+			err := controller.Sync(context.TODO(), syncContext)
+			if err != nil {
+				t.Errorf("expected no error when sync: %v", err)
+			}
+			c.validateAddonActions(t, fakeAddonClient.Actions())
+
+		})
+	}
+}

pkg/addon/manager.go

@@ -26,7 +26,8 @@ import (
 	"open-cluster-management.io/ocm/pkg/addon/controllers/addonowner"
 	"open-cluster-management.io/ocm/pkg/addon/controllers/addonprogressing"
 	"open-cluster-management.io/ocm/pkg/addon/controllers/addontemplate"
-	"open-cluster-management.io/ocm/pkg/addon/controllers/managementaddoninstallprogression"
+	"open-cluster-management.io/ocm/pkg/addon/controllers/cmainstallprogression"
+	"open-cluster-management.io/ocm/pkg/addon/controllers/cmamanagedby"
 )
 
 func RunManager(ctx context.Context, controllerContext *controllercmd.ControllerContext) error {
@@ -165,7 +166,16 @@ func RunControllerManagerWithInformers(
 		controllerContext.EventRecorder,
 	)
 
-	mgmtAddonInstallProgressionController := managementaddoninstallprogression.NewManagementAddonInstallProgressionController(
+	// This controller is used during migrating addons to be managed by addon-manager.
+	// This should be removed when the migration is done.
+	// The migration plan refer to https://github.com/open-cluster-management-io/ocm/issues/355.
+	managementAddonController := cmamanagedby.NewCMAManagedByController(
+		hubAddOnClient,
+		addonInformers.Addon().V1alpha1().ClusterManagementAddOns(),
+		controllerContext.EventRecorder,
+	)
+
+	mgmtAddonInstallProgressionController := cmainstallprogression.NewCMAInstallProgressionController(
 		hubAddOnClient,
 		addonInformers.Addon().V1alpha1().ManagedClusterAddOns(),
 		addonInformers.Addon().V1alpha1().ClusterManagementAddOns(),
@@ -188,6 +198,7 @@ func RunControllerManagerWithInformers(
 	go addonConfigurationController.Run(ctx, 2)
 	go addonOwnerController.Run(ctx, 2)
 	go addonProgressingController.Run(ctx, 2)
+	go managementAddonController.Run(ctx, 2)
 	go mgmtAddonInstallProgressionController.Run(ctx, 2)
 	// There should be only one instance of addonTemplateController running, since the addonTemplateController will
 	// start a goroutine for each template-type addon it watches.

test/integration/addon/addon_configs_test.go

@@ -49,6 +49,8 @@ var _ = ginkgo.Describe("AddConfigs", func() {
 		_, err = createClusterManagementAddOn(testAddOnConfigsImpl.name, configDefaultNamespace, configDefaultName)
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 
+		assertClusterManagementAddOnAnnotations(testAddOnConfigsImpl.name)
+
 		// prepare default config
 		configDefaultNS := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: configDefaultNamespace}}
 		_, err = hubKubeClient.CoreV1().Namespaces().Create(context.Background(), configDefaultNS, metav1.CreateOptions{})
@@ -132,9 +134,6 @@ var _ = ginkgo.Describe("AddConfigs", func() {
 		cma, err := hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().Get(context.Background(), testAddOnConfigsImpl.name, metav1.GetOptions{})
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 
-		cma.Annotations = map[string]string{
-			addonapiv1alpha1.AddonLifecycleAnnotationKey: addonapiv1alpha1.AddonLifecycleAddonManagerAnnotationValue,
-		}
 		cma.Spec.InstallStrategy = addonapiv1alpha1.InstallStrategy{
 			Type: addonapiv1alpha1.AddonInstallStrategyPlacements,
 			Placements: []addonapiv1alpha1.PlacementStrategy{