[WIP] refactor(ci): remove hardcoded job map and update version matrices in multi-version trigger#77844
[WIP] refactor(ci): remove hardcoded job map and update version matrices in multi-version trigger#77844mgencur wants to merge 3 commits intoopenshift:mainfrom
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplace static job mappings with computed AWS job names per guest version, simplify multi-version maps to single-entry mappings, expose runtime/config variables (dirs, batching, token path, Gangway API), and update Gangway trigger/polling to read the bearer token from a configurable path. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mgencur 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 |
|
/pj-rehearse periodic-ci-openshift-hypershift-main-mce-multi-version-aws |
|
@mgencur: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh (1)
42-42:⚠️ Potential issue | 🟠 MajorLocal
GANGWAY_APIshadows the global variable, causing inconsistent behavior.The local declaration here shadows the global
GANGWAY_APIdefined on line 15. This means:
trigger_prow_job()always uses the hardcoded URLcheck_jobs()(line 88) uses the configurable global variableIf
GANGWAY_APIis overridden via environment, jobs will be triggered against the hardcoded endpoint but status will be checked against the override—potentially different endpoints.🔧 Proposed fix: Remove the local declaration
function trigger_prow_job() { - local GANGWAY_API='https://gangway-ci.apps.ci.l2s4.p1.openshiftapps.com' - local _job_name="$1"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh` at line 42, The GANGWAY_API variable is being shadowed by a local declaration which causes trigger_prow_job() to use the hardcoded URL while check_jobs() uses the global/overridden value; remove the local declaration "local GANGWAY_API='https://gangway-ci.apps.ci.l2s4.p1.openshiftapps.com'" so both trigger_prow_job() and check_jobs() reference the same global GANGWAY_API (or alternatively rename the local to a different identifier if a distinct value is required), ensuring environment overrides take effect consistently.
🧹 Nitpick comments (1)
ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh (1)
180-182: Misleading comment:JOB_DURATIONis not in seconds.The comment says "sleeping for ${JOB_DURATION} seconds" but
JOB_DURATIONdefaults to"2.5h"(hours). Whilesleephandles duration suffixes correctly, the comment is confusing.📝 Suggested fix
if ((job_count > JOB_PARALLEL)); then - echo "Reached $JOB_PARALLEL jobs, sleeping for ${JOB_DURATION} seconds..." + echo "Reached $JOB_PARALLEL jobs, sleeping for ${JOB_DURATION}..." sleep "${JOB_DURATION}"Apply the same fix to line 190.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh` around lines 180 - 182, The log message incorrectly states seconds while JOB_DURATION can include duration suffixes (e.g., "2.5h"); update the two echo lines that reference JOB_DURATION (the one inside the if guarding job_count > JOB_PARALLEL and the similar echo at the other occurrence around line 190) to remove "seconds" and instead print the raw ${JOB_DURATION} (e.g., "sleeping for ${JOB_DURATION}...") so the message accurately reflects the value used by sleep.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh`:
- Line 42: The GANGWAY_API variable is being shadowed by a local declaration
which causes trigger_prow_job() to use the hardcoded URL while check_jobs() uses
the global/overridden value; remove the local declaration "local
GANGWAY_API='https://gangway-ci.apps.ci.l2s4.p1.openshiftapps.com'" so both
trigger_prow_job() and check_jobs() reference the same global GANGWAY_API (or
alternatively rename the local to a different identifier if a distinct value is
required), ensuring environment overrides take effect consistently.
---
Nitpick comments:
In
`@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh`:
- Around line 180-182: The log message incorrectly states seconds while
JOB_DURATION can include duration suffixes (e.g., "2.5h"); update the two echo
lines that reference JOB_DURATION (the one inside the if guarding job_count >
JOB_PARALLEL and the similar echo at the other occurrence around line 190) to
remove "seconds" and instead print the raw ${JOB_DURATION} (e.g., "sleeping for
${JOB_DURATION}...") so the message accurately reflects the value used by sleep.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 937dd3ea-c00a-41ac-ab52-e677ac549254
📒 Files selected for processing (1)
ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh
mgencur
force-pushed
the
fix_mce_compatibility_test_trigger
branch
from
5ccb5d3 to
1817578
Compare
mgencur
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh (1)
167-185:⚠️ Potential issue | 🟠 MajorFail fast for unsupported platforms before reading
$result.The script has
set -o nounset(line 3), which will cause it to exit when accessing undefined variables. Only theawsbranch (line 175) assignsresult, but line 184 unconditionally reads it. With any platform other thanaws, the script will crash with an unbound variable error.Add
result=""at the start of the loop and usecontinueto skip further processing for unsupported platforms, or validate the platform earlier:Proposed fix
for guest_version in $guest_versions; do + result="" case $HOSTEDCLUSTER_PLATFORM in aws) job_name="periodic-ci-openshift-hypershift-release-${guest_version}-periodics-mce-e2e-aws-critical" post_data=$(jq -n --arg mce_version "$mce_version" \ --arg release_image "${payload_list[$hub_version]}" \ '{job_execution_type: "1", pod_spec_options: {envs: {MULTISTAGE_PARAM_OVERRIDE_MCE_VERSION: $mce_version, RELEASE_IMAGE_LATEST: $release_image, MULTISTAGE_PARAM_OVERRIDE_TEST_EXTRACT: "true"}}}') result=$(trigger_prow_job "$job_name" "$post_data") ;; agent) - `#todo` + echo "HOSTEDCLUSTER_PLATFORM=agent is not implemented yet" >&2 + continue ;; *) - echo "not support ${HOSTEDCLUSTER_PLATFORM}" + echo "unsupported HOSTEDCLUSTER_PLATFORM=${HOSTEDCLUSTER_PLATFORM}" >&2 + continue ;; esac🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh` around lines 167 - 185, The loop over guest_versions reads $result unconditionally but only the aws branch assigns it, causing an unbound-variable error under set -o nounset; fix by initializing result="" (and job_name="") at the top of the loop (inside the for loop) and ensure unsupported branches (the agent and *) either set result="" or use continue after echo so the code does not proceed to read $result and compute job_id; update the case for agent to set result="" (or perform its trigger) or add a continue in that branch and keep the job_id/job_list append only for branches that actually trigger a job.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh`:
- Around line 14-15: The script currently defines GANGWAY_API but then hardcodes
a different Gangway URL inside the trigger_prow_job function, causing POST and
check_jobs to hit different endpoints; edit trigger_prow_job to remove the
hardcoded URL and use the GANGWAY_API variable consistently for the POST
endpoint (and any subsequent calls within trigger_prow_job), ensuring check_jobs
and trigger_prow_job both use the same GANGWAY_API value.
- Around line 25-27: The script currently assumes mce_to_guest contains entries
for every hub_to_mce mapping (variables: hub_to_mce, mce_to_guest and index
mce_version), but mce_to_guest lacks keys for the MCE versions produced (e.g.,
"2.6","2.7") so the strict set -o nounset causes an unbound variable error when
expanding ${mce_to_guest[$mce_version]}; fix by guarding the lookup—before using
${mce_to_guest[$mce_version]} check its existence with a safe test like if [ -n
"${mce_to_guest[$mce_version]:-}" ] or use the parameter-expansion existence
test ${mce_to_guest[$mce_version]+x} and continue/skip the iteration when
missing so unsupported version pairs are skipped rather than causing the script
to exit.
---
Outside diff comments:
In
`@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh`:
- Around line 167-185: The loop over guest_versions reads $result
unconditionally but only the aws branch assigns it, causing an unbound-variable
error under set -o nounset; fix by initializing result="" (and job_name="") at
the top of the loop (inside the for loop) and ensure unsupported branches (the
agent and *) either set result="" or use continue after echo so the code does
not proceed to read $result and compute job_id; update the case for agent to set
result="" (or perform its trigger) or add a continue in that branch and keep the
job_id/job_list append only for branches that actually trigger a job.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f017b74d-5f11-4a0f-a77a-0e82f007814e
📒 Files selected for processing (1)
ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh
… multi-version trigger Dynamically construct job names from guest version. Update hub/MCE/guest version maps to current releases (4.16-4.22). Extract config into env vars. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mgencur
force-pushed
the
fix_mce_compatibility_test_trigger
branch
from
1817578 to
9f52aaa
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh (2)
182-184:⚠️ Potential issue | 🟡 MinorMisleading log message:
JOB_DURATIONis in hours, not seconds.
JOB_DURATIONdefaults to"2.5h", but the echo message says "sleeping for ${JOB_DURATION} seconds". The same issue exists on line 192.Proposed fix
if ((job_count > JOB_PARALLEL)); then - echo "Reached $JOB_PARALLEL jobs, sleeping for ${JOB_DURATION} seconds..." + echo "Reached $JOB_PARALLEL jobs, sleeping for ${JOB_DURATION}..." sleep "${JOB_DURATION}" job_count=1 fiAnd for line 192:
if ((job_count > 1)); then - echo "Final batch, sleeping for ${JOB_DURATION} seconds..." + echo "Final batch, sleeping for ${JOB_DURATION}..." sleep "${JOB_DURATION}" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh` around lines 182 - 184, The log message incorrectly states seconds while JOB_DURATION is an hours string (e.g., "2.5h"); update the echo lines that reference JOB_DURATION (the block using job_count, JOB_PARALLEL and JOB_DURATION) to reflect the correct unit (hours) or convert JOB_DURATION to seconds before printing; fix both occurrences (the sleep/echo block that checks if ((job_count > JOB_PARALLEL)) and the similar echo at line ~192) so the message accurately shows the duration unit or the converted value.
160-178:⚠️ Potential issue | 🟠 MajorUnset
resultvariable will cause script failure for non-AWS platforms.With
set -o nounset(line 3), ifHOSTEDCLUSTER_PLATFORMisagentor an unsupported value,resultis never assigned but is referenced on line 177. This will trigger an "unbound variable" error and abort the script.Proposed fix
guest_versions="${mce_to_guest[$mce_version]}" - job_name="" + job_name="" + result="" for guest_version in $guest_versions; do case $HOSTEDCLUSTER_PLATFORM in aws) job_name="periodic-ci-openshift-hypershift-release-${guest_version}-periodics-mce-e2e-aws-critical" post_data=$(jq -n --arg mce_version "$mce_version" \ --arg release_image "${payload_list[$hub_version]}" \ '{job_execution_type: "1", pod_spec_options: {envs: {MULTISTAGE_PARAM_OVERRIDE_MCE_VERSION: $mce_version, RELEASE_IMAGE_LATEST: $release_image, MULTISTAGE_PARAM_OVERRIDE_TEST_EXTRACT: "true"}}}') result=$(trigger_prow_job "$job_name" "$post_data") ;; agent) - `#todo` + job_name="agent-platform-job-placeholder" + echo "Agent platform not yet implemented, skipping" + continue ;; *) echo "not support ${HOSTEDCLUSTER_PLATFORM}" + continue ;; esac🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh` around lines 160 - 178, The script fails with nounset because result is only assigned in the aws case; initialize or set a safe default for result before the case (e.g., result="") or ensure all case branches (agent and *) assign result (for example an empty JSON or an error message) so the later job_id extraction (job_id=$(echo "$result" | grep "JOB_ID###" ...)) cannot reference an unbound variable; update the loop around guest_versions where result and job_name are set to guarantee result is always defined regardless of HOSTEDCLUSTER_PLATFORM.
♻️ Duplicate comments (1)
ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh (1)
43-48:⚠️ Potential issue | 🟠 MajorRemove the hardcoded
GANGWAY_APIoverride to use the configurable global value.Line 44 declares a local
GANGWAY_APIthat shadows the global configurable variable from Line 15. This causestrigger_prow_jobandcheck_jobsto potentially use different API endpoints whenGANGWAY_APIis overridden externally.Proposed fix
function trigger_prow_job() { - local GANGWAY_API='https://gangway-ci.apps.ci.l2s4.p1.openshiftapps.com' - local _job_name="$1" local _http_post_data="$2"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh` around lines 43 - 48, The function trigger_prow_job currently shadows the global GANGWAY_API by declaring a local GANGWAY_API; remove that local declaration so trigger_prow_job uses the globally configurable GANGWAY_API (consistent with check_jobs) — locate the trigger_prow_job function and delete the line "local GANGWAY_API='...'" (or stop redeclaring GANGWAY_API as local) so the global variable is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh`:
- Around line 182-184: The log message incorrectly states seconds while
JOB_DURATION is an hours string (e.g., "2.5h"); update the echo lines that
reference JOB_DURATION (the block using job_count, JOB_PARALLEL and
JOB_DURATION) to reflect the correct unit (hours) or convert JOB_DURATION to
seconds before printing; fix both occurrences (the sleep/echo block that checks
if ((job_count > JOB_PARALLEL)) and the similar echo at line ~192) so the
message accurately shows the duration unit or the converted value.
- Around line 160-178: The script fails with nounset because result is only
assigned in the aws case; initialize or set a safe default for result before the
case (e.g., result="") or ensure all case branches (agent and *) assign result
(for example an empty JSON or an error message) so the later job_id extraction
(job_id=$(echo "$result" | grep "JOB_ID###" ...)) cannot reference an unbound
variable; update the loop around guest_versions where result and job_name are
set to guarantee result is always defined regardless of HOSTEDCLUSTER_PLATFORM.
---
Duplicate comments:
In
`@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh`:
- Around line 43-48: The function trigger_prow_job currently shadows the global
GANGWAY_API by declaring a local GANGWAY_API; remove that local declaration so
trigger_prow_job uses the globally configurable GANGWAY_API (consistent with
check_jobs) — locate the trigger_prow_job function and delete the line "local
GANGWAY_API='...'" (or stop redeclaring GANGWAY_API as local) so the global
variable is used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a350ed8c-ac9e-45c7-bda9-e1d2c7134d30
📒 Files selected for processing (1)
ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh
|
/pj-rehearse periodic-ci-openshift-hypershift-main-mce-multi-version-aws |
|
@mgencur: 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: |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh (1)
161-179:⚠️ Potential issue | 🟠 MajorGuard
$resultfor non-AWS branches to prevent nounset abort.In
agent/unsupported branches,resultis never assigned, but Line 178 always expands$result. Withset -o nounset, this exits the script.Proposed fix
for mce_version in $mce_versions; do guest_versions="${mce_to_guest[$mce_version]}" job_name="" + result="" for guest_version in $guest_versions; do case $HOSTEDCLUSTER_PLATFORM in aws) job_name="periodic-ci-openshift-hypershift-release-${guest_version}-periodics-mce-e2e-aws-critical" post_data=$(jq -n --arg mce_version "$mce_version" \ --arg release_image "${payload_list[$hub_version]}" \ '{job_execution_type: "1", pod_spec_options: {envs: {MULTISTAGE_PARAM_OVERRIDE_MCE_VERSION: $mce_version, RELEASE_IMAGE_LATEST: $release_image, MULTISTAGE_PARAM_OVERRIDE_TEST_EXTRACT: "true"}}}') result=$(trigger_prow_job "$job_name" "$post_data") ;; agent) - `#todo` + echo "Platform ${HOSTEDCLUSTER_PLATFORM} is not implemented yet; skipping." + continue ;; *) - echo "not support ${HOSTEDCLUSTER_PLATFORM}" + echo "not support ${HOSTEDCLUSTER_PLATFORM}, skipping" + continue ;; esac job_id=$(echo "$result" | grep "JOB_ID###" | cut -d'#' -f4 || true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh` around lines 161 - 179, The loop assumes $result is set after the case but in non-AWS branches (case patterns "agent" or default) it isn’t, causing nounset failures when expanding result; ensure result is always defined before using it by either setting a default (e.g. empty) at the top of the loop or by moving the job_id extraction/echo that references result inside the aws branch (the block that calls trigger_prow_job and sets job_name). Update references to result, job_name, HOSTEDCLUSTER_PLATFORM and trigger_prow_job accordingly so job_id is only parsed when trigger_prow_job was invoked or result has been initialized.
♻️ Duplicate comments (1)
ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh (1)
14-15:⚠️ Potential issue | 🟠 MajorRemove local
GANGWAY_APIshadowing so overrides work consistently.Line 45 re-hardcodes
GANGWAY_API, which bypasses the configurable value from Line 15. Trigger and polling can hit different endpoints when overridden.Proposed fix
function trigger_prow_job() { - local GANGWAY_API='https://gangway-ci.apps.ci.l2s4.p1.openshiftapps.com' - local _job_name="$1" local _http_post_data="$2"Also applies to: 45-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh` around lines 14 - 15, The GANGWAY_API default is being redefined later which shadows any overrides; remove the second hardcoded assignment so the script uses the initial default/overridden GANGWAY_API (referenced as GANGWAY_API and ${GANGWAY_API:-...}) everywhere (including the trigger and polling code paths) and ensure any uses reference the single variable instead of assigning a new value; if needed, replace the later assignment with a simple reference to GANGWAY_API or parameter expansion to preserve external overrides.
🧹 Nitpick comments (1)
ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh (1)
8-13: Create fallback directories when using default paths.When running outside ci-operator, default
/tmp/shared_dirand/tmp/artifactsmay not exist, causing later writes to fail.Proposed fix
export SHARED_DIR=${SHARED_DIR:-/tmp/shared_dir} export ARTIFACT_DIR=${ARTIFACT_DIR:-/tmp/artifacts} export HOSTEDCLUSTER_PLATFORM=${HOSTEDCLUSTER_PLATFORM:-"aws"} export JOB_PARALLEL=${JOB_PARALLEL:-"5"} export JOB_DURATION=${JOB_DURATION:-"2.5h"} + +mkdir -p "${SHARED_DIR}" "${ARTIFACT_DIR}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh` around lines 8 - 13, The script sets SHARED_DIR and ARTIFACT_DIR defaults but doesn't ensure those paths exist; after the exports for SHARED_DIR and ARTIFACT_DIR, add directory creation using mkdir -p "$SHARED_DIR" "$ARTIFACT_DIR" and check the command's exit status (or use set -e) so writes won't fail later; if mkdir fails, print a short error message and exit non‑zero to fail fast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh`:
- Around line 17-25: The matrices mce_to_guest and hub_to_mce currently contain
only a single mapping, which disables multi-version coverage; update the
declarations for mce_to_guest and hub_to_mce to include the full set of versions
(e.g., map each MCE key to the latest three HostedCluster guest versions in
mce_to_guest and map each hub version to the MCE and two older MCE versions in
hub_to_mce) so the script iterates over multiple hub/MCE/guest combinations;
locate the arrays named mce_to_guest and hub_to_mce and expand their entries to
the intended multi-version lists.
---
Outside diff comments:
In
`@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh`:
- Around line 161-179: The loop assumes $result is set after the case but in
non-AWS branches (case patterns "agent" or default) it isn’t, causing nounset
failures when expanding result; ensure result is always defined before using it
by either setting a default (e.g. empty) at the top of the loop or by moving the
job_id extraction/echo that references result inside the aws branch (the block
that calls trigger_prow_job and sets job_name). Update references to result,
job_name, HOSTEDCLUSTER_PLATFORM and trigger_prow_job accordingly so job_id is
only parsed when trigger_prow_job was invoked or result has been initialized.
---
Duplicate comments:
In
`@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh`:
- Around line 14-15: The GANGWAY_API default is being redefined later which
shadows any overrides; remove the second hardcoded assignment so the script uses
the initial default/overridden GANGWAY_API (referenced as GANGWAY_API and
${GANGWAY_API:-...}) everywhere (including the trigger and polling code paths)
and ensure any uses reference the single variable instead of assigning a new
value; if needed, replace the later assignment with a simple reference to
GANGWAY_API or parameter expansion to preserve external overrides.
---
Nitpick comments:
In
`@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh`:
- Around line 8-13: The script sets SHARED_DIR and ARTIFACT_DIR defaults but
doesn't ensure those paths exist; after the exports for SHARED_DIR and
ARTIFACT_DIR, add directory creation using mkdir -p "$SHARED_DIR"
"$ARTIFACT_DIR" and check the command's exit status (or use set -e) so writes
won't fail later; if mkdir fails, print a short error message and exit non‑zero
to fail fast.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 67c35192-0aa7-46fc-bd23-5534eec6999f
📒 Files selected for processing (1)
ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh
| # Each MCE supports the latest three HostedCluster versions | ||
| declare -A mce_to_guest=( | ||
| [2.4]="4.14" | ||
| [2.5]="4.14 4.15" | ||
| [2.6]="4.14 4.15 4.16" | ||
| [2.7]="4.14 4.15 4.16 4.17" | ||
| [2.8]="4.14 4.15 4.16 4.17 4.18" | ||
| [2.9]="4.15 4.16 4.17 4.18 4.19" | ||
| [2.17]="4.22" | ||
| ) | ||
|
|
||
| # Each MCE is available on the latest hub version and two versions back | ||
| declare -A hub_to_mce=( | ||
| [4.14]="2.4 2.5 2.6" | ||
| [4.15]="2.5 2.6 2.7" | ||
| [4.16]="2.6 2.7 2.8" | ||
| [4.17]="2.7 2.8 2.9" | ||
| [4.18]="2.8 2.9" | ||
| [4.19]="2.9" | ||
| [4.22]="2.17" | ||
| ) |
There was a problem hiding this comment.
The “multi-version” matrices are currently single-version only.
Lines 19 and 24 reduce coverage to one hub/MCE/guest path (4.22/2.17/4.22). This effectively disables multi-version validation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ci-operator/step-registry/hypershift/mce/multi-version-test/trigger-jobs/hypershift-mce-multi-version-test-trigger-jobs-commands.sh`
around lines 17 - 25, The matrices mce_to_guest and hub_to_mce currently contain
only a single mapping, which disables multi-version coverage; update the
declarations for mce_to_guest and hub_to_mce to include the full set of versions
(e.g., map each MCE key to the latest three HostedCluster guest versions in
mce_to_guest and map each hub version to the MCE and two older MCE versions in
hub_to_mce) so the script iterates over multiple hub/MCE/guest combinations;
locate the arrays named mce_to_guest and hub_to_mce and expand their entries to
the intended multi-version lists.
|
@mgencur: 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
Dynamically construct job names from guest version. Update hub/MCE/guest
version maps to current releases (4.16-4.22). Extract config into env vars.
Summary by CodeRabbit