ci(provision): centralize CI provisioning scripts into ARO-HCP - #6296
ci(provision): centralize CI provisioning scripts into ARO-HCP#6296gmfrasca wants to merge 7 commits into
Conversation
|
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Pull request overview
This PR centralizes OpenShift CI provisioning logic that previously lived in openshift/release step-registry scripts into reusable, in-repo scripts under hack/ci/. The goal is to reduce duplication and allow step-registry commands.sh wrappers to become thin shims that delegate to these shared scripts.
Changes:
- Added shared Azure login helper (
az-login.sh) and ACR image resolution helper (resolve-acr-images.sh) for digest-pinned service image selection by commit SHA. - Added two new entry scripts for common CI flows: baseline provisioning from base branch (
provision-from-main.sh) and hypershift PR testing provisioning (provision-hypershift.sh). - Updated
provision-environment.shto delegate Azure auth toaz-login.shand to support configurable JUnit output and completion marker naming.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| hack/ci/resolve-acr-images.sh | New helper to poll ACR for SHA-tagged images, walk back history on misses, and resolve tag → digest for service images. |
| hack/ci/provision-hypershift.sh | New entry script to fetch main, resolve service images from ACR, and run environment provisioning for hypershift PR testing. |
| hack/ci/provision-from-main.sh | New entry script to provision a baseline environment from the base commit (or fetched main for rehearsals), then resolve ACR images and provision. |
| hack/ci/provision-environment.sh | Refactors shared provisioning to use centralized Azure login and adds parameterized artifact naming plus MGMT sizing overrides. |
| hack/ci/az-login.sh | New shared “sourceable” Azure SP login + subscription selection helper used by provisioning scripts. |
e2f7891 to
38dcb7e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
hack/ci/resolve-acr-images.sh:22
- ACR_URL is hard-coded to "${ACR_NAME}.azurecr.io", but the rendered config already carries the ACR DNS suffix (acrDNSSuffix). Using the config value avoids incorrect image refs in non-public Azure clouds (or if the suffix changes).
ACR_NAME=$(yq '.acr.svc.name' "${ACR_CONFIG_FILE}")
ACR_URL="${ACR_NAME}.azurecr.io"
hack/ci/provision-from-main.sh:63
- The fallback to config/rendered/dev/dev/centralus.yaml will never succeed in this repo (dev/dev currently only contains westus3.yaml). If westus3.yaml ever goes away, this will fail with a misleading error. Consider selecting the first available rendered config under config/rendered/dev/${IMAGES_DEPLOY_ENV}/ instead of hard-coding region filenames.
IMAGES_DEPLOY_ENV="dev"
export ACR_CONFIG_FILE="config/rendered/dev/${IMAGES_DEPLOY_ENV}/westus3.yaml"
if [[ ! -f "${ACR_CONFIG_FILE}" ]]; then
ACR_CONFIG_FILE="config/rendered/dev/${IMAGES_DEPLOY_ENV}/centralus.yaml"
fi
if [[ ! -f "${ACR_CONFIG_FILE}" ]]; then
echo "ERROR: No rendered config found for ${IMAGES_DEPLOY_ENV} (tried westus3.yaml, centralus.yaml)"
exit 1
fi
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
hack/ci/resolve-acr-images.sh:22
ACR_NAME(and other repo coordinates) are taken from yq without validating for empty/nullvalues. If the rendered config is missing.acr.svc.name, this script will proceed and fail later with less actionable ACR errors (e.g. pollingnull.azurecr.io). Add an early, explicit check so failures point at the config/key immediately.
ACR_NAME=$(yq '.acr.svc.name' "${ACR_CONFIG_FILE}")
ACR_URL="${ACR_NAME}.azurecr.io"
hack/ci/provision-from-main.sh:63
- This script hard-codes
IMAGES_DEPLOY_ENV="dev"but then falls back toconfig/rendered/dev/dev/centralus.yaml, which does not exist in the repo (onlywestus3.yamlexists for thedevenv). This makes the fallback and error message misleading and increases the chance of silent bitrot if paths change. Prefer checking only the known-good path (or selecting from whatever rendered config exists in that env) and update the error accordingly.
IMAGES_DEPLOY_ENV="dev"
export ACR_CONFIG_FILE="config/rendered/dev/${IMAGES_DEPLOY_ENV}/westus3.yaml"
if [[ ! -f "${ACR_CONFIG_FILE}" ]]; then
ACR_CONFIG_FILE="config/rendered/dev/${IMAGES_DEPLOY_ENV}/centralus.yaml"
fi
if [[ ! -f "${ACR_CONFIG_FILE}" ]]; then
echo "ERROR: No rendered config found for ${IMAGES_DEPLOY_ENV} (tried westus3.yaml, centralus.yaml)"
exit 1
fi
|
/retest |
1 similar comment
|
/retest |
| echo "Images for ${TARGET_SHA} not found after polling. Walking back through history ..." | ||
| MAX_WALK=20 | ||
| IMAGE_SHA="" | ||
| for sha in $(git log "${TARGET_SHA}" --format='%h' --abbrev=7 --skip=1 -n ${MAX_WALK}); do |
There was a problem hiding this comment.
The current live aro-hcp-hypershift-deploy-commands.sh deliberately avoids relying on local git history for main — it resolves the tip via git ls-remote and walks back history via the GitHub REST API (curl api.github.com/.../commits?sha=main) instead of git log. provision-hypershift.sh only does a git fetch ... main before sourcing this script, and CI checkouts here are commonly shallow/scoped to the PR's own tree, so this local git log "${TARGET_SHA}" --skip=1 walk-back could silently come up empty if the fetch doesn't bring in full ancestry. Worth a comment noting this assumption (full, non-shallow fetch of main's history) so it gets scrutinized/validated in the rehearsal run, since the walk-back path isn't exercised by the happy-path case.
There was a problem hiding this comment.
Since shallow git trees could be a concern, I think actually going the opposite direction and leveraging the GitHub REST API in this ACR resolver script, rather than git log and simply documenting a potential shortcoming, may be the better/safer path. Updated to do so in a new commit so we can easily revert if there are other considerations we should make.
| # Scale up when identity containers are leased (E2E runs), set minCount=1 | ||
| # for leased healthcheck workflows, skip entirely when no lease is held | ||
| # (e.g. hypershift-deploy) to preserve the base config sizing. | ||
| if [[ -n "${LEASED_MSI_CONTAINERS:-}" ]]; then |
There was a problem hiding this comment.
This whole block doesn't match what's actually live today. The only MGMT sizing override that currently exists upstream is the single check in aro-hcp-provision-environment-commands.sh (https://github.com/openshift/release/blob/a322c1c804c2a2da31d34918de8b2c8a5e9e0956/ci-operator/step-registry/aro-hcp/provision/environment/aro-hcp-provision-environment-commands.sh#L137-L143):
if [[ -z "${LEASED_MSI_CONTAINERS:-}" ]]; then
yq -i ".clouds.dev.environments.${DEPLOY_ENV}.defaults.mgmt.aks.userAgentPool.minCount = 1" "${OVERRIDE_CONFIG_FILE}"
fi
The minCount=7/infraAgentPool.vmSize=Standard_D8ds_v6 override and the elif LEASED_MSI_MOCK_SP branch appear to be stale config that survived in some of the other scripts (e.g. provision-from-main-commands.sh) but aren't the current source of truth. Please drop this block down to just the single LEASED_MSI_CONTAINERS-gated minCount=1 override to match what's actually live, rather than centralizing the stale variant.
There was a problem hiding this comment.
ah good catch - this was sourced from the from-main provision commands, and I suppose only the provision-environment script was updated with the source-of-truth MGMT sizing override we want to use (and actually demonstrates one of the main motivations/benefits of this change). updated to use current MGMT sizing override from provision-environment
| # Exports: BACKEND_IMAGE, FRONTEND_IMAGE, ADMIN_API_IMAGE, SESSIONGATE_IMAGE, | ||
| # FLEET_IMAGE, MGMT_AGENT_IMAGE, KUBE_APPLIER_IMAGE | ||
| # | ||
| # hcpRecovery is not pushed to ACR by images-push, so it is excluded. |
There was a problem hiding this comment.
aro-hcp-images-push-commands.sh does push a customExporter/exporter image to ACR by commit SHA (alongside backend/frontend/etc.), so it should probably be resolved here too — this script only excludes hcpRecovery (correctly, since that one truly isn't pushed). This gap in the exporter/hcpRecovery distinction was already caught and fixed in the upstream openshift/release scripts recently, so it looks like this PR was written against an earlier snapshot. Worth syncing this script with the current upstream image list before merging.
There was a problem hiding this comment.
Yep, synced. Added EXPORTER_REPO/EXPORTER_DIGEST/EXPORTER_IMAGE to resolve-acr-images.sh and EXPORTER to the IMAGE_MAP in build-config-override.sh.
| #!/bin/bash | ||
| # Provision an ARO HCP environment with ACR-resolved main-branch images. | ||
| # Used by the hypershift PR testing workflow to deploy a full environment | ||
| # with current main service images plus PR-built HO/CPO image overrides. | ||
| # | ||
| # Called by the aro-hcp-hypershift-deploy step-registry wrapper. | ||
| # Named provision-hypershift.sh to follow the provision-* convention. | ||
| # Callers must set: CLUSTER_PROFILE_DIR, ARO_HCP_DEPLOY_ENV, SHARED_DIR, | ||
| # ARTIFACT_DIR, LOCATION | ||
| set -o errexit | ||
| set -o nounset | ||
| set -o pipefail | ||
|
|
||
| # --- Resolve ARO-HCP service images from dev ACR by main commit SHA --- | ||
| # The images-push-postsubmit job publishes service images on every merge | ||
| # to ARO-HCP main, tagged with the 7-char commit SHA. We resolve images | ||
| # by SHA to guarantee version coherence across all services. | ||
|
|
||
| # shellcheck source=hack/ci/az-login.sh | ||
| source "$(dirname "$0")/az-login.sh" | ||
|
|
||
| export ACR_CONFIG_FILE="${SHARED_DIR}/config.yaml" | ||
|
|
||
| git fetch https://github.com/Azure/ARO-HCP.git main | ||
| export TARGET_SHA | ||
| TARGET_SHA=$(git rev-parse --short=7 FETCH_HEAD) | ||
| echo "ARO-HCP main HEAD: ${TARGET_SHA}" | ||
|
|
||
| # Resolve service images from ACR | ||
| # shellcheck source=hack/ci/resolve-acr-images.sh | ||
| source "$(dirname "$0")/resolve-acr-images.sh" | ||
|
|
||
| exec "$(dirname "$0")/provision-environment.sh" |
There was a problem hiding this comment.
Wouldn't it make more sense that this lives in the hypershift provision step instead?
There was a problem hiding this comment.
As in the step-registry script? The intended pattern is for each step-registry wrapper to be as light as possible (just handle slot envs then exec the appropriate provision script) and do the real work in ARO-HCP, where we have more control and flexibility.
| #!/bin/bash | ||
| # Resolve ARO-HCP service images from ACR by commit SHA. | ||
| # Sourced (not exec'd) by provision-from-main.sh and provision-hypershift.sh. | ||
| # | ||
| # Callers must set: | ||
| # ACR_CONFIG_FILE — path to rendered config YAML (for ACR name + repo paths) | ||
| # TARGET_SHA — 7-char git commit SHA to look up in ACR | ||
| # | ||
| # Callers must ensure TARGET_SHA is reachable in local git history | ||
| # (via checkout or fetch) so the walk-back fallback can use git log. | ||
| # | ||
| # Exports: BACKEND_IMAGE, FRONTEND_IMAGE, ADMIN_API_IMAGE, SESSIONGATE_IMAGE, | ||
| # FLEET_IMAGE, MGMT_AGENT_IMAGE, KUBE_APPLIER_IMAGE | ||
| # | ||
| # hcpRecovery is not pushed to ACR by images-push, so it is excluded. | ||
|
|
||
| : "${ACR_CONFIG_FILE:?ACR_CONFIG_FILE must be set}" | ||
| : "${TARGET_SHA:?TARGET_SHA must be set}" | ||
|
|
||
| ACR_NAME=$(yq '.acr.svc.name' "${ACR_CONFIG_FILE}") | ||
| ACR_URL="${ACR_NAME}.azurecr.io" | ||
|
|
||
| BACKEND_REPO=$(yq '.backend.image.repository' "${ACR_CONFIG_FILE}") | ||
| FRONTEND_REPO=$(yq '.frontend.image.repository' "${ACR_CONFIG_FILE}") | ||
| ADMIN_API_REPO=$(yq '.adminApi.image.repository' "${ACR_CONFIG_FILE}") | ||
| SESSIONGATE_REPO=$(yq '.sessiongate.image.repository' "${ACR_CONFIG_FILE}") | ||
| FLEET_REPO=$(yq '.fleet.image.repository' "${ACR_CONFIG_FILE}") | ||
| MGMT_AGENT_REPO=$(yq '.mgmtAgent.image.repository' "${ACR_CONFIG_FILE}") | ||
| KUBE_APPLIER_REPO=$(yq '.kubeApplier.image.repository' "${ACR_CONFIG_FILE}") | ||
| # hcpRecovery is not pushed to ACR by images-push; its config digest is | ||
| # empty by default, so we leave it as-is rather than trying to resolve it. | ||
|
|
||
| echo "ACR: ${ACR_URL}, target SHA: ${TARGET_SHA}" | ||
| echo "Repos: backend=${BACKEND_REPO} frontend=${FRONTEND_REPO} admin-api=${ADMIN_API_REPO} sessiongate=${SESSIONGATE_REPO} fleet=${FLEET_REPO} mgmt-agent=${MGMT_AGENT_REPO} kube-applier=${KUBE_APPLIER_REPO}" | ||
|
|
||
| # Prefer the latest main commit's images. Poll ACR in case the postsubmit | ||
| # images-push job is still running. If HEAD's images never appear, walk | ||
| # back through history to find the newest commit with images available. | ||
| MAX_POLL=30 | ||
| POLL_INTERVAL=30 | ||
| echo "Polling ACR for ${FLEET_REPO}:${TARGET_SHA} (up to $((MAX_POLL * POLL_INTERVAL))s) ..." | ||
| FOUND_HEAD=false | ||
| for attempt in $(seq 1 ${MAX_POLL}); do | ||
| if az acr manifest show -r "${ACR_NAME}" -n "${FLEET_REPO}:${TARGET_SHA}" &>/dev/null; then | ||
| echo "Images for ${TARGET_SHA} available after attempt ${attempt}" | ||
| FOUND_HEAD=true | ||
| break | ||
| fi | ||
| echo "Attempt ${attempt}/${MAX_POLL}: not yet available, retrying in ${POLL_INTERVAL}s ..." | ||
| sleep ${POLL_INTERVAL} | ||
| done | ||
|
|
||
| if [[ "${FOUND_HEAD}" != "true" ]]; then | ||
| echo "Images for ${TARGET_SHA} not found after polling. Walking back through history ..." | ||
| MAX_WALK=20 | ||
| IMAGE_SHA="" | ||
| for sha in $(git log "${TARGET_SHA}" --format='%h' --abbrev=7 --skip=1 -n ${MAX_WALK}); do | ||
| if az acr manifest show -r "${ACR_NAME}" -n "${FLEET_REPO}:${sha}" &>/dev/null; then | ||
| IMAGE_SHA="${sha}" | ||
| echo "Found images in ACR for commit ${sha}" | ||
| break | ||
| fi | ||
| echo " ${sha}: not in ACR, trying older ..." | ||
| done | ||
|
|
||
| if [[ -z "${IMAGE_SHA}" ]]; then | ||
| echo "ERROR: No images found in ${ACR_NAME} for any of the last ${MAX_WALK} commits from ${TARGET_SHA}. Aborting." | ||
| exit 1 | ||
| fi | ||
| TARGET_SHA="${IMAGE_SHA}" | ||
| fi | ||
|
|
||
| # Resolve each image tag to its digest from ACR. | ||
| resolve_digest() { | ||
| local repo=$1 tag=$2 | ||
| local digest | ||
| digest=$(az acr manifest show-metadata -r "${ACR_NAME}" -n "${repo}:${tag}" --query 'digest' -o tsv) || true | ||
| if [[ -z "${digest}" ]]; then | ||
| echo "ERROR: Failed to resolve digest for ${repo}:${tag}" >&2 | ||
| return 1 | ||
| fi | ||
| echo "${digest}" | ||
| } | ||
|
|
||
| echo "Resolving image digests for tag ${TARGET_SHA} ..." | ||
| BACKEND_DIGEST=$(resolve_digest "${BACKEND_REPO}" "${TARGET_SHA}") | ||
| FRONTEND_DIGEST=$(resolve_digest "${FRONTEND_REPO}" "${TARGET_SHA}") | ||
| ADMIN_API_DIGEST=$(resolve_digest "${ADMIN_API_REPO}" "${TARGET_SHA}") | ||
| SESSIONGATE_DIGEST=$(resolve_digest "${SESSIONGATE_REPO}" "${TARGET_SHA}") | ||
| FLEET_DIGEST=$(resolve_digest "${FLEET_REPO}" "${TARGET_SHA}") | ||
| MGMT_AGENT_DIGEST=$(resolve_digest "${MGMT_AGENT_REPO}" "${TARGET_SHA}") | ||
| KUBE_APPLIER_DIGEST=$(resolve_digest "${KUBE_APPLIER_REPO}" "${TARGET_SHA}") | ||
|
|
||
| echo "Resolved digests:" | ||
| echo " backend: ${BACKEND_DIGEST}" | ||
| echo " frontend: ${FRONTEND_DIGEST}" | ||
| echo " admin-api: ${ADMIN_API_DIGEST}" | ||
| echo " sessiongate: ${SESSIONGATE_DIGEST}" | ||
| echo " fleet: ${FLEET_DIGEST}" | ||
| echo " mgmt-agent: ${MGMT_AGENT_DIGEST}" | ||
| echo " kube-applier: ${KUBE_APPLIER_DIGEST}" | ||
|
|
||
| export BACKEND_IMAGE="${ACR_URL}/${BACKEND_REPO}@${BACKEND_DIGEST}" | ||
| export FRONTEND_IMAGE="${ACR_URL}/${FRONTEND_REPO}@${FRONTEND_DIGEST}" | ||
| export ADMIN_API_IMAGE="${ACR_URL}/${ADMIN_API_REPO}@${ADMIN_API_DIGEST}" | ||
| export SESSIONGATE_IMAGE="${ACR_URL}/${SESSIONGATE_REPO}@${SESSIONGATE_DIGEST}" | ||
| export FLEET_IMAGE="${ACR_URL}/${FLEET_REPO}@${FLEET_DIGEST}" | ||
| export MGMT_AGENT_IMAGE="${ACR_URL}/${MGMT_AGENT_REPO}@${MGMT_AGENT_DIGEST}" | ||
| export KUBE_APPLIER_IMAGE="${ACR_URL}/${KUBE_APPLIER_REPO}@${KUBE_APPLIER_DIGEST}" |
There was a problem hiding this comment.
A follow up to create a small go CLI out of this is probably worth it - better control flow, retries, etc.
There was a problem hiding this comment.
Can we extract the overriding into a build-config-override.sh script? It would be called by aro-hcp-test-local-upgrade-commands.sh, as mentioned in openshift/release#80778.
aro-hcp-test-local-upgrade-commands.sh covers:
- parsing optional
*_IMAGEpullspecs - write image overrides via
IMAGE_MAP USE_OC_LOGIN_REGISTRIES- MSI / AKS sizing (callers can skip via env if unused)
- merge
${SHARED_DIR}/hypershift-image-overrides.yamlwhen present
There was a problem hiding this comment.
Done - Extracted into hack/ci/build-config-override.sh, and now provision-environment.sh (and other scripts) can source it to reuse the build config override functionality
…call Prepare shared provision script for use by openshift/release step-registry scripts, centralizing duplicated provisioning logic. Add MGMT cluster sizing overrides block, parameterize --abort-if-regional-exist via ARO_HCP_PROVISION_ABORT_IF_EXISTS, and add PROVISION_STEP_NAME for configurable junit output filenames. Signed-off-by: Giulio Frasca <gfrasca@redhat.com>
Extract ACR poll, history walk-back, and digest resolution logic into resolve-acr-images.sh for use by provision-from-main and hypershift-deploy entrypoints. Signed-off-by: Giulio Frasca <gfrasca@redhat.com>
Extract base-branch checkout, rehearsal detection, and ACR config resolution from openshift/release into a dedicated entrypoint that sources resolve-acr-images.sh and exec's provision-environment.sh. Signed-off-by: Giulio Frasca <gfrasca@redhat.com>
Move hypershift-deploy ACR resolution logic from openshift/release step-registry into ARO-HCP, using shared resolve-acr-images.sh. Signed-off-by: Giulio Frasca <gfrasca@redhat.com>
Replace inline Azure credential setup and login across all three provision scripts with a shared az-login.sh sourced helper. Signed-off-by: Giulio Frasca <gfrasca@redhat.com>
Avoids shallow clone issues by resolving main SHA via git ls-remote and walking back commit history via the GitHub REST API. Signed-off-by: Giulio Frasca <gfrasca@redhat.com>
Move image parsing, MSI/AKS sizing, and hypershift override merging into a standalone script that can be sourced by both provision-environment.sh and future upgrade workflows. Signed-off-by: Giulio Frasca <gfrasca@redhat.com>
56fdf7b to
48d432c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
hack/ci/resolve-acr-images.sh:56
- The GitHub API history walk-back uses
sha=${TARGET_SHA}, butTARGET_SHAis documented/produced as a 7-char SHA tag. The commits API does not reliably accept abbreviated SHAs, and withset -euo pipefailinherited from the callers, any transient curl/jq failure will currently terminate the script without reaching the explicit "No images found" error. Consider resolving a full SHA (via local git when available, otherwise via the GitHub API commit endpoint) and guarding the curl/jq pipeline so failures produce a clear error message (optionally usingGITHUB_TOKENauth to avoid rate limits).
echo "Images for ${TARGET_SHA} not found after polling. Walking back through history ..."
MAX_WALK=20
IMAGE_SHA=""
for sha in $(curl -sS "https://api.github.com/repos/Azure/ARO-HCP/commits?sha=${TARGET_SHA}&per_page=${MAX_WALK}" | jq -r '.[].sha' | cut -c1-7 | tail -n +2); do
if az acr manifest show -r "${ACR_NAME}" -n "${FLEET_REPO}:${sha}" &>/dev/null; then
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gmfrasca, raelga 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 |
ARO-28195
Supercedes #5866
What
Extracts CI provisioning logic from
openshift/releasestep-registry scripts into shared scripts underhack/ci/, preparing for the step-registry wrappers to become thin shims thatexecinto these scripts.New scripts:
az-login.sh— shared Azure SP login (sourced by all others)resolve-acr-images.sh— ACR polling, git history walk-back, and digest resolutionprovision-from-main.sh— upgrade baseline: checks outPULL_BASE_SHA, resolves ACR images, runs provisioningprovision-hypershift.sh— hypershift PR testing: fetches main, resolves ACR images, runs provisioningUpdated:
provision-environment.sh— parameterized JUnit output and completion marker names, added MGMT sizing overrides, delegates Azure login toaz-login.shWhy
The three provisioning step-registry scripts (
provision-environment,provision-from-main,hypershift-deploy) had significant duplicated logic acrossopenshift/release. Centralizing into ARO-HCP makes the logic easier to test, review, and evolve without cross-repo coordination for every change.The companion
openshift/releasePR will reduce those step-registry scripts to ~17-line wrappers.Testing
These scripts are additive and not yet consumed until the companion
openshift/releasePR lands. The existingprovision-environment.shchanges are backward-compatible with current callers.openshift/releasePR.Special notes for your reviewer
openshift/releasePR, which rewrites the step-registrycommands.shfiles toexecinto these scripts.az-login.shis created after scripts that source it), but the final tree state is correct.PR Checklist