Fix: fix the dependency gc policy to reverse dependency (#4065)

Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com>
(cherry picked from commit 45cf38edb0)

Co-authored-by: FogDong <dongtianxin.tx@alibaba-inc.com>

docs/examples/app-with-policy/gc-policy/reverse-dependency.md

@@ -1,6 +1,6 @@
 # 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.
+If you want to garbage collect resources in the order of reverse 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.
 
@@ -8,7 +8,7 @@ In the following example, component `test1` depends on `test2`, and `test2` need
 
 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`.
+When we add `order: dependency` in `garbage-collect` policy and delete the application, the order of garbage collect is: `test1 -> test2 -> test3`.
 
 ```yaml
 apiVersion: core.oam.dev/v1beta1

pkg/controller/core.oam.dev/v1alpha2/application/gc_policy_test.go

@@ -561,15 +561,21 @@ var _ = Describe("Test Application with GC options", func() {
 
 			By("delete application")
 			Expect(k8sClient.Delete(ctx, app)).Should(BeNil())
-			By("worker3 will be deleted")
+			By("worker1 will be deleted")
 			testutil.ReconcileOnce(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)})
 			Expect(k8sClient.List(ctx, workerList, listOpts...)).Should(BeNil())
+			for _, worker := range workerList.Items {
+				Expect(worker.Name).ShouldNot(Equal("worker1"))
+			}
 			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")
+			for _, worker := range workerList.Items {
+				Expect(worker.Name).ShouldNot(Equal("worker2"))
+			}
+			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(0))

pkg/resourcekeeper/gc.go

@@ -327,26 +327,25 @@ func (h *gcHandler) deleteManagedResource(ctx context.Context, mr v1beta1.Manage
 
 func (h *gcHandler) checkDependentComponent(mr v1beta1.ManagedResource) []string {
 	dependent := make([]string, 0)
-	inputs := make([]string, 0)
+	outputs := 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)
+			for _, output := range comp.Outputs {
+				outputs = append(outputs, output.Name)
+			}
+		} else {
+			for _, dependsOn := range comp.DependsOn {
+				if dependsOn == mr.Component {
+					dependent = append(dependent, comp.Name)
+					break
 				}
-			} 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)
-				}
+		for _, input := range comp.Inputs {
+			if utils.StringsContain(outputs, input.From) {
+				dependent = append(dependent, comp.Name)
 			}
 		}
 	}

pkg/resourcekeeper/gc_test.go

@@ -253,3 +253,86 @@ func TestResourceKeeperGarbageCollect(t *testing.T) {
 	r.NoError(err)
 	r.True(finished)
 }
+
+func TestCheckDependentComponent(t *testing.T) {
+	rk := &resourceKeeper{
+		app: &v1beta1.Application{
+			Spec: v1beta1.ApplicationSpec{
+				Components: []apicommon.ApplicationComponent{
+					{
+						Name: "comp-1",
+						Outputs: apicommon.StepOutputs{
+							{
+								Name: "output-1",
+							},
+						},
+					},
+					{
+						Name: "comp-2",
+						Outputs: apicommon.StepOutputs{
+							{
+								Name: "output-2",
+							},
+						},
+					},
+					{
+						Name: "comp-3",
+						Inputs: apicommon.StepInputs{
+							{
+								From: "output-1",
+							},
+							{
+								From: "output-2",
+							},
+						},
+					},
+					{
+						Name:      "comp-4",
+						DependsOn: []string{"comp-3"},
+					},
+					{
+						Name:      "comp-5",
+						DependsOn: []string{"comp-4", "comp-3"},
+					},
+				},
+			},
+		},
+	}
+	testCases := []struct {
+		comp   string
+		result []string
+	}{
+		{
+			comp:   "comp-1",
+			result: []string{"comp-3"},
+		},
+		{
+			comp:   "comp-2",
+			result: []string{"comp-3"},
+		},
+		{
+			comp:   "comp-3",
+			result: []string{"comp-4", "comp-5"},
+		},
+		{
+			comp:   "comp-4",
+			result: []string{"comp-5"},
+		},
+		{
+			comp:   "comp-5",
+			result: []string{},
+		},
+	}
+	gcHandler := &gcHandler{
+		resourceKeeper: rk,
+	}
+	r := require.New(t)
+	for _, tc := range testCases {
+		mr := v1beta1.ManagedResource{
+			OAMObjectReference: apicommon.OAMObjectReference{
+				Component: tc.comp,
+			},
+		}
+		r.Equal(gcHandler.checkDependentComponent(mr), tc.result)
+	}
+}