Skip to content

Commit

Permalink
fix(enter): start default services before executing subcommand
Browse files Browse the repository at this point in the history
The --run option in the enter command should ensure that the default
services have all started before executing the subcommand, as
specified in the help info.

This, however, does not seem to be the case as the current
implementation only sends a request to the API to start the default
services and immediately afterwards, executes the subcommand.
This can have undesired behavior. For example, the following fails:

    $ pebble enter --run stop srv1
    2023-06-01T09:52:52.516Z [pebble] Started daemon.
    2023-06-01T09:52:52.546Z [pebble] POST /v1/services 29.059682ms 202
    2023-06-01T09:52:52.546Z [pebble] Started default services with change 386.
    2023-06-01T09:52:52.565Z [pebble] Service "srv1" starting: <cmd..>
    2023-06-01T09:52:52.584Z [pebble] POST /v1/services 37.51026ms 202
    error: cannot perform the following tasks:
    - Stop service "srv1" (cannot stop service while starting)

It may also happen that the stop subcommand is executed, before the
service hasn't even been initiated/started.

    $ pebble enter --run stop srv99
    2023-06-22T05:46:02.604Z [pebble] Started daemon.
    2023-06-22T05:46:02.619Z [pebble] Service "srv55" starting: <cmd..>
    2023-06-22T05:46:02.629Z [pebble] POST /v1/services 23.863368ms 202
    2023-06-22T05:46:02.630Z [pebble] Started default services with change 9.
    2023-06-22T05:46:02.637Z [pebble] Service "srv1" starting: <cmd..>
    2023-06-22T05:46:02.646Z [pebble] POST /v1/services 14.761043ms 202
    2023-06-22T11:46:02+06:00 INFO Service "srv99" has never been started.

This commit should ensure that the default services have all started,
before executing the subcommand. It does so by ensuring the "change"
made by the "autostart" request is ready, before running the
subcommand. The status of the "change" can be "Error" once it's ready;
the subcommand will still be executed after.
  • Loading branch information
rebornplusplus committed Jul 7, 2023
1 parent 37e791e commit c9e980b
Showing 1 changed file with 18 additions and 6 deletions.
24 changes: 18 additions & 6 deletions internals/cli/cmd_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,9 @@ func runDaemon(rcmd *cmdRun, ch chan os.Signal, ready chan<- func()) error {

logger.Debugf("activation done in %v", time.Now().Truncate(time.Millisecond).Sub(t0))

var autoStartReady chan struct{}
var stop chan struct{}

if !rcmd.Hold {
servopts := client.ServiceOptions{}
changeID, err := rcmd.client.AutoStart(&servopts)
Expand All @@ -202,13 +205,18 @@ func runDaemon(rcmd *cmdRun, ch chan os.Signal, ready chan<- func()) error {
} else {
logger.Noticef("Started default services with change %s.", changeID)
}
}

var stop chan struct{}
if ready != nil {
stop = make(chan struct{}, 1)
ready <- func() { close(stop) }
close(ready)
if ready != nil {
autoStartReady = make(chan struct{}, 1)
go func() {
waitCmd := waitMixin{
hideProgress: true,
}
waitCmd.setClient(rcmd.client)
waitCmd.wait(changeID)
autoStartReady <- struct{}{}
}()
}
}

out:
Expand All @@ -226,6 +234,10 @@ out:
d.SetDegradedMode(nil)
tic.Stop()
}
case <-autoStartReady:
stop = make(chan struct{}, 1)
ready <- func() { close(stop) }
close(ready)
case <-stop:
break out
}
Expand Down

0 comments on commit c9e980b

Please sign in to comment.