diff options
-rw-r--r-- | cmd/podman/attach.go | 2 | ||||
-rw-r--r-- | cmd/podman/cleanup.go | 22 | ||||
-rw-r--r-- | cmd/podman/cliconfig/config.go | 1 | ||||
-rw-r--r-- | cmd/podman/run.go | 26 | ||||
-rw-r--r-- | cmd/podman/start.go | 7 | ||||
-rw-r--r-- | libpod/container_attach_linux.go | 4 | ||||
-rw-r--r-- | libpod/container_internal.go | 15 | ||||
-rw-r--r-- | libpod/errors.go | 5 | ||||
-rw-r--r-- | pkg/spec/createconfig.go | 17 | ||||
-rw-r--r-- | test/e2e/create_test.go | 8 | ||||
-rw-r--r-- | test/e2e/run_test.go | 2 | ||||
-rw-r--r-- | utils/utils.go | 12 |
12 files changed, 76 insertions, 45 deletions
diff --git a/cmd/podman/attach.go b/cmd/podman/attach.go index c29886825..ed175bdf4 100644 --- a/cmd/podman/attach.go +++ b/cmd/podman/attach.go @@ -74,7 +74,7 @@ func attachCmd(c *cliconfig.AttachValues) error { inputStream = nil } - if err := startAttachCtr(ctr, os.Stdout, os.Stderr, inputStream, c.DetachKeys, c.SigProxy, false); err != nil { + if err := startAttachCtr(ctr, os.Stdout, os.Stderr, inputStream, c.DetachKeys, c.SigProxy, false); err != nil && errors.Cause(err) != libpod.ErrDetach { return errors.Wrapf(err, "error attaching to container %s", ctr.ID()) } diff --git a/cmd/podman/cleanup.go b/cmd/podman/cleanup.go index b1f727d33..537679d75 100644 --- a/cmd/podman/cleanup.go +++ b/cmd/podman/cleanup.go @@ -37,6 +37,7 @@ func init() { flags.BoolVarP(&cleanupCommand.All, "all", "a", false, "Cleans up all containers") flags.BoolVarP(&cleanupCommand.Latest, "latest", "l", false, "Act on the latest container podman is aware of") + flags.BoolVar(&cleanupCommand.Remove, "rm", false, "After cleanup, remove the container entirely") } func cleanupCmd(c *cliconfig.CleanupValues) error { @@ -55,12 +56,25 @@ func cleanupCmd(c *cliconfig.CleanupValues) error { ctx := getContext() for _, ctr := range cleanupContainers { - if err = ctr.Cleanup(ctx); err != nil { - if lastError != nil { - fmt.Fprintln(os.Stderr, lastError) + hadError := false + if c.Remove { + if err := runtime.RemoveContainer(ctx, ctr, false); err != nil { + if lastError != nil { + fmt.Fprintln(os.Stderr, lastError) + } + lastError = errors.Wrapf(err, "failed to cleanup and remove container %v", ctr.ID()) + hadError = true } - lastError = errors.Wrapf(err, "failed to cleanup container %v", ctr.ID()) } else { + if err := ctr.Cleanup(ctx); err != nil { + if lastError != nil { + fmt.Fprintln(os.Stderr, lastError) + } + lastError = errors.Wrapf(err, "failed to cleanup container %v", ctr.ID()) + hadError = true + } + } + if !hadError { fmt.Println(ctr.ID()) } } diff --git a/cmd/podman/cliconfig/config.go b/cmd/podman/cliconfig/config.go index b925d29ff..85ded6da0 100644 --- a/cmd/podman/cliconfig/config.go +++ b/cmd/podman/cliconfig/config.go @@ -531,6 +531,7 @@ type CleanupValues struct { PodmanCommand All bool Latest bool + Remove bool } type SystemPruneValues struct { diff --git a/cmd/podman/run.go b/cmd/podman/run.go index 8649dc190..16ec7c3c0 100644 --- a/cmd/podman/run.go +++ b/cmd/podman/run.go @@ -118,6 +118,14 @@ func runCmd(c *cliconfig.RunValues) error { } } if err := startAttachCtr(ctr, outputStream, errorStream, inputStream, c.String("detach-keys"), c.Bool("sig-proxy"), true); err != nil { + // We've manually detached from the container + // Do not perform cleanup, or wait for container exit code + // Just exit immediately + if errors.Cause(err) == libpod.ErrDetach { + exitCode = 0 + return nil + } + // This means the command did not exist exitCode = 127 if strings.Index(err.Error(), "permission denied") > -1 { @@ -147,28 +155,12 @@ func runCmd(c *cliconfig.RunValues) error { exitCode = int(ecode) } - if createConfig.Rm { - return runtime.RemoveContainer(ctx, ctr, true) - } - - if err := ctr.Cleanup(ctx); err != nil { - // If the container has been removed already, no need to error on cleanup - // Also, if it was restarted, don't error either - if errors.Cause(err) == libpod.ErrNoSuchCtr || - errors.Cause(err) == libpod.ErrCtrRemoved || - errors.Cause(err) == libpod.ErrCtrStateInvalid { - return nil - } - - return err - } - return nil } // Read a container's exit file func readExitFile(runtimeTmp, ctrID string) (int, error) { - exitFile := filepath.Join(runtimeTmp, "exits", ctrID) + exitFile := filepath.Join(runtimeTmp, "exits", fmt.Sprintf("%s-old", ctrID)) logrus.Debugf("Attempting to read container %s exit code from file %s", ctrID, exitFile) diff --git a/cmd/podman/start.go b/cmd/podman/start.go index 344719fca..d1434508d 100644 --- a/cmd/podman/start.go +++ b/cmd/podman/start.go @@ -108,6 +108,13 @@ func startCmd(c *cliconfig.StartValues) error { // attach to the container and also start it not already running err = startAttachCtr(ctr, os.Stdout, os.Stderr, inputStream, c.DetachKeys, sigProxy, !ctrRunning) + if errors.Cause(err) == libpod.ErrDetach { + // User manually detached + // Exit cleanly immediately + exitCode = 0 + return nil + } + if ctrRunning { return err } diff --git a/libpod/container_attach_linux.go b/libpod/container_attach_linux.go index 1d6f0bd96..3ff6ddc76 100644 --- a/libpod/container_attach_linux.go +++ b/libpod/container_attach_linux.go @@ -109,8 +109,8 @@ func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSi case err := <-receiveStdoutError: return err case err := <-stdinDone: - if _, ok := err.(utils.DetachError); ok { - return nil + if err == ErrDetach { + return err } if streams.AttachOutput || streams.AttachError { return <-receiveStdoutError diff --git a/libpod/container_internal.go b/libpod/container_internal.go index b0dcc853e..f82cbd674 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -489,9 +489,20 @@ func (c *Container) removeConmonFiles() error { return errors.Wrapf(err, "error removing container %s OOM file", c.ID()) } + // Instead of outright deleting the exit file, rename it (if it exists). + // We want to retain it so we can get the exit code of containers which + // are removed (at least until we have a workable events system) exitFile := filepath.Join(c.runtime.ociRuntime.exitsDir, c.ID()) - if err := os.Remove(exitFile); err != nil && !os.IsNotExist(err) { - return errors.Wrapf(err, "error removing container %s exit file", c.ID()) + oldExitFile := filepath.Join(c.runtime.ociRuntime.exitsDir, fmt.Sprintf("%s-old", c.ID())) + if _, err := os.Stat(exitFile); err != nil { + if !os.IsNotExist(err) { + return errors.Wrapf(err, "error running stat on container %s exit file", c.ID()) + } + } else if err == nil { + // Rename should replace the old exit file (if it exists) + if err := os.Rename(exitFile, oldExitFile); err != nil { + return errors.Wrapf(err, "error renaming container %s exit file", c.ID()) + } } return nil diff --git a/libpod/errors.go b/libpod/errors.go index 30a19d30f..dd82d0796 100644 --- a/libpod/errors.go +++ b/libpod/errors.go @@ -4,6 +4,7 @@ import ( "errors" "github.com/containers/libpod/libpod/image" + "github.com/containers/libpod/utils" ) var ( @@ -56,6 +57,10 @@ var ( // ErrInternal indicates an internal library error ErrInternal = errors.New("internal libpod error") + // ErrDetach indicates that an attach session was manually detached by + // the user. + ErrDetach = utils.ErrDetach + // ErrRuntimeStopped indicates that the runtime has already been shut // down and no further operations can be performed on it ErrRuntimeStopped = errors.New("runtime has already been stopped") diff --git a/pkg/spec/createconfig.go b/pkg/spec/createconfig.go index f6c77d61c..8edab831f 100644 --- a/pkg/spec/createconfig.go +++ b/pkg/spec/createconfig.go @@ -11,7 +11,6 @@ import ( "github.com/containers/libpod/libpod" "github.com/containers/libpod/pkg/namespaces" - "github.com/containers/libpod/pkg/rootless" "github.com/containers/storage" "github.com/cri-o/ocicni/pkg/ocicni" "github.com/docker/go-connections/nat" @@ -340,7 +339,13 @@ func (c *CreateConfig) createExitCommand() []string { if c.Syslog { command = append(command, "--syslog") } - return append(command, []string{"container", "cleanup"}...) + command = append(command, []string{"container", "cleanup"}...) + + if c.Rm { + command = append(command, "--rm") + } + + return command } // GetContainerCreateOptions takes a CreateConfig and returns a slice of CtrCreateOptions @@ -518,11 +523,9 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l if c.CgroupParent != "" { options = append(options, libpod.WithCgroupParent(c.CgroupParent)) } - // For a rootless container always cleanup the storage/network as they - // run in a different namespace thus not reusable when we restart. - if c.Detach || rootless.IsRootless() { - options = append(options, libpod.WithExitCommand(c.createExitCommand())) - } + + // Always use a cleanup process to clean up Podman after termination + options = append(options, libpod.WithExitCommand(c.createExitCommand())) return options, nil } diff --git a/test/e2e/create_test.go b/test/e2e/create_test.go index b28a0c428..9a526b778 100644 --- a/test/e2e/create_test.go +++ b/test/e2e/create_test.go @@ -142,7 +142,7 @@ var _ = Describe("Podman create", func() { } mountPath := filepath.Join(podmanTest.TempDir, "secrets") os.Mkdir(mountPath, 0755) - session := podmanTest.Podman([]string{"create", "--name", "test", "--rm", "--mount", fmt.Sprintf("type=bind,src=%s,target=/create/test", mountPath), ALPINE, "grep", "/create/test", "/proc/self/mountinfo"}) + session := podmanTest.Podman([]string{"create", "--name", "test", "--mount", fmt.Sprintf("type=bind,src=%s,target=/create/test", mountPath), ALPINE, "grep", "/create/test", "/proc/self/mountinfo"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) session = podmanTest.Podman([]string{"start", "test"}) @@ -153,7 +153,7 @@ var _ = Describe("Podman create", func() { Expect(session.ExitCode()).To(Equal(0)) Expect(session.OutputToString()).To(ContainSubstring("/create/test rw")) - session = podmanTest.Podman([]string{"create", "--name", "test_ro", "--rm", "--mount", fmt.Sprintf("type=bind,src=%s,target=/create/test,ro", mountPath), ALPINE, "grep", "/create/test", "/proc/self/mountinfo"}) + session = podmanTest.Podman([]string{"create", "--name", "test_ro", "--mount", fmt.Sprintf("type=bind,src=%s,target=/create/test,ro", mountPath), ALPINE, "grep", "/create/test", "/proc/self/mountinfo"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) session = podmanTest.Podman([]string{"start", "test_ro"}) @@ -164,7 +164,7 @@ var _ = Describe("Podman create", func() { Expect(session.ExitCode()).To(Equal(0)) Expect(session.OutputToString()).To(ContainSubstring("/create/test ro")) - session = podmanTest.Podman([]string{"create", "--name", "test_shared", "--rm", "--mount", fmt.Sprintf("type=bind,src=%s,target=/create/test,shared", mountPath), ALPINE, "grep", "/create/test", "/proc/self/mountinfo"}) + session = podmanTest.Podman([]string{"create", "--name", "test_shared", "--mount", fmt.Sprintf("type=bind,src=%s,target=/create/test,shared", mountPath), ALPINE, "grep", "/create/test", "/proc/self/mountinfo"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) session = podmanTest.Podman([]string{"start", "test_shared"}) @@ -180,7 +180,7 @@ var _ = Describe("Podman create", func() { mountPath = filepath.Join(podmanTest.TempDir, "scratchpad") os.Mkdir(mountPath, 0755) - session = podmanTest.Podman([]string{"create", "--name", "test_tmpfs", "--rm", "--mount", "type=tmpfs,target=/create/test", ALPINE, "grep", "/create/test", "/proc/self/mountinfo"}) + session = podmanTest.Podman([]string{"create", "--name", "test_tmpfs", "--mount", "type=tmpfs,target=/create/test", ALPINE, "grep", "/create/test", "/proc/self/mountinfo"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) session = podmanTest.Podman([]string{"start", "test_tmpfs"}) diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 22a36bb6c..6bd49de33 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -322,7 +322,7 @@ var _ = Describe("Podman run", func() { os.Setenv("NOTIFY_SOCKET", sock) defer os.Unsetenv("NOTIFY_SOCKET") - session := podmanTest.Podman([]string{"run", "--rm", ALPINE, "printenv", "NOTIFY_SOCKET"}) + session := podmanTest.Podman([]string{"run", ALPINE, "printenv", "NOTIFY_SOCKET"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) Expect(len(session.OutputToStringArray())).To(BeNumerically(">", 0)) diff --git a/utils/utils.go b/utils/utils.go index c7c5ab5cf..4a91b304f 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -9,6 +9,7 @@ import ( systemdDbus "github.com/coreos/go-systemd/dbus" "github.com/godbus/dbus" + "github.com/pkg/errors" ) // ExecCmd executes a command with args and returns its output as a string along @@ -82,12 +83,9 @@ func newProp(name string, units interface{}) systemdDbus.Property { } } -// DetachError is special error which returned in case of container detach. -type DetachError struct{} - -func (DetachError) Error() string { - return "detached from container" -} +// ErrDetach is an error indicating that the user manually detached from the +// container. +var ErrDetach = errors.New("detached from container") // CopyDetachable is similar to io.Copy but support a detach key sequence to break out. func CopyDetachable(dst io.Writer, src io.Reader, keys []byte) (written int64, err error) { @@ -108,7 +106,7 @@ func CopyDetachable(dst io.Writer, src io.Reader, keys []byte) (written int64, e } if i == len(keys)-1 { // src.Close() - return 0, DetachError{} + return 0, ErrDetach } nr, er = src.Read(buf) } |