From 5370d9cb76a8075332f5ab6a0efef9fba28ba19b Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Sat, 15 Jun 2019 06:24:42 -0400 Subject: [PATCH] Add new exit codes to rm & rmi for running containers & dependencies This enables programs and scripts wrapping the podman command to handle 'podman rm' and 'podman rmi' failures caused by paused or running containers or due to images having other child images or dependent containers. These errors are common enough that it makes sense to have a more machine readable way of detecting them than parsing the standard error output. Signed-off-by: Ondrej Zoder Signed-off-by: Daniel J Walsh --- cmd/podman/errors.go | 13 +++++++++++++ cmd/podman/errors_remote.go | 20 ++++++++++++++++++++ cmd/podman/rm.go | 13 +++++-------- cmd/podman/rmi.go | 1 + docs/podman-rm.1.md | 3 ++- docs/podman-rmi.1.md | 3 ++- pkg/varlinkapi/containers.go | 6 ++++++ test/e2e/checkpoint_test.go | 2 +- test/e2e/pause_test.go | 4 ++-- test/e2e/rm_test.go | 2 +- test/e2e/rmi_test.go | 2 +- 11 files changed, 54 insertions(+), 15 deletions(-) diff --git a/cmd/podman/errors.go b/cmd/podman/errors.go index 9731037f4a..ae9e73e62f 100644 --- a/cmd/podman/errors.go +++ b/cmd/podman/errors.go @@ -8,6 +8,8 @@ import ( "os/exec" "syscall" + "github.com/containers/libpod/libpod/define" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -24,3 +26,14 @@ func outputError(err error) { fmt.Fprintln(os.Stderr, "Error:", err.Error()) } } + +func setExitCode(err error) int { + cause := errors.Cause(err) + switch cause { + case define.ErrNoSuchCtr: + return 1 + case define.ErrCtrStateInvalid: + return 2 + } + return exitCode +} diff --git a/cmd/podman/errors_remote.go b/cmd/podman/errors_remote.go index 1e276be10c..19df2d2d86 100644 --- a/cmd/podman/errors_remote.go +++ b/cmd/podman/errors_remote.go @@ -9,6 +9,7 @@ import ( "syscall" "github.com/containers/libpod/cmd/podman/varlink" + "github.com/containers/libpod/libpod/define" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -43,3 +44,22 @@ func outputError(err error) { fmt.Fprintln(os.Stderr, "Error:", ne.Error()) } } + +func setExitCode(err error) int { + cause := errors.Cause(err) + switch e := cause.(type) { + // For some reason golang wont let me list them with commas so listing them all. + case *iopodman.ContainerNotFound: + return 1 + case *iopodman.InvalidState: + return 2 + default: + switch e { + case define.ErrNoSuchCtr: + return 1 + case define.ErrCtrStateInvalid: + return 2 + } + } + return exitCode +} diff --git a/cmd/podman/rm.go b/cmd/podman/rm.go index 958ca1c60c..9e3ce4d0ba 100644 --- a/cmd/podman/rm.go +++ b/cmd/podman/rm.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/containers/libpod/cmd/podman/cliconfig" - "github.com/containers/libpod/libpod/define" "github.com/containers/libpod/pkg/adapter" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -65,18 +64,16 @@ func rmCmd(c *cliconfig.RmValues) error { ok, failures, err := runtime.RemoveContainers(getContext(), c) if err != nil { - if errors.Cause(err) == define.ErrNoSuchCtr { - if len(c.InputArgs) > 1 { - exitCode = 125 - } else { - exitCode = 1 - } + if len(c.InputArgs) < 2 { + exitCode = setExitCode(err) } return err } if len(failures) > 0 { - exitCode = 125 + for _, err := range failures { + exitCode = setExitCode(err) + } } return printCmdResults(ok, failures) diff --git a/cmd/podman/rmi.go b/cmd/podman/rmi.go index 57e78c34ae..3f621116e4 100644 --- a/cmd/podman/rmi.go +++ b/cmd/podman/rmi.go @@ -74,6 +74,7 @@ func rmiCmd(c *cliconfig.RmiValues) error { 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 !adapter.IsImageNotFound(err) { + exitCode = 2 failureCnt++ } if lastError != nil { diff --git a/docs/podman-rm.1.md b/docs/podman-rm.1.md index 32a8c29432..7f39c09ad7 100644 --- a/docs/podman-rm.1.md +++ b/docs/podman-rm.1.md @@ -69,7 +69,8 @@ podman rm -f --latest ## Exit Status **_0_** if all specified containers removed **_1_** if one of the specified containers did not exist, and no other failures -**_125_** if command fails for a reason other then an container did not exist +**_2_** if one of the specified containers is paused or running +**_125_** if the command fails for a reason other than container did not exist or is paused/running ## SEE ALSO podman(1), podman-image-rm(1) diff --git a/docs/podman-rmi.1.md b/docs/podman-rmi.1.md index 2cba8a22d0..6b242c94eb 100644 --- a/docs/podman-rmi.1.md +++ b/docs/podman-rmi.1.md @@ -43,7 +43,8 @@ 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 +**_2_** if one of the specified images has child images or is being used by a container +**_125_** if the command fails for a reason other than an image did not exist or is in use ## SEE ALSO podman(1) diff --git a/pkg/varlinkapi/containers.go b/pkg/varlinkapi/containers.go index cd5f305c9b..bb66ff9624 100644 --- a/pkg/varlinkapi/containers.go +++ b/pkg/varlinkapi/containers.go @@ -488,6 +488,12 @@ func (i *LibpodAPI) RemoveContainer(call iopodman.VarlinkCall, name string, forc return call.ReplyContainerNotFound(name, err.Error()) } if err := i.Runtime.RemoveContainer(ctx, ctr, force, removeVolumes); err != nil { + if errors.Cause(err) == define.ErrNoSuchCtr { + return call.ReplyContainerExists(1) + } + if errors.Cause(err) == define.ErrCtrStateInvalid { + return call.ReplyInvalidState(ctr.ID(), err.Error()) + } return call.ReplyErrorOccurred(err.Error()) } return call.ReplyRemoveContainer(ctr.ID()) diff --git a/test/e2e/checkpoint_test.go b/test/e2e/checkpoint_test.go index 8f3cf5c109..0261acbd9f 100644 --- a/test/e2e/checkpoint_test.go +++ b/test/e2e/checkpoint_test.go @@ -147,7 +147,7 @@ var _ = Describe("Podman checkpoint", func() { result = podmanTest.Podman([]string{"rm", cid}) result.WaitWithDefaultTimeout() - Expect(result.ExitCode()).To(Equal(125)) + Expect(result.ExitCode()).To(Equal(2)) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1)) result = podmanTest.Podman([]string{"rm", "-f", cid}) diff --git a/test/e2e/pause_test.go b/test/e2e/pause_test.go index 01fb6d91b4..455f609379 100644 --- a/test/e2e/pause_test.go +++ b/test/e2e/pause_test.go @@ -126,7 +126,7 @@ var _ = Describe("Podman pause", func() { result = podmanTest.Podman([]string{"rm", cid}) result.WaitWithDefaultTimeout() - Expect(result.ExitCode()).To(Equal(125)) + Expect(result.ExitCode()).To(Equal(2)) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.GetContainerStatus()).To(ContainSubstring(pausedState)) @@ -179,7 +179,7 @@ var _ = Describe("Podman pause", func() { result = podmanTest.Podman([]string{"rm", cid}) result.WaitWithDefaultTimeout() - Expect(result.ExitCode()).To(Equal(125)) + Expect(result.ExitCode()).To(Equal(2)) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1)) result = podmanTest.Podman([]string{"rm", "-f", cid}) diff --git a/test/e2e/rm_test.go b/test/e2e/rm_test.go index 2dbabbf6ad..8ee7dc7500 100644 --- a/test/e2e/rm_test.go +++ b/test/e2e/rm_test.go @@ -49,7 +49,7 @@ var _ = Describe("Podman rm", func() { result := podmanTest.Podman([]string{"rm", cid}) result.WaitWithDefaultTimeout() - Expect(result.ExitCode()).To(Equal(125)) + Expect(result.ExitCode()).To(Equal(2)) }) It("podman rm created container", func() { diff --git a/test/e2e/rmi_test.go b/test/e2e/rmi_test.go index 1b0329a839..d4e2407ec0 100644 --- a/test/e2e/rmi_test.go +++ b/test/e2e/rmi_test.go @@ -145,7 +145,7 @@ var _ = Describe("Podman rmi", func() { session = podmanTest.PodmanNoCache([]string{"rmi", "-f", untaggedImg}) session.WaitWithDefaultTimeout() - Expect(session.ExitCode()).To(Not(Equal(0))) + Expect(session.ExitCode()).To(Equal(2)) }) It("podman rmi image that is created from another named imaged", func() {