Merge pull request #9833 from rhatdan/resize

Remove resize race condition
containers · Mar 27, 2021 · 4d0b583 · 4d0b583
2 parents f3024b9 + dcabf6d
commit 4d0b583
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 13 deletions.
diff --git a/pkg/api/handlers/compat/resize.go b/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.

diff --git a/pkg/api/server/register_containers.go b/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:

diff --git a/pkg/api/server/register_exec.go b/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:

diff --git a/pkg/bindings/containers/attach.go b/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

diff --git a/pkg/bindings/containers/types.go b/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

diff --git a/pkg/bindings/containers/types_resizetty_options.go b/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
+}
diff --git a/test/system/450-interactive.bats b/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"
 }