Skip to content

Commit

Permalink
Added option struct for run hook functions, also added workdir option
Browse files Browse the repository at this point in the history
Signed-off-by: Fang-Pen Lin <[email protected]>
  • Loading branch information
fangpenlin committed Jun 23, 2023
1 parent 3273ac8 commit 4aa9558
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 18 deletions.
44 changes: 39 additions & 5 deletions pkg/hooks/exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,50 @@ import (
// DefaultPostKillTimeout is the recommended default post-kill timeout.
const DefaultPostKillTimeout = time.Duration(10) * time.Second

type RunOptions struct {
// The hook to run
Hook *rspec.Hook
// The workdir to change when invoking the hook
Dir string
// The container state data to pass into the hook process
State []byte
// Stdout from the hook process
Stdout io.Writer
// Stderr from the hook process
Stderr io.Writer
// Timeout for waiting process killed
PostKillTimeout time.Duration
}

// Run executes the hook and waits for it to complete or for the
// context or hook-specified timeout to expire.
//
// Deprecated: Too many arguments, has been refactored and replaced by RunHook instead
func Run(ctx context.Context, hook *rspec.Hook, state []byte, stdout io.Writer, stderr io.Writer, postKillTimeout time.Duration) (hookErr, err error) {
return RunHook(
ctx,
RunOptions{
Hook: hook,
State: state,
Stdout: stdout,
Stderr: stderr,
PostKillTimeout: postKillTimeout,
},
)
}

// RunHook Run executes the hook and waits for it to complete or for the
// context or hook-specified timeout to expire.
func RunHook(ctx context.Context, options RunOptions) (hookErr, err error) {
hook := options.Hook
cmd := osexec.Cmd{
Path: hook.Path,
Args: hook.Args,
Env: hook.Env,
Stdin: bytes.NewReader(state),
Stdout: stdout,
Stderr: stderr,
Dir: options.Dir,
Stdin: bytes.NewReader(options.State),
Stdout: options.Stdout,
Stderr: options.Stderr,
}
if cmd.Env == nil {
cmd.Env = []string{}
Expand Down Expand Up @@ -57,11 +91,11 @@ func Run(ctx context.Context, hook *rspec.Hook, state []byte, stdout io.Writer,
if err := cmd.Process.Kill(); err != nil {
logrus.Errorf("Failed to kill pid %v", cmd.Process)
}
timer := time.NewTimer(postKillTimeout)
timer := time.NewTimer(options.PostKillTimeout)
defer timer.Stop()
select {
case <-timer.C:
err = fmt.Errorf("failed to reap process within %s of the kill signal", postKillTimeout)
err = fmt.Errorf("failed to reap process within %s of the kill signal", options.PostKillTimeout)
case err = <-exit:
}
return err, ctx.Err()
Expand Down
34 changes: 28 additions & 6 deletions pkg/hooks/exec/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestRun(t *testing.T) {
Args: []string{"sh", "-c", "cat"},
}
var stderr, stdout bytes.Buffer
hookErr, err := Run(ctx, hook, []byte("{}"), &stdout, &stderr, DefaultPostKillTimeout)
hookErr, err := RunHook(ctx, RunOptions{Hook: hook, State: []byte("{}"), Stdout: &stdout, Stderr: &stderr, PostKillTimeout: DefaultPostKillTimeout})
if err != nil {
t.Fatal(err)
}
Expand All @@ -46,7 +46,7 @@ func TestRunIgnoreOutput(t *testing.T) {
Path: path,
Args: []string{"sh", "-c", "cat"},
}
hookErr, err := Run(ctx, hook, []byte("{}"), nil, nil, DefaultPostKillTimeout)
hookErr, err := RunHook(ctx, RunOptions{Hook: hook, State: []byte("{}"), PostKillTimeout: DefaultPostKillTimeout})
if err != nil {
t.Fatal(err)
}
Expand All @@ -60,7 +60,7 @@ func TestRunFailedStart(t *testing.T) {
hook := &rspec.Hook{
Path: "/does/not/exist",
}
hookErr, err := Run(ctx, hook, []byte("{}"), nil, nil, DefaultPostKillTimeout)
hookErr, err := RunHook(ctx, RunOptions{Hook: hook, State: []byte("{}"), PostKillTimeout: DefaultPostKillTimeout})
if err == nil {
t.Fatal("unexpected success")
}
Expand Down Expand Up @@ -125,7 +125,7 @@ func TestRunEnvironment(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
var stderr, stdout bytes.Buffer
hook.Env = test.env
hookErr, err := Run(ctx, hook, []byte("{}"), &stdout, &stderr, DefaultPostKillTimeout)
hookErr, err := RunHook(ctx, RunOptions{Hook: hook, State: []byte("{}"), Stdout: &stdout, Stderr: &stderr, PostKillTimeout: DefaultPostKillTimeout})
if err != nil {
t.Fatal(err)
}
Expand All @@ -143,6 +143,28 @@ func TestRunEnvironment(t *testing.T) {
}
}

func TestRunCwd(t *testing.T) {
ctx := context.Background()
hook := &rspec.Hook{
Path: path,
Args: []string{"sh", "-c", "pwd"},
}
cwd, err := os.MkdirTemp("", "userdata")
if err != nil {
t.Fatal(err)
}
var stderr, stdout bytes.Buffer
hookErr, err := RunHook(ctx, RunOptions{Hook: hook, Dir: cwd, State: []byte("{}"), Stdout: &stdout, Stderr: &stderr, PostKillTimeout: DefaultPostKillTimeout})
if err != nil {
t.Fatal(err)
}
if hookErr != nil {
t.Fatal(hookErr)
}
assert.Equal(t, "", stderr.String())
assert.Equal(t, strings.TrimSuffix(stdout.String(), "\n"), cwd)
}

func TestRunCancel(t *testing.T) {
hook := &rspec.Hook{
Path: path,
Expand Down Expand Up @@ -186,7 +208,7 @@ func TestRunCancel(t *testing.T) {
defer cancel()
}
hook.Timeout = test.hookTimeout
hookErr, err := Run(ctx, hook, []byte("{}"), &stdout, &stderr, DefaultPostKillTimeout)
hookErr, err := RunHook(ctx, RunOptions{Hook: hook, State: []byte("{}"), Stdout: &stdout, Stderr: &stderr, PostKillTimeout: DefaultPostKillTimeout})
assert.Equal(t, test.expectedRunError, err)
if test.expectedHookError == "" {
if hookErr != nil {
Expand All @@ -208,7 +230,7 @@ func TestRunKillTimeout(t *testing.T) {
Path: path,
Args: []string{"sh", "-c", "sleep 1"},
}
hookErr, err := Run(ctx, hook, []byte("{}"), nil, nil, time.Duration(0))
hookErr, err := RunHook(ctx, RunOptions{Hook: hook, State: []byte("{}"), PostKillTimeout: time.Duration(0)})
assert.Equal(t, context.DeadlineExceeded, err)
assert.Regexp(t, "^(failed to reap process within 0s of the kill signal|executing \\[sh -c sleep 1]: signal: killed)$", hookErr)
}
Expand Down
37 changes: 31 additions & 6 deletions pkg/hooks/exec/runtimeconfigfilter.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,44 @@ var spewConfig = spew.ConfigState{
SortKeys: true,
}

type RuntimeConfigFilterOptions struct {
// The hooks to run
Hooks []spec.Hook
// The workdir to change when invoking the hook
Dir string
// The container config spec to pass into the hook processes and potentially get modified by them
Config *spec.Spec
// Timeout for waiting process killed
PostKillTimeout time.Duration
}

// RuntimeConfigFilter calls a series of hooks. But instead of
// passing container state on their standard input,
// RuntimeConfigFilter passes the proposed runtime configuration (and
// reads back a possibly-altered form from their standard output).
//
// Deprecated: Too many arguments, has been refactored and replaced by RuntimeConfigFilterWithOptions instead
func RuntimeConfigFilter(ctx context.Context, hooks []spec.Hook, config *spec.Spec, postKillTimeout time.Duration) (hookErr, err error) {
data, err := json.Marshal(config)
return RuntimeConfigFilterWithOptions(ctx, RuntimeConfigFilterOptions{
Hooks: hooks,
Config: config,
PostKillTimeout: postKillTimeout,
})
}

// RuntimeConfigFilterWithOptions calls a series of hooks. But instead of
// passing container state on their standard input,
// RuntimeConfigFilterWithOptions passes the proposed runtime configuration (and
// reads back a possibly-altered form from their standard output).
func RuntimeConfigFilterWithOptions(ctx context.Context, options RuntimeConfigFilterOptions) (hookErr, err error) {
data, err := json.Marshal(options.Config)
if err != nil {
return nil, err
}
for i, hook := range hooks {
for i, hook := range options.Hooks {
hook := hook
var stdout bytes.Buffer
hookErr, err = Run(ctx, &hook, data, &stdout, nil, postKillTimeout)
hookErr, err = RunHook(ctx, RunOptions{Hook: &hook, Dir: options.Dir, State: data, Stdout: &stdout, PostKillTimeout: options.PostKillTimeout})
if err != nil {
return hookErr, err
}
Expand All @@ -46,8 +71,8 @@ func RuntimeConfigFilter(ctx context.Context, hooks []spec.Hook, config *spec.Sp
return nil, fmt.Errorf("unmarshal output from config-filter hook %d: %w", i, err)
}

if !reflect.DeepEqual(config, &newConfig) {
oldConfig := spewConfig.Sdump(config)
if !reflect.DeepEqual(options.Config, &newConfig) {
oldConfig := spewConfig.Sdump(options.Config)
newConfig := spewConfig.Sdump(&newConfig)
diff, err := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{
A: difflib.SplitLines(oldConfig),
Expand All @@ -65,7 +90,7 @@ func RuntimeConfigFilter(ctx context.Context, hooks []spec.Hook, config *spec.Sp
}
}

*config = newConfig
*options.Config = newConfig
}

return nil, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/hooks/exec/runtimeconfigfilter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func TestRuntimeConfigFilter(t *testing.T) {
ctx, cancel = context.WithTimeout(ctx, test.contextTimeout)
defer cancel()
}
hookErr, err := RuntimeConfigFilter(ctx, test.hooks, test.input, DefaultPostKillTimeout)
hookErr, err := RuntimeConfigFilterWithOptions(ctx, RuntimeConfigFilterOptions{Hooks: test.hooks, Config: test.input, PostKillTimeout: DefaultPostKillTimeout})
if test.expectedRunErrorString != "" {
// We have to compare the error strings in that case because
// errors.Is works differently.
Expand Down

0 comments on commit 4aa9558

Please sign in to comment.