environment/exec: Seperate Exec and Wait

cmd/mysql.go

@@ -32,10 +32,7 @@ func (mysql) Description() wisski_distillery.Description {
 }
 
 func (ms mysql) Run(context wisski_distillery.Context) error {
-	code, err := context.Environment.SQL().Shell(context.Context, context.IOStream, ms.Positionals.Args...)
+	code := context.Environment.SQL().Shell(context.Context, context.IOStream, ms.Positionals.Args...)
-	if err != nil {
-		return err
-	}
 	if code != 0 {
 		return exit.Error{
 			ExitCode: exit.ExitCode(uint8(code)),

cmd/shell.go

@@ -43,10 +43,7 @@ func (sh shell) Run(context wisski_distillery.Context) error {
 		return err
 	}
 
-	code, err := instance.Barrel().Shell(context.Context, context.IOStream, sh.Positionals.Args...)
+	code := instance.Barrel().Shell(context.Context, context.IOStream, sh.Positionals.Args...)()
-	if err != nil {
-		return errShell.WithMessageF(err)
-	}
 	if code != 0 {
 		return exit.Error{
 			ExitCode: exit.ExitCode(uint8(code)),

cmd/system_update.go

@@ -194,7 +194,7 @@ func (si systemupdate) mustExec(context wisski_distillery.Context, workdir strin
 	if workdir == "" {
 		workdir = dis.Config.DeployRoot
 	}
-	code := dis.Still.Environment.Exec(context.Context, context.IOStream, workdir, exe, argv...)
+	code := dis.Still.Environment.Exec(context.Context, context.IOStream, workdir, exe, argv...)()
 	if code != 0 {
 		err := errMustExecFailed.WithMessageF(code)
 		err.ExitCode = exit.ExitCode(code)

internal/dis/component/sql/backup.go

@@ -18,10 +18,7 @@ func (*SQL) BackupName() string {
 // Backup makes a backup of all SQL databases into the path dest.
 func (sql *SQL) Backup(scontext component.StagingContext) error {
 	return scontext.AddFile("", func(ctx context.Context, file io.Writer) error {
-		code, err := sql.Stack(sql.Environment).Exec(ctx, stream.NonInteractive(scontext.Progress()), "sql", "mysqldump", "--all-databases")
+		code := sql.Stack(sql.Environment).Exec(ctx, stream.NonInteractive(scontext.Progress()), "sql", "mysqldump", "--all-databases")()
-		if err != nil {
-			return err
-		}
 		if code != 0 {
 			return errSQLBackup
 		}

internal/dis/component/sql/snapshot.go

@@ -23,10 +23,7 @@ func (sql *SQL) Snapshot(wisski models.Instance, scontext component.StagingConte
 
 // SnapshotDB makes a backup of the sql database into dest.
 func (sql *SQL) SnapshotDB(ctx context.Context, progress io.Writer, dest io.Writer, database string) error {
-	code, err := sql.Stack(sql.Environment).Exec(ctx, stream.NonInteractive(progress), "sql", "mysqldump", "--databases", database)
+	code := sql.Stack(sql.Environment).Exec(ctx, stream.NonInteractive(progress), "sql", "mysqldump", "--databases", database)()
-	if err != nil {
-		return err
-	}
 	if code != 0 {
 		return errSQLBackup
 	}

internal/dis/component/sql/update.go

@@ -18,23 +18,23 @@ import (
 // Shell runs a mysql shell with the provided databases.
 //
 // NOTE(twiesing): This command should not be used to connect to the database or execute queries except in known situations.
-func (sql *SQL) Shell(ctx context.Context, io stream.IOStream, argv ...string) (int, error) {
+func (sql *SQL) Shell(ctx context.Context, io stream.IOStream, argv ...string) int {
-	return sql.Stack(sql.Environment).Exec(ctx, io, "sql", "mysql", argv...)
+	return sql.Stack(sql.Environment).Exec(ctx, io, "sql", "mysql", argv...)()
 }
 
 // unsafeWaitShell waits for a connection via the database shell to succeed
 func (sql *SQL) unsafeWaitShell(ctx context.Context) error {
 	n := stream.FromNil()
 	return timex.TickUntilFunc(func(time.Time) bool {
-		code, err := sql.Shell(ctx, n, "-e", "select 1;")
+		code := sql.Shell(ctx, n, "-e", "select 1;")
-		return err == nil && code == 0
+		return code == 0
 	}, ctx, sql.PollInterval)
 }
 
 // unsafeQuery shell executes a raw database query.
 func (sql *SQL) unsafeQueryShell(ctx context.Context, query string) bool {
-	code, err := sql.Shell(ctx, stream.FromNil(), "-e", query)
+	code := sql.Shell(ctx, stream.FromNil(), "-e", query)
-	return err == nil && code == 0
+	return code == 0
 }
 
 var errSQLUnableToCreateUser = errors.New("unable to create administrative user")

internal/dis/component/stack.go

@@ -33,10 +33,7 @@ type Stack struct {
 var errStackKill = errors.New("Stack.Kill: Kill returned non-zero exit code")
 
 func (ds Stack) Kill(ctx context.Context, progress io.Writer, service string, signal os.Signal) error {
-	code, err := ds.compose(ctx, stream.NonInteractive(progress), "kill", service, "-s", signal.String())
+	code := ds.compose(ctx, stream.NonInteractive(progress), "kill", service, "-s", signal.String())()
-	if err != nil {
-		return err
-	}
 	if code != 0 {
 		return errStackKill
 	}
@@ -51,25 +48,14 @@ var errStackUpdateBuild = errors.New("Stack.Update: Build returned non-zero exit
 //
 // See also Up.
 func (ds Stack) Update(ctx context.Context, progress io.Writer, start bool) error {
-	{
+	if code := ds.compose(ctx, stream.NonInteractive(progress), "pull")(); code != 0 {
-		code, err := ds.compose(ctx, stream.NonInteractive(progress), "pull")
+		return errStackUpdatePull
-		if err != nil {
-			return err
-		}
-		if code != 0 {
-			return errStackUpdatePull
-		}
 	}
 
-	{
+	if code := ds.compose(ctx, stream.NonInteractive(progress), "build", "--pull")(); code != 0 {
-		code, err := ds.compose(ctx, stream.NonInteractive(progress), "build", "--pull")
+		return errStackUpdateBuild
-		if err != nil {
-			return err
-		}
-		if code != 0 {
-			return errStackUpdateBuild
-		}
 	}
+
 	if start {
 		return ds.Up(ctx, progress)
 	}
@@ -81,11 +67,7 @@ var errStackUp = errors.New("Stack.Up: Up returned non-zero exit code")
 // Up creates and starts the containers in this Stack.
 // It is equivalent to 'docker compose up --remove-orphans --detach' on the shell.
 func (ds Stack) Up(ctx context.Context, progress io.Writer) error {
-	code, err := ds.compose(ctx, stream.NonInteractive(progress), "up", "--remove-orphans", "--detach")
+	if code := ds.compose(ctx, stream.NonInteractive(progress), "up", "--remove-orphans", "--detach")(); code != 0 {
-	if err != nil {
-		return err
-	}
-	if code != 0 {
 		return errStackUp
 	}
 	return nil
@@ -95,14 +77,16 @@ func (ds Stack) Up(ctx context.Context, progress io.Writer) error {
 // It is equivalent to 'docker compose exec $service $executable $args...'.
 //
 // It returns the exit code of the process.
-func (ds Stack) Exec(ctx context.Context, io stream.IOStream, service, executable string, args ...string) (int, error) {
+func (ds Stack) Exec(ctx context.Context, io stream.IOStream, service, executable string, args ...string) func() int {
 	compose := []string{"exec"}
 	if io.StdinIsATerminal() {
 		compose = append(compose, "-ti")
 	}
+
 	compose = append(compose, service)
 	compose = append(compose, executable)
 	compose = append(compose, args...)
+
 	return ds.compose(ctx, io, compose...)
 }
 
@@ -121,10 +105,7 @@ func (ds Stack) Run(ctx context.Context, io stream.IOStream, autoRemove bool, se
 	compose = append(compose, service, command)
 	compose = append(compose, args...)
 
-	code, err := ds.compose(ctx, io, compose...)
+	code := ds.compose(ctx, io, compose...)()
-	if err != nil {
-		return environment.ExecCommandError, nil
-	}
 	return code, nil
 }
 
@@ -133,10 +114,7 @@ var errStackRestart = errors.New("Stack.Restart: Restart returned non-zero exit
 // Restart restarts all containers in this Stack.
 // It is equivalent to 'docker compose restart' on the shell.
 func (ds Stack) Restart(ctx context.Context, progress io.Writer) error {
-	code, err := ds.compose(ctx, stream.NonInteractive(progress), "restart")
+	code := ds.compose(ctx, stream.NonInteractive(progress), "restart")()
-	if err != nil {
-		return err
-	}
 	if code != 0 {
 		return errStackRestart
 	}
@@ -151,10 +129,7 @@ func (ds Stack) Ps(ctx context.Context, progress io.Writer) ([]string, error) {
 	var buffer bytes.Buffer
 
 	// read the ids from the command!
-	code, err := ds.compose(ctx, stream.NewIOStream(&buffer, progress, nil, 0), "ps", "-q")
+	code := ds.compose(ctx, stream.NewIOStream(&buffer, progress, nil, 0), "ps", "-q")()
-	if err != nil {
-		return nil, err
-	}
 	if code != 0 {
 		return nil, errStackPs
 	}
@@ -180,10 +155,7 @@ var errStackDown = errors.New("Stack.Down: Down returned non-zero exit code")
 // Down stops and removes all containers in this Stack.
 // It is equivalent to 'docker compose down -v' on the shell.
 func (ds Stack) Down(ctx context.Context, progress io.Writer) error {
-	code, err := ds.compose(ctx, stream.NonInteractive(progress), "down", "-v")
+	code := ds.compose(ctx, stream.NonInteractive(progress), "down", "-v")()
-	if err != nil {
-		return err
-	}
 	if code != 0 {
 		return errStackDown
 	}
@@ -194,15 +166,15 @@ func (ds Stack) Down(ctx context.Context, progress io.Writer) error {
 //
 // NOTE(twiesing): Check if this can be replaced by an internal call to libcompose.
 // But probably not.
-func (ds Stack) compose(ctx context.Context, io stream.IOStream, args ...string) (int, error) {
+func (ds Stack) compose(ctx context.Context, io stream.IOStream, args ...string) func() int {
 	if ds.DockerExecutable == "" {
 		var err error
 		ds.DockerExecutable, err = ds.Env.LookPathAbs("docker")
 		if err != nil {
-			return environment.ExecCommandError, err
+			return environment.ExecCommandErrorFunc
 		}
 	}
-	return ds.Env.Exec(ctx, io, ds.Dir, ds.DockerExecutable, append([]string{"compose"}, args...)...), nil
+	return ds.Env.Exec(ctx, io, ds.Dir, ds.DockerExecutable, append([]string{"compose"}, args...)...)
 }
 
 // StackWithResources represents a Stack that can be automatically installed from a set of resources.

internal/wisski/ingredient/barrel/drush/cron.go

@@ -20,14 +20,10 @@ var errCronFailed = exit.Error{
 }
 
 func (drush *Drush) Cron(ctx context.Context, progress io.Writer) error {
-	code, err := drush.Barrel.Shell(ctx, stream.NonInteractive(progress), "/runtime/cron.sh")
+	code := drush.Barrel.Shell(ctx, stream.NonInteractive(progress), "/runtime/cron.sh")()
-	if err != nil {
-		logging.ProgressF(progress, ctx, "%v", err)
-	}
 	if code != 0 {
 		// keep going, because we want to run as many crons as possible
-		err = errCronFailed.WithMessageF(drush.Slug, code)
+		logging.ProgressF(progress, ctx, "%v", errCronFailed.WithMessageF(drush.Slug, code))
-		logging.ProgressF(progress, ctx, "%v", err)
 	}
 
 	return nil

internal/wisski/ingredient/barrel/drush/update.go

@@ -9,7 +9,6 @@ import (
 	"github.com/FAU-CDI/wisski-distillery/internal/status"
 	"github.com/FAU-CDI/wisski-distillery/internal/wisski/ingredient"
 	"github.com/FAU-CDI/wisski-distillery/internal/wisski/ingredient/mstore"
-	"github.com/FAU-CDI/wisski-distillery/pkg/environment"
 	"github.com/tkw1536/goprogram/exit"
 	"github.com/tkw1536/goprogram/stream"
 )
@@ -21,10 +20,7 @@ var errBlindUpdateFailed = exit.Error{
 
 // Update performs a blind drush update
 func (drush *Drush) Update(ctx context.Context, progress io.Writer) error {
-	code, err := drush.Barrel.Shell(ctx, stream.NonInteractive(progress), "/runtime/blind_update.sh")
+	code := drush.Barrel.Shell(ctx, stream.NonInteractive(progress), "/runtime/blind_update.sh")()
-	if err != nil {
-		return errBlindUpdateFailed.WithMessageF(drush.Slug, environment.ExecCommandError)
-	}
 	if code != 0 {
 		return errBlindUpdateFailed.WithMessageF(drush.Slug, code)
 	}

internal/wisski/ingredient/barrel/shell.go

@@ -7,6 +7,6 @@ import (
 )
 
 // Shell executes a shell command inside the instance.
-func (barrel *Barrel) Shell(ctx context.Context, io stream.IOStream, argv ...string) (int, error) {
+func (barrel *Barrel) Shell(ctx context.Context, io stream.IOStream, argv ...string) func() int {
 	return barrel.Stack().Exec(ctx, io, "barrel", "/bin/sh", append([]string{"/user_shell.sh"}, argv...)...)
 }

internal/wisski/ingredient/php/server.go

@@ -21,6 +21,6 @@ func (php *PHP) NewServer() *phpx.Server {
 }
 
 func (php *PHP) spawn(ctx context.Context, str stream.IOStream, code string) error {
-	_, err := php.Barrel.Shell(ctx, str, "-c", shellescape.QuoteCommand([]string{"drush", "php:eval", code}))
+	php.Barrel.Shell(ctx, str, "-c", shellescape.QuoteCommand([]string{"drush", "php:eval", code}))()
-	return err
+	return nil
 }

pkg/environment/environment.go

@@ -45,7 +45,7 @@ type Environment interface {
 	DialContext(context context.Context, network, address string) (net.Conn, error)
 
 	Executable() (string, error)
-	Exec(ctx context.Context, io stream.IOStream, workdir string, exe string, argv ...string) int
+	Exec(ctx context.Context, io stream.IOStream, workdir string, exe string, argv ...string) func() int
 	LookPathAbs(name string) (string, error)
 }
 

pkg/environment/globals.go

@@ -14,6 +14,11 @@ import (
 // This typically hints that the executable cannot be found, but may have other causes.
 const ExecCommandError = 127
 
+// ExecCommandErrorFunc always returns ExecCommandError.
+func ExecCommandErrorFunc() int {
+	return ExecCommandError
+}
+
 // DefaultFilePerm is the default mode to use for files
 const DefaultFilePerm fs.FileMode = 0666
 
@@ -66,5 +71,5 @@ func ReadFile(env Environment, path string) ([]byte, error) {
 
 // MustExec is like Exec, except that it returns true if the command exited successfully, and else false.
 func MustExec(ctx context.Context, env Environment, io stream.IOStream, workdir string, exe string, argv ...string) bool {
-	return env.Exec(ctx, io, workdir, exe, argv...) == 0
+	return env.Exec(ctx, io, workdir, exe, argv...)() == 0
 }

pkg/environment/native_exec.go

@@ -4,15 +4,18 @@ import (
 	"context"
 	"os/exec"
 
-	"github.com/FAU-CDI/wisski-distillery/pkg/cancel"
+	"github.com/rs/zerolog"
 	"github.com/tkw1536/goprogram/stream"
 )
 
 // Exec executes a system command with the specified input/output streams, working directory, and arguments.
 //
-// If the command executes, it's exit code will be returned.
+// The command is started immediatly.
-// If the command can not be executed, returns [ExecCommandError].
+// The returned function is guaranteed to be non-nil and returns an exit code.
-func (*Native) Exec(ctx context.Context, io stream.IOStream, workdir string, exe string, argv ...string) int {
+//
+// If the command executes, the returns the exit code as soon as the process executes.
+// If the command can not be executed, the returned function is [ExecCommandErrorFunc] and returns [ExecCommandError].
+func (*Native) Exec(ctx context.Context, io stream.IOStream, workdir string, exe string, argv ...string) func() int {
 	// setup the command
 	cmd := exec.Command(exe, argv...)
 	cmd.Dir = workdir
@@ -20,40 +23,53 @@ func (*Native) Exec(ctx context.Context, io stream.IOStream, workdir string, exe
 	cmd.Stdout = io.Stdout
 	cmd.Stderr = io.Stderr
 
-	// run the process in a cancelable fashion
+	// context is already cancelled, don't run it!
-	err, cErr := cancel.WithContext(ctx, func(cancelable func()) error {
+	if err := ctx.Err(); err != nil {
-		// start the process
+		return ExecCommandErrorFunc
-		err := cmd.Start()
-		if err != nil {
-			return err
-		}
-
-		// allow it to be cancellable
-		cancelable()
-
-		// and wait for the rest of the process
-		return cmd.Wait()
-	}, func() {
-		if cmd.Process != nil {
-			cmd.Process.Kill()
-		}
-	})
-	if err == nil {
-		err = cErr
 	}
 
-	// non-zero exit
+	// start the command, but if something happens, return nil
-	if err, ok := err.(*exec.ExitError); ok {
+	err := cmd.Start()
-		return err.ExitCode()
+	zerolog.Ctx(ctx).Debug().Str("exe", exe).Strs("argv", argv).Err(err).Msg("exec.Command.Start")
-	}
-
-	// unknown error
 	if err != nil {
-		return ExecCommandError
+		return ExecCommandErrorFunc
 	}
 
-	// everything is fine!
+	waitdone := make(chan struct{}) // closed once Wait() below returns
-	return 0
+	alldone := make(chan struct{})  // closed once the kill goroutine exits
+	go func() {
+		defer close(alldone)
+
+		select {
+		case <-ctx.Done():
+			err := cmd.Process.Kill()
+			zerolog.Ctx(ctx).Debug().Str("exe", exe).Strs("argv", argv).Err(err).Msg("exec.Command.Kill")
+		case <-waitdone:
+		}
+	}()
+
+	// create a new command
+	return func() int {
+		defer func() {
+			// wait for the goroutine to exit
+			close(waitdone)
+			<-alldone
+		}()
+
+		err := cmd.Wait()
+		zerolog.Ctx(ctx).Debug().Str("exe", exe).Strs("argv", argv).Err(err).Msg("exec.Command.Wait")
+
+		// non-zero exit
+		if err, ok := err.(*exec.ExitError); ok {
+			return err.ExitCode()
+		}
+
+		if err != nil {
+			return ExecCommandError
+		}
+
+		return 0
+	}
 }
 
 func (n *Native) LookPathAbs(file string) (string, error) {