Skip to content

OCPBUGS-84969: fix(e2e): wait for Karpenter node cleanup in parallel tests to prevent vCPU flake - #8414

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
bryan-cox:fix-karpenter-e2e-vcpu-flake
May 5, 2026
Merged

OCPBUGS-84969: fix(e2e): wait for Karpenter node cleanup in parallel tests to prevent vCPU flake#8414
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
bryan-cox:fix-karpenter-e2e-vcpu-flake

Conversation

@bryan-cox

@bryan-cox bryan-cox commented May 5, 2026

Copy link
Copy Markdown
Member

What this PR does / why we need it:

Fixes a 50% flake rate in TestKarpenter/Main/Billing_vCPUs,_consolidation,_and_cluster_deletion_with_blocking_PDB across all e2e-aws runs.

testCapacityReservation and testNodeClassVersionField provision Karpenter nodes but only rely on t.Cleanup for deletion without waiting for nodes to fully terminate. When the sequential testBillingConsolidationAndPDB runs next and asserts AutoNode.VCPUs=0, stale NodeClaims from these parallel tests cause the assertion to fail with AutoNode.VCPUs=4, want 0 (one lingering t3.xlarge = 4 vCPUs).

This adds explicit WaitForReadyNodesByLabels(..., 0, ...) calls after deleting workloads and NodePools, matching the pattern already used by testARM64Provisioning and testInstanceProfileAnnotation.

Which issue(s) this PR fixes:

