Merge pull request #2421 from rhatdan/rmi

Change exit code to 1 on podman rmi nosuch image

5 files changed, 44 insertions, 44 deletions

diff --git a/cmd/podman/errors.go b/cmd/podman/errors.go
index 192f97049..2572b8779 100644
--- a/cmd/podman/errors.go
+++ b/cmd/podman/errors.go
@@ -6,8 +6,6 @@ import (
 	"os/exec"
 	"syscall"
 
-	"github.com/containers/libpod/cmd/podman/varlink"
-	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
@@ -20,22 +18,6 @@ func outputError(err error) {
 				exitCode = status.ExitStatus()
 			}
 		}
-		var ne error
-		switch e := err.(type) {
-		// For some reason golang wont let me list them with commas so listing them all.
-		case *iopodman.ImageNotFound:
-			ne = errors.New(e.Reason)
-		case *iopodman.ContainerNotFound:
-			ne = errors.New(e.Reason)
-		case *iopodman.PodNotFound:
-			ne = errors.New(e.Reason)
-		case *iopodman.VolumeNotFound:
-			ne = errors.New(e.Reason)
-		case *iopodman.ErrorOccurred:
-			ne = errors.New(e.Reason)
-		default:
-			ne = err
-		}
-		fmt.Fprintln(os.Stderr, "Error:", ne.Error())
+		fmt.Fprintln(os.Stderr, "Error:", err.Error())
 	}
 }
diff --git a/cmd/podman/rmi.go b/cmd/podman/rmi.go
index fbaa19336..709ed14e0 100644
--- a/cmd/podman/rmi.go
+++ b/cmd/podman/rmi.go
@@ -5,6 +5,8 @@ import (
 	"os"
 
 	"github.com/containers/libpod/cmd/podman/cliconfig"
+	"github.com/containers/libpod/cmd/podman/varlink"
+	"github.com/containers/libpod/libpod/image"
 	"github.com/containers/libpod/pkg/adapter"
 	"github.com/containers/storage"
 	"github.com/pkg/errors"
@@ -29,6 +31,17 @@ var (
 	}
 )
 
+func imageNotFound(err error) bool {
+	if errors.Cause(err) == image.ErrNoSuchImage {
+		return true
+	}
+	switch err.(type) {
+	case *iopodman.ImageNotFound:
+		return true
+	}
+	return false
+}
+
 func init() {
 	rmiCommand.Command = _rmiCommand
 	rmiCommand.SetUsageTemplate(UsageTemplate())
@@ -39,10 +52,8 @@ func init() {
 
 func rmiCmd(c *cliconfig.RmiValues) error {
 	var (
-		lastError error
-		deleted   bool
-		deleteErr error
-		msg       string
+		lastError  error
+		failureCnt int
 	)
 
 	ctx := getContext()
@@ -64,19 +75,21 @@ func rmiCmd(c *cliconfig.RmiValues) error {
 	images := args[:]
 
 	removeImage := func(img *adapter.ContainerImage) {
-		deleted = true
-		msg, deleteErr = runtime.RemoveImage(ctx, img, c.Force)
-		if deleteErr != nil {
-			if errors.Cause(deleteErr) == storage.ErrImageUsedByContainer {
+		msg, err := runtime.RemoveImage(ctx, img, c.Force)
+		if err != nil {
+			if errors.Cause(err) == storage.ErrImageUsedByContainer {
 				fmt.Printf("A container associated with containers/storage, i.e. via Buildah, CRI-O, etc., may be associated with this image: %-12.12s\n", img.ID())
 			}
+			if !imageNotFound(err) {
+				failureCnt++
+			}
 			if lastError != nil {
 				fmt.Fprintln(os.Stderr, lastError)
 			}
-			lastError = deleteErr
-		} else {
-			fmt.Println(msg)
+			lastError = err
+			return
 		}
+		fmt.Println(msg)
 	}
 
 	if removeAll {
@@ -121,22 +134,21 @@ func rmiCmd(c *cliconfig.RmiValues) error {
 		for _, i := range images {
 			newImage, err := runtime.NewImageFromLocal(i)
 			if err != nil {
-				fmt.Fprintln(os.Stderr, err)
+				if lastError != nil {
+					if !imageNotFound(lastError) {
+						failureCnt++
+					}
+					fmt.Fprintln(os.Stderr, lastError)
+				}
+				lastError = err
 				continue
 			}
 			removeImage(newImage)
 		}
 	}
 
-	// If the user calls remove all and there are none, it should not be a
-	// non-zero exit
-	if !deleted && removeAll {
-		return nil
-	}
-	// the user tries to remove images that do not exist, that should be a
-	// non-zero exit
-	if !deleted {
-		return errors.Errorf("no valid images to delete")
+	if imageNotFound(lastError) && failureCnt == 0 {
+		exitCode = 1
 	}
 
 	return lastError
diff --git a/docs/podman-container-cleanup.1.md b/docs/podman-container-cleanup.1.md
index e375c12ec..2ad39d214 100644
--- a/docs/podman-container-cleanup.1.md
+++ b/docs/podman-container-cleanup.1.md
@@ -30,7 +30,7 @@ The latest option is not supported on the remote client.
 
 `podman container cleanup 860a4b23`
 
-`podman container-cleanup -a`
+`podman container cleanup -a`
 
 `podman container cleanup --latest`
 
diff --git a/docs/podman-rmi.1.md b/docs/podman-rmi.1.md
index 9c080c9f1..8c22bba2c 100644
--- a/docs/podman-rmi.1.md
+++ b/docs/podman-rmi.1.md
@@ -1,9 +1,11 @@
-% podman-rmi(1)
+% podman-image-rm(1)
 
 ## NAME
-podman\-rmi - Removes one or more images
+podman\-image\-rm (podman\-rmi) - Removes one or more images
 
 ## SYNOPSIS
+**podman image rm** *image* ...
+
 **podman rmi** *image* ...
 
 ## DESCRIPTION
@@ -38,6 +40,10 @@ Remove all images and containers.
 ```
 podman rmi -a -f
 ```
+## Exit Status
+**_0_** if all specified images removed
+**_1_** if one of the specified images did not exist, and no other failures
+**_125_** if command fails for a reason other then an image did not exist
 
 ## SEE ALSO
 podman(1)
diff --git a/test/e2e/rmi_test.go b/test/e2e/rmi_test.go
index c160e1bc5..dcbda2df4 100644
--- a/test/e2e/rmi_test.go
+++ b/test/e2e/rmi_test.go
@@ -36,7 +36,7 @@ var _ = Describe("Podman rmi", func() {
 	It("podman rmi bogus image", func() {
 		session := podmanTest.Podman([]string{"rmi", "debian:6.0.10"})
 		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(125))
+		Expect(session.ExitCode()).To(Equal(1))
 
 	})