Fix podman unpause,pause,kill --all to work like podman stop --all

Currently if you execute podman unpause --all, podman pause --all Podman shows attempts to unpause containers that are not paused and prints an error. This PR catches this error and only prints errors if a paused container was not able to be unpaused. Currently if you execute podman pause --all or podman kill --all, Podman Podman shows attempts to pause or kill containers that are not running and prints an error. This PR catches this error and only prints errors if a running container was not able to be paused or killed. Also change printing of multiple errors to go to stderr and to prefix "Error: " in front to match the output of the last error. Fixes: containers#11098 Signed-off-by: Daniel J Walsh <[email protected]>
rhatdan · Aug 4, 2021 · 41f94a4 · 41f94a4
1 parent 77f8c65
commit 41f94a4
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 5 deletions.
diff --git a/cmd/podman/utils/error.go b/cmd/podman/utils/error.go
@@ -1,6 +1,9 @@
 package utils
 
-import "fmt"
+import (
+	"fmt"
+	"os"
+)
 
 type OutputErrors []error
 
@@ -10,7 +13,7 @@ func (o OutputErrors) PrintErrors() (lastError error) {
 	}
 	lastError = o[len(o)-1]
 	for e := 0; e < len(o)-1; e++ {
-		fmt.Println(o[e])
+		fmt.Fprintf(os.Stderr, "Error: %s\n", o[e])
 	}
 	return
 }
diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go
@@ -119,6 +119,10 @@ func (ic *ContainerEngine) ContainerPause(ctx context.Context, namesOrIds []stri
 	report := make([]*entities.PauseUnpauseReport, 0, len(ctrs))
 	for _, c := range ctrs {
 		err := c.Pause()
+		if err != nil && options.All && errors.Cause(err) == define.ErrCtrStateInvalid {
+			logrus.Debugf("Container %s is not running", c.ID())
+			continue
+		}
 		report = append(report, &entities.PauseUnpauseReport{Id: c.ID(), Err: err})
 	}
 	return report, nil
@@ -132,6 +136,10 @@ func (ic *ContainerEngine) ContainerUnpause(ctx context.Context, namesOrIds []st
 	report := make([]*entities.PauseUnpauseReport, 0, len(ctrs))
 	for _, c := range ctrs {
 		err := c.Unpause()
+		if err != nil && options.All && errors.Cause(err) == define.ErrCtrStateInvalid {
+			logrus.Debugf("Container %s is not paused", c.ID())
+			continue
+		}
 		report = append(report, &entities.PauseUnpauseReport{Id: c.ID(), Err: err})
 	}
 	return report, nil
@@ -220,9 +228,14 @@ func (ic *ContainerEngine) ContainerKill(ctx context.Context, namesOrIds []strin
 	}
 	reports := make([]*entities.KillReport, 0, len(ctrs))
 	for _, con := range ctrs {
+		err := con.Kill(uint(sig))
+		if options.All && errors.Cause(err) == define.ErrCtrStateInvalid {
+			logrus.Debugf("Container %s is not running", con.ID())
+			continue
+		}
 		reports = append(reports, &entities.KillReport{
 			Id:       con.ID(),
-			Err:      con.Kill(uint(sig)),
+			Err:      err,
 			RawInput: ctrMap[con.ID()],
 		})
 	}

diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go
@@ -63,19 +63,27 @@ func (ic *ContainerEngine) ContainerPause(ctx context.Context, namesOrIds []stri
 	reports := make([]*entities.PauseUnpauseReport, 0, len(ctrs))
 	for _, c := range ctrs {
 		err := containers.Pause(ic.ClientCtx, c.ID, nil)
+		if err != nil && options.All && errors.Cause(err).Error() == define.ErrCtrStateInvalid.Error() {
+			logrus.Debugf("Container %s is not running", c.ID)
+			continue
+		}
 		reports = append(reports, &entities.PauseUnpauseReport{Id: c.ID, Err: err})
 	}
 	return reports, nil
 }
 
 func (ic *ContainerEngine) ContainerUnpause(ctx context.Context, namesOrIds []string, options entities.PauseUnPauseOptions) ([]*entities.PauseUnpauseReport, error) {
+	reports := []*entities.PauseUnpauseReport{}
 	ctrs, err := getContainersByContext(ic.ClientCtx, options.All, false, namesOrIds)
 	if err != nil {
 		return nil, err
 	}
-	reports := make([]*entities.PauseUnpauseReport, 0, len(ctrs))
 	for _, c := range ctrs {
 		err := containers.Unpause(ic.ClientCtx, c.ID, nil)
+		if err != nil && options.All && errors.Cause(err).Error() == define.ErrCtrStateInvalid.Error() {
+			logrus.Debugf("Container %s is not paused", c.ID)
+			continue
+		}
 		reports = append(reports, &entities.PauseUnpauseReport{Id: c.ID, Err: err})
 	}
 	return reports, nil
@@ -136,9 +144,14 @@ func (ic *ContainerEngine) ContainerKill(ctx context.Context, namesOrIds []strin
 	options := new(containers.KillOptions).WithSignal(opts.Signal)
 	reports := make([]*entities.KillReport, 0, len(ctrs))
 	for _, c := range ctrs {
+		err := containers.Kill(ic.ClientCtx, c.ID, options)
+		if err != nil && opts.All && errors.Cause(err).Error() == define.ErrCtrStateInvalid.Error() {
+			logrus.Debugf("Container %s is not running", c.ID)
+			continue
+		}
 		reports = append(reports, &entities.KillReport{
 			Id:       c.ID,
-			Err:      containers.Kill(ic.ClientCtx, c.ID, options),
+			Err:      err,
 			RawInput: ctrMap[c.ID],
 		})
 	}

diff --git a/test/system/080-pause.bats b/test/system/080-pause.bats
@@ -57,4 +57,23 @@ load helpers
     run_podman 125 unpause $cname
 }
 
+@test "podman unpause --all" {
+    if is_rootless && ! is_cgroupsv2; then
+        skip "'podman pause' (rootless) only works with cgroups v2"
+    fi
+
+    cname=$(random_string 10)
+    run_podman create --name notrunning $IMAGE
+    run_podman run -d --name $cname $IMAGE sleep 100
+    cid="$output"
+    run_podman pause $cid
+    run_podman inspect --format '{{.State.Status}}' $cid
+    is "$output" "paused" "podman inspect .State.Status"
+    run_podman unpause --all
+    is "$output" "$cid" "podman unpause output"
+    run_podman ps --format '{{.ID}} {{.Names}} {{.Status}}'
+    is "$output" "${cid:0:12} $cname Up.*" "podman ps on resumed container"
+    run_podman rm -f $cname
+    run_podman rm -f notrunning
+}
 # vim: filetype=sh