AUTOSCALE-681: various karpenter and karpenterupgrade test fixes#8510
AUTOSCALE-681: various karpenter and karpenterupgrade test fixes#8510maxcao13 wants to merge 3 commits into
Conversation
waitForReadyKarpenterPods listed all pods in the default namespace, which picked up pods from other parallel subtests. This caused count mismatches and wrong-node assertions. Filter by the workload's app label so each test only sees its own pods. Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Max Cao <macao@redhat.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Skipping CI for Draft Pull Request. |
|
@maxcao13: This pull request references AUTOSCALE-681 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 task 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. |
📝 WalkthroughWalkthroughThe pull request updates two Karpenter E2E test files: the control-plane upgrade test now uses Sequence Diagram(s)Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 10 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/test e2e-aws |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8510 +/- ##
=======================================
Coverage 40.00% 40.00%
=======================================
Files 751 751
Lines 92838 92838
=======================================
Hits 37137 37137
Misses 53014 53014
Partials 2687 2687
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/e2e/karpenter_test.go`:
- Around line 318-320: The ARM64 e2e workload is using a public registry image
("registry.k8s.io/pause:3.10") in the armWorkLoads created via
testWorkloadWithImage, which introduces external network dependency; update the
image argument passed to testWorkloadWithImage for the "arm-app" workload (and
any related armNodePool usage) to a CI-backed/mirrored multi-arch image hosted
on our internal registry (or a known-mirrored-by-CI image) that supports arm64
so the test no longer pulls from the public internet; ensure the chosen image is
multi-arch and referenced consistently in the test setup.
🪄 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: Enterprise
Run ID: 98e58f21-8bf9-43d3-9337-e13ead63f350
📒 Files selected for processing (2)
test/e2e/karpenter_control_plane_upgrade_test.gotest/e2e/karpenter_test.go
The quay.io/openshift/origin-pod manifest was is not multi-arch and run on arm nodes. Though, it can schedule. We use quay.io/hypershift/sleep:multiarch specifically for the arm test. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Max Cao <macao@redhat.com>
80fe16f to
9b68e6a
Compare
|
/test e2e-aws |
Test Resultse2e-aws
|
|
@maxcao13: The following test 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. |
|
/approve feel free to cancel afterwards with the test passing |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre, maxcao13 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 |
What this PR does / why we need it:
Scope
waitForReadyKarpenterPodsto workload labels: The function listed all pods in the default namespace, which picked up pods from other parallel subtests running concurrently. This caused count mismatches and wrong-node assertions. Now each caller passes its workload's app label so it only sees its own pods.Use correct version gate for
TestKarpenterUpgradeControlPlane: Switch fromAtLeast(Version422)toShouldRunKarpenterTests, which is the standard gate used by the other karpenter tests and respects theRUN_KARPENTER_TESTSenv var.Use arm-compatible image in ARM64 test:
quay.io/openshift/origin-podis amd64-only and can't actually run on arm nodes. Usequay.io/hypershift/sleep:multiarch(multi-arch) for the arm test via a newtestWorkloadWithImagehelper.Which issue(s) this PR fixes:
Fixes
Special notes for your reviewer:
I was unaware of #8466 when I merged #8498. This PR reconciles that.
Checklist:
Summary by CodeRabbit