From 7fc1a329bd014d61f9895fc212aef452f6fb8f84 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Fri, 22 Jun 2018 16:44:59 -0400 Subject: [PATCH] Add `podman container cleanup` to CLI When we run containers in detach mode, nothing cleans up the network stack or the mount points. This patch will tell conmon to execute the cleanup code when the container exits. It can also be called to attempt to cleanup previously running containers. Signed-off-by: Daniel J Walsh Closes: #942 Approved by: mheon --- Dockerfile | 2 +- Dockerfile.CentOS | 2 +- Dockerfile.Fedora | 2 +- cmd/podman/cleanup.go | 92 +++++++++++++++++++++++++++++ cmd/podman/container.go | 1 + cmd/podman/create.go | 2 +- cmd/podman/run.go | 2 +- docs/podman-container-cleanup.1.md | 38 ++++++++++++ docs/podman-container.1.md | 1 + libpod/container.go | 4 ++ libpod/container_inspect.go | 1 + libpod/container_internal.go | 19 ++++-- libpod/oci.go | 6 ++ libpod/options.go | 12 ++++ pkg/inspect/inspect.go | 1 + pkg/spec/createconfig.go | 22 ++++++- pkg/varlinkapi/containers_create.go | 2 +- test/e2e/run_cleanup_test.go | 47 +++++++++++++++ 18 files changed, 243 insertions(+), 13 deletions(-) create mode 100644 cmd/podman/cleanup.go create mode 100644 docs/podman-container-cleanup.1.md create mode 100644 test/e2e/run_cleanup_test.go diff --git a/Dockerfile b/Dockerfile index 1ae4de13f5..53db6a3bce 100644 --- a/Dockerfile +++ b/Dockerfile @@ -58,7 +58,7 @@ RUN set -x \ && rm -rf "$GOPATH" # Install conmon -ENV CRIO_COMMIT 66788a10e57f42faf741c2f149d0ee6635063014 +ENV CRIO_COMMIT f9ae39e395880507d52295ca58e3683f22524777 RUN set -x \ && export GOPATH="$(mktemp -d)" \ && git clone https://github.com/kubernetes-incubator/cri-o.git "$GOPATH/src/github.com/kubernetes-incubator/cri-o.git" \ diff --git a/Dockerfile.CentOS b/Dockerfile.CentOS index 865585cfd2..ed79042e74 100644 --- a/Dockerfile.CentOS +++ b/Dockerfile.CentOS @@ -57,7 +57,7 @@ RUN set -x \ && go get github.com/onsi/gomega/... # Install conmon -ENV CRIO_COMMIT 66788a10e57f42faf741c2f149d0ee6635063014 +ENV CRIO_COMMIT f9ae39e395880507d52295ca58e3683f22524777 RUN set -x \ && export GOPATH="$(mktemp -d)" \ && git clone https://github.com/kubernetes-incubator/cri-o.git "$GOPATH/src/github.com/kubernetes-incubator/cri-o.git" \ diff --git a/Dockerfile.Fedora b/Dockerfile.Fedora index 589180bebb..6415db32be 100644 --- a/Dockerfile.Fedora +++ b/Dockerfile.Fedora @@ -59,7 +59,7 @@ RUN set -x \ && go get github.com/onsi/gomega/... # Install conmon -ENV CRIO_COMMIT 66788a10e57f42faf741c2f149d0ee6635063014 +ENV CRIO_COMMIT f9ae39e395880507d52295ca58e3683f22524777 RUN set -x \ && export GOPATH="$(mktemp -d)" \ && git clone https://github.com/kubernetes-incubator/cri-o.git "$GOPATH/src/github.com/kubernetes-incubator/cri-o.git" \ diff --git a/cmd/podman/cleanup.go b/cmd/podman/cleanup.go new file mode 100644 index 0000000000..33b0fad450 --- /dev/null +++ b/cmd/podman/cleanup.go @@ -0,0 +1,92 @@ +package main + +import ( + "fmt" + "os" + + "github.com/pkg/errors" + "github.com/projectatomic/libpod/cmd/podman/libpodruntime" + "github.com/projectatomic/libpod/libpod" + "github.com/urfave/cli" +) + +var ( + cleanupFlags = []cli.Flag{ + cli.BoolFlag{ + Name: "all, a", + Usage: "Cleans up all containers", + }, + LatestFlag, + } + cleanupDescription = ` + podman container cleanup + + Cleans up mount points and network stacks on one or more containers from the host. The container name or ID can be used. This command is used internally when running containers, but can also be used if container cleanup has failed when a container exits. +` + cleanupCommand = cli.Command{ + Name: "cleanup", + Usage: "Cleanup network and mountpoints of one or more containers", + Description: cleanupDescription, + Flags: cleanupFlags, + Action: cleanupCmd, + ArgsUsage: "CONTAINER-NAME [CONTAINER-NAME ...]", + } +) + +func cleanupCmd(c *cli.Context) error { + if err := validateFlags(c, cleanupFlags); err != nil { + return err + } + runtime, err := libpodruntime.GetRuntime(c) + if err != nil { + return errors.Wrapf(err, "could not get runtime") + } + defer runtime.Shutdown(false) + + args := c.Args() + + var lastError error + var cleanupContainers []*libpod.Container + if c.Bool("all") { + if c.Bool("lastest") { + return errors.New("--all and --latest cannot be used together") + } + if len(args) != 0 { + return errors.New("--all and explicit container IDs cannot be used together") + } + cleanupContainers, err = runtime.GetContainers() + if err != nil { + return errors.Wrapf(err, "unable to get container list") + } + } else if c.Bool("latest") { + if len(args) != 0 { + return errors.New("--latest and explicit container IDs cannot be used together") + } + lastCtr, err := runtime.GetLatestContainer() + if err != nil { + return errors.Wrapf(err, "unable to get latest container") + } + cleanupContainers = append(cleanupContainers, lastCtr) + } else { + for _, i := range args { + container, err := runtime.LookupContainer(i) + if err != nil { + fmt.Fprintln(os.Stderr, err) + lastError = errors.Wrapf(err, "unable to find container %s", i) + continue + } + cleanupContainers = append(cleanupContainers, container) + } + } + for _, ctr := range cleanupContainers { + if err = ctr.Cleanup(); err != nil { + if lastError != nil { + fmt.Fprintln(os.Stderr, lastError) + } + lastError = errors.Wrapf(err, "failed to cleanup container %v", ctr.ID()) + } else { + fmt.Println(ctr.ID()) + } + } + return lastError +} diff --git a/cmd/podman/container.go b/cmd/podman/container.go index e3256d78b6..e0684a3fdf 100644 --- a/cmd/podman/container.go +++ b/cmd/podman/container.go @@ -7,6 +7,7 @@ import ( var ( subCommands = []cli.Command{ attachCommand, + cleanupCommand, commitCommand, createCommand, diffCommand, diff --git a/cmd/podman/create.go b/cmd/podman/create.go index 4404069036..19e87c3061 100644 --- a/cmd/podman/create.go +++ b/cmd/podman/create.go @@ -116,7 +116,7 @@ func createCmd(c *cli.Context) error { return err } - options, err := createConfig.GetContainerCreateOptions() + options, err := createConfig.GetContainerCreateOptions(runtime) if err != nil { return err } diff --git a/cmd/podman/run.go b/cmd/podman/run.go index 421b868ad2..8eb7476868 100644 --- a/cmd/podman/run.go +++ b/cmd/podman/run.go @@ -110,7 +110,7 @@ func runCmd(c *cli.Context) error { return err } - options, err := createConfig.GetContainerCreateOptions() + options, err := createConfig.GetContainerCreateOptions(runtime) if err != nil { return err } diff --git a/docs/podman-container-cleanup.1.md b/docs/podman-container-cleanup.1.md new file mode 100644 index 0000000000..50054afe27 --- /dev/null +++ b/docs/podman-container-cleanup.1.md @@ -0,0 +1,38 @@ +% podman-container-cleanup "1" + +## NAME +podman\-container\-cleanup - Cleanup Container storage and networks + +## SYNOPSIS +**podman container cleanup [OPTIONS] CONTAINER** + +## DESCRIPTION +`podman container cleanup` cleans up exited containers by removing all mountpoints and network configuration from the host. The container name or ID can be used. The cleanup command does not remove the containers. Running containers will not be cleaned up. +Sometimes container's mount points and network stacks can remain if the podman command was killed or the container ran in daemon mode. This command is automatically executed when you run containers in daemon mode by the conmon process when the container exits. + +## OPTIONS + +**--all, a** + +Cleanup all containers. + +**--latest, -l** +Instead of providing the container name or ID, use the last created container. If you use methods other than Podman +to run containers such as CRI-O, the last started container could be from either of those methods. +## EXAMPLE + +`podman container cleanup mywebserver` + +`podman container cleanup mywebserver myflaskserver 860a4b23` + +`podman container cleanup 860a4b23` + +`podman container-cleanup -a` + +`podman container cleanup --latest` + +## SEE ALSO +podman(1), podman-container(1) + +## HISTORY +Jun 2018, Originally compiled by Dan Walsh diff --git a/docs/podman-container.1.md b/docs/podman-container.1.md index f79215270e..71ee9489f0 100644 --- a/docs/podman-container.1.md +++ b/docs/podman-container.1.md @@ -14,6 +14,7 @@ The container command allows you to manage containers | Command | Man Page | Description | | ------- | --------------------------------------------------- | ---------------------------------------------------------------------------- | | attach | [podman-attach(1)](podman-attach.1.md) | Attach to a running container. | +| cleanup | [podman-container-cleanup(1)](podman-container-cleanup.1.md) | Cleanup containers network and mountpoints. | | commit | [podman-commit(1)](podman-commit.1.md) | Create new image based on the changed container. | | create | [podman-create(1)](podman-create.1.md) | Create a new container. | | diff | [podman-diff(1)](podman-diff.1.md) | Inspect changes on a container or image's filesystem. | diff --git a/libpod/container.go b/libpod/container.go index 37dabd80d6..b7189ae0bb 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -314,6 +314,10 @@ type ContainerConfig struct { // TODO log options for log drivers PostConfigureNetNS bool `json:"postConfigureNetNS"` + + // ExitCommand is the container's exit command. + // This Command will be executed when the container exits + ExitCommand []string `json:"exitCommand,omitempty"` } // ContainerStatus returns a string representation for users diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go index 7dc4d34b99..1381341c23 100644 --- a/libpod/container_inspect.go +++ b/libpod/container_inspect.go @@ -71,6 +71,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *inspect.Data) }, ImageID: config.RootfsImageID, ImageName: config.RootfsImageName, + ExitCommand: config.ExitCommand, Rootfs: config.Rootfs, ResolvConfPath: resolvPath, HostnamePath: hostnamePath, diff --git a/libpod/container_internal.go b/libpod/container_internal.go index ba6cbe5aad..5c5bf80620 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -695,7 +695,8 @@ func (c *Container) stop(timeout uint) error { return err } - return c.cleanup() + // Container should clean itself up + return nil } // Internal, non-locking function to pause a container @@ -928,11 +929,17 @@ func (c *Container) cleanup() error { } if err := c.cleanupCgroups(); err != nil { - if lastError != nil { - logrus.Errorf("Error cleaning up container %s CGroups: %v", c.ID(), err) - } else { - lastError = err - } + /* + if lastError != nil { + logrus.Errorf("Error cleaning up container %s CGroups: %v", c.ID(), err) + } else { + lastError = err + } + */ + // For now we are going to only warn on failures to clean up cgroups + // We have a conflict with running podman containers cleanup in same cgroup as container + logrus.Warnf("Ignoring Error cleaning up container %s CGroups: %v", c.ID(), err) + } // Unmount storage diff --git a/libpod/oci.go b/libpod/oci.go index 8710696271..dfed3d6b8a 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -268,6 +268,12 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string) (er if ctr.config.ConmonPidFile != "" { args = append(args, "--conmon-pidfile", ctr.config.ConmonPidFile) } + if len(ctr.config.ExitCommand) > 0 { + args = append(args, "--exit-command", ctr.config.ExitCommand[0]) + for _, arg := range ctr.config.ExitCommand[1:] { + args = append(args, []string{"--exit-command-arg", arg}...) + } + } args = append(args, "--socket-dir-path", r.socketsDir) if ctr.config.Spec.Process.Terminal { args = append(args, "-t") diff --git a/libpod/options.go b/libpod/options.go index 02bcb86288..9f07db7ed1 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -485,6 +485,18 @@ func WithIDMappings(idmappings storage.IDMappingOptions) CtrCreateOption { } } +// WithExitCommand sets the ExitCommand for the container, appending on the ctr.ID() to the end +func WithExitCommand(exitCommand []string) CtrCreateOption { + return func(ctr *Container) error { + if ctr.valid { + return ErrCtrFinalized + } + + ctr.config.ExitCommand = append(exitCommand, ctr.ID()) + return nil + } +} + // WithIPCNSFrom indicates the the container should join the IPC namespace of // the given container. // If the container has joined a pod, it can only join the namespaces of diff --git a/pkg/inspect/inspect.go b/pkg/inspect/inspect.go index 765ee43a7e..61776f1e2b 100644 --- a/pkg/inspect/inspect.go +++ b/pkg/inspect/inspect.go @@ -167,6 +167,7 @@ type ContainerInspectData struct { Mounts []specs.Mount `json:"Mounts"` Dependencies []string `json:"Dependencies"` NetworkSettings *NetworkSettings `json:"NetworkSettings"` //TODO + ExitCommand []string `json:"ExitCommand"` } // ContainerInspectState represents the state of a container. diff --git a/pkg/spec/createconfig.go b/pkg/spec/createconfig.go index af0a62c65d..27df0c395e 100644 --- a/pkg/spec/createconfig.go +++ b/pkg/spec/createconfig.go @@ -316,8 +316,25 @@ func (c *CreateConfig) GetTmpfsMounts() []spec.Mount { return m } +func createExitCommand(runtime *libpod.Runtime) []string { + config := runtime.GetConfig() + + cmd, _ := os.Executable() + command := []string{cmd, + "--root", config.StorageConfig.GraphRoot, + "--runroot", config.StorageConfig.RunRoot, + "--log-level", logrus.GetLevel().String(), + "--cgroup-manager", config.CgroupManager, + "--tmpdir", config.TmpDir, + } + if config.StorageConfig.GraphDriverName != "" { + command = append(command, []string{"--storage-driver", config.StorageConfig.GraphDriverName}...) + } + return append(command, []string{"container", "cleanup"}...) +} + // GetContainerCreateOptions takes a CreateConfig and returns a slice of CtrCreateOptions -func (c *CreateConfig) GetContainerCreateOptions() ([]libpod.CtrCreateOption, error) { +func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime) ([]libpod.CtrCreateOption, error) { var options []libpod.CtrCreateOption var portBindings []ocicni.PortMapping var err error @@ -434,6 +451,9 @@ func (c *CreateConfig) GetContainerCreateOptions() ([]libpod.CtrCreateOption, er if c.CgroupParent != "" { options = append(options, libpod.WithCgroupParent(c.CgroupParent)) } + if c.Detach { + options = append(options, libpod.WithExitCommand(createExitCommand(runtime))) + } return options, nil } diff --git a/pkg/varlinkapi/containers_create.go b/pkg/varlinkapi/containers_create.go index 8268a3fdd2..da6707248f 100644 --- a/pkg/varlinkapi/containers_create.go +++ b/pkg/varlinkapi/containers_create.go @@ -47,7 +47,7 @@ func (i *LibpodAPI) CreateContainer(call ioprojectatomicpodman.VarlinkCall, conf return call.ReplyErrorOccurred(err.Error()) } - options, err := createConfig.GetContainerCreateOptions() + options, err := createConfig.GetContainerCreateOptions(runtime) if err != nil { return call.ReplyErrorOccurred(err.Error()) } diff --git a/test/e2e/run_cleanup_test.go b/test/e2e/run_cleanup_test.go new file mode 100644 index 0000000000..8adf8888cf --- /dev/null +++ b/test/e2e/run_cleanup_test.go @@ -0,0 +1,47 @@ +package integration + +import ( + "os" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Podman run exit", func() { + var ( + tempdir string + err error + podmanTest PodmanTest + ) + + BeforeEach(func() { + tempdir, err = CreateTempDirInTempDir() + if err != nil { + os.Exit(1) + } + podmanTest = PodmanCreate(tempdir) + podmanTest.RestoreAllArtifacts() + }) + + AfterEach(func() { + podmanTest.Cleanup() + + }) + + It("podman run -d mount cleanup test", func() { + mount := podmanTest.SystemExec("mount", nil) + mount.WaitWithDefaultTimeout() + out1 := mount.OutputToString() + result := podmanTest.Podman([]string{"run", "-d", ALPINE, "echo", "hello"}) + result.WaitWithDefaultTimeout() + Expect(result.ExitCode()).To(Equal(0)) + + result = podmanTest.SystemExec("sleep", []string{"5"}) + result.WaitWithDefaultTimeout() + + mount = podmanTest.SystemExec("mount", nil) + mount.WaitWithDefaultTimeout() + out2 := mount.OutputToString() + Expect(out1).To(Equal(out2)) + }) +})