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 3 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration 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)
WalkthroughReplaces a namespace/name-based debug-pod exception with detection via annotations/labels containing "debug.openshift.io"; adds a Jira exception entry for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
pkg/monitortests/authentication/nodefaultserviceaccountoperatortests/monitortest.go
Show resolved
Hide resolved
|
Scheduling required tests: |
|
/retest |
|
@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
force-pushed
the
fix-debug-pod-exception
branch
from
e53f764 to
0e37c91
Compare
|
@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. |
|
Manually triggering additional tests to see if debug issue resolved. Once these tests pass, I'll do another check on Sippy to ensure all cases have been caught so that the monitor tests won't fail due to uncaught reasonable exceptions. /test e2e-aws-csi |
|
@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