Keep the finalizers unchanged for unstructured (open-cluster-manageme…

…nt-io#128) * Merge finalizer for unstructured Signed-off-by: zhujian <[email protected]> * Keep the finalizers unchanged Signed-off-by: zhujian <[email protected]> * Add test for keeping the finalizers unchanged Signed-off-by: zhujian <[email protected]>
xuezhaojun · Apr 2, 2022 · 9e9e6b2 · 9e9e6b2
1 parent ed589b9
commit 9e9e6b2
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 0 deletions.
diff --git a/pkg/spoke/controllers/manifestcontroller/manifestwork_controller.go b/pkg/spoke/controllers/manifestcontroller/manifestwork_controller.go
@@ -333,6 +333,9 @@ func (m *ManifestWorkController) applyUnstructured(
 	required.SetLabels(existingLabels)
 	required.SetAnnotations(existingAnnotations)
 
+	// Keep the finalizers unchanged
+	required.SetFinalizers(existing.GetFinalizers())
+
 	// Compare and update the unstrcuctured.
 	if !*modified && isSameUnstructured(required, existing) {
 		return existing, false, nil

diff --git a/pkg/spoke/controllers/manifestcontroller/manifestwork_controller_test.go b/pkg/spoke/controllers/manifestcontroller/manifestwork_controller_test.go
@@ -925,6 +925,32 @@ func TestApplyUnstructred(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "set existing finalizer",
+			existingObject: []runtime.Object{
+				func() runtime.Object {
+					obj := spoketesting.NewUnstructured(
+						"v1", "Secret", "ns1", "test", metav1.OwnerReference{APIVersion: "v1", Name: "test1", UID: "testowner1"})
+					obj.SetFinalizers([]string{"foo"})
+					return obj
+				}(),
+			},
+			owner: metav1.OwnerReference{APIVersion: "v1", Name: "test1", UID: "testowner1"},
+			required: func() *unstructured.Unstructured {
+				obj := spoketesting.NewUnstructured(
+					"v1", "Secret", "ns1", "test", metav1.OwnerReference{APIVersion: "v1", Name: "test1", UID: "testowner1"})
+				obj.SetFinalizers([]string{"foo1"})
+				return obj
+			}(),
+			gvr: schema.GroupVersionResource{Version: "v1", Resource: "secrets"},
+			validateActions: func(t *testing.T, actions []clienttesting.Action) {
+				if len(actions) != 1 {
+					t.Errorf("Expect 1 actions, but have %d", len(actions))
+				}
+
+				spoketesting.AssertAction(t, actions[0], "get")
+			},
+		},
 		{
 			name: "nothing to update",
 			existingObject: []runtime.Object{

diff --git a/test/integration/work_test.go b/test/integration/work_test.go
@@ -305,6 +305,70 @@ var _ = ginkgo.Describe("ManifestWork", func() {
 
 		})
 
+		ginkgo.It("should keep the finalizer unchanged of existing CR", func() {
+			util.AssertWorkCondition(work.Namespace, work.Name, hubWorkClient, string(workapiv1.WorkApplied), metav1.ConditionTrue,
+				[]metav1.ConditionStatus{metav1.ConditionTrue, metav1.ConditionTrue}, eventuallyTimeout, eventuallyInterval)
+			util.AssertWorkCondition(work.Namespace, work.Name, hubWorkClient, string(workapiv1.WorkAvailable), metav1.ConditionTrue,
+				[]metav1.ConditionStatus{metav1.ConditionTrue, metav1.ConditionTrue}, eventuallyTimeout, eventuallyInterval)
+
+			var namespaces, names []string
+			for _, obj := range objects {
+				namespaces = append(namespaces, obj.GetNamespace())
+				names = append(names, obj.GetName())
+			}
+
+			util.AssertExistenceOfResources(gvrs, namespaces, names, spokeDynamicClient, eventuallyTimeout, eventuallyInterval)
+			util.AssertAppliedResources(hubHash, work.Name, gvrs, namespaces, names, hubWorkClient, eventuallyTimeout, eventuallyInterval)
+
+			// update object finalizer
+			obj, gvr, err := util.GuestbookCr(o.SpokeClusterName, "guestbook1")
+			gomega.Expect(err).ToNot(gomega.HaveOccurred())
+
+			cr, err := util.GetResource(obj.GetNamespace(), obj.GetName(), gvr, spokeDynamicClient)
+			gomega.Expect(err).ToNot(gomega.HaveOccurred())
+
+			cr.SetFinalizers([]string{"foo"})
+			updated, err := spokeDynamicClient.Resource(gvr).Namespace(obj.GetNamespace()).Update(context.TODO(), cr, metav1.UpdateOptions{})
+			gomega.Expect(err).ToNot(gomega.HaveOccurred())
+			rsvBefore := updated.GetResourceVersion()
+
+			// Update manifestwork
+			obj.SetFinalizers(nil)
+			// set an annotation to make sure the cr will be updated, so that we can check whether the finalizer changest.
+			obj.SetAnnotations(map[string]string{"foo": "bar"})
+			updatework, err := hubWorkClient.WorkV1().ManifestWorks(work.Namespace).Get(context.TODO(), work.Name, metav1.GetOptions{})
+			gomega.Expect(err).ToNot(gomega.HaveOccurred())
+			updatework.Spec.Workload.Manifests[1] = util.ToManifest(obj)
+			_, err = hubWorkClient.WorkV1().ManifestWorks(work.Namespace).Update(context.TODO(), updatework, metav1.UpdateOptions{})
+			gomega.Expect(err).ToNot(gomega.HaveOccurred())
+
+			// wait for annotation merge
+			gomega.Eventually(func() error {
+				cr, err := util.GetResource(obj.GetNamespace(), obj.GetName(), gvr, spokeDynamicClient)
+				if err != nil {
+					return err
+				}
+
+				if len(cr.GetAnnotations()) != 1 {
+					return fmt.Errorf("expect 1 annotation but get %v", cr.GetAnnotations())
+				}
+
+				return nil
+			}, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed())
+
+			// check if finalizer exists
+			cr, err = util.GetResource(obj.GetNamespace(), obj.GetName(), gvr, spokeDynamicClient)
+			gomega.Expect(err).ToNot(gomega.HaveOccurred())
+			gomega.Expect(cr.GetFinalizers()).NotTo(gomega.BeNil())
+			gomega.Expect(cr.GetFinalizers()[0]).To(gomega.Equal("foo"))
+			gomega.Expect(cr.GetResourceVersion()).NotTo(gomega.Equal(rsvBefore))
+
+			// remove the finalizer
+			cr.SetFinalizers(nil)
+			_, err = spokeDynamicClient.Resource(gvr).Namespace(obj.GetNamespace()).Update(context.TODO(), cr, metav1.UpdateOptions{})
+			gomega.Expect(err).ToNot(gomega.HaveOccurred())
+		})
+
 		ginkgo.It("should delete CRD and CR successfully", func() {
 			util.AssertWorkCondition(work.Namespace, work.Name, hubWorkClient, string(workapiv1.WorkApplied), metav1.ConditionTrue,
 				[]metav1.ConditionStatus{metav1.ConditionTrue, metav1.ConditionTrue}, eventuallyTimeout, eventuallyInterval)