diff --git a/pkg/spoke/controllers/finalizercontroller/unmanaged_appliedmanifestwork_controller.go b/pkg/spoke/controllers/finalizercontroller/unmanaged_appliedmanifestwork_controller.go index edd5687ed..49bcc13f3 100644 --- a/pkg/spoke/controllers/finalizercontroller/unmanaged_appliedmanifestwork_controller.go +++ b/pkg/spoke/controllers/finalizercontroller/unmanaged_appliedmanifestwork_controller.go @@ -23,8 +23,9 @@ import ( const byWorkNameAndAgentID = "UnManagedAppliedManifestWork-byWorkNameAndAgentID" -// ManifestWorkFinalizeController handles cleanup of manifestwork resources before deletion is allowed. +// UnManagedAppliedWorkController deletes unmanaged applied works. type UnManagedAppliedWorkController struct { + manifestWorkLister worklister.ManifestWorkNamespaceLister appliedManifestWorkClient workv1client.AppliedManifestWorkInterface appliedManifestWorkLister worklister.AppliedManifestWorkLister appliedManifestWorkIndexer cache.Indexer @@ -34,12 +35,15 @@ type UnManagedAppliedWorkController struct { func NewUnManagedAppliedWorkController( recorder events.Recorder, + manifestWorkInformer workinformer.ManifestWorkInformer, + manifestWorkLister worklister.ManifestWorkNamespaceLister, appliedManifestWorkClient workv1client.AppliedManifestWorkInterface, appliedManifestWorkInformer workinformer.AppliedManifestWorkInformer, hubHash, agentID string, ) factory.Controller { controller := &UnManagedAppliedWorkController{ + manifestWorkLister: manifestWorkLister, appliedManifestWorkClient: appliedManifestWorkClient, appliedManifestWorkLister: appliedManifestWorkInformer.Lister(), appliedManifestWorkIndexer: appliedManifestWorkInformer.Informer().GetIndexer(), @@ -55,6 +59,10 @@ func NewUnManagedAppliedWorkController( } return factory.New(). + WithInformersQueueKeyFunc(func(obj runtime.Object) string { + accessor, _ := meta.Accessor(obj) + return fmt.Sprintf("%s-%s", hubHash, accessor.GetName()) + }, manifestWorkInformer.Informer()). WithFilteredEventsInformersQueueKeyFunc(func(obj runtime.Object) string { accessor, _ := meta.Accessor(obj) return accessor.GetName() @@ -71,6 +79,26 @@ func (m *UnManagedAppliedWorkController) sync(ctx context.Context, controllerCon // work not found, could have been deleted, do nothing. return nil } + if err != nil { + return err + } + + // We delete the old applied work until the work of this applied work is applied. + // This will avoid to delete the old applied work prematurely, if an applied work is an + // owner of its applied resource, if we delete the old applied work prematurely, this will + // casue to delete the applied resource. + manifestWork, err := m.manifestWorkLister.Get(appliedManifestWork.Spec.ManifestWorkName) + if errors.IsNotFound(err) { + // work not found, could have been deleted, do nothing. + return nil + } + if err != nil { + return err + } + if !meta.IsStatusConditionTrue(manifestWork.Status.Conditions, workapiv1.WorkApplied) { + // the work is not applied, do nothing. + return nil + } unManagedAppliedWorks, err := m.getUnManagedAppliedManifestWorksByIndex(appliedManifestWork.Spec.ManifestWorkName, appliedManifestWork.Spec.AgentID) if err != nil { diff --git a/pkg/spoke/controllers/finalizercontroller/unmanaged_appliedmanifestwork_controller_test.go b/pkg/spoke/controllers/finalizercontroller/unmanaged_appliedmanifestwork_controller_test.go index e3d7c5336..50bba806a 100644 --- a/pkg/spoke/controllers/finalizercontroller/unmanaged_appliedmanifestwork_controller_test.go +++ b/pkg/spoke/controllers/finalizercontroller/unmanaged_appliedmanifestwork_controller_test.go @@ -21,6 +21,7 @@ func TestSyncUnamanagedAppliedWork(t *testing.T) { workName string hubHash string agentID string + works []runtime.Object appliedWorks []runtime.Object validateAppliedManifestWorkActions func(t *testing.T, actions []clienttesting.Action) }{ @@ -29,6 +30,22 @@ func TestSyncUnamanagedAppliedWork(t *testing.T) { workName: "test", hubHash: "hubhash1", agentID: "test-agent", + works: []runtime.Object{ + &workapiv1.ManifestWork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + }, + Status: workapiv1.ManifestWorkStatus{ + Conditions: []metav1.Condition{ + { + Type: workapiv1.WorkApplied, + Status: metav1.ConditionTrue, + }, + }, + }, + }, + }, appliedWorks: []runtime.Object{ &workapiv1.AppliedManifestWork{ ObjectMeta: metav1.ObjectMeta{ @@ -59,11 +76,64 @@ func TestSyncUnamanagedAppliedWork(t *testing.T) { spoketesting.AssertAction(t, actions[0], "delete") }, }, + { + name: "no action if the work is not applied", + workName: "test", + hubHash: "hubhash1", + agentID: "test-agent", + works: []runtime.Object{ + &workapiv1.ManifestWork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + }, + }, + }, + appliedWorks: []runtime.Object{ + &workapiv1.AppliedManifestWork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hubhash-test", + }, + Spec: workapiv1.AppliedManifestWorkSpec{ + ManifestWorkName: "test", + HubHash: "hubhash", + AgentID: "test-agent", + }, + }, + &workapiv1.AppliedManifestWork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hubhash1-test", + }, + Spec: workapiv1.AppliedManifestWorkSpec{ + ManifestWorkName: "test", + HubHash: "hubhash1", + AgentID: "test-agent", + }, + }, + }, + validateAppliedManifestWorkActions: noAction, + }, { name: "no action for different AgentID", workName: "test", hubHash: "hubhash1", agentID: "test-agent", + works: []runtime.Object{ + &workapiv1.ManifestWork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + }, + Status: workapiv1.ManifestWorkStatus{ + Conditions: []metav1.Condition{ + { + Type: workapiv1.WorkApplied, + Status: metav1.ConditionTrue, + }, + }, + }, + }, + }, appliedWorks: []runtime.Object{ &workapiv1.AppliedManifestWork{ ObjectMeta: metav1.ObjectMeta{ @@ -93,6 +163,22 @@ func TestSyncUnamanagedAppliedWork(t *testing.T) { workName: "test", hubHash: "hubhash1", agentID: "test-agent", + works: []runtime.Object{ + &workapiv1.ManifestWork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + }, + Status: workapiv1.ManifestWorkStatus{ + Conditions: []metav1.Condition{ + { + Type: workapiv1.WorkApplied, + Status: metav1.ConditionTrue, + }, + }, + }, + }, + }, appliedWorks: []runtime.Object{ &workapiv1.AppliedManifestWork{ ObjectMeta: metav1.ObjectMeta{ @@ -129,6 +215,11 @@ func TestSyncUnamanagedAppliedWork(t *testing.T) { if err != nil { t.Fatal(err) } + for _, work := range c.works { + if err := informerFactory.Work().V1().ManifestWorks().Informer().GetStore().Add(work); err != nil { + t.Fatal(err) + } + } for _, appliedWork := range c.appliedWorks { if err := informerFactory.Work().V1().AppliedManifestWorks().Informer().GetStore().Add(appliedWork); err != nil { t.Fatal(err) @@ -136,6 +227,7 @@ func TestSyncUnamanagedAppliedWork(t *testing.T) { } controller := &UnManagedAppliedWorkController{ + manifestWorkLister: informerFactory.Work().V1().ManifestWorks().Lister().ManifestWorks("test"), appliedManifestWorkClient: fakeClient.WorkV1().AppliedManifestWorks(), appliedManifestWorkLister: informerFactory.Work().V1().AppliedManifestWorks().Lister(), appliedManifestWorkIndexer: informerFactory.Work().V1().AppliedManifestWorks().Informer().GetIndexer(), diff --git a/pkg/spoke/spokeagent.go b/pkg/spoke/spokeagent.go index 6c3b42cdb..8e5a947d7 100644 --- a/pkg/spoke/spokeagent.go +++ b/pkg/spoke/spokeagent.go @@ -161,6 +161,8 @@ func (o *WorkloadAgentOptions) RunWorkloadAgent(ctx context.Context, controllerC ) unmanagedAppliedManifestWorkController := finalizercontroller.NewUnManagedAppliedWorkController( controllerContext.EventRecorder, + workInformerFactory.Work().V1().ManifestWorks(), + workInformerFactory.Work().V1().ManifestWorks().Lister().ManifestWorks(o.SpokeClusterName), spokeWorkClient.WorkV1().AppliedManifestWorks(), spokeWorkInformerFactory.Work().V1().AppliedManifestWorks(), hubhash, agentID, diff --git a/test/integration/suite_test.go b/test/integration/suite_test.go index 2b469f4cd..35d21c9de 100644 --- a/test/integration/suite_test.go +++ b/test/integration/suite_test.go @@ -1,7 +1,6 @@ package integration import ( - "io/ioutil" "os" "path" "path/filepath" @@ -60,7 +59,7 @@ var _ = ginkgo.BeforeSuite(func() { gomega.Expect(cfg).ToNot(gomega.BeNil()) // create kubeconfig file for hub in a tmp dir - tempDir, err = ioutil.TempDir("", "test") + tempDir, err = os.MkdirTemp("", "test") gomega.Expect(err).ToNot(gomega.HaveOccurred()) gomega.Expect(tempDir).ToNot(gomega.BeEmpty()) hubKubeconfigFileName = path.Join(tempDir, "kubeconfig") diff --git a/test/integration/unmanaged_appliedwork_test.go b/test/integration/unmanaged_appliedwork_test.go index 4eca2cca5..b720bd5a6 100644 --- a/test/integration/unmanaged_appliedwork_test.go +++ b/test/integration/unmanaged_appliedwork_test.go @@ -3,7 +3,6 @@ package integration import ( "context" "fmt" - "io/ioutil" "os" "path" "path/filepath" @@ -65,7 +64,7 @@ var _ = ginkgo.Describe("Unmanaged ApplieManifestWork", func() { newCfg, err := newHub.Start() gomega.Expect(err).ToNot(gomega.HaveOccurred()) - newHubTempDir, err = ioutil.TempDir("", "unmanaged_work_test") + newHubTempDir, err = os.MkdirTemp("", "unmanaged_work_test") gomega.Expect(err).ToNot(gomega.HaveOccurred()) newHubKubeConfigFile = path.Join(newHubTempDir, "kubeconfig")