Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start API server only once #2382

Merged
merged 1 commit into from
Jul 1, 2019
Merged

Start API server only once #2382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 55 additions & 32 deletions cmd/skaffold/app/cmd/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean default to false, if the api server is always started right now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean it should default to true for dev mode but the user should be able to disable it. Currently, it's always on for dev mode

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about either

  • changing the --enable-rpc flag to a *bool, so we can tell if the user didn't provide the flag (vs. explicitly providing --enable-rpc=false, or
  • adding a separate flag --disable-rpc, and checking that here e.g. if cmd.Use == "dev" && opts.DisableRPC == false. we'd have to have a way to make sure users don't set --enable-rpc and --disable-rpc at the same time though (don't know if there's an easy way to do this using cobra)

I'm fine with this being out of scope for this PR though.

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()
Expand Down Expand Up @@ -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")
}
Expand Down
2 changes: 0 additions & 2 deletions cmd/skaffold/app/cmd/dev.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion cmd/skaffold/app/cmd/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
9 changes: 2 additions & 7 deletions pkg/skaffold/build/local/local_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{}),
Expand Down
10 changes: 1 addition & 9 deletions pkg/skaffold/build/sequence_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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{},
})
}
2 changes: 1 addition & 1 deletion pkg/skaffold/deploy/helm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 12 additions & 16 deletions pkg/skaffold/event/event.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -35,10 +34,7 @@ const (
Failed = "Failed"
)

var (
handler *eventHandler
once sync.Once
)
var handler = &eventHandler{}

type eventHandler struct {
eventLog []proto.LogEntry
Expand Down Expand Up @@ -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{
Expand All @@ -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.
Expand Down Expand Up @@ -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{
Expand Down
30 changes: 15 additions & 15 deletions pkg/skaffold/event/event_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 })
Expand All @@ -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 })
Expand All @@ -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 })
Expand All @@ -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",
}},
Expand All @@ -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",
}},
Expand All @@ -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",
}},
Expand All @@ -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 })
Expand Down
5 changes: 2 additions & 3 deletions pkg/skaffold/kubernetes/portforward/pod_forwarder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down
Loading