WIP test: share single HCP cluster across SRE admin API E2E tests - #6677
WIP test: share single HCP cluster across SRE admin API E2E tests#6677Gerd Oberlechner (geoberle) wants to merge 1 commit into
Conversation
The three admin API tests (breakglass, serial console, boot diagnostics) each provisioned their own HCP cluster independently. Wrap them in an Ordered Context with a shared BeforeAll that creates one cluster and one worker nodepool, eliminating two redundant cluster provisions per suite run.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: geoberle 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 |
There was a problem hiding this comment.
Pull request overview
This PR refactors SRE admin API E2E tests to share one HCP cluster and worker nodepool across three scenarios.
Changes:
- Adds ordered shared setup with
BeforeAll. - Reuses shared cluster, nodepool, and identity state.
- Consolidates setup for breakglass, serial console, and boot diagnostics tests.
Suppressed comments (3)
test/e2e/admin_api.go:38
- Even after fixing cleanup,
Orderedmakes these three checks one failure domain: when an earlier spec fails, Ginkgo skips later specs. A breakglass regression would therefore suppress the independent serial-console and boot-diagnostics checks, contrary to the self-contained test requirement intest/AGENTS.md:29-30,467. Keep the specs independent, or continue ordered execution after failures and make each spec safe after preceding mutations.
Context("admin API", Ordered,
test/e2e/admin_api.go:59
- Constructing the test context during spec registration also runs
setupAzureLoggingbefore anyItis executing. That installs a process-global Azure SDK log listener for this context, but later tests'NewTestContextcalls replace it; by the time this ordered context runs, its Azure logs can be written to another test's file (or the closed file from the preceding test), so the admin API diagnostics are lost/misattributed. The shared setup needs a runtime context initialization that binds logging for this ordered scope, or the framework must explicitly support shared contexts.
tc := framework.NewTestContext()
test/e2e/admin_api.go:61
run-suitedoes not execute these leaf specs in one Ginkgo process:aro-hcp-testsdocuments that openshift-tests-extension launches each spec in a separaterun-testworker (test/cmd/aro-hcp-tests/main.go:172-176, 434-437). As a result, thisBeforeAlland its variables are recreated per worker, so CI still provisions one cluster per test andOrderedcannot provide cross-test ordering. To share setup, these checks need to be one leaf spec or use a runner-level/shared-resource mechanism.
BeforeAll(func(ctx context.Context) {
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| By("testing error case: boot diagnostics disabled") | ||
| err = tc.DisableVMBootDiagnostics(ctx, managedResourceGroupName, vmName) | ||
| Expect(err).NotTo(HaveOccurred(), "failed to disable boot diagnostics on VM %q", vmName) | ||
| BeforeAll(func(ctx context.Context) { |
|
Gerd Oberlechner (@geoberle): 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. |
What
The three admin API tests (breakglass, serial console, boot diagnostics) each provisioned their own HCP cluster independently. Wrap them in an Ordered Context with a shared BeforeAll that creates one cluster and one worker nodepool, eliminating two redundant cluster provisions per suite run.
Why
Testing
Testing is required for feature completion and tests should be part of the pull
request along with the feature changes.
Describe the testing provided. If you did not add tests, provide a clear
justification.
Special notes for your reviewer
PR Checklist
If E2E tests are included:
demonstrate that the test is able to detect a defect/error and fail with
proper error message and logs which communicates nature of the problem.