CNTRLPLANE-3033:test/e2e: migrate apiservice-ca-bundle-injection to OTE serial suite#332
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRemoved inline APIService CA-bundle polling from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
openshift-ci
Bot
added
the
approved
wangke19
force-pushed
the
ote-migrate-ca-bundle-update
branch
from
2b8b928 to
a64dbd5
Compare
wangke19
changed the title
wangke19
force-pushed
the
ote-migrate-ca-bundle-update
branch
from
a64dbd5 to
fc2f346
Compare
wangke19
changed the title
wangke19
force-pushed
the
ote-migrate-ca-bundle-update
branch
from
fc2f346 to
aada6d6
Compare
![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 `@test/e2e/e2e.go`:
- Around line 1875-1881: Replace panic-style client construction with explicit
error handling: change apiserviceclient.NewForConfigOrDie(adminConfig) to
apiserviceclient.NewForConfig(adminConfig), check the returned error and call
t.Fatalf("error creating APIService client: %v", err) if non-nil before using
.ApiregistrationV1().APIServices(); do the same in the other occurrence where
apiextclient.NewForConfigOrDie(...) is used (replace with
apiextclient.NewForConfig(...) and t.Fatalf on error) so client creation follows
the prior getKubeClientAndConfig() error-handling pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d51dd26-0f12-490b-a13a-41deb1809b7a
📒 Files selected for processing (2)
test/e2e/e2e.gotest/e2e/e2e_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e/e2e_test.go
wangke19
force-pushed
the
ote-migrate-ca-bundle-update
branch
from
aada6d6 to
28be6b6
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 `@test/e2e/e2e.go`:
- Around line 1925-1929: The Update of the APIService (client.Update called on
injectedObj) can 409 due to concurrent kube-aggregator reconciliation; wrap the
mutate+update in retry.RetryOnConflict, and inside the retry closure re-fetch
the latest object (client.Get with context.TODO and metav1.GetOptions), modify
injectedObj.Spec.CABundle (append the invalid bytes) and then call
client.Update; this ensures you always operate on the latest resourceVersion and
retry on conflicts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9c0ab820-219b-497a-b292-c9986de38398
📒 Files selected for processing (2)
test/e2e/e2e.gotest/e2e/e2e_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e/e2e_test.go
Extract the apiservice-ca-bundle-injection test from TestE2E in e2e_test.go into a shared testAPIServiceCABundleInjection(testing.TB) function in e2e.go, enabling dual-mode execution via both the legacy go test path and the OTE Ginkgo framework. - Add testAPIServiceCABundleInjection in e2e.go using testing.TB - Add pollForSigningCABundleTB and pollForAPIServiceTB helpers in e2e.go using pollForResourceGinkgo for Ginkgo compatibility - Add [Operator][Serial] Ginkgo context in e2e.go - Add required imports (apiregv1, apiserviceclient, apiserviceclientv1) to e2e.go - Replace inline test body in e2e_test.go with a call to the shared function and add the standard OTE migration NOTE comment - Remove now-unused pollForAPIService helper from e2e_test.go - Remove now-unused apiregv1, apiserviceclient, apiserviceclientv1 imports from e2e_test.go This test is not disruptive (no CA rotation triggered) so it uses the [Operator][Serial] label without [Disruptive]. Jira: https://redhat.atlassian.net/browse/CNTRLPLANE-3000
wangke19
force-pushed
the
ote-migrate-ca-bundle-update
branch
from
28be6b6 to
6e2bf9e
Compare
|
/retest |
|
@wangke19: all tests passed! 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. |
|
/verified by CI |
|
/lgtm |
openshift-ci-robot
added
the
verified
|
@wangke19: 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. |
wangke19
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@wangke19: This pull request references CNTRLPLANE-3033 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 sub-task to target the "4.22.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. |
|
/jira refresh |
|
@wangke19: This pull request references CNTRLPLANE-3033 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gangwgr, wangke19 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 |
3 participants
Summary
Migrate the
apiservice-ca-bundle-injectiontest from an inline body inTestE2E(e2e_test.go) into a sharedtestAPIServiceCABundleInjection(testing.TB)function in e2e.go, enabling dual-mode execution via both the legacygo testpath and the OTE Ginkgo framework.Jira: https://redhat.atlassian.net/browse/CNTRLPLANE-3000
Changes
test/e2e/e2e.go[Operator][Serial]Ginkgo context forapiservice-ca-bundle-injectiontestAPIServiceCABundleInjection(testing.TB)shared implementationpollForSigningCABundleTBandpollForAPIServiceTBTB-compatible poll helpersapiregv1,apiserviceclient,apiserviceclientv1)test/e2e/e2e_test.gotestAPIServiceCABundleInjectionand standard OTE migration NOTE commentpollForAPIServicehelperapiregv1,apiserviceclient,apiserviceclientv1importsNotes
This test is not disruptive (no CA rotation triggered), so it uses
[Operator][Serial]without[Disruptive].Test Plan
go test ./test/e2e/...