rm -f now removes a paused container

We now can remove a paused container by sending it a kill signal while it
is paused.  We then unpause the container and it is immediately killed.

Also, reworked how the parallelWorker results are handled to provide a
more consistent approach to how each subcommand implements it. It also
fixes a bug where if one container errors, the error message is duplicated
when printed out.

Signed-off-by: baude <bbaude@redhat.com>

cmd/podman/kill.go

@@ -43,7 +43,6 @@ var (
 // killCmd kills one or more containers with a signal
 func killCmd(c *cli.Context) error {
 	var (
-		lastError  error
 		killFuncs  []shared.ParallelWorkerInput
 		killSignal uint = uint(syscall.SIGTERM)
 	)
@@ -75,8 +74,12 @@ func killCmd(c *cli.Context) error {
 
 	containers, err := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "running")
 	if err != nil {
-		return err
+		if len(containers) == 0 {
+			return err
+		}
+		fmt.Println(err.Error())
 	}
+
 	for _, ctr := range containers {
 		con := ctr
 		f := func() error {
@@ -95,18 +98,6 @@ func killCmd(c *cli.Context) error {
 	}
 	logrus.Debugf("Setting maximum workers to %d", maxWorkers)
 
-	killErrors := shared.ParallelExecuteWorkerPool(maxWorkers, killFuncs)
+	killErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, killFuncs)
-
+	return printParallelOutput(killErrors, errCount)
-	for cid, result := range killErrors {
-		if result != nil {
-			if len(killErrors) > 1 {
-				fmt.Println(result.Error())
-			}
-			lastError = result
-			continue
-		}
-		fmt.Println(cid)
-	}
-
-	return lastError
 }

cmd/podman/pause.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"fmt"
 	"os"
 
 	"github.com/containers/libpod/cmd/podman/libpodruntime"
@@ -37,7 +36,6 @@ var (
 
 func pauseCmd(c *cli.Context) error {
 	var (
-		lastError       error
 		pauseContainers []*libpod.Container
 		pauseFuncs      []shared.ParallelWorkerInput
 	)
@@ -90,17 +88,6 @@ func pauseCmd(c *cli.Context) error {
 	}
 	logrus.Debugf("Setting maximum workers to %d", maxWorkers)
 
-	pauseErrors := shared.ParallelExecuteWorkerPool(maxWorkers, pauseFuncs)
+	pauseErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, pauseFuncs)
-
+	return printParallelOutput(pauseErrors, errCount)
-	for cid, result := range pauseErrors {
-		if result != nil {
-			if len(pauseErrors) > 1 {
-				fmt.Println(result.Error())
-			}
-			lastError = result
-			continue
-		}
-		fmt.Println(cid)
-	}
-	return lastError
 }

cmd/podman/restart.go

@@ -1,8 +1,6 @@
 package main
 
 import (
-	"fmt"
-
 	"github.com/containers/libpod/cmd/podman/libpodruntime"
 	"github.com/containers/libpod/cmd/podman/shared"
 	"github.com/containers/libpod/libpod"
@@ -46,7 +44,6 @@ func restartCmd(c *cli.Context) error {
 	var (
 		restartFuncs      []shared.ParallelWorkerInput
 		containers        []*libpod.Container
-		lastError         error
 		restartContainers []*libpod.Container
 	)
 
@@ -124,15 +121,6 @@ func restartCmd(c *cli.Context) error {
 
 	logrus.Debugf("Setting maximum workers to %d", maxWorkers)
 
-	restartErrors := shared.ParallelExecuteWorkerPool(maxWorkers, restartFuncs)
+	restartErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, restartFuncs)
-
+	return printParallelOutput(restartErrors, errCount)
-	for cid, result := range restartErrors {
-		if result != nil {
-			fmt.Println(result.Error())
-			lastError = result
-			continue
-		}
-		fmt.Println(cid)
-	}
-	return lastError
 }

cmd/podman/rm.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"github.com/containers/libpod/cmd/podman/libpodruntime"
 	"github.com/containers/libpod/cmd/podman/shared"
-	"github.com/containers/libpod/libpod"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
@@ -46,9 +45,7 @@ Running containers will not be removed without the -f option.
 // saveCmd saves the image to either docker-archive or oci
 func rmCmd(c *cli.Context) error {
 	var (
-		delContainers []*libpod.Container
+		deleteFuncs []shared.ParallelWorkerInput
-		lastError     error
-		deleteFuncs   []shared.ParallelWorkerInput
 	)
 
 	ctx := getContext()
@@ -65,7 +62,13 @@ func rmCmd(c *cli.Context) error {
 		return err
 	}
 
-	delContainers, lastError = getAllOrLatestContainers(c, runtime, -1, "all")
+	delContainers, err := getAllOrLatestContainers(c, runtime, -1, "all")
+	if err != nil {
+		if len(delContainers) == 0 {
+			return err
+		}
+		fmt.Println(err.Error())
+	}
 
 	for _, container := range delContainers {
 		con := container
@@ -84,14 +87,7 @@ func rmCmd(c *cli.Context) error {
 	}
 	logrus.Debugf("Setting maximum workers to %d", maxWorkers)
 
-	deleteErrors := shared.ParallelExecuteWorkerPool(maxWorkers, deleteFuncs)
+	// Run the parallel funcs
-	for cid, result := range deleteErrors {
+	deleteErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, deleteFuncs)
-		if result != nil {
+	return printParallelOutput(deleteErrors, errCount)
-			fmt.Println(result.Error())
-			lastError = result
-			continue
-		}
-		fmt.Println(cid)
-	}
-	return lastError
 }

cmd/podman/shared/parallel.go

@@ -30,9 +30,10 @@ func ParallelWorker(wg *sync.WaitGroup, jobs <-chan ParallelWorkerInput, results
 
 // ParallelExecuteWorkerPool takes container jobs and performs them in parallel.  The worker
 // int determines how many workers/threads should be premade.
-func ParallelExecuteWorkerPool(workers int, functions []ParallelWorkerInput) map[string]error {
+func ParallelExecuteWorkerPool(workers int, functions []ParallelWorkerInput) (map[string]error, int) {
 	var (
-		wg sync.WaitGroup
+		wg         sync.WaitGroup
+		errorCount int
 	)
 
 	resultChan := make(chan containerError, len(functions))
@@ -62,9 +63,12 @@ func ParallelExecuteWorkerPool(workers int, functions []ParallelWorkerInput) map
 	close(resultChan)
 	for ctrError := range resultChan {
 		results[ctrError.ContainerID] = ctrError.Err
+		if ctrError.Err != nil {
+			errorCount += 1
+		}
 	}
 
-	return results
+	return results, errorCount
 }
 
 // Parallelize provides the maximum number of parallel workers (int) as calculated by a basic

cmd/podman/stop.go

@@ -59,7 +59,13 @@ func stopCmd(c *cli.Context) error {
 	}
 	defer runtime.Shutdown(false)
 
-	containers, lastError := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "running")
+	containers, err := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "running")
+	if err != nil {
+		if len(containers) == 0 {
+			return err
+		}
+		fmt.Println(err.Error())
+	}
 
 	var stopFuncs []shared.ParallelWorkerInput
 	for _, ctr := range containers {
@@ -85,17 +91,6 @@ func stopCmd(c *cli.Context) error {
 	}
 	logrus.Debugf("Setting maximum workers to %d", maxWorkers)
 
-	stopErrors := shared.ParallelExecuteWorkerPool(maxWorkers, stopFuncs)
+	stopErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, stopFuncs)
-
+	return printParallelOutput(stopErrors, errCount)
-	for cid, result := range stopErrors {
-		if result != nil && result != libpod.ErrCtrStopped {
-			if len(stopErrors) > 1 {
-				fmt.Println(result.Error())
-			}
-			lastError = result
-			continue
-		}
-		fmt.Println(cid)
-	}
-	return lastError
 }

cmd/podman/unpause.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"fmt"
 	"os"
 
 	"github.com/containers/libpod/cmd/podman/libpodruntime"
@@ -37,7 +36,6 @@ var (
 
 func unpauseCmd(c *cli.Context) error {
 	var (
-		lastError         error
 		unpauseContainers []*libpod.Container
 		unpauseFuncs      []shared.ParallelWorkerInput
 	)
@@ -90,18 +88,6 @@ func unpauseCmd(c *cli.Context) error {
 	}
 	logrus.Debugf("Setting maximum workers to %d", maxWorkers)
 
-	unpauseErrors := shared.ParallelExecuteWorkerPool(maxWorkers, unpauseFuncs)
+	unpauseErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, unpauseFuncs)
-
+	return printParallelOutput(unpauseErrors, errCount)
-	for cid, result := range unpauseErrors {
-		if result != nil && result != libpod.ErrCtrStopped {
-			if len(unpauseErrors) > 1 {
-				fmt.Println(result.Error())
-			}
-			lastError = result
-			continue
-		}
-		fmt.Println(cid)
-	}
-
-	return lastError
 }

cmd/podman/utils.go

@@ -207,3 +207,20 @@ func getPodsFromContext(c *cli.Context, r *libpod.Runtime) ([]*libpod.Pod, error
 	}
 	return pods, lastError
 }
+
+//printParallelOutput takes the map of parallel worker results and outputs them
+// to stdout
+func printParallelOutput(m map[string]error, errCount int) error {
+	var lastError error
+	for cid, result := range m {
+		if result != nil {
+			if errCount > 1 {
+				fmt.Println(result.Error())
+			}
+			lastError = result
+			continue
+		}
+		fmt.Println(cid)
+	}
+	return lastError
+}

docs/podman-rm.1.md

@@ -13,7 +13,7 @@ podman\-rm - Remove one or more containers
 
 **--force, f**
 
-Force the removal of a running container
+Force the removal of a running and paused containers
 
 **--all, a**
 
@@ -29,16 +29,29 @@ to run containers such as CRI-O, the last started container could be from either
 Remove the volumes associated with the container. (Not yet implemented)
 
 ## EXAMPLE
-
+Remove a container by its name *mywebserver*
+```
 podman rm mywebserver
-
+```
+Remove several containers by name and container id.
+```
 podman rm mywebserver myflaskserver 860a4b23
+```
 
+Forcibly remove a container by container ID.
+```
 podman rm -f 860a4b23
+```
 
+Remove all containers regardless of its run state.
+```
 podman rm -f -a
+```
 
+Forcibly remove the latest container created.
+```
 podman rm -f --latest
+```
 
 ## SEE ALSO
 podman(1), podman-rmi(1)

libpod/runtime_ctr.go

@@ -246,7 +246,19 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool)
 	}
 
 	if c.state.State == ContainerStatePaused {
-		return errors.Wrapf(ErrCtrStateInvalid, "container %s is paused, cannot remove until unpaused", c.ID())
+		if !force {
+			return errors.Wrapf(ErrCtrStateInvalid, "container %s is paused, cannot remove until unpaused", c.ID())
+		}
+		if err := c.runtime.ociRuntime.killContainer(c, 9); err != nil {
+			return err
+		}
+		if err := c.unpause(); err != nil {
+			return err
+		}
+		// Need to update container state to make sure we know it's stopped
+		if err := c.waitForExitFileAndSync(); err != nil {
+			return err
+		}
 	}
 
 	// Check that the container's in a good state to be removed

