hypershift: Add AWS v2 lifecycle step-registry components and update workflow - #82719
hypershift: Add AWS v2 lifecycle step-registry components and update workflow#82719ironcladlou wants to merge 1 commit into
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
WalkthroughThe change adds AWS HyperShift guest creation, e2e testing, diagnostics, and destruction chains. It updates the AWS e2e-v2 workflow to use these chains and adds ownership metadata. ChangesAWS HyperShift e2e workflow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant CreateGuests
participant RunE2E
participant DumpGuests
participant DestroyGuests
Workflow->>CreateGuests: create AWS guest clusters
CreateGuests->>RunE2E: provide guest clusters
RunE2E->>DumpGuests: run diagnostics after tests
DumpGuests->>DestroyGuests: provide diagnostic results
DestroyGuests->>Workflow: destroy AWS guest clusters
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ironcladlou 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 |
05ef3da to
6bad574
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-commands.sh (1)
3-3: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable global shell tracing in the AWS credential step.
This script handles AWS credentials and sources a shared file while
set -xis active. Remove-xfrom the default shell options. If diagnostics require tracing, enable it only around non-sensitive commands.As per coding guidelines, step-registry command scripts must default to
set -euo pipefailwithout-xand must limit tracing around sensitive operations.Suggested fix
-set -exuo pipefail +set -euo pipefailAlso applies to: 7-10
🤖 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 `@ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-commands.sh` at line 3, Remove the global xtrace option from the shell settings in the AWS credential step, changing the default to set -euo pipefail. Keep sensitive credential handling and shared-file sourcing untraced; only add narrowly scoped tracing around non-sensitive diagnostics if required.Source: Coding guidelines
🤖 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
`@ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-ref.metadata.json`:
- Around line 3-20: Synchronize generated ownership metadata with OWNERS by
adding ironcladlou to both the approvers and reviewers arrays in
ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-ref.metadata.json
lines 3-20 and
ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-chain.metadata.json
lines 3-20, or regenerate both metadata files.
In
`@ci-operator/step-registry/hypershift/aws/dump-guests/hypershift-aws-dump-guests-chain.yaml`:
- Around line 11-20: Update the hypershift-aws-dump-guests chain environment
around the hypershift-debug and hypershift-k8sgpt refs so k8sgpt receives the
management cluster kubeconfig at the path it expects. Add the management
kubeconfig as the relevant ref environment default or copy it to
${SHARED_DIR}/kubeconfig before hypershift-k8sgpt runs, ensuring it scans the
management cluster.
In
`@ci-operator/step-registry/hypershift/aws/run-e2e-v2/hypershift-aws-run-e2e-v2-chain.metadata.json`:
- Around line 4-18: Regenerate both chain metadata files from their matching
OWNERS files: add ironcladlou to the approvers and reviewers arrays in
ci-operator/step-registry/hypershift/aws/run-e2e-v2/hypershift-aws-run-e2e-v2-chain.metadata.json
(lines 4-18) and
ci-operator/step-registry/hypershift/aws/dump-guests/hypershift-aws-dump-guests-chain.metadata.json
(lines 4-18).
---
Nitpick comments:
In
`@ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-commands.sh`:
- Line 3: Remove the global xtrace option from the shell settings in the AWS
credential step, changing the default to set -euo pipefail. Keep sensitive
credential handling and shared-file sourcing untraced; only add narrowly scoped
tracing around non-sensitive diagnostics if required.
🪄 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: 1df4c46c-fe6b-4161-ad17-a4d4cbfe961d
📒 Files selected for processing (17)
ci-operator/step-registry/hypershift/OWNERSci-operator/step-registry/hypershift/aws/create-guests/OWNERSci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-chain.metadata.jsonci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-chain.yamlci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-commands.shci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-ref.metadata.jsonci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-ref.yamlci-operator/step-registry/hypershift/aws/destroy-guests/OWNERSci-operator/step-registry/hypershift/aws/destroy-guests/hypershift-aws-destroy-guests-chain.metadata.jsonci-operator/step-registry/hypershift/aws/destroy-guests/hypershift-aws-destroy-guests-chain.yamlci-operator/step-registry/hypershift/aws/dump-guests/OWNERSci-operator/step-registry/hypershift/aws/dump-guests/hypershift-aws-dump-guests-chain.metadata.jsonci-operator/step-registry/hypershift/aws/dump-guests/hypershift-aws-dump-guests-chain.yamlci-operator/step-registry/hypershift/aws/e2e-v2/hypershift-aws-e2e-v2-workflow.yamlci-operator/step-registry/hypershift/aws/run-e2e-v2/OWNERSci-operator/step-registry/hypershift/aws/run-e2e-v2/hypershift-aws-run-e2e-v2-chain.metadata.jsonci-operator/step-registry/hypershift/aws/run-e2e-v2/hypershift-aws-run-e2e-v2-chain.yaml
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🧹 Nitpick comments (1)
ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-commands.sh (1)
3-3: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable global shell tracing in the AWS credential step.
This script handles AWS credentials and sources a shared file while
set -xis active. Remove-xfrom the default shell options. If diagnostics require tracing, enable it only around non-sensitive commands.As per coding guidelines, step-registry command scripts must default to
set -euo pipefailwithout-xand must limit tracing around sensitive operations.Suggested fix
-set -exuo pipefail +set -euo pipefailAlso applies to: 7-10
🤖 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 `@ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-commands.sh` at line 3, Remove the global xtrace option from the shell settings in the AWS credential step, changing the default to set -euo pipefail. Keep sensitive credential handling and shared-file sourcing untraced; only add narrowly scoped tracing around non-sensitive diagnostics if required.Source: Coding guidelines
🤖 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
`@ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-ref.metadata.json`:
- Around line 3-20: Synchronize generated ownership metadata with OWNERS by
adding ironcladlou to both the approvers and reviewers arrays in
ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-ref.metadata.json
lines 3-20 and
ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-chain.metadata.json
lines 3-20, or regenerate both metadata files.
In
`@ci-operator/step-registry/hypershift/aws/dump-guests/hypershift-aws-dump-guests-chain.yaml`:
- Around line 11-20: Update the hypershift-aws-dump-guests chain environment
around the hypershift-debug and hypershift-k8sgpt refs so k8sgpt receives the
management cluster kubeconfig at the path it expects. Add the management
kubeconfig as the relevant ref environment default or copy it to
${SHARED_DIR}/kubeconfig before hypershift-k8sgpt runs, ensuring it scans the
management cluster.
In
`@ci-operator/step-registry/hypershift/aws/run-e2e-v2/hypershift-aws-run-e2e-v2-chain.metadata.json`:
- Around line 4-18: Regenerate both chain metadata files from their matching
OWNERS files: add ironcladlou to the approvers and reviewers arrays in
ci-operator/step-registry/hypershift/aws/run-e2e-v2/hypershift-aws-run-e2e-v2-chain.metadata.json
(lines 4-18) and
ci-operator/step-registry/hypershift/aws/dump-guests/hypershift-aws-dump-guests-chain.metadata.json
(lines 4-18).
---
Nitpick comments:
In
`@ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-commands.sh`:
- Line 3: Remove the global xtrace option from the shell settings in the AWS
credential step, changing the default to set -euo pipefail. Keep sensitive
credential handling and shared-file sourcing untraced; only add narrowly scoped
tracing around non-sensitive diagnostics if required.
🪄 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: 1df4c46c-fe6b-4161-ad17-a4d4cbfe961d
📒 Files selected for processing (17)
ci-operator/step-registry/hypershift/OWNERSci-operator/step-registry/hypershift/aws/create-guests/OWNERSci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-chain.metadata.jsonci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-chain.yamlci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-commands.shci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-ref.metadata.jsonci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-ref.yamlci-operator/step-registry/hypershift/aws/destroy-guests/OWNERSci-operator/step-registry/hypershift/aws/destroy-guests/hypershift-aws-destroy-guests-chain.metadata.jsonci-operator/step-registry/hypershift/aws/destroy-guests/hypershift-aws-destroy-guests-chain.yamlci-operator/step-registry/hypershift/aws/dump-guests/OWNERSci-operator/step-registry/hypershift/aws/dump-guests/hypershift-aws-dump-guests-chain.metadata.jsonci-operator/step-registry/hypershift/aws/dump-guests/hypershift-aws-dump-guests-chain.yamlci-operator/step-registry/hypershift/aws/e2e-v2/hypershift-aws-e2e-v2-workflow.yamlci-operator/step-registry/hypershift/aws/run-e2e-v2/OWNERSci-operator/step-registry/hypershift/aws/run-e2e-v2/hypershift-aws-run-e2e-v2-chain.metadata.jsonci-operator/step-registry/hypershift/aws/run-e2e-v2/hypershift-aws-run-e2e-v2-chain.yaml
🛑 Comments failed to post (3)
ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-ref.metadata.json (1)
3-20: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Synchronize generated ownership metadata with
OWNERS.Both metadata files omit
ironcladlou, althoughci-operator/step-registry/hypershift/aws/create-guests/OWNERSlists that user as both approver and reviewer.
ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-ref.metadata.json#L3-L20: addironcladlouto both owner arrays or regenerate the metadata.ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-chain.metadata.json#L3-L20: addironcladlouto both owner arrays or regenerate the metadata.📍 Affects 2 files
ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-ref.metadata.json#L3-L20(this comment)ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-chain.metadata.json#L3-L20🤖 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 `@ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-ref.metadata.json` around lines 3 - 20, Synchronize generated ownership metadata with OWNERS by adding ironcladlou to both the approvers and reviewers arrays in ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-ref.metadata.json lines 3-20 and ci-operator/step-registry/hypershift/aws/create-guests/hypershift-aws-create-guests-chain.metadata.json lines 3-20, or regenerate both metadata files.ci-operator/step-registry/hypershift/aws/dump-guests/hypershift-aws-dump-guests-chain.yaml (1)
11-20: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail while IFS= read -r file; do rg -n -C 4 'KUBECONFIG|management_cluster_kubeconfig|mgmt_kubeconfig' "$file" || true done < <( fd --type f --glob 'hypershift-debug*' ci-operator/step-registry/hypershift/debug fd --type f --glob 'hypershift-k8sgpt*' ci-operator/step-registry/hypershift/k8sgpt )Repository: openshift/release
Length of output: 207
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== locate chain file ==" fd -a 'hypershift-aws-dump-guests-chain.yaml' ci-operator/step-registry/hypershift || true echo "== locate candidate ref files ==" fd --type f 'hypershift-debug|hypershift-k8sgpt|aws-dump-guests' ci-operator/step-registry || true echo "== chain content ==" file="$(fd -a 'hypershift-aws-dump-guests-chain.yaml' ci-operator/step-registry/hypershift | head -n1 || true)" if [ -n "${file:-}" ]; then cat -n "$file" fiRepository: openshift/release
Length of output: 1696
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== all kubeconfig mentions in hypershift step-registry ==" rg -n -C 3 'KUBECONFIG|kubeconfig|management_cluster_kubeconfig|mgmt_kubeconfig' ci-operator/step-registry/hypershift || true echo "== candidate ref candidates with matching names ==" fd --type f --glob '*debug*' ci-operator/step-registry/hypershift | sed -n '1,80p' fd --type f --glob '*k8sgpt*' ci-operator/step-registry/hypershift | sed -n '1,80p'Repository: openshift/release
Length of output: 50374
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== hypershift-debug ref metadata ==" cat -n ci-notices 2>/dev/null || true cat -n ci-operator/step-registry/hypershift/debug/hypershift-debug-ref.yaml echo cat -n ci-operator/step-registry/hypershift/debug/hypershift-debug-commands.sh echo "== hypershift-k8sgpt ref metadata ==" cat -n ci-operator/step-registry/hypershift/k8sgpt/hypershift-k8sgpt-ref.yaml echo cat -n ci-operator/step-registry/hypershift/k8sgpt/hypershift-k8sgpt-commands.sh echo "== config/env defaults for hypershift-k8sgpt-ref only ==" rg -n -C 2 'name: KUBECONFIG|from: |default:.*/management_cluster_kubeconfig|management_cluster_kubeconfig' \ ci-operator/step-registry/hypershift/k8sgpt/hypershift-k8sgpt-ref.yaml \ ci-operator/step-registry/hypershift/k8sgpt/hypershift-k8sgpt-commands.sh || trueRepository: openshift/release
Length of output: 10760
Set kubeconfig for the
k8sgptdiagnostic step.
hypershift-k8sgptonly pointsKUBECONFIGto${SHARED_DIR}/kubeconfig, but this chain writes the management kubeconfig to${SHARED_DIR}/management_cluster_kubeconfigand thedumpstep does not propagate it as chain environment. Add the management kubeconfig as a ref environment default, or copy/provide${SHARED_DIR}/kubeconfigfrom that file, sok8sgptscans the management cluster instead of skipping it from an incorrect kubeconfig.🤖 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 `@ci-operator/step-registry/hypershift/aws/dump-guests/hypershift-aws-dump-guests-chain.yaml` around lines 11 - 20, Update the hypershift-aws-dump-guests chain environment around the hypershift-debug and hypershift-k8sgpt refs so k8sgpt receives the management cluster kubeconfig at the path it expects. Add the management kubeconfig as the relevant ref environment default or copy it to ${SHARED_DIR}/kubeconfig before hypershift-k8sgpt runs, ensuring it scans the management cluster.ci-operator/step-registry/hypershift/aws/run-e2e-v2/hypershift-aws-run-e2e-v2-chain.metadata.json (1)
4-18: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Regenerate both chain metadata files from their matching
OWNERSfiles.Both metadata files omit
ironcladlou, while their matching OWNERS files list that identity as both an approver and reviewer.
ci-operator/step-registry/hypershift/aws/run-e2e-v2/hypershift-aws-run-e2e-v2-chain.metadata.json#L4-L18: addironcladlouto both arrays to matchci-operator/step-registry/hypershift/aws/run-e2e-v2/OWNERS.ci-operator/step-registry/hypershift/aws/dump-guests/hypershift-aws-dump-guests-chain.metadata.json#L4-L18: addironcladlouto both arrays to matchci-operator/step-registry/hypershift/aws/dump-guests/OWNERS.📍 Affects 2 files
ci-operator/step-registry/hypershift/aws/run-e2e-v2/hypershift-aws-run-e2e-v2-chain.metadata.json#L4-L18(this comment)ci-operator/step-registry/hypershift/aws/dump-guests/hypershift-aws-dump-guests-chain.metadata.json#L4-L18🤖 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 `@ci-operator/step-registry/hypershift/aws/run-e2e-v2/hypershift-aws-run-e2e-v2-chain.metadata.json` around lines 4 - 18, Regenerate both chain metadata files from their matching OWNERS files: add ironcladlou to the approvers and reviewers arrays in ci-operator/step-registry/hypershift/aws/run-e2e-v2/hypershift-aws-run-e2e-v2-chain.metadata.json (lines 4-18) and ci-operator/step-registry/hypershift/aws/dump-guests/hypershift-aws-dump-guests-chain.metadata.json (lines 4-18).
6bad574 to
6b1a2f8
Compare
|
/pj-rehearse list |
|
@ironcladlou: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
Please remove sjennings from the approvers and reviewers. This applies to the rest of the owners files in this PR too.
There was a problem hiding this comment.
to keep this one as small as possible I'd prefer to leave OWNERS as is and make a followup at the root of the tree to remove Seth and add myself since it'll touch everything
There was a problem hiding this comment.
I'm wondering for tests, would it be possible to cut this number down to 2?
There was a problem hiding this comment.
I think you were reviewing a commit I erased before I fixed unwanted OWNERS changes, can you add the comment to the new line? github isn't showing me what you're commenting on
|
/pj-rehearse pull-ci-openshift-hypershift-main-e2e-v2-aws periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-v2-aws periodic-ci-openshift-hypershift-release-4.22-periodics-e2e-v2-aws |
|
@ironcladlou: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@ironcladlou: The following tests 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. |
Add create-guests, destroy-guests, dump-guests, and run-e2e-v2 chains for AWS that use the Go binary lifecycle orchestration (create-guests, run-tests, destroy-guests, dump-guests). Update the hypershift-aws-e2e-v2 workflow to use these new chains instead of the shell-script based hypershift-aws-create and hypershift-e2e-v2 chains.
6b1a2f8 to
c32f0a9
Compare
|
New tests failed because I didn't mount the guest AWS credentials /pj-rehearse pull-ci-openshift-hypershift-main-e2e-v2-aws periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-v2-aws periodic-ci-openshift-hypershift-release-4.22-periodics-e2e-v2-aws |
|
@ironcladlou: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
Add create-guests, destroy-guests, dump-guests, and run-e2e-v2 chains for AWS that use the Go binary lifecycle orchestration (create-guests, run-tests, destroy-guests, dump-guests). Update the hypershift-aws-e2e-v2 workflow to use these new chains instead of the shell-script based hypershift-aws-create and hypershift-e2e-v2 chains.
Part of CNTRLPLANE-3646
Summary by CodeRabbit
Adds AWS HyperShift CI step-registry chains for creating, testing, diagnosing, and destroying guest clusters.
Updates the
hypershift-aws-e2e-v2workflow to use Go-based lifecycle orchestration instead of shell-script-based chains. The workflow now provisions AWS guest clusters, runsrun-tests, collects diagnostics, and performs best-effort guest cleanup.Adds ownership and metadata definitions for the new step-registry components.