CNTRLPLANE-2202: Check for debug pod (regardless of ns) in default service account monitor test#30815
CNTRLPLANE-2202: Check for debug pod (regardless of ns) in default service account monitor test#30815ehearne-redhat wants to merge 2 commits intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@ehearne-redhat: This pull request references CNTRLPLANE-2202 which is a valid jira issue. DetailsIn response to this:
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. |
WalkthroughBroadens a monitoring test exception: the check now treats any pod whose name contains "debug" as an exception regardless of namespace; related comment was generalized to match the new behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
|
@ehearne-redhat: This pull request references CNTRLPLANE-2202 which is a valid jira issue. DetailsIn response to this:
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. |
|
@ehearne-redhat: This pull request references CNTRLPLANE-2202 which is a valid jira issue. DetailsIn response to this:
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. |
|
Scheduling required tests: |
|
/retest |
|
/approve |
openshift-ci
bot
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ehearne-redhat, everettraven, neisw The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold We seem to have missed one.
I'm going to add this one and discuss this with my team on next steps. |
openshift-ci
bot
added
the
do-not-merge/hold
|
New changes are detected. LGTM label has been removed. |
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@ehearne-redhat: This pull request references CNTRLPLANE-2202 which is a valid jira issue. DetailsIn response to this:
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@pkg/monitortests/authentication/nodefaultserviceaccountoperatortests/monitortest.go`:
- Around line 92-96: The current debug-pod detection in the anonymous function
(func(pod corev1.Pod) (string, bool)) uses strings.Contains(pod.Name, "debug")
and should be tightened; change the logic to detect true oc debug pods by either
using strings.HasSuffix(pod.Name, "-debug") or, preferably, checking for the
debug annotation on the Pod (presence of "debug.openshift.io/source-resource" in
pod.Annotations) and return the same exemption when that condition is met
instead of the broad substring match.
ℹ️ Review info
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
pkg/monitortests/authentication/nodefaultserviceaccountoperatortests/monitortest.go
| // Handle the outlier manually | ||
| // This one checks if it is a debug pod or not. | ||
| func(pod corev1.Pod) (string, bool) { | ||
| if pod.Namespace == "openshift-commatrix-test" && strings.Contains(pod.Name, "debug") { | ||
| if strings.Contains(pod.Name, "debug") { | ||
| return "https://issues.redhat.com/browse/OCPBUGS-77201", true |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
OpenShift oc debug command pod naming pattern identity metadata labels
💡 Result:
oc debug pod naming pattern
- Debugging a workload/pod (e.g.,
oc debug pod/foo,oc debug deploy/bar,oc debug dc/baz): the debug pod name is typically<original-name>-debug(example:nbviewer-debug). [1] - Debugging a node (e.g.,
oc debug node/<node>): the debug pod name is formed from the node name with dots (.) replaced by dashes (-), with-debugappended. [2] - Node debugging may also create a temporary namespace with a generated name like
openshift-debug-…/openshift-debug-node-…(implementation/detail varies by version/config). [3][4]
Identity / traceability metadata (annotations)
oc debug uses annotations to record what you debugged:
debug.openshift.io/source-resource: points at the source object (pod/node/etc.). [5]debug.openshift.io/source-container: records the source container name. [5]
Labels behavior
- By default, the debug pod is a copy of the source pod but with labels stripped (unless you explicitly request otherwise, e.g.
--keep-labels). [6]
Sources:
[1] OpenShift Cookbook example showing nbviewer-debug pod name output from oc debug dc/nbviewer. (cookbook.openshift.org)
[2] Red Hat blog describing node debug pod name creation (node-name-debug, dots replaced with dashes). (redhat.com)
[3] Red Hat Customer Portal example showing Creating debug namespace/openshift-debug-node-.... (access.redhat.com)
[4] oc source code showing temp namespace GenerateName: "openshift-debug-" for node debug when needed. (raw.githubusercontent.com)
[5] Example debug pod YAML showing debug.openshift.io/source-resource and debug.openshift.io/source-container annotations. (publicdoc.rbbn.com)
[6] oc debug help text stating the debug pod is created with “labels stripped” (unless kept). (docs.okd.io)
Citations:
- 1: https://cookbook.openshift.org/logging-monitoring-and-debugging/how-do-i-debug-an-application-that-fails-to-start-up.html?utm_source=openai
- 2: https://www.redhat.com/es/blog/how-oc-debug-works?utm_source=openai
- 3: https://access.redhat.com/solutions/6961235?utm_source=openai
- 4: https://raw.githubusercontent.com/openshift/oc/e468aa68e754e22997f526fcdbbd470387ff5e33/pkg/cli/debug/debug.go
- 5: https://publicdoc.rbbn.com/display/SBXDOC121/Debug%2Bat%2Bthe%2BNode%2BLevel%2Bto%2BAvoid%2BSSH%2Bat%2BWorker%2BNode%2BLevel?utm_source=openai
- 6: https://docs.okd.io/4.18/cli_reference/openshift_cli/developer-cli-commands.html?utm_source=openai
Use the documented debug pod pattern instead of a broad substring match.
Line 95 exempts any pod with "debug" anywhere in its name using strings.Contains(). OpenShift's documented pattern for debug pods created via oc debug is more specific: workload debug pods are named <original-name>-debug (suffix), and debug pods carry annotations like debug.openshift.io/source-resource. The current substring match risks incorrectly exempting unrelated workloads with "debug" in their name (e.g., "debugging-tool", "debugger").
Replace with strings.HasSuffix(pod.Name, "-debug") or, more reliably, check for the presence of the debug.openshift.io/source-resource annotation to properly distinguish true debug pods.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@pkg/monitortests/authentication/nodefaultserviceaccountoperatortests/monitortest.go`
around lines 92 - 96, The current debug-pod detection in the anonymous function
(func(pod corev1.Pod) (string, bool)) uses strings.Contains(pod.Name, "debug")
and should be tightened; change the logic to detect true oc debug pods by either
using strings.HasSuffix(pod.Name, "-debug") or, preferably, checking for the
debug annotation on the Pod (presence of "debug.openshift.io/source-resource" in
pod.Annotations) and return the same exemption when that condition is met
instead of the broad substring match.
|
Scheduling required tests: |
|
@ehearne-redhat: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
4 participants
This fix addresses
debugpod usingdefaultservice account detection in https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-network-operator/2868/pull-ci-openshift-cluster-network-operator-master-4.22-upgrade-from-stable-4.21-e2e-azure-ovn-upgrade/2026955427433943040 .Summary by CodeRabbit