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.

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

gracefull shutdown RPC even when build is in error #4384

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 6 additions & 10 deletions cmd/skaffold/app/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,17 @@ import (
)

var (
opts config.SkaffoldOptions
v string
forceColors bool
defaultColor int
overwrite bool
opts config.SkaffoldOptions
v string
forceColors bool
defaultColor int
overwrite bool
shutdownAPIServer func() error
)

func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {
updateMsg := make(chan string)
surveyPrompt := make(chan bool)
var shutdownAPIServer func() error

rootCmd := &cobra.Command{
Use: "skaffold",
Expand Down Expand Up @@ -115,10 +115,6 @@ func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {
}
default:
}

if shutdownAPIServer != nil {
tejal29 marked this conversation as resolved.
Show resolved Hide resolved
shutdownAPIServer()
}
},
}

Expand Down
16 changes: 14 additions & 2 deletions cmd/skaffold/app/cmd/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,27 @@ func (b *builder) Hidden() Builder {
func (b *builder) ExactArgs(argCount int, action func(context.Context, io.Writer, []string) error) *cobra.Command {
b.cmd.Args = cobra.ExactArgs(argCount)
b.cmd.RunE = func(_ *cobra.Command, args []string) error {
return handleWellKnownErrors(action(b.cmd.Context(), b.cmd.OutOrStdout(), args))
err := handleWellKnownErrors(action(b.cmd.Context(), b.cmd.OutOrStdout(), args))
// clean up server at end of the execution since post run hooks are only executed if
// RunE is successful
if shutdownAPIServer != nil {
shutdownAPIServer()
}
return err
}
return &b.cmd
}

func (b *builder) NoArgs(action func(context.Context, io.Writer) error) *cobra.Command {
b.cmd.Args = cobra.NoArgs
b.cmd.RunE = func(*cobra.Command, []string) error {
return handleWellKnownErrors(action(b.cmd.Context(), b.cmd.OutOrStdout()))
err := handleWellKnownErrors(action(b.cmd.Context(), b.cmd.OutOrStdout()))
// clean up server at end of the execution since post run hooks are only executed if
// RunE is successful
if shutdownAPIServer != nil {
shutdownAPIServer()
}
return err
}
return &b.cmd
}
Expand Down
37 changes: 32 additions & 5 deletions pkg/skaffold/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"fmt"
"net"
"net/http"
"time"

"github.com/grpc-ecosystem/grpc-gateway/runtime"
"github.com/sirupsen/logrus"
Expand All @@ -34,7 +35,12 @@ import (
"github.com/GoogleContainerTools/skaffold/proto"
)

var srv *server
var (
srv *server

// waits for 1 second before forcing a server shutdown
forceShutdownTimeout = 1 * time.Second
)

type server struct {
buildIntentCallback func()
Expand Down Expand Up @@ -155,8 +161,21 @@ func newGRPCServer(port int) (func() error, error) {
}
}()
return func() error {
s.Stop()
return l.Close()
ctx, cancel := context.WithTimeout(context.Background(), forceShutdownTimeout)
defer cancel()
ch := make(chan bool, 1)
go func() {
s.GracefulStop()
ch <- true
}()
for {
select {
case <-ctx.Done():
return l.Close()
case <-ch:
return l.Close()
}
}
}, nil
}

Expand All @@ -174,9 +193,17 @@ func newHTTPServer(port, proxyPort int) (func() error, error) {
}
logrus.Infof("starting gRPC HTTP server on port %d", port)

go http.Serve(l, mux)
server := &http.Server{
Handler: mux,
}

return l.Close, nil
go server.Serve(l)

return func() error {
ctx, cancel := context.WithTimeout(context.Background(), forceShutdownTimeout)
defer cancel()
return server.Shutdown(ctx)
}, nil
}

type errResponse struct {
Expand Down