diff options
author | W. Trevor King <wking@tremily.us> | 2018-05-31 11:47:17 -0700 |
---|---|---|
committer | Atomic Bot <atomic-devel@projectatomic.io> | 2018-06-04 18:36:40 +0000 |
commit | c9f763456cd8263c3f2d84c6b2b6e17ad81cf3ba (patch) | |
tree | b1cffd82978145f372893f62754432d75003b31d /libpod | |
parent | 28d1cec9f64cca11d42410c6e33c43b01b1d7678 (diff) | |
download | podman-c9f763456cd8263c3f2d84c6b2b6e17ad81cf3ba.tar.gz podman-c9f763456cd8263c3f2d84c6b2b6e17ad81cf3ba.tar.bz2 podman-c9f763456cd8263c3f2d84c6b2b6e17ad81cf3ba.zip |
libpod: Execute poststop hooks locally
Instead of delegating to the runtime, since some runtimes do not seem
to handle these reliably [1].
[1]: https://github.com/projectatomic/libpod/issues/730#issuecomment-392959938
Signed-off-by: W. Trevor King <wking@tremily.us>
Closes: #864
Approved by: rhatdan
Diffstat (limited to 'libpod')
-rw-r--r-- | libpod/container.go | 4 | ||||
-rw-r--r-- | libpod/container_internal.go | 82 | ||||
-rw-r--r-- | libpod/container_internal_test.go | 80 | ||||
-rw-r--r-- | libpod/runtime_ctr.go | 11 | ||||
-rw-r--r-- | libpod/runtime_img.go | 4 | ||||
-rw-r--r-- | libpod/runtime_pod.go | 8 |
6 files changed, 166 insertions, 23 deletions
diff --git a/libpod/container.go b/libpod/container.go index 1e3b923c0..85b6fa3dd 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -166,6 +166,10 @@ type containerState struct { // UserNSRoot is the directory used as root for the container when using // user namespaces. UserNSRoot string `json:"userNSRoot,omitempty"` + + // ExtensionStageHooks holds hooks which will be executed by libpod + // and not delegated to the OCI runtime. + ExtensionStageHooks map[string][]spec.Hook `json:"extensionStageHooks,omitempty"` } // ExecSession contains information on an active exec session diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 258ee6ffd..84724e013 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1,6 +1,7 @@ package libpod import ( + "bytes" "context" "encoding/json" "fmt" @@ -27,6 +28,7 @@ import ( crioAnnotations "github.com/projectatomic/libpod/pkg/annotations" "github.com/projectatomic/libpod/pkg/chrootuser" "github.com/projectatomic/libpod/pkg/hooks" + "github.com/projectatomic/libpod/pkg/hooks/exec" "github.com/projectatomic/libpod/pkg/secrets" "github.com/projectatomic/libpod/pkg/util" "github.com/sirupsen/logrus" @@ -548,10 +550,10 @@ func (c *Container) reinit(ctx context.Context) error { return err } - // Delete the container in the runtime - if err := c.runtime.ociRuntime.deleteContainer(c); err != nil { - return errors.Wrapf(err, "error removing container %s from runtime", c.ID()) + if err := c.delete(ctx); err != nil { + return err } + // Our state is now Configured, as we've removed ourself from // the runtime // Set and save now to make sure that, if the init() below fails @@ -850,6 +852,62 @@ func (c *Container) cleanup() error { return lastError } +// delete deletes the container and runs any configured poststop +// hooks. +func (c *Container) delete(ctx context.Context) (err error) { + if err := c.runtime.ociRuntime.deleteContainer(c); err != nil { + return errors.Wrapf(err, "error removing container %s from runtime", c.ID()) + } + + if err := c.postDeleteHooks(ctx); err != nil { + return errors.Wrapf(err, "container %s poststop hooks", c.ID()) + } + + return nil +} + +// postDeleteHooks runs the poststop hooks (if any) as specified by +// the OCI Runtime Specification (which requires them to run +// post-delete, despite the stage name). +func (c *Container) postDeleteHooks(ctx context.Context) (err error) { + if c.state.ExtensionStageHooks != nil { + extensionHooks, ok := c.state.ExtensionStageHooks["poststop"] + if ok { + state, err := json.Marshal(spec.State{ + Version: spec.Version, + ID: c.ID(), + Status: "stopped", + Bundle: c.bundlePath(), + Annotations: c.config.Spec.Annotations, + }) + if err != nil { + return err + } + for i, hook := range extensionHooks { + logrus.Debugf("container %s: invoke poststop hook %d", c.ID(), i) + var stderr, stdout bytes.Buffer + hookErr, err := exec.Run(ctx, &hook, state, &stdout, &stderr, exec.DefaultPostKillTimeout) + if err != nil { + logrus.Warnf("container %s: poststop hook %d: %v", c.ID(), i, err) + if hookErr != err { + logrus.Debugf("container %s: poststop hook %d (hook error): %v", c.ID(), i, hookErr) + } + stdoutString := stdout.String() + if stdoutString != "" { + logrus.Debugf("container %s: poststop hook %d: stdout:\n%s", c.ID(), i, stdoutString) + } + stderrString := stderr.String() + if stderrString != "" { + logrus.Debugf("container %s: poststop hook %d: stderr:\n%s", c.ID(), i, stderrString) + } + } + } + } + } + + return nil +} + // Make standard bind mounts to include in the container func (c *Container) makeBindMounts() error { if err := os.Chown(c.state.RunDir, c.RootUID(), c.RootGID()); err != nil { @@ -1092,7 +1150,8 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { } } - if err := c.setupOCIHooks(ctx, &g); err != nil { + var err error + if c.state.ExtensionStageHooks, err = c.setupOCIHooks(ctx, &g); err != nil { return nil, errors.Wrapf(err, "error setting up OCI Hooks") } // Bind builtin image volumes @@ -1313,9 +1372,9 @@ func (c *Container) saveSpec(spec *spec.Spec) error { return nil } -func (c *Container) setupOCIHooks(ctx context.Context, g *generate.Generator) error { +func (c *Container) setupOCIHooks(ctx context.Context, g *generate.Generator) (extensionStageHooks map[string][]spec.Hook, err error) { if c.runtime.config.HooksDir == "" { - return nil + return nil, nil } var locale string @@ -1341,19 +1400,18 @@ func (c *Container) setupOCIHooks(ctx context.Context, g *generate.Generator) er logrus.Warnf("failed to parse language %q: %s", langString, err) lang, err = language.Parse("und-u-va-posix") if err != nil { - return err + return nil, err } } - manager, err := hooks.New(ctx, []string{c.runtime.config.HooksDir}, []string{}, lang) + manager, err := hooks.New(ctx, []string{c.runtime.config.HooksDir}, []string{"poststop"}, lang) if err != nil { if c.runtime.config.HooksDirNotExistFatal || !os.IsNotExist(err) { - return err + return nil, err } logrus.Warnf("failed to load hooks: {}", err) - return nil + return nil, nil } - _, err = manager.Hooks(g.Spec(), c.Spec().Annotations, len(c.config.UserVolumes) > 0) - return err + return manager.Hooks(g.Spec(), c.Spec().Annotations, len(c.config.UserVolumes) > 0) } diff --git a/libpod/container_internal_test.go b/libpod/container_internal_test.go new file mode 100644 index 000000000..78891a672 --- /dev/null +++ b/libpod/container_internal_test.go @@ -0,0 +1,80 @@ +package libpod + +import ( + "context" + "fmt" + "io/ioutil" + "os" + "path/filepath" + "runtime" + "testing" + + rspec "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/assert" +) + +// hookPath is the path to an example hook executable. +var hookPath string + +func TestPostDeleteHooks(t *testing.T) { + ctx := context.Background() + dir, err := ioutil.TempDir("", "libpod_test_") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + statePath := filepath.Join(dir, "state") + copyPath := filepath.Join(dir, "copy") + c := Container{ + config: &ContainerConfig{ + ID: "123abc", + Spec: &rspec.Spec{ + Annotations: map[string]string{ + "a": "b", + }, + }, + StaticDir: dir, // not the bundle, but good enough for this test + }, + state: &containerState{ + ExtensionStageHooks: map[string][]rspec.Hook{ + "poststop": { + rspec.Hook{ + Path: hookPath, + Args: []string{"sh", "-c", fmt.Sprintf("cat >%s", statePath)}, + }, + rspec.Hook{ + Path: "/does/not/exist", + }, + rspec.Hook{ + Path: hookPath, + Args: []string{"sh", "-c", fmt.Sprintf("cp %s %s", statePath, copyPath)}, + }, + }, + }, + }, + } + err = c.postDeleteHooks(ctx) + if err != nil { + t.Fatal(err) + } + + stateRegexp := "{\"ociVersion\":\"1\\.0\\.0\",\"id\":\"123abc\",\"status\":\"stopped\",\"bundle\":\"/tmp/libpod_test_[0-9]*\",\"annotations\":{\"a\":\"b\"}}" + for _, path := range []string{statePath, copyPath} { + t.Run(path, func(t *testing.T) { + content, err := ioutil.ReadFile(path) + if err != nil { + t.Fatal(err) + } + assert.Regexp(t, stateRegexp, string(content)) + }) + } +} + +func init() { + if runtime.GOOS != "windows" { + hookPath = "/bin/sh" + } else { + panic("we need a reliable executable path on Windows") + } +} diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index c6973ff2a..bde9db764 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -154,16 +154,16 @@ func (r *Runtime) NewContainer(ctx context.Context, rSpec *spec.Spec, options .. // RemoveContainer removes the given container // If force is specified, the container will be stopped first // Otherwise, RemoveContainer will return an error if the container is running -func (r *Runtime) RemoveContainer(c *Container, force bool) error { +func (r *Runtime) RemoveContainer(ctx context.Context, c *Container, force bool) error { r.lock.Lock() defer r.lock.Unlock() - return r.removeContainer(c, force) + return r.removeContainer(ctx, c, force) } // Internal function to remove a container // Locks the container, but does not lock the runtime -func (r *Runtime) removeContainer(c *Container, force bool) error { +func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool) error { if !c.valid { // Container probably already removed // Or was never in the runtime to begin with @@ -263,8 +263,9 @@ func (r *Runtime) removeContainer(c *Container, force bool) error { // Only do this if we're not ContainerStateConfigured - if we are, // we haven't been created in the runtime yet if c.state.State != ContainerStateConfigured { - if err := r.ociRuntime.deleteContainer(c); err != nil { - return errors.Wrapf(err, "error removing container %s from OCI runtime", c.ID()) + err = c.delete(ctx) + if err != nil { + return err } } diff --git a/libpod/runtime_img.go b/libpod/runtime_img.go index 2392c41d4..8b0c08cd3 100644 --- a/libpod/runtime_img.go +++ b/libpod/runtime_img.go @@ -75,7 +75,7 @@ type CopyOptions struct { // RemoveImage deletes an image from local storage // Images being used by running containers can only be removed if force=true -func (r *Runtime) RemoveImage(image *image.Image, force bool) (string, error) { +func (r *Runtime) RemoveImage(ctx context.Context, image *image.Image, force bool) (string, error) { r.lock.Lock() defer r.lock.Unlock() @@ -97,7 +97,7 @@ func (r *Runtime) RemoveImage(image *image.Image, force bool) (string, error) { if len(imageCtrs) > 0 && len(image.Names()) <= 1 { if force { for _, ctr := range imageCtrs { - if err := r.removeContainer(ctr, true); err != nil { + if err := r.removeContainer(ctx, ctr, true); err != nil { return "", errors.Wrapf(err, "error removing image %s: container %s using image could not be removed", image.ID(), ctr.ID()) } } diff --git a/libpod/runtime_pod.go b/libpod/runtime_pod.go index 3872a4b67..067a110fa 100644 --- a/libpod/runtime_pod.go +++ b/libpod/runtime_pod.go @@ -1,6 +1,7 @@ package libpod import ( + "context" "path" "path/filepath" "strings" @@ -91,7 +92,7 @@ func (r *Runtime) NewPod(options ...PodCreateOption) (*Pod, error) { // If force is specified with removeCtrs, all containers will be stopped before // being removed // Otherwise, the pod will not be removed if any containers are running -func (r *Runtime) RemovePod(p *Pod, removeCtrs, force bool) error { +func (r *Runtime) RemovePod(ctx context.Context, p *Pod, removeCtrs, force bool) error { r.lock.Lock() defer r.lock.Unlock() @@ -206,11 +207,10 @@ func (r *Runtime) RemovePod(p *Pod, removeCtrs, force bool) error { // Delete the container from runtime (only if we are not // ContainerStateConfigured) if ctr.state.State != ContainerStateConfigured { - if err := r.ociRuntime.deleteContainer(ctr); err != nil { - return errors.Wrapf(err, "error removing container %s from runtime", ctr.ID()) + if err := ctr.delete(ctx); err != nil { + return err } } - } // Remove containers from the state |