Oringial 50 hack - #82608
Conversation
|
/pj-rehearse periodic-ci-openshift-machine-config-operator-release-4.23-periodics-e2e-aws-ipi-disc-priv-tp-pin-mco |
WalkthroughAdds weekly AWS disconnected-private MCO periodic jobs for releases 4.23 and 5.0, removes superseded nightly entries, permits single upgrade-edge targets, and introduces ChangesMCO disconnected-private periodic coverage
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PeriodicJob
participant UpgradeTarget
participant QERegistry
participant SanityChain
PeriodicJob->>UpgradeTarget: write upgrade-edge
UpgradeTarget->>UpgradeTarget: select single target
PeriodicJob->>QERegistry: pass SKIP_QE_APP_REGISTRY
QERegistry->>QERegistry: skip setup when enabled
PeriodicJob->>SanityChain: run disconnected sanity tests
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 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 |
|
@sergiordlr: |
|
@sergiordlr: your |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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: Update CUSTOM_OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE in the weekly
periodic configuration to use an existing promoted stable/ci release image
instead of the ephemeral ci-ln-39q90fb build-farm reference. If no suitable
promoted image exists, replace it with a durable image reference whose ownership
is documented.
🪄 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: 2f27dd9a-8127-4785-a0d9-859d474af0a0
⛔ 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 (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/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/enable-qe-catalogsource/disconnected/enable-qe-catalogsource-disconnected-commands.shci-operator/step-registry/enable-qe-catalogsource/disconnected/enable-qe-catalogsource-disconnected-ref.yaml
💤 Files with no reviewable changes (2)
- 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/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-39q90fb/release:latest |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt around job =="
sed -n '410,455p' ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yaml
echo
echo "== all references to ci-ln-39q90fb or build10 release images in this repo =="
rg -n "ci-ln-39q90fb|registry\.build10\.ci\.openshift\.org|CUSTOM_OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE|OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE" -S .
echo
echo "== nearby job definitions =="
sed -n '1,80p' ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.23__periodics.yamlRepository: openshift/release
Length of output: 50374
🌐 Web query:
registry.build10.ci.openshift.org ci-ln-39q90fb/release:latest
💡 Result:
The pullspec registry.build10.ci.openshift.org/ci-ln-39q90fb/release:latest refers to a container image stored within the OpenShift CI infrastructure [1][2]. Specifically: 1. Registry: registry.build10.ci.openshift.org is an integrated internal image registry associated with the build10 build farm cluster in the OpenShift CI environment [1][2][3]. 2. Namespace: ci-ln-39q90fb represents a specific, ephemeral namespace created for a CI job execution (often associated with a pull request or automated task) on that build farm [3]. 3. Image/Tag: release:latest points to a specific image stream and tag used within that CI job's context [3]. In the context of OpenShift CI, registry.build10.ci.openshift.org is primarily used for the temporary storage of build artifacts during job execution [3]. Official documentation advises that users and external integrations should not rely on these specific build-farm registry pullspecs for persistent image access [3]. Instead, the authoritative source for OpenShift CI images is the central registry, quay.io/openshift/ci [3]. Accessing images within these test namespaces typically requires being the author of the relevant pull request and performing an authentication via oc registry login [3].
Citations:
- 1: https://github.com/openshift/release/blob/main/core-services/ci-secret-bootstrap/_config.yaml
- 2: https://github.com/openshift/release/blob/master/core-services/ci-secret-bootstrap/gsm-config.yaml
- 3: https://docs.ci.openshift.org/how-tos/use-registries-in-build-farm/
🌐 Web query:
OpenShift CI build registry build10 ci registry imagestreams release ci-ln-39q90fb
💡 Result:
The term ci-ln-39q90fb refers to an OpenShift CI build artifact identifier, specifically a unique namespace associated with a Prow job execution on a CI build farm [1]. In the OpenShift CI architecture, job names and environment variables often contain these identifiers to track resources and execution environments [2]. The prefix ci-ln- is a standard naming convention for namespaces created dynamically on CI build clusters (such as build10) to host pods and other resources during the execution of a test job [1]. Regarding the registry components mentioned: 1. QCI (quay.io/openshift/ci): This is the authoritative, central CI image registry and the source of truth for all images used in OpenShift CI [1][3]. 2. registry.ci.openshift.org: This is the integrated image registry on the app.ci cluster [1][3]. While it served as the central CI registry historically, it is now primarily used for internal automation, such as by Release Controllers to host promoted images for release-related tasks [1][3]. CI users are generally advised to reference images from QCI rather than the app.ci registry [1]. When troubleshooting, engineers often use the build farm's specific namespace (e.g., ci-ln-...) to pull image artifacts or inspect logs associated with a specific job execution [1]. If an image is missing, it is typically because it has not yet been promoted to or mirrored in the relevant registry [3].
Citations:
- 1: https://docs.ci.openshift.org/how-tos/use-registries-in-build-farm/
- 2: https://github.com/openshift/release/blob/main/ci-operator/jobs/openshift/release/openshift-release-infra-periodics.yaml
- 3: https://docs.ci.openshift.org/internals/images-in-ci/
Do not rely on an ephemeral build-farm image for a weekly periodic.
registry.build10.ci.openshift.org/ci-ln-39q90fb/release:latest is a CI build-farm namespace (ci-ln-*), not a durable release; these images are not promoted and do not outlive the job that creates them. If the build namespace is unavailable or cleaned up, this periodic fails before testing MCO. Use an existing promoted stable/ci release image or create and document ownership for a durable image reference.
🤖 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, Update CUSTOM_OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE in the
weekly periodic configuration to use an existing promoted stable/ci release
image instead of the ephemeral ci-ln-39q90fb build-farm reference. If no
suitable promoted image exists, replace it with a durable image reference whose
ownership is documented.
|
/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. |
fc84655 to
88012b1
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. |
|
/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. |
|
/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 |
|
[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 |
|
@sergiordlr: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
A total of 1149 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/mco/conf/day2/pintargetrelease/mco-conf-day2-pintargetrelease-commands.sh`:
- Around line 24-32: Parse the upgrade-edge contents once into a validated
target value before assigning TARGET. Update the validation around upgrade-edge
to reject blank entries, malformed comma-separated values, and multiple targets;
then assign TARGET from that parsed value so the later oc adm release info
lookup never receives an empty or invalid target.
🪄 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: 2708fa34-91b7-4446-8215-cf85842df794
📒 Files selected for processing (1)
ci-operator/step-registry/mco/conf/day2/pintargetrelease/mco-conf-day2-pintargetrelease-commands.sh
| # 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 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
fd -a 'mco-conf-day2-pintargetrelease|pintargetrelease|commands\.sh' . | sed 's#^\./##' | sort
main="ci-operator/step-registry/mco/conf/day2/pintargetrelease/mco-conf-day2-pintargetrelease-commands.sh"
ref="ci-operator/step-registry/mco/conf/day2/pintargetrelease/mco-conf-day2-pintargetrelease-ref.yaml"
if [[ -f "$main" ]]; then
echo
echo "Commands outline/size:"
wc -l "$main"
echo
echo "Line-numbers 1-120:"
cat -n "$main" | sed -n '1,120p'
fi
echo
if [[ -f "$ref" ]]; then
wc -l "$ref"
cat -n "$ref" | sed -n '1,180p'
else
echo "ref missing"
fi
echo
echo "Search upgrade-edge/OPENSHIFT_UPGRADE_RELEASE_IMAGE_OVERRIDE:"
rg -n "upgrade-edge|OPENSHIFT_UPGRADE_RELEASE_IMAGE_OVERRIDE|TARGET|SERIAL|Serial" ci-operator/step-registry/mco/conf/day2/pintargetrelease ci-operator/step-registry 2>/dev/null | head -n 200Repository: openshift/release
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
target_file="ci-operator/step-registry/mco/conf/day2/pintargetrelease/mco-conf-day2-pintargetrelease-commands.sh"
echo "Candidate command file exists:"
ls -l "$target_file" || true
echo
echo "Line-numbers 1-80:"
cat -n "$target_file" | sed -n '1,80p' || true
echo
echo "Target file references:"
rg -n "OPENSHIFT_UPGRADE_RELEASE_IMAGE_OVERRIDE|upgrade-edge|TARGET|Serial|serial" "$target_file" || true
echo
echo "Behavioral probe for current parsing logic:"
python3 - <<'PY'
cases = ["release-image", ",release-image", "release-image,", ",,", "release-image,2", " ,release-image "]
for c in cases:
edge_count = len([x for x in c.split(",") if x])
target = c.split(",")[0]
print(f"input={c!r} -> edge_count={edge_count} target={target!r}")
PYRepository: openshift/release
Length of output: 5415
Parse and validate upgrade-edge once before overriding TARGET.
At lines 24-37, empty or malformed entries are accepted by the count check but can still be assigned to TARGET; for example, ,release-image passes validation, then TARGET="" and the later oc adm release info "${TARGET}" lookup fails. Reject blank/malformed input and assign TARGET from the same parsed value.
🤖 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 - 32, Parse the upgrade-edge contents once into a validated
target value before assigning TARGET. Update the validation around upgrade-edge
to reject blank entries, malformed comma-separated values, and multiple targets;
then assign TARGET from that parsed value so the later oc adm release info
lookup never receives an empty or invalid target.
|
@sergiordlr: 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
wip
Summary by CodeRabbit
e2e-aws-ipi-disc-priv-tp-pin-mcoin bothopenshift-machine-config-operator-release-4.23__periodics.yamlandopenshift-machine-config-operator-release-5.0__periodics.yaml, targeting theopenshift-org-awscluster profile with anOPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDEofrelease:initialand environment/config to pin the desired target release while removing the pull secret and using the disconnected rehearsal workflow +openshift-upgrade-qe-sanity-disconnectedchain.aws-ipi-disc-priv-tp-pin-mco-f14) from the 5.0 automated nightly upgrade-from-stable upgrade configurations (...upgrade-from-stable-4.22.yamland...upgrade-from-stable-5.0.yaml) so coverage is consolidated into the new periodic job.SKIP_QE_APP_REGISTRY=truevia a wired-throughSKIP_QE_APP_REGISTRYenvironment variable and an early successful exit.pintargetreleasestep script to better handleupgrade-edge: it now allows a single upgrade-edge entry (parses it and overridesTARGETaccordingly) and only fails when multiple entries are present.