test/e2e/pause_test.go

@@ -91,7 +91,7 @@ var _ = Describe("Podman pause", func() {
 
 	})
 
-	It("podman remove a paused container by id", func() {
+	It("podman remove a paused container by id without force", func() {
 		session := podmanTest.RunTopContainer("")
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
@@ -111,25 +111,26 @@ var _ = Describe("Podman pause", func() {
 		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
 		Expect(podmanTest.GetContainerStatus()).To(ContainSubstring(pausedState))
 
-		result = podmanTest.Podman([]string{"rm", "--force", cid})
+	})
+
+	It("podman remove a paused container by id with force", func() {
+		session := podmanTest.RunTopContainer("")
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		cid := session.OutputToString()
+
+		result := podmanTest.Podman([]string{"pause", cid})
 		result.WaitWithDefaultTimeout()
 
-		Expect(result.ExitCode()).To(Equal(125))
+		Expect(result.ExitCode()).To(Equal(0))
 		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
 		Expect(podmanTest.GetContainerStatus()).To(ContainSubstring(pausedState))
 
-		result = podmanTest.Podman([]string{"unpause", cid})
-		result.WaitWithDefaultTimeout()
-
-		Expect(result.ExitCode()).To(Equal(0))
-		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))
-
 		result = podmanTest.Podman([]string{"rm", "--force", cid})
 		result.WaitWithDefaultTimeout()
 
 		Expect(result.ExitCode()).To(Equal(0))
 		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
-
 	})
 
 	It("podman stop a paused container by id", func() {