Start API server only once

Signed-off-by: David Gageot <david@gageot.net>

cmd/skaffold/app/cmd/cmd.go

@@ -26,6 +26,8 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/server"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/update"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/version"
 	"github.com/pkg/errors"
@@ -44,47 +46,68 @@ var (
 
 func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {
 	updateMsg := make(chan string)
+	var shutdownAPIServer func() error
 
 	rootCmd := &cobra.Command{
 		Use:           "skaffold",
 		Short:         "A tool that facilitates continuous development for Kubernetes applications.",
 		SilenceErrors: true,
-	}
-
-	rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
-		opts.Command = cmd.Use
+		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
+			cmd.Root().SilenceUsage = true
 
-		if err := SetUpLogs(err, v); err != nil {
-			return err
-		}
+			opts.Command = cmd.Use
 
-		if forceColors {
-			color.ForceColors()
-		}
+			// Setup colors
+			if forceColors {
+				color.ForceColors()
+			}
+			color.OverwriteDefault(color.Color(defaultColor))
 
-		rootCmd.SilenceUsage = true
-		logrus.Infof("Skaffold %+v", version.Get())
-		color.OverwriteDefault(color.Color(defaultColor))
+			// Setup logs
+			if err := setUpLogs(err, v); err != nil {
+				return err
+			}
 
-		if quietFlag {
-			logrus.Debugf("Update check is disabled because of quiet mode")
-		} else {
-			go func() {
-				if err := updateCheck(updateMsg); err != nil {
-					logrus.Infof("update check failed: %s", err)
-				}
-			}()
-		}
+			// Start API Server
+			if cmd.Use == "dev" {
+				// TODO(dgageot): api server is always started in dev mode, right now.
+				// It should instead default to true.
+				opts.EnableRPC = true
+			}
+			shutdown, err := server.Initialize(opts)
+			if err != nil {
+				return errors.Wrap(err, "initializing api server")
+			}
+			shutdownAPIServer = shutdown
+
+			// Print version
+			version := version.Get()
+			logrus.Infof("Skaffold %+v", version)
+			event.LogSkaffoldMetadata(version)
+
+			if quietFlag {
+				logrus.Debugf("Update check is disabled because of quiet mode")
+			} else {
+				go func() {
+					if err := updateCheck(updateMsg); err != nil {
+						logrus.Infof("update check failed: %s", err)
+					}
+				}()
+			}
 
-		return nil
-	}
+			return nil
+		},
+		PersistentPostRun: func(cmd *cobra.Command, args []string) {
+			select {
+			case msg := <-updateMsg:
+				fmt.Fprintf(out, "%s\n", msg)
+			default:
+			}
 
-	rootCmd.PersistentPostRun = func(cmd *cobra.Command, args []string) {
-		select {
-		case msg := <-updateMsg:
-			fmt.Fprintf(out, "%s\n", msg)
-		default:
-		}
+			if shutdownAPIServer != nil {
+				shutdownAPIServer()
+			}
+		},
 	}
 
 	SetUpFlags()
@@ -158,9 +181,9 @@ func FlagToEnvVarName(f *pflag.Flag) string {
 	return fmt.Sprintf("SKAFFOLD_%s", strings.Replace(strings.ToUpper(f.Name), "-", "_", -1))
 }
 
