BUG OCPBUGS-119955: Fix oc debug -T race condition with container attach by davegord · Pull Request #2392 · openshift/oc

davegord · 2026-09-03T18:56:55Z

Summary

oc debug -T (--no-tty) fails with "unable to upgrade connection: container container-00 not found" on OCP 5.0 (OCPBUGS-119955)
Root cause: when -T is set, Stdin remains true, routing through the SPDY/WebSocket attach path instead of log-streaming. For fast-exiting commands, the container terminates before the upgrade completes.
Fix: disable Stdin when -T is set, matching the existing behavior for commands passed without -T (lines 302-304). This routes through the log-streaming path which can't race with container completion.

Details

In Complete(), the case o.DisableTTY: branch (line 299) only set o.Attach.TTY = false but left o.Attach.Stdin = true (the default). This caused RunDebug() to hit the default: case (line 665) which calls o.Attach.Run() — an attach that requires a SPDY/WebSocket connection upgrade. When the container exits before the upgrade completes, the kubelet returns "container not found".

The existing case len(o.Command) > 0: branch (line 302) already correctly sets both TTY=false and Stdin=false, but -T has higher priority in the switch and was missing the Stdin=false.

The code even has a TODO acknowledging this race: // TODO: attach can race with pod completion, allow attach to switch to logs

Test plan

oc debug node/<node> -T -- cat /etc/os-release should succeed consistently (was 100% failure rate before)
oc debug node/<node> -- cat /etc/os-release should continue to work (no behavior change)
oc debug node/<node> (interactive) should continue to work with TTY
oc debug -t node/<node> (explicit TTY) should continue to work

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- When running debug attachment with --no-tty, standard input is now disabled as expected.

When using `oc debug -T` (--no-tty), only TTY was disabled but Stdin remained true. This caused the command to take the attach code path (SPDY/WebSocket upgrade) instead of the log-streaming path. For fast-exiting commands, the container terminates before the connection upgrade completes, resulting in "unable to upgrade connection: container container-00 not found". The fix disables Stdin when -T is set, matching the behavior already used when a command is passed without -T (lines 302-304). This routes -T commands through the log-streaming path which doesn't require an attach and can't race with container completion. Fixes: OCPBUGS-119955 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

openshift-merge-bot · 2026-09-03T18:56:57Z

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

coderabbitai · 2026-09-03T18:57:19Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 4a839f8a-8fe0-4eed-98dc-eb7ca63a511f

📥 Commits

Reviewing files that changed from the base of the PR and between e0e4c04 and d8bd38d.

📒 Files selected for processing (1)

pkg/cli/debug/debug.go

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

The debug attachment command now disables STDIN forwarding when --no-tty is specified.

Changes

Debug attachment behavior

Layer / File(s)	Summary
Disable STDIN for no-TTY attachments `pkg/cli/debug/debug.go`	The `--no-tty` path disables both TTY allocation and STDIN forwarding.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to d8bd3

