CNTRLPLANE-3040: add regression tests for CAS pause/unpause replica clamping#8262
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
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:
📝 WalkthroughWalkthroughAdded a unit test Sequence Diagram(s)sequenceDiagram
participant Test as Test Harness
participant NP as NodePool
participant CAPI as CAPI (MachineDeployment / MachineSet)
participant Work as Workload
participant CA as Cluster Autoscaler
participant Cluster as Cluster Nodes
Test->>NP: enable autoscaling & set pausedUntil / pause
Test->>Work: create workload to increase load
Work->>Cluster: create pods -> triggers scale up
Cluster->>CAPI: increase Spec.Replicas (autoscaler effect)
Test->>CAPI: verify PausedAnnotation present
Test->>CA: ensure CA ready (aggressive timers)
Test->>Work: delete workload
CA->>CA: evaluate nodegroups
CA->>CAPI: detect paused node group (logs "discovered a paused node group")
Note right of CAPI: while paused, Spec.Replicas remains unchanged
Test->>NP: remove pausedUntil / unpause
Test->>CAPI: trigger reconcile -> unpause (remove annotation)
CAPI->>CAPI: clamp Spec.Replicas to [effectiveMin, autoscalingMax]
CAPI->>Cluster: adjust node count
Cluster->>Test: converge to expected numNodes
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
area/hypershift-operator
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jparrill 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/autoscaling_test.go`:
- Around line 551-592: The test incorrectly requires the MachineDeployment
replica count to equal replicasBeforePause and waits for `max` ready nodes
immediately after unpause; remove the strict equality check comparing
`replicasAfterUnpause` to `replicasBeforePause` and drop the immediate
`e2eutil.WaitForNReadyNodes(..., max, ...)` assertion. Instead, after confirming
the MachineDeployment is unpaused (the existing `EventuallyObject` block), log
the current replica count if desired and proceed directly to wait for
convergence back to `numNodes` using `e2eutil.WaitForNReadyNodes(t, ctx,
guestClient, numNodes, hostedCluster.Spec.Platform.Type)` (the existing Step 9),
ensuring the test only asserts eventual scale-down to `numNodes` and not an
immediate preservation of `max`. Reference symbols: `replicasAfterUnpause`,
`replicasBeforePause`, `max`, `numNodes`, and `e2eutil.WaitForNReadyNodes`.
- Around line 438-493: This subtest never enables autoscaling on the NodePool
returned by getFirstNodePool, so make the test explicitly enable autoscaling and
clear fixed replicas before waiting for scale-up: fetch the NodePool (nodepool),
set nodepool.Spec.AutoScaling to the desired hyperv1.NodePoolAutoScaling config
and set nodepool.Spec.Replicas = nil (or remove fixed replicas), then persist
the change with mgtClient.Update inside a retry.RetryOnConflict block (use
crclient.ObjectKeyFromObject(nodepool) to refetch) so the NodePool is
independently configured for autoscaling even if other subtests are not run or
reordered.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 32012c66-572c-455c-ac17-d0c2ff29d53a
📒 Files selected for processing (2)
hypershift-operator/controllers/nodepool/capi_test.gotest/e2e/autoscaling_test.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8262 +/- ##
==========================================
- Coverage 39.85% 39.85% -0.01%
==========================================
Files 751 751
Lines 92554 92556 +2
==========================================
- Hits 36889 36888 -1
- Misses 52992 52994 +2
- Partials 2673 2674 +1 see 1 file with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
jparrill
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@jparrill: This pull request references CNTRLPLANE-3040 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. |
jparrill
force-pushed
the
CNTRLPLANE-3040
branch
from
1da08b9 to
dca8bd9
Compare
|
@jparrill: This pull request references CNTRLPLANE-3040 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. |
jparrill
force-pushed
the
CNTRLPLANE-3040
branch
from
dca8bd9 to
32754b4
Compare
|
@jparrill: This pull request references CNTRLPLANE-3040 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. |
|
/test e2e-aws |
There was a problem hiding this comment.
Solid regression test coverage overall. The 9 table-driven unit cases systematically hit every branch of setMachineDeploymentReplicas and both MachineDeployment/MachineSet code paths. The DRY refactors are clean.
testAutoscaling and testAutoscalerRespectsNodePoolPause
As-is, adding testAutoscalerRespectsNodePoolPause as a separate subtest has significant cost:
- ~10 extra minutes of CI wall time — a full scale-up/down cycle on real AWS infrastructure, duplicating what
testAutoscalingalready does - ~10 extra minutes of EC2 spend per run — provisioning and draining
maxnodes a second time - Doubles the flake surface — two independent scale-up/down cycles means two chances for node provisioning timeouts, API throttling, or slow kubelet registration to cause spurious failures
- Multiplied across every PR — this runs on every PR that triggers the autoscaling e2e job
The two subtests follow very similar flows:
testAutoscaling (existing):
- Enable autoscaling (min=numNodes, max=numNodes+2)
- Create workload → scale up to max
- Delete workload → verify scale-down to numNodes
testAutoscalerRespectsNodePoolPause (new):
- Enable autoscaling (same min/max)
- Configure aggressive CAS timers
- Create workload → scale up to max
- Pause → delete workload → verify NO scale-down
- Unpause → verify scale-down to numNodes
Steps 1-3 and the final scale-down are essentially duplicated. A merged flow could be:
- Enable autoscaling + configure aggressive CAS timers
- Create workload → scale up to max (proves scale-up works)
- Pause NodePool → delete workload → verify replicas unchanged (proves pause blocks CAS)
- Unpause → verify scale-down to numNodes (proves scale-down works after unpause)
One scale-up/down cycle instead of two. Same coverage, tells a stronger story: "CAS scales up, respects pause, and resumes scale-down cleanly" as one coherent lifecycle.
The tradeoff is fault isolation — if the pause logic breaks, a merged test would also mask whether basic scale-up/down still works. That said, the unit tests already cover the controller logic thoroughly, so the e2e is really validating the CAS integration end-to-end where the merged flow makes more sense.
Review generated with Claude Code
Test Resultse2e-aws
|
jparrill
force-pushed
the
CNTRLPLANE-3040
branch
from
32754b4 to
7396132
Compare
|
@jparrill: This pull request references CNTRLPLANE-3040 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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e/autoscaling_test.go (1)
420-478: Make the log-poll helper honorctx.Done().The raw sleeps here keep waiting even after the subtest has been canceled, which can burn the full three-minute window on failure paths.
♻️ Suggested direction
cfg, err := e2eutil.GetConfig() if err != nil { t.Logf("Failed to get management cluster config for CAS log polling, falling back to timeout: %v", err) - time.Sleep(timeout) + select { + case <-ctx.Done(): + case <-time.After(timeout): + } return false } kubeClient, err := kubeclient.NewForConfig(cfg) if err != nil { t.Logf("Failed to create kubeclient for CAS log polling, falling back to timeout: %v", err) - time.Sleep(timeout) + select { + case <-ctx.Done(): + case <-time.After(timeout): + } return false } @@ pods, err := kubeClient.CoreV1().Pods(controlPlaneNamespace).List(ctx, metav1.ListOptions{ LabelSelector: "app=cluster-autoscaler", }) if err != nil || len(pods.Items) == 0 { - time.Sleep(casLogPollInterval) + select { + case <-ctx.Done(): + return false + case <-time.After(casLogPollInterval): + } continue } @@ stream, err := kubeClient.CoreV1().Pods(controlPlaneNamespace).GetLogs(pods.Items[0].Name, logOpts).Stream(ctx) if err != nil { - time.Sleep(casLogPollInterval) + select { + case <-ctx.Done(): + return false + case <-time.After(casLogPollInterval): + } continue } @@ - time.Sleep(casLogPollInterval) + select { + case <-ctx.Done(): + return false + case <-time.After(casLogPollInterval): + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/autoscaling_test.go` around lines 420 - 478, pollCASLogsForPausedNodeGroup currently uses time.Sleep calls that ignore ctx cancellation; update the function so every sleep or blocking wait honors ctx.Done() (e.g., replace time.Sleep(timeout) and time.Sleep(casLogPollInterval) with a select that returns early on <-ctx.Done(), and when waiting for the pod log stream use context-aware GetLogs(...).Stream(ctx) already accepts ctx but also ensure the scanner/reading loop breaks if ctx is done); specifically modify the error/fallback branches and the loop waiting on casLogPollInterval to select between time.After(casLogPollInterval) and <-ctx.Done(), and return false (or exit) immediately when ctx is cancelled so pollCASLogsForPausedNodeGroup stops promptly.
🤖 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/autoscaling_test.go`:
- Around line 576-585: After successfully pausing the NodePool in the
retry.RetryOnConflict block, add a deferred cleanup that will unpause the same
nodepool to avoid leaving the shared fixture paused; implement the defer to
fetch the latest NodePool (using mgtClient.Get or the same retry.RetryOnConflict
pattern), clear nodepool.Spec.PausedUntil (set to nil/""), and update it
(mgtClient.Update) with retries on conflict to ensure the unpause runs even if
the test fails before the explicit unpause at Line 637; reference the same
symbols nodepool, mgtClient.Get, mgtClient.Update, and retry.RetryOnConflict to
locate where to insert the defer.
---
Nitpick comments:
In `@test/e2e/autoscaling_test.go`:
- Around line 420-478: pollCASLogsForPausedNodeGroup currently uses time.Sleep
calls that ignore ctx cancellation; update the function so every sleep or
blocking wait honors ctx.Done() (e.g., replace time.Sleep(timeout) and
time.Sleep(casLogPollInterval) with a select that returns early on <-ctx.Done(),
and when waiting for the pod log stream use context-aware
GetLogs(...).Stream(ctx) already accepts ctx but also ensure the scanner/reading
loop breaks if ctx is done); specifically modify the error/fallback branches and
the loop waiting on casLogPollInterval to select between
time.After(casLogPollInterval) and <-ctx.Done(), and return false (or exit)
immediately when ctx is cancelled so pollCASLogsForPausedNodeGroup stops
promptly.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 90440ea1-460c-4d9b-90ce-ace734345bb7
📒 Files selected for processing (2)
hypershift-operator/controllers/nodepool/capi_test.gotest/e2e/autoscaling_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- hypershift-operator/controllers/nodepool/capi_test.go
jparrill
force-pushed
the
CNTRLPLANE-3040
branch
from
7396132 to
98e69f1
Compare
|
@jparrill: This pull request references CNTRLPLANE-3040 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. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/e2e/autoscaling_test.go (1)
569-578:⚠️ Potential issue | 🟠 MajorAdd a deferred unpause cleanup to prevent fixture corruption.
If any assertion fails between pause (line 577) and unpause (line 637), the shared
NodePoolremains paused and subsequent sibling subtests (e.g.,TestAutoscalingBalancing) inherit a broken fixture. Add a deferred cleanup immediately after the pause succeeds.🛡️ Recommended safeguard
err = retry.RetryOnConflict(retry.DefaultRetry, func() error { if err := mgtClient.Get(ctx, crclient.ObjectKeyFromObject(nodepool), nodepool); err != nil { return err } nodepool.Spec.PausedUntil = ptr.To("true") return mgtClient.Update(ctx, nodepool) }) g.Expect(err).NotTo(HaveOccurred(), "failed to pause NodePool") t.Logf("Paused NodePool %s", nodepool.Name) + + // Ensure unpause on test failure to avoid breaking subsequent subtests. + defer func() { + cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cleanupCancel() + _ = retry.RetryOnConflict(retry.DefaultRetry, func() error { + if err := mgtClient.Get(cleanupCtx, crclient.ObjectKeyFromObject(nodepool), nodepool); err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return err + } + if nodepool.Spec.PausedUntil == nil { + return nil // Already unpaused + } + nodepool.Spec.PausedUntil = nil + return mgtClient.Update(cleanupCtx, nodepool) + }) + }()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/autoscaling_test.go` around lines 569 - 578, After successfully pausing the NodePool, add a deferred cleanup that unpauses it to avoid leaving the shared fixture paused if the test fails; specifically, immediately after the block that sets nodepool.Spec.PausedUntil = ptr.To("true") and checks err, add a defer that uses retry.RetryOnConflict (same pattern) with mgtClient.Get + set nodepool.Spec.PausedUntil = nil (or remove the pause flag) and mgtClient.Update to restore the NodePool, logging failures but not crashing the test harness; reference the existing nodepool variable, mgtClient, and retry.RetryOnConflict to implement this cleanup.
🧹 Nitpick comments (1)
hypershift-operator/controllers/nodepool/capi_test.go (1)
2541-2548: Consider using platform-appropriate templates for KubeVirt test cases.Using
capiaws.AWSMachineTemplatefor KubeVirt platform test cases (lines 2379-2387 and 2419-2427) is technically inconsistent, though it works because the test focuses on replica clamping logic rather than infrastructure references.♻️ Optional: use KubeVirt template for non-AWS platforms
+ var template client.Object + if tc.platformType == hyperv1.KubevirtPlatform { + template = &capikubevirt.KubevirtMachineTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-template", + Namespace: controlPlaneNamespace, + }, + } + } else { + template = &capiaws.AWSMachineTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-template", + Namespace: controlPlaneNamespace, + }, + } + } - template := &capiaws.AWSMachineTemplate{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-template", - Namespace: controlPlaneNamespace, - }, - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hypershift-operator/controllers/nodepool/capi_test.go` around lines 2541 - 2548, The test uses capiaws.AWSMachineTemplate even for KubeVirt-platform cases; update those KubeVirt-specific test instances to use the corresponding KubeVirt template type (e.g., capikubevirt.KubevirtMachineTemplate) instead of capiaws.AWSMachineTemplate so the infrastructure objects match the platform under test—locate the declarations named template in capi_test.go and replace the AWSMachineTemplate type with the KubeVirt machine template type, keeping the same metadata setup and test logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/e2e/autoscaling_test.go`:
- Around line 569-578: After successfully pausing the NodePool, add a deferred
cleanup that unpauses it to avoid leaving the shared fixture paused if the test
fails; specifically, immediately after the block that sets
nodepool.Spec.PausedUntil = ptr.To("true") and checks err, add a defer that uses
retry.RetryOnConflict (same pattern) with mgtClient.Get + set
nodepool.Spec.PausedUntil = nil (or remove the pause flag) and mgtClient.Update
to restore the NodePool, logging failures but not crashing the test harness;
reference the existing nodepool variable, mgtClient, and retry.RetryOnConflict
to implement this cleanup.
---
Nitpick comments:
In `@hypershift-operator/controllers/nodepool/capi_test.go`:
- Around line 2541-2548: The test uses capiaws.AWSMachineTemplate even for
KubeVirt-platform cases; update those KubeVirt-specific test instances to use
the corresponding KubeVirt template type (e.g.,
capikubevirt.KubevirtMachineTemplate) instead of capiaws.AWSMachineTemplate so
the infrastructure objects match the platform under test—locate the declarations
named template in capi_test.go and replace the AWSMachineTemplate type with the
KubeVirt machine template type, keeping the same metadata setup and test logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: baa9ddf1-0440-4794-8cf5-c3a9b4754469
📒 Files selected for processing (2)
hypershift-operator/controllers/nodepool/capi_test.gotest/e2e/autoscaling_test.go
|
/test e2e-aws |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
openshift-ci
Bot
added
the
area/documentation
jparrill
force-pushed
the
CNTRLPLANE-3040
branch
2 times, most recently
from
ba544cb to
09ad081
Compare
|
Rebased |
…amping Add unit and E2E tests for OCPBUGS-78152 / CNTRLPLANE-3040 to prevent CAS from accumulating replica decrements on paused MachineDeployments. Unit tests: 9 table-driven cases covering replica clamping for both MachineDeployment and MachineSet upgrade types, including non-AWS effectiveMin=1 enforcement. E2E test: verifies CAS respects pause annotation by polling CAS logs for paused node group detection, with deferred unpause cleanup. Extracts shared helpers and constants to reduce duplication across autoscaling tests. Refs: OCPBUGS-78152, CNTRLPLANE-3040 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
|
Rebased again |
|
/lgtm |
openshift-ci
Bot
added
the
do-not-merge/hold
|
Scheduling tests matching the |
| // - When reconcileMachineDeployment/reconcileMachineSet runs after unpause, the | ||
| // pause annotation is removed and autoscaler annotations are preserved | ||
| // - setMachineDeploymentReplicas clamps replicas within [min, max] bounds regardless | ||
| // of external modifications (e.g. CAS decrementing replicas while paused) |
There was a problem hiding this comment.
isn't all this really effectively the same than adding a few test cases for TestSetMachineDeploymentReplicas and TestSetMachineSetReplicas?
There was a problem hiding this comment.
Fair point — the clamping logic itself is the same. The difference is that TestPauseUnpauseCycle exercises the full reconcile flow end-to-end:
Pause annotation set on NodePool
│
▼
External replica modification (CAS decrements while paused)
│
▼
reconcileMachineDeployment / reconcileMachineSet
│
▼
Pause annotation removed
│
▼
Replicas clamped back to [min, max]
Adding cases to TestSetMachineDeploymentReplicas/TestSetMachineSetReplicas would cover the clamping in isolation, but wouldn't validate that all the pieces work together through the reconcile cycle. I think both are worth keeping — unit cases for the function, integration-style test for the flow.
| } | ||
|
|
||
| const ( | ||
| aggressiveDelayAfterAddSeconds int32 = 30 |
There was a problem hiding this comment.
is this setting aggressive config for existing tests?
There was a problem hiding this comment.
Right — from the point this test runs until the end of the autoscaling suite, the aggressive policy stays set on the HostedCluster. I could restore the original config after the test, but is it worth it? The e2e tests are passing with the aggressive timers and we're saving wall time on scale-down waits for the remaining subtests. What do you think?
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
/retest-required |
|
/verified by e2e |
openshift-ci-robot
added
the
verified
|
@jparrill: 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. |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
I now have all the evidence needed. The picture is clear. Here is the final report: Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryNone of the 10 test failures are related to PR #8262. The PR adds unit tests in Root CauseAll 10 failures decompose into 4 independent root causes, none related to the PR: 1. TestKarpenterUpgradeControlPlane/Main — After the control plane upgrade completed and Karpenter detected drift on NodeClaim 2. TestNodePool/HostedCluster0/Main/TestNTOMachineConfigAppliedInPlace — The NTO MachineConfig in-place rollout test created a 2-replica NodePool, applied a Tuned configuration (config update completed in 4m20s), but only 1 of 2 verification DaemonSet pods reached Ready state after 15 minutes. The management cluster was under significant scheduling pressure with 310 FailedScheduling events due to anti-affinity rules and too-many-pods conditions across the 6 management nodes running many parallel hosted clusters. The parent test 3. TestCreateClusterPrivateWithRouteKAS/Teardown — The hosted cluster 4. TestKarpenter/Teardown — Same class of issue as #3 but with 10 remaining AWS resources spanning multiple Karpenter NodePools (version-test, kubelet-config-test, instance-profile-test, on-demand, capacity-reservation-test, arbitrary-subnet-test). Teardown took ~25 minutes. The bastion destruction also failed because the HostedCluster CR was already deleted. Recommendations
Evidence
|
|
@jparrill: 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. |
6 participants
Summary
getFirstNodePool,waitForAutoscalerDeploymentReady) and constants to reduce duplication across autoscaling testsChanges
Unit tests (
capi_test.go)TestPauseUnpauseCycle— 9 table-driven test cases:min=0→effectiveMin=1enforcement (KubeVirt)E2E test (
autoscaling_test.go)testAutoscalerRespectsNodePoolPause— placed betweentestAutoscalingandtestAutoscalingBalancing:DRY improvements
getFirstNodePool()— replaces 5-line list+assert pattern in 3 test functionswaitForAutoscalerDeploymentReady()— replaces 15-line EventuallyObject pattern in 2 test functionsaggressiveDelayAfterAddSeconds,aggressiveUnneededDurationSeconds,casScaleDownWaitDurationcapiv1.PausedAnnotationconstant instead of string literalTest plan
go vetpasses on both packagesTestPauseUnpauseCycle— 9/9 cases PASSgo vet -tags e2e ./test/e2e/)Refs: OCPBUGS-78152, CNTRLPLANE-3040
🤖 Generated with Claude Code
Summary by CodeRabbit