Set a finalizer on ClusterManagementAddOn objects

Ref: https://issues.redhat.com/browse/ACM-8473
Signed-off-by: yiraeChristineKim <[email protected]>

go.mod

@@ -128,7 +128,7 @@ require (
 	gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect
 	gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
-	k8s.io/kms v0.29.0 // indirect
+	k8s.io/kms v0.29.2 // indirect
 	k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00 // indirect
 	sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.28.0 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect

go.sum

@@ -445,8 +445,8 @@ open-cluster-management.io/sdk-go v0.13.1-0.20240416062924-20307e6fe090 h1:zFmHu
 open-cluster-management.io/sdk-go v0.13.1-0.20240416062924-20307e6fe090/go.mod h1:w2OaxtCyegxeyFLU42UQ3oxUz01QdsBQkcHI17T/l48=
 sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.28.0 h1:TgtAeesdhpm2SGwkQasmbeqDo8th5wOBA5h/AjTKA4I=
 sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.28.0/go.mod h1:VHVDI/KrK4fjnV61bE2g3sA7tiETLn8sooImelsCx3Y=
-sigs.k8s.io/controller-runtime v0.17.2 h1:FwHwD1CTUemg0pW2otk7/U5/i5m2ymzvOXdbeGOUvw0=
-sigs.k8s.io/controller-runtime v0.17.2/go.mod h1:+MngTvIQQQhfXtwfdGw/UOQ/aIaqsYywfCINOtwMO/s=
+sigs.k8s.io/controller-runtime v0.17.3 h1:65QmN7r3FWgTxDMz9fvGnO1kbf2nu+acg9p2R9oYYYk=
+sigs.k8s.io/controller-runtime v0.17.3/go.mod h1:N0jpP5Lo7lMTF9aL56Z/B2oWBJjey6StQM0jRbKQXtY=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
 sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4=

pkg/addonmanager/controllers/cmamanagedby/controller.go

@@ -2,18 +2,19 @@ package cmamanagedby
 
 import (
 	"context"
+	"fmt"
 
 	"k8s.io/apimachinery/pkg/api/errors"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/cache"
+	"open-cluster-management.io/addon-framework/pkg/agent"
+	"open-cluster-management.io/addon-framework/pkg/basecontroller/factory"
 	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 (
@@ -80,6 +81,11 @@ func (c *cmaManagedByController) sync(ctx context.Context, syncCtx factory.SyncC
 		return err
 	}
 
+	err = c.handleFinalizer(ctx, cma, addonName)
+	if err != nil {
+		return err
+	}
+
 	// Remove the annotation value "self" since the WithInstallStrategy() is removed in addon-framework.
 	// The migration plan refer to https://github.com/open-cluster-management-io/ocm/issues/355.
 	cmaCopy := cma.DeepCopy()
@@ -92,3 +98,27 @@ func (c *cmaManagedByController) sync(ctx context.Context, syncCtx factory.SyncC
 	_, err = c.addonPatcher.PatchLabelAnnotations(ctx, cmaCopy, cmaCopy.ObjectMeta, cma.ObjectMeta)
 	return err
 }
+
+func (c *cmaManagedByController) handleFinalizer(ctx context.Context, cma *addonapiv1alpha1.ClusterManagementAddOn, addonName string) error {
+	managedClusterAddonList, err := c.addonClient.AddonV1alpha1().ManagedClusterAddOns("").List(ctx, v1.ListOptions{
+		FieldSelector: fmt.Sprintf("metadata.name=%s", addonName),
+	})
+	if err != nil {
+		return err
+	}
+
+	if len(managedClusterAddonList.Items) == 0 {
+		err = c.addonPatcher.RemoveFinalizer(ctx, cma, addonapiv1alpha1.AddonPreDeleteHookFinalizer)
+		if err != nil {
+			return err
+		}
+	} else {
+		// length of managedClusterAddonList.Items > 0
+		_, err = c.addonPatcher.AddFinalizer(ctx, cma, addonapiv1alpha1.AddonPreDeleteHookFinalizer)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}

pkg/addonmanager/controllers/cmamanagedby/controller_test.go

@@ -48,7 +48,7 @@ func TestReconcile(t *testing.T) {
 		{
 			name:                 "no patch annotation if nil",
 			cma:                  []runtime.Object{newClusterManagementAddonWithAnnotation("test", nil)},
-			validateAddonActions: addontesting.AssertNoActions,
+			validateAddonActions: validateWithFinalizerAction,
 			testaddons: map[string]agent.AgentAddon{
 				"test": &testAgent{name: "test"},
 			},
@@ -58,7 +58,7 @@ func TestReconcile(t *testing.T) {
 			cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
 				"test": "test",
 			})},
-			validateAddonActions: addontesting.AssertNoActions,
+			validateAddonActions: validateWithFinalizerAction,
 			testaddons: map[string]agent.AgentAddon{
 				"test": &testAgent{name: "test"},
 			},
@@ -69,7 +69,7 @@ func TestReconcile(t *testing.T) {
 				"test": "test",
 				addonapiv1alpha1.AddonLifecycleAnnotationKey: "xxx",
 			})},
-			validateAddonActions: addontesting.AssertNoActions,
+			validateAddonActions: validateWithFinalizerAction,
 			testaddons: map[string]agent.AgentAddon{
 				"test": &testAgent{name: "test"},
 			},
@@ -81,8 +81,9 @@ func TestReconcile(t *testing.T) {
 				addonapiv1alpha1.AddonLifecycleAnnotationKey: addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue,
 			})},
 			validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
