HYPERFLEET-757 HYPERFLEET-758 - test: automate concurrent processing E2E tests#55
HYPERFLEET-757 HYPERFLEET-758 - test: automate concurrent processing E2E tests#55tzhou5 wants to merge 2 commits intoopenshift-hyperfleet:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis pull request adds two new end-to-end Ginkgo tests that exercise concurrent creation of clusters (5 concurrent requests) and nodepools (3 concurrent requests), updates test design docs to mark the tests as automated and to run nodepool creation requests in parallel, and changes a Helm sentinel invocation to set owner reference fields ( Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant API as Control Plane API
participant ClusterMgr as Cluster Controller
participant K8s as Kubernetes
participant AdapterSvc as Adapter Services
Note over TestRunner,API: Spawn N concurrent create requests
TestRunner->>API: POST /clusters (concurrent x5)
API->>ClusterMgr: Enqueue cluster creation tasks
ClusterMgr->>K8s: Create cluster-specific resources (namespaces, configmaps, etc.)
ClusterMgr->>AdapterSvc: Trigger adapter reconciliation
AdapterSvc->>ClusterMgr: Adapter status updates (Applied/Available/Health)
ClusterMgr->>API: Update cluster status (Ready/Available)
API-->>TestRunner: Return creation responses (IDs)
Note over TestRunner,API: Wait and poll until Ready and adapter statuses report success
sequenceDiagram
participant TestRunner
participant API as Control Plane API
participant ClusterMgr as Cluster Controller
participant K8s as Kubernetes
participant AdapterSvc as Adapter Services
Note over TestRunner,API: For each nodepool in parallel (x3)
TestRunner->>API: POST /clusters/{id}/nodepools (concurrent x3)
API->>ClusterMgr: Enqueue nodepool creation tasks
ClusterMgr->>K8s: Create nodepool-scoped resources (configmaps, labels)
ClusterMgr->>AdapterSvc: Trigger nodepool adapter reconciliation
AdapterSvc->>ClusterMgr: Nodepool adapter status updates
ClusterMgr->>API: Update nodepool status (Ready/Available)
API-->>TestRunner: Return nodepool creation responses (IDs)
Note over TestRunner,API: Wait and poll until each nodepool Ready and adapters report success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 `@e2e/nodepool/concurrent_creation.go`:
- Around line 179-199: The AfterEach cleanup currently only calls
h.CleanupTestCluster and may leave nodepool API resources orphaned; update the
ginkgo.AfterEach to explicitly iterate the collected nodepoolIDs and call the
handler's nodepool deletion method for each (e.g., use h.DeleteNodepool(ctx,
clusterID, nodepoolID) or the appropriate h method) before calling
h.CleanupTestCluster, checking errors with Expect(err).NotTo(HaveOccurred()) for
each deletion; keep the existing WaitForClusterCondition and CleanupTestCluster
calls but ensure nodepool deletions run first to remove nodepool API resources.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b19c53a6-4421-4e23-9ce3-39fabb8b8385
📒 Files selected for processing (4)
deploy-scripts/lib/sentinel.she2e/cluster/concurrent_creation.goe2e/nodepool/concurrent_creation.gotest-design/testcases/concurrent-processing.md
| ginkgo.AfterEach(func(ctx context.Context) { | ||
| if h == nil || clusterID == "" { | ||
| return | ||
| } | ||
|
|
||
| ginkgo.By("Verify final cluster state to ensure Ready before cleanup") | ||
| err := h.WaitForClusterCondition( | ||
| ctx, | ||
| clusterID, | ||
| client.ConditionTypeReady, | ||
| openapi.ResourceConditionStatusTrue, | ||
| h.Cfg.Timeouts.Cluster.Ready, | ||
| ) | ||
| if err != nil { | ||
| ginkgo.GinkgoWriter.Printf("WARNING: cluster %s did not reach Ready state before cleanup: %v\n", clusterID, err) | ||
| } | ||
|
|
||
| ginkgo.By("cleaning up test cluster " + clusterID) | ||
| err = h.CleanupTestCluster(ctx, clusterID) | ||
| Expect(err).NotTo(HaveOccurred(), "failed to cleanup cluster %s", clusterID) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what CleanupTestCluster does
ast-grep --pattern 'func ($_ *Helper) CleanupTestCluster($$$) {
$$$
}'Repository: openshift-hyperfleet/hyperfleet-e2e
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Search for CleanupTestCluster function definition
rg "CleanupTestCluster" -A 10 --type goRepository: openshift-hyperfleet/hyperfleet-e2e
Length of output: 4202
🏁 Script executed:
#!/bin/bash
# Also check the full concurrent_creation.go file to understand context
wc -l e2e/nodepool/concurrent_creation.goRepository: openshift-hyperfleet/hyperfleet-e2e
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Look at the full AfterEach and surrounding code in the test file
cat -n e2e/nodepool/concurrent_creation.go | head -220 | tail -60Repository: openshift-hyperfleet/hyperfleet-e2e
Length of output: 3028
Nodepool resources not explicitly cleaned up before cluster deletion.
The test creates and collects nodepoolIDs but the AfterEach only calls CleanupTestCluster, which is a temporary workaround that deletes the Kubernetes namespace using client-go—it does not delete nodepool API resources. Explicitly delete the created nodepools during cleanup to prevent orphaned API resources.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/nodepool/concurrent_creation.go` around lines 179 - 199, The AfterEach
cleanup currently only calls h.CleanupTestCluster and may leave nodepool API
resources orphaned; update the ginkgo.AfterEach to explicitly iterate the
collected nodepoolIDs and call the handler's nodepool deletion method for each
(e.g., use h.DeleteNodepool(ctx, clusterID, nodepoolID) or the appropriate h
method) before calling h.CleanupTestCluster, checking errors with
Expect(err).NotTo(HaveOccurred()) for each deletion; keep the existing
WaitForClusterCondition and CleanupTestCluster calls but ensure nodepool
deletions run first to remove nodepool API resources.
Summary
Add concurrent cluster creation test (5 clusters in parallel) and
concurrent nodepool creation test (3 nodepools under same cluster).
Both tests verify resource isolation and adapter status completion.
Fix sentinel nodepool deployment: owner_references must be set as
nested object fields (id, href, kind) instead of a single string,
matching the format used in hyperfleet-infra.
Test Plan
make test-allpassesmake lintpassesmake test-helm(if applicable)Summary by CodeRabbit
Tests
Chores