From 7befa2a0048d346f4f97fc457a1dd29c0914e2be Mon Sep 17 00:00:00 2001 From: Paul Tyng Date: Mon, 5 Oct 2020 13:09:56 -0400 Subject: [PATCH] Properly handle context cancellation/deadline and kill --- tfexec/apply.go | 2 +- tfexec/cmd.go | 20 +++----------- tfexec/cmd_default.go | 45 +++++++++++++++++++++++++++++++ tfexec/cmd_linux.go | 54 ++++++++++++++++++++++++++++++++++++++ tfexec/destroy.go | 2 +- tfexec/fmt.go | 6 ++--- tfexec/import.go | 2 +- tfexec/init.go | 2 +- tfexec/output.go | 2 +- tfexec/plan.go | 2 +- tfexec/providers_schema.go | 2 +- tfexec/refresh.go | 2 +- tfexec/show.go | 8 +++--- tfexec/state_mv.go | 2 +- tfexec/upgrade012.go | 2 +- tfexec/validate.go | 9 ++++--- tfexec/version.go | 2 +- tfexec/workspace_list.go | 2 +- tfexec/workspace_new.go | 2 +- tfexec/workspace_select.go | 2 +- 20 files changed, 129 insertions(+), 41 deletions(-) create mode 100644 tfexec/cmd_default.go create mode 100644 tfexec/cmd_linux.go diff --git a/tfexec/apply.go b/tfexec/apply.go index b055f418..82d09d5f 100644 --- a/tfexec/apply.go +++ b/tfexec/apply.go @@ -91,7 +91,7 @@ func (tf *Terraform) Apply(ctx context.Context, opts ...ApplyOption) error { if err != nil { return err } - return tf.runTerraformCmd(cmd) + return tf.runTerraformCmd(ctx, cmd) } func (tf *Terraform) applyCmd(ctx context.Context, opts ...ApplyOption) (*exec.Cmd, error) { diff --git a/tfexec/cmd.go b/tfexec/cmd.go index 3b9005a6..e792dc9c 100644 --- a/tfexec/cmd.go +++ b/tfexec/cmd.go @@ -171,7 +171,8 @@ func (tf *Terraform) buildEnv(mergeEnv map[string]string) []string { } func (tf *Terraform) buildTerraformCmd(ctx context.Context, mergeEnv map[string]string, args ...string) *exec.Cmd { - cmd := exec.CommandContext(ctx, tf.execPath, args...) + cmd := exec.Command(tf.execPath, args...) + cmd.Env = tf.buildEnv(mergeEnv) cmd.Dir = tf.workingDir @@ -180,11 +181,11 @@ func (tf *Terraform) buildTerraformCmd(ctx context.Context, mergeEnv map[string] return cmd } -func (tf *Terraform) runTerraformCmdJSON(cmd *exec.Cmd, v interface{}) error { +func (tf *Terraform) runTerraformCmdJSON(ctx context.Context, cmd *exec.Cmd, v interface{}) error { var outbuf = bytes.Buffer{} cmd.Stdout = mergeWriters(cmd.Stdout, &outbuf) - err := tf.runTerraformCmd(cmd) + err := tf.runTerraformCmd(ctx, cmd) if err != nil { return err } @@ -194,19 +195,6 @@ func (tf *Terraform) runTerraformCmdJSON(cmd *exec.Cmd, v interface{}) error { return dec.Decode(v) } -func (tf *Terraform) runTerraformCmd(cmd *exec.Cmd) error { - var errBuf strings.Builder - - cmd.Stdout = mergeWriters(cmd.Stdout, tf.stdout) - cmd.Stderr = mergeWriters(cmd.Stderr, tf.stderr, &errBuf) - - err := cmd.Run() - if err != nil { - return tf.parseError(err, errBuf.String()) - } - return nil -} - // mergeUserAgent does some minor deduplication to ensure we aren't // just using the same append string over and over. func mergeUserAgent(uas ...string) string { diff --git a/tfexec/cmd_default.go b/tfexec/cmd_default.go new file mode 100644 index 00000000..facf22b9 --- /dev/null +++ b/tfexec/cmd_default.go @@ -0,0 +1,45 @@ +// +build !linux + +package tfexec + +import ( + "context" + "os/exec" + "strings" +) + +func (tf *Terraform) runTerraformCmd(ctx context.Context, cmd *exec.Cmd) error { + var errBuf strings.Builder + + cmd.Stdout = mergeWriters(cmd.Stdout, tf.stdout) + cmd.Stderr = mergeWriters(cmd.Stderr, tf.stderr, &errBuf) + + go func() { + <-ctx.Done() + if ctx.Err() == context.DeadlineExceeded || ctx.Err() == context.Canceled { + if cmd != nil && cmd.Process != nil && cmd.ProcessState != nil { + err := cmd.Process.Kill() + if err != nil { + tf.logger.Printf("error from kill: %s", err) + } + } + } + }() + + // check for early cancellation + select { + case <-ctx.Done(): + return ctx.Err() + default: + } + + err := cmd.Run() + if err == nil && ctx.Err() != nil { + err = ctx.Err() + } + if err != nil { + return tf.parseError(err, errBuf.String()) + } + + return nil +} diff --git a/tfexec/cmd_linux.go b/tfexec/cmd_linux.go new file mode 100644 index 00000000..267099f4 --- /dev/null +++ b/tfexec/cmd_linux.go @@ -0,0 +1,54 @@ +package tfexec + +import ( + "context" + "os/exec" + "strings" + "syscall" +) + +func (tf *Terraform) runTerraformCmd(ctx context.Context, cmd *exec.Cmd) error { + var errBuf strings.Builder + + cmd.Stdout = mergeWriters(cmd.Stdout, tf.stdout) + cmd.Stderr = mergeWriters(cmd.Stderr, tf.stderr, &errBuf) + + cmd.SysProcAttr = &syscall.SysProcAttr{ + // kill children if parent is dead + Pdeathsig: syscall.SIGKILL, + // set process group ID + Setpgid: true, + } + + go func() { + <-ctx.Done() + if ctx.Err() == context.DeadlineExceeded || ctx.Err() == context.Canceled { + if cmd != nil && cmd.Process != nil && cmd.ProcessState != nil { + // send SIGINT to process group + err := syscall.Kill(-cmd.Process.Pid, syscall.SIGINT) + if err != nil { + tf.logger.Printf("error from SIGINT: %s", err) + } + } + + // TODO: send a kill if it doesn't respond for a bit? + } + }() + + // check for early cancellation + select { + case <-ctx.Done(): + return ctx.Err() + default: + } + + err := cmd.Run() + if err == nil && ctx.Err() != nil { + err = ctx.Err() + } + if err != nil { + return tf.parseError(err, errBuf.String()) + } + + return nil +} diff --git a/tfexec/destroy.go b/tfexec/destroy.go index 61305580..8011c0ba 100644 --- a/tfexec/destroy.go +++ b/tfexec/destroy.go @@ -92,7 +92,7 @@ func (tf *Terraform) Destroy(ctx context.Context, opts ...DestroyOption) error { if err != nil { return err } - return tf.runTerraformCmd(cmd) + return tf.runTerraformCmd(ctx, cmd) } func (tf *Terraform) destroyCmd(ctx context.Context, opts ...DestroyOption) (*exec.Cmd, error) { diff --git a/tfexec/fmt.go b/tfexec/fmt.go index de30890a..10f6cb4c 100644 --- a/tfexec/fmt.go +++ b/tfexec/fmt.go @@ -63,7 +63,7 @@ func (tf *Terraform) Format(ctx context.Context, unformatted io.Reader, formatte cmd.Stdin = unformatted cmd.Stdout = mergeWriters(cmd.Stdout, formatted) - return tf.runTerraformCmd(cmd) + return tf.runTerraformCmd(ctx, cmd) } // FormatWrite attempts to format and modify all config files in the working or selected (via DirOption) directory. @@ -82,7 +82,7 @@ func (tf *Terraform) FormatWrite(ctx context.Context, opts ...FormatOption) erro return err } - return tf.runTerraformCmd(cmd) + return tf.runTerraformCmd(ctx, cmd) } // FormatCheck returns true if the config files in the working or selected (via DirOption) directory are already formatted. @@ -104,7 +104,7 @@ func (tf *Terraform) FormatCheck(ctx context.Context, opts ...FormatOption) (boo var outBuf bytes.Buffer cmd.Stdout = mergeWriters(cmd.Stdout, &outBuf) - err = tf.runTerraformCmd(cmd) + err = tf.runTerraformCmd(ctx, cmd) if err == nil { return true, nil, nil } diff --git a/tfexec/import.go b/tfexec/import.go index cffb4e92..e243d728 100644 --- a/tfexec/import.go +++ b/tfexec/import.go @@ -78,7 +78,7 @@ func (tf *Terraform) Import(ctx context.Context, address, id string, opts ...Imp if err != nil { return err } - return tf.runTerraformCmd(cmd) + return tf.runTerraformCmd(ctx, cmd) } func (tf *Terraform) importCmd(ctx context.Context, address, id string, opts ...ImportOption) (*exec.Cmd, error) { diff --git a/tfexec/init.go b/tfexec/init.go index 7d2a6bce..69010f54 100644 --- a/tfexec/init.go +++ b/tfexec/init.go @@ -98,7 +98,7 @@ func (tf *Terraform) Init(ctx context.Context, opts ...InitOption) error { if err != nil { return err } - return tf.runTerraformCmd(cmd) + return tf.runTerraformCmd(ctx, cmd) } func (tf *Terraform) initCmd(ctx context.Context, opts ...InitOption) (*exec.Cmd, error) { diff --git a/tfexec/output.go b/tfexec/output.go index 4c204025..b16b8b72 100644 --- a/tfexec/output.go +++ b/tfexec/output.go @@ -37,7 +37,7 @@ func (tf *Terraform) Output(ctx context.Context, opts ...OutputOption) (map[stri outputCmd := tf.outputCmd(ctx, opts...) outputs := map[string]OutputMeta{} - err := tf.runTerraformCmdJSON(outputCmd, &outputs) + err := tf.runTerraformCmdJSON(ctx, outputCmd, &outputs) if err != nil { return nil, err } diff --git a/tfexec/plan.go b/tfexec/plan.go index 07541acd..bfe77db7 100644 --- a/tfexec/plan.go +++ b/tfexec/plan.go @@ -96,7 +96,7 @@ func (tf *Terraform) Plan(ctx context.Context, opts ...PlanOption) (bool, error) if err != nil { return false, err } - err = tf.runTerraformCmd(cmd) + err = tf.runTerraformCmd(ctx, cmd) if err != nil && cmd.ProcessState.ExitCode() == 2 { return true, nil } diff --git a/tfexec/providers_schema.go b/tfexec/providers_schema.go index 75e593a9..52efc5db 100644 --- a/tfexec/providers_schema.go +++ b/tfexec/providers_schema.go @@ -12,7 +12,7 @@ func (tf *Terraform) ProvidersSchema(ctx context.Context) (*tfjson.ProviderSchem schemaCmd := tf.providersSchemaCmd(ctx) var ret tfjson.ProviderSchemas - err := tf.runTerraformCmdJSON(schemaCmd, &ret) + err := tf.runTerraformCmdJSON(ctx, schemaCmd, &ret) if err != nil { return nil, err } diff --git a/tfexec/refresh.go b/tfexec/refresh.go index 5839d2df..78f6b4b5 100644 --- a/tfexec/refresh.go +++ b/tfexec/refresh.go @@ -75,7 +75,7 @@ func (tf *Terraform) Refresh(ctx context.Context, opts ...RefreshCmdOption) erro if err != nil { return err } - return tf.runTerraformCmd(cmd) + return tf.runTerraformCmd(ctx, cmd) } func (tf *Terraform) refreshCmd(ctx context.Context, opts ...RefreshCmdOption) (*exec.Cmd, error) { diff --git a/tfexec/show.go b/tfexec/show.go index dbb58f04..a8d67f1a 100644 --- a/tfexec/show.go +++ b/tfexec/show.go @@ -50,7 +50,7 @@ func (tf *Terraform) Show(ctx context.Context, opts ...ShowOption) (*tfjson.Stat var ret tfjson.State ret.UseJSONNumber(true) - err = tf.runTerraformCmdJSON(showCmd, &ret) + err = tf.runTerraformCmdJSON(ctx, showCmd, &ret) if err != nil { return nil, err } @@ -93,7 +93,7 @@ func (tf *Terraform) ShowStateFile(ctx context.Context, statePath string, opts . var ret tfjson.State ret.UseJSONNumber(true) - err = tf.runTerraformCmdJSON(showCmd, &ret) + err = tf.runTerraformCmdJSON(ctx, showCmd, &ret) if err != nil { return nil, err } @@ -135,7 +135,7 @@ func (tf *Terraform) ShowPlanFile(ctx context.Context, planPath string, opts ... showCmd := tf.showCmd(ctx, true, mergeEnv, planPath) var ret tfjson.Plan - err = tf.runTerraformCmdJSON(showCmd, &ret) + err = tf.runTerraformCmdJSON(ctx, showCmd, &ret) if err != nil { return nil, err } @@ -175,7 +175,7 @@ func (tf *Terraform) ShowPlanFileRaw(ctx context.Context, planPath string, opts var ret bytes.Buffer showCmd.Stdout = &ret - err := tf.runTerraformCmd(showCmd) + err := tf.runTerraformCmd(ctx, showCmd) if err != nil { return "", err } diff --git a/tfexec/state_mv.go b/tfexec/state_mv.go index 1646e52c..fc7eecf8 100644 --- a/tfexec/state_mv.go +++ b/tfexec/state_mv.go @@ -60,7 +60,7 @@ func (tf *Terraform) StateMv(ctx context.Context, source string, destination str if err != nil { return err } - return tf.runTerraformCmd(cmd) + return tf.runTerraformCmd(ctx, cmd) } func (tf *Terraform) stateMvCmd(ctx context.Context, source string, destination string, opts ...StateMvCmdOption) (*exec.Cmd, error) { diff --git a/tfexec/upgrade012.go b/tfexec/upgrade012.go index 5af05b7e..e55237a7 100644 --- a/tfexec/upgrade012.go +++ b/tfexec/upgrade012.go @@ -40,7 +40,7 @@ func (tf *Terraform) Upgrade012(ctx context.Context, opts ...Upgrade012Option) e if err != nil { return err } - return tf.runTerraformCmd(cmd) + return tf.runTerraformCmd(ctx, cmd) } func (tf *Terraform) upgrade012Cmd(ctx context.Context, opts ...Upgrade012Option) (*exec.Cmd, error) { diff --git a/tfexec/validate.go b/tfexec/validate.go index cac9d6b8..756eccd7 100644 --- a/tfexec/validate.go +++ b/tfexec/validate.go @@ -22,14 +22,15 @@ func (tf *Terraform) Validate(ctx context.Context) (*tfjson.ValidateOutput, erro var outbuf = bytes.Buffer{} cmd.Stdout = &outbuf - err = tf.runTerraformCmd(cmd) + err = tf.runTerraformCmd(ctx, cmd) // TODO: this command should not exit 1 if you pass -json as its hard to differentiate other errors if err != nil && cmd.ProcessState.ExitCode() != 1 { return nil, err } - var out tfjson.ValidateOutput - jsonErr := json.Unmarshal(outbuf.Bytes(), &out) + var ret tfjson.ValidateOutput + // TODO: ret.UseJSONNumber(true) validate output should support JSON numbers + jsonErr := json.Unmarshal(outbuf.Bytes(), &ret) if jsonErr != nil { // the original call was possibly bad, if it has an error, actually just return that if err != nil { @@ -39,5 +40,5 @@ func (tf *Terraform) Validate(ctx context.Context) (*tfjson.ValidateOutput, erro return nil, jsonErr } - return &out, nil + return &ret, nil } diff --git a/tfexec/version.go b/tfexec/version.go index 6f3f1395..2e45dda1 100644 --- a/tfexec/version.go +++ b/tfexec/version.go @@ -44,7 +44,7 @@ func (tf *Terraform) version(ctx context.Context) (*version.Version, map[string] var outBuf bytes.Buffer versionCmd.Stdout = &outBuf - err := tf.runTerraformCmd(versionCmd) + err := tf.runTerraformCmd(ctx, versionCmd) if err != nil { return nil, nil, err } diff --git a/tfexec/workspace_list.go b/tfexec/workspace_list.go index edf9adab..b8d03094 100644 --- a/tfexec/workspace_list.go +++ b/tfexec/workspace_list.go @@ -14,7 +14,7 @@ func (tf *Terraform) WorkspaceList(ctx context.Context) ([]string, string, error var outBuf bytes.Buffer wlCmd.Stdout = &outBuf - err := tf.runTerraformCmd(wlCmd) + err := tf.runTerraformCmd(ctx, wlCmd) if err != nil { return nil, "", err } diff --git a/tfexec/workspace_new.go b/tfexec/workspace_new.go index 1925c286..2e05ffdb 100644 --- a/tfexec/workspace_new.go +++ b/tfexec/workspace_new.go @@ -41,7 +41,7 @@ func (tf *Terraform) WorkspaceNew(ctx context.Context, workspace string, opts .. if err != nil { return err } - return tf.runTerraformCmd(cmd) + return tf.runTerraformCmd(ctx, cmd) } func (tf *Terraform) workspaceNewCmd(ctx context.Context, workspace string, opts ...WorkspaceNewCmdOption) (*exec.Cmd, error) { diff --git a/tfexec/workspace_select.go b/tfexec/workspace_select.go index 87f5301e..5a51330f 100644 --- a/tfexec/workspace_select.go +++ b/tfexec/workspace_select.go @@ -6,5 +6,5 @@ import "context" func (tf *Terraform) WorkspaceSelect(ctx context.Context, workspace string) error { // TODO: [DIR] param option - return tf.runTerraformCmd(tf.buildTerraformCmd(ctx, nil, "workspace", "select", "-no-color", workspace)) + return tf.runTerraformCmd(ctx, tf.buildTerraformCmd(ctx, nil, "workspace", "select", "-no-color", workspace)) }