-				addontesting.AssertActions(t, actions, "patch")
-				patch := actions[0].(clienttesting.PatchActionImpl).Patch
+				addontesting.AssertActions(t, actions, "patch", "patch")
+				// action[0] is setting finalizer. Ignore this action
+				patch := actions[1].(clienttesting.PatchActionImpl).Patch
 				cma := &addonapiv1alpha1.ClusterManagementAddOn{}
 				err := json.Unmarshal(patch, cma)
 				if err != nil {
@@ -132,3 +133,8 @@ func TestReconcile(t *testing.T) {
 		})
 	}
 }
+
+// Expect that the incoming test case has only one action that is setting finalizer
+func validateWithFinalizerAction(t *testing.T, actions []clienttesting.Action) {
+	addontesting.AssertActions(t, actions, "patch")
+}

test/integration/kube/agent_deploy_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -145,17 +146,25 @@ var _ = ginkgo.Describe("Agent deploy", func() {
 		cma, err = hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().Create(context.Background(),
 			cma, metav1.CreateOptions{})
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
-
 	})
 
 	ginkgo.AfterEach(func() {
+		err = hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().Delete(context.Background(),
+			testAddonImpl.name, metav1.DeleteOptions{})
+		gomega.Expect(err).ToNot(gomega.HaveOccurred())
+		gomega.Eventually(func() bool {
+			_, err := hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().
+				Get(context.Background(), testAddonImpl.name, metav1.GetOptions{})
+			if errors.IsNotFound(err) {
+				return true
+			}
+
+			return false
+		}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue(), "ClusterManagementAddOns should be deleted")
 		err = hubKubeClient.CoreV1().Namespaces().Delete(context.Background(), managedClusterName, metav1.DeleteOptions{})
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 		err = hubClusterClient.ClusterV1().ManagedClusters().Delete(context.Background(), managedClusterName, metav1.DeleteOptions{})
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
-		err = hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().Delete(context.Background(),
-			testAddonImpl.name, metav1.DeleteOptions{})
-		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 	})
 
 	ginkgo.It("Should deploy agent when cma is managed by self successfully", func() {

test/integration/kube/agent_hook_deploy_test.go

@@ -101,13 +101,22 @@ var _ = ginkgo.Describe("Agent hook deploy", func() {
 	})
 
 	ginkgo.AfterEach(func() {
+		err = hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().Delete(context.Background(),
+			testAddonImpl.name, metav1.DeleteOptions{})
+		gomega.Expect(err).ToNot(gomega.HaveOccurred())
+		gomega.Eventually(func() bool {
+			_, err := hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().
+				Get(context.Background(), testAddonImpl.name, metav1.GetOptions{})
+			if errors.IsNotFound(err) {
+				return true
+			}
+
+			return false
+		}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue(), "ClusterManagementAddOns should be deleted")
 		err = hubKubeClient.CoreV1().Namespaces().Delete(context.Background(), managedClusterName, metav1.DeleteOptions{})
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 		err = hubClusterClient.ClusterV1().ManagedClusters().Delete(context.Background(), managedClusterName, metav1.DeleteOptions{})
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
-		err = hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().Delete(context.Background(),
-			testAddonImpl.name, metav1.DeleteOptions{})
-		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 	})
 
 	ginkgo.It("Should install and uninstall agent successfully", func() {
@@ -261,7 +270,5 @@ var _ = ginkgo.Describe("Agent hook deploy", func() {
 			}
 			return fmt.Errorf("addon is expceted to be deleted")
 		}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())
-
 	})
-
 })

