From a590d93e763764b26bafc7e14aa9a46e0361d37e Mon Sep 17 00:00:00 2001 From: Tom Wiesing Date: Wed, 14 Dec 2022 08:53:45 +0100 Subject: [PATCH] environment/exec: Seperate Exec and Wait --- cmd/mysql.go | 5 +- cmd/shell.go | 5 +- cmd/system_update.go | 2 +- internal/dis/component/sql/backup.go | 5 +- internal/dis/component/sql/snapshot.go | 5 +- internal/dis/component/sql/update.go | 12 +-- internal/dis/component/stack.go | 62 ++++---------- .../wisski/ingredient/barrel/drush/cron.go | 8 +- .../wisski/ingredient/barrel/drush/update.go | 6 +- internal/wisski/ingredient/barrel/shell.go | 2 +- internal/wisski/ingredient/php/server.go | 4 +- pkg/environment/environment.go | 2 +- pkg/environment/globals.go | 7 +- pkg/environment/native_exec.go | 82 +++++++++++-------- 14 files changed, 90 insertions(+), 117 deletions(-) diff --git a/cmd/mysql.go b/cmd/mysql.go index 962cd6d..4a8ce8d 100644 --- a/cmd/mysql.go +++ b/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...) - if err != nil { - return err - } + code := context.Environment.SQL().Shell(context.Context, context.IOStream, ms.Positionals.Args...) if code != 0 { return exit.Error{ ExitCode: exit.ExitCode(uint8(code)), diff --git a/cmd/shell.go b/cmd/shell.go index 93925bc..d1a01c9 100644 --- a/cmd/shell.go +++ b/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...) - if err != nil { - return errShell.WithMessageF(err) - } + code := instance.Barrel().Shell(context.Context, context.IOStream, sh.Positionals.Args...)() if code != 0 { return exit.Error{ ExitCode: exit.ExitCode(uint8(code)), diff --git a/cmd/system_update.go b/cmd/system_update.go index 3d54673..2af3116 100644 --- a/cmd/system_update.go +++ b/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) diff --git a/internal/dis/component/sql/backup.go b/internal/dis/component/sql/backup.go index 1db5149..a6d168c 100644 --- a/internal/dis/component/sql/backup.go +++ b/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") - if err != nil { - return err - } + code := sql.Stack(sql.Environment).Exec(ctx, stream.NonInteractive(scontext.Progress()), "sql", "mysqldump", "--all-databases")() if code != 0 { return errSQLBackup } diff --git a/internal/dis/component/sql/snapshot.go b/internal/dis/component/sql/snapshot.go index 200f388..51f8f8c 100644 --- a/internal/dis/component/sql/snapshot.go +++ b/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) - if err != nil { - return err - } + code := sql.Stack(sql.Environment).Exec(ctx, stream.NonInteractive(progress), "sql", "mysqldump", "--databases", database)() if code != 0 { return errSQLBackup } diff --git a/internal/dis/component/sql/update.go b/internal/dis/component/sql/update.go index 9507974..6594f18 100644 --- a/internal/dis/component/sql/update.go +++ b/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) { - return sql.Stack(sql.Environment).Exec(ctx, io, "sql", "mysql", argv...) +func (sql *SQL) Shell(ctx context.Context, io stream.IOStream, argv ...string) int { + 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;") - return err == nil && code == 0 + code := sql.Shell(ctx, n, "-e", "select 1;") + 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) - return err == nil && code == 0 + code := sql.Shell(ctx, stream.FromNil(), "-e", query) + return code == 0 } var errSQLUnableToCreateUser = errors.New("unable to create administrative user") diff --git a/internal/dis/component/stack.go b/internal/dis/component/stack.go index 3f5283d..84bcc32 100644 --- a/internal/dis/component/stack.go +++ b/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()) - if err != nil { - return err - } + code := ds.compose(ctx, stream.NonInteractive(progress), "kill", service, "-s", signal.String())() 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 { - { - code, err := ds.compose(ctx, stream.NonInteractive(progress), "pull") - if err != nil { - return err - } - if code != 0 { - return errStackUpdatePull - } + if code := ds.compose(ctx, stream.NonInteractive(progress), "pull")(); code != 0 { + return errStackUpdatePull } - { - code, err := ds.compose(ctx, stream.NonInteractive(progress), "build", "--pull") - if err != nil { - return err - } - if code != 0 { - return errStackUpdateBuild - } + if code := ds.compose(ctx, stream.NonInteractive(progress), "build", "--pull")(); 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 err != nil { - return err - } - if code != 0 { + if code := ds.compose(ctx, stream.NonInteractive(progress), "up", "--remove-orphans", "--detach")(); 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...) - if err != nil { - return environment.ExecCommandError, nil - } + code := ds.compose(ctx, io, compose...)() 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") - if err != nil { - return err - } + code := ds.compose(ctx, stream.NonInteractive(progress), "restart")() 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") - if err != nil { - return nil, err - } + code := ds.compose(ctx, stream.NewIOStream(&buffer, progress, nil, 0), "ps", "-q")() 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") - if err != nil { - return err - } + code := ds.compose(ctx, stream.NonInteractive(progress), "down", "-v")() 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. diff --git a/internal/wisski/ingredient/barrel/drush/cron.go b/internal/wisski/ingredient/barrel/drush/cron.go index bbb02dc..7851823 100644 --- a/internal/wisski/ingredient/barrel/drush/cron.go +++ b/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") - if err != nil { - logging.ProgressF(progress, ctx, "%v", err) - } + code := drush.Barrel.Shell(ctx, stream.NonInteractive(progress), "/runtime/cron.sh")() 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", err) + logging.ProgressF(progress, ctx, "%v", errCronFailed.WithMessageF(drush.Slug, code)) } return nil diff --git a/internal/wisski/ingredient/barrel/drush/update.go b/internal/wisski/ingredient/barrel/drush/update.go index 29f4c3c..a150a1c 100644 --- a/internal/wisski/ingredient/barrel/drush/update.go +++ b/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") - if err != nil { - return errBlindUpdateFailed.WithMessageF(drush.Slug, environment.ExecCommandError) - } + code := drush.Barrel.Shell(ctx, stream.NonInteractive(progress), "/runtime/blind_update.sh")() if code != 0 { return errBlindUpdateFailed.WithMessageF(drush.Slug, code) } diff --git a/internal/wisski/ingredient/barrel/shell.go b/internal/wisski/ingredient/barrel/shell.go index 5e87514..66aef1b 100644 --- a/internal/wisski/ingredient/barrel/shell.go +++ b/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...)...) } diff --git a/internal/wisski/ingredient/php/server.go b/internal/wisski/ingredient/php/server.go index ea79094..515225d 100644 --- a/internal/wisski/ingredient/php/server.go +++ b/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})) - return err + php.Barrel.Shell(ctx, str, "-c", shellescape.QuoteCommand([]string{"drush", "php:eval", code}))() + return nil } diff --git a/pkg/environment/environment.go b/pkg/environment/environment.go index 992ed5e..68c47f0 100644 --- a/pkg/environment/environment.go +++ b/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) } diff --git a/pkg/environment/globals.go b/pkg/environment/globals.go index f7a47f3..9499498 100644 --- a/pkg/environment/globals.go +++ b/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 } diff --git a/pkg/environment/native_exec.go b/pkg/environment/native_exec.go index a0506a6..f3b0fca 100644 --- a/pkg/environment/native_exec.go +++ b/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. -// If the command can not be executed, returns [ExecCommandError]. -func (*Native) Exec(ctx context.Context, io stream.IOStream, workdir string, exe string, argv ...string) int { +// The command is started immediatly. +// The returned function is guaranteed to be non-nil and returns an exit code. +// +// 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 - err, cErr := cancel.WithContext(ctx, func(cancelable func()) error { - // start the process - 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 + // context is already cancelled, don't run it! + if err := ctx.Err(); err != nil { + return ExecCommandErrorFunc } - // non-zero exit - if err, ok := err.(*exec.ExitError); ok { - return err.ExitCode() - } - - // unknown error + // start the command, but if something happens, return nil + err := cmd.Start() + zerolog.Ctx(ctx).Debug().Str("exe", exe).Strs("argv", argv).Err(err).Msg("exec.Command.Start") if err != nil { - return ExecCommandError + return ExecCommandErrorFunc } - // everything is fine! - return 0 + waitdone := make(chan struct{}) // closed once Wait() below returns + 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) {