diff --git a/pkg/skaffold/deploy/deploy_mux.go b/pkg/skaffold/deploy/deploy_mux.go index b43bf3e98b4..6a4f472e390 100644 --- a/pkg/skaffold/deploy/deploy_mux.go +++ b/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 { diff --git a/pkg/skaffold/deploy/deploy_mux_test.go b/pkg/skaffold/deploy/deploy_mux_test.go index b041215499c..048f85a10f9 100644 --- a/pkg/skaffold/deploy/deploy_mux_test.go +++ b/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{})) + }) + } +}