Skip to content

Commit

Permalink
add missing cleanup for local temp script file
Browse files Browse the repository at this point in the history
this file is uploaded to remote temp and removed properly as a part of returned teardown(). However the local file created first and the removal was missing
  • Loading branch information
umputun committed Dec 19, 2024
1 parent 89b6396 commit c7b3bd9
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 3 deletions.
2 changes: 1 addition & 1 deletion pkg/executor/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ func (ex *Remote) Delete(ctx context.Context, remoteFile string, opts *DeleteOpt
}
}

log.Printf("[INFO] deleted recursively %s", remoteFile)
log.Printf("[INFO] deleted recursively %s from %s", remoteFile, ex.hostName)
}

if fileInfo.IsDir() && !recursive {
Expand Down
9 changes: 8 additions & 1 deletion pkg/runner/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,12 @@ func (ec *execCmd) prepScript(ctx context.Context, s string, r io.Reader) (cmd,
if err = tmp.Close(); err != nil {
return "", "", nil, ec.errorFmt("can't close temporary file: %w", err)
}

defer func() {
// remove local copy of the script after upload or in case of error
if err := os.Remove(tmp.Name()); err != nil {
log.Printf("[WARN] can't remove local temp script %s: %v", tmp.Name(), err)
}
}()
// make the script executable locally, upload preserves the permissions
if err = os.Chmod(tmp.Name(), 0o700); err != nil { // nolint
return "", "", nil, ec.errorFmt("can't chmod temporary file: %w", err)
Expand All @@ -471,6 +476,8 @@ func (ec *execCmd) prepScript(ctx context.Context, s string, r io.Reader) (cmd,
cmd = fmt.Sprintf("%s -c %s", ec.shell(), dst)

teardown = func() error {
log.Printf("[DEBUG] removed local temp script %s", tmp.Name())

// remove the temp dir with the script from the remote hostAddr,
// should be invoked by the caller after the command is executed
if err := ec.exec.Delete(ctx, tmpRemoteDir, &executor.DeleteOpts{Recursive: true}); err != nil {
Expand Down
32 changes: 31 additions & 1 deletion pkg/runner/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,37 @@ func Test_execCmd(t *testing.T) {
assert.Equal(t, " {copy: testdata/inventory.yml -> /tmp/inventory.txt}", resp.details)
})

t.Run("script temp files cleanup", func(t *testing.T) {
wr := bytes.NewBuffer(nil)
log.SetOutput(io.MultiWriter(wr, os.Stdout))

// run a multi-line script that will create temp files
ec := execCmd{exec: sess, tsk: &config.Task{Name: "test"}, cmd: config.Cmd{
Script: "echo 'line1'\necho 'line2'\necho 'line3'",
}}
resp, err := ec.Script(ctx)
require.NoError(t, err)
t.Logf("resp: %+v", resp)

// check that the temp file was cleaned up
_, err = sess.Run(ctx, "ls -la /tmp/spot*", nil)
require.Error(t, err, "temp script file should be cleaned up")
_, err = sess.Run(ctx, "ls -la /tmp/.spot-*", nil)
require.Error(t, err, "temp dir should be cleaned up")

// also test cleanup on failure
ec = execCmd{exec: sess, tsk: &config.Task{Name: "test"}, cmd: config.Cmd{
Script: "echo 'line1'\nexit 1\necho 'line3'",
}}
_, err = ec.Script(ctx)
require.Error(t, err)

// check cleanup after failure
_, err = sess.Run(ctx, "ls -la /tmp/spot*", nil)
require.Error(t, err, "temp script file should be cleaned up even after failure")
_, err = sess.Run(ctx, "ls -la /tmp/.spot-*", nil)
require.Error(t, err, "temp dir should be cleaned up even after failure")
})
}

func Test_execCmdWithTmp(t *testing.T) {
Expand All @@ -501,7 +532,6 @@ func Test_execCmdWithTmp(t *testing.T) {
re := regexp.MustCompile(pattern)
match := re.FindStringSubmatch(log)
if len(match) > 1 {

return strings.ReplaceAll(match[1], "localhost:", "")
}
return ""
Expand Down

0 comments on commit c7b3bd9

Please sign in to comment.