From d0505d6bac30267296295e47268006893f092ae4 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 23 Jun 2023 16:48:57 +0200 Subject: [PATCH] pkg/api: top return error to client Wait before sending status code 200 for the first top call and if that fails return a proper error code. This was leading to some confusion in [1] because podman just reported 200 but did not wirte anything back. [1] https://bugzilla.redhat.com/show_bug.cgi?id=2215572 Signed-off-by: Paul Holzinger --- pkg/api/handlers/compat/containers_top.go | 20 +++++++++++++------- test/apiv2/20-containers.at | 9 +++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/pkg/api/handlers/compat/containers_top.go b/pkg/api/handlers/compat/containers_top.go index 69e4160430..8da18ae446 100644 --- a/pkg/api/handlers/compat/containers_top.go +++ b/pkg/api/handlers/compat/containers_top.go @@ -48,12 +48,8 @@ func TopContainer(w http.ResponseWriter, r *http.Request) { return } - // We are committed now - all errors logged but not reported to client, ship has sailed - w.WriteHeader(http.StatusOK) + statusWritten := false w.Header().Set("Content-Type", "application/json") - if f, ok := w.(http.Flusher); ok { - f.Flush() - } encoder := json.NewEncoder(w) @@ -65,7 +61,11 @@ loop: // break out of for/select infinite` loop default: output, err := c.Top(strings.Split(query.PsArgs, ",")) if err != nil { - logrus.Infof("Error from %s %q : %v", r.Method, r.URL, err) + if !statusWritten { + utils.InternalServerError(w, err) + } else { + logrus.Errorf("From %s %q : %v", r.Method, r.URL, err) + } break loop } @@ -86,9 +86,15 @@ loop: // break out of for/select infinite` loop } if err := encoder.Encode(body); err != nil { - logrus.Infof("Error from %s %q : %v", r.Method, r.URL, err) + if !statusWritten { + utils.InternalServerError(w, err) + } else { + logrus.Errorf("From %s %q : %v", r.Method, r.URL, err) + } break loop } + // after the first write we can no longer send a different status code + statusWritten = true if f, ok := w.(http.Flusher); ok { f.Flush() } diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at index 3a38976e73..af22997247 100644 --- a/test/apiv2/20-containers.at +++ b/test/apiv2/20-containers.at @@ -132,6 +132,15 @@ if root; then podman rm -f $CTRNAME fi +CTRNAME=test123 +podman run --name $CTRNAME -d $IMAGE top +t GET libpod/containers/$CTRNAME/top?ps_args=--invalid 500 \ + .cause~".*unrecognized option.*" +t GET containers/$CTRNAME/top?ps_args=--invalid 500 \ + .cause~".*unrecognized option.*" + +podman rm -f $CTRNAME + # Issue #15765: make sure the memory limit is capped if root; then CTRNAME=ctr-with-limit