Skip to content

HYPERFLEET-757 HYPERFLEET-758 - test: automate concurrent processing E2E tests#55

Open
tzhou5 wants to merge 2 commits intoopenshift-hyperfleet:mainfrom
tzhou5:HYPERFLEET-757
Open

HYPERFLEET-757 HYPERFLEET-758 - test: automate concurrent processing E2E tests#55
tzhou5 wants to merge 2 commits intoopenshift-hyperfleet:mainfrom
tzhou5:HYPERFLEET-757

Conversation

@tzhou5
Copy link
Contributor

@tzhou5 tzhou5 commented Mar 24, 2026

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

  • Unit tests added/updated
  • make test-all passes
  • make lint passes
  • Helm chart changes validated with make test-helm (if applicable)
  • Deployed to a development cluster and verified (if Helm/config changes)
  • E2E tests passed (if cross-component or major changes)

Summary by CodeRabbit

  • Tests

    • Added end-to-end tests validating concurrent cluster creation handling (multiple simultaneous requests), including readiness verification, namespace isolation, and adapter status completion.
    • Added end-to-end tests validating concurrent nodepool creation within clusters (multiple simultaneous requests), ensuring resource isolation, readiness, uniqueness, and adapter status conditions.
  • Chores

    • Marked concurrent processing test cases as automated in documentation.

@openshift-ci openshift-ci bot requested review from aredenba-rh and vkareh March 24, 2026 05:52
@openshift-ci
Copy link

openshift-ci bot commented Mar 24, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign mischulee for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link

coderabbitai bot commented Mar 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 951d144c-3ef9-4bcc-9958-3e6a19f16b7a

📥 Commits

Reviewing files that changed from the base of the PR and between d1c93b5 and 62468e2.

📒 Files selected for processing (1)
  • e2e/cluster/concurrent_creation.go

Walkthrough

This 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 (id, href, kind) as separate --set arguments instead of using a single resource.owner_references assignment.

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
Loading
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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: automating concurrent processing E2E tests for both cluster and nodepool scenarios, which aligns with the substantial additions of two new test suites and test case updates.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 00fc4eb and d1c93b5.

📒 Files selected for processing (4)
  • deploy-scripts/lib/sentinel.sh
  • e2e/cluster/concurrent_creation.go
  • e2e/nodepool/concurrent_creation.go
  • test-design/testcases/concurrent-processing.md

Comment on lines +179 to +199
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)
})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 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 go

Repository: 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.go

Repository: 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 -60

Repository: 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant