Merge pull request #10549 from Luap99/fix-9859

remote: always send resize before the container starts

pkg/api/handlers/compat/resize.go

@@ -46,20 +46,13 @@ func ResizeTTY(w http.ResponseWriter, r *http.Request) {
 			utils.ContainerNotFound(w, name, err)
 			return
 		}
-		if state, err := ctnr.State(); err != nil {
-			utils.InternalServerError(w, errors.Wrapf(err, "cannot obtain container state"))
-			return
-		} 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 {
-			if errors.Cause(err) != define.ErrCtrStateInvalid || !query.IgnoreNotRunning {
+			if errors.Cause(err) != define.ErrCtrStateInvalid {
 				utils.InternalServerError(w, errors.Wrapf(err, "cannot resize container"))
-				return
+			} else {
+				utils.Error(w, "Container not running", http.StatusConflict, err)
 			}
+			return
 		}
 		// This is not a 204, even though we write nothing, for compatibility
 		// reasons.

pkg/api/server/register_containers.go

@@ -1364,6 +1364,8 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error {
 	//     $ref: "#/responses/ok"
 	//   404:
 	//     $ref: "#/responses/NoSuchContainer"
+	//   409:
+	//     $ref: "#/responses/ConflictError"
 	//   500:
 	//     $ref: "#/responses/InternalError"
 	r.HandleFunc(VersionedPath("/libpod/containers/{name}/resize"), s.APIHandler(compat.ResizeTTY)).Methods(http.MethodPost)

pkg/bindings/containers/attach.go

@@ -138,7 +138,7 @@ func Attach(ctx context.Context, nameOrID string, stdin io.Reader, stdout io.Wri
 		winCtx, winCancel := context.WithCancel(ctx)
 		defer winCancel()
 
-		go attachHandleResize(ctx, winCtx, winChange, false, nameOrID, file)
+		attachHandleResize(ctx, winCtx, winChange, false, nameOrID, file)
 	}
 
 	// If we are attaching around a start, we need to "signal"
@@ -327,32 +327,38 @@ func (f *rawFormatter) Format(entry *logrus.Entry) ([]byte, error) {
 	return append(buffer, '\r'), nil
 }
 
-// This is intended to be run as a goroutine, handling resizing for a container
-// or exec session.
+// This is intended to not be run as a goroutine, handling resizing for a container
+// or exec session. It will call resize once and then starts a goroutine which calls resize on winChange
 func attachHandleResize(ctx, winCtx context.Context, winChange chan os.Signal, isExec bool, id string, file *os.File) {
-	// Prime the pump, we need one reset to ensure everything is ready
-	winChange <- sig.SIGWINCH
-	for {
-		select {
-		case <-winCtx.Done():
-			return
-		case <-winChange:
-			w, h, err := terminal.GetSize(int(file.Fd()))
-			if err != nil {
-				logrus.Warnf("failed to obtain TTY size: %v", err)
-			}
+	resize := func() {
+		w, h, err := terminal.GetSize(int(file.Fd()))
+		if err != nil {
+			logrus.Warnf("failed to obtain TTY size: %v", err)
+		}
 
-			var resizeErr error
-			if isExec {
-				resizeErr = ResizeExecTTY(ctx, id, new(ResizeExecTTYOptions).WithHeight(h).WithWidth(w))
-			} else {
-				resizeErr = ResizeContainerTTY(ctx, id, new(ResizeTTYOptions).WithHeight(h).WithWidth(w))
-			}
-			if resizeErr != nil {
-				logrus.Warnf("failed to resize TTY: %v", resizeErr)
-			}
+		var resizeErr error
+		if isExec {
+			resizeErr = ResizeExecTTY(ctx, id, new(ResizeExecTTYOptions).WithHeight(h).WithWidth(w))
+		} else {
+			resizeErr = ResizeContainerTTY(ctx, id, new(ResizeTTYOptions).WithHeight(h).WithWidth(w))
+		}
+		if resizeErr != nil {
+			logrus.Warnf("failed to resize TTY: %v", resizeErr)
 		}
 	}
+
+	resize()
+
+	go func() {
+		for {
+			select {
+			case <-winCtx.Done():
+				return
+			case <-winChange:
+				resize()
+			}
+		}
+	}()
 }
 
 // Configure the given terminal for raw mode
@@ -457,7 +463,7 @@ func ExecStartAndAttach(ctx context.Context, sessionID string, options *ExecStar
 		winCtx, winCancel := context.WithCancel(ctx)
 		defer winCancel()
 
-		go attachHandleResize(ctx, winCtx, winChange, true, sessionID, terminalFile)
+		attachHandleResize(ctx, winCtx, winChange, true, sessionID, terminalFile)
 	}
 
 	if options.GetAttachInput() {

test/apiv2/python/rest_api/fixtures/api_testcase.py

@@ -49,7 +49,7 @@ class APITestCase(unittest.TestCase):
 
     def setUp(self):
         super().setUp()
-        APITestCase.podman.run("run", "alpine", "/bin/ls", check=True)
+        APITestCase.podman.run("run", "-d", "alpine", "top", check=True)
 
     def tearDown(self) -> None:
         APITestCase.podman.run("pod", "rm", "--all", "--force", check=True)

test/apiv2/python/rest_api/test_v2_0_0_container.py

@@ -12,7 +12,7 @@ class ContainerTestCase(APITestCase):
         r = requests.get(self.uri("/containers/json"), timeout=5)
         self.assertEqual(r.status_code, 200, r.text)
         obj = r.json()
-        self.assertEqual(len(obj), 0)
+        self.assertEqual(len(obj), 1)
 
     def test_list_all(self):
         r = requests.get(self.uri("/containers/json?all=true"))
@@ -36,7 +36,7 @@ class ContainerTestCase(APITestCase):
             self.assertId(r.content)
 
     def test_delete(self):
-        r = requests.delete(self.uri(self.resolve_container("/containers/{}")))
+        r = requests.delete(self.uri(self.resolve_container("/containers/{}?force=true")))
         self.assertEqual(r.status_code, 204, r.text)
 
     def test_stop(self):

test/system/450-interactive.bats

@@ -56,8 +56,7 @@ function teardown() {
     stty rows $rows cols $cols <$PODMAN_TEST_PTY
 
     # ...and make sure stty under podman reads that.
-    # FIXME: 'sleep 1' is needed for podman-remote; without it, there's
-    run_podman run -it --name mystty $IMAGE sh -c 'sleep 1;stty size' <$PODMAN_TEST_PTY
+    run_podman run -it --name mystty $IMAGE stty size <$PODMAN_TEST_PTY
     is "$output" "$rows $cols" "stty under podman reads the correct dimensions"
 }