-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Implement hooks in libcontainer code base #261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,11 @@ | ||
| package configs | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "encoding/json" | ||
| "os/exec" | ||
| ) | ||
|
|
||
| type Rlimit struct { | ||
| Type int `json:"type"` | ||
| Hard uint64 `json:"hard"` | ||
|
|
@@ -159,4 +165,75 @@ type Config struct { | |
| // A number of rules are given, each having an action to be taken if a syscall matches it. | ||
| // A default action to be taken if no rules match is also given. | ||
| Seccomp *Seccomp `json:"seccomp"` | ||
|
|
||
| // Hooks are a collection of actions to perform at various container lifecycle events. | ||
| // Hooks are not able to be marshaled to json but they are also not needed to. | ||
| Hooks *Hooks `json:"-"` | ||
| } | ||
|
|
||
| type Hooks struct { | ||
| // Prestart commands are executed after the container namespaces are created, | ||
| // but before the user supplied command is executed from init. | ||
| Prestart []Hook | ||
|
|
||
| // PostStop commands are executed after the container init process exits. | ||
| Poststop []Hook | ||
| } | ||
|
|
||
| // HookState is the payload provided to a hook on execution. | ||
| type HookState struct { | ||
| ID string `json:"id"` | ||
| Pid int `json:"pid"` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also pass the path to the root here.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, didn't we want to pull in the latest spec to use the state from there? |
||
| } | ||
|
|
||
| type Hook interface { | ||
| // Run executes the hook with the provided state. | ||
| Run(*HookState) error | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 |
||
| } | ||
|
|
||
| // NewFunctionHooks will call the provided function when the hook is run. | ||
| func NewFunctionHook(f func(*HookState) error) *FuncHook { | ||
| return &FuncHook{ | ||
| run: f, | ||
| } | ||
| } | ||
|
|
||
| type FuncHook struct { | ||
| run func(*HookState) error | ||
| } | ||
|
|
||
| func (f *FuncHook) Run(s *HookState) error { | ||
| return f.run(s) | ||
| } | ||
|
|
||
| type Command struct { | ||
| Path string `json:"path"` | ||
| Args []string `json:"args"` | ||
| Env []string `json:"env"` | ||
| Dir string `json:"dir"` | ||
| } | ||
|
|
||
| // NewCommandHooks will execute the provided command when the hook is run. | ||
| func NewCommandHook(cmd Command) *CommandHook { | ||
| return &CommandHook{ | ||
| Command: cmd, | ||
| } | ||
| } | ||
|
|
||
| type CommandHook struct { | ||
| Command | ||
| } | ||
|
|
||
| func (c *Command) Run(s *HookState) error { | ||
| b, err := json.Marshal(s) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| cmd := exec.Cmd{ | ||
| Path: c.Path, | ||
| Args: c.Args, | ||
| Env: c.Env, | ||
| Stdin: bytes.NewReader(b), | ||
| } | ||
| return cmd.Run() | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -932,3 +932,52 @@ func TestOomScoreAdj(t *testing.T) { | |
| t.Fatalf("Expected oom_score_adj %d; got %q", config.OomScoreAdj, outputOomScoreAdj) | ||
| } | ||
| } | ||
|
|
||
| func TestPrestartHook(t *testing.T) { | ||
| if testing.Short() { | ||
| return | ||
| } | ||
| root, err := newTestRoot() | ||
| ok(t, err) | ||
| defer os.RemoveAll(root) | ||
|
|
||
| rootfs, err := newRootfs() | ||
| ok(t, err) | ||
| defer remove(rootfs) | ||
|
|
||
| config := newTemplateConfig(rootfs) | ||
| config.Hooks = &configs.Hooks{ | ||
| Prestart: []configs.Hook{ | ||
| configs.NewFunctionHook(func(s *configs.HookState) error { | ||
| f, err := os.Create(filepath.Join(rootfs, "test")) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dont think we should reach out to rootfs from here. We should pass it as hook state as @mrunalp mentioned before. |
||
| if err != nil { | ||
| return err | ||
| } | ||
| return f.Close() | ||
| }), | ||
| }, | ||
| } | ||
| container, err := factory.Create("test", config) | ||
| ok(t, err) | ||
| defer container.Destroy() | ||
|
|
||
| var stdout bytes.Buffer | ||
| pconfig := libcontainer.Process{ | ||
| Args: []string{"sh", "-c", "ls /test"}, | ||
| Env: standardEnvironment, | ||
| Stdin: nil, | ||
| Stdout: &stdout, | ||
| } | ||
| err = container.Start(&pconfig) | ||
| ok(t, err) | ||
|
|
||
| // Wait for process | ||
| waitProcess(&pconfig, t) | ||
|
|
||
| outputLs := string(stdout.Bytes()) | ||
|
|
||
| // Check that the ls output has the expected file touched by the prestart hook | ||
| if !strings.Contains(outputLs, "/test") { | ||
| t.Fatal("ls output doesn't have the expected file: ", outputLs) | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. need test for post stop here :p |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I sorta like how it is in comment.
PreStart/PostStoplooks better for me.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should match what is in the spec where it is lower case https://github.com/opencontainers/specs/blob/master/runtime_config.go#L24
If we change, then should change there as well and keep it consistent :)