test/integration/kube/agent_hosting_deploy_test.go

@@ -118,6 +118,18 @@ var _ = ginkgo.Describe("Agent deploy", func() {
 	})
 
 	ginkgo.AfterEach(func() {
+		err = hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().Delete(context.Background(),
+			testHostedAddonImpl.name, metav1.DeleteOptions{})
+		gomega.Expect(err).ToNot(gomega.HaveOccurred())
+		gomega.Eventually(func() bool {
+			_, err := hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().
+				Get(context.Background(), testHostedAddonImpl.name, metav1.GetOptions{})
+			if errors.IsNotFound(err) {
+				return true
+			}
+
+			return false
+		}, eventuallyTimeout*100, eventuallyInterval).Should(gomega.BeTrue(), "ClusterManagementAddOns should be deleted")
 		err = hubKubeClient.CoreV1().Namespaces().Delete(
 			context.Background(), managedClusterName, metav1.DeleteOptions{})
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
@@ -131,10 +143,6 @@ var _ = ginkgo.Describe("Agent deploy", func() {
 		err = hubClusterClient.ClusterV1().ManagedClusters().Delete(
 			context.Background(), hostingClusterName, metav1.DeleteOptions{})
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
-
-		err = hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().Delete(context.Background(),
-			testHostedAddonImpl.name, metav1.DeleteOptions{})
-		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 	})
 
 	ginkgo.It("Should deploy and delete agent on hosting cluster successfully", func() {
@@ -477,5 +485,4 @@ var _ = ginkgo.Describe("Agent deploy", func() {
 		}
 		testHostedAddonImpl.hostInfoFn = constants.GetHostedModeInfo
 	})
-
 })

test/integration/kube/agent_hosting_hook_deploy_test.go

@@ -74,7 +74,7 @@ var _ = ginkgo.Describe("Agent hook deploy", func() {
 	var managedClusterName, hostingClusterName string
 	var err error
 	var hostingManifestWorkName string
-	var hostingJobCompleteValue = "True"
+	hostingJobCompleteValue := "True"
 	var cma *addonapiv1alpha1.ClusterManagementAddOn
 	ginkgo.BeforeEach(func() {
 		suffix := rand.String(5)
@@ -121,6 +121,18 @@ var _ = ginkgo.Describe("Agent hook deploy", func() {
 	})
 
 	ginkgo.AfterEach(func() {
+		err = hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().Delete(context.Background(),
+			testHostedAddonImpl.name, metav1.DeleteOptions{})
+		gomega.Expect(err).ToNot(gomega.HaveOccurred())
+		gomega.Eventually(func() bool {
+			_, err := hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().
+				Get(context.Background(), testHostedAddonImpl.name, metav1.GetOptions{})
+			if errors.IsNotFound(err) {
+				return true
+			}
+
+			return false
+		}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue(), "ClusterManagementAddOns should be deleted")
 		err = hubKubeClient.CoreV1().Namespaces().Delete(
 			context.Background(), managedClusterName, metav1.DeleteOptions{})
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
@@ -134,9 +146,6 @@ var _ = ginkgo.Describe("Agent hook deploy", func() {
 		err = hubClusterClient.ClusterV1().ManagedClusters().Delete(
 			context.Background(), hostingClusterName, metav1.DeleteOptions{})
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
-		err = hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().Delete(context.Background(),
-			testHostedAddonImpl.name, metav1.DeleteOptions{})
-		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 	})
 
 	ginkgo.It("Should install and uninstall agent successfully", func() {

test/integration/kube/cluster_management_addon_test.go

@@ -9,8 +9,10 @@ import (
 	certificatesv1 "k8s.io/api/certificates/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/util/rand"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
 	clusterv1 "open-cluster-management.io/api/cluster/v1"
@@ -44,7 +46,6 @@ var _ = ginkgo.Describe("ClusterManagementAddon", func() {
 				SignerName: certificatesv1.KubeAPIServerClientSignerName,
 			},
 		}
-
 	})
 
 	ginkgo.AfterEach(func() {
@@ -97,5 +98,71 @@ var _ = ginkgo.Describe("ClusterManagementAddon", func() {
 
 		err = hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().Delete(context.Background(), testAddonImpl.name, metav1.DeleteOptions{})
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
+
+		gomega.Eventually(func() bool {
+			_, err := hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().
+				Get(context.Background(), testAddonImpl.name, metav1.GetOptions{})
+			if errors.IsNotFound(err) {
+				return true
+			}
+
+			return false
+		}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue(), "ClusterManagementAddOns should be deleted")
+	})
+	ginkgo.It("Should wait until managedclusteraddon cleaned successfully", func() {
+		const cmaFinalizer = "cma.open-cluster-management.io/cma-pre-delete"
+		// Create clustermanagement addon
+		cma := &addonapiv1alpha1.ClusterManagementAddOn{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: testAddonImpl.name,
+			},
+			Spec: addonapiv1alpha1.ClusterManagementAddOnSpec{
+				InstallStrategy: addonapiv1alpha1.InstallStrategy{
+					Type: addonapiv1alpha1.AddonInstallStrategyManual,
+				},
+			},
+		}
+		cma, err := hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().Create(context.Background(), cma, metav1.CreateOptions{})
+		gomega.Expect(err).ToNot(gomega.HaveOccurred())
+
+		// Create managed cluster addon
+		addon := &addonapiv1alpha1.ManagedClusterAddOn{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: testAddonImpl.name,
+			},
+			Spec: addonapiv1alpha1.ManagedClusterAddOnSpec{
+				InstallNamespace: "test",
+			},
+		}
+		createManagedClusterAddOnwithOwnerRefs(managedClusterName, addon, cma)
+
+		cma, err = hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().Get(context.Background(), testAddonImpl.name, metav1.GetOptions{})
+		gomega.Expect(err).ToNot(gomega.HaveOccurred())
+		gomega.Expect(controllerutil.ContainsFinalizer(cma, addonapiv1alpha1.AddonPreDeleteHookFinalizer)).
+			Should(gomega.BeTrue(), "The ClusterManagementAddOns should have the AddonPreDeleteHook Finalizer")
+
+		err = hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().
+			Delete(context.Background(), testAddonImpl.name, metav1.DeleteOptions{})
+		gomega.Expect(err).ToNot(gomega.HaveOccurred(), "Deleting ClusterManagementAddOns should be successful")
+
+		gomega.Eventually(func() bool {
+			_, err := hubAddonClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).
+				Get(context.Background(), testAddonImpl.name, metav1.GetOptions{})
+			if errors.IsNotFound(err) {
+				return true
+			}
+
+			return false
+		}, eventuallyTimeout*2, eventuallyInterval).Should(gomega.BeTrue(), "ManagedClusterAddOns should be deleted")
+
+		gomega.Eventually(func() bool {
+			_, err := hubAddonClient.AddonV1alpha1().ClusterManagementAddOns().
+				Get(context.Background(), testAddonImpl.name, metav1.GetOptions{})
+			if errors.IsNotFound(err) {
+				return true
+			}
+
+			return false
+		}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue(), "ClusterManagementAddOns should be deleted")
 	})
 })