Pruning should happen after Cleanup

cmd/skaffold/app/cmd/dev.go

@@ -43,17 +43,17 @@ func NewCmdDev(out io.Writer) *cobra.Command {
 }
 
 func doDev(ctx context.Context, out io.Writer) error {
-	cleanup := func() {}
-	if opts.Cleanup {
+	prune := func() {}
+	if opts.Prune() {
 		defer func() {
-			cleanup()
+			prune()
 		}()
 	}
 
-	prune := func() {}
-	if opts.Prune() {
+	cleanup := func() {}
+	if opts.Cleanup {
 		defer func() {
-			prune()
+			cleanup()
 		}()
 	}
 

cmd/skaffold/app/cmd/dev_test.go

@@ -0,0 +1,112 @@
+/*
+Copyright 2019 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cmd
+
+import (
+	"context"
+	"io"
+	"io/ioutil"
+	"testing"
+
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+	"github.com/GoogleContainerTools/skaffold/testutil"
+)
+
+type mockDevRunner struct {
+	runner.Runner
+	hasBuilt    bool
+	hasDeployed bool
+	errDev      error
+	calls       []string
+}
+
+func (r *mockDevRunner) Dev(context.Context, io.Writer, []*latest.Artifact) error {
+	r.calls = append(r.calls, "Dev")
+	return r.errDev
+}
+
+func (r *mockDevRunner) HasBuilt() bool {
+	r.calls = append(r.calls, "HasBuilt")
+	return r.hasBuilt
+}
+
+func (r *mockDevRunner) HasDeployed() bool {
+	r.calls = append(r.calls, "HasDeployed")
+	return r.hasDeployed
+}
+
+func (r *mockDevRunner) Prune(context.Context, io.Writer) error {
+	r.calls = append(r.calls, "Prune")
+	return nil
+}
+
+func (r *mockDevRunner) Cleanup(context.Context, io.Writer) error {
+	r.calls = append(r.calls, "Cleanup")
+	return nil
+}
+
+func TestDoDev(t *testing.T) {
+	tests := []struct {
+		description   string
+		hasBuilt      bool
+		hasDeployed   bool
+		expectedCalls []string
+	}{
+		{
+			description:   "cleanup and then prune",
+			hasBuilt:      true,
+			hasDeployed:   true,
+			expectedCalls: []string{"Dev", "HasDeployed", "HasBuilt", "Cleanup", "Prune"},
+		},
+		{
+			description:   "hasn't deployed",
+			hasBuilt:      true,
+			hasDeployed:   false,
+			expectedCalls: []string{"Dev", "HasDeployed", "HasBuilt", "Prune"},
+		},
+		{
+			description:   "hasn't built",
+			hasBuilt:      false,
+			hasDeployed:   false,
+			expectedCalls: []string{"Dev", "HasDeployed", "HasBuilt"},
+		},
+	}
+
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			mockRunner := &mockDevRunner{
+				hasBuilt:    test.hasBuilt,
+				hasDeployed: test.hasDeployed,
+				errDev:      context.Canceled,
+			}
+			t.Override(&createRunner, func(*config.SkaffoldOptions) (runner.Runner, *latest.SkaffoldConfig, error) {
+				return mockRunner, &latest.SkaffoldConfig{}, nil
+			})
+			t.Override(&opts, &config.SkaffoldOptions{
+				Cleanup: true,
+				NoPrune: false,
+			})
+
+			err := doDev(context.Background(), ioutil.Discard)
+
+			t.CheckDeepEqual(test.expectedCalls, mockRunner.calls)
+			t.CheckDeepEqual(true, err == context.Canceled)
+		})
+	}
+}

pkg/skaffold/build/local/local_test.go

@@ -287,6 +287,7 @@ func TestNewBuilder(t *testing.T) {
 				pushImages:         true, //this will be true
 				skipTests:          false,
 				prune:              true,
+				pruneChildren:      true,
 				insecureRegistries: nil,
 			},
 		}, {
@@ -314,6 +315,7 @@ func TestNewBuilder(t *testing.T) {
 
 				skipTests:          false,
 				prune:              true,
+				pruneChildren:      true,
 				insecureRegistries: nil,
 			},
 		},

pkg/skaffold/build/local/types.go

@@ -38,6 +38,7 @@ type Builder struct {
 	localCluster       bool
 	pushImages         bool
 	prune              bool
+	pruneChildren      bool
 	skipTests          bool
 	kubeContext        string
 	builtImages        []string
@@ -81,6 +82,7 @@ func NewBuilder(runCtx *runcontext.RunContext) (*Builder, error) {
 		pushImages:         pushImages,
 		skipTests:          runCtx.Opts.SkipTests,
 		prune:              runCtx.Opts.Prune(),
+		pruneChildren:      !runCtx.Opts.NoPruneChildren,
 		insecureRegistries: runCtx.InsecureRegistries,
 	}, nil
 }
@@ -101,5 +103,5 @@ func (b *Builder) Labels() map[string]string {
 
 // Prune uses the docker API client to remove all images built with Skaffold
 func (b *Builder) Prune(ctx context.Context, out io.Writer) error {
-	return docker.Prune(ctx, out, b.builtImages, b.localDocker)
+	return b.localDocker.Prune(ctx, out, b.builtImages, b.pruneChildren)
 }

pkg/skaffold/docker/image.go

@@ -55,6 +55,7 @@ type LocalDaemon interface {
 	RepoDigest(ctx context.Context, ref string) (string, error)
 	ImageList(ctx context.Context, options types.ImageListOptions) ([]types.ImageSummary, error)
 	ImageExists(ctx context.Context, ref string) bool
+	Prune(ctx context.Context, out io.Writer, images []string, pruneChildren bool) error
 }
 
 type localDaemon struct {
@@ -410,3 +411,25 @@ func EvaluateBuildArgs(args map[string]*string) (map[string]*string, error) {
 
 	return evaluated, nil
 }
+
+func (l *localDaemon) Prune(ctx context.Context, out io.Writer, images []string, pruneChildren bool) error {
+	for _, id := range images {
+		resp, err := l.ImageRemove(ctx, id, types.ImageRemoveOptions{
+			Force:         true,
+			PruneChildren: pruneChildren,
+		})
+		if err != nil {
+			return errors.Wrap(err, "pruning images")
+		}
+		for _, r := range resp {
+			if r.Deleted != "" {
+				fmt.Fprintf(out, "deleted image %s\n", r.Deleted)
+			}
+			if r.Untagged != "" {
+				fmt.Fprintf(out, "untagged image %s\n", r.Untagged)
+			}
+		}
+	}
+
+	return nil
+}

pkg/skaffold/docker/image_util.go

@@ -18,48 +18,13 @@ package docker
 
 import (
 	"context"
-	"fmt"
-	"io"
 	"strings"
 
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
-	"github.com/docker/docker/api/types"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
-var (
-	opts = &config.SkaffoldOptions{}
-)
-
-func Prune(ctx context.Context, out io.Writer, images []string, client LocalDaemon) error {
-	pruneChildren := true
-
-	if opts.NoPruneChildren {
-		pruneChildren = false
-	}
-
-	for _, id := range images {
-		resp, err := client.ImageRemove(ctx, id, types.ImageRemoveOptions{
-			Force:         true,
-			PruneChildren: pruneChildren,
-		})
-		if err != nil {
-			return errors.Wrap(err, "pruning images")
-		}
-		for _, r := range resp {
-			if r.Deleted != "" {
-				fmt.Fprintf(out, "deleted image %s\n", r.Deleted)
-			}
-			if r.Untagged != "" {
-				fmt.Fprintf(out, "untagged image %s\n", r.Untagged)
-			}
-		}
-	}
-	return nil
-}
-
 func RetrieveWorkingDir(tagged string, insecureRegistries map[string]bool) (string, error) {
 	var cf *v1.ConfigFile
 	var err error