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>

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)

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)
+	}
+}

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)
+	}
+}

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)
+	}
+}

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)
 

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)
 

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)

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)

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)
 

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")

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
 		}
 	}

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

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())

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() {

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() {

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()

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.