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

Fix update endpoint. #214

Merged
merged 5 commits into from
Mar 9, 2020
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
2 changes: 1 addition & 1 deletion pkg/hypervisor/hypervisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func (hv *Hypervisor) ServeHTTP(w http.ResponseWriter, req *http.Request) {
r.Put("/visors/{pk}/routes/{rid}", hv.putRoute())
r.Delete("/visors/{pk}/routes/{rid}", hv.deleteRoute())
r.Get("/visors/{pk}/routegroups", hv.getRouteGroups())
r.Get("/visors/{pk}/restart", hv.restart())
r.Post("/visors/{pk}/restart", hv.restart())
r.Post("/visors/{pk}/exec", hv.exec())
r.Post("/visors/{pk}/update", hv.update())
})
Expand Down
27 changes: 20 additions & 7 deletions pkg/restart/restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ type Context struct {
// therefore calling CaptureContext immediately after starting executable is recommended.
func CaptureContext() *Context {
cmd := exec.Command(os.Args[0], os.Args[1:]...) // nolint:gosec

cmd.Stdout = os.Stdout
cmd.Stdin = os.Stdin
cmd.Stderr = os.Stderr
Expand Down Expand Up @@ -70,24 +69,38 @@ func (c *Context) CmdPath() string {
}

// Start starts a new executable using Context.
func (c *Context) Start() error {
func (c *Context) Start() (err error) {
if !atomic.CompareAndSwapInt32(&c.isStarted, 0, 1) {
return ErrAlreadyStarted
}

errCh := c.startExec()

ticker := time.NewTicker(c.checkDelay)
defer ticker.Stop()

select {
case err := <-errCh:
case err = <-errCh:
c.errorLogger()("Failed to start new instance: %v", err)
return err

// Reset c.cmd on failure so it can be reused.
c.cmd = copyCmd(c.cmd)
atomic.StoreInt32(&c.isStarted, 0)

case <-ticker.C:
c.infoLogger()("New instance started successfully, exiting from the old one")
return nil
}

ticker.Stop()
return err
}

func copyCmd(oldCmd *exec.Cmd) *exec.Cmd {
newCmd := exec.Command(oldCmd.Path, oldCmd.Args...) // nolint:gosec
newCmd.Stdout = oldCmd.Stdout
newCmd.Stdin = oldCmd.Stdin
newCmd.Stderr = oldCmd.Stderr
newCmd.Env = oldCmd.Env

return newCmd
}

func (c *Context) startExec() chan error {
Expand Down
8 changes: 3 additions & 5 deletions pkg/restart/restart_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ func TestContext_Start(t *testing.T) {
cc := CaptureContext()
assert.NotZero(t, len(cc.cmd.Args))

cmd := "touch"
path := "/tmp/test_start"
cc.cmd = exec.Command(cmd, path) // nolint:gosec
cmd := "sleep"
duration := "5"
cc.cmd = exec.Command(cmd, duration) // nolint:gosec
cc.appendDelay = false

errCh := make(chan error, 1)
Expand All @@ -82,8 +82,6 @@ func TestContext_Start(t *testing.T) {

assert.Contains(t, errors, ErrAlreadyStarted)
assert.Contains(t, errors, nil)

assert.NoError(t, os.Remove(path))
})
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/util/updater/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type Updater struct {
log *logging.Logger
restartCtx *restart.Context
appsPath string
updating uint32
updating int32
}

// New returns a new Updater.
Expand All @@ -72,9 +72,10 @@ func New(log *logging.Logger, restartCtx *restart.Context, appsPath string) *Upd
// Update performs an update operation.
// NOTE: Update may call os.Exit.
func (u *Updater) Update() (err error) {
if !atomic.CompareAndSwapUint32(&u.updating, 0, 1) {
if !atomic.CompareAndSwapInt32(&u.updating, 0, 1) {
return ErrAlreadyStarted
}
defer atomic.StoreInt32(&u.updating, 0)

u.log.Infof("Looking for updates")

Expand Down