AUTOSCALE-166: fix yq dependency for running upstream karpenter tests#9060
AUTOSCALE-166: fix yq dependency for running upstream karpenter tests#9060fishereskew wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@fishereskew: This pull request references AUTOSCALE-166 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. |
openshift-ci
Bot
added
do-not-merge/needs-area
area/karpenter-operator
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@karpenter-operator/e2e/adjust-ec2nodeclass.sh`:
- Around line 10-13: Quote the repository-local yq executable expansion in both
invocations within the adjust-ec2nodeclass script: update the commands deleting
metadata fields and status to invoke "$YQ", preserving their existing
expressions and arguments.
🪄 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: 9ecb3a4e-ac4a-43bf-a273-6cb285d4f0f3
📒 Files selected for processing (2)
Makefilekarpenter-operator/e2e/adjust-ec2nodeclass.sh
| $YQ 'del(.metadata.creationTimestamp, .metadata.generation, .metadata.managedFields, .metadata.resourceVersion, .metadata.selfLink, .metadata.uid)' -i "$FILE" | ||
|
|
||
| # delete status | ||
| yq 'del(.status)' -i "$FILE" | ||
| $YQ 'del(.status)' -i "$FILE" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="karpenter-operator/e2e/adjust-ec2nodeclass.sh"
wc -l "$file"
cat -n "$file" | sed -n '1,120p'Repository: openshift/hypershift
Length of output: 698
Quote the repository-local yq path. $YQ is expanded unquoted, so karpenter-operator/e2e/adjust-ec2nodeclass.sh breaks when the checkout path contains whitespace or glob characters. Quote both invocations.
Proposed fix
-"$YQ" ...
+"$YQ" ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $YQ 'del(.metadata.creationTimestamp, .metadata.generation, .metadata.managedFields, .metadata.resourceVersion, .metadata.selfLink, .metadata.uid)' -i "$FILE" | |
| # delete status | |
| yq 'del(.status)' -i "$FILE" | |
| $YQ 'del(.status)' -i "$FILE" | |
| "$YQ" 'del(.metadata.creationTimestamp, .metadata.generation, .metadata.managedFields, .metadata.resourceVersion, .metadata.selfLink, .metadata.uid)' -i "$FILE" | |
| # delete status | |
| "$YQ" 'del(.status)' -i "$FILE" |
🤖 Prompt for 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.
In `@karpenter-operator/e2e/adjust-ec2nodeclass.sh` around lines 10 - 13, Quote
the repository-local yq executable expansion in both invocations within the
adjust-ec2nodeclass script: update the commands deleting metadata fields and
status to invoke "$YQ", preserving their existing expressions and arguments.
|
lgtm except for failing gitlint |
Signed-off-by: Fisher Eskew <feskew@redhat.com>
809e3ec to
e6b3d6d
Compare
|
/approve I thought changing makefile would require elevated approver rights, but I guess not? EDIT: never mind still does, I was just baited by the lone karpenter-operator label |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fishereskew, maxcao13 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 |
|
@fishereskew: 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9060 +/- ##
=======================================
Coverage 44.50% 44.50%
=======================================
Files 774 774
Lines 96980 96980
=======================================
Hits 43164 43164
Misses 50828 50828
Partials 2988 2988
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
3 participants
What this PR does / why we need it:
Updates the yq dependency for running upstream karpenter tests using the
karpenter-upstream-e2emake target to use the repo's yq.Which issue(s) this PR fixes:
Fixes issue related to AUTOSCALE-166.
Updates the make target and adjust-ec2nodeclass.sh script to use the repo's managed yq tool instead of relying on a manually downloaded yq.
Special notes for your reviewer:
Related PR for adding upstream regression tests to aws-karpenter-provider-aws: openshift/release#82258
Checklist:
Summary by CodeRabbit