From fb6ce75eaef1fcbc38f7ecf32182961ebbd58982 Mon Sep 17 00:00:00 2001 From: Yang Le Date: Tue, 15 Aug 2023 13:06:15 +0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20skip=20sync=20of=20appliedmanife?= =?UTF-8?q?stwork=20if=20work=20is=20not=20applied=20yet=20(#244)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Yang Le --- .../appliedmanifestwork_controller.go | 10 ++++++ .../appliedmanifestwork_controller_test.go | 34 ++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/pkg/work/spoke/controllers/appliedmanifestcontroller/appliedmanifestwork_controller.go b/pkg/work/spoke/controllers/appliedmanifestcontroller/appliedmanifestwork_controller.go index bcac940c5..21717a5e5 100644 --- a/pkg/work/spoke/controllers/appliedmanifestcontroller/appliedmanifestwork_controller.go +++ b/pkg/work/spoke/controllers/appliedmanifestcontroller/appliedmanifestwork_controller.go @@ -10,6 +10,7 @@ import ( "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" utilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -99,6 +100,15 @@ func (m *AppliedManifestWorkController) sync(ctx context.Context, controllerCont return nil } + // In a case where a managed cluster switches to a new hub with the same hub hash, the same manifestworks + // will be created for this cluster on the new hub without any condition. Once the work agent connects to + // the new hub, the applied resources of those manifestwork on this managed cluster should not be removed + // before the manifestworks are applied the first time. + if appliedCondition := meta.FindStatusCondition(manifestWork.Status.Conditions, workapiv1.WorkApplied); appliedCondition == nil { + // if the manifestwork has not been applied on the managed cluster yet, do nothing + return nil + } + return m.syncManifestWork(ctx, controllerContext, manifestWork, appliedManifestWork) } diff --git a/pkg/work/spoke/controllers/appliedmanifestcontroller/appliedmanifestwork_controller_test.go b/pkg/work/spoke/controllers/appliedmanifestcontroller/appliedmanifestwork_controller_test.go index 1b688264f..29e78e9af 100644 --- a/pkg/work/spoke/controllers/appliedmanifestcontroller/appliedmanifestwork_controller_test.go +++ b/pkg/work/spoke/controllers/appliedmanifestcontroller/appliedmanifestwork_controller_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/davecgh/go-spew/spew" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -45,6 +46,7 @@ func TestSyncManifestWork(t *testing.T) { cases := []struct { name string + applied bool existingResources []runtime.Object appliedResources []workapiv1.AppliedManifestResourceMeta manifests []workapiv1.ManifestCondition @@ -53,7 +55,17 @@ func TestSyncManifestWork(t *testing.T) { expectedQueueLen int }{ { - name: "skip when no applied resource changed", + name: "skip if the manifestwork has not been applied yet", + validateAppliedManifestWorkActions: func(t *testing.T, actions []clienttesting.Action) { + if len(actions) > 0 { + t.Fatal(spew.Sdump(actions)) + } + }, + expectedDeleteActions: []clienttesting.DeleteActionImpl{}, + }, + { + name: "skip when no applied resource changed", + applied: true, existingResources: []runtime.Object{ spoketesting.NewUnstructuredSecret("ns1", "n1", false, "ns1-n1", *owner), }, @@ -65,7 +77,8 @@ func TestSyncManifestWork(t *testing.T) { expectedDeleteActions: []clienttesting.DeleteActionImpl{}, }, { - name: "delete untracked resources", + name: "delete untracked resources", + applied: true, existingResources: []runtime.Object{ spoketesting.NewUnstructuredSecret("ns1", "n1", false, "ns1-n1", *owner), spoketesting.NewUnstructuredSecret("ns2", "n2", false, "ns2-n2", *owner), @@ -110,7 +123,8 @@ func TestSyncManifestWork(t *testing.T) { }, }, { - name: "requeue work when applied resource for stale manifest is deleting", + name: "requeue work when applied resource for stale manifest is deleting", + applied: true, existingResources: []runtime.Object{ spoketesting.NewUnstructuredSecret("ns1", "n1", false, "ns1-n1", *owner), spoketesting.NewUnstructuredSecret("ns2", "n2", false, "ns2-n2", *owner), @@ -130,7 +144,8 @@ func TestSyncManifestWork(t *testing.T) { expectedQueueLen: 1, }, { - name: "ignore re-created resource", + name: "ignore re-created resource", + applied: true, existingResources: []runtime.Object{ spoketesting.NewUnstructuredSecret("ns3", "n3", false, "ns3-n3-recreated", *owner), spoketesting.NewUnstructuredSecret("ns1", "n1", false, "ns1-n1", *owner), @@ -161,7 +176,8 @@ func TestSyncManifestWork(t *testing.T) { expectedDeleteActions: []clienttesting.DeleteActionImpl{}, }, { - name: "update resource uid", + name: "update resource uid", + applied: true, existingResources: []runtime.Object{ spoketesting.NewUnstructuredSecret("ns1", "n1", false, "ns1-n1", *owner), spoketesting.NewUnstructuredSecret("ns2", "n2", false, "ns2-n2-updated", *owner), @@ -195,6 +211,14 @@ func TestSyncManifestWork(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { testingWork, _ := spoketesting.NewManifestWork(0) + if c.applied { + testingWork.Status.Conditions = []metav1.Condition{ + { + Type: workapiv1.WorkApplied, + Status: metav1.ConditionTrue, + }, + } + } testingAppliedWork := appliedWork.DeepCopy() testingAppliedWork.Status.AppliedResources = c.appliedResources testingWork.Status.ResourceStatus.Manifests = c.manifests