add annotation if addon uses InstallStrategy

examples/deploy/addon/helloworld-helm/resources/cluster_role.yaml

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

examples/deploy/addon/helloworld/resources/cluster_role.yaml

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

pkg/addonfactory/addonfactory.go

@@ -81,6 +81,7 @@ func (f *AgentAddonFactory) WithGetValuesFuncs(getValuesFuncs ...GetValuesFunc)
 // WithInstallStrategy defines the installation strategy of the manifests prescribed by Manifests(..).
 // Deprecated: add annotation "addon.open-cluster-management.io/lifecycle: addon-manager" to ClusterManagementAddon
 // and define install strategy in ClusterManagementAddon spec.installStrategy instead.
+// The migration plan refer to https://github.com/open-cluster-management-io/ocm/issues/355.
 func (f *AgentAddonFactory) WithInstallStrategy(strategy *agent.InstallStrategy) *AgentAddonFactory {
 	if strategy.InstallNamespace == "" {
 		strategy.InstallNamespace = AddonDefaultInstallNamespace

pkg/addonmanager/controllers/managementaddon/controller.go

@@ -0,0 +1,95 @@
+package managementaddon
+
+import (
+	"context"
+
+	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/tools/cache"
+	addonapiv1alpha1 "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/addon-framework/pkg/agent"
+	"open-cluster-management.io/addon-framework/pkg/basecontroller/factory"
+)
+
+const (
+	controllerName = "management-addon-controller"
+)
+
+// clusterManagementAddonController reconciles cma on the hub.
+type clusterManagementAddonController struct {
+	addonClient                  addonv1alpha1client.Interface
+	clusterManagementAddonLister addonlisterv1alpha1.ClusterManagementAddOnLister
+	agentAddons                  map[string]agent.AgentAddon
+	addonFilterFunc              factory.EventFilterFunc
+	addonPatcher                 patcher.Patcher[*addonapiv1alpha1.ClusterManagementAddOn,
+		addonapiv1alpha1.ClusterManagementAddOnSpec,
+		addonapiv1alpha1.ClusterManagementAddOnStatus]
+}
+
+func NewManagementAddonController(
+	addonClient addonv1alpha1client.Interface,
+	clusterManagementAddonInformers addoninformerv1alpha1.ClusterManagementAddOnInformer,
+	agentAddons map[string]agent.AgentAddon,
+	addonFilterFunc factory.EventFilterFunc,
+) factory.Controller {
+	syncCtx := factory.NewSyncContext(controllerName)
+
+	c := &clusterManagementAddonController{
+		addonClient:                  addonClient,
+		clusterManagementAddonLister: clusterManagementAddonInformers.Lister(),
+		agentAddons:                  agentAddons,
+		addonFilterFunc:              addonFilterFunc,
+		addonPatcher: patcher.NewPatcher[*addonapiv1alpha1.ClusterManagementAddOn,
+			addonapiv1alpha1.ClusterManagementAddOnSpec,
+			addonapiv1alpha1.ClusterManagementAddOnStatus](addonClient.AddonV1alpha1().ClusterManagementAddOns()),
+	}
+
+	return factory.New().
+		WithSyncContext(syncCtx).
+		WithFilteredEventsInformersQueueKeysFunc(
+			func(obj runtime.Object) []string {
+				key, _ := cache.DeletionHandlingMetaNamespaceKeyFunc(obj)
+				return []string{key}
+			},
+			c.addonFilterFunc, clusterManagementAddonInformers.Informer()).
+		WithSync(c.sync).ToController(controllerName)
+}
+
+func (c *clusterManagementAddonController) sync(ctx context.Context, syncCtx factory.SyncContext, key string) error {
+	_, addonName, err := cache.SplitMetaNamespaceKey(key)
+	if err != nil {
+		// ignore addon whose key is invalid
+		return nil
+	}
+
+	cma, err := c.clusterManagementAddonLister.Get(addonName)
+	if errors.IsNotFound(err) {
+		// addon cloud be deleted, ignore
+		return nil
+	}
+	if err != nil {
+		return err
+	}
+
+	addon := c.agentAddons[cma.GetName()]
+	if addon.GetAgentAddonOptions().InstallStrategy == nil {
+		return nil
+	}
+
+	// If the addon defines install strategy via WithInstallStrategy(), force add annotation "addon.open-cluster-management.io/lifecycle: self" to cma.
+	// The annotation with value "self" will be removed when remove WithInstallStrategy() in addon-framework.
+	// 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{}
+	}
+	cmaCopy.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey] = addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue
+
+	_, err = c.addonPatcher.PatchLabelAnnotations(ctx, cmaCopy, cmaCopy.ObjectMeta, cma.ObjectMeta)
+	return err
+}

