summaryrefslogtreecommitdiff
path: root/cmd
diff options
context:
space:
mode:
authorMatthew Heon <mheon@redhat.com>2019-02-11 12:57:08 -0500
committerMatthew Heon <mheon@redhat.com>2019-02-12 10:14:57 -0500
commitb6775d5d22d463e4d92d6358ccd48dab6f8a1862 (patch)
treea0c0373125736b8b9bf59c93a794c7cadadd05c2 /cmd
parentbdf537f4fc983c30c945297d9ee33891a127f9bd (diff)
downloadpodman-b6775d5d22d463e4d92d6358ccd48dab6f8a1862.tar.gz
podman-b6775d5d22d463e4d92d6358ccd48dab6f8a1862.tar.bz2
podman-b6775d5d22d463e4d92d6358ccd48dab6f8a1862.zip
Fix manual detach from containers to not wait for exit
At present, when manually detaching from an attached container (using the detach hotkeys, default C-p C-q), Podman will still wait for the container to exit to obtain its exit code (so we can set Podman's exit code to match). This is correct in the case where attach finished because the container exited, but very wrong for the manual detach case. As a result of this, we can no longer guarantee that the cleanup and --rm functions will fire at the end of 'podman run' - we may be exiting before we get that far. Cleanup is easy enough - we swap to unconditionally using the cleanup processes we've used for detached and rootless containers all along. To duplicate --rm we need to also teach 'podman cleanup' to optionally remove containers instead of cleaning them up. (There is an argument for just using 'podman rm' instead of 'podman cleanup --rm', but cleanup does have different semantics given that we only ever expect it to run when the container has just exited. I think it might be useful to keep the two separate for things like 'podman events'...) Signed-off-by: Matthew Heon <mheon@redhat.com>
Diffstat (limited to 'cmd')
-rw-r--r--cmd/podman/attach.go2
-rw-r--r--cmd/podman/cleanup.go22
-rw-r--r--cmd/podman/cliconfig/config.go1
-rw-r--r--cmd/podman/run.go24
-rw-r--r--cmd/podman/start.go7
-rw-r--r--cmd/podman/utils.go2
6 files changed, 36 insertions, 22 deletions
diff --git a/cmd/podman/attach.go b/cmd/podman/attach.go
index c29886825..8e760582d 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 && 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..064551189 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.Rm, "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.Rm {
+ 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..e895b4a49 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
+ Rm bool
}
type SystemPruneValues struct {
diff --git a/cmd/podman/run.go b/cmd/podman/run.go
index 8649dc190..6002578ff 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 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,22 +155,6 @@ 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
}
diff --git a/cmd/podman/start.go b/cmd/podman/start.go
index 344719fca..1de258aa4 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 err == libpod.ErrDetach {
+ // User manually detached
+ // Exit cleanly immediately
+ exitCode = 0
+ return nil
+ }
+
if ctrRunning {
return err
}
diff --git a/cmd/podman/utils.go b/cmd/podman/utils.go
index 744d010d5..c62015344 100644
--- a/cmd/podman/utils.go
+++ b/cmd/podman/utils.go
@@ -91,7 +91,7 @@ func startAttachCtr(ctr *libpod.Container, stdout, stderr, stdin *os.File, detac
err = <-attachChan
if err != nil {
- return errors.Wrapf(err, "error attaching to container %s", ctr.ID())
+ return err
}
return nil