ARO-HCP: move provisioning step-registry scripts to ARO-HCP repo - #82572
ARO-HCP: move provisioning step-registry scripts to ARO-HCP repo#82572gmfrasca wants to merge 1 commit into
Conversation
|
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: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe ARO HCP step wrappers now perform lightweight environment setup and delegate Hypershift and environment provisioning to shared scripts. Inline Azure authentication, image resolution, validation, finalization, and completion logic were removed. ChangesARO HCP provisioning delegation
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 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 |
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
![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
🧹 Nitpick comments (1)
ci-operator/step-registry/aro-hcp/hypershift-deploy/aro-hcp-hypershift-deploy-commands.sh (1)
17-17: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAll three wrappers
execan unqualified relative path with no CWD anchor. The shared root cause is that each shim assumes the step's working directory is the ARO-HCP clone root; acwdoverride in the ref, a job in another repo referencing these steps, or a missing executable bit produces an opaqueNo such file or directorywith none of the previous inline diagnostics. Anchoring the path (e.g.cd "/go/src/github.com/Azure/ARO-HCP"or resolving via a repo-root variable) plus a[[ -x ... ]]precheck makes the failure self-explanatory.
ci-operator/step-registry/aro-hcp/hypershift-deploy/aro-hcp-hypershift-deploy-commands.sh#L17-L17: anchor the path beforeexec hack/ci/provision-hypershift.shand precheck that it is executable.ci-operator/step-registry/aro-hcp/provision/environment/aro-hcp-provision-environment-commands.sh#L17-L17: apply the same anchor/precheck aroundexec hack/ci/provision-environment.sh.ci-operator/step-registry/aro-hcp/provision/from-main/aro-hcp-provision-from-main-commands.sh#L17-L17: apply the same anchor/precheck aroundexec hack/ci/provision-from-main.sh.♻️ Illustrative pattern (apply per file with the matching script name)
-exec hack/ci/provision-hypershift.sh +provision_script="hack/ci/provision-hypershift.sh" +if [[ ! -x "${provision_script}" ]]; then + echo "ERROR: ${provision_script} not found or not executable in $(pwd); expected the ARO-HCP repository root" >&2 + exit 1 +fi +exec "${provision_script}"🤖 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/aro-hcp/hypershift-deploy/aro-hcp-hypershift-deploy-commands.sh` at line 17, Anchor the repository path and add an executable precheck before the exec command in ci-operator/step-registry/aro-hcp/hypershift-deploy/aro-hcp-hypershift-deploy-commands.sh:17-17, then exec the anchored hack/ci/provision-hypershift.sh. Apply the same anchor and precheck in ci-operator/step-registry/aro-hcp/provision/environment/aro-hcp-provision-environment-commands.sh:17-17 for provision-environment.sh and ci-operator/step-registry/aro-hcp/provision/from-main/aro-hcp-provision-from-main-commands.sh:17-17 for provision-from-main.sh.
🤖 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/aro-hcp/hypershift-deploy/aro-hcp-hypershift-deploy-commands.sh`:
- Line 15: Update the CLUSTER_PROFILE_DIR assignment to safely handle an unset
VAULT_SECRET_PROFILE under nounset, defaulting the profile value to dev while
preserving explicitly provided values.
---
Nitpick comments:
In
`@ci-operator/step-registry/aro-hcp/hypershift-deploy/aro-hcp-hypershift-deploy-commands.sh`:
- Line 17: Anchor the repository path and add an executable precheck before the
exec command in
ci-operator/step-registry/aro-hcp/hypershift-deploy/aro-hcp-hypershift-deploy-commands.sh:17-17,
then exec the anchored hack/ci/provision-hypershift.sh. Apply the same anchor
and precheck in
ci-operator/step-registry/aro-hcp/provision/environment/aro-hcp-provision-environment-commands.sh:17-17
for provision-environment.sh and
ci-operator/step-registry/aro-hcp/provision/from-main/aro-hcp-provision-from-main-commands.sh:17-17
for provision-from-main.sh.
🪄 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: f064c54a-113d-4797-914e-30f127516e4a
📒 Files selected for processing (3)
ci-operator/step-registry/aro-hcp/hypershift-deploy/aro-hcp-hypershift-deploy-commands.shci-operator/step-registry/aro-hcp/provision/environment/aro-hcp-provision-environment-commands.shci-operator/step-registry/aro-hcp/provision/from-main/aro-hcp-provision-from-main-commands.sh
|
/pj-rehearse pull-ci-Azure-ARO-HCP-main-e2e-parallel |
|
@gmfrasca: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-hypershift-main-e2e-aro-hcp |
|
@gmfrasca: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-hypershift-main-e2e-aro-hcp |
|
@gmfrasca: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
Upon further investigation, it appears the hypershift job has historically only run thrice - it passed in the PR which it was introduced, and was aborted or failed in the other 2 runs: Given that history, the rehearsal failures are suspicious and unlikely to be related to the script migration. Running a couple other rehearsals for more coverage/confidence: |
|
@gmfrasca: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@gmfrasca: requesting more than one rehearsal in one comment is not supported. If you would like to rehearse multiple specific jobs, please separate the job names by a space in a single command. |
|
/pj-rehearse pull-ci-Azure-ARO-HCP-main-upgrade-e2e-parallel |
|
@gmfrasca: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-Azure-ARO-HCP-main-upgrade-e2e-parallel |
|
@gmfrasca: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/retest |
|
/pj-rehearse pull-ci-openshift-hypershift-main-e2e-aro-hcp pull-ci-Azure-ARO-HCP-main-upgrade-e2e-parallel |
|
@gmfrasca: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Delegate provisioning logic to shared scripts in the ARO-HCP repo Signed-off-by: Giulio Frasca <gfrasca@redhat.com>
e2e9b40 to
a2b6c68
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: gmfrasca 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 |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse pull-ci-openshift-hypershift-main-e2e-aro-hcp pull-ci-Azure-ARO-HCP-main-e2e-parallel |
|
@gmfrasca: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse rehearse-82572-pull-ci-openshift-hypershift-main-e2e-aro-hcp Retesting to get a new baseline - last run was around 2 weeks ago, possible some changes may potentially have affected the job in question. |
|
@gmfrasca: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@gmfrasca: job(s): rehearse-82572-pull-ci-openshift-hypershift-main-e2e-aro-hcp either don't exist or were not found to be affected, and cannot be rehearsed |
|
/pj-rehearse pull-ci-openshift-hypershift-main-e2e-aro-hcp |
|
@gmfrasca: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-hypershift-main-e2e-aro-hcp Previous rehearsal had new error shape, Suspect transient DNS failure, retrying while investigating in parallel |
|
@gmfrasca: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-hypershift-main-e2e-aro-hcp Cause of last failure was exaustion of RoleAssignments: quota issue unrelated to PR. Manually verified the sub has RA capacity now at time of re-run |
|
@gmfrasca: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@gmfrasca: 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. |
1 participant
Rewrite three ARO-HCP provisioning step-registry scripts as thin wrappers
that
execinto shared scripts in the ARO-HCP repository:aro-hcp-provision-environment→hack/ci/provision-environment.sharo-hcp-provision-from-main→hack/ci/provision-from-main.sharo-hcp-hypershift-deploy→hack/ci/provision-hypershift.shEach wrapper now only handles slot environment setup (sourcing
aro-hcp-slot.env, exportingLOCATIONandCLUSTER_PROFILE_DIR)before delegating to the ARO-HCP repo. This reduces 515 lines of
duplicated provisioning logic to 3x 17-line shims.
Depends on: Azure/ARO-HCP#6296
Summary by CodeRabbit
The ARO-HCP step registry now uses thin wrappers for environment provisioning, provisioning from main, and Hypershift deployment. Each wrapper loads
aro-hcp-slot.env, validates and exportsLOCATION, setsCLUSTER_PROFILE_DIR, and delegates to the shared ARO-HCP script.This removes 515 lines of duplicated Azure, image, provisioning, and cleanup logic. The change depends on ARO-HCP pull request 6296.