Remove resize race condition

Since podman-remote resize requests can come in at random times, this
generates a real potential for race conditions. We should only be
attempting to resize TTY on running containers, but the containers can
go from running to stopped at any time, and returning an error to the
caller is just causing noice.

This change will basically ignore requests to resize terminals if the
container is not running and return the caller to success.  All other
callers will still return failure.

Fixes: #9831

Signed-off-by: Daniel J Walsh <[email protected]>

pkg/api/handlers/compat/resize.go

@@ -19,8 +19,9 @@ func ResizeTTY(w http.ResponseWriter, r *http.Request) {
 
 	// /containers/{id}/resize
 	query := struct {
-		Height uint16 `schema:"h"`
-		Width  uint16 `schema:"w"`
+		Height           uint16 `schema:"h"`
+		Width            uint16 `schema:"w"`
+		IgnoreNotRunning bool   `schema:"running"`
 	}{
 		// override any golang type defaults
 	}
@@ -48,14 +49,17 @@ func ResizeTTY(w http.ResponseWriter, r *http.Request) {
 		if state, err := ctnr.State(); err != nil {
 			utils.InternalServerError(w, errors.Wrapf(err, "cannot obtain container state"))
 			return
-		} else if state != define.ContainerStateRunning {
+		} else if state != define.ContainerStateRunning && !query.IgnoreNotRunning {
 			utils.Error(w, "Container not running", http.StatusConflict,
 				fmt.Errorf("container %q in wrong state %q", name, state.String()))
 			return
 		}
+		// If container is not running, ignore since this can be a race condition, and is expected
 		if err := ctnr.AttachResize(sz); err != nil {
-			utils.InternalServerError(w, errors.Wrapf(err, "cannot resize container"))
-			return
+			if errors.Cause(err) != define.ErrCtrStateInvalid || !query.IgnoreNotRunning {
+				utils.InternalServerError(w, errors.Wrapf(err, "cannot resize container"))
+				return
+			}
 		}
 		// This is not a 204, even though we write nothing, for compatibility
 		// reasons.
@@ -70,14 +74,16 @@ func ResizeTTY(w http.ResponseWriter, r *http.Request) {
 		if state, err := ctnr.State(); err != nil {
 			utils.InternalServerError(w, errors.Wrapf(err, "cannot obtain session container state"))
 			return
-		} else if state != define.ContainerStateRunning {
+		} else if state != define.ContainerStateRunning && !query.IgnoreNotRunning {
 			utils.Error(w, "Container not running", http.StatusConflict,
 				fmt.Errorf("container %q in wrong state %q", name, state.String()))
 			return
 		}
 		if err := ctnr.ExecResize(name, sz); err != nil {
-			utils.InternalServerError(w, errors.Wrapf(err, "cannot resize session"))
-			return
+			if errors.Cause(err) != define.ErrCtrStateInvalid || !query.IgnoreNotRunning {
+				utils.InternalServerError(w, errors.Wrapf(err, "cannot resize session"))
+				return
+			}
 		}
 		// This is not a 204, even though we write nothing, for compatibility
 		// reasons.

pkg/api/server/register_containers.go

@@ -587,6 +587,11 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error {
 	//    type: integer
 	//    required: false
 	//    description: Width to set for the terminal, in characters
+	//  - in: query
+	//    name: running
+	//    type: boolean
+	//    required: false
+	//    description: Ignore containers not running errors
 	// produces:
 	// - application/json
 	// responses:

pkg/api/server/register_exec.go

@@ -136,6 +136,11 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error {
 	//    name: w
 	//    type: integer
 	//    description: Width of the TTY session in characters
+	//  - in: query
+	//    name: running
+	//    type: boolean
+	//    required: false
+	//    description: Ignore containers not running errors
 	// produces:
 	// - application/json
 	// responses:

pkg/bindings/containers/attach.go

@@ -307,6 +307,7 @@ func resizeTTY(ctx context.Context, endpoint string, height *int, width *int) er
 	if width != nil {
 		params.Set("w", strconv.Itoa(*width))
 	}
+	params.Set("running", "true")
 	rsp, err := conn.DoRequest(nil, http.MethodPost, endpoint, params, nil)
 	if err != nil {
 		return err

pkg/bindings/containers/types.go

@@ -210,8 +210,9 @@ type RenameOptions struct {
 // ResizeTTYOptions are optional options for resizing
 // container TTYs
 type ResizeTTYOptions struct {
-	Height *int
-	Width  *int
+	Height  *int
+	Width   *int
+	Running *bool
 }
 
 //go:generate go run ../generator/generator.go ResizeExecTTYOptions

pkg/bindings/containers/types_resizetty_options.go

@@ -51,3 +51,19 @@ func (o *ResizeTTYOptions) GetWidth() int {
 	}
 	return *o.Width
 }
+
+// WithRunning
+func (o *ResizeTTYOptions) WithRunning(value bool) *ResizeTTYOptions {
+	v := &value
+	o.Running = v
+	return o
+}
+
+// GetRunning
+func (o *ResizeTTYOptions) GetRunning() bool {
+	var running bool
+	if o.Running == nil {
+		return running
+	}
+	return *o.Running
+}

test/system/450-interactive.bats

@@ -57,9 +57,6 @@ function teardown() {
 
     # ...and make sure stty under podman reads that.
     # FIXME: 'sleep 1' is needed for podman-remote; without it, there's
-    # a race condition resulting in the following warning:
-    #   WARN[0000] failed to resize TTY: container "xx" in wrong state "stopped"
-    # (also "created")
     run_podman run -it --name mystty $IMAGE sh -c 'sleep 1;stty size' <$PODMAN_TEST_PTY
     is "$output" "$rows $cols" "stty under podman reads the correct dimensions"
 }