diff --git a/libcontainer/configs/config.go b/libcontainer/configs/config.go index 37d8f9406..ac523b417 100644 --- a/libcontainer/configs/config.go +++ b/libcontainer/configs/config.go @@ -239,10 +239,6 @@ const ( Poststop = "poststop" ) -var ( - HookNameList = []HookName{Prestart, CreateRuntime, CreateContainer, StartContainer, Poststart, Poststop} -) - // TODO move this to runtime-spec // See: https://github.com/opencontainers/runtime-spec/pull/1046 const ( diff --git a/libcontainer/configs/config_linux_test.go b/libcontainer/configs/config_linux_test.go index 9c5f0febe..a16b09823 100644 --- a/libcontainer/configs/config_linux_test.go +++ b/libcontainer/configs/config_linux_test.go @@ -8,6 +8,10 @@ import ( "testing" ) +var ( + HookNameList = []HookName{Prestart, CreateRuntime, CreateContainer, StartContainer, Poststart, Poststop} +) + func loadConfig(name string) (*Config, error) { f, err := os.Open(filepath.Join("../sample_configs", name)) if err != nil { diff --git a/libcontainer/configs/config_test.go b/libcontainer/configs/config_test.go index c89a764de..db9ccaee6 100644 --- a/libcontainer/configs/config_test.go +++ b/libcontainer/configs/config_test.go @@ -15,27 +15,29 @@ import ( func TestUnmarshalHooks(t *testing.T) { timeout := time.Second - prestartCmd := configs.NewCommandHook(configs.Command{ - Path: "/var/vcap/hooks/prestart", + hookCmd := configs.NewCommandHook(configs.Command{ + Path: "/var/vcap/hooks/hook", Args: []string{"--pid=123"}, Env: []string{"FOO=BAR"}, Dir: "/var/vcap", Timeout: &timeout, }) - prestart, err := json.Marshal(prestartCmd.Command) + + hookJson, err := json.Marshal(hookCmd) if err != nil { t.Fatal(err) } - hook := configs.Hooks{} - err = hook.UnmarshalJSON([]byte(fmt.Sprintf(`{"Prestart" :[%s]}`, prestart))) - if err != nil { - t.Fatal(err) - } + for _, hookName := range configs.HookNameList { + hooks := configs.Hooks{} + err = hooks.UnmarshalJSON([]byte(fmt.Sprintf(`{"%s" :[%s]}`, hookName, hookJson))) + if err != nil { + t.Fatal(err) + } - if !reflect.DeepEqual(hook.Prestart[0], prestartCmd) { - t.Errorf("Expected prestart to equal %+v but it was %+v", - prestartCmd, hook.Prestart[0]) + if !reflect.DeepEqual(hooks[hookName], hookCmd) { + t.Errorf("Expected %s to equal %+v but it was %+v", hookName, hookCmd, hooks[hookName]) + } } } @@ -50,8 +52,8 @@ func TestUnmarshalHooksWithInvalidData(t *testing.T) { func TestMarshalHooks(t *testing.T) { timeout := time.Second - prestartCmd := configs.NewCommandHook(configs.Command{ - Path: "/var/vcap/hooks/prestart", + hookCmd := configs.NewCommandHook(configs.Command{ + Path: "/var/vcap/hooks/hook", Args: []string{"--pid=123"}, Env: []string{"FOO=BAR"}, Dir: "/var/vcap", @@ -59,14 +61,21 @@ func TestMarshalHooks(t *testing.T) { }) hook := configs.Hooks{ - Prestart: []configs.Hook{prestartCmd}, + configs.Prestart: configs.HookList{hookCmd}, + configs.CreateRuntime: configs.HookList{hookCmd}, + configs.CreateContainer: configs.HookList{hookCmd}, + configs.StartContainer: configs.HookList{hookCmd}, + configs.Poststart: configs.HookList{hookCmd}, + configs.Poststop: configs.HookList{hookCmd}, } hooks, err := hook.MarshalJSON() if err != nil { t.Fatal(err) } - h := `{"poststart":null,"poststop":null,"prestart":[{"path":"/var/vcap/hooks/prestart","args":["--pid=123"],"env":["FOO=BAR"],"dir":"/var/vcap","timeout":1000000000}]}` + // Note Marshal seems to output fields in alphabetical order + hookCmdJson := `[{"path":"/var/vcap/hooks/hook","args":["--pid=123"],"env":["FOO=BAR"],"dir":"/var/vcap","timeout":1000000000}]` + h := fmt.Sprintf(`{"createContainer":%[1]s,"createRuntime":%[1]s,"poststart":%[1]s,"poststop":%[1]s,"prestart":%[1]s,"startContainer":%[1]s}`, hookCmdJson) if string(hooks) != h { t.Errorf("Expected hooks %s to equal %s", string(hooks), h) } @@ -75,8 +84,8 @@ func TestMarshalHooks(t *testing.T) { func TestMarshalUnmarshalHooks(t *testing.T) { timeout := time.Second - prestart := configs.NewCommandHook(configs.Command{ - Path: "/var/vcap/hooks/prestart", + hookCmd := configs.NewCommandHook(configs.Command{ + Path: "/var/vcap/hooks/hook", Args: []string{"--pid=123"}, Env: []string{"FOO=BAR"}, Dir: "/var/vcap", @@ -84,7 +93,12 @@ func TestMarshalUnmarshalHooks(t *testing.T) { }) hook := configs.Hooks{ - Prestart: []configs.Hook{prestart}, + configs.Prestart: configs.HookList{hookCmd}, + configs.CreateRuntime: configs.HookList{hookCmd}, + configs.CreateContainer: configs.HookList{hookCmd}, + configs.StartContainer: configs.HookList{hookCmd}, + configs.Poststart: configs.HookList{hookCmd}, + configs.Poststop: configs.HookList{hookCmd}, } hooks, err := hook.MarshalJSON() if err != nil { @@ -96,8 +110,8 @@ func TestMarshalUnmarshalHooks(t *testing.T) { if err != nil { t.Fatal(err) } - if !reflect.DeepEqual(umMhook.Prestart[0], prestart) { - t.Errorf("Expected hooks to be equal after mashaling -> unmarshaling them: %+v, %+v", umMhook.Prestart[0], prestart) + if !reflect.DeepEqual(umMhook, hook) { + t.Errorf("Expected hooks to be equal after mashaling -> unmarshaling them: %+v, %+v", umMhook, hook) } } @@ -106,14 +120,14 @@ func TestMarshalHooksWithUnexpectedType(t *testing.T) { return nil }) hook := configs.Hooks{ - Prestart: []configs.Hook{fHook}, + configs.CreateRuntime: configs.HookList{fHook}, } hooks, err := hook.MarshalJSON() if err != nil { t.Fatal(err) } - h := `{"poststart":null,"poststop":null,"prestart":null}` + h := `{"createContainer":null,"createRuntime":null,"poststart":null,"poststop":null,"prestart":null,"startContainer":null}` if string(hooks) != h { t.Errorf("Expected hooks %s to equal %s", string(hooks), h) } diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 3808099a1..ada513b55 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -1118,39 +1118,67 @@ func TestHook(t *testing.T) { } return config.Rootfs, nil } + createFileFromBundle := func(filename, bundle string) error { + root, err := getRootfsFromBundle(bundle) + if err != nil { + return err + } - config.Hooks = &configs.Hooks{ - Prestart: []configs.Hook{ + f, err := os.Create(filepath.Join(root, filename)) + if err != nil { + return err + } + return f.Close() + } + + // Note FunctionHooks can't be serialized to json this means they won't be passed down to the container + // For CreateContainer and StartContainer which run in the container namespace, this means we need to pass Command Hooks. + hookFiles := map[configs.HookName]string{ + configs.Prestart: "prestart", + configs.CreateRuntime: "createRuntime", + configs.CreateContainer: "createContainer", + configs.StartContainer: "startContainer", + configs.Poststart: "poststart", + } + + config.Hooks = configs.Hooks{ + configs.Prestart: configs.HookList{ configs.NewFunctionHook(func(s *specs.State) error { if s.Bundle != expectedBundle { t.Fatalf("Expected prestart hook bundlePath '%s'; got '%s'", expectedBundle, s.Bundle) } - - root, err := getRootfsFromBundle(s.Bundle) - if err != nil { - return err - } - f, err := os.Create(filepath.Join(root, "test")) - if err != nil { - return err - } - return f.Close() + return createFileFromBundle(hookFiles[configs.Prestart], s.Bundle) }), }, - Poststart: []configs.Hook{ + configs.CreateRuntime: configs.HookList{ + configs.NewFunctionHook(func(s *specs.State) error { + if s.Bundle != expectedBundle { + t.Fatalf("Expected createRuntime hook bundlePath '%s'; got '%s'", expectedBundle, s.Bundle) + } + return createFileFromBundle(hookFiles[configs.CreateRuntime], s.Bundle) + }), + }, + configs.CreateContainer: configs.HookList{ + configs.NewCommandHook(configs.Command{ + Path: "/bin/bash", + Args: []string{"/bin/bash", "-c", fmt.Sprintf("touch ./%s", hookFiles[configs.CreateContainer])}, + }), + }, + configs.StartContainer: configs.HookList{ + configs.NewCommandHook(configs.Command{ + Path: "/bin/sh", + Args: []string{"/bin/sh", "-c", fmt.Sprintf("touch /%s", hookFiles[configs.StartContainer])}, + }), + }, + configs.Poststart: configs.HookList{ configs.NewFunctionHook(func(s *specs.State) error { if s.Bundle != expectedBundle { t.Fatalf("Expected poststart hook bundlePath '%s'; got '%s'", expectedBundle, s.Bundle) } - - root, err := getRootfsFromBundle(s.Bundle) - if err != nil { - return err - } - return ioutil.WriteFile(filepath.Join(root, "test"), []byte("hello world"), 0755) + return createFileFromBundle(hookFiles[configs.Poststart], s.Bundle) }), }, - Poststop: []configs.Hook{ + configs.Poststop: configs.HookList{ configs.NewFunctionHook(func(s *specs.State) error { if s.Bundle != expectedBundle { t.Fatalf("Expected poststop hook bundlePath '%s'; got '%s'", expectedBundle, s.Bundle) @@ -1160,7 +1188,13 @@ func TestHook(t *testing.T) { if err != nil { return err } - return os.RemoveAll(filepath.Join(root, "test")) + + for _, hook := range hookFiles { + if err = os.RemoveAll(filepath.Join(root, hook)); err != nil { + return err + } + } + return nil }), }, } @@ -1173,10 +1207,16 @@ func TestHook(t *testing.T) { container, err := newContainerWithName("test", config) ok(t, err) + // e.g: 'ls /prestart ...' + cmd := "ls " + for _, hook := range hookFiles { + cmd += "/" + hook + " " + } + var stdout bytes.Buffer pconfig := libcontainer.Process{ Cwd: "/", - Args: []string{"sh", "-c", "ls /test"}, + Args: []string{"sh", "-c", cmd}, Env: standardEnvironment, Stdin: nil, Stdout: &stdout, @@ -1188,30 +1228,15 @@ func TestHook(t *testing.T) { // 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") { - container.Destroy() - t.Fatalf("ls output doesn't have the expected file: %s", outputLs) - } - - // Check that the file is written by the poststart hook - testFilePath := filepath.Join(rootfs, "test") - contents, err := ioutil.ReadFile(testFilePath) - if err != nil { - t.Fatalf("cannot read file '%s': %s", testFilePath, err) - } - if string(contents) != "hello world" { - t.Fatalf("Expected test file to contain 'hello world'; got '%s'", string(contents)) - } - if err := container.Destroy(); err != nil { t.Fatalf("container destroy %s", err) } - fi, err := os.Stat(filepath.Join(rootfs, "test")) - if err == nil || !os.IsNotExist(err) { - t.Fatalf("expected file to not exist, got %s", fi.Name()) + + for _, hook := range []string{"prestart", "createRuntime", "poststart"} { + fi, err := os.Stat(filepath.Join(rootfs, hook)) + if err == nil || !os.IsNotExist(err) { + t.Fatalf("expected file '%s to not exists, but it does", fi.Name()) + } } } diff --git a/libcontainer/specconv/spec_linux_test.go b/libcontainer/specconv/spec_linux_test.go index 18bb7c7fd..1a506adc1 100644 --- a/libcontainer/specconv/spec_linux_test.go +++ b/libcontainer/specconv/spec_linux_test.go @@ -42,6 +42,33 @@ func TestCreateHooks(t *testing.T) { Args: []string{"--some", "thing"}, }, }, + CreateRuntime: []specs.Hook{ + { + Path: "/some/hook/path", + }, + { + Path: "/some/hook2/path", + Args: []string{"--some", "thing"}, + }, + }, + CreateContainer: []specs.Hook{ + { + Path: "/some/hook/path", + }, + { + Path: "/some/hook2/path", + Args: []string{"--some", "thing"}, + }, + }, + StartContainer: []specs.Hook{ + { + Path: "/some/hook/path", + }, + { + Path: "/some/hook2/path", + Args: []string{"--some", "thing"}, + }, + }, Poststart: []specs.Hook{ { Path: "/some/hook/path", @@ -77,19 +104,37 @@ func TestCreateHooks(t *testing.T) { conf := &configs.Config{} createHooks(rspec, conf) - prestart := conf.Hooks.Prestart + prestart := conf.Hooks[configs.Prestart] if len(prestart) != 2 { t.Error("Expected 2 Prestart hooks") } - poststart := conf.Hooks.Poststart + createRuntime := conf.Hooks[configs.CreateRuntime] + + if len(createRuntime) != 2 { + t.Error("Expected 2 createRuntime hooks") + } + + createContainer := conf.Hooks[configs.CreateContainer] + + if len(createContainer) != 2 { + t.Error("Expected 2 createContainer hooks") + } + + startContainer := conf.Hooks[configs.StartContainer] + + if len(startContainer) != 2 { + t.Error("Expected 2 startContainer hooks") + } + + poststart := conf.Hooks[configs.Poststart] if len(poststart) != 3 { t.Error("Expected 3 Poststart hooks") } - poststop := conf.Hooks.Poststop + poststop := conf.Hooks[configs.Poststop] if len(poststop) != 4 { t.Error("Expected 4 Poststop hooks")