Skip to content

Commit

Permalink
replace direct exectution of on-error in shell with the real script e…
Browse files Browse the repository at this point in the history
…xecutor on local
  • Loading branch information
umputun committed Jul 18, 2023
1 parent 4064896 commit a56cc8e
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 33 deletions.
2 changes: 1 addition & 1 deletion pkg/runner/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type execCmdErr struct {
cmd execCmd
}

func (e *execCmdErr) Error() string { return e.err.Error() }
func (e execCmdErr) Error() string { return e.err.Error() }

const tmpRemoteDirPrefix = "/tmp/.spot-" // this is a directory on remote host to store temporary files

Expand Down
77 changes: 47 additions & 30 deletions pkg/runner/runner.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
package runner

import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"log"
"os"
"os/exec"
"strings"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -112,7 +110,7 @@ func (p *Process) Run(ctx context.Context, task, target string) (s ProcResp, err

// execute on-error command if any error occurred during task execution and on-error command is defined
if err != nil && tsk.OnError != "" {
p.onError(ctx, tsk)
p.onError(ctx, err)
}

return ProcResp{Hosts: len(targetHosts), Commands: int(atomic.LoadInt32(&commands)), Vars: allVars}, err
Expand Down Expand Up @@ -284,32 +282,51 @@ func (p *Process) pickCmdExecutor(cmd config.Cmd, ec execCmd, hostAddr, hostName
return ec
}

// onError executes on-error command if any error occurred during task execution and on-error command is defined
func (p *Process) onError(ctx context.Context, tsk *config.Task) {

// ex := execCmd{
// tsk: tsk,
// cmd: config.Cmd{
// Name: "on-error",
// Script: tsk.OnError,
// Options: config.CmdOptions{
// Local: true,
// },
// },
// }

onErrCmd := exec.CommandContext(ctx, "sh", "-c", tsk.OnError) // nolint we want to run shell here
onErrCmd.Env = os.Environ()

outLog, errLog := executor.MakeOutAndErrWriters("localhost", "", p.Verbose, p.secrets)
outLog.Write([]byte(tsk.OnError)) // nolint

var stdoutBuf bytes.Buffer
mwr := io.MultiWriter(outLog, &stdoutBuf)
onErrCmd.Stdout, onErrCmd.Stderr = mwr, errLog
onErrCmd.Stdout, onErrCmd.Stderr = mwr, executor.NewStdoutLogWriter("!", "WARN", p.secrets)
if exErr := onErrCmd.Run(); exErr != nil {
log.Printf("[WARN] can't run on-error command %q: %v", tsk.OnError, exErr)
// onError executes on-error command locally if any error occurred during task execution and on-error command is defined
func (p *Process) onError(ctx context.Context, err error) {

// unwrapError unwraps error to get execCmdErr with all details about command execution
unwrapError := func(err error) (execCmdErr, bool) {
execErr := &execCmdErr{}
if errors.As(err, &execErr) {
return *execErr, true
}
multiErr := &syncs.MultiError{}
if errors.As(err, &multiErr) {
for _, e := range multiErr.Errors() {
if errors.As(e, &execErr) {
// we need only the first error as we don't need to execute
// on-error command multiple times if multiple commands failed
return *execErr, true
}
}
}
return execCmdErr{}, false
}

execErr, ok := unwrapError(err)
if !ok {
log.Printf("[WARN] can't unwrap error: %v", err)
return
}

ec := execCmd{
tsk: execErr.cmd.tsk,
cmd: config.Cmd{
Name: execErr.cmd.tsk.Name,
Script: execErr.cmd.tsk.OnError,
Options: config.CmdOptions{Local: true}, // force local execution for on-error command
},
hostAddr: execErr.cmd.hostAddr,
hostName: execErr.cmd.hostName,
verbose: p.Verbose,
}

ec = p.pickCmdExecutor(ec.cmd, ec, "localhost", ec.hostName) // pick executor for local command
tmpl := templater{hostAddr: ec.hostAddr, hostName: ec.hostName, task: ec.tsk, command: ec.cmd.Name, err: execErr}
ec.cmd.Script = tmpl.apply(ec.cmd.Script)
if _, err = ec.Script(ctx); err != nil {
log.Printf("[WARN] can't run on-error command for %q, %q: %v", ec.cmd.Name, ec.cmd.Script, err)
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/runner/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ func TestProcess_RunFailed_WithOnError(t *testing.T) {
t.Log(buf.String())
require.NotContains(t, buf.String(), "onerror called")
assert.Contains(t, buf.String(), "[WARN]")
assert.Contains(t, buf.String(), `can't run on-error command "exit 1`)
assert.Contains(t, buf.String(), `can't run on-error command for "failed_task_with_bad_onerror", "exit 1"`)
})
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/runner/testdata/conf.yml
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ tasks:
script: echo good command 2

- name: failed_task_with_onerror
on_error: "echo onerror called. task: ${SPOT_TASK}, host: ${SPOT_REMOTE_HOST}, error: ${SPOT_ERROR}, name: ${SPOT_REMOTE_NAME}"
on_error: 'echo "onerror called. task: ${SPOT_TASK}, host: ${SPOT_REMOTE_HOST}, error: ${SPOT_ERROR}"'
commands:
- name: good command
script: echo good command 1
Expand Down

0 comments on commit a56cc8e

Please sign in to comment.