OCPBUGS-4556: internal/oci: fix terminal resize race condition #9246

sohankunkerkar · 2025-06-04T16:42:06Z

This fixes the issue where container terminals were getting stuck at incorrect sizes due to rapid successive resize events not being handled properly. When 'oc debug' or 'kubectl exec' sends the typical resize sequence (inflate +1, then restore to original), the second event was being processed but not applied correctly, leaving the terminal at the wrong size.

Before this fix:

 $ kubectl debug node/192.168.1.16 -i -t --image=ubuntu -- sh
Creating debugging pod node-debugger-192.168.1.16-nqfmj with container debugger on node 192.168.1.16.
Warning: metadata.name: this is used in the Pod's hostname, which can result in surprising behavior; a DNS label is recommended: [must not contain dots]
If you don't see a command prompt, try pressing enter.

# tput cols ; tput lines
191
47
# exit

$ tput cols ; tput lines
190
46

After the fix:

$ kubectl debug node/192.168.1.16 -i -t --image=ubuntu -- sh
Creating debugging pod node-debugger-192.168.1.16-nqfmj with container debugger on node 192.168.1.16.
Warning: metadata.name: this is used in the Pod's hostname, which can result in surprising behavior; a DNS label is recommended: [must not contain dots]
If you don't see a command prompt, try pressing enter.

# tput cols ; tput lines
190
46
# exit

$ tput cols ; tput lines
190
46

What type of PR is this?

/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Fix terminal resize race condition

openshift-ci-robot · 2025-06-04T17:01:10Z

@sohankunkerkar: This pull request references Jira Issue OCPBUGS-4556, which is invalid:

expected the bug to target the "4.20.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

The code relied on the resize channel to provide the initial terminal size, but there's no guarantee that a resize event will be sent immediately upon container attachment. This created a race condition where container starts with default/incorrect terminal size. User's actual terminal size was never communicated to the container. Only subsequent resize events (if any) would correct the size. CRI-O should explicitly communicate terminal dimensions.

Before this fix:
$ kubectl debug node/192.168.1.16 -i -t --image=ubuntu -- sh
Creating debugging pod node-debugger-192.168.1.16-nqfmj with container debugger on node 192.168.1.16.
Warning: metadata.name: this is used in the Pod's hostname, which can result in surprising behavior; a DNS label is recommended: [must not contain dots]
If you don't see a command prompt, try pressing enter.

# tput cols ; tput lines
191
47
# exit

$ tput cols ; tput lines
190
46
After the fix:
$ kubectl debug node/192.168.1.16 -i -t --image=ubuntu -- sh
Creating debugging pod node-debugger-192.168.1.16-nqfmj with container debugger on node 192.168.1.16.
Warning: metadata.name: this is used in the Pod's hostname, which can result in surprising behavior; a DNS label is recommended: [must not contain dots]
If you don't see a command prompt, try pressing enter.

# tput cols ; tput lines
190
46
# exit

$ tput cols ; tput lines
190
46
What type of PR is this?

/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?
Fix initial terminal size while attaching a container

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

codecov · 2025-06-04T17:03:42Z

Codecov Report

Attention: Patch coverage is 33.33333% with 12 lines in your changes missing coverage. Please review.

Project coverage is 66.97%. Comparing base (b4ccbcd) to head (0d449e0).
Report is 14 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9246      +/-   ##
==========================================
+ Coverage   65.87%   66.97%   +1.09%     
==========================================
  Files         198      198              
  Lines       27171    27193      +22     
==========================================
+ Hits        17900    18212     +312     
+ Misses       7781     7483     -298     
- Partials     1490     1498       +8

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sohankunkerkar · 2025-06-04T17:04:46Z

/jira refresh

openshift-ci-robot · 2025-06-04T17:04:52Z

@sohankunkerkar: This pull request references Jira Issue OCPBUGS-4556, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.20.0) matches configured target version for branch (4.20.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @zhouying7780

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2025-06-04T17:04:55Z

@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: zhouying7780.

Note that only cri-o members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

@sohankunkerkar: This pull request references Jira Issue OCPBUGS-4556, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)

bug target version (4.20.0) matches configured target version for branch (4.20.0)

bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @zhouying7780

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

internal/oci/runtime_oci.go

sohankunkerkar · 2025-06-10T12:45:11Z

/retest

internal/oci/runtime_oci.go

sohankunkerkar · 2025-06-11T18:23:49Z

@cri-o/cri-o-maintainers PTAL

internal/oci/runtime_oci.go

kolyshkin

You can use golang.org/x/term functions IsTerminal and GetSize to make the code simpler.
I don't think querying stdin/stdout/stderr of the current process for terminal size is a good idea, since the current process is a cri-o daemon which is not running on the same terminal as whoever is trying to do exec.

Probably the correct solution belongs elsewhere -- whoever is calling Attach should send a resize event.

sohankunkerkar · 2025-06-12T11:41:57Z

You can use golang.org/x/term functions IsTerminal and GetSize to make the code simpler.

I don't think querying stdin/stdout/stderr of the current process for terminal size is a good idea, since the current process is a cri-o daemon which is not running on the same terminal as whoever is trying to do exec.

Probably the correct solution belongs elsewhere -- whoever is calling Attach should send a resize event.

Thanks @kolyshkin
I think it makes complete sense. However, this bug has already been triaged by the workloads team. I think resizing the terminal size during commands such as oc exec/attach is managed through bidirectional communication between the local machine and the pod. In this case, oc attach command sends 2 resizing events and cri-o first gets the increased size and then re-renders upon receiving the original size again. I checked the logs in the debug mode and it seems that second event is ignored by CRI-O somehow due to the race condition and monitor size is stuck at +1 version of size. That's where it falls under the purview of CRI-O where we need to evaluate why it's failing to show the correct terminal size.

openshift-ci-robot · 2025-06-12T13:13:19Z

@sohankunkerkar: This pull request references Jira Issue OCPBUGS-4556, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.20.0) matches configured target version for branch (4.20.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @zhouying7780

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

This fixes the issue where container terminals were getting stuck at incorrect sizes due to rapid successive resize events not being handled properly. When 'oc debug' or 'kubectl exec' sends the typical resize sequence (inflate +1, then restore to original), the second event was being processed but not applied correctly, leaving the terminal at the wrong size.

Before this fix:
$ kubectl debug node/192.168.1.16 -i -t --image=ubuntu -- sh
Creating debugging pod node-debugger-192.168.1.16-nqfmj with container debugger on node 192.168.1.16.
Warning: metadata.name: this is used in the Pod's hostname, which can result in surprising behavior; a DNS label is recommended: [must not contain dots]
If you don't see a command prompt, try pressing enter.

# tput cols ; tput lines
191
47
# exit

$ tput cols ; tput lines
190
46
After the fix:
$ kubectl debug node/192.168.1.16 -i -t --image=ubuntu -- sh
Creating debugging pod node-debugger-192.168.1.16-nqfmj with container debugger on node 192.168.1.16.
Warning: metadata.name: this is used in the Pod's hostname, which can result in surprising behavior; a DNS label is recommended: [must not contain dots]
If you don't see a command prompt, try pressing enter.

# tput cols ; tput lines
190
46
# exit

$ tput cols ; tput lines
190
46
What type of PR is this?

/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?
Fix terminal resize race condition

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2025-06-12T13:13:22Z

@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: zhouying7780.

Note that only cri-o members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

@sohankunkerkar: This pull request references Jira Issue OCPBUGS-4556, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)

bug target version (4.20.0) matches configured target version for branch (4.20.0)

bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @zhouying7780

The bug has been updated to refer to the pull request using the external bug tracker.
In response to this:
This fixes the issue where container terminals were getting stuck at incorrect sizes due to rapid successive resize events not being handled properly. When 'oc debug' or 'kubectl exec' sends the typical resize sequence (inflate +1, then restore to original), the second event was being processed but not applied correctly, leaving the terminal at the wrong size.

Before this fix:
$ kubectl debug node/192.168.1.16 -i -t --image=ubuntu -- sh
Creating debugging pod node-debugger-192.168.1.16-nqfmj with container debugger on node 192.168.1.16.
Warning: metadata.name: this is used in the Pod's hostname, which can result in surprising behavior; a DNS label is recommended: [must not contain dots]
If you don't see a command prompt, try pressing enter.