6 out of 12 e2e-aws runs in the last 24h failed due to this flake, blocking unrelated PRs (#8385, #8396, #8404, #8409).

https://redhat.atlassian.net/browse/OCPBUGS-84969

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

  • Tests
    • Enhanced test flow to ensure explicit cleanup of managed resources and full node termination before running subsequent checks, preventing resource leakage and improving overall test reliability across multiple test scenarios.

…t vCPU flake

testCapacityReservation and testNodeClassVersionField provision Karpenter
nodes but only rely on t.Cleanup for deletion without waiting for nodes
to fully terminate. When the sequential testBillingConsolidationAndPDB
runs next and asserts AutoNode.VCPUs=0, stale NodeClaims from these
parallel tests cause a 50% flake rate across all e2e-aws runs.

Add explicit WaitForReadyNodesByLabels(..., 0, ...) calls after deleting
workloads and NodePools, matching the pattern already used by
testARM64Provisioning and testInstanceProfileAnnotation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 5, 2026
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 5, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@bryan-cox: This pull request explicitly references no jira issue.

Details

In response to this:

What this PR does / why we need it:

Fixes a 50% flake rate in TestKarpenter/Main/Billing_vCPUs,_consolidation,_and_cluster_deletion_with_blocking_PDB across all e2e-aws runs.

testCapacityReservation and testNodeClassVersionField provision Karpenter nodes but only rely on t.Cleanup for deletion without waiting for nodes to fully terminate. When the sequential testBillingConsolidationAndPDB runs next and asserts AutoNode.VCPUs=0, stale NodeClaims from these parallel tests cause the assertion to fail with AutoNode.VCPUs=4, want 0 (one lingering t3.xlarge = 4 vCPUs).

This adds explicit WaitForReadyNodesByLabels(..., 0, ...) calls after deleting workloads and NodePools, matching the pattern already used by testARM64Provisioning and testInstanceProfileAnnotation.

Which issue(s) this PR fixes:

6 out of 12 e2e-aws runs in the last 24h failed due to this flake, blocking unrelated PRs (#8385, #8396, #8404, #8409).

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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.

@openshift-ci

openshift-ci Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@coderabbitai

coderabbitai Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The test file test/e2e/karpenter_test.go was modified to include explicit cleanup steps in two test paths. In the version field test scenario, after validating EC2 instance metadata options, the test now actively deletes the test workloads and node pool, then waits for node termination. Similarly, in the capacity reservation test scenario, after verifying instance placement, the test deletes the workload and node pool and waits for provisioned nodes to fully disappear. These changes ensure deterministic cleanup completion before proceeding to subsequent test operations.

🚥 Pre-merge checks | ✅ 11 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (11 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed All test names are static strings with no dynamic identifiers. No pod names, timestamps, UUIDs, node names, namespaces, IPs, or changing values found. PR changes only add cleanup logic.
Test Structure And Quality ✅ Passed Tests meet all quality requirements: proper single responsibility, cleanup with explicit waits, timeouts configured, meaningful assertions, and consistency with codebase patterns.
Microshift Test Compatibility ✅ Passed PR modifies existing e2e tests (not adding new ones). Only test entry point has explicit AWS platform skip guard, preventing MicroShift execution.
Single Node Openshift (Sno) Test Compatibility ✅ Passed This PR modifies tests using Go's standard testing package (t.Run), not Ginkgo (It/Describe/Context). The custom check applies only to Ginkgo e2e tests, so this check does not apply.
Topology-Aware Scheduling Compatibility ✅ Passed This PR modifies only e2e test code. It does not add deployment manifests, operator code, controllers, or scheduling constraints affecting topology compatibility (SNO, TNF, TNA, HyperShift).
Ote Binary Stdout Contract ✅ Passed Changes are inside test function closures only. No fmt/log/klog stdout writes. No process-level code modified. No OTE Binary Stdout Contract violations.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo tests added. Changes modify only existing test functions with cleanup improvements. Custom check applies to new tests only.
Title check ✅ Passed The title clearly and specifically describes the main change: fixing Karpenter node cleanup in parallel e2e tests to prevent a vCPU flake, which directly matches the file changes that add explicit node termination waits.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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.

@openshift-ci

openshift-ci Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bryan-cox

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

The pull request process is described 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

@openshift-ci openshift-ci Bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. area/testing Indicates the PR includes changes for e2e testing and removed do-not-merge/needs-area labels May 5, 2026
@bryan-cox
bryan-cox marked this pull request as ready for review May 5, 2026 00:11
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 5, 2026
@openshift-ci
openshift-ci Bot requested review from cblecker and clebs May 5, 2026 00:12
@bryan-cox bryan-cox changed the title NO-JIRA: fix(e2e): wait for Karpenter node cleanup in parallel tests to prevent vCPU flake OCPBUGS-84969: fix(e2e): wait for Karpenter node cleanup in parallel tests to prevent vCPU flake May 5, 2026
@openshift-ci-robot openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label May 5, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@bryan-cox: This pull request references Jira Issue OCPBUGS-84969, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.0.0) matches configured target version for branch (5.0.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

What this PR does / why we need it:

Fixes a 50% flake rate in TestKarpenter/Main/Billing_vCPUs,_consolidation,_and_cluster_deletion_with_blocking_PDB across all e2e-aws runs.

testCapacityReservation and testNodeClassVersionField provision Karpenter nodes but only rely on t.Cleanup for deletion without waiting for nodes to fully terminate. When the sequential testBillingConsolidationAndPDB runs next and asserts AutoNode.VCPUs=0, stale NodeClaims from these parallel tests cause the assertion to fail with AutoNode.VCPUs=4, want 0 (one lingering t3.xlarge = 4 vCPUs).

This adds explicit WaitForReadyNodesByLabels(..., 0, ...) calls after deleting workloads and NodePools, matching the pattern already used by testARM64Provisioning and testInstanceProfileAnnotation.

Which issue(s) this PR fixes:

6 out of 12 e2e-aws runs in the last 24h failed due to this flake, blocking unrelated PRs (#8385, #8396, #8404, #8409).

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

  • Tests
  • Enhanced test flow to ensure explicit cleanup of managed resources and full node termination before running subsequent checks, preventing resource leakage and improving overall test reliability across multiple test scenarios.

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.

@openshift-ci-robot

Copy link
Copy Markdown

@bryan-cox: This pull request references Jira Issue OCPBUGS-84969, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.0.0) matches configured target version for branch (5.0.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
Details

In response to this:

What this PR does / why we need it:

Fixes a 50% flake rate in TestKarpenter/Main/Billing_vCPUs,_consolidation,_and_cluster_deletion_with_blocking_PDB across all e2e-aws runs.

testCapacityReservation and testNodeClassVersionField provision Karpenter nodes but only rely on t.Cleanup for deletion without waiting for nodes to fully terminate. When the sequential testBillingConsolidationAndPDB runs next and asserts AutoNode.VCPUs=0, stale NodeClaims from these parallel tests cause the assertion to fail with AutoNode.VCPUs=4, want 0 (one lingering t3.xlarge = 4 vCPUs).

This adds explicit WaitForReadyNodesByLabels(..., 0, ...) calls after deleting workloads and NodePools, matching the pattern already used by testARM64Provisioning and testInstanceProfileAnnotation.

Which issue(s) this PR fixes:

6 out of 12 e2e-aws runs in the last 24h failed due to this flake, blocking unrelated PRs (#8385, #8396, #8404, #8409).

https://redhat.atlassian.net/browse/OCPBUGS-84969

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

  • Tests
  • Enhanced test flow to ensure explicit cleanup of managed resources and full node termination before running subsequent checks, preventing resource leakage and improving overall test reliability across multiple test scenarios.

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.

@codecov

codecov Bot commented May 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 37.23%. Comparing base (ad88854) to head (8e84889).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8414   +/-   ##
=======================================
  Coverage   37.23%   37.23%           
=======================================
  Files         752      752           
  Lines       91829    91829           
=======================================
  Hits        34195    34195           
  Misses      54993    54993           
  Partials     2641     2641           
Flag Coverage Δ
cmd-support 32.06% <ø> (ø)
cpo-hostedcontrolplane 36.50% <ø> (ø)
cpo-other 37.73% <ø> (ø)
hypershift-operator 47.85% <ø> (ø)
other 27.77% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bryan-cox

Copy link
Copy Markdown
Member Author

/test e2e-aws

@bryan-cox

Copy link
Copy Markdown
Member Author

/verified by e2e

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label May 5, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@bryan-cox: This PR has been marked as verified by e2e.

Details

In response to this:

/verified by e2e

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.

@joshbranham joshbranham left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 5, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage.

@bryan-cox

Copy link
Copy Markdown
Member Author

/pipeline auto

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification

The pipeline-auto label has been added to this PR. Second-stage tests will be triggered automatically when all first-stage tests pass.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage.

@bryan-cox

Copy link
Copy Markdown
Member Author

/pipeline required

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks-4-22
/test e2e-aws-4-22
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-azure-self-managed
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 0ca85b1 and 2 for PR HEAD 8e84889 in total

@cwbotbot

cwbotbot commented May 5, 2026

Copy link
Copy Markdown

Test Results

e2e-aws

e2e-aks

@bryan-cox

Copy link
Copy Markdown
Member Author

/retest

@bryan-cox

bryan-cox commented May 5, 2026

Copy link
Copy Markdown
Member Author

Confirmed from the e2e-aws build log that both tests waited for nodes to fully terminate before the billing test ran:

testCapacityReservation — waited 1m39s for nodes to drain:

=== NAME  TestKarpenter/Main/Parallel_provisioning_tests/Capacity_reservation_selector_propagation
    util.go:571: Successfully waited for 0 nodes to become ready in 1m39s

testNodeClassVersionField — waited 1m36s for nodes to drain:

=== NAME  TestKarpenter/Main/Parallel_provisioning_tests/OpenshiftEC2NodeClass_version_field_and_MetadataOptions
    util.go:571: Successfully waited for 0 nodes to become ready in 1m36s

testBillingConsolidationAndPDB then ran with a clean slate — AutoNode.VCPUs=0 converged immediately (0s), confirming no stale NodeClaims leaked:

=== RUN   TestKarpenter/Main/Billing_vCPUs,_consolidation,_and_cluster_deletion_with_blocking_PDB
    karpenter_test.go:1253: Validating AutoNode.VCPUs converges to 0
    karpenter_test.go:1253: Successfully waited for HostedCluster e2e-clusters-kzpsb/karpenter-tj4k9 AutoNode.VCPUs=0 in 0s

All Karpenter subtests passed, including the previously flaky billing test:

--- PASS: TestKarpenter/Main/Parallel_provisioning_tests/Capacity_reservation_selector_propagation (455.93s)
--- PASS: TestKarpenter/Main/Parallel_provisioning_tests/OpenshiftEC2NodeClass_version_field_and_MetadataOptions (420.88s)
--- PASS: TestKarpenter/Main/Billing_vCPUs,_consolidation,_and_cluster_deletion_with_blocking_PDB (777.35s)

@openshift-ci

openshift-ci Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

@bryan-cox: you cannot LGTM your own PR.

Details

In response to this:

Confirmed that both test functions now properly wait for node termination before proceeding:

  1. testNodeClassVersionField — after deleting testWorkLoads and testNodePool, calls WaitForReadyNodesByLabels(..., 0, testNodeLabels) to block until all nodes with those labels are fully terminated.

  2. testCapacityReservation — after deleting crWorkload and crNodePool, calls WaitForReadyNodesByLabels(..., 0, crNodeLabels) to do the same.

Both use a target count of 0, ensuring no stale NodeClaims can leak vCPUs into the subsequent sequential testBillingConsolidationAndPDB. This matches the existing cleanup pattern already used by testARM64Provisioning and testInstanceProfileAnnotation.

/lgtm

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 kubernetes-sigs/prow repository.

@openshift-ci

openshift-ci Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

@bryan-cox: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot
openshift-merge-bot Bot merged commit e09cc2d into openshift:main May 5, 2026
40 checks passed
@openshift-ci-robot

Copy link
Copy Markdown

@bryan-cox: Jira Issue Verification Checks: Jira Issue OCPBUGS-84969
✔️ This pull request was pre-merge verified.
✔️ All associated pull requests have merged.
✔️ All associated, merged pull requests were pre-merge verified.

Jira Issue OCPBUGS-84969 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓

Details

In response to this:

What this PR does / why we need it:

Fixes a 50% flake rate in TestKarpenter/Main/Billing_vCPUs,_consolidation,_and_cluster_deletion_with_blocking_PDB across all e2e-aws runs.

testCapacityReservation and testNodeClassVersionField provision Karpenter nodes but only rely on t.Cleanup for deletion without waiting for nodes to fully terminate. When the sequential testBillingConsolidationAndPDB runs next and asserts AutoNode.VCPUs=0, stale NodeClaims from these parallel tests cause the assertion to fail with AutoNode.VCPUs=4, want 0 (one lingering t3.xlarge = 4 vCPUs).

This adds explicit WaitForReadyNodesByLabels(..., 0, ...) calls after deleting workloads and NodePools, matching the pattern already used by testARM64Provisioning and testInstanceProfileAnnotation.

Which issue(s) this PR fixes:

6 out of 12 e2e-aws runs in the last 24h failed due to this flake, blocking unrelated PRs (#8385, #8396, #8404, #8409).

https://redhat.atlassian.net/browse/OCPBUGS-84969

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

  • Tests
  • Enhanced test flow to ensure explicit cleanup of managed resources and full node termination before running subsequent checks, preventing resource leakage and improving overall test reliability across multiple test scenarios.

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.

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/testing Indicates the PR includes changes for e2e testing jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. pipeline-auto verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants