delete old applied work after its new work is applied (#172)

Signed-off-by: Wei Liu <liuweixa@redhat.com>

Signed-off-by: Wei Liu <liuweixa@redhat.com>
This commit is contained in:
Wei Liu
2023-01-03 09:36:02 +08:00
committed by GitHub
parent 5b740461ad
commit 5417cc44c9
5 changed files with 125 additions and 5 deletions

View File

@@ -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 {

View File

@@ -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(),

View File

@@ -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,

View File

@@ -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")

View File

@@ -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")