Fix handling of remove of bogus volumes, networks and Pods

In podman containers rm and podman images rm, the commands
exit with error code 1 if the object does not exists.

This PR implements similar functionality to volumes, networks, and Pods.

Similarly if volumes or Networks are in use by other containers, and return
exit code 2.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

17 files changed, 113 insertions, 15 deletions

diff --git a/cmd/podman/containers/rm.go b/cmd/podman/containers/rm.go
index 8d0c7920f..f8f12234d 100644
--- a/cmd/podman/containers/rm.go
+++ b/cmd/podman/containers/rm.go
@@ -105,7 +105,7 @@ func removeContainers(namesOrIDs []string, rmOptions entities.RmOptions, setExit
 	}
 	responses, err := registry.ContainerEngine().ContainerRm(context.Background(), namesOrIDs, rmOptions)
 	if err != nil {
-		if setExit && len(namesOrIDs) < 2 {
+		if setExit {
 			setExitCode(err)
 		}
 		return err
@@ -132,7 +132,7 @@ func setExitCode(err error) {
 	switch {
 	case cause == define.ErrNoSuchCtr:
 		registry.SetExitCode(1)
-	case strings.Contains(cause.Error(), define.ErrNoSuchImage.Error()):
+	case strings.Contains(cause.Error(), define.ErrNoSuchCtr.Error()):
 		registry.SetExitCode(1)
 	case cause == define.ErrCtrStateInvalid:
 		registry.SetExitCode(2)
diff --git a/cmd/podman/networks/rm.go b/cmd/podman/networks/rm.go
index ac49993b7..86aad43cb 100644
--- a/cmd/podman/networks/rm.go
+++ b/cmd/podman/networks/rm.go
@@ -2,10 +2,13 @@ package network
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/containers/podman/v2/cmd/podman/registry"
 	"github.com/containers/podman/v2/cmd/podman/utils"
+	"github.com/containers/podman/v2/libpod/define"
 	"github.com/containers/podman/v2/pkg/domain/entities"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -47,14 +50,30 @@ func networkRm(cmd *cobra.Command, args []string) error {
 
 	responses, err := registry.ContainerEngine().NetworkRm(registry.Context(), args, networkRmOptions)
 	if err != nil {
+		setExitCode(err)
 		return err
 	}
 	for _, r := range responses {
 		if r.Err == nil {
 			fmt.Println(r.Name)
 		} else {
+			setExitCode(r.Err)
 			errs = append(errs, r.Err)
 		}
 	}
 	return errs.PrintErrors()
 }
+
+func setExitCode(err error) {
+	cause := errors.Cause(err)
+	switch {
+	case cause == define.ErrNoSuchNetwork:
+		registry.SetExitCode(1)
+	case strings.Contains(cause.Error(), define.ErrNoSuchNetwork.Error()):
+		registry.SetExitCode(1)
+	case cause == define.ErrNetworkInUse:
+		registry.SetExitCode(2)
+	case strings.Contains(cause.Error(), define.ErrNetworkInUse.Error()):
+		registry.SetExitCode(2)
+	}
+}
diff --git a/cmd/podman/pods/rm.go b/cmd/podman/pods/rm.go
index 0bb35e1ca..2975db3e8 100644
--- a/cmd/podman/pods/rm.go
+++ b/cmd/podman/pods/rm.go
@@ -3,12 +3,15 @@ package pods
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/containers/podman/v2/cmd/podman/common"
 	"github.com/containers/podman/v2/cmd/podman/registry"
 	"github.com/containers/podman/v2/cmd/podman/utils"
 	"github.com/containers/podman/v2/cmd/podman/validate"
+	"github.com/containers/podman/v2/libpod/define"
 	"github.com/containers/podman/v2/pkg/domain/entities"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -73,6 +76,7 @@ func removePods(namesOrIDs []string, rmOptions entities.PodRmOptions, printIDs b
 
 	responses, err := registry.ContainerEngine().PodRm(context.Background(), namesOrIDs, rmOptions)
 	if err != nil {
+		setExitCode(err)
 		return err
 	}
 
@@ -83,8 +87,19 @@ func removePods(namesOrIDs []string, rmOptions entities.PodRmOptions, printIDs b
 				fmt.Println(r.Id)
 			}
 		} else {
+			setExitCode(r.Err)
 			errs = append(errs, r.Err)
 		}
 	}
 	return errs.PrintErrors()
 }
+
+func setExitCode(err error) {
+	cause := errors.Cause(err)
+	switch {
+	case cause == define.ErrNoSuchPod:
+		registry.SetExitCode(1)
+	case strings.Contains(cause.Error(), define.ErrNoSuchPod.Error()):
+		registry.SetExitCode(1)
+	}
+}
diff --git a/cmd/podman/volumes/rm.go b/cmd/podman/volumes/rm.go
index 5b23eb5e6..4c960d4d5 100644
--- a/cmd/podman/volumes/rm.go
+++ b/cmd/podman/volumes/rm.go
@@ -3,9 +3,11 @@ package volumes
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/containers/podman/v2/cmd/podman/registry"
 	"github.com/containers/podman/v2/cmd/podman/utils"
+	"github.com/containers/podman/v2/libpod/define"
 	"github.com/containers/podman/v2/pkg/domain/entities"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -51,14 +53,30 @@ func rm(cmd *cobra.Command, args []string) error {
 	}
 	responses, err := registry.ContainerEngine().VolumeRm(context.Background(), args, rmOptions)
 	if err != nil {
+		setExitCode(err)
 		return err
 	}
 	for _, r := range responses {
 		if r.Err == nil {
 			fmt.Println(r.Id)
 		} else {
+			setExitCode(r.Err)
 			errs = append(errs, r.Err)
 		}
 	}
 	return errs.PrintErrors()
 }
+
+func setExitCode(err error) {
+	cause := errors.Cause(err)
+	switch {
+	case cause == define.ErrNoSuchVolume:
+		registry.SetExitCode(1)
+	case strings.Contains(cause.Error(), define.ErrNoSuchVolume.Error()):
+		registry.SetExitCode(1)
+	case cause == define.ErrVolumeBeingUsed:
+		registry.SetExitCode(2)
+	case strings.Contains(cause.Error(), define.ErrVolumeBeingUsed.Error()):
+		registry.SetExitCode(2)
+	}
+}
diff --git a/docs/source/markdown/podman-network-rm.1.md b/docs/source/markdown/podman-network-rm.1.md
index 9ce4d1cd8..616bb2514 100644
--- a/docs/source/markdown/podman-network-rm.1.md
+++ b/docs/source/markdown/podman-network-rm.1.md
@@ -31,6 +31,15 @@ Delete the `fred` network and all containers associated with the network.
 Deleted: fred
 ```
 
+## Exit Status
+  **0**   All specified networks removed
+
+  **1**   One of the specified networks did not exist, and no other failures
+
+  **2**   The network is in use by a container or a Pod
+
+  **125** The command fails for any other reason
+
 ## SEE ALSO
 podman(1), podman-network(1), podman-network-inspect(1)
 
diff --git a/docs/source/markdown/podman-pod-rm.1.md b/docs/source/markdown/podman-pod-rm.1.md
index 95e7ab002..dd89694ec 100644
--- a/docs/source/markdown/podman-pod-rm.1.md
+++ b/docs/source/markdown/podman-pod-rm.1.md
@@ -49,6 +49,15 @@ podman pod rm -fa
 
 podman pod rm --pod-id-file /path/to/id/file
 
+## Exit Status
+  **0**   All specified pods removed
+
+  **1**   One of the specified pods did not exist, and no other failures
+
+  **2**   One of the specified pods is attached to a container
+
+  **125** The command fails for any other reason
+
 ## SEE ALSO
 podman-pod(1)
 
diff --git a/docs/source/markdown/podman-rm.1.md b/docs/source/markdown/podman-rm.1.md
index 990af0cd1..e3e6740df 100644
--- a/docs/source/markdown/podman-rm.1.md
+++ b/docs/source/markdown/podman-rm.1.md
@@ -93,7 +93,7 @@ $ podman rm -f --latest
 
   **2**   One of the specified containers is paused or running
 
-  **125** The command fails for a reason other than container did not exist or is paused/running
+  **125** The command fails for any other reason
 
 ## SEE ALSO
 podman(1), podman-image-rm(1), podman-ps(1), podman-build(1)
diff --git a/docs/source/markdown/podman-rmi.1.md b/docs/source/markdown/podman-rmi.1.md
index 58280e831..27fe3b235 100644
--- a/docs/source/markdown/podman-rmi.1.md
+++ b/docs/source/markdown/podman-rmi.1.md
@@ -47,7 +47,7 @@ $ podman rmi -a -f
 
   **2**   One of the specified images has child images or is being used by a container
 
-  **125** The command fails for a reason other than an image did not exist or is in use
+  **125** The command fails for any other reason
 
 ## SEE ALSO
 podman(1)
diff --git a/docs/source/markdown/podman-volume-rm.1.md b/docs/source/markdown/podman-volume-rm.1.md
index 9a2fe8c99..ed4a83f9e 100644
--- a/docs/source/markdown/podman-volume-rm.1.md
+++ b/docs/source/markdown/podman-volume-rm.1.md
@@ -39,6 +39,15 @@ $ podman volume rm --all
 $ podman volume rm --force myvol
 ```
 
+## Exit Status
+  **0**   All specified volumes removed
+
+  **1**   One of the specified volumes did not exist, and no other failures
+
+  **2**   One of the specified volumes is being used by a container
+
+  **125** The command fails for any other reason
+
 ## SEE ALSO
 podman-volume(1)
 
diff --git a/libpod/define/errors.go b/libpod/define/errors.go
index b3f6483d1..627928ef7 100644
--- a/libpod/define/errors.go
+++ b/libpod/define/errors.go
@@ -162,6 +162,9 @@ var (
 	// in a pod.  This cannot be done as the infra container has all the network information
 	ErrNetworkOnPodContainer = errors.New("network cannot be configured when it is shared with a pod")
 
+	// ErrNetworkInUse indicates the requested operation failed because the network was in use
+	ErrNetworkInUse = errors.New("network is being used")
+
 	// ErrStoreNotInitialized indicates that the container storage was never
 	// initialized.
 	ErrStoreNotInitialized = errors.New("the container storage was never initialized")
diff --git a/pkg/api/handlers/libpod/networks.go b/pkg/api/handlers/libpod/networks.go
index dfece2a4e..b3c4840b8 100644
--- a/pkg/api/handlers/libpod/networks.go
+++ b/pkg/api/handlers/libpod/networks.go
@@ -92,8 +92,8 @@ func RemoveNetwork(w http.ResponseWriter, r *http.Request) {
 	}
 	if reports[0].Err != nil {
 		// If the network cannot be found, we return a 404.
-		if errors.Cause(err) == define.ErrNoSuchNetwork {
-			utils.Error(w, "Something went wrong", http.StatusNotFound, err)
+		if errors.Cause(reports[0].Err) == define.ErrNoSuchNetwork {
+			utils.Error(w, "Something went wrong", http.StatusNotFound, reports[0].Err)
 			return
 		}
 	}
diff --git a/pkg/domain/infra/abi/network.go b/pkg/domain/infra/abi/network.go
index 053be6528..5acfea853 100644
--- a/pkg/domain/infra/abi/network.go
+++ b/pkg/domain/infra/abi/network.go
@@ -12,6 +12,7 @@ import (
 	"github.com/containernetworking/cni/libcni"
 	cniversion "github.com/containernetworking/cni/pkg/version"
 	"github.com/containers/podman/v2/libpod"
+	"github.com/containers/podman/v2/libpod/define"
 	"github.com/containers/podman/v2/pkg/domain/entities"
 	"github.com/containers/podman/v2/pkg/network"
 	"github.com/containers/podman/v2/pkg/util"
@@ -85,7 +86,7 @@ func (ic *ContainerEngine) NetworkRm(ctx context.Context, namesOrIds []string, o
 				// if user passes force, we nuke containers and pods
 				if !options.Force {
 					// Without the force option, we return an error
-					return reports, errors.Errorf("%q has associated containers with it. Use -f to forcibly delete containers and pods", name)
+					return reports, errors.Wrapf(define.ErrNetworkInUse, "%q has associated containers with it. Use -f to forcibly delete containers and pods", name)
 				}
 				if c.IsInfra() {
 					// if we have a infra container we need to remove the pod
diff --git a/test/e2e/network_test.go b/test/e2e/network_test.go
index 2ea8291fc..2beaf93dd 100644
--- a/test/e2e/network_test.go
+++ b/test/e2e/network_test.go
@@ -265,6 +265,12 @@ var _ = Describe("Podman network", func() {
 		Expect(rmAll.ExitCode()).To(BeZero())
 	})
 
+	It("podman network remove bogus", func() {
+		session := podmanTest.Podman([]string{"network", "rm", "bogus"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(1))
+	})
+
 	It("podman network remove --force with pod", func() {
 		netName := "testnet"
 		session := podmanTest.Podman([]string{"network", "create", netName})
@@ -280,6 +286,10 @@ var _ = Describe("Podman network", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(BeZero())
 
+		session = podmanTest.Podman([]string{"network", "rm", netName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(2))
+
 		session = podmanTest.Podman([]string{"network", "rm", "--force", netName})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(BeZero())
diff --git a/test/e2e/pod_rm_test.go b/test/e2e/pod_rm_test.go
index cb9b93a15..24643e6b2 100644
--- a/test/e2e/pod_rm_test.go
+++ b/test/e2e/pod_rm_test.go
@@ -195,8 +195,7 @@ var _ = Describe("Podman pod rm", func() {
 	It("podman rm bogus pod", func() {
 		session := podmanTest.Podman([]string{"pod", "rm", "bogus"})
 		session.WaitWithDefaultTimeout()
-		// TODO: `podman rm` returns 1 for a bogus container. Should the RC be consistent?
-		Expect(session.ExitCode()).To(Equal(125))
+		Expect(session.ExitCode()).To(Equal(1))
 	})
 
 	It("podman rm bogus pod and a running pod", func() {
@@ -209,11 +208,11 @@ var _ = Describe("Podman pod rm", func() {
 
 		session = podmanTest.Podman([]string{"pod", "rm", "bogus", "test1"})
 		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(125))
+		Expect(session.ExitCode()).To(Equal(1))
 
 		session = podmanTest.Podman([]string{"pod", "rm", "test1", "bogus"})
 		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(125))
+		Expect(session.ExitCode()).To(Equal(1))
 	})
 
 	It("podman rm --ignore bogus pod and a running pod", func() {
diff --git a/test/e2e/rm_test.go b/test/e2e/rm_test.go
index cc2f7daf1..7eff8c6ed 100644
--- a/test/e2e/rm_test.go
+++ b/test/e2e/rm_test.go
@@ -228,11 +228,11 @@ var _ = Describe("Podman rm", func() {
 
 		session = podmanTest.Podman([]string{"rm", "bogus", "test1"})
 		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(125))
+		Expect(session.ExitCode()).To(Equal(1))
 
 		session = podmanTest.Podman([]string{"rm", "test1", "bogus"})
 		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(125))
+		Expect(session.ExitCode()).To(Equal(1))
 	})
 
 	It("podman rm --ignore bogus container and a running container", func() {
diff --git a/test/e2e/volume_rm_test.go b/test/e2e/volume_rm_test.go
index a072bc824..cdced1f13 100644
--- a/test/e2e/volume_rm_test.go
+++ b/test/e2e/volume_rm_test.go
@@ -55,7 +55,7 @@ var _ = Describe("Podman volume rm", func() {
 
 		session = podmanTest.Podman([]string{"volume", "rm", "myvol"})
 		session.WaitWithDefaultTimeout()
-		Expect(session).To(ExitWithError())
+		Expect(session.ExitCode()).To(Equal(2))
 		Expect(session.ErrorToString()).To(ContainSubstring(cid))
 
 		session = podmanTest.Podman([]string{"volume", "rm", "-f", "myvol"})
@@ -70,6 +70,12 @@ var _ = Describe("Podman volume rm", func() {
 		podmanTest.Cleanup()
 	})
 
+	It("podman volume remove bogus", func() {
+		session := podmanTest.Podman([]string{"volume", "rm", "bogus"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(1))
+	})
+
 	It("podman rm with --all flag", func() {
 		session := podmanTest.Podman([]string{"volume", "create", "myvol"})
 		session.WaitWithDefaultTimeout()
diff --git a/test/system/500-networking.bats b/test/system/500-networking.bats
index d2454fbf4..150626ded 100644
--- a/test/system/500-networking.bats
+++ b/test/system/500-networking.bats
@@ -99,7 +99,7 @@ load helpers
        "Trying to create an already-existing network"
 
     run_podman network rm $mynetname
-    run_podman 125 network rm $mynetname
+    run_podman 1 network rm $mynetname
 
     # rootless CNI leaves behind an image pulled by SHA, hence with no tag.
     # Remove it if present; we can only remove it by ID.