# tput cols ; tput lines
191
47
# exit

$ tput cols ; tput lines
190
46
After the fix:
$ kubectl debug node/192.168.1.16 -i -t --image=ubuntu -- sh
Creating debugging pod node-debugger-192.168.1.16-nqfmj with container debugger on node 192.168.1.16.
Warning: metadata.name: this is used in the Pod's hostname, which can result in surprising behavior; a DNS label is recommended: [must not contain dots]
If you don't see a command prompt, try pressing enter.

# tput cols ; tput lines
190
46
# exit

$ tput cols ; tput lines
190
46
What type of PR is this?

/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?
Fix terminal resize race condition
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

This fixes the issue where container terminals were getting stuck at incorrect sizes due to rapid successive resize events not being handled properly. When 'oc debug' or 'kubectl exec' sends the typical resize sequence (inflate +1, then restore to original), the second event was being processed but not applied correctly, leaving the terminal at the wrong size. Signed-off-by: Sohan Kunkerkar <[email protected]>

openshift-ci · 2025-06-12T15:25:52Z

@sohankunkerkar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/ci-e2e-evented-pleg	`0d449e0`	link	false	`/test ci-e2e-evented-pleg`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

kolyshkin

The new logic is totally different, it would make sense to close this PR and open a new one.

Anyway, the changes LGTM.

Have you confirmed this fixes the issue?

openshift-ci · 2025-06-12T19:40:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kolyshkin, sohankunkerkar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [kolyshkin,sohankunkerkar]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2025-06-12T19:45:25Z

@sohankunkerkar: Jira Issue OCPBUGS-4556: All pull requests linked via external trackers have merged:

cri-o/cri-o#9246

Jira Issue OCPBUGS-4556 has been moved to the MODIFIED state.

In response to this:

This fixes the issue where container terminals were getting stuck at incorrect sizes due to rapid successive resize events not being handled properly. When 'oc debug' or 'kubectl exec' sends the typical resize sequence (inflate +1, then restore to original), the second event was being processed but not applied correctly, leaving the terminal at the wrong size.

Before this fix:
$ kubectl debug node/192.168.1.16 -i -t --image=ubuntu -- sh
Creating debugging pod node-debugger-192.168.1.16-nqfmj with container debugger on node 192.168.1.16.
Warning: metadata.name: this is used in the Pod's hostname, which can result in surprising behavior; a DNS label is recommended: [must not contain dots]
If you don't see a command prompt, try pressing enter.

# tput cols ; tput lines
191
47
# exit

$ tput cols ; tput lines
190
46
After the fix:
$ kubectl debug node/192.168.1.16 -i -t --image=ubuntu -- sh
Creating debugging pod node-debugger-192.168.1.16-nqfmj with container debugger on node 192.168.1.16.
Warning: metadata.name: this is used in the Pod's hostname, which can result in surprising behavior; a DNS label is recommended: [must not contain dots]
If you don't see a command prompt, try pressing enter.

# tput cols ; tput lines
190
46
# exit

$ tput cols ; tput lines
190
46
What type of PR is this?

/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?
Fix terminal resize race condition

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

sohankunkerkar · 2025-06-12T20:18:37Z

Have you confirmed this fixes the issue?

Yeah, the current fix worked for me locally.

internal/oci/runtime_oci.go

This reverts cri-o#9246 The actual fix should be conmon and not in CRI-O. Signed-off-by: Sohan Kunkerkar <[email protected]>

sohankunkerkar requested a review from mrunalp as a code owner June 4, 2025 16:42

openshift-ci bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jun 4, 2025

openshift-ci bot requested review from hasan4791 and klihub June 4, 2025 16:42

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 4, 2025

sohankunkerkar changed the title ~~[OCPBUGS-4556]: internal/oci: set initial terminal size while attaching container~~ OCPBUGS-4556: internal/oci: set initial terminal size while attaching container Jun 4, 2025

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jun 4, 2025

sohankunkerkar force-pushed the fix-terminal-size branch 2 times, most recently from 73f867d to d6f437d Compare June 5, 2025 01:23

