TRT-2652: Allow OTE info and list tests commands to work without KUBECONFIG#3170
Conversation
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
WalkthroughKubeconfig retrieval is made conditional: when kubeconfig retrieval fails ChangesKubeconfig & Test Filtering
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Kubeconfig
participant InfraProvider
participant GinkgoBeforeAll
participant TestFramework
CLI->>Kubeconfig: getKubeConfig()
alt kubeconfig succeeds
Kubeconfig->>InfraProvider: create ocpInfra / providers
CLI->>GinkgoBeforeAll: register BeforeAll (cfgErr=nil, infraErr=nil)
GinkgoBeforeAll->>TestFramework: initializeTestFramework(cfg)
TestFramework->>TestFramework: run tests
else kubeconfig fails
Kubeconfig-->>CLI: cfgErr != nil
CLI->>GinkgoBeforeAll: register BeforeAll (cfgErr!=nil)
note right of GinkgoBeforeAll: listing/info flows allowed\nshouldIncludeTest treats ocpInfra as nil
GinkgoBeforeAll->>GinkgoBeforeAll: panic on cfgErr/infraErr if executed
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
petr-muller
force-pushed
the
fix-info-without-kubeconfig
branch
from
1ae04d9 to
20254e3
Compare
|
/test all |
|
/test unit |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
![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 `@openshift/cmd/ovn-kubernetes-tests-ext/main.go`:
- Around line 109-123: The current code discards the original getKubeConfig()
error and later panics with a generic "KUBECONFIG is required to run tests" in
specs.AddBeforeAll; capture and preserve the error from getKubeConfig() (e.g.,
store it in a variable like kubeCfgErr) and in the AddBeforeAll closure check
that error instead of only cfg being nil—if kubeCfgErr is non-nil, panic (or
log+panic) including the original error message so the real cause (file
unreadable, parse error, etc.) is surfaced; keep existing logic that uses cfg
when err is nil and only change the AddBeforeAll condition to reference the
preserved error variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Enterprise
Run ID: 3bc3919f-3584-4f1a-b0d8-a96681ad49d3
📒 Files selected for processing (1)
openshift/cmd/ovn-kubernetes-tests-ext/main.go
petr-muller
force-pushed
the
fix-info-without-kubeconfig
branch
from
20254e3 to
a0a2356
Compare
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 `@openshift/cmd/ovn-kubernetes-tests-ext/main.go`:
- Around line 109-117: Currently main() panics on ocpinfraprovider.New(cfg)
failure; change it to keep ocpInfra nil on New() error (do not panic) and only
call infraprovider.Set(...) and ocpdeploymentconfig.New() when
ocpinfraprovider.New succeeds; capture the error and store/surface it later in
AddBeforeAll alongside cfgErr so test execution (list/info) remains resilient.
Update both occurrences where ocpinfraprovider.New(cfg) is called to remove the
panic and defer error handling to AddBeforeAll, referencing getKubeConfig,
ocpinfraprovider.New, ocpInfra, infraprovider.Set, deploymentconfig.Set and
AddBeforeAll.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Enterprise
Run ID: 04a155be-245e-4bd4-a233-5789e220dbf4
📒 Files selected for processing (1)
openshift/cmd/ovn-kubernetes-tests-ext/main.go
Make getKubeConfig() failure non-fatal in main(). When KUBECONFIG is absent, ocpInfra stays nil and shouldIncludeTest() includes all eligible tests without cluster-dependent filtering (e.g. EVPN checks). The AddBeforeAll guard ensures that running tests still requires KUBECONFIG. This enables workflows that need to discover or list tests from ovn-kubernetes without a live cluster connection, such as the openshift-tests "list all-tests" command. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
petr-muller
force-pushed
the
fix-info-without-kubeconfig
branch
from
a0a2356 to
0e57b94
Compare
petr-muller
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@petr-muller: This pull request references TRT-2652 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. 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. |
|
Before: $ KUBECONFIG= go run ./cmd/ovn-kubernetes-tests-ext info
panic: KUBECONFIG env variable not set
goroutine 1 [running]:
main.main()
/home/afri/Projects/RH/github.com/openshift/ovn-kubernetes/openshift/cmd/ovn-kubernetes-tests-ext/main.go:105 +0x14d9
exit status 2
$ KUBECONFIG= go run ./cmd/ovn-kubernetes-tests-ext list tests
panic: KUBECONFIG env variable not set
goroutine 1 [running]:
main.main()
/home/afri/Projects/RH/github.com/openshift/ovn-kubernetes/openshift/cmd/ovn-kubernetes-tests-ext/main.go:105 +0x14d9
exit status 2 |
|
After: $ KUBECONFIG= go run ./cmd/ovn-kubernetes-tests-ext info | jq .suites[].name
"ovn-kubernetes/conformance/serial"
"ovn-kubernetes/conformance/parallel"
$ KUBECONFIG= go run ./cmd/ovn-kubernetes-tests-ext list tests | jq '.[] | .name' | head -n 5
"[Feature:NetworkSegmentation][ovn-kubernetes-ote][sig-network] Network Segmentation a user defined primary network created using NetworkAttachmentDefinitions creates a networkStatus Annotation with UDN interface L2 primary UDN [Suite:openshift/conformance/parallel]"
"[Feature:NetworkSegmentation][ovn-kubernetes-ote][sig-network] Network Segmentation a user defined primary network created using NetworkAttachmentDefinitions creates a networkStatus Annotation with UDN interface L2 primary UDN with custom network [Suite:openshift/conformance/parallel]"
"[Feature:NetworkSegmentation][ovn-kubernetes-ote][sig-network] Network Segmentation a user defined primary network created using NetworkAttachmentDefinitions creates a networkStatus Annotation with UDN interface L3 primary UDN [Suite:openshift/conformance/parallel]"
"[Feature:NetworkSegmentation][ovn-kubernetes-ote][sig-network] Network Segmentation a user defined primary network created using NetworkAttachmentDefinitions can perform east/west traffic between nodes two pods connected over a L2 primary UDN [Suite:openshift/conformance/parallel]"
"[Feature:NetworkSegmentation][ovn-kubernetes-ote][sig-network] Network Segmentation a user defined primary network created using NetworkAttachmentDefinitions can perform east/west traffic between nodes two pods connected over a L2 primary UDN with custom network [Suite:openshift/conformance/parallel]" |
|
Neither of the CI job failures look suspicious wrt the OTE change in this PR |
petr-muller
changed the title
|
/lgtm |
|
/assign @abhat For |
|
Is #3170 (comment) sufficient for me to do a |
|
/verified by CI and manual testing from Petr |
openshift-ci-robot
added
the
verified
|
@jluhrsen: This PR has been marked as verified by 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. |
|
/retest |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhat, jluhrsen, petr-muller 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 |
openshift-ci
Bot
added
the
approved
no plan that I know of. we just ignore it and have been forever. I think it's failing all over openshift projects, TBH. it's because of issues in vendored code though, not ours. I tried to look at it a long time ago, and failed. just added a story to our tech dept epic about this. maybe with some help from AI I can get this cleaned up. it's always annoyed me. |
|
@petr-muller: 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. |
openshift-merge-bot
Bot
merged commit a9a7b4e
into
openshift:master
4 participants
Summary
Make
getKubeConfig()failure non-fatal in theovn-kubernetes-tests-extbinary'smain(). When KUBECONFIG is absent,ocpInfrastays nil andshouldIncludeTest()includes all eligible tests without cluster-dependent filtering (e.g. EVPN checks). TheAddBeforeAllguard ensures that actually running tests still requires KUBECONFIG.This enables workflows that need to discover or list tests from ovn-kubernetes without a live cluster connection, such as the
openshift-tests list all-testscommand being added in openshift/origin#31105.Related
openshift-tests list all-testscommand, which callsinfoandlist testson all payload extension binaries without cluster access🤖 Generated with Claude Code
Summary by CodeRabbit