-func SetUpLogs(out io.Writer, level string) error {
+func setUpLogs(out io.Writer, level string) error {
 	logrus.SetOutput(out)
-	lvl, err := logrus.ParseLevel(v)
+	lvl, err := logrus.ParseLevel(level)
 	if err != nil {
 		return errors.Wrap(err, "parsing log level")
 	}

cmd/skaffold/app/cmd/dev.go

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

cmd/skaffold/app/cmd/runner.go

@@ -42,9 +42,9 @@ func withRunner(ctx context.Context, action func(runner.Runner, *latest.Skaffold
 	if err != nil {
 		return errors.Wrap(err, "creating runner")
 	}
-	defer runner.Stop()
 
 	err = action(runner, config)
+
 	return alwaysSucceedWhenCancelled(ctx, err)
 }
 

pkg/skaffold/build/local/local_test.go

@@ -222,17 +222,12 @@ func TestLocalRun(t *testing.T) {
 			fakeWarner := &warnings.Collect{}
 			t.Override(&warnings.Printf, fakeWarner.Warnf)
 
-			cfg := latest.BuildConfig{
+			event.InitializeState(latest.BuildConfig{
 				BuildType: latest.BuildType{
 					LocalBuild: &latest.LocalBuild{},
 				},
-			}
-			event.InitializeState(&runcontext.RunContext{
-				Cfg: &latest.Pipeline{
-					Build: cfg,
-				},
-				Opts: &config.SkaffoldOptions{},
 			})
+
 			l := Builder{
 				cfg:         &latest.LocalBuild{},
 				localDocker: docker.NewLocalDaemon(&test.api, nil, false, map[string]bool{}),

pkg/skaffold/build/sequence_test.go

@@ -25,9 +25,7 @@ import (
 	"testing"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
-	runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/context"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
@@ -143,15 +141,9 @@ func (t *concatTagger) doBuild(ctx context.Context, out io.Writer, artifact *lat
 }
 
 func initializeEvents() {
-	cfg := latest.BuildConfig{
+	event.InitializeState(latest.BuildConfig{
 		BuildType: latest.BuildType{
 			LocalBuild: &latest.LocalBuild{},
 		},
-	}
-	event.InitializeState(&runcontext.RunContext{
-		Cfg: &latest.Pipeline{
-			Build: cfg,
-		},
-		Opts: &config.SkaffoldOptions{},
 	})
 }

pkg/skaffold/deploy/helm_test.go

@@ -459,7 +459,7 @@ func TestHelmDeploy(t *testing.T) {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			t.Override(&util.DefaultExecCommand, test.cmd)
 
-			event.InitializeState(test.runContext)
+			event.InitializeState(test.runContext.Cfg.Build)
 			err := NewHelmDeployer(test.runContext).Deploy(context.Background(), ioutil.Discard, test.builds, nil)
 
 			t.CheckError(test.shouldErr, err)

pkg/skaffold/event/event.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"sync"
 
-	runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/context"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/server/proto"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/version"
@@ -35,10 +34,7 @@ const (
 	Failed     = "Failed"
 )
 
-var (
-	handler *eventHandler
-	once    sync.Once
-)
+var handler = &eventHandler{}
 
 type eventHandler struct {
 	eventLog []proto.LogEntry
@@ -126,12 +122,10 @@ func (ev *eventHandler) forEachEvent(callback func(*proto.LogEntry) error) error
 	return <-listener.errors
 }
 
-func emptyState(build *latest.BuildConfig) proto.State {
+func emptyState(build latest.BuildConfig) proto.State {
 	builds := map[string]string{}
-	if build != nil {
-		for _, a := range build.Artifacts {
-			builds[a.ImageName] = NotStarted
-		}
+	for _, a := range build.Artifacts {
+		builds[a.ImageName] = NotStarted
 	}
 
 	return proto.State{
@@ -146,12 +140,8 @@ func emptyState(build *latest.BuildConfig) proto.State {
 }
 
 // InitializeState instantiates the global state of the skaffold runner, as well as the event log.
-func InitializeState(runCtx *runcontext.RunContext) {
-	once.Do(func() {
-		handler = &eventHandler{
-			state: emptyState(&runCtx.Cfg.Build),
-		}
-	})
+func InitializeState(build latest.BuildConfig) {
+	handler.setState(emptyState(build))
 }
 
 // DeployInProgress notifies that a deployment has been started.
@@ -202,6 +192,12 @@ func PortForwarded(localPort, remotePort int32, podName, containerName, namespac
 	})
 }
 
+func (ev *eventHandler) setState(state proto.State) {
+	ev.stateLock.Lock()
+	ev.state = state
+	ev.stateLock.Unlock()
+}
+
 func (ev *eventHandler) handleDeployEvent(e *proto.DeployEvent) {
 	go ev.handle(&proto.Event{
 		EventType: &proto.Event_DeployEvent{

pkg/skaffold/event/event_test.go

@@ -55,7 +55,7 @@ func TestGetLogEvents(t *testing.T) {
 
 func TestGetState(t *testing.T) {
 	ev := &eventHandler{
-		state: emptyState(nil),
+		state: emptyState(latest.BuildConfig{}),
 	}
 
 	ev.stateLock.Lock()
@@ -68,10 +68,10 @@ func TestGetState(t *testing.T) {
 }
 
 func TestDeployInProgress(t *testing.T) {
-	defer func() { handler = nil }()
+	defer func() { handler = &eventHandler{} }()
 
 	handler = &eventHandler{
-		state: emptyState(nil),
+		state: emptyState(latest.BuildConfig{}),
 	}
 
 	wait(t, func() bool { return handler.getState().DeployState.Status == NotStarted })
@@ -80,10 +80,10 @@ func TestDeployInProgress(t *testing.T) {
 }
 
 func TestDeployFailed(t *testing.T) {
-	defer func() { handler = nil }()
+	defer func() { handler = &eventHandler{} }()
 
 	handler = &eventHandler{
-		state: emptyState(nil),
+		state: emptyState(latest.BuildConfig{}),
 	}
 
 	wait(t, func() bool { return handler.getState().DeployState.Status == NotStarted })
@@ -92,10 +92,10 @@ func TestDeployFailed(t *testing.T) {
 }
 
 func TestDeployComplete(t *testing.T) {
-	defer func() { handler = nil }()
+	defer func() { handler = &eventHandler{} }()
 
 	handler = &eventHandler{
-		state: emptyState(nil),
+		state: emptyState(latest.BuildConfig{}),
 	}
 
 	wait(t, func() bool { return handler.getState().DeployState.Status == NotStarted })
@@ -104,10 +104,10 @@ func TestDeployComplete(t *testing.T) {
 }
 
 func TestBuildInProgress(t *testing.T) {
-	defer func() { handler = nil }()
+	defer func() { handler = &eventHandler{} }()
 
 	handler = &eventHandler{
-		state: emptyState(&latest.BuildConfig{
+		state: emptyState(latest.BuildConfig{
 			Artifacts: []*latest.Artifact{{
 				ImageName: "img",
 			}},
@@ -120,10 +120,10 @@ func TestBuildInProgress(t *testing.T) {
 }
 
 func TestBuildFailed(t *testing.T) {
-	defer func() { handler = nil }()
+	defer func() { handler = &eventHandler{} }()
 
 	handler = &eventHandler{
-		state: emptyState(&latest.BuildConfig{
+		state: emptyState(latest.BuildConfig{
 			Artifacts: []*latest.Artifact{{
 				ImageName: "img",
 			}},
@@ -136,10 +136,10 @@ func TestBuildFailed(t *testing.T) {
 }
 
 func TestBuildComplete(t *testing.T) {
-	defer func() { handler = nil }()
+	defer func() { handler = &eventHandler{} }()
 
 	handler = &eventHandler{
-		state: emptyState(&latest.BuildConfig{
+		state: emptyState(latest.BuildConfig{
 			Artifacts: []*latest.Artifact{{
 				ImageName: "img",
 			}},
@@ -152,10 +152,10 @@ func TestBuildComplete(t *testing.T) {
 }
 
 func TestPortForwarded(t *testing.T) {
-	defer func() { handler = nil }()
+	defer func() { handler = &eventHandler{} }()
 
 	handler = &eventHandler{
-		state: emptyState(nil),
+		state: emptyState(latest.BuildConfig{}),
 	}
 
 	wait(t, func() bool { return handler.getState().ForwardedPorts["container"] == nil })

pkg/skaffold/kubernetes/portforward/pod_forwarder_test.go

@@ -27,7 +27,6 @@ import (
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
-	runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/context"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 	v1 "k8s.io/api/core/v1"
@@ -436,7 +435,7 @@ func TestAutomaticPortForwardPod(t *testing.T) {
 	}
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
-			event.InitializeState(&runcontext.RunContext{Cfg: &latest.Pipeline{Build: latest.BuildConfig{}}})
+			event.InitializeState(latest.BuildConfig{})
 			taken := map[int]struct{}{}
 
 			forwardingTimeoutTime = time.Second
@@ -499,7 +498,7 @@ func TestStartPodForwarder(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			event.InitializeState(&runcontext.RunContext{Cfg: &latest.Pipeline{Build: latest.BuildConfig{}}})
+			event.InitializeState(latest.BuildConfig{})
 			client := fakekubeclientset.NewSimpleClientset(&v1.Pod{})
 			fakeWatcher := watch.NewRaceFreeFake()
 			client.PrependWatchReactor("*", testutil.SetupFakeWatcher(fakeWatcher))