Skip to content

Commit

Permalink
refactor executors to support options in extendable way
Browse files Browse the repository at this point in the history 
refactor runner with command options
  • Loading branch information
@umputun
umputun committed May 21, 2023
1 parent fefb833 commit ae9b3f0
Show file tree
Hide file tree
Showing 9 changed files with 178 additions and 135 deletions.
22 changes: 15 additions & 7 deletions pkg/executor/dry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ func (ex *Dry) SetSecrets(secrets []string) {
}

// Run shows the command content, doesn't execute it
func (ex *Dry) Run(_ context.Context, cmd string, verbose bool) (out []string, err error) {
func (ex *Dry) Run(_ context.Context, cmd string, opts *RunOpts) (out []string, err error) {
log.Printf("[DEBUG] run %s", cmd)
outLog, _ := MakeOutAndErrWriters(ex.hostAddr, ex.hostName, verbose, ex.secrets)
outLog, _ := MakeOutAndErrWriters(ex.hostAddr, ex.hostName, opts != nil && opts.Verbose, ex.secrets)
var stdoutBuf bytes.Buffer
mwr := io.MultiWriter(outLog, &stdoutBuf)
mwr.Write([]byte(cmd)) //nolint
Expand All @@ -44,7 +44,8 @@ func (ex *Dry) Run(_ context.Context, cmd string, verbose bool) (out []string, e
}

// Upload doesn't actually upload, just prints the command
func (ex *Dry) Upload(_ context.Context, local, remote string, mkdir bool) (err error) {
func (ex *Dry) Upload(_ context.Context, local, remote string, opts *UpDownOpts) (err error) {
mkdir := opts != nil && opts.Mkdir
log.Printf("[DEBUG] upload %s to %s, mkdir: %v", local, remote, mkdir)
if strings.Contains(remote, "spot-script") {
// this is a temp script created by spot to perform script execution on remote host
Expand All @@ -69,19 +70,26 @@ func (ex *Dry) Upload(_ context.Context, local, remote string, mkdir bool) (err
}

// Download file from remote server with scp
func (ex *Dry) Download(_ context.Context, remote, local string, mkdir bool) (err error) {
func (ex *Dry) Download(_ context.Context, remote, local string, opts *UpDownOpts) (err error) {
mkdir := opts != nil && opts.Mkdir
log.Printf("[DEBUG] download %s to %s, mkdir: %v", local, remote, mkdir)
return nil
}

// Sync doesn't sync anything, just prints the command
func (ex *Dry) Sync(_ context.Context, localDir, remoteDir string, del bool, exclude []string) ([]string, error) {
log.Printf("[DEBUG] sync %s to %s, delite: %v, exlcude: %v", localDir, remoteDir, del, exclude) //nolint
func (ex *Dry) Sync(_ context.Context, localDir, remoteDir string, opts *SyncOpts) ([]string, error) {
del := opts != nil && opts.Delete
exclude := []string{}
if opts != nil {
exclude = opts.Exclude
}
log.Printf("[DEBUG] sync %s to %s, delete: %v, exlcude: %v", localDir, remoteDir, del, exclude) //nolint
return nil, nil
}

// Delete doesn't delete anything, just prints the command
func (ex *Dry) Delete(_ context.Context, remoteFile string, recursive bool) (err error) {
func (ex *Dry) Delete(_ context.Context, remoteFile string, opts *DeleteOpts) (err error) {
recursive := opts != nil && opts.Recursive
log.Printf("[DEBUG] delete %s, recursive: %v", remoteFile, recursive)
return nil
}
Expand Down
14 changes: 7 additions & 7 deletions pkg/executor/dry_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
func TestDry_Run(t *testing.T) {
ctx := context.Background()
dry := NewDry("hostAddr", "hostName")
res, err := dry.Run(ctx, "ls -la /srv", true)
res, err := dry.Run(ctx, "ls -la /srv", &RunOpts{Verbose: true})
require.NoError(t, err)
require.Len(t, res, 1)
require.Equal(t, "ls -la /srv", res[0])
Expand All @@ -37,7 +37,7 @@ func TestDryUpload(t *testing.T) {
}

stdout := captureOutput(func() {
err = dry.Upload(context.Background(), tempFile.Name(), "remote/path/spot-script", true)
err = dry.Upload(context.Background(), tempFile.Name(), "remote/path/spot-script", &UpDownOpts{Mkdir: true})
})

require.NoError(t, err)
Expand All @@ -58,7 +58,7 @@ func TestDryUpload_FileOpenError(t *testing.T) {
hostName: "host1",
}

err := dry.Upload(context.Background(), nonExistentFile, "remote/path/spot-script", true)
err := dry.Upload(context.Background(), nonExistentFile, "remote/path/spot-script", &UpDownOpts{Mkdir: true})
require.Error(t, err)
assert.Contains(t, err.Error(), "open non_existent_file", "expected error message containing 'open non_existent_file' not found")
}
Expand All @@ -77,22 +77,22 @@ func TestDryOperations(t *testing.T) {
{
name: "download",
operation: func() error {
return dry.Download(context.Background(), "remote/path", "local/path", true)
return dry.Download(context.Background(), "remote/path", "local/path", &UpDownOpts{Mkdir: true})
},
expectedLog: "[DEBUG] download local/path to remote/path, mkdir: true",
},
{
name: "sync",
operation: func() error {
_, err := dry.Sync(context.Background(), "local/dir", "remote/dir", true, nil)
_, err := dry.Sync(context.Background(), "local/dir", "remote/dir", &SyncOpts{Delete: true})
return err
},
expectedLog: "[DEBUG] sync local/dir to remote/dir, delite: true",
expectedLog: "[DEBUG] sync local/dir to remote/dir, delete: true",
},
{
name: "delete",
operation: func() error {
return dry.Delete(context.Background(), "remote/file", true)
return dry.Delete(context.Background(), "remote/file", &DeleteOpts{Recursive: true})
},
expectedLog: "[DEBUG] delete remote/file, recursive: true",
},
Expand Down
35 changes: 30 additions & 5 deletions pkg/executor/executor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,39 @@ import (
// Implemented by Remote and Local structs.
type Interface interface {
SetSecrets(secrets []string)
Run(ctx context.Context, c string, verbose bool) (out []string, err error)
Upload(ctx context.Context, local, remote string, mkdir bool) (err error)
Download(ctx context.Context, remote, local string, mkdir bool) (err error)
Sync(ctx context.Context, localDir, remoteDir string, del bool, exclude []string) ([]string, error)
Delete(ctx context.Context, remoteFile string, recursive bool) (err error)
Run(ctx context.Context, c string, opts *RunOpts) (out []string, err error)
Upload(ctx context.Context, local, remote string, opts *UpDownOpts) (err error)
Download(ctx context.Context, remote, local string, opts *UpDownOpts) (err error)
Sync(ctx context.Context, localDir, remoteDir string, opts *SyncOpts) ([]string, error)
Delete(ctx context.Context, remoteFile string, opts *DeleteOpts) (err error)
Close() error
}

// RunOpts is a struct for run options.
type RunOpts struct {
Verbose bool // print more info to primary stdout
}

// UpDownOpts is a struct for upload and download options.
type UpDownOpts struct {
Mkdir bool // create remote directory if it does not exist
Checksum bool // compare checksums of local and remote files, default is size and modtime
Force bool // overwrite existing files on remote
}

// SyncOpts is a struct for sync options.
type SyncOpts struct {
Delete bool // delete extra files on remote
Exclude []string // exclude files matching the given patterns
Checksum bool // compare checksums of local and remote files, default is size and modtime
Force bool // overwrite existing files on remote
}

// DeleteOpts is a struct for delete options.
type DeleteOpts struct {
Recursive bool // delete directories recursively
}

// StdOutLogWriter is a writer that writes to log with a prefix and a log level.
type StdOutLogWriter struct {
prefix string
Expand Down
23 changes: 14 additions & 9 deletions pkg/executor/local.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ func (l *Local) SetSecrets(secrets []string) {
}

// Run executes command on local hostAddr, inside the shell
func (l *Local) Run(ctx context.Context, cmd string, verbose bool) (out []string, err error) {
func (l *Local) Run(ctx context.Context, cmd string, opts *RunOpts) (out []string, err error) {
command := exec.CommandContext(ctx, "sh", "-c", cmd)

outLog, errLog := MakeOutAndErrWriters("localhost", "", verbose, l.secrets)
outLog, errLog := MakeOutAndErrWriters("localhost", "", opts != nil && opts.Verbose, l.secrets)
outLog.Write([]byte(cmd)) //nolint

var stdoutBuf bytes.Buffer
Expand All @@ -47,7 +47,7 @@ func (l *Local) Run(ctx context.Context, cmd string, verbose bool) (out []string
}

// Upload just copy file from one place to another
func (l *Local) Upload(_ context.Context, src, dst string, mkdir bool) (err error) {
func (l *Local) Upload(_ context.Context, src, dst string, opts *UpDownOpts) (err error) {

// check if the local parameter contains a glob pattern
matches, err := filepath.Glob(src)
Expand All @@ -59,7 +59,7 @@ func (l *Local) Upload(_ context.Context, src, dst string, mkdir bool) (err erro
return fmt.Errorf("source file %q not found", src)
}

if mkdir {
if opts != nil && opts.Mkdir {
if err = os.MkdirAll(filepath.Dir(dst), 0o750); err != nil {
return fmt.Errorf("can't create local dir %s: %w", filepath.Dir(dst), err)
}
Expand All @@ -78,18 +78,22 @@ func (l *Local) Upload(_ context.Context, src, dst string, mkdir bool) (err erro
}

// Download just copy file from one place to another
func (l *Local) Download(_ context.Context, src, dst string, mkdir bool) (err error) {
return l.Upload(context.Background(), src, dst, mkdir) // same as upload for local
func (l *Local) Download(_ context.Context, src, dst string, opts *UpDownOpts) (err error) {
return l.Upload(context.Background(), src, dst, opts) // same as upload for local
}

// Sync directories from src to dst
func (l *Local) Sync(ctx context.Context, src, dst string, del bool, excl []string) ([]string, error) {
func (l *Local) Sync(ctx context.Context, src, dst string, opts *SyncOpts) ([]string, error) {
excl := []string{}
if opts != nil {
excl = opts.Exclude
}
copiedFiles, err := l.syncSrcToDst(ctx, src, dst, excl)
if err != nil {
return nil, err
}

if del {
if opts != nil && opts.Delete {
if err := l.removeExtraDstFiles(ctx, src, dst); err != nil {
return nil, err
}
Expand All @@ -99,7 +103,8 @@ func (l *Local) Sync(ctx context.Context, src, dst string, del bool, excl []stri
}

// Delete file or directory
func (l *Local) Delete(_ context.Context, remoteFile string, recursive bool) (err error) {
func (l *Local) Delete(_ context.Context, remoteFile string, opts *DeleteOpts) (err error) {
recursive := opts != nil && opts.Recursive
if !recursive {
return os.Remove(remoteFile)
}
Expand Down
33 changes: 16 additions & 17 deletions pkg/executor/local_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,39 +19,39 @@ func TestRun(t *testing.T) {
l := &Local{}

t.Run("single line out success", func(t *testing.T) {
out, e := l.Run(ctx, "echo 'hello world'", false)
out, e := l.Run(ctx, "echo 'hello world'", &RunOpts{Verbose: true})
require.NoError(t, e)
assert.Equal(t, []string{"hello world"}, out)
})

t.Run("single line out fail", func(t *testing.T) {
_, e := l.Run(ctx, "nonexistent-command", false)
_, e := l.Run(ctx, "nonexistent-command", nil)
require.Error(t, e)
})

t.Run("multi line out success", func(t *testing.T) {
// Prepare the test environment
_, err := l.Run(ctx, "mkdir -p /tmp/st", true)
_, err := l.Run(ctx, "mkdir -p /tmp/st", &RunOpts{Verbose: true})
require.NoError(t, err)
_, err = l.Run(ctx, "cp testdata/data1.txt /tmp/st/data1.txt", true)
_, err = l.Run(ctx, "cp testdata/data1.txt /tmp/st/data1.txt", &RunOpts{Verbose: true})
require.NoError(t, err)
_, err = l.Run(ctx, "cp testdata/data2.txt /tmp/st/data2.txt", true)
_, err = l.Run(ctx, "cp testdata/data2.txt /tmp/st/data2.txt", &RunOpts{Verbose: true})
require.NoError(t, err)

out, err := l.Run(ctx, "ls -1 /tmp/st", false)
out, err := l.Run(ctx, "ls -1 /tmp/st", nil)
require.NoError(t, err)
assert.Equal(t, 2, len(out))
assert.Equal(t, "data1.txt", out[0])
assert.Equal(t, "data2.txt", out[1])
})

t.Run("multi line out fail", func(t *testing.T) {
_, err := l.Run(ctx, "nonexistent-command", false)
_, err := l.Run(ctx, "nonexistent-command", nil)
require.Error(t, err)
})

t.Run("find out", func(t *testing.T) {
out, e := l.Run(ctx, "find /tmp/st -type f", true)
out, e := l.Run(ctx, "find /tmp/st -type f", &RunOpts{Verbose: true})
require.NoError(t, e)
sort.Slice(out, func(i, j int) bool { return out[i] < out[j] })
assert.Contains(t, out, "/tmp/st/data1.txt")
Expand All @@ -66,7 +66,7 @@ func TestRun(t *testing.T) {
// Set up the test environment
l.SetSecrets([]string{"data2"})
defer l.SetSecrets(nil)
out, e := l.Run(ctx, "find /tmp/st -type f", true)
out, e := l.Run(ctx, "find /tmp/st -type f", &RunOpts{Verbose: true})
writer.Close()
os.Stdout = originalStdout

Expand Down Expand Up @@ -112,7 +112,7 @@ func TestUploadAndDownload(t *testing.T) {
}

// we want to test both upload and download, so we create a function type. those functions should do the same thing
type fn func(ctx context.Context, src, dst string, mkdir bool) (err error)
type fn func(ctx context.Context, src, dst string, opts *UpDownOpts) (err error)
l := &Local{}
fns := []struct {
name string
Expand Down Expand Up @@ -141,7 +141,7 @@ func TestUploadAndDownload(t *testing.T) {

dstFile := filepath.Join(dstDir, filepath.Base(srcFile.Name()))

err = fn.fn(context.Background(), srcFile.Name(), dstFile, tc.mkdir)
err = fn.fn(context.Background(), srcFile.Name(), dstFile, &UpDownOpts{Mkdir: tc.mkdir})

if tc.expectError {
assert.Error(t, err, "expected an error")
Expand Down Expand Up @@ -176,7 +176,7 @@ func TestUploadDownloadWithGlob(t *testing.T) {
require.NoError(t, err)
defer os.RemoveAll(dstDir)

type fn func(ctx context.Context, src, dst string, mkdir bool) (err error)
type fn func(ctx context.Context, src, dst string, opts *UpDownOpts) (err error)

l := &Local{}
fns := []struct {
Expand Down Expand Up @@ -226,8 +226,7 @@ func TestUploadDownloadWithGlob(t *testing.T) {
} {
for _, fn := range fns {
t.Run(fmt.Sprintf("%s#%s", tc.name, fn.name), func(t *testing.T) {
err := fn.fn(context.Background(), tc.src, tc.dst, tc.mkdir)

err := fn.fn(context.Background(), tc.src, tc.dst, &UpDownOpts{Mkdir: tc.mkdir})
if tc.expectError {
assert.Error(t, err, "expected an error")
return
Expand Down Expand Up @@ -449,7 +448,7 @@ func TestLocal_Sync(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

copiedFiles, err := svc.Sync(ctx, srcDir, dstDir, tc.del, tc.exclude)
copiedFiles, err := svc.Sync(ctx, srcDir, dstDir, &SyncOpts{Delete: tc.del, Exclude: tc.exclude})
require.NoError(t, err)
assert.ElementsMatch(t, tc.expected, copiedFiles)

Expand Down Expand Up @@ -542,7 +541,7 @@ func TestDelete(t *testing.T) {
}

l := &Local{}
err = l.Delete(context.Background(), remoteFile, tc.recursive)
err = l.Delete(context.Background(), remoteFile, &DeleteOpts{Recursive: tc.recursive})
if tc.expectError {
assert.Error(t, err, "expected an error")
} else {
Expand Down Expand Up @@ -596,7 +595,7 @@ func TestUpload_SpecialCharacterInPath(t *testing.T) {

dstFile := filepath.Join(dstDir, "file_with_special_#_character.txt")

err = l.Upload(context.Background(), srcFile.Name(), dstFile, true)
err = l.Upload(context.Background(), srcFile.Name(), dstFile, &UpDownOpts{Mkdir: true})
assert.NoError(t, err, "unexpected error")

dstContent, err := os.ReadFile(dstFile)
Expand Down
Loading

0 comments on commit ae9b3f0

Please sign in to comment.