50 hack debug - #82599
Conversation
WalkthroughChangesDisconnected MCO rehearsal
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (13 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 |
|
/pj-rehearse periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-ipi-disc-priv-tp-pin-mco periodic-ci-openshift-machine-config-operator-release-4.23-periodics-e2e-aws-ipi-disc-priv-tp-pin-mco |
|
@sergiordlr: 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.
Actionable comments posted: 2
🤖 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/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yaml`:
- Line 437: Replace the mutable latest release image override with an immutable
digest or durable promoted release image in both
CUSTOM_OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE entries:
ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yaml:437
and
ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-5.0__periodics.yaml:472.
Keep the machine-config-operator periodic test configuration unchanged
otherwise.
In
`@ci-operator/step-registry/set-sample-operator/disconnected/set-sample-operator-disconnected-commands.sh`:
- Around line 95-99: Update the new-API guard around the
ImageDigestMirrorSet/ImageTagMirrorSet creation flow to skip only when both
resources exist; retain the legacy ImageContentSourcePolicy handling as
appropriate. Replace the non-idempotent creation path with oc apply so a missing
ImageTagMirrorSet is created when ImageDigestMirrorSet already exists, including
the corresponding logic in the later resource-creation block.
🪄 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: f5aeb3c0-bd4e-4bb2-941c-964e76db8490
⛔ Files ignored due to path filters (3)
ci-operator/jobs/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23-periodics.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/machine-config-operator/openshift-machine-config-operator-release-5.0-periodics.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/openshift-tests-private/openshift-openshift-tests-private-release-5.0-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (8)
ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yamlci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-5.0__periodics.yamlci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-5.0__automated-release-nightly-5.0-upgrade-from-stable-4.22.yamlci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-5.0__automated-release-nightly-5.0-upgrade-from-stable-5.0.yamlci-operator/step-registry/cucushift/installer/rehearse/aws/ipi/disconnected/private/provision/cucushift-installer-rehearse-aws-ipi-disconnected-private-provision-chain.yamlci-operator/step-registry/enable-qe-catalogsource/disconnected/enable-qe-catalogsource-disconnected-commands.shci-operator/step-registry/enable-qe-catalogsource/disconnected/enable-qe-catalogsource-disconnected-ref.yamlci-operator/step-registry/set-sample-operator/disconnected/set-sample-operator-disconnected-commands.sh
💤 Files with no reviewable changes (3)
- ci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-5.0__automated-release-nightly-5.0-upgrade-from-stable-5.0.yaml
- ci-operator/step-registry/cucushift/installer/rehearse/aws/ipi/disconnected/private/provision/cucushift-installer-rehearse-aws-ipi-disconnected-private-provision-chain.yaml
- ci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-5.0__automated-release-nightly-5.0-upgrade-from-stable-4.22.yaml
| dependencies: | ||
| OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE: release:initial | ||
| env: | ||
| CUSTOM_OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE: registry.build10.ci.openshift.org/ci-ln-xitccvk/release:latest |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
if rg -n 'CUSTOM_OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE:.*:latest' ci-operator/config; then
echo "Mutable release-image override remains" >&2
exit 1
fiRepository: openshift/release
Length of output: 1835
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
for f in \
ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yaml \
ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-5.0__periodics.yaml
do
echo "===== $f ====="
rg -n 'name:|cron:|interval:|periodics:|test:|CUSTOM_OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE:|CUSTOM_OPENSHIFT_INSTALL_RELEASE_IMAGE:' "$f" | sed -n '1,180p'
doneRepository: openshift/release
Length of output: 2335
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
for f in \
ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yaml \
ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-5.0__periodics.yaml
do
echo "===== contexts around line 437/472 in $f ====="
sed -n '412,475p' "$f" | nl -ba -v412
doneRepository: openshift/release
Length of output: 355
Do not use a mutable build image for recurring pinning tests.
Both machine-config-operator periodics run on 72h/weekly schedules but override the release image with registry.build${N}.ci.openshift.org/ci-ln-xitccvk/release:latest. Move these to an immutable digest or durable promoted release image; otherwise the release-under-test can change between runs or fail if the build namespace image is removed.
ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yaml#L437ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-5.0__periodics.yaml#L472
📍 Affects 2 files
ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yaml#L437-L437(this comment)ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-5.0__periodics.yaml#L472-L472
🤖 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/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yaml`
at line 437, Replace the mutable latest release image override with an immutable
digest or durable promoted release image in both
CUSTOM_OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE entries:
ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yaml:437
and
ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-5.0__periodics.yaml:472.
Keep the machine-config-operator periodic test configuration unchanged
otherwise.
| if oc get ImageContentSourcePolicy image-policy-aosqe &>/dev/null || \ | ||
| oc get ImageDigestMirrorSet image-policy-aosqe &>/dev/null; then | ||
| echo "ICSP/IDMS already exists, skip creating." | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require both IDMS and ITMS before skipping.
For the new-API path, an existing ImageDigestMirrorSet alone is insufficient: tag-based pulls require ImageTagMirrorSet. A partial prior oc create leaves IDMS present and ITMS absent; this guard exits successfully and cannot repair it. Check for both resources, then use oc apply so a missing ITMS is created.
Proposed fix
- if oc get ImageContentSourcePolicy image-policy-aosqe &>/dev/null || \
- oc get ImageDigestMirrorSet image-policy-aosqe &>/dev/null; then
- echo "ICSP/IDMS already exists, skip creating."
- return 0
- fi
-
- echo "ICSP/IDMS not found, creating..."
kube_minor=$(oc version -o json | grep -o '"minor": *"[^"]*"' | tail -1 | grep -o '[0-9]*')
icsp_num=$(oc get ImageContentSourcePolicy -o name 2>/dev/null | wc -l)
if [[ $icsp_num -gt 0 || $kube_minor -lt 26 ]]; then
+ oc get ImageContentSourcePolicy image-policy-aosqe &>/dev/null && return 0
cat <<EOF | oc apply -f -
+ elif oc get ImageDigestMirrorSet image-policy-aosqe &>/dev/null && \
+ oc get ImageTagMirrorSet image-policy-aosqe &>/dev/null; then
+ return 0
+ else
+ cat <<EOF | oc apply -f -This matches the upstream mirror-policy contract, which creates IDMS and ITMS together.
Also applies to: 130-174
🤖 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/set-sample-operator/disconnected/set-sample-operator-disconnected-commands.sh`
around lines 95 - 99, Update the new-API guard around the
ImageDigestMirrorSet/ImageTagMirrorSet creation flow to skip only when both
resources exist; retain the legacy ImageContentSourcePolicy handling as
appropriate. Replace the non-idempotent creation path with oc apply so a missing
ImageTagMirrorSet is created when ImageDigestMirrorSet already exists, including
the corresponding logic in the later resource-creation block.
ce4c714 to
a5f75d5
Compare
|
/pj-rehearse periodic-ci-openshift-machine-config-operator-release-4.23-periodics-e2e-aws-ipi-disc-priv-tp-pin-mco |
|
@sergiordlr: |
|
@sergiordlr: your |
|
/pj-rehearse periodic-ci-openshift-machine-config-operator-release-4.23-periodics-e2e-aws-ipi-disc-priv-tp-pin-mco |
|
@sergiordlr: your |
|
/pj-rehearse periodic-ci-openshift-machine-config-operator-release-4.23-periodics-e2e-aws-ipi-disc-priv-tp-pin-mco |
|
@sergiordlr: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-machine-config-operator-release-4.23-periodics-e2e-aws-ipi-disc-priv-tp-pin-mco |
|
@sergiordlr: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-machine-config-operator-release-4.23-periodics-e2e-aws-ipi-disc-priv-tp-pin-mco |
|
@sergiordlr: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
a5f75d5 to
4ce35ec
Compare
|
/pj-rehearse periodic-ci-openshift-machine-config-operator-release-4.23-periodics-e2e-aws-ipi-disc-priv-tp-pin-mco |
|
@sergiordlr: 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.
Actionable comments posted: 2
🤖 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/cucushift/upgrade/mirror-images/by-oc-adm/cucushift-upgrade-mirror-images-by-oc-adm-commands.sh`:
- Around line 94-97: Update the retry failure branch in the mirroring loop to
log “Trying again...” and sleep only when attempt is less than max_attempts. On
the final failed attempt, skip both actions so the script terminates
immediately.
In
`@ci-operator/step-registry/mco/conf/day2/pintargetrelease/mco-conf-day2-pintargetrelease-commands.sh`:
- Around line 24-39: Align the upgrade-edge consumer with the producer’s
“intermediate,target” format in the edge_count validation and TARGET assignment.
Do not reject the two-entry value as a serial upgrade, and select the target
(second comma-separated entry) for pinning while preserving the existing
override behavior.
🪄 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: 980faf7c-b67e-4ce2-95b4-ce30a437a3fa
📒 Files selected for processing (6)
ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yamlci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-5.0__periodics.yamlci-operator/step-registry/cucushift/installer/rehearse/aws/ipi/disconnected/private/provision/cucushift-installer-rehearse-aws-ipi-disconnected-private-provision-chain.yamlci-operator/step-registry/cucushift/upgrade/mirror-images/by-oc-adm/cucushift-upgrade-mirror-images-by-oc-adm-commands.shci-operator/step-registry/mco/conf/day2/pintargetrelease/mco-conf-day2-pintargetrelease-commands.shci-operator/step-registry/mirror-images/by-oc-adm/mirror-images-by-oc-adm-commands.sh
💤 Files with no reviewable changes (1)
- ci-operator/step-registry/cucushift/installer/rehearse/aws/ipi/disconnected/private/provision/cucushift-installer-rehearse-aws-ipi-disconnected-private-provision-chain.yaml
| else | ||
| echo "Mirroring images attempt ${attempt} failed. Trying again..." | ||
| sleep 120 | ||
| fi |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Avoid sleeping after the final failed attempt.
On attempt 10, the script still logs “Trying again” and sleeps for 120 seconds before the loop terminates. Gate the log and sleep on (( attempt < max_attempts )) to fail immediately when retries are exhausted.
🤖 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/cucushift/upgrade/mirror-images/by-oc-adm/cucushift-upgrade-mirror-images-by-oc-adm-commands.sh`
around lines 94 - 97, Update the retry failure branch in the mirroring loop to
log “Trying again...” and sleep only when attempt is less than max_attempts. On
the final failed attempt, skip both actions so the script terminates
immediately.
| # If those file exists with multiple releases we fail the execution since serial upgrades are not supported in this step | ||
| if [[ -f "${SHARED_DIR}/upgrade-edge" ]]; then | ||
| echo "ERROR: Serial upgrades are not supported!!" | ||
| exit 255 | ||
| edge_count=$(tr ',' '\n' < "${SHARED_DIR}/upgrade-edge" | grep -c .) | ||
| if (( edge_count > 1 )); then | ||
| echo "ERROR: Serial upgrades are not supported!!" | ||
| exit 255 | ||
| fi | ||
| echo "Single upgrade target override found in upgrade-edge, continuing..." | ||
| fi | ||
|
|
||
|
|
||
| TARGET="${OPENSHIFT_UPGRADE_RELEASE_IMAGE_OVERRIDE}" | ||
| if [[ -f "${SHARED_DIR}/upgrade-edge" ]]; then | ||
| TARGET=$(head -n 1 "${SHARED_DIR}/upgrade-edge" | tr ',' '\n' | head -1) | ||
| echo "Overriding upgrade target from upgrade-edge: ${TARGET}" | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Align upgrade-edge parsing with its producer.
The supplied producer writes ${RELEASE_IMAGE_INTERMEDIATE},${RELEASE_IMAGE_TARGET}. This code counts two entries and exits at Line 29, so that flow cannot reach pinning. If the check is relaxed, Line 37 selects the intermediate rather than the target. Align the producer/consumer contract or select the target entry here; otherwise disconnected MCO pinning will fail or pin the wrong release.
🤖 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/mco/conf/day2/pintargetrelease/mco-conf-day2-pintargetrelease-commands.sh`
around lines 24 - 39, Align the upgrade-edge consumer with the producer’s
“intermediate,target” format in the edge_count validation and TARGET assignment.
Do not reject the two-entry value as a serial upgrade, and select the target
(second comma-separated entry) for pinning while preserving the existing
override behavior.
4ce35ec to
5a5477c
Compare
|
/pj-rehearse periodic-ci-openshift-machine-config-operator-release-4.23-periodics-e2e-aws-ipi-disc-priv-tp-pin-mco |
|
@sergiordlr: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sergiordlr 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-machine-config-operator-release-4.23-periodics-e2e-aws-ipi-disc-priv-tp-pin-mco |
|
@sergiordlr: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@sergiordlr, |
5a5477c to
02a0793
Compare
|
/pj-rehearse periodic-ci-openshift-machine-config-operator-release-4.23-periodics-e2e-aws-ipi-disc-priv-tp-pin-mco |
|
@sergiordlr: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
02a0793 to
80493f8
Compare
|
/pj-rehearse periodic-ci-openshift-machine-config-operator-release-4.23-periodics-e2e-aws-ipi-disc-priv-tp-pin-mco |
|
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. |
|
@sergiordlr: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
A total of 3631 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here 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
🤖 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/cucushift/upgrade/toimage/cucushift-upgrade-toimage-commands.sh`:
- Around line 665-667: Update the upgrade function so non-expected failure paths
use return-based propagation instead of exit 1, including failed run_command
calls around the command execution paths, allowing upgrade_rc to always capture
the function status. Preserve intentional negative-test failures with explicit
statuses and comments, and keep successful execution behavior unchanged.
🪄 Autofix
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: 3505b9d4-b357-4f8d-9fd1-816a4e9785a0
⛔ Files ignored due to path filters (3)
ci-operator/jobs/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23-periodics.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/machine-config-operator/openshift-machine-config-operator-release-5.0-periodics.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/openshift-tests-private/openshift-openshift-tests-private-release-5.0-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (11)
ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yamlci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-5.0__periodics.yamlci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-5.0__automated-release-nightly-5.0-upgrade-from-stable-4.22.yamlci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-5.0__automated-release-nightly-5.0-upgrade-from-stable-5.0.yamlci-operator/step-registry/cucushift/installer/rehearse/aws/ipi/disconnected/private/provision/cucushift-installer-rehearse-aws-ipi-disconnected-private-provision-chain.yamlci-operator/step-registry/cucushift/upgrade/mirror-images/by-oc-adm/cucushift-upgrade-mirror-images-by-oc-adm-commands.shci-operator/step-registry/cucushift/upgrade/toimage/cucushift-upgrade-toimage-commands.shci-operator/step-registry/enable-qe-catalogsource/disconnected/enable-qe-catalogsource-disconnected-commands.shci-operator/step-registry/enable-qe-catalogsource/disconnected/enable-qe-catalogsource-disconnected-ref.yamlci-operator/step-registry/mirror-images/by-oc-adm/mirror-images-by-oc-adm-commands.shci-operator/step-registry/set-sample-operator/disconnected/set-sample-operator-disconnected-commands.sh
💤 Files with no reviewable changes (3)
- ci-operator/step-registry/cucushift/installer/rehearse/aws/ipi/disconnected/private/provision/cucushift-installer-rehearse-aws-ipi-disconnected-private-provision-chain.yaml
- ci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-5.0__automated-release-nightly-5.0-upgrade-from-stable-4.22.yaml
- ci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-5.0__automated-release-nightly-5.0-upgrade-from-stable-5.0.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
- ci-operator/step-registry/cucushift/upgrade/mirror-images/by-oc-adm/cucushift-upgrade-mirror-images-by-oc-adm-commands.sh
- ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yaml
- ci-operator/step-registry/enable-qe-catalogsource/disconnected/enable-qe-catalogsource-disconnected-commands.sh
- ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-5.0__periodics.yaml
- ci-operator/step-registry/enable-qe-catalogsource/disconnected/enable-qe-catalogsource-disconnected-ref.yaml
- ci-operator/step-registry/mirror-images/by-oc-adm/mirror-images-by-oc-adm-commands.sh
| set +e | ||
| upgrade | ||
| upgrade_rc=$? |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ci-operator/step-registry/cucushift/upgrade/toimage/cucushift-upgrade-toimage-commands.sh"
printf 'File exists: '
test -f "$file" && printf 'yes\n' || printf 'no\n'
printf '\nRelevant function/header sections:\n'
sed -n '1,80p' "$file"
printf '\n'
sed -n '380,455p' "$file"
printf '\n'
sed -n '655,690p' "$file"
printf '\nRun command definitions/usages:\n'
rg -n '^[[:space:]]*(run_command|function run_command|upgrade_rc|check_upgrade_status|exit 1)\b' "$file"Repository: openshift/release
Length of output: 10600
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ci-operator/step-registry/cucushift/upgrade/toimage/cucushift-upgrade-toimage-commands.sh"
printf 'Candidate files matching name:\n'
fd -a 'cucushift-upgrade-toimage-commands\.sh$' /
printf '\nAll run_command definitions in repo:\n'
rg -n '^[[:space:]]*(run_command|function run_command)\b' ci-operator/step-registry || trueRepository: openshift/release
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ci-operator/step-registry/cucushift/upgrade/toimage/cucushift-upgrade-toimage-commands.sh"
printf 'run_command definition:\n'
sed -n '280,315p' "$file"
printf '\n'
printf 'Relevant run_command callers around upgrade():\n'
sed -n '370,450p' "$file"
printf '\nBash set -e status probe for function return vs exit:\n'
bash -c '
set -e
run() {
local cmd
cmd="false"
(eval "$cmd")
:
}
set +e
run
rc=$?
echo "shell_rc=$rc"
'
printf '\n'
printf 'Bash set -e probe for expected failures using exit 1 and uncheck command in function:\n'
bash -c '
set -e
function upgrade() {
if false; then
run_command "echo fail"
exit 1
fi
run_command "false"
echo "started"
}
run_command() {
eval "$1"
}
set +e
upgrade
upgrade_rc=$?
echo "upgrade_rc=${upgrade_rc:-set-to-undefined}"
'Repository: openshift/release
Length of output: 5042
Make upgrade() return explicit failures.
upgrade() uses exit 1, so failures inside the function can terminate the script before upgrade_rc is assigned. run_command also returns the eval status, so a failed command at Lines 436 or 441 can continue and fall through to Line 442. Replace non-expected exit 1 paths with return/|| return $? so the caller receives the status reliably.
Keep intentional negative-test failures handled with explicit status and comments.
🤖 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/cucushift/upgrade/toimage/cucushift-upgrade-toimage-commands.sh`
around lines 665 - 667, Update the upgrade function so non-expected failure
paths use return-based propagation instead of exit 1, including failed
run_command calls around the command execution paths, allowing upgrade_rc to
always capture the function status. Preserve intentional negative-test failures
with explicit statuses and comments, and keep successful execution behavior
unchanged.
Source: MCP tools
|
@sergiordlr: 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. |
1 participant
wip
Summary by CodeRabbit
/tmp/continueis provided.