failed containers with --rm should remove themselves

when starting or running a container that has --rm, if the starting
container fails (like due to an invalid command), the container should
get removed.

Resolves: #1985

Signed-off-by: baude <bbaude@redhat.com>

4 files changed, 72 insertions, 0 deletions

diff --git a/cmd/podman/run.go b/cmd/podman/run.go
index a4b5c918e..20cb85347 100644
--- a/cmd/podman/run.go
+++ b/cmd/podman/run.go
@@ -116,6 +116,11 @@ func runCmd(c *cli.Context) error {
 		if strings.Index(err.Error(), "permission denied") > -1 {
 			exitCode = 126
 		}
+		if c.IsSet("rm") {
+			if deleteError := runtime.RemoveContainer(ctx, ctr, true); deleteError != nil {
+				logrus.Errorf("unable to remove container %s after failing to start and attach to it", ctr.ID())
+			}
+		}
 		return err
 	}
 
diff --git a/cmd/podman/start.go b/cmd/podman/start.go
index 8cf85405e..8bb386c68 100644
--- a/cmd/podman/start.go
+++ b/cmd/podman/start.go
@@ -1,11 +1,13 @@
 package main
 
 import (
+	"encoding/json"
 	"fmt"
 	"os"
 
 	"github.com/containers/libpod/cmd/podman/libpodruntime"
 	"github.com/containers/libpod/libpod"
+	cc "github.com/containers/libpod/pkg/spec"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
@@ -132,6 +134,18 @@ func startCmd(c *cli.Context) error {
 		}
 		// Handle non-attach start
 		if err := ctr.Start(ctx); err != nil {
+			var createArtifact cc.CreateConfig
+			artifact, artifactErr := ctr.GetArtifact("create-config")
+			if artifactErr == nil {
+				if jsonErr := json.Unmarshal(artifact, &createArtifact); jsonErr != nil {
+					logrus.Errorf("unable to detect if container %s should be deleted", ctr.ID())
+				}
+				if createArtifact.Rm {
+					if rmErr := runtime.RemoveContainer(ctx, ctr, true); rmErr != nil {
+						logrus.Errorf("unable to remove container %s after it failed to start", ctr.ID())
+					}
+				}
+			}
 			if lastError != nil {
 				fmt.Fprintln(os.Stderr, lastError)
 			}
diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go
index 4a9bd4e46..2104991b2 100644
--- a/test/e2e/run_test.go
+++ b/test/e2e/run_test.go
@@ -639,4 +639,31 @@ USER mail`
 		match, _ := check.GrepString("foobar")
 		Expect(match).To(BeTrue())
 	})
+
+	It("podman run --rm should work", func() {
+		session := podmanTest.Podman([]string{"run", "--rm", ALPINE, "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		numContainers := podmanTest.NumberOfContainers()
+		Expect(numContainers).To(Equal(0))
+	})
+
+	It("podman run --rm failed container should delete itself", func() {
+		session := podmanTest.Podman([]string{"run", "--rm", ALPINE, "foo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Not(Equal(0)))
+
+		numContainers := podmanTest.NumberOfContainers()
+		Expect(numContainers).To(Equal(0))
+	})
+
+	It("podman run failed container should NOT delete itself", func() {
+		session := podmanTest.Podman([]string{"run", ALPINE, "foo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Not(Equal(0)))
+
+		numContainers := podmanTest.NumberOfContainers()
+		Expect(numContainers).To(Equal(1))
+	})
 })
diff --git a/test/e2e/start_test.go b/test/e2e/start_test.go
index c11511d1f..64245c609 100644
--- a/test/e2e/start_test.go
+++ b/test/e2e/start_test.go
@@ -89,4 +89,30 @@ var _ = Describe("Podman start", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(125))
 	})
+
+	It("podman failed to start with --rm should delete the container", func() {
+		session := podmanTest.Podman([]string{"create", "-it", "--rm", ALPINE, "foo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		start := podmanTest.Podman([]string{"start", "-l"})
+		start.WaitWithDefaultTimeout()
+		Expect(start.ExitCode()).To(Not(Equal(0)))
+
+		numContainers := podmanTest.NumberOfContainers()
+		Expect(numContainers).To(BeZero())
+	})
+
+	It("podman failed to start without --rm should NOT delete the container", func() {
+		session := podmanTest.Podman([]string{"create", "-it", ALPINE, "foo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		start := podmanTest.Podman([]string{"start", "-l"})
+		start.WaitWithDefaultTimeout()
+		Expect(start.ExitCode()).To(Not(Equal(0)))
+
+		numContainers := podmanTest.NumberOfContainers()
+		Expect(numContainers).To(Equal(1))
+	})
 })