Merge pull request #19188 from mheon/fix_19159

Fix container errors not being sent via pod removal API

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"