Merge pull request #4540 from vrothberg/systemd-improvements

podman rm/stop: add --allow-missing flag

17 files changed, 200 insertions, 19 deletions

diff --git a/cmd/podman/cliconfig/config.go b/cmd/podman/cliconfig/config.go
index 745f49651..27d94f769 100644
--- a/cmd/podman/cliconfig/config.go
+++ b/cmd/podman/cliconfig/config.go
@@ -368,6 +368,7 @@ type PodRestartValues struct {
 type PodRmValues struct {
 	PodmanCommand
 	All    bool
+	Ignore bool
 	Force  bool
 	Latest bool
 }
@@ -389,6 +390,7 @@ type PodStatsValues struct {
 type PodStopValues struct {
 	PodmanCommand
 	All     bool
+	Ignore  bool
 	Latest  bool
 	Timeout uint
 }
@@ -484,6 +486,7 @@ type RmValues struct {
 	PodmanCommand
 	All      bool
 	Force    bool
+	Ignore   bool
 	Latest   bool
 	Storage  bool
 	Volumes  bool
@@ -561,6 +564,7 @@ type StatsValues struct {
 type StopValues struct {
 	PodmanCommand
 	All      bool
+	Ignore   bool
 	Latest   bool
 	Timeout  uint
 	CIDFiles []string
diff --git a/cmd/podman/pod_rm.go b/cmd/podman/pod_rm.go
index 86d6d2f27..fcf1d5bc7 100644
--- a/cmd/podman/pod_rm.go
+++ b/cmd/podman/pod_rm.go
@@ -41,7 +41,9 @@ func init() {
 	flags := podRmCommand.Flags()
 	flags.BoolVarP(&podRmCommand.All, "all", "a", false, "Remove all running pods")
 	flags.BoolVarP(&podRmCommand.Force, "force", "f", false, "Force removal of a running pod by first stopping all containers, then removing all containers in the pod.  The default is false")
+	flags.BoolVarP(&podRmCommand.Ignore, "ignore", "i", false, "Ignore errors when a specified pod is missing")
 	flags.BoolVarP(&podRmCommand.Latest, "latest", "l", false, "Remove the latest pod podman is aware of")
+	markFlagHiddenForRemoteClient("ignore", flags)
 	markFlagHiddenForRemoteClient("latest", flags)
 }
 
diff --git a/cmd/podman/pod_stop.go b/cmd/podman/pod_stop.go
index 579e4f1d3..7d3951ec4 100644
--- a/cmd/podman/pod_stop.go
+++ b/cmd/podman/pod_stop.go
@@ -41,8 +41,10 @@ func init() {
 	podStopCommand.SetUsageTemplate(UsageTemplate())
 	flags := podStopCommand.Flags()
 	flags.BoolVarP(&podStopCommand.All, "all", "a", false, "Stop all running pods")
+	flags.BoolVarP(&podStopCommand.Ignore, "ignore", "i", false, "Ignore errors when a specified pod is missing")
 	flags.BoolVarP(&podStopCommand.Latest, "latest", "l", false, "Stop the latest pod podman is aware of")
 	flags.UintVarP(&podStopCommand.Timeout, "timeout", "t", 0, "Seconds to wait for pod stop before killing the container")
+	markFlagHiddenForRemoteClient("ignore", flags)
 	markFlagHiddenForRemoteClient("latest", flags)
 }
 
diff --git a/cmd/podman/rm.go b/cmd/podman/rm.go
index b3bc8e1b9..e69565e95 100644
--- a/cmd/podman/rm.go
+++ b/cmd/podman/rm.go
@@ -40,14 +40,16 @@ func init() {
 	rmCommand.SetUsageTemplate(UsageTemplate())
 	flags := rmCommand.Flags()
 	flags.BoolVarP(&rmCommand.All, "all", "a", false, "Remove all containers")
+	flags.BoolVarP(&rmCommand.Ignore, "ignore", "i", false, "Ignore errors when a specified container is missing")
 	flags.BoolVarP(&rmCommand.Force, "force", "f", false, "Force removal of a running or unusable container.  The default is false")
 	flags.BoolVarP(&rmCommand.Latest, "latest", "l", false, "Act on the latest container podman is aware of")
 	flags.BoolVar(&rmCommand.Storage, "storage", false, "Remove container from storage library")
 	flags.BoolVarP(&rmCommand.Volumes, "volumes", "v", false, "Remove anonymous volumes associated with the container")
 	flags.StringArrayVarP(&rmCommand.CIDFiles, "cidfile", "", nil, "Read the container ID from the file")
-	markFlagHiddenForRemoteClient("storage", flags)
-	markFlagHiddenForRemoteClient("latest", flags)
+	markFlagHiddenForRemoteClient("ignore", flags)
 	markFlagHiddenForRemoteClient("cidfile", flags)
+	markFlagHiddenForRemoteClient("latest", flags)
+	markFlagHiddenForRemoteClient("storage", flags)
 }
 
 // rmCmd removes one or more containers
@@ -58,10 +60,10 @@ func rmCmd(c *cliconfig.RmValues) error {
 	}
 	defer runtime.DeferredShutdown(false)
 
-	// Storage conflicts with --all/--latest/--volumes
+	// Storage conflicts with --all/--latest/--volumes/--cidfile/--ignore
 	if c.Storage {
-		if c.All || c.Latest || c.Volumes || c.CIDFiles != nil {
-			return errors.Errorf("--storage conflicts with --volumes, --all, --latest and --cidfile")
+		if c.All || c.Ignore || c.Latest || c.Volumes || c.CIDFiles != nil {
+			return errors.Errorf("--storage conflicts with --volumes, --all, --latest, --ignore and --cidfile")
 		}
 	}
 
diff --git a/cmd/podman/stop.go b/cmd/podman/stop.go
index acecb298e..c62da80df 100644
--- a/cmd/podman/stop.go
+++ b/cmd/podman/stop.go
@@ -39,12 +39,14 @@ func init() {
 	stopCommand.SetUsageTemplate(UsageTemplate())
 	flags := stopCommand.Flags()
 	flags.BoolVarP(&stopCommand.All, "all", "a", false, "Stop all running containers")
+	flags.BoolVarP(&stopCommand.Ignore, "ignore", "i", false, "Ignore errors when a specified container is missing")
+	flags.StringArrayVarP(&stopCommand.CIDFiles, "cidfile", "", nil, "Read the container ID from the file")
 	flags.BoolVarP(&stopCommand.Latest, "latest", "l", false, "Act on the latest container podman is aware of")
 	flags.UintVar(&stopCommand.Timeout, "time", define.CtrRemoveTimeout, "Seconds to wait for stop before killing the container")
 	flags.UintVarP(&stopCommand.Timeout, "timeout", "t", define.CtrRemoveTimeout, "Seconds to wait for stop before killing the container")
-	flags.StringArrayVarP(&stopCommand.CIDFiles, "cidfile", "", nil, "Read the container ID from the file")
 	markFlagHiddenForRemoteClient("latest", flags)
 	markFlagHiddenForRemoteClient("cidfile", flags)
+	markFlagHiddenForRemoteClient("ignore", flags)
 }
 
 // stopCmd stops a container or containers
diff --git a/completions/bash/podman b/completions/bash/podman
index 98950799e..7b64c2a80 100644
--- a/completions/bash/podman
+++ b/completions/bash/podman
@@ -2155,6 +2155,8 @@ _podman_rm() {
 	-f
 	--help
 	-h
+	--ignore
+	-i
 	--latest
 	-l
 	--storage
@@ -2430,6 +2432,8 @@ _podman_stop() {
 	  --cidfile
 	  -h
 	  --help
+	  --ignore
+	  -i
 	  --latest
 	  -l
     "
@@ -2990,6 +2994,8 @@ _podman_pod_rm() {
       --all
       --help
       -h
+	  --ignore
+	  -i
       -f
       --force
       --latest
@@ -3040,6 +3046,8 @@ _podman_pod_stop() {
       -a
       --cleanup
       --help
+	  --ignore
+	  -i
       -h
       --latest
       -l
diff --git a/docs/source/markdown/podman-pod-rm.1.md b/docs/source/markdown/podman-pod-rm.1.md
index 6659534b4..aee582dc6 100644
--- a/docs/source/markdown/podman-pod-rm.1.md
+++ b/docs/source/markdown/podman-pod-rm.1.md
@@ -15,6 +15,12 @@ podman\-pod\-rm - Remove one or more pods
 
 Remove all pods.  Can be used in conjunction with \-f as well.
 
+**--ignore**, **-i**
+
+Ignore errors when specified pods are not in the container store.  A user might
+have decided to manually remove a pod which would lead to a failure during the
+ExecStop directive of a systemd service referencing that pod.
+
 **--latest**, **-l**
 
 Instead of providing the pod name or ID, remove the last created pod.
diff --git a/docs/source/markdown/podman-pod-stop.1.md b/docs/source/markdown/podman-pod-stop.1.md
index b3ce47d72..73c347cec 100644
--- a/docs/source/markdown/podman-pod-stop.1.md
+++ b/docs/source/markdown/podman-pod-stop.1.md
@@ -15,6 +15,12 @@ Stop containers in one or more pods.  You may use pod IDs or names as input.
 
 Stops all pods
 
+**--ignore**, **-i**
+
+Ignore errors when specified pods are not in the container store.  A user might
+have decided to manually remove a pod which would lead to a failure during the
+ExecStop directive of a systemd service referencing that pod.
+
 **--latest**, **-l**
 
 Instead of providing the pod name or ID, stop the last created pod.
diff --git a/docs/source/markdown/podman-rm.1.md b/docs/source/markdown/podman-rm.1.md
index 74831fef6..782feac6f 100644
--- a/docs/source/markdown/podman-rm.1.md
+++ b/docs/source/markdown/podman-rm.1.md
@@ -30,6 +30,12 @@ Containers could have been created by a different container engine.
 In addition, forcing can be used to remove unusable containers, e.g. containers
 whose OCI runtime has become unavailable.
 
+**--ignore**, **-i**
+
+Ignore errors when specified containers are not in the container store.  A user
+might have decided to manually remove a container which would lead to a failure
+during the ExecStop directive of a systemd service referencing that container.
+
 **--latest**, **-l**
 
 Instead of providing the container name or ID, use the last created container. If you use methods other than Podman
diff --git a/docs/source/markdown/podman-stop.1.md b/docs/source/markdown/podman-stop.1.md
index 5e8056e92..3b5f17057 100644
--- a/docs/source/markdown/podman-stop.1.md
+++ b/docs/source/markdown/podman-stop.1.md
@@ -25,6 +25,12 @@ Stop all running containers.  This does not include paused containers.
 
 Read container ID from the specified file and remove the container.  Can be specified multiple times.
 
+**--ignore**, **-i**
+
+Ignore errors when specified containers are not in the container store.  A user
+might have decided to manually remove a container which would lead to a failure
+during the ExecStop directive of a systemd service referencing that container.
+
 **--latest**, **-l**
 
 Instead of providing the container name or ID, use the last created container. If you use methods other than Podman
diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go
index 02da9ec8c..bc9554193 100644
--- a/pkg/adapter/containers.go
+++ b/pkg/adapter/containers.go
@@ -90,7 +90,7 @@ func (r *LocalRuntime) StopContainers(ctx context.Context, cli *cliconfig.StopVa
 	}
 
 	ctrs, err := shortcuts.GetContainersByContext(cli.All, cli.Latest, names, r.Runtime)
-	if err != nil {
+	if err != nil && !(cli.Ignore && errors.Cause(err) == define.ErrNoSuchCtr) {
 		return nil, nil, err
 	}
 
@@ -224,7 +224,7 @@ func (r *LocalRuntime) RemoveContainers(ctx context.Context, cli *cliconfig.RmVa
 	}
 
 	ctrs, err := shortcuts.GetContainersByContext(cli.All, cli.Latest, names, r.Runtime)
-	if err != nil {
+	if err != nil && !(cli.Ignore && errors.Cause(err) == define.ErrNoSuchCtr) {
 		// Failed to get containers. If force is specified, get the containers ID
 		// and evict them
 		if !cli.Force {
@@ -235,6 +235,10 @@ func (r *LocalRuntime) RemoveContainers(ctx context.Context, cli *cliconfig.RmVa
 			logrus.Debugf("Evicting container %q", ctr)
 			id, err := r.EvictContainer(ctx, ctr, cli.Volumes)
 			if err != nil {
+				if cli.Ignore && errors.Cause(err) == define.ErrNoSuchCtr {
+					logrus.Debugf("Ignoring error (--allow-missing): %v", err)
+					continue
+				}
 				failures[ctr] = errors.Wrapf(err, "Failed to evict container: %q", id)
 				continue
 			}
@@ -252,6 +256,10 @@ func (r *LocalRuntime) RemoveContainers(ctx context.Context, cli *cliconfig.RmVa
 			Fn: func() error {
 				err := r.RemoveContainer(ctx, c, cli.Force, cli.Volumes)
 				if err != nil {
+					if cli.Ignore && errors.Cause(err) == define.ErrNoSuchCtr {
+						logrus.Debugf("Ignoring error (--allow-missing): %v", err)
+						return nil
+					}
 					logrus.Debugf("Failed to remove container %s: %s", c.ID(), err.Error())
 				}
 				return err
diff --git a/pkg/adapter/pods.go b/pkg/adapter/pods.go
index d73a8b21b..e9f3d41a9 100644
--- a/pkg/adapter/pods.go
+++ b/pkg/adapter/pods.go
@@ -15,6 +15,7 @@ import (
 	"github.com/containers/libpod/cmd/podman/cliconfig"
 	"github.com/containers/libpod/cmd/podman/shared"
 	"github.com/containers/libpod/libpod"
+	"github.com/containers/libpod/libpod/define"
 	"github.com/containers/libpod/libpod/image"
 	"github.com/containers/libpod/pkg/adapter/shortcuts"
 	ann "github.com/containers/libpod/pkg/annotations"
@@ -94,7 +95,7 @@ func (r *LocalRuntime) RemovePods(ctx context.Context, cli *cliconfig.PodRmValue
 		podids []string
 	)
 	pods, err := shortcuts.GetPodsByContext(cli.All, cli.Latest, cli.InputArgs, r.Runtime)
-	if err != nil {
+	if err != nil && !(cli.Ignore && errors.Cause(err) == define.ErrNoSuchPod) {
 		errs = append(errs, err)
 		return nil, errs
 	}
@@ -151,7 +152,7 @@ func (r *LocalRuntime) StopPods(ctx context.Context, cli *cliconfig.PodStopValue
 		podids []string
 	)
 	pods, err := shortcuts.GetPodsByContext(cli.All, cli.Latest, cli.InputArgs, r.Runtime)
-	if err != nil {
+	if err != nil && !(cli.Ignore && errors.Cause(err) == define.ErrNoSuchPod) {
 		errs = append(errs, err)
 		return nil, errs
 	}
diff --git a/pkg/adapter/shortcuts/shortcuts.go b/pkg/adapter/shortcuts/shortcuts.go
index 3e4eff555..4f6cfd6a3 100644
--- a/pkg/adapter/shortcuts/shortcuts.go
+++ b/pkg/adapter/shortcuts/shortcuts.go
@@ -2,9 +2,11 @@ package shortcuts
 
 import (
 	"github.com/containers/libpod/libpod"
+	"github.com/sirupsen/logrus"
 )
 
-// GetPodsByContext gets pods whether all, latest, or a slice of names/ids
+// GetPodsByContext returns a slice of pods. Note that all, latest and pods are
+// mutually exclusive arguments.
 func GetPodsByContext(all, latest bool, pods []string, runtime *libpod.Runtime) ([]*libpod.Pod, error) {
 	var outpods []*libpod.Pod
 	if all {
@@ -18,17 +20,24 @@ func GetPodsByContext(all, latest bool, pods []string, runtime *libpod.Runtime)
 		outpods = append(outpods, p)
 		return outpods, nil
 	}
+	var err error
 	for _, p := range pods {
-		pod, err := runtime.LookupPod(p)
-		if err != nil {
-			return nil, err
+		pod, e := runtime.LookupPod(p)
+		if e != nil {
+			// Log all errors here, so callers don't need to.
+			logrus.Debugf("Error looking up pod %q: %v", p, e)
+			if err == nil {
+				err = e
+			}
+		} else {
+			outpods = append(outpods, pod)
 		}
-		outpods = append(outpods, pod)
 	}
-	return outpods, nil
+	return outpods, err
 }
 
 // GetContainersByContext gets pods whether all, latest, or a slice of names/ids
+// is specified.
 func GetContainersByContext(all, latest bool, names []string, runtime *libpod.Runtime) (ctrs []*libpod.Container, err error) {
 	var ctr *libpod.Container
 	ctrs = []*libpod.Container{}
@@ -41,10 +50,15 @@ func GetContainersByContext(all, latest bool, names []string, runtime *libpod.Ru
 	} else {
 		for _, n := range names {
 			ctr, e := runtime.LookupContainer(n)
-			if e != nil && err == nil {
-				err = e
+			if e != nil {
+				// Log all errors here, so callers don't need to.
+				logrus.Debugf("Error looking up container %q: %v", n, e)
+				if err == nil {
+					err = e
+				}
+			} else {
+				ctrs = append(ctrs, ctr)
 			}
-			ctrs = append(ctrs, ctr)
 		}
 	}
 	return
diff --git a/test/e2e/pod_rm_test.go b/test/e2e/pod_rm_test.go
index de68e885a..c0277ca0d 100644
--- a/test/e2e/pod_rm_test.go
+++ b/test/e2e/pod_rm_test.go
@@ -186,4 +186,47 @@ var _ = Describe("Podman pod rm", func() {
 		result.WaitWithDefaultTimeout()
 		Expect(result.OutputToString()).To(BeEmpty())
 	})
+
+	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))
+	})
+
+	It("podman rm bogus pod and a running pod", func() {
+		_, ec, podid1 := podmanTest.CreatePod("")
+		Expect(ec).To(Equal(0))
+
+		session := podmanTest.RunTopContainerInPod("test1", podid1)
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"pod", "rm", "bogus", "test1"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(125))
+
+		session = podmanTest.Podman([]string{"pod", "rm", "test1", "bogus"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(125))
+	})
+
+	It("podman rm --ignore bogus pod and a running pod", func() {
+		SkipIfRemote()
+
+		_, ec, podid1 := podmanTest.CreatePod("")
+		Expect(ec).To(Equal(0))
+
+		session := podmanTest.RunTopContainerInPod("test1", podid1)
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"pod", "rm", "--force", "--ignore", "bogus", "test1"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"pod", "rm", "--ignore", "test1", "bogus"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+	})
 })
diff --git a/test/e2e/pod_stop_test.go b/test/e2e/pod_stop_test.go
index 361a63a7f..a61917adb 100644
--- a/test/e2e/pod_stop_test.go
+++ b/test/e2e/pod_stop_test.go
@@ -38,6 +38,46 @@ var _ = Describe("Podman pod stop", func() {
 		Expect(session.ExitCode()).To(Equal(125))
 	})
 
+	It("podman pod stop --ignore bogus pod", func() {
+		SkipIfRemote()
+
+		session := podmanTest.Podman([]string{"pod", "stop", "--ignore", "123"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+	})
+
+	It("podman stop bogus pod and a running pod", func() {
+		_, ec, podid1 := podmanTest.CreatePod("")
+		Expect(ec).To(Equal(0))
+
+		session := podmanTest.RunTopContainerInPod("test1", podid1)
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"pod", "stop", "bogus", "test1"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(125))
+	})
+
+	It("podman stop --ignore bogus pod and a running pod", func() {
+		SkipIfRemote()
+
+		_, ec, podid1 := podmanTest.CreatePod("")
+		Expect(ec).To(Equal(0))
+
+		session := podmanTest.RunTopContainerInPod("test1", podid1)
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"pod", "stop", "--ignore", "bogus", "test1"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"pod", "stop", "--ignore", "test1", "bogus"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+	})
+
 	It("podman pod stop single empty pod", func() {
 		_, ec, podid := podmanTest.CreatePod("")
 		Expect(ec).To(Equal(0))
diff --git a/test/e2e/rm_test.go b/test/e2e/rm_test.go
index 531f14feb..4eb568879 100644
--- a/test/e2e/rm_test.go
+++ b/test/e2e/rm_test.go
@@ -232,4 +232,20 @@ var _ = Describe("Podman rm", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(125))
 	})
+
+	It("podman rm --ignore bogus container and a running container", func() {
+		SkipIfRemote()
+
+		session := podmanTest.RunTopContainer("test1")
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"rm", "--force", "--ignore", "bogus", "test1"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"rm", "--ignore", "test1", "bogus"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+	})
 })
diff --git a/test/e2e/stop_test.go b/test/e2e/stop_test.go
index c76cccfef..54c64d66b 100644
--- a/test/e2e/stop_test.go
+++ b/test/e2e/stop_test.go
@@ -40,6 +40,21 @@ var _ = Describe("Podman stop", func() {
 		Expect(session.ExitCode()).To(Equal(125))
 	})
 
+	It("podman stop --ignore bogus container", func() {
+		SkipIfRemote()
+
+		session := podmanTest.RunTopContainer("")
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		cid := session.OutputToString()
+
+		session = podmanTest.Podman([]string{"stop", "--ignore", "foobar", cid})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		output := session.OutputToString()
+		Expect(output).To(ContainSubstring(cid))
+	})
+
 	It("podman stop container by id", func() {
 		session := podmanTest.RunTopContainer("")
 		session.WaitWithDefaultTimeout()