The change makes non-TTY debug attachments avoid stdin forwarding, preventing the unnecessary interactive connection upgrade for fast-exiting containers. No current merge-readiness risk remains.

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	PASS. The pull request changes only `pkg/cli/debug/debug.go` and adds `o.Attach.Stdin = false` in `Complete()`. The diff introduces no Ginkgo test declarations or test titles, so it cannot introduce d…
Test Structure And Quality	✅ Passed	PASS: The pull request changes only one production line in `pkg/cli/debug/debug.go`. It adds `o.Attach.Stdin = false` to the `DisableTTY` branch. No Ginkgo test code or test files changed, so the list…
Microshift Test Compatibility	✅ Passed	PASS — The pull request changes only `pkg/cli/debug/debug.go` by setting `o.Attach.Stdin = false` in the `--no-tty` branch. The parent-to-HEAD diff contains no new Ginkgo tests or test files, so the M…
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PASS: The pull request changes only `pkg/cli/debug/debug.go` by setting `o.Attach.Stdin = false` in the existing `case o.DisableTTY:` branch. No Ginkgo e2e tests or new `It()`, `Describe()`, `Context(…
Topology-Aware Scheduling Compatibility	✅ Passed	PASS: The pull request changes only `pkg/cli/debug/debug.go` and adds `o.Attach.Stdin = false` in the `--no-tty` branch of `Complete()`. It does not add or modify deployment manifests, operator code, …
Ote Binary Stdout Contract	✅ Passed	PASS: The pull request changes only `pkg/cli/debug/debug.go` by setting `o.Attach.Stdin = false`. The added line does not write to stdout and is not process-level OTE code. The OTE entry point is unch…
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PASS: The pull request changes only `pkg/cli/debug/debug.go` and adds one assignment in `Complete()`. The diff contains no new Ginkgo declarations or test files, so this compatibility check does not a…
No-Weak-Crypto	✅ Passed	PASS. The pull request changes one line: `o.Attach.Stdin = false` in `Complete()` for `--no-tty`. The patch introduces no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom cryptography, or secret/token…
Container-Privileges	✅ Passed	PASS: The pull request changes only `o.Attach.Stdin = false` in `pkg/cli/debug/debug.go`. The privilege-related pod fields (`Privileged: true`, `HostPID`, `HostNetwork`, `HostIPC`, and `RunAsUser: 0`)…
No-Sensitive-Data-In-Logs	✅ Passed	PASS: The pull request adds only `o.Attach.Stdin = false` in the `--no-tty` branch. The diff adds no logging or output statement and introduces no sensitive value handling. Existing `klog` and `fmt` m…
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the affected command, the `--no-tty` mode, and the race condition addressed by the change. It matches the pull request objectives.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

Full details: Stable And Deterministic Test Names

Explanation

PASS. The pull request changes only pkg/cli/debug/debug.go and adds o.Attach.Stdin = false in Complete(). The diff introduces no Ginkgo test declarations or test titles, so it cannot introduce dynamic or overly specific test names.

Full details: Test Structure And Quality

Explanation

PASS: The pull request changes only one production line in pkg/cli/debug/debug.go. It adds o.Attach.Stdin = false to the DisableTTY branch. No Ginkgo test code or test files changed, so the listed Ginkgo test-quality requirements do not apply.

Full details: Microshift Test Compatibility

Explanation

PASS — The pull request changes only pkg/cli/debug/debug.go by setting o.Attach.Stdin = false in the --no-tty branch. The parent-to-HEAD diff contains no new Ginkgo tests or test files, so the MicroShift test compatibility check does not apply.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

PASS: The pull request changes only pkg/cli/debug/debug.go by setting o.Attach.Stdin = false in the existing case o.DisableTTY: branch. No Ginkgo e2e tests or new It(), Describe(), Context(), or When() blocks were added. Therefore, the SNO multi-node compatibility check is not applicable.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS: The pull request changes only pkg/cli/debug/debug.go and adds o.Attach.Stdin = false in the --no-tty branch of Complete(). It does not add or modify deployment manifests, operator code, controllers, replica settings, affinities, topology spread constraints, node selectors, taint tolerations, or PDBs. The topology-aware scheduling check is therefore not applicable.

Full details: Ote Binary Stdout Contract

Explanation

PASS: The pull request changes only pkg/cli/debug/debug.go by setting o.Attach.Stdin = false. The added line does not write to stdout and is not process-level OTE code. The OTE entry point is unchanged, and its existing klog.Fatal calls are present in both the parent and current revisions.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

PASS: The pull request changes only pkg/cli/debug/debug.go and adds one assignment in Complete(). The diff contains no new Ginkgo declarations or test files, so this compatibility check does not apply.

Full details: No-Weak-Crypto

Explanation

PASS. The pull request changes one line: o.Attach.Stdin = false in Complete() for --no-tty. The patch introduces no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom cryptography, or secret/token comparisons.

Full details: Container-Privileges

Explanation

PASS: The pull request changes only o.Attach.Stdin = false in pkg/cli/debug/debug.go. The privilege-related pod fields (Privileged: true, HostPID, HostNetwork, HostIPC, and RunAsUser: 0) are pre-existing and identical in HEAD^ and HEAD. The change adds no SYS_ADMIN capability, allowPrivilegeEscalation, or new root execution configuration.

Full details: No-Sensitive-Data-In-Logs

Explanation

PASS: The pull request adds only o.Attach.Stdin = false in the --no-tty branch. The diff adds no logging or output statement and introduces no sensitive value handling. Existing klog and fmt messages are unchanged. The change only selects the existing log-streaming path instead of the attach path.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

openshift-ci · 2026-09-03T18:57:38Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: davegord
Once this PR has been reviewed and has the lgtm label, please assign ardaguclu for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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 · 2026-09-03T19:12:09Z

@davegord: This pull request references Jira Issue OCPBUGS-119955, which is invalid:

expected the bug to target the "5.1.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.

Details

In response to this:

Summary

oc debug -T (--no-tty) fails with "unable to upgrade connection: container container-00 not found" on OCP 5.0 (OCPBUGS-119955)

Root cause: when -T is set, Stdin remains true, routing through the SPDY/WebSocket attach path instead of log-streaming. For fast-exiting commands, the container terminates before the upgrade completes.

Fix: disable Stdin when -T is set, matching the existing behavior for commands passed without -T (lines 302-304). This routes through the log-streaming path which can't race with container completion.

Details

In Complete(), the case o.DisableTTY: branch (line 299) only set o.Attach.TTY = false but left o.Attach.Stdin = true (the default). This caused RunDebug() to hit the default: case (line 665) which calls o.Attach.Run() — an attach that requires a SPDY/WebSocket connection upgrade. When the container exits before the upgrade completes, the kubelet returns "container not found".

The existing case len(o.Command) > 0: branch (line 302) already correctly sets both TTY=false and Stdin=false, but -T has higher priority in the switch and was missing the Stdin=false.

The code even has a TODO acknowledging this race: // TODO: attach can race with pod completion, allow attach to switch to logs

Test plan

oc debug node/<node> -T -- cat /etc/os-release should succeed consistently (was 100% failure rate before)

oc debug node/<node> -- cat /etc/os-release should continue to work (no behavior change)

oc debug node/<node> (interactive) should continue to work with TTY

oc debug -t node/<node> (explicit TTY) should continue to work

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes

When running debug attachment with --no-tty, standard input is now disabled as expected.

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.

davegord · 2026-09-03T19:12:59Z

/jira refresh

openshift-ci-robot · 2026-09-03T19:13:04Z

@davegord: This pull request references Jira Issue OCPBUGS-119955, which is invalid:

expected the bug to target the "5.1.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.

Details

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.

davegord · 2026-09-03T19:13:33Z

/jira refresh

openshift-ci-robot · 2026-09-03T19:13:40Z

@davegord: This pull request references Jira Issue OCPBUGS-119955, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

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

Details

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 · 2026-09-03T19:53:08Z

@davegord: 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/okd-scos-images	`d8bd38d`	link	true	`/test okd-scos-images`

Full PR test history. Your PR dashboard.

Details

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.

openshift-ci Bot requested review from ardaguclu and tchap September 3, 2026 18:57

davegord changed the title ~~Fix oc debug -T race condition with container attach~~ BUG OCPBUGS-119955: Fix oc debug -T race condition with container attach Sep 3, 2026

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 Sep 3, 2026

Navigation Menu

Uh oh!

BUG OCPBUGS-119955: Fix oc debug -T race condition with container attach - #2392

BUG OCPBUGS-119955: Fix oc debug -T race condition with container attach#2392
davegord wants to merge 1 commit into
openshift:mainfrom
davegord:fix/debug-no-tty-race-condition

davegord commented Sep 3, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

openshift-merge-bot Bot commented Sep 3, 2026

Uh oh!

coderabbitai Bot commented Sep 3, 2026 •

edited

Loading

Uh oh!

openshift-ci Bot commented Sep 3, 2026

Uh oh!

openshift-ci-robot commented Sep 3, 2026

Summary

Details

Test plan

Summary by CodeRabbit

Uh oh!

davegord commented Sep 3, 2026

Uh oh!

openshift-ci-robot commented Sep 3, 2026

Uh oh!

davegord commented Sep 3, 2026

Uh oh!

openshift-ci-robot commented Sep 3, 2026

Uh oh!

openshift-ci Bot commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

davegord commented Sep 3, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Test plan

Summary by CodeRabbit

Uh oh!

openshift-merge-bot Bot commented Sep 3, 2026

Uh oh!

coderabbitai Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

openshift-ci Bot commented Sep 3, 2026

Uh oh!

openshift-ci-robot commented Sep 3, 2026

Summary

Details

Test plan

Summary by CodeRabbit

Uh oh!

davegord commented Sep 3, 2026

Uh oh!

openshift-ci-robot commented Sep 3, 2026

Uh oh!

davegord commented Sep 3, 2026

Uh oh!

openshift-ci-robot commented Sep 3, 2026

Uh oh!

openshift-ci Bot commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

davegord commented Sep 3, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 3, 2026 •

edited

Loading