diff --git a/pkg/addonmanager/controllers/agentdeploy/controller.go b/pkg/addonmanager/controllers/agentdeploy/controller.go index 84bb60970..d66e13d14 100644 --- a/pkg/addonmanager/controllers/agentdeploy/controller.go +++ b/pkg/addonmanager/controllers/agentdeploy/controller.go @@ -166,7 +166,7 @@ func (c *addonDeployController) sync(ctx context.Context, syncCtx factory.SyncCo c.setStatusFeedbackRule(work, agentAddon) // apply work - work, err = applyWork(c.workClient, c.workLister, c.cache, c.eventRecorder, ctx, work) + work, err = applyWork(ctx, c.workClient, c.workLister, c.cache, c.eventRecorder, work) if err != nil { meta.SetStatusCondition(&managedClusterAddonCopy.Status.Conditions, metav1.Condition{ Type: "ManifestApplied", diff --git a/pkg/addonmanager/controllers/agentdeploy/hookcontroller.go b/pkg/addonmanager/controllers/agentdeploy/hookcontroller.go index 7350d79a0..4d9c43822 100644 --- a/pkg/addonmanager/controllers/agentdeploy/hookcontroller.go +++ b/pkg/addonmanager/controllers/agentdeploy/hookcontroller.go @@ -176,7 +176,7 @@ func (c *addonHookDeployController) sync(ctx context.Context, syncCtx factory.Sy // apply hookWork when addon is deleting hookWork.OwnerReferences = []metav1.OwnerReference{*owner} - hookWork, applyErr := applyWork(c.workClient, c.workLister, c.cache, c.eventRecorder, ctx, hookWork) + hookWork, applyErr := applyWork(ctx, c.workClient, c.workLister, c.cache, c.eventRecorder, hookWork) completed := hookWorkIsCompleted(hookWork) if completed && hasFinalizer(managedClusterAddonCopy.Finalizers, constants.PreDeleteHookFinalizer) { finalizer := removeFinalizer(managedClusterAddonCopy.Finalizers, constants.PreDeleteHookFinalizer) diff --git a/pkg/addonmanager/controllers/agentdeploy/util_test.go b/pkg/addonmanager/controllers/agentdeploy/util_test.go new file mode 100644 index 000000000..73a5de44a --- /dev/null +++ b/pkg/addonmanager/controllers/agentdeploy/util_test.go @@ -0,0 +1,73 @@ +package agentdeploy + +import ( + "context" + "testing" + "time" + + "k8s.io/apimachinery/pkg/runtime" + "open-cluster-management.io/addon-framework/pkg/addonmanager/addontesting" + fakework "open-cluster-management.io/api/client/work/clientset/versioned/fake" + workinformers "open-cluster-management.io/api/client/work/informers/externalversions" + workapiv1 "open-cluster-management.io/api/work/v1" +) + +func TestApplyWork(t *testing.T) { + cache := newWorkCache() + fakeWorkClient := fakework.NewSimpleClientset() + workInformerFactory := workinformers.NewSharedInformerFactory(fakeWorkClient, 10*time.Minute) + syncContext := addontesting.NewFakeSyncContext(t, "test") + + work, _, _ := buildManifestWorkFromObject("cluster1", "addon1", []runtime.Object{addontesting.NewUnstructured("batch/v1", "Job", "default", "test")}) + + _, err := applyWork(context.TODO(), fakeWorkClient, workInformerFactory.Work().V1().ManifestWorks().Lister(), cache, syncContext.Recorder(), work) + if err != nil { + t.Errorf("failed to apply work with err %v", err) + } + + addontesting.AssertActions(t, fakeWorkClient.Actions(), "create") + + // IF work is not changed, we should not update + newWorkCopy := work.DeepCopy() + fakeWorkClient.ClearActions() + workInformerFactory.Work().V1().ManifestWorks().Informer().GetStore().Add(work) + _, err = applyWork(context.TODO(), fakeWorkClient, workInformerFactory.Work().V1().ManifestWorks().Lister(), cache, syncContext.Recorder(), newWorkCopy) + if err != nil { + t.Errorf("failed to apply work with err %v", err) + } + addontesting.AssertNoActions(t, fakeWorkClient.Actions()) + + // Update work spec to update it + newWork, _, _ := buildManifestWorkFromObject("cluster1", "addon1", []runtime.Object{addontesting.NewUnstructured("batch/v1", "Job", "default", "test")}) + newWork.Spec.DeleteOption = &workapiv1.DeleteOption{PropagationPolicy: workapiv1.DeletePropagationPolicyTypeOrphan} + fakeWorkClient.ClearActions() + _, err = applyWork(context.TODO(), fakeWorkClient, workInformerFactory.Work().V1().ManifestWorks().Lister(), cache, syncContext.Recorder(), newWork) + if err != nil { + t.Errorf("failed to apply work with err %v", err) + } + addontesting.AssertActions(t, fakeWorkClient.Actions(), "update") + + // Do not update if generation is not changed + work.Spec.DeleteOption = &workapiv1.DeleteOption{PropagationPolicy: workapiv1.DeletePropagationPolicyTypeForeground} + workInformerFactory.Work().V1().ManifestWorks().Informer().GetStore().Update(work) + + fakeWorkClient.ClearActions() + workInformerFactory.Work().V1().ManifestWorks().Informer().GetStore().Update(work) + _, err = applyWork(context.TODO(), fakeWorkClient, workInformerFactory.Work().V1().ManifestWorks().Lister(), cache, syncContext.Recorder(), newWork) + if err != nil { + t.Errorf("failed to apply work with err %v", err) + } + addontesting.AssertNoActions(t, fakeWorkClient.Actions()) + + // change generation will cause update + work.Generation = 1 + workInformerFactory.Work().V1().ManifestWorks().Informer().GetStore().Update(work) + + fakeWorkClient.ClearActions() + workInformerFactory.Work().V1().ManifestWorks().Informer().GetStore().Update(work) + _, err = applyWork(context.TODO(), fakeWorkClient, workInformerFactory.Work().V1().ManifestWorks().Lister(), cache, syncContext.Recorder(), newWork) + if err != nil { + t.Errorf("failed to apply work with err %v", err) + } + addontesting.AssertActions(t, fakeWorkClient.Actions(), "update") +} diff --git a/pkg/addonmanager/controllers/agentdeploy/utils.go b/pkg/addonmanager/controllers/agentdeploy/utils.go index 4c64dcac1..745283c59 100644 --- a/pkg/addonmanager/controllers/agentdeploy/utils.go +++ b/pkg/addonmanager/controllers/agentdeploy/utils.go @@ -160,11 +160,11 @@ func buildManifestWorkFromObject( } func applyWork( + ctx context.Context, workClient workv1client.Interface, workLister worklister.ManifestWorkLister, cache *workCache, eventRecorder events.Recorder, - ctx context.Context, required *workapiv1.ManifestWork) (*workapiv1.ManifestWork, error) { existingWork, err := workLister.ManifestWorks(required.Namespace).Get(required.Name) existingWork = existingWork.DeepCopy() @@ -190,7 +190,7 @@ func applyWork( return existingWork, nil } - existingWork.Spec.Workload = required.Spec.Workload + existingWork.Spec = required.Spec existingWork, err = workClient.WorkV1().ManifestWorks(existingWork.Namespace).Update(ctx, existingWork, metav1.UpdateOptions{}) if err == nil { cache.updateCache(required, existingWork)