From 7937411fb15d35bdac3d6c50c6b1318989ce6acc Mon Sep 17 00:00:00 2001 From: Roke Jung Date: Wed, 23 Sep 2020 12:01:33 -0400 Subject: [PATCH 1/2] Update synchronizer.go --- pkg/synchronizer/kubernetes/synchronizer.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/synchronizer/kubernetes/synchronizer.go b/pkg/synchronizer/kubernetes/synchronizer.go index 10c128101..7f514d422 100644 --- a/pkg/synchronizer/kubernetes/synchronizer.go +++ b/pkg/synchronizer/kubernetes/synchronizer.go @@ -267,7 +267,11 @@ func (sync *KubeSynchronizer) updateResourceByTemplateUnit(ri dynamic.ResourceIn newobj := tplunit.Unstructured.DeepCopy() newobj.SetResourceVersion(obj.GetResourceVersion()) - if overwrite { + // If subscription-admin chooses merge option, remove the typical annotations we add. This will avoid the resources being + // deleted when the subscription is removed. + // If subscription-admin chooses replace option, keep the typical annotations we add. Subscription takes over the resources. + // When the subscription is removed, the resources will be removed too. + if overwrite && merge { // If overwriting someone else's resource, remove annotations like hosting subscription, hostring deployables... etc newobj = utils.RemoveSubAnnotations(newobj) newobj = utils.RemoveSubOwnerRef(newobj) From 1c3503635ad712e9e2d878367344cb349733e28c Mon Sep 17 00:00:00 2001 From: Roke Jung Date: Wed, 23 Sep 2020 13:04:33 -0400 Subject: [PATCH 2/2] Update sync_test.go --- pkg/synchronizer/kubernetes/sync_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/synchronizer/kubernetes/sync_test.go b/pkg/synchronizer/kubernetes/sync_test.go index a06c81cb1..924a79ed6 100644 --- a/pkg/synchronizer/kubernetes/sync_test.go +++ b/pkg/synchronizer/kubernetes/sync_test.go @@ -784,11 +784,10 @@ var _ = Describe("test resource overwrite", func() { // age field should be deleted because the reconcile option was replace Expect(cm.Data["age"]).Should(Equal("")) - // The hosting annotations should not be added when existing resource, that is not owned by the subscription, - // gets overwritten + // If reconcile option is replace, the hosting annotations should added to existing resource, that is not owned by the subscription. cmAnnotations = cm.GetAnnotations() - Expect(cmAnnotations["apps.open-cluster-management.io/hosting-deployable"]).To(Equal("")) - Expect(cmAnnotations["apps.open-cluster-management.io/hosting-subscription"]).To(Equal("")) + Expect(cmAnnotations["apps.open-cluster-management.io/hosting-deployable"]).To(Equal(configMapSharedkey.Namespace + "/" + configMapSharedkey.Name)) + Expect(cmAnnotations["apps.open-cluster-management.io/hosting-subscription"]).To(Equal(configMapSharedkey.Namespace + "/" + configMapSharedkey.Name)) }) It("resource owned by others can be merged by subscription", func() { @@ -865,7 +864,7 @@ var _ = Describe("test resource overwrite", func() { // age field should be kept because the reconcile option was merge Expect(cm.Data["age"]).Should(Equal("19")) - // The hosting annotations should not be added when existing resource, that is not owned by the subscription, + // With merge option, the hosting annotations should not be added when existing resource, that is not owned by the subscription, // gets overwritten cmAnnotations = cm.GetAnnotations() Expect(cmAnnotations["apps.open-cluster-management.io/hosting-deployable"]).To(Equal(""))