e2e: move AdminKubeClient() from init to BeforeEach in router config_manager test#31335
e2e: move AdminKubeClient() from init to BeforeEach in router config_manager test#31335redhat-chai-bot wants to merge 1 commit into
Conversation
…manager test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci
Bot
added
the
ready-for-human-review
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughIn ChangesRouter config manager test setup
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: redhat-chai-bot The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| @@ -65,9 +65,9 @@ var _ = g.Describe("[sig-network][Feature:Router][apigroup:route.openshift.io]", | |||
| // in order to allow binding 80/443 without being root. Without the privilege, | |||
| // the capability does not take effect and haproxy fails to start. | |||
| oc = exutil.NewCLIWithPodSecurityLevel("router-config-manager", api.LevelPrivileged) | |||
There was a problem hiding this comment.
The oc here is also suspect to me, I think this is also a previously known problem to have it outside in places that get run when ginkgo builds the tree
There was a problem hiding this comment.
See Chai's comment below, it really should've replied in the thread
|
Good question! I investigated this and Here's what it does at construction time (Describe scope):
The heavy lifting (kubeconfig file reads, API calls, project creation) all happens inside those deferred In contrast, This is the standard pattern across hundreds of test files in openshift/origin — moving |
|
/test verify-deps |
|
@redhat-chai-bot: The following test 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. |
|
/retest verify-deps |
|
@redhat-chai-bot: The The following commands are available to trigger optional jobs: Use 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 kubernetes-sigs/prow repository. |
|
|
|
/retest verify-deps |
|
@redhat-chai-bot: The The following commands are available to trigger optional jobs: Use 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 kubernetes-sigs/prow repository. |
3 participants
Summary
Move
kubeClient = oc.AdminKubeClient()from Ginkgo Describe-block scope (test tree construction time) to inside theBeforeEachblock (test runtime) intest/extended/router/config_manager.go.Root Cause
The
AdminKubeClient()call at line 68 executes during Ginkgo'sBuildTree()phase — when tests are being registered, not executed. If the kubeconfig file is unavailable at registration time (e.g. credential lifecycle issue, file cleanup race),AdminKubeClient()callsFatalErr()which panics the entire Ginkgo process.This kills every test scheduled in that binary — not just the router tests — causing cascading failures across unrelated SIGs (
sig-api-machinery,sig-apps,sig-auth,sig-cli,sig-node, etc.).Impact
In the last 7 days on OCP 5.0:
ci-5.0-e2e-gcp-ovn-upgraderouter.init.func2+config_manager.go:68stacktraceFix
Move the single
kubeClient = oc.AdminKubeClient()assignment into the existingBeforeEachblock. Theoc = exutil.NewCLIWithPodSecurityLevel(...)call remains at Describe scope — it only creates the CLI struct without making API calls.This is the only
AdminKubeClient()call at Describe scope across all router test files.Failing Job
periodic-ci-openshift-release-main-ci-5.0-e2e-gcp-ovn-upgrade #2069723201876791296 — 435 test failures, 165 caused by the
router.init.func2panic.Summary by CodeRabbit