pkg/addonmanager/controllers/managementaddon/controller_test.go

@@ -0,0 +1,151 @@
+package managementaddon
+
+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"
+	"open-cluster-management.io/addon-framework/pkg/agent"
+	"open-cluster-management.io/addon-framework/pkg/utils"
+	addonapiv1alpha1 "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"
+	clusterv1 "open-cluster-management.io/api/cluster/v1"
+	"open-cluster-management.io/sdk-go/pkg/patcher"
+)
+
+type testAgent struct {
+	name     string
+	strategy *agent.InstallStrategy
+}
+
+func (t *testAgent) Manifests(cluster *clusterv1.ManagedCluster, addon *addonapiv1alpha1.ManagedClusterAddOn) ([]runtime.Object, error) {
+	return nil, nil
+}
+
+func (t *testAgent) GetAgentAddonOptions() agent.AgentAddonOptions {
+	return agent.AgentAddonOptions{
+		AddonName:       t.name,
+		InstallStrategy: t.strategy,
+	}
+}
+
+func newClusterManagementAddonWithAnnotation(name string, annotations map[string]string) *addonapiv1alpha1.ClusterManagementAddOn {
+	cma := addontesting.NewClusterManagementAddon(name, "", "").Build()
+	cma.Annotations = annotations
+	return cma
+}
+
+func TestReconcile(t *testing.T) {
+	cases := []struct {
+		name                 string
+		cma                  []runtime.Object
+		testaddons           map[string]agent.AgentAddon
+		validateAddonActions func(t *testing.T, actions []clienttesting.Action)
+	}{
+		{
+			name: "add annotation when uses install strategy",
+			cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
+				"test": "test",
+			})},
+			validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
+				addontesting.AssertActions(t, actions, "patch")
+				patch := actions[0].(clienttesting.PatchActionImpl).Patch
+				cma := &addonapiv1alpha1.ClusterManagementAddOn{}
+				err := json.Unmarshal(patch, cma)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				if len(cma.Annotations) != 1 || cma.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey] != addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue {
+					t.Errorf("cma annotation is not correct, expected self but got %s", cma.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey])
+				}
+			},
+			testaddons: map[string]agent.AgentAddon{
+				"test": &testAgent{name: "test", strategy: agent.InstallAllStrategy("test")},
+			},
+		},
+		{
+			name: "override annotation when uses install strategy",
+			cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
+				"test": "test",
+				addonapiv1alpha1.AddonLifecycleAnnotationKey: addonapiv1alpha1.AddonLifecycleAddonManagerAnnotationValue,
+			})},
+			validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
+				addontesting.AssertActions(t, actions, "patch")
+				patch := actions[0].(clienttesting.PatchActionImpl).Patch
+				cma := &addonapiv1alpha1.ClusterManagementAddOn{}
+				err := json.Unmarshal(patch, cma)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				if len(cma.Annotations) != 1 || cma.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey] != addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue {
+					t.Errorf("cma annotation is not correct, expected self but got %s", cma.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey])
+				}
+			},
+			testaddons: map[string]agent.AgentAddon{
+				"test": &testAgent{name: "test", strategy: agent.InstallAllStrategy("test")},
+			},
+		},
+		{
+			name: "no patch annotation if managed by self",
+			cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
+				"test": "test",
+				addonapiv1alpha1.AddonLifecycleAnnotationKey: addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue,
+			})},
+			validateAddonActions: addontesting.AssertNoActions,
+			testaddons: map[string]agent.AgentAddon{
+				"test": &testAgent{name: "test", strategy: agent.InstallAllStrategy("test")},
+			},
+		},
+		{
+			name: "no patch annotation if no install strategy",
+			cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
+				"test": "test",
+				addonapiv1alpha1.AddonLifecycleAnnotationKey: addonapiv1alpha1.AddonLifecycleAddonManagerAnnotationValue,
+			})},
+			validateAddonActions: addontesting.AssertNoActions,
+			testaddons: map[string]agent.AgentAddon{
+				"test": &testAgent{name: "test"},
+			},
+		},
+	}
+
+	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)
+				}
+			}
+
+			controller := clusterManagementAddonController{
+				addonClient:                  fakeAddonClient,
+				clusterManagementAddonLister: addonInformers.Addon().V1alpha1().ClusterManagementAddOns().Lister(),
+				agentAddons:                  c.testaddons,
+				addonFilterFunc:              utils.FilterByAddonName(c.testaddons),
+				addonPatcher: patcher.NewPatcher[*addonapiv1alpha1.ClusterManagementAddOn,
+					addonapiv1alpha1.ClusterManagementAddOnSpec,
+					addonapiv1alpha1.ClusterManagementAddOnStatus](fakeAddonClient.AddonV1alpha1().ClusterManagementAddOns()),
+			}
+
+			for _, obj := range c.cma {
+				cma := obj.(*addonapiv1alpha1.ClusterManagementAddOn)
+				syncContext := addontesting.NewFakeSyncContext(t)
+				err := controller.sync(context.TODO(), syncContext, cma.Name)
+				if err != nil {
+					t.Errorf("expected no error when sync: %v", err)
+				}
+			}
+			c.validateAddonActions(t, fakeAddonClient.Actions())
+		})
+	}
+}

