enter: start default services before executing subcommand (#257)

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 behaviour. 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.

internals/cli/cmd_enter_test.go

@@ -19,6 +19,7 @@ import (
 	"path/filepath"
 	"strings"
 	"text/template"
+	"time"
 
 	. "gopkg.in/check.v1"
 
@@ -241,3 +242,38 @@ func (s *PebbleSuite) TestEnterHelpCommandHelpArg(c *C) {
 	c.Check(stdout, Matches, "(?s).*\\bThe help command displays information about commands\\..*")
 	c.Check(exitCode, Equals, 0)
 }
+
+// TestEnterSubCommandWaits checks that the subcommand in enter
+// starts **after** the default services have started.
+func (s *PebbleSuite) TestEnterSubCommandWaits(c *C) {
+	layerTemplate := dumbDedent(`
+		services:
+		  stat:
+		    override: replace
+		    command: /bin/sh -c 'date --rfc-3339=ns > $PEBBLE/enter-wait; sleep 1;'
+		    startup: enabled
+	`)
+	layerPath := filepath.Join(s.pebbleDir, "layers", "001-stat.yaml")
+	writeTemplate(layerPath, layerTemplate, nil)
+
+	cmd := []string{"pebble", "enter", "--run", "exec", "date", "--rfc-3339=ns"}
+	restore := fakeArgs(cmd...)
+	defer restore()
+
+	exitCode := cli.PebbleMain()
+	c.Check(exitCode, Equals, 0)
+	// stderr is written to stdout buffer because of "combine stderr" mode,
+	// see cmd/pebble/cmd_exec.go:163
+	c.Check(s.Stderr(), Equals, "")
+	stdout := s.Stdout()
+
+	svcOut, err := os.ReadFile(filepath.Join(s.pebbleDir, "enter-wait"))
+	c.Check(err, IsNil)
+
+	layout := "2006-01-02 15:04:05.000000000-07:00"
+	subCmdExecTime, err := time.Parse(layout, strings.TrimSpace(stdout))
+	c.Check(err, IsNil)
+	svcStartTime, err := time.Parse(layout, strings.TrimSpace(string(svcOut)))
+	c.Check(err, IsNil)
+	c.Check(svcStartTime.Before(subCmdExecTime), Equals, true)
+}

internals/cli/cmd_run.go

@@ -229,21 +229,40 @@ func runDaemon(rcmd *cmdRun, ch chan os.Signal, ready chan<- func()) (err error)
 
 	logger.Debugf("activation done in %v", time.Now().Truncate(time.Millisecond).Sub(t0))
 
+	// The "stop" channel is used by the "enter" command to stop the daemon.
+	var stop chan struct{}
+	if ready != nil {
+		stop = make(chan struct{}, 1)
+	}
+	notifyReady := func() {
+		ready <- func() { close(stop) }
+		close(ready)
+	}
+
 	if !rcmd.Hold {
+		// Start the default services (those configured with startup: enabled).
 		servopts := client.ServiceOptions{}
 		changeID, err := rcmd.client.AutoStart(&servopts)
 		if err != nil {
 			logger.Noticef("Cannot start default services: %v", err)
 		} else {
-			logger.Noticef("Started default services with change %s.", changeID)
+			// Wait for the default services to actually start and then notify
+			// the ready channel (for the "enter" command).
+			go func() {
+				logger.Debugf("Waiting for default services to autostart with change %s.", changeID)
+				_, err := rcmd.client.WaitChange(changeID, nil)
+				if err != nil {
+					logger.Noticef("Cannot wait for autostart change %s: %v", changeID, err)
+				} else {
+					logger.Noticef("Started default services with change %s.", changeID)
+				}
+				if ready != nil {
+					notifyReady()
+				}
+			}()
 		}
-	}
-
-	var stop chan struct{}
-	if ready != nil {
-		stop = make(chan struct{}, 1)
-		ready <- func() { close(stop) }
-		close(ready)
+	} else if ready != nil {
+		notifyReady()
 	}
 
 out: