Merge pull request #7825 from rhatdan/exitcode

Fix handling of remove of bogus volumes, networks and Pods

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 {
+		if errors.Cause(reports[0].Err) == define.ErrNoSuchNetwork {
-			utils.Error(w, "Something went wrong", http.StatusNotFound, err)
+			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(1))
-		Expect(session.ExitCode()).To(Equal(125))
 	})
 
 	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.