breaking change: do not add lifecycle annotation to cma and default managed by addon-manager (#856)

Signed-off-by: Qing Hao <qhao@redhat.com>

go.mod

@@ -35,7 +35,7 @@ require (
 	k8s.io/klog/v2 v2.130.1
 	k8s.io/kube-aggregator v0.31.4
 	k8s.io/utils v0.0.0-20240921022957-49e7df575cb6
-	open-cluster-management.io/addon-framework v0.11.1-0.20250227151105-aede957fa2ca
+	open-cluster-management.io/addon-framework v0.11.1-0.20250303151103-b2865de5c39b
 	open-cluster-management.io/api v0.15.1-0.20250219064651-4281b7684d9b
 	open-cluster-management.io/sdk-go v0.15.1-0.20241125015855-1536c3970f8f
 	sigs.k8s.io/cluster-inventory-api v0.0.0-20240730014211-ef0154379848

go.sum

@@ -487,8 +487,8 @@ k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 h1:BZqlfIlq5YbRMFko6/PM7F
 k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340/go.mod h1:yD4MZYeKMBwQKVht279WycxKyM84kkAx2DPrTXaeb98=
 k8s.io/utils v0.0.0-20240921022957-49e7df575cb6 h1:MDF6h2H/h4tbzmtIKTuctcwZmY0tY9mD9fNT47QO6HI=
 k8s.io/utils v0.0.0-20240921022957-49e7df575cb6/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
-open-cluster-management.io/addon-framework v0.11.1-0.20250227151105-aede957fa2ca h1:uyAu/0kKHbukKFHnEanHHFLT8qrnHdkn0TKVmARsX8o=
-open-cluster-management.io/addon-framework v0.11.1-0.20250227151105-aede957fa2ca/go.mod h1:3+UAkReHIEyqsDuq0Iv5w+ZRgZr254iehYV/JR2j038=
+open-cluster-management.io/addon-framework v0.11.1-0.20250303151103-b2865de5c39b h1:vEemE32F9iiVvKfFsFEdiyGdDnSb9Cp9Dch2Jkc4Nfg=
+open-cluster-management.io/addon-framework v0.11.1-0.20250303151103-b2865de5c39b/go.mod h1:3+UAkReHIEyqsDuq0Iv5w+ZRgZr254iehYV/JR2j038=
 open-cluster-management.io/api v0.15.1-0.20250219064651-4281b7684d9b h1:1ScdOKBMLbzA/k84P9Z64uSq3sxRclquej3tT1zhsqU=
 open-cluster-management.io/api v0.15.1-0.20250219064651-4281b7684d9b/go.mod h1:9erZEWEn4bEqh0nIX2wA7f/s3KCuFycQdBrPrRzi0QM=
 open-cluster-management.io/sdk-go v0.15.1-0.20241125015855-1536c3970f8f h1:zeC7QrFNarfK2zY6jGtd+mX+yDrQQmnH/J8A7n5Nh38=

pkg/addon/controllers/addonmanagement/controller_test.go

@@ -11,7 +11,6 @@ import (
 	"k8s.io/client-go/tools/cache"
 
 	"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"
 	addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
 	fakeaddon "open-cluster-management.io/api/client/addon/clientset/versioned/fake"
@@ -259,7 +258,7 @@ func TestAddonInstallReconcile(t *testing.T) {
 				placementLister:            clusterInformers.Cluster().V1beta1().Placements().Lister(),
 				placementDecisionLister:    clusterInformers.Cluster().V1beta1().PlacementDecisions().Lister(),
 				managedClusterAddonIndexer: addonInformers.Addon().V1alpha1().ManagedClusterAddOns().Informer().GetIndexer(),
-				addonFilterFunc:            utils.ManagedBySelf(map[string]agent.AgentAddon{"test": nil}),
+				addonFilterFunc:            utils.ManagedByAddonManager,
 			}
 
 			_, _, err = reconcile.reconcile(context.TODO(), c.clusterManagementAddon)

pkg/addon/controllers/addonprogressing/controller_test.go

@@ -12,7 +12,6 @@ import (
 	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"
 	"open-cluster-management.io/api/addon/v1alpha1"
 	addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
@@ -744,7 +743,7 @@ func TestReconcile(t *testing.T) {
 				addonInformers.Addon().V1alpha1().ManagedClusterAddOns(),
 				addonInformers.Addon().V1alpha1().ClusterManagementAddOns(),
 				workInformers.Work().V1().ManifestWorks(),
-				utils.ManagedBySelf(map[string]agent.AgentAddon{"test": nil}),
+				utils.ManagedByAddonManager,
 				recorder,
 			)
 
@@ -1472,7 +1471,7 @@ func TestReconcileHostedAddons(t *testing.T) {
 				addonInformers.Addon().V1alpha1().ManagedClusterAddOns(),
 				addonInformers.Addon().V1alpha1().ClusterManagementAddOns(),
 				workInformers.Work().V1().ManifestWorks(),
-				utils.ManagedBySelf(map[string]agent.AgentAddon{"test": nil}),
+				utils.ManagedByAddonManager,
 				recorder,
 			)
 

pkg/addon/controllers/cmainstallprogression/controller_test.go

@@ -10,7 +10,6 @@ import (
 	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"
@@ -347,7 +346,7 @@ func TestReconcile(t *testing.T) {
 				fakeAddonClient,
 				addonInformers.Addon().V1alpha1().ManagedClusterAddOns(),
 				addonInformers.Addon().V1alpha1().ClusterManagementAddOns(),
-				utils.ManagedBySelf(map[string]agent.AgentAddon{"test": nil}),
+				utils.ManagedByAddonManager,
 				recorder,
 			)
 

pkg/addon/controllers/cmamanagedby/controller.go

@@ -1,75 +0,0 @@
-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

@@ -1,120 +0,0 @@
-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

@@ -27,7 +27,6 @@ import (
 	"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/cmainstallprogression"
-	"open-cluster-management.io/ocm/pkg/addon/controllers/cmamanagedby"
 	addonindex "open-cluster-management.io/ocm/pkg/addon/index"
 )
 
@@ -169,15 +168,6 @@ func RunControllerManagerWithInformers(
 		controllerContext.EventRecorder,
 	)
 
-	// 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(),
@@ -204,7 +194,6 @@ 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/assertion_test.go

@@ -150,10 +150,9 @@ func assertClusterManagementAddOnAnnotations(name string) {
 			return err
 		}
 
-		if actual.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey] != addonapiv1alpha1.AddonLifecycleAddonManagerAnnotationValue {
-			return fmt.Errorf("expected annotation %v to be %v, actual: %v",
+		if _, ok := actual.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey]; ok {
+			return fmt.Errorf("expected annotation %v to be empty, actual: %v",
 				addonapiv1alpha1.AddonLifecycleAnnotationKey,
-				addonapiv1alpha1.AddonLifecycleAddonManagerAnnotationValue,
 				actual.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey])
 		}
 

vendor/modules.txt

@@ -1691,7 +1691,7 @@ k8s.io/utils/pointer
 k8s.io/utils/ptr
 k8s.io/utils/strings/slices
 k8s.io/utils/trace
-# open-cluster-management.io/addon-framework v0.11.1-0.20250227151105-aede957fa2ca
+# open-cluster-management.io/addon-framework v0.11.1-0.20250303151103-b2865de5c39b
 ## explicit; go 1.22.0
 open-cluster-management.io/addon-framework/pkg/addonfactory
 open-cluster-management.io/addon-framework/pkg/addonmanager

vendor/open-cluster-management.io/addon-framework/pkg/utils/helpers.go

@@ -296,39 +296,17 @@ func ManagedByAddonManager(obj interface{}) bool {
 	accessor, _ := meta.Accessor(obj)
 	annotations := accessor.GetAnnotations()
 	if len(annotations) == 0 {
-		return false
+		return true
 	}
 
 	value, ok := annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey]
 	if !ok {
-		return false
+		return true
 	}
 
 	return value == addonapiv1alpha1.AddonLifecycleAddonManagerAnnotationValue
 }
 
-func ManagedBySelf(agentAddons map[string]agent.AgentAddon) func(obj interface{}) bool {
-	return func(obj interface{}) bool {
-		accessor, _ := meta.Accessor(obj)
-		if _, ok := agentAddons[accessor.GetName()]; !ok {
-			return false
-		}
-
-		annotations := accessor.GetAnnotations()
-
-		if len(annotations) == 0 {
-			return true
-		}
-
-		value, ok := annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey]
-		if !ok {
-			return true
-		}
-
-		return value == addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue
-	}
-}
-
 func FilterByAddonName(agentAddons map[string]agent.AgentAddon) func(obj interface{}) bool {
 	return func(obj interface{}) bool {
 		accessor, _ := meta.Accessor(obj)