From 7eb000269264a64f0df724b8d97bbe3278e392fa Mon Sep 17 00:00:00 2001 From: Tianxin Dong Date: Fri, 22 Apr 2022 16:34:07 +0800 Subject: [PATCH] Feat: add sequential in gc policy (#3701) * Feat: add sequential in gc policy Signed-off-by: FogDong * tidy the code Signed-off-by: FogDong * add suite test Signed-off-by: FogDong * add example docs and update the field Signed-off-by: FogDong * fix lint Signed-off-by: FogDong * change the name to dependency Signed-off-by: FogDong --- .../v1alpha1/garbagecollect_types.go | 11 ++ .../gc-policy/reverse-dependency.md | 50 ++++++++ .../v1alpha2/application/gc_policy_test.go | 105 +++++++++++++++++ pkg/resourcekeeper/gc.go | 107 ++++++++++++++++-- pkg/resourcekeeper/gc_test.go | 82 ++++++++++---- pkg/resourcekeeper/options.go | 8 ++ 6 files changed, 332 insertions(+), 31 deletions(-) create mode 100644 docs/examples/app-with-policy/gc-policy/reverse-dependency.md diff --git a/apis/core.oam.dev/v1alpha1/garbagecollect_types.go b/apis/core.oam.dev/v1alpha1/garbagecollect_types.go index b3746b42a..1e9e49f5c 100644 --- a/apis/core.oam.dev/v1alpha1/garbagecollect_types.go +++ b/apis/core.oam.dev/v1alpha1/garbagecollect_types.go @@ -33,11 +33,22 @@ type GarbageCollectPolicySpec struct { // outdated resources will be kept until resourcetracker be deleted manually KeepLegacyResource bool `json:"keepLegacyResource,omitempty"` + // Order defines the order of garbage collect + Order GarbageCollectOrder `json:"order,omitempty"` + // Rules defines list of rules to control gc strategy at resource level // if one resource is controlled by multiple rules, first rule will be used Rules []GarbageCollectPolicyRule `json:"rules,omitempty"` } +// GarbageCollectOrder is the order of garbage collect +type GarbageCollectOrder string + +const ( + // OrderDependency is the order of dependency + OrderDependency GarbageCollectOrder = "dependency" +) + // GarbageCollectPolicyRule defines a single garbage-collect policy rule type GarbageCollectPolicyRule struct { Selector GarbageCollectPolicyRuleSelector `json:"selector"` diff --git a/docs/examples/app-with-policy/gc-policy/reverse-dependency.md b/docs/examples/app-with-policy/gc-policy/reverse-dependency.md new file mode 100644 index 000000000..b094a1487 --- /dev/null +++ b/docs/examples/app-with-policy/gc-policy/reverse-dependency.md @@ -0,0 +1,50 @@ +# How to garbage collect resources in the order of dependency + +If you want to garbage collect resources in the order of dependency, you can add `order: dependency` in the `garbage-collect` policy. + +> Notice that this order policy is only valid for the resources that are created in the components. + +In the following example, component `test1` depends on `test2`, and `test2` need the output from `test3`. + +So the order of deployment is: `test3 -> test2 -> test1`. + +When we add `order: dependency` in `garbage-collect` policy and delete the application, the order of garbage collect is: `test3 -> test2 -> test1`. + +```yaml +apiVersion: core.oam.dev/v1beta1 +kind: Application +metadata: + name: gc-dependency + namespace: default +spec: + components: + - name: test1 + type: webservice + properties: + image: crccheck/hello-world + port: 8000 + dependsOn: + - "test2" + - name: test2 + type: webservice + properties: + image: crccheck/hello-world + port: 8000 + inputs: + - from: test3-output + parameterKey: test + - name: test3 + type: webservice + properties: + image: crccheck/hello-world + port: 8000 + outputs: + - name: test3-output + valueFrom: output.metadata.name + + policies: + - name: gc-dependency + type: garbage-collect + properties: + order: dependency +``` diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/gc_policy_test.go b/pkg/controller/core.oam.dev/v1alpha2/application/gc_policy_test.go index 7a4494109..882cdfe55 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/gc_policy_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/gc_policy_test.go @@ -473,6 +473,111 @@ var _ = Describe("Test Application with GC options", func() { Expect(len(rtList.Items)).Should(Equal(0)) }) }) + + Context("Test Application enable gc option sequential", func() { + baseApp := &v1beta1.Application{ + TypeMeta: metav1.TypeMeta{ + Kind: "Application", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "sequential-gc", + Namespace: "default", + }, + Spec: v1beta1.ApplicationSpec{ + Components: []common.ApplicationComponent{ + { + Name: "worker1", + Type: "worker", + Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox"}`)}, + DependsOn: []string{ + "worker2", + }, + }, + { + Name: "worker2", + Type: "worker", + Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox"}`)}, + Inputs: common.StepInputs{ + { + From: "worker3-output", + ParameterKey: "test", + }, + }, + }, + { + Name: "worker3", + Type: "worker", + Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox"}`)}, + Outputs: common.StepOutputs{ + { + Name: "worker3-output", + ValueFrom: "output.metadata.name", + }, + }, + }, + }, + Policies: []v1beta1.AppPolicy{{ + Name: "reverse-dependency", + Type: "garbage-collect", + Properties: &runtime.RawExtension{Raw: []byte(`{"order": "dependency"}`)}, + }}, + }, + } + + It("Test GC with sequential", func() { + resourcekeeper.MarkWithProbability = 1.0 + app := baseApp.DeepCopy() + + Expect(k8sClient.Create(ctx, app)).Should(BeNil()) + appV1 := new(v1beta1.Application) + Eventually(func() error { + _, err := testutil.ReconcileOnceAfterFinalizer(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + if err != nil { + return err + } + if err = k8sClient.Get(ctx, client.ObjectKeyFromObject(app), appV1); err != nil { + return err + } + if appV1.Status.Phase != common.ApplicationRunning { + return errors.New("app is not in running status") + } + return nil + }, 3*time.Second, 300*time.Second).Should(BeNil()) + + By("check the resourceTrackers number") + listOpts := []client.ListOption{ + client.MatchingLabels{ + oam.LabelAppName: app.Name, + oam.LabelAppNamespace: app.Namespace, + }} + + rtList := &v1beta1.ResourceTrackerList{} + Expect(k8sClient.List(ctx, rtList, listOpts...)).Should(BeNil()) + Expect(len(rtList.Items)).Should(Equal(2)) + workerList := &v1.DeploymentList{} + Expect(k8sClient.List(ctx, workerList, listOpts...)).Should(BeNil()) + Expect(len(workerList.Items)).Should(Equal(3)) + + By("delete application") + Expect(k8sClient.Delete(ctx, app)).Should(BeNil()) + By("worker3 will be deleted") + testutil.ReconcileOnce(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + Expect(k8sClient.List(ctx, workerList, listOpts...)).Should(BeNil()) + Expect(len(workerList.Items)).Should(Equal(2)) + By("worker2 will be deleted") + testutil.ReconcileOnce(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + Expect(k8sClient.List(ctx, workerList, listOpts...)).Should(BeNil()) + Expect(len(workerList.Items)).Should(Equal(1)) + By("worker1 will be deleted") + testutil.ReconcileOnce(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + Expect(k8sClient.List(ctx, workerList, listOpts...)).Should(BeNil()) + Expect(len(workerList.Items)).Should(Equal(0)) + testutil.ReconcileOnce(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + Expect(k8sClient.List(ctx, rtList, listOpts...)).Should(BeNil()) + Expect(len(rtList.Items)).Should(Equal(0)) + }) + }) }) const ( diff --git a/pkg/resourcekeeper/gc.go b/pkg/resourcekeeper/gc.go index 058601004..980db5d76 100644 --- a/pkg/resourcekeeper/gc.go +++ b/pkg/resourcekeeper/gc.go @@ -40,6 +40,7 @@ import ( "github.com/oam-dev/kubevela/pkg/multicluster" "github.com/oam-dev/kubevela/pkg/oam" "github.com/oam-dev/kubevela/pkg/resourcetracker" + "github.com/oam-dev/kubevela/pkg/utils" version2 "github.com/oam-dev/kubevela/version" ) @@ -61,6 +62,8 @@ type gcConfig struct { disableFinalize bool disableComponentRevisionGC bool disableLegacyGC bool + + order v1alpha1.GarbageCollectOrder } func newGCConfig(options ...GCOption) *gcConfig { @@ -95,8 +98,15 @@ func newGCConfig(options ...GCOption) *gcConfig { // NOTE: Mark Stage will only work when Workflow succeeds. Check/Finalize Stage will always work. // For one single application, the deletion will follow Mark -> Finalize -> Sweep func (h *resourceKeeper) GarbageCollect(ctx context.Context, options ...GCOption) (finished bool, waiting []v1beta1.ManagedResource, err error) { - if h.garbageCollectPolicy != nil && h.garbageCollectPolicy.KeepLegacyResource { - options = append(options, PassiveGCOption{}) + if h.garbageCollectPolicy != nil { + if h.garbageCollectPolicy.KeepLegacyResource { + options = append(options, PassiveGCOption{}) + } + switch h.garbageCollectPolicy.Order { + case v1alpha1.OrderDependency: + options = append(options, DependencyGCOption{}) + default: + } } cfg := newGCConfig(options...) return h.garbageCollect(ctx, cfg) @@ -247,23 +257,100 @@ func (h *gcHandler) Sweep(ctx context.Context) (finished bool, waiting []v1beta1 } func (h *gcHandler) recycleResourceTracker(ctx context.Context, rt *v1beta1.ResourceTracker) error { + switch h.cfg.order { + case v1alpha1.OrderDependency: + for _, mr := range rt.Spec.ManagedResources { + if err := h.deleteIndependentComponent(ctx, mr, rt); err != nil { + return err + } + } + return nil + default: + } for _, mr := range rt.Spec.ManagedResources { - entry := h.cache.get(ctx, mr) - if entry.gcExecutorRT != rt { - continue + if err := h.deleteManagedResource(ctx, mr, rt); err != nil { + return err } - if entry.err != nil { - return entry.err + } + return nil +} + +func (h *gcHandler) deleteIndependentComponent(ctx context.Context, mr v1beta1.ManagedResource, rt *v1beta1.ResourceTracker) error { + dependent := h.checkDependentComponent(mr) + if len(dependent) == 0 { + if err := h.deleteManagedResource(ctx, mr, rt); err != nil { + return err } - if entry.exists { - if err := h.Client.Delete(multicluster.ContextWithClusterName(ctx, mr.Cluster), entry.obj); err != nil && !kerrors.IsNotFound(err) { - return errors.Wrapf(err, "failed to delete resource %s", mr.ResourceKey()) + } else { + dependentClear := true + for _, mr := range rt.Spec.ManagedResources { + if utils.StringsContain(dependent, mr.Component) { + entry := h.cache.get(ctx, mr) + if entry.gcExecutorRT != rt { + continue + } + if entry.err != nil { + continue + } + if entry.exists { + dependentClear = false + break + } + } + } + if dependentClear { + if err := h.deleteManagedResource(ctx, mr, rt); err != nil { + return err } } } return nil } +func (h *gcHandler) deleteManagedResource(ctx context.Context, mr v1beta1.ManagedResource, rt *v1beta1.ResourceTracker) error { + entry := h.cache.get(ctx, mr) + if entry.gcExecutorRT != rt { + return nil + } + if entry.err != nil { + return entry.err + } + if entry.exists { + if err := h.Client.Delete(multicluster.ContextWithClusterName(ctx, mr.Cluster), entry.obj); err != nil && !kerrors.IsNotFound(err) { + return errors.Wrapf(err, "failed to delete resource %s", mr.ResourceKey()) + } + } + return nil +} + +func (h *gcHandler) checkDependentComponent(mr v1beta1.ManagedResource) []string { + dependent := make([]string, 0) + inputs := make([]string, 0) + for _, comp := range h.app.Spec.Components { + if comp.Name == mr.Component { + dependent = comp.DependsOn + if len(comp.Inputs) > 0 { + for _, input := range comp.Inputs { + inputs = append(inputs, input.From) + } + } else { + return dependent + } + break + } + } + for _, comp := range h.app.Spec.Components { + if len(comp.Outputs) > 0 { + for _, output := range comp.Outputs { + if utils.StringsContain(inputs, output.Name) { + dependent = append(dependent, comp.Name) + } + } + } + } + return dependent +} + func (h *gcHandler) Finalize(ctx context.Context) error { cb := h.monitor("finalize") defer cb() diff --git a/pkg/resourcekeeper/gc_test.go b/pkg/resourcekeeper/gc_test.go index 1d7467ab1..dbcf9973e 100644 --- a/pkg/resourcekeeper/gc_test.go +++ b/pkg/resourcekeeper/gc_test.go @@ -30,6 +30,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + apicommon "github.com/oam-dev/kubevela/apis/core.oam.dev/common" "github.com/oam-dev/kubevela/apis/core.oam.dev/v1alpha1" "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" "github.com/oam-dev/kubevela/pkg/oam" @@ -125,14 +126,16 @@ func TestResourceKeeperGarbageCollect(t *testing.T) { r.Equal(crCount, len(_crs.Items)) } - createRK := func(gen int64, keepLegacy bool) *resourceKeeper { + createRK := func(gen int64, keepLegacy bool, order v1alpha1.GarbageCollectOrder, components ...apicommon.ApplicationComponent) *resourceKeeper { _rk, err := NewResourceKeeper(ctx, cli, &v1beta1.Application{ ObjectMeta: v12.ObjectMeta{Name: "app", Namespace: "default", UID: "uid", Generation: gen}, + Spec: v1beta1.ApplicationSpec{Components: components}, }) r.NoError(err) rk := _rk.(*resourceKeeper) - if keepLegacy { - rk.garbageCollectPolicy = &v1alpha1.GarbageCollectPolicySpec{KeepLegacyResource: true} + rk.garbageCollectPolicy = &v1alpha1.GarbageCollectPolicySpec{ + Order: order, + KeepLegacyResource: keepLegacy, } return rk } @@ -145,70 +148,107 @@ func TestResourceKeeperGarbageCollect(t *testing.T) { addConfigMapToRT(3, 2, 3) createRT(3) addConfigMapToRT(4, 3, 3) - checkCount(4, 4, 3) + createRT(4) + addConfigMapToRT(5, 4, 4) + addConfigMapToRT(6, 4, 5) + addConfigMapToRT(7, 4, 6) + checkCount(7, 5, 6) opts := []GCOption{DisableLegacyGCOption{}} // no need to gc - rk := createRK(3, true) + rk := createRK(4, true, "") finished, _, err := rk.GarbageCollect(ctx, opts...) r.NoError(err) r.True(finished) - checkCount(4, 4, 3) + checkCount(7, 5, 6) // delete rt2, trigger gc for cm3 dt := v12.Now() rtMaps[2].SetDeletionTimestamp(&dt) r.NoError(cli.Update(ctx, rtMaps[2])) - rk = createRK(3, true) + rk = createRK(4, true, "") finished, _, err = rk.GarbageCollect(ctx, opts...) r.NoError(err) r.False(finished) - rk = createRK(3, true) + rk = createRK(4, true, "") finished, _, err = rk.GarbageCollect(ctx, opts...) r.NoError(err) r.True(finished) - checkCount(3, 3, 3) + checkCount(6, 4, 6) // delete cm4, trigger gc for rt3, comp-3 no use r.NoError(cli.Delete(ctx, cmMaps[4])) - rk = createRK(4, true) + rk = createRK(5, true, "") finished, _, err = rk.GarbageCollect(ctx, opts...) r.NoError(err) r.True(finished) - checkCount(2, 2, 2) + checkCount(5, 3, 5) // upgrade and gc legacy rt1 - rk = createRK(4, false) + rk = createRK(4, false, "") finished, _, err = rk.GarbageCollect(ctx, opts...) r.NoError(err) r.False(finished) - rk = createRK(4, false) + rk = createRK(4, false, "") + finished, _, err = rk.GarbageCollect(ctx, opts...) + r.NoError(err) + r.True(finished) + checkCount(3, 2, 3) + + // delete with sequential + comps := []apicommon.ApplicationComponent{ + { + Name: "comp-5", + DependsOn: []string{ + "comp-6", + }, + }, + { + Name: "comp-6", + DependsOn: []string{ + "comp-7", + }, + }, + { + Name: "comp-7", + }, + } + rk = createRK(5, false, v1alpha1.OrderDependency, comps...) + rtMaps[3].SetDeletionTimestamp(&dt) + finished, _, err = rk.GarbageCollect(ctx, opts...) + r.NoError(err) + r.False(finished) + rk = createRK(5, false, v1alpha1.OrderDependency, comps...) + finished, _, err = rk.GarbageCollect(ctx, opts...) + r.NoError(err) + r.False(finished) + rk = createRK(5, false, v1alpha1.OrderDependency, comps...) finished, _, err = rk.GarbageCollect(ctx, opts...) r.NoError(err) r.True(finished) - checkCount(0, 1, 0) r.NoError(cli.Get(ctx, client.ObjectKeyFromObject(crRT), crRT)) // recreate rt, delete app, gc all createRT(5) - addConfigMapToRT(5, 5, 4) - addConfigMapToRT(6, 5, 4) + addConfigMapToRT(8, 5, 8) + addConfigMapToRT(9, 5, 8) createRT(6) - addConfigMapToRT(6, 6, 4) - addConfigMapToRT(7, 6, 4) + addConfigMapToRT(9, 6, 8) + addConfigMapToRT(10, 6, 8) checkCount(3, 3, 1) - rk = createRK(6, false) + + rk = createRK(6, false, "") rk.app.SetDeletionTimestamp(&dt) finished, _, err = rk.GarbageCollect(ctx, opts...) r.NoError(err) r.False(finished) - rk = createRK(6, false) + rk = createRK(6, false, "") finished, _, err = rk.GarbageCollect(ctx, opts...) r.NoError(err) r.True(finished) checkCount(0, 0, 0) - rk = createRK(7, false) + rk = createRK(7, false, "") finished, _, err = rk.GarbageCollect(ctx, opts...) r.NoError(err) r.True(finished) diff --git a/pkg/resourcekeeper/options.go b/pkg/resourcekeeper/options.go index 2828aad2c..411e3cdaf 100644 --- a/pkg/resourcekeeper/options.go +++ b/pkg/resourcekeeper/options.go @@ -67,6 +67,14 @@ type PassiveGCOption struct{} // ApplyToGCConfig apply change to gc config func (option PassiveGCOption) ApplyToGCConfig(cfg *gcConfig) { cfg.passive = true } +// DependencyGCOption recycle the resource in the order of reverse dependency +type DependencyGCOption struct{} + +// ApplyToGCConfig apply change to gc config +func (option DependencyGCOption) ApplyToGCConfig(cfg *gcConfig) { + cfg.order = v1alpha1.OrderDependency +} + // DisableMarkStageGCOption disable the mark stage in gc process (no rt will be marked to be deleted) // this option should be switched on when application workflow is suspending/terminating since workflow is not // finished so outdated versions should be kept