bitoku reviewed Jun 9, 2025

View reviewed changes

internal/oci/runtime_oci.go Outdated Show resolved Hide resolved

haircommander reviewed Jun 11, 2025

View reviewed changes

internal/oci/runtime_oci.go Outdated Show resolved Hide resolved

sohankunkerkar force-pushed the fix-terminal-size branch 2 times, most recently from 7042630 to 135f9fc Compare June 11, 2025 18:04

haircommander reviewed Jun 11, 2025

View reviewed changes

internal/oci/runtime_oci.go Outdated Show resolved Hide resolved

sohankunkerkar force-pushed the fix-terminal-size branch from 135f9fc to 8f7a435 Compare June 11, 2025 21:12

kolyshkin requested changes Jun 11, 2025

View reviewed changes

openshift-ci bot assigned kolyshkin Jun 11, 2025

sohankunkerkar force-pushed the fix-terminal-size branch 2 times, most recently from 755c480 to 6b520f3 Compare June 12, 2025 13:12

sohankunkerkar changed the title ~~OCPBUGS-4556: internal/oci: set initial terminal size while attaching container~~ OCPBUGS-4556: internal/oci: fix terminal resize race condition Jun 12, 2025

sohankunkerkar force-pushed the fix-terminal-size branch from 6b520f3 to e7104d9 Compare June 12, 2025 13:14

sohankunkerkar force-pushed the fix-terminal-size branch from e7104d9 to 0d449e0 Compare June 12, 2025 13:27

sohankunkerkar requested a review from kolyshkin June 12, 2025 13:28

kolyshkin approved these changes Jun 12, 2025

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 12, 2025

openshift-merge-bot bot merged commit c51883b into cri-o:main Jun 12, 2025
73 of 75 checks passed

haircommander reviewed Jun 12, 2025

View reviewed changes

internal/oci/runtime_oci.go Show resolved Hide resolved

sohankunkerkar added a commit to sohankunkerkar/cri-o that referenced this pull request Jun 12, 2025

[revert] internal/oci: fix terminal resize race condition

2f9b660

This reverts cri-o#9246 The actual fix should be conmon and not in CRI-O. Signed-off-by: Sohan Kunkerkar <[email protected]>

sohankunkerkar added a commit to sohankunkerkar/cri-o that referenced this pull request Jun 12, 2025

[revert] internal/oci: fix terminal resize race condition

cb2965f

This reverts cri-o#9246 The actual fix should be conmon and not in CRI-O. Signed-off-by: Sohan Kunkerkar <[email protected]>

sohankunkerkar mentioned this pull request Jun 12, 2025

[revert] internal/oci: fix terminal resize race condition #9255

Merged

github-actions bot mentioned this pull request Sep 14, 2025

Bump crio from v1.30.3 to v1.34.0 kokyhm/kubespray#293

Open

OCPBUGS-4556: internal/oci: fix terminal resize race condition #9246

OCPBUGS-4556: internal/oci: fix terminal resize race condition #9246

Uh oh!

Conversation

sohankunkerkar commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

openshift-ci-robot commented Jun 4, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

codecov bot commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sohankunkerkar commented Jun 4, 2025

Uh oh!

openshift-ci-robot commented Jun 4, 2025

Uh oh!

openshift-ci bot commented Jun 4, 2025

Uh oh!

Uh oh!

sohankunkerkar commented Jun 10, 2025

Uh oh!

Uh oh!

sohankunkerkar commented Jun 11, 2025

Uh oh!

Uh oh!

kolyshkin left a comment

Choose a reason for hiding this comment

Uh oh!

sohankunkerkar commented Jun 12, 2025

Uh oh!

openshift-ci-robot commented Jun 12, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

openshift-ci bot commented Jun 12, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

openshift-ci bot commented Jun 12, 2025

Uh oh!

kolyshkin left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Jun 12, 2025

Uh oh!

Uh oh!

openshift-ci-robot commented Jun 12, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

sohankunkerkar commented Jun 12, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

sohankunkerkar commented Jun 4, 2025 •

edited

Loading

codecov bot commented Jun 4, 2025 •

edited

Loading