Merge pull request #1343 from dgageot/fix-812

 Cleanup only if something was actually deployed

Author: dgageot

cmd/skaffold/app/cmd/dev.go

@@ -52,11 +52,10 @@ func dev(out io.Writer) error {
 	defer cancel()
 	catchCtrlC(cancel)
 
+	cleanup := func() {}
 	if opts.Cleanup {
 		defer func() {
-			if err := delete(out); err != nil {
-				logrus.Warnln("cleanup:", err)
-			}
+			cleanup()
 		}()
 	}
 
@@ -70,7 +69,15 @@ func dev(out io.Writer) error {
 				return errors.Wrap(err, "creating runner")
 			}
 
-			if _, err := r.Dev(ctx, out, config.Build.Artifacts); err != nil {
+			err = r.Dev(ctx, out, config.Build.Artifacts)
+			if r.HasDeployed() {
+				cleanup = func() {
+					if err := r.Cleanup(context.Background(), out); err != nil {
+						logrus.Warnln("cleanup:", err)
+					}
+				}
+			}
+			if err != nil {
 				if errors.Cause(err) != runner.ErrorConfigurationChanged {
 					return err
 				}

pkg/skaffold/runner/runner.go

@@ -59,6 +59,7 @@ type SkaffoldRunner struct {
 	opts         *config.SkaffoldOptions
 	watchFactory watch.Factory
 	builds       []build.Artifact
+	hasDeployed  bool
 	imageList    *kubernetes.ImageList
 }
 
@@ -207,6 +208,11 @@ func (r *SkaffoldRunner) newLogger(out io.Writer, artifacts []*latest.Artifact)
 	return kubernetes.NewLogAggregator(out, imageNames, r.imageList)
 }
 
+// HasDeployed returns true if this runner has deployed something.
+func (r *SkaffoldRunner) HasDeployed() bool {
+	return r.hasDeployed
+}
+
 func (r *SkaffoldRunner) buildTestDeploy(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) error {
 	bRes, err := r.BuildAndTest(ctx, out, artifacts)
 	if err != nil {
@@ -260,6 +266,13 @@ func (r *SkaffoldRunner) BuildAndTest(ctx context.Context, out io.Writer, artifa
 	return bRes, err
 }
 
+// Deploy deploys the given artifacts
+func (r *SkaffoldRunner) Deploy(ctx context.Context, out io.Writer, artifacts []build.Artifact) ([]deploy.Artifact, error) {
+	dRes, err := r.Deployer.Deploy(ctx, out, artifacts)
+	r.hasDeployed = true
+	return dRes, err
+}
+
 // TailLogs prints the logs for deployed artifacts.
 func (r *SkaffoldRunner) TailLogs(ctx context.Context, out io.Writer, artifacts []*latest.Artifact, bRes []build.Artifact) error {
 	if !r.opts.Tail {
@@ -281,7 +294,7 @@ func (r *SkaffoldRunner) TailLogs(ctx context.Context, out io.Writer, artifacts
 
 // Dev watches for changes and runs the skaffold build and deploy
 // pipeline until interrrupted by the user.
-func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) ([]build.Artifact, error) {
+func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) error {
 	logger := r.newLogger(out, artifacts)
 
 	// Create watcher and register artifacts to build current state of files.
@@ -349,7 +362,7 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*la
 			func() ([]string, error) { return DependenciesForArtifact(ctx, artifact) },
 			func(e watch.Events) { changed.AddDirtyArtifact(artifact, e) },
 		); err != nil {
-			return nil, errors.Wrapf(err, "watching files for artifact %s", artifact.ImageName)
+			return errors.Wrapf(err, "watching files for artifact %s", artifact.ImageName)
 		}
 	}
 
@@ -358,47 +371,47 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*la
 		func() ([]string, error) { return r.TestDependencies() },
 		func(watch.Events) { changed.needsRedeploy = true },
 	); err != nil {
-		return nil, errors.Wrap(err, "watching test files")
+		return errors.Wrap(err, "watching test files")
 	}
 
 	// Watch deployment configuration
 	if err := watcher.Register(
 		func() ([]string, error) { return r.Dependencies() },
 		func(watch.Events) { changed.needsRedeploy = true },
 	); err != nil {
-		return nil, errors.Wrap(err, "watching files for deployer")
+		return errors.Wrap(err, "watching files for deployer")
 	}
 
 	// Watch Skaffold configuration
 	if err := watcher.Register(
 		func() ([]string, error) { return []string{r.opts.ConfigurationFile}, nil },
 		func(watch.Events) { changed.needsReload = true },
 	); err != nil {
-		return nil, errors.Wrapf(err, "watching skaffold configuration %s", r.opts.ConfigurationFile)
+		return errors.Wrapf(err, "watching skaffold configuration %s", r.opts.ConfigurationFile)
 	}
 
 	// First run
 	if err := r.buildTestDeploy(ctx, out, artifacts); err != nil {
-		return nil, errors.Wrap(err, "exiting dev mode because first run failed")
+		return errors.Wrap(err, "exiting dev mode because first run failed")
 	}
 
 	// Start logs
 	if r.opts.TailDev {
 		if err := logger.Start(ctx); err != nil {
-			return nil, errors.Wrap(err, "starting logger")
+			return errors.Wrap(err, "starting logger")
 		}
 	}
 
 	if r.opts.PortForward {
 		portForwarder := kubernetes.NewPortForwarder(out, r.imageList)
 
 		if err := portForwarder.Start(ctx); err != nil {
-			return nil, errors.Wrap(err, "starting port-forwarder")
+			return errors.Wrap(err, "starting port-forwarder")
 		}
 	}
 
 	r.Trigger.WatchForChanges(out)
-	return nil, watcher.Run(ctx, r.Trigger, onChange)
+	return watcher.Run(ctx, r.Trigger, onChange)
 }
 
 func (r *SkaffoldRunner) shouldWatch(artifact *latest.Artifact) bool {

pkg/skaffold/runner/runner_test.go

@@ -394,7 +394,7 @@ func TestDev(t *testing.T) {
 			runner.Deployer = test.deployer
 			runner.watchFactory = test.watcherFactory
 
-			_, err := runner.Dev(context.Background(), ioutil.Discard, nil)
+			err := runner.Dev(context.Background(), ioutil.Discard, nil)
 
 			testutil.CheckError(t, test.shouldErr, err)
 		})
@@ -417,7 +417,7 @@ func TestBuildAndDeployAllArtifacts(t *testing.T) {
 
 	// Both artifacts are changed
 	runner.watchFactory = NewWatcherFactory(nil, nil, []int{0, 1})
-	_, err := runner.Dev(ctx, ioutil.Discard, artifacts)
+	err := runner.Dev(ctx, ioutil.Discard, artifacts)
 
 	if err != nil {
 		t.Errorf("Didn't expect an error. Got %s", err)
@@ -431,7 +431,7 @@ func TestBuildAndDeployAllArtifacts(t *testing.T) {
 
 	// Only one is changed
 	runner.watchFactory = NewWatcherFactory(nil, nil, []int{1})
-	_, err = runner.Dev(ctx, ioutil.Discard, artifacts)
+	err = runner.Dev(ctx, ioutil.Discard, artifacts)
 
 	if err != nil {
 		t.Errorf("Didn't expect an error. Got %s", err)