fix: reverse order of deployers during cleanup (GoogleContainerTools#…

…7284)

pkg/skaffold/deploy/deploy_mux.go

@@ -59,6 +59,14 @@ func (m DeployerMux) GetDeployers() []Deployer {
 	return m.deployers
 }
 
+func (m DeployerMux) GetDeployersInverse() []Deployer {
+	inverse := m.deployers
+	for i, j := 0, len(inverse)-1; i < j; i, j = i+1, j-1 {
+		inverse[i], inverse[j] = inverse[j], inverse[i]
+	}
+	return inverse
+}
+
 func (m DeployerMux) GetAccessor() access.Accessor {
 	var accessors access.AccessorMux
 	for _, deployer := range m.deployers {
@@ -159,7 +167,13 @@ func (m DeployerMux) Dependencies() ([]string, error) {
 }
 
 func (m DeployerMux) Cleanup(ctx context.Context, w io.Writer, dryRun bool) error {
-	for _, deployer := range m.deployers {
+	// Reverse order of deployers for cleanup to ensure resources
+	// are removed before their definitions are removed.
+	revDeployers := m.deployers
+	for i, j := 0, len(revDeployers)-1; i < j; i, j = i+1, j-1 {
+		revDeployers[i], revDeployers[j] = revDeployers[j], revDeployers[i]
+	}
+	for _, deployer := range revDeployers {
 		ctx, endTrace := instrumentation.StartTrace(ctx, "Cleanup")
 		if dryRun {
 			output.Yellow.Fprintln(w, "Following resources would be deleted:")
@@ -174,7 +188,9 @@ func (m DeployerMux) Cleanup(ctx context.Context, w io.Writer, dryRun bool) erro
 
 func (m DeployerMux) Render(ctx context.Context, w io.Writer, as []graph.Artifact, offline bool, filepath string) error {
 	resources, buf := []string{}, &bytes.Buffer{}
-	for _, deployer := range m.deployers {
+	// Reverse order of deployers for cleanup to ensure resources
+	// are removed before their definitions are removed.
+	for _, deployer := range m.GetDeployersInverse() {
 		ctx, endTrace := instrumentation.StartTrace(ctx, "Render")
 		buf.Reset()
 		if err := deployer.Render(ctx, buf, as, offline, "" /* never write to files */); err != nil {

pkg/skaffold/deploy/deploy_mux_test.go

@@ -25,6 +25,8 @@ import (
 	"os"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/access"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/debug"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
@@ -295,3 +297,45 @@ func TestDeployerMux_Render(t *testing.T) {
 		testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expectedRender, string(content))
 	})
 }
+
+func TestDeployerMux_GetDeployersReverse(t *testing.T) {
+	d1 := NewMockDeployer()
+	d2 := NewMockDeployer()
+	d3 := NewMockDeployer()
+	d4 := NewMockDeployer()
+	d5 := NewMockDeployer()
+
+	tests := []struct {
+		name     string
+		args     []Deployer
+		expected []Deployer
+	}{
+		{
+			name:     "uneven slice",
+			args:     []Deployer{d1, d2, d3, d4, d5},
+			expected: []Deployer{d5, d4, d3, d2, d1},
+		},
+		{
+			name:     "even slice",
+			args:     []Deployer{d1, d2, d3, d4},
+			expected: []Deployer{d4, d3, d2, d1},
+		},
+		{
+			name:     "slice of one",
+			args:     []Deployer{d1},
+			expected: []Deployer{d1},
+		},
+		{
+			name:     "slice of zero",
+			args:     []Deployer{},
+			expected: []Deployer{},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			deployerMux := DeployerMux{deployers: test.args, iterativeStatusCheck: false}
+			testutil.CheckDeepEqual(t, test.expected, deployerMux.GetDeployersInverse(), cmp.AllowUnexported(MockDeployer{}))
+		})
+	}
+}