Fix container errors not being sent via pod removal API

When I reworked pod removal to provide more detailed errors
(including per-container errors, not just a single multierror
with all errors squashed), I made it part of the struct returned
by the REST API and assumed that would be enough to get errors
through to clients. Unfortunately, in case of an overarching
error removing the pod (as any error with any container would
cause), we don't send the response struct that would include the
container errors - we just send a standardized REST error. We
could work around this with custom, potentially backwards
incompatible error handling for the REST pod delete endpoint, or
we could just do what was done before, and package up all the
errors in a multierror to send to the other side. Of those
options, the multierror seems far simpler.

Fixes #19159

Signed-off-by: Matt Heon <mheon@redhat.com>

pkg/api/handlers/libpod/pods.go

@@ -20,6 +20,7 @@ import (
 	"github.com/containers/podman/v4/pkg/specgenutil"
 	"github.com/containers/podman/v4/pkg/util"
 	"github.com/gorilla/schema"
+	"github.com/hashicorp/go-multierror"
 	"github.com/sirupsen/logrus"
 )
 
@@ -248,6 +249,19 @@ func PodDelete(w http.ResponseWriter, r *http.Request) {
 	}
 	ctrs, err := runtime.RemovePod(r.Context(), pod, true, query.Force, query.Timeout)
 	if err != nil {
+		if len(ctrs) > 0 {
+			// We have container errors to send as well.
+			// Since we're just writing an error, and we don't want
+			// special error-handling for just this endpoint: use a
+			// multierror to package up all container errors.
+			var allCtrErrors error
+			for _, ctrErr := range ctrs {
+				allCtrErrors = multierror.Append(allCtrErrors, ctrErr)
+			}
+
+			err = fmt.Errorf("%w. %s", err, allCtrErrors.Error())
+		}
+
 		utils.Error(w, http.StatusInternalServerError, err)
 		return
 	}

test/apiv2/40-pods.at

@@ -3,6 +3,9 @@
 # test pod-related endpoints
 #
 
+# Need to make 1 container
+podman pull $IMAGE &>/dev/null
+
 t GET  "libpod/pods/json (clean slate at start)"   200 '[]'
 
 t POST libpod/pods/create name=foo 201 .Id~[0-9a-f]\\{64\\}
@@ -28,11 +31,17 @@ t POST "libpod/pods/create (dup pod)" name=foo 409 \
 
 #t POST libpod/pods/create a=b 400 .cause='bad parameter'  # FIXME: unimplemented
 
+# Some tests require a single running container.
+# Don't bother saving CID, it will be removed as part of the pod.
+t POST libpod/containers/create?name=testctr Image=${IMAGE} Pod=foo Entrypoint='["top"]' 201 \
+  .Id~[0-9a-f]\\{64\\}
+cid=$(jq -r '.Id' <<<"$output")
+
 t POST libpod/pods/foo/start 200 \
   .Errs=null \
   .Id=$pod_id
 
-t POST libpod/pods/foo/start 304 \
+t POST libpod/pods/foo/start 304
 
 t POST libpod/pods/fakename/start 404 \
   .cause="no such pod" \
@@ -52,11 +61,17 @@ t POST "libpod/pods/fakename/unpause" 404\
   .cause="no such pod" \
   .message="no pod with name or ID fakename found: no such pod"
 
+t DELETE libpod/pods/foo?force=false 500 \
+  .cause="removing pod containers" \
+  .message~".*cannot remove container .* as it is running.*"
 
 t POST libpod/pods/foo/stop 200 \
   .Errs=null \
   .Id=$pod_id
 
+# Remove container as it is no longer required
+t DELETE libpod/containers/$cid 200 .[0].Id=$cid
+
 t POST "libpod/pods/foo/stop (pod is already stopped)" 304
 t POST "libpod/pods/fakename/stop" 404\
   .cause="no such pod" \
@@ -85,7 +100,7 @@ t POST "libpod/pods/bar/stop?t=invalid" 400 \
   .cause="schema: error converting value for \"t\"" \
   .message~"failed to parse parameters for"
 
-podman run -d --pod bar busybox sleep 999
+podman run -d --pod bar busybox top
 
 t POST libpod/pods/bar/stop?t=1 200 \
   .Errs=null \

test/system/200-pod.bats

@@ -55,6 +55,16 @@ function teardown() {
         is "$output" ".*0 \+1 \+0 \+[0-9. ?s]\+/pause" "there is a /pause container"
     fi
 
+    # Cannot remove pod while containers are still running. Error messages
+    # differ slightly between local and remote; these are the common elements.
+    run_podman 125 pod rm $podid
+    assert "${lines[0]}" =~ "Error: not all containers could be removed from pod $podid: removing pod containers.*" \
+           "pod rm while busy: error message line 1 of 3"
+    assert "${lines[1]}" =~ "cannot remove container .* as it is running - running or paused containers cannot be removed without force: container state improper" \
+           "pod rm while busy: error message line 2 of 3"
+    assert "${lines[2]}" =~ "cannot remove container .* as it is running - running or paused containers cannot be removed without force: container state improper" \
+           "pod rm while busy: error message line 3 of 3"
+
     # Clean up
     run_podman --noout pod rm -f -t 0 $podid
     is "$output" "" "output should be empty"