ARO-HCP: add optional hypershift in-place upgrade E2E workflow#80778
ARO-HCP: add optional hypershift in-place upgrade E2E workflow#80778patriksuba wants to merge 1 commit into
Conversation
|
Skipping CI for Draft Pull Request. |
|
@patriksuba, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
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 ignored due to path filters (1)
📒 Files selected for processing (24)
💤 Files with no reviewable changes (23)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughIntroduces a complete ARO HCP HyperShift upgrade E2E CI pipeline: a new Boskos lease slot resource, two git-checkout steps (baseline ChangesARO HCP HyperShift Upgrade E2E Workflow
Sequence Diagram(s)sequenceDiagram
actor Prow as Prow CI
participant Boskos as Boskos Lease
participant GitCheckout as git-checkout (main)
participant UpgradeInfra as upgrade-infra
participant GitCheckoutHead as git-checkout-head (PR)
participant PreUpgrade as pre-upgrade test
participant PostUpgrade as post-upgrade test
participant SharedDir as SHARED_DIR
rect rgba(100, 150, 200, 0.5)
note over Prow,UpgradeInfra: pre phase
Prow->>Boskos: acquire aro-hcp-dev-upgrade-westus3-slot
Boskos-->>Prow: slot leased (aro-hcp-slot.env)
Prow->>GitCheckout: checkout main ref
GitCheckout->>SharedDir: git-checkout-ref, git-checkout-sha
Prow->>UpgradeInfra: run HyperShift upgrade
UpgradeInfra->>SharedDir: config-override.yaml, provision-phase=upgrade
end
rect rgba(100, 200, 150, 0.5)
note over Prow,GitCheckoutHead: test phase (head checkout)
Prow->>GitCheckoutHead: checkout PR head (GIT_REF)
GitCheckoutHead->>SharedDir: git-checkout-ref (head), git-checkout-sha
Prow->>PreUpgrade: run upgrade/create suite
PreUpgrade->>SharedDir: e2e-setup.json, junit-pre-upgrade.xml.gz
end
rect rgba(200, 150, 100, 0.5)
note over Prow,PostUpgrade: post phase
Prow->>PostUpgrade: run upgrade/post-infra suite
PostUpgrade->>SharedDir: read e2e-setup.json
PostUpgrade->>SharedDir: junit-post-upgrade.xml.gz
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: patriksuba 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
ci-operator/step-registry/aro-hcp/upgrade/infra/aro-hcp-upgrade-infra-ref.yaml (1)
20-22: Consider aligning ARO_HCP_DEPLOY_ENV defaults across upgrade chain steps.The upgrade-infra step defaults to
ci00while the post-upgrade step defaults toprow. Although the job-level env override (ARO_HCP_DEPLOY_ENV: ci01) prevents drift in the currente2e-parallel-hypershift-upgradejob, inconsistent step-level defaults could cause silent environment selection issues if the workflow is reused in other contexts without explicit env overrides.🤖 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/upgrade/infra/aro-hcp-upgrade-infra-ref.yaml` around lines 20 - 22, The ARO_HCP_DEPLOY_ENV variable in the aro-hcp-upgrade-infra step defaults to "ci00" while the post-upgrade step defaults to "prow", creating inconsistency that could cause silent environment selection issues if the upgrade chain is reused without explicit environment overrides. Align the default value of ARO_HCP_DEPLOY_ENV in the upgrade-infra step to match the default used in the post-upgrade step to ensure consistent behavior across all upgrade chain steps regardless of job context.
🤖 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/git/checkout-head/aro-hcp-git-checkout-head-commands.sh`:
- Around line 13-14: The git fetch commands on lines 13-14 fail hard when the
ref is a commit SHA (from PULL_PULL_SHA in presubmits) because git cannot fetch
raw commit SHAs directly. Since the commit is already available locally after
clonerefs clones the PR, change both git fetch commands to soft-fail by
appending || true to each command, and add a verification step after the fetches
to check if the ref exists locally (using git rev-parse --verify "${ref}")
before attempting the git checkout to ensure the ref is available.
In
`@ci-operator/step-registry/aro-hcp/git/checkout/aro-hcp-git-checkout-commands.sh`:
- Around line 12-14: The git fetch command on line 12 fails hard when the remote
does not support SHA-based fetches, causing the entire script to exit due to set
-o errexit before the git checkout command can attempt to resolve a valid local
commit. Make both fetch attempts best-effort by ensuring they do not cause
script failure (using || true pattern), then add an explicit verification step
that validates the ref is locally resolvable before attempting the git checkout
on line 14. Apply the same pattern fix to aro-hcp-git-checkout-head-commands.sh
which has the identical issue.
In
`@ci-operator/step-registry/aro-hcp/test/local-post-upgrade/aro-hcp-test-local-post-upgrade-ref.yaml`:
- Around line 30-32: The ARO_HCP_DEPLOY_ENV parameter in the post-upgrade step
defaults to "prow" but should be aligned with the pre-upgrade step which
defaults to "ci00". Change the default value of the ARO_HCP_DEPLOY_ENV variable
from "prow" to "ci00" to ensure consistency between the pre-upgrade and
post-upgrade workflow steps and prevent configuration mismatches when this step
is reused in other workflows.
In
`@ci-operator/step-registry/aro-hcp/test/local-pre-upgrade/aro-hcp-test-local-pre-upgrade-commands.sh`:
- Around line 53-54: The fallback value for the junit_shared_name variable on
line 53 uses the same default name as the post-upgrade step's fallback, which
causes artifact collision when E2E_JUNIT_SHARED_NAME is not explicitly set.
Change the default fallback from "junit-e2e-suite.xml.gz" to a
pre-upgrade-specific name (for example, by adding "pre-upgrade" to the filename)
so that the pre-upgrade and post-upgrade steps each write to distinct artifact
files and prevent the post-upgrade from overwriting the pre-upgrade output.
---
Nitpick comments:
In
`@ci-operator/step-registry/aro-hcp/upgrade/infra/aro-hcp-upgrade-infra-ref.yaml`:
- Around line 20-22: The ARO_HCP_DEPLOY_ENV variable in the
aro-hcp-upgrade-infra step defaults to "ci00" while the post-upgrade step
defaults to "prow", creating inconsistency that could cause silent environment
selection issues if the upgrade chain is reused without explicit environment
overrides. Align the default value of ARO_HCP_DEPLOY_ENV in the upgrade-infra
step to match the default used in the post-upgrade step to ensure consistent
behavior across all upgrade chain steps regardless of job context.
🪄 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: 355acf0a-a1bf-4e36-a534-176f3d584248
⛔ Files ignored due to path filters (1)
ci-operator/jobs/Azure/ARO-HCP/Azure-ARO-HCP-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (26)
ci-operator/config/Azure/ARO-HCP/Azure-ARO-HCP-main.yamlci-operator/step-registry/aro-hcp/git/checkout-head/OWNERSci-operator/step-registry/aro-hcp/git/checkout-head/aro-hcp-git-checkout-head-commands.shci-operator/step-registry/aro-hcp/git/checkout-head/aro-hcp-git-checkout-head-ref.metadata.jsonci-operator/step-registry/aro-hcp/git/checkout-head/aro-hcp-git-checkout-head-ref.yamlci-operator/step-registry/aro-hcp/git/checkout/OWNERSci-operator/step-registry/aro-hcp/git/checkout/aro-hcp-git-checkout-commands.shci-operator/step-registry/aro-hcp/git/checkout/aro-hcp-git-checkout-ref.metadata.jsonci-operator/step-registry/aro-hcp/git/checkout/aro-hcp-git-checkout-ref.yamlci-operator/step-registry/aro-hcp/local-e2e-upgrade/OWNERSci-operator/step-registry/aro-hcp/local-e2e-upgrade/aro-hcp-local-e2e-upgrade-workflow.metadata.jsonci-operator/step-registry/aro-hcp/local-e2e-upgrade/aro-hcp-local-e2e-upgrade-workflow.yamlci-operator/step-registry/aro-hcp/test/local-post-upgrade/OWNERSci-operator/step-registry/aro-hcp/test/local-post-upgrade/aro-hcp-test-local-post-upgrade-commands.shci-operator/step-registry/aro-hcp/test/local-post-upgrade/aro-hcp-test-local-post-upgrade-ref.metadata.jsonci-operator/step-registry/aro-hcp/test/local-post-upgrade/aro-hcp-test-local-post-upgrade-ref.yamlci-operator/step-registry/aro-hcp/test/local-pre-upgrade/OWNERSci-operator/step-registry/aro-hcp/test/local-pre-upgrade/aro-hcp-test-local-pre-upgrade-commands.shci-operator/step-registry/aro-hcp/test/local-pre-upgrade/aro-hcp-test-local-pre-upgrade-ref.metadata.jsonci-operator/step-registry/aro-hcp/test/local-pre-upgrade/aro-hcp-test-local-pre-upgrade-ref.yamlci-operator/step-registry/aro-hcp/upgrade/infra/OWNERSci-operator/step-registry/aro-hcp/upgrade/infra/aro-hcp-upgrade-infra-commands.shci-operator/step-registry/aro-hcp/upgrade/infra/aro-hcp-upgrade-infra-ref.metadata.jsonci-operator/step-registry/aro-hcp/upgrade/infra/aro-hcp-upgrade-infra-ref.yamlcore-services/prow/02_config/_boskos.yamlcore-services/prow/02_config/generate-boskos.py
| git fetch --tags origin "${ref}" 2>/dev/null || git fetch origin "${ref}" | ||
| git fetch --unshallow origin 2>/dev/null || true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify SHA input path and strict fetch behavior in checkout-head step.
rg -n 'ref="\$\{GIT_REF:-\$\{PULL_PULL_SHA:-\}\}"|PULL_PULL_SHA|git fetch --tags origin "\$\{ref\}"|git fetch origin "\$\{ref\}"' \
ci-operator/step-registry/aro-hcp/git/checkout-head/aro-hcp-git-checkout-head-commands.shRepository: openshift/release
Length of output: 269
🏁 Script executed:
cat -n ci-operator/step-registry/aro-hcp/git/checkout-head/aro-hcp-git-checkout-head-commands.shRepository: openshift/release
Length of output: 794
🏁 Script executed:
# Also check if there's documentation about how clonerefs works and whether it pre-fetches
rg -l "clonerefs" ci-operator/step-registry/ | head -5Repository: openshift/release
Length of output: 267
🏁 Script executed:
# Look for other git checkout steps to see how they handle SHA refs
fd -e "commands.sh" ci-operator/step-registry/ | xargs grep -l "git fetch.*origin.*\${ref}" | head -5Repository: openshift/release
Length of output: 471
🏁 Script executed:
# Check the other git checkout steps found earlier
cat ci-operator/step-registry/openshift/microshift/includes/openshift-microshift-includes-commands.sh | grep -A 10 -B 5 "git fetch"Repository: openshift/release
Length of output: 43
🏁 Script executed:
# Look for documentation on clonerefs or how the repo is prepared
rg "clonerefs|clone.*repository" ci-operator/config/ --max-count 5Repository: openshift/release
Length of output: 353
🏁 Script executed:
# Search for git checkout patterns in step registry to see how others handle SHA refs
rg "git fetch.*origin.*ref\}" ci-operator/step-registry/ --max-count 10 -A 2 -B 2Repository: openshift/release
Length of output: 1278
🏁 Script executed:
# Check if git checkout works on a locally existing SHA without fetching
# First, understand the actual git behavior with SHAs
git --version
# Search for documentation on how clonerefs/cloner works
rg "cloner|clonerefs" --type yaml ci-operator/ | head -10Repository: openshift/release
Length of output: 1640
🏁 Script executed:
# Look for git clone or git fetch patterns that handle SHAs properly
rg "git.*checkout|git.*fetch" ci-operator/step-registry/ --type sh -B 2 -A 2 | grep -E "(fetch|checkout|SHA|ref)" | head -20Repository: openshift/release
Length of output: 3074
🏁 Script executed:
# Check the base image used for git steps to understand what git capabilities are available
fd "checkout-head" -e "ref.yaml" ci-operator/step-registry/aro-hcp/git/Repository: openshift/release
Length of output: 43
🏁 Script executed:
# Verify: Can you fetch a raw commit SHA with git fetch?
# Create a test to understand git fetch behavior with SHAs
cat > /tmp/test_git_sha.md << 'EOF'
Git fetch behavior with SHAs:
- `git fetch origin <BRANCH>` - works (refs/heads/BRANCH)
- `git fetch origin <TAG>` - works (refs/tags/TAG)
- `git fetch origin <SHA>` - FAILS (SHA is not a ref name)
Presubmit context:
- PULL_PULL_SHA is a commit SHA (e.g., "abc123def456...")
- Remote doesn't have this as a named ref
- Attempt to fetch SHA directly will fail
Solution:
- If clonerefs pre-clones the repo, SHA already exists locally
- Don't need to fetch it
- Just verify it exists locally and checkout
EOF
cat /tmp/test_git_sha.mdRepository: openshift/release
Length of output: 567
🏁 Script executed:
# Check what clonerefs does - look at CI operator config documentation
rg "clone|git.*fetch" ci-operator/config/openshift/ -t yaml --max-count 5 | head -20Repository: openshift/release
Length of output: 3076
🏁 Script executed:
# Look at the step reference file to understand environment setup
cat ci-operator/step-registry/aro-hcp/git/checkout-head/aro-hcp-git-checkout-head-ref.yamlRepository: openshift/release
Length of output: 523
Soften git fetch for commit SHA refs to prevent presubmit failures.
When PULL_PULL_SHA is used (standard for presubmits), ${ref} is a commit SHA. Git cannot fetch raw commit SHAs directly—only branch/tag names. The hard-fail on line 13 will always fail for SHA refs and break the step, even though the commit is already available locally after clonerefs clones the PR. The fallback also fails for the same reason.
Change line 13-14 to soft-fail all fetches and verify the ref exists locally before checkout:
Suggested fix
- git fetch --tags origin "${ref}" 2>/dev/null || git fetch origin "${ref}"
- git fetch --unshallow origin 2>/dev/null || true
+ git fetch --tags origin 2>/dev/null || true
+ git fetch --unshallow origin 2>/dev/null || true
+ git fetch origin "${ref}" 2>/dev/null || true
+ git rev-parse --verify --quiet "${ref}^{commit}" >/dev/null || {
+ echo "ERROR: ref ${ref} is not available locally after fetch"
+ exit 1
+ }📝 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.
| git fetch --tags origin "${ref}" 2>/dev/null || git fetch origin "${ref}" | |
| git fetch --unshallow origin 2>/dev/null || true | |
| git fetch --tags origin 2>/dev/null || true | |
| git fetch --unshallow origin 2>/dev/null || true | |
| git fetch origin "${ref}" 2>/dev/null || true | |
| git rev-parse --verify --quiet "${ref}^{commit}" >/dev/null || { | |
| echo "ERROR: ref ${ref} is not available locally after fetch" | |
| exit 1 | |
| } |
🤖 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/git/checkout-head/aro-hcp-git-checkout-head-commands.sh`
around lines 13 - 14, The git fetch commands on lines 13-14 fail hard when the
ref is a commit SHA (from PULL_PULL_SHA in presubmits) because git cannot fetch
raw commit SHAs directly. Since the commit is already available locally after
clonerefs clones the PR, change both git fetch commands to soft-fail by
appending || true to each command, and add a verification step after the fetches
to check if the ref exists locally (using git rev-parse --verify "${ref}")
before attempting the git checkout to ensure the ref is available.
| git fetch --tags origin "${ref}" 2>/dev/null || git fetch origin "${ref}" | ||
| git fetch --unshallow origin 2>/dev/null || true | ||
| git checkout "${ref}" || { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify this script advertises SHA support but currently hard-fails on fetch.
rg -n 'Git ref to checkout \(branch, tag, or SHA\)|git fetch --tags origin "\$\{ref\}"|git fetch origin "\$\{ref\}"|git checkout "\$\{ref\}"' \
ci-operator/step-registry/aro-hcp/git/checkout/aro-hcp-git-checkout-ref.yaml \
ci-operator/step-registry/aro-hcp/git/checkout/aro-hcp-git-checkout-commands.shRepository: openshift/release
Length of output: 489
🏁 Script executed:
cat -n ci-operator/step-registry/aro-hcp/git/checkout/aro-hcp-git-checkout-commands.shRepository: openshift/release
Length of output: 721
🏁 Script executed:
# Check for similar fetch patterns in step registry
rg 'git fetch.*\$\{ref\}.*\|\|' ci-operator/step-registry/ -A1 -B1
# Also check if there's any documentation about git fetch limitations with SHAs
rg 'SHA|commit.*hash|fetch.*tag' ci-operator/step-registry/aro-hcp/git/checkout/ -A2 -B2Repository: openshift/release
Length of output: 2525
🏁 Script executed:
# Check if these are the only two occurrences and examine the checkout-head variant
cat -n ci-operator/step-registry/aro-hcp/git/checkout-head/aro-hcp-git-checkout-head-commands.shRepository: openshift/release
Length of output: 794
Fetch pattern hard-fails on SHA refs when remote rejects SHA-based fetches.
The step documentation explicitly allows SHA refs ("Git ref to checkout (branch, tag, or SHA)"), but line 12 fails hard if the remote does not support fetching by SHA. With set -o errexit, the script exits before checkout can try resolving a valid local commit.
Make all fetch attempts best-effort, then explicitly verify the ref is locally resolvable:
Suggested patch
- git fetch --tags origin "${ref}" 2>/dev/null || git fetch origin "${ref}"
+ git fetch --tags origin 2>/dev/null || true
+ git fetch origin "${ref}" 2>/dev/null || true
+ git rev-parse --verify --quiet "${ref}^{commit}" >/dev/null || {
+ echo "ERROR: ref ${ref} is not available locally after fetch"
+ exit 1
+ }Note: The same pattern exists in aro-hcp-git-checkout-head-commands.sh.
🤖 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/git/checkout/aro-hcp-git-checkout-commands.sh`
around lines 12 - 14, The git fetch command on line 12 fails hard when the
remote does not support SHA-based fetches, causing the entire script to exit due
to set -o errexit before the git checkout command can attempt to resolve a valid
local commit. Make both fetch attempts best-effort by ensuring they do not cause
script failure (using || true pattern), then add an explicit verification step
that validates the ref is locally resolvable before attempting the git checkout
on line 14. Apply the same pattern fix to aro-hcp-git-checkout-head-commands.sh
which has the identical issue.
| - name: ARO_HCP_DEPLOY_ENV | ||
| default: "prow" | ||
| documentation: Config environment name. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate upgrade workflow and job config:"
fd 'aro-hcp-local-e2e-upgrade-workflow.yaml'
fd 'Azure-ARO-HCP-main.yaml'
echo
echo "Inspect deploy env wiring across pre/post refs, workflow, and job:"
rg -n -C2 'ARO_HCP_DEPLOY_ENV|aro-hcp-test-local-pre-upgrade|aro-hcp-test-local-post-upgrade|aro-hcp-local-e2e-upgrade' \
ci-operator/step-registry/aro-hcp/test/local-pre-upgrade/aro-hcp-test-local-pre-upgrade-ref.yaml \
ci-operator/step-registry/aro-hcp/test/local-post-upgrade/aro-hcp-test-local-post-upgrade-ref.yaml \
ci-operator/step-registry/aro-hcp/local-e2e-upgrade/aro-hcp-local-e2e-upgrade-workflow.yaml \
ci-operator/config/Azure/ARO-HCP/Azure-ARO-HCP-main.yamlRepository: openshift/release
Length of output: 12036
🏁 Script executed:
# Check for other jobs using the aro-hcp-local-e2e-upgrade workflow
rg -n 'aro-hcp-local-e2e-upgrade' ci-operator/config --type yaml
# Check if post-upgrade step is referenced elsewhere
rg -n 'aro-hcp-test-local-post-upgrade' ci-operator --type yamlRepository: openshift/release
Length of output: 598
Align ARO_HCP_DEPLOY_ENV defaults between pre/post upgrade steps.
Line 30 defaults to "prow" here, while the pre-upgrade step defaults to "ci00". This misalignment can cause the post-upgrade step to pick the wrong infra subscription config. Although the single job using this workflow overrides the default to ci01 at the job level, the inconsistency risks failures if the post-upgrade step is reused in other workflows or jobs without an explicit override.
Suggested fix
- - name: ARO_HCP_DEPLOY_ENV
- default: "prow"
+ - name: ARO_HCP_DEPLOY_ENV
+ default: "ci00"📝 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.
| - name: ARO_HCP_DEPLOY_ENV | |
| default: "prow" | |
| documentation: Config environment name. | |
| - name: ARO_HCP_DEPLOY_ENV | |
| default: "ci00" | |
| documentation: Config environment name. |
🤖 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/test/local-post-upgrade/aro-hcp-test-local-post-upgrade-ref.yaml`
around lines 30 - 32, The ARO_HCP_DEPLOY_ENV parameter in the post-upgrade step
defaults to "prow" but should be aligned with the pre-upgrade step which
defaults to "ci00". Change the default value of the ARO_HCP_DEPLOY_ENV variable
from "prow" to "ci00" to ensure consistency between the pre-upgrade and
post-upgrade workflow steps and prevent configuration mismatches when this step
is reused in other workflows.
| junit_shared_name="${E2E_JUNIT_SHARED_NAME:-junit-e2e-suite.xml.gz}" | ||
| gzip -c "${ARTIFACT_DIR}/junit.xml" > "${SHARED_DIR}/${junit_shared_name}" |
There was a problem hiding this comment.
Use a pre-upgrade-specific fallback JUnit artifact name.
The current fallback can collide with post-upgrade’s command-level fallback (junit-e2e-suite.xml.gz) when E2E_JUNIT_SHARED_NAME isn’t set, which risks artifact overwrite.
Suggested patch
-junit_shared_name="${E2E_JUNIT_SHARED_NAME:-junit-e2e-suite.xml.gz}"
+junit_shared_name="${E2E_JUNIT_SHARED_NAME:-junit-e2e-pre-upgrade.xml.gz}"
gzip -c "${ARTIFACT_DIR}/junit.xml" > "${SHARED_DIR}/${junit_shared_name}"📝 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.
| junit_shared_name="${E2E_JUNIT_SHARED_NAME:-junit-e2e-suite.xml.gz}" | |
| gzip -c "${ARTIFACT_DIR}/junit.xml" > "${SHARED_DIR}/${junit_shared_name}" | |
| junit_shared_name="${E2E_JUNIT_SHARED_NAME:-junit-e2e-pre-upgrade.xml.gz}" | |
| gzip -c "${ARTIFACT_DIR}/junit.xml" > "${SHARED_DIR}/${junit_shared_name}" |
🤖 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/test/local-pre-upgrade/aro-hcp-test-local-pre-upgrade-commands.sh`
around lines 53 - 54, The fallback value for the junit_shared_name variable on
line 53 uses the same default name as the post-upgrade step's fallback, which
causes artifact collision when E2E_JUNIT_SHARED_NAME is not explicitly set.
Change the default fallback from "junit-e2e-suite.xml.gz" to a
pre-upgrade-specific name (for example, by adding "pre-upgrade" to the filename)
so that the pre-upgrade and post-upgrade steps each write to distinct artifact
files and prevent the post-upgrade from overwriting the pre-upgrade output.
Source: Learnings
3b0a609 to
86d532f
Compare
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
ARO-27647
What
Adds an optional presubmit job and step-registry workflow for in-place Hypershift infrastructure upgrade E2E on ARO-HCP.
New job: e2e-parallel-hypershift-upgrade → workflow aro-hcp-local-e2e-upgrade
Workflow phases:
New step-registry components:
Cross-step state: both test steps set SETUP_FILEPATH=${SHARED_DIR}/e2e-setup.json so cluster identity written by upgrade/create is available to upgrade/post-infra after git-checkout-head.
Leasing: dedicated Boskos type aro-hcp-dev-upgrade-westus3-slot (1 slot); job restricted to ARO HCP E2E Hosted Clusters - Dev - 03 / westus3.
Why
We need CI coverage that validates in-place Hypershift operator upgrades roll node pools as expected — not just greenfield provision on a single ref. The multi-step design separates concerns:
A dedicated lease slot (slot_count: 1) keeps the same regional environment isolated across both test phases without interference from concurrent E2E jobs.
The job is optional until companion work lands in Azure/ARO-HCP: upgrade/create and upgrade/post-infra test suites, slot catalog entry for aro-hcp-dev-upgrade-westus3-slot, and SETUP_FILEPATH support in the test framework (draft Azure/ARO-HCP#5725).
Summary by CodeRabbit
This PR adds an optional ARO-HCP CI “presubmit” job to exercise in-place Hypershift infrastructure upgrades end-to-end using step-registry automation. The new job
e2e-parallel-hypershift-upgraderuns thearo-hcp-local-e2e-upgradeworkflow, provisioning a stable baseline onmain, upgrading with PR-head changes, and validating results before teardown.New CI job (optional)
e2e-parallel-hypershift-upgradeunder the Azure ARO-HCP CI test configuration.run_if_changedmatchesdev-infrastructure/,config/, paths containinghypershift, or.ci-operator.yaml.aro-hcp-local-e2e-upgradeworkflow with ARO-HCP deployment-related environment (includingALLOWED_SUBSCRIPTIONS) and a12htimeout.New workflow:
aro-hcp-local-e2e-upgradeImplements a three-phase upgrade test lifecycle:
main), write configuration, and provision regional infra.New step-registry components
Adds five building blocks used by the workflow:
aro-hcp-git-checkout: base ref checkout (defaultmain), recording checked-out SHA/ref for later phases.aro-hcp-git-checkout-head: PR-head checkout right before the upgrade step.aro-hcp-test-local-pre-upgrade: runsupgrade/createwithARO_E2E_SKIP_CLEANUPenabled and producese2e-setup.json.aro-hcp-upgrade-infra: performs the in-place Hypershift infrastructure upgrade by running theRP.HypershiftOperatorpipeline with an upgrade override derived from Hypershift defaults; writes a provisioning marker/timestamp.aro-hcp-test-local-post-upgrade: runsupgrade/post-infra, verifies expected hash changes, captures JUnit artifacts, and handles cluster cleanup.Cross-step state is persisted via
SETUP_FILEPATH=${SHARED_DIR}/e2e-setup.json, so the cluster identity captured during upgrade/create is available after git checkout for the post-upgrade validation.Dedicated Boskos leasing for the upgrade E2E
aro-hcp-dev-upgrade-westus3-slotwith a single slot (aro-hcp-dev-upgrade-westus3-slot-00).Ownership/metadata wiring
Adds/updates
OWNERSand*.metadata.jsonfor the new workflow and step-registry entries to set explicit approvers/reviewers.Dependency on companion repo work
The job remains optional until the Azure/ARO-HCP repository lands the referenced upgrade test suites, the lease catalog entry, and support for the
SETUP_FILEPATHshared setup mechanism.