pkg/addonmanager/manager.go

@@ -25,6 +25,7 @@ import (
 	"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/addoninstall"
 	"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/agentdeploy"
 	"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/certificate"
+	"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/managementaddon"
 	"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/managementaddonconfig"
 	"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/registration"
 	"open-cluster-management.io/addon-framework/pkg/agent"
@@ -247,6 +248,16 @@ func (a *addonManager) StartWithInformers(ctx context.Context,
 		a.addonAgents,
 	)
 
+	// 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 := managementaddon.NewManagementAddonController(
+		addonClient,
+		addonInformers.Addon().V1alpha1().ClusterManagementAddOns(),
+		a.addonAgents,
+		utils.FilterByAddonName(a.addonAgents),
+	)
+
 	// This is a duplicate controller in general addon-manager. This should be removed when we
 	// alway enable the addon-manager
 	addonOwnerController := addonowner.NewAddonOwnerController(
@@ -324,6 +335,7 @@ func (a *addonManager) StartWithInformers(ctx context.Context,
 	go deployController.Run(ctx, 1)
 	go registrationController.Run(ctx, 1)
 	go addonInstallController.Run(ctx, 1)
+	go managementAddonController.Run(ctx, 1)
 
 	go addonOwnerController.Run(ctx, 1)
 	if addonConfigController != nil {

pkg/agent/inteface.go

@@ -53,6 +53,7 @@ type AgentAddonOptions struct {
 	// Addon will not be installed automatically until a ManagedClusterAddon is applied to the cluster's
 	// namespace if InstallStrategy is nil.
 	// Deprecated: use installStrategy config in ClusterManagementAddOn API instead
+	// The migration plan refer to https://github.com/open-cluster-management-io/ocm/issues/355.
 	// +optional
 	InstallStrategy *InstallStrategy
 

test/e2e/helloworld_helm_test.go

@@ -146,6 +146,20 @@ var _ = ginkgo.Describe("install/uninstall helloworld helm addons", func() {
 	})
 
 	ginkgo.It("addon should be available", func() {
+		ginkgo.By("Make sure cma annotation is not added since no install strategy defined")
+		gomega.Eventually(func() error {
+			cma, err := hubAddOnClient.AddonV1alpha1().ClusterManagementAddOns().Get(context.Background(), helloWorldHelmAddonName, metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
+
+			if _, exist := cma.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey]; exist {
+				return fmt.Errorf("addon should not have annotation, but get %v", cma.Annotations)
+			}
+
+			return nil
+		}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())
+
 		ginkgo.By("Make sure addon is available and has pre-delete finalizer")
 		gomega.Eventually(func() error {
 			addon, err := hubAddOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.Background(), helloWorldHelmAddonName, metav1.GetOptions{})

test/e2e/helloworld_test.go

@@ -101,6 +101,20 @@ var _ = ginkgo.Describe("install/uninstall helloworld addons", func() {
 	})
 
 	ginkgo.It("addon should be worked", func() {
+		ginkgo.By("Make sure cma annotation managed by self is added")
+		gomega.Eventually(func() error {
+			cma, err := hubAddOnClient.AddonV1alpha1().ClusterManagementAddOns().Get(context.Background(), addonName, metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
+
+			if cma.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey] != addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue {
+				return fmt.Errorf("addon should have annotation, but get %v", cma.Annotations)
+			}
+
+			return nil
+		}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())
+
 		ginkgo.By("Make sure addon is available")
 		gomega.Eventually(func() error {
 			addon, err := hubAddOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.Background(), addonName, metav1.GetOptions{})