fix(sync-ci-images): fix oc wait, manifest-list digests, and IS import - #3199
Merged
ashwindasr merged 5 commits intoJul 27, 2026
Merged
Conversation
The post-build wait in `start-builds` used two `--for=jsonpath` flags:
--for=jsonpath='{.status.phase}'=Complete
--for=jsonpath='{.status.phase}'=Failed
In modern oc/kubectl, multiple --for flags are ANDed, so this waited
for phase=Complete AND phase=Failed simultaneously — an impossible
condition that always timed out after 1 hour.
This meant the post-build imagestream tag update never ran, leaving
IS tags pointing to stale digests. When the old digest became
unreachable through quay-proxy, `prs open` failed with
"manifest unknown" because `oc image info` could not resolve the
stale reference.
Replace with a polling loop that checks the build phase every 15s
until it reaches a terminal state (Complete, Failed, Cancelled, Error).
Co-authored-by: Cursor <cursoragent@cursor.com>
rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
Contributor
|
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:
WalkthroughImage mirroring now reuses digest-pinned sources when applicable, imagestream patches omit ChangesImage synchronization
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
do-not-merge/hold
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@doozer/doozerlib/cli/images_streams.py`:
- Around line 981-999: The polling loop in the build-status wait flow must
enforce overall_timeout for both sleeping and command execution. Update the
logic around exectools.cmd_gather and time.sleep so each subprocess wait and
sleep is bounded by the remaining overall timeout, and exit promptly when the
deadline is reached while preserving terminal-phase handling.
🪄 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-eng/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b2269e6c-3d48-434a-b363-88fa9fab9d3f
📒 Files selected for processing (1)
doozer/doozerlib/cli/images_streams.py
Comment on lines
+981
to
+999
| poll_interval = 15 | ||
| phase = '' | ||
| while True: | ||
| elapsed_wait = time.time() - overall_start_time | ||
| if elapsed_wait >= overall_timeout: | ||
| runtime.logger.warning(f'Overall timeout exceeded while waiting for {build_name}') | ||
| break | ||
|
|
||
| check_phase_cmd = f'oc -n ci get build/{build_name} -o jsonpath={{.status.phase}}' | ||
| if as_user: | ||
| check_phase_cmd += f' --as {as_user}' | ||
|
|
||
| rc_phase, phase_stdout, _ = exectools.cmd_gather(check_phase_cmd) | ||
| if rc_phase == 0: | ||
| phase = phase_stdout.strip() | ||
| if phase in ('Complete', 'Failed', 'Cancelled', 'Error'): | ||
| break | ||
|
|
||
| time.sleep(poll_interval) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C 6 '\bdef cmd_gather\b|\bcmd_gather\s*=' doozer
rg -n -C 3 '\bcmd_gather\(' doozerRepository: openshift-eng/art-tools
Length of output: 33415
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '99,190p' doozer/rundoozer/rundoozer.py
printf '\n---\n'
sed -n '970,1005p' doozer/doozerlib/cli/images_streams.pyRepository: openshift-eng/art-tools
Length of output: 234
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a 'rundoozer' doozer
printf '\n---\n'
git ls-files 'doozer/rundoozer*'
printf '\n---\n'
rg -n -C 8 '\bdef cmd_gather\b|\bcmd_gather\s*=' doozerRepository: openshift-eng/art-tools
Length of output: 1307
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '99,190p' doozer/rundoozer/rundoozer
printf '\n---\n'
sed -n '978,1002p' doozer/doozerlib/cli/images_streams.pyRepository: openshift-eng/art-tools
Length of output: 4507
Cap the polling loop and subprocess wait to the shared timeout. The 15-second sleep can overshoot overall_timeout, and cmd_gather() has no subprocess timeout here, so a hung oc get can still block past the deadline.
🤖 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 `@doozer/doozerlib/cli/images_streams.py` around lines 981 - 999, The polling
loop in the build-status wait flow must enforce overall_timeout for both
sleeping and command execution. Update the logic around exectools.cmd_gather and
time.sleep so each subprocess wait and sleep is bounded by the remaining overall
timeout, and exit promptly when the deadline is reached while preserving
terminal-phase handling.
Now that start-builds polls for build completion, builds are already finished when _wait_for_builds runs. The 20-minute static sleep was only needed because the old oc wait never detected completion. Signed-off-by: Ashwin Das <ashwindasr@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
`oc image info` cannot resolve manifest list (multi-arch) tags, causing get_image_digest to always return None for images mirrored with --keep-manifest-list. This meant the IS tag update was skipped for every manifest-list image (golang, base-rhel9, centos streams), leaving imagestreams pointing to stale digests indefinitely. For digest-pinned sources mirrored with --keep-manifest-list, the manifest list digest is content-addressable and preserved at the destination. Extract the source digest directly from the mirror command instead of calling the broken oc image info. This fixes the stale golang IS (e.g. openshift/release:rhel-9- release-golang-1.25-openshift-4.22 stuck on Go 1.25.9 instead of 1.25.11) that has been broken since mirror_manifest_list was enabled. Signed-off-by: Ashwin Das <ashwindasr@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…manifests The QCI migration (79f6852) added reference=True to all ImageStream tag patches. This prevents the image controller from importing the image into the integrated registry, causing `oc image info` against registry.ci.openshift.org to fail with "manifest unknown". Remove reference=True so the image controller imports manifests from quay-proxy into the integrated registry. referencePolicy=Source and importPolicy=PreserveOriginal are retained. Co-authored-by: Cursor <cursoragent@cursor.com>
ashwindasr
force-pushed
the
fix-oc-wait-and-semantics
branch
from
July 24, 2026 15:29
fe921f0 to
5633bf6
Compare
Contributor
Author
ashwindasr
changed the title
fgallott
reviewed
Jul 27, 2026
start-builds already polls for build completion, so this method was a dead no-op left over from that change. rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Contributor
openshift-ci
Bot
removed
the
do-not-merge/hold
Contributor
|
@ashwindasr: 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. |
Contributor
Author
|
commit check failing on a false positive - proceeding to merge |
pruan-rht
pushed a commit
to pruan-rht/art-tools
that referenced
this pull request
Jul 27, 2026
openshift-eng#3199) * fix(start-builds): replace impossible oc wait AND condition with polling The post-build wait in `start-builds` used two `--for=jsonpath` flags: --for=jsonpath='{.status.phase}'=Complete --for=jsonpath='{.status.phase}'=Failed In modern oc/kubectl, multiple --for flags are ANDed, so this waited for phase=Complete AND phase=Failed simultaneously — an impossible condition that always timed out after 1 hour. This meant the post-build imagestream tag update never ran, leaving IS tags pointing to stale digests. When the old digest became unreachable through quay-proxy, `prs open` failed with "manifest unknown" because `oc image info` could not resolve the stale reference. Replace with a polling loop that checks the build phase every 15s until it reaches a terminal state (Complete, Failed, Cancelled, Error). Co-authored-by: Cursor <cursoragent@cursor.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED * fix(sync-ci-images): remove redundant 20-minute sleep after builds Now that start-builds polls for build completion, builds are already finished when _wait_for_builds runs. The 20-minute static sleep was only needed because the old oc wait never detected completion. Signed-off-by: Ashwin Das <ashwindasr@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED * fix(mirror): use source digest for manifest-list mirrors `oc image info` cannot resolve manifest list (multi-arch) tags, causing get_image_digest to always return None for images mirrored with --keep-manifest-list. This meant the IS tag update was skipped for every manifest-list image (golang, base-rhel9, centos streams), leaving imagestreams pointing to stale digests indefinitely. For digest-pinned sources mirrored with --keep-manifest-list, the manifest list digest is content-addressable and preserved at the destination. Extract the source digest directly from the mirror command instead of calling the broken oc image info. This fixes the stale golang IS (e.g. openshift/release:rhel-9- release-golang-1.25-openshift-4.22 stuck on Go 1.25.9 instead of 1.25.11) that has been broken since mirror_manifest_list was enabled. Signed-off-by: Ashwin Das <ashwindasr@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> * fix(imagestreams): remove reference=True so image controller imports manifests The QCI migration (79f6852) added reference=True to all ImageStream tag patches. This prevents the image controller from importing the image into the integrated registry, causing `oc image info` against registry.ci.openshift.org to fail with "manifest unknown". Remove reference=True so the image controller imports manifests from quay-proxy into the integrated registry. referencePolicy=Source and importPolicy=PreserveOriginal are retained. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(sync-ci-images): remove no-op _wait_for_builds hook start-builds already polls for build completion, so this method was a dead no-op left over from that change. rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED --------- Signed-off-by: Ashwin Das <ashwindasr@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three bugs in the sync-ci-images pipeline that were causing stale ImageStream tags, failed
oc image infolookups, and outdated golang versions in CI images.1. Replace impossible
oc waitAND condition with pollingThe post-build wait in
start-buildsused two--for=jsonpathflags (phase=Completeandphase=Failed). In modernoc/kubectl, multiple--forflags are ANDed, making this condition impossible to satisfy. The wait always timed out after 1 hour, preventing the post-build IS tag update from ever running.Fix: Replaced with a polling loop that checks the build phase every 15s until it reaches a terminal state (
Complete,Failed,Cancelled,Error).2. Use source digest for manifest-list mirrors
For images mirrored with
--keep-manifest-listfrom a digest-pinned source,oc image infocannot resolve the manifest list tag on Quay. This causedget_image_digestto fail, returningNone, which skipped the IS tag update entirely — leaving stale digests and outdated image content (e.g. golang v1.25.9 instead of v1.25.11).Fix: When
--keep-manifest-listis used with a digest-pinned source (@sha256:), extract the source digest directly instead of callingget_image_digest. Manifest list digests are content-addressable and preserved during mirroring.3. Remove
reference=Truefrom ImageStream tag patchesThe QCI migration (
79f685261) addedreference: trueto all IS tag patches. This tells the image controller not to import the image into the integrated registry, causingoc image info registry.ci.openshift.org/...to fail with "manifest unknown". Before the migration, images were natively in the integrated registry via BuildConfig pushes.Fix: Removed
reference: truefrom all five IS tag patch sites. The image controller now imports manifests from quay-proxy into the integrated registry.referencePolicy: SourceandimportPolicy: PreserveOriginalare retained.4. Remove redundant 20-minute sleep
The
_wait_for_buildsmethod had a 20-minute sleep that was redundant now that build completion is polled instart-builds.Verified
Test plan
doozer/tests/cli/test_images_streams.py)oc image infoagainst registry.ci resolves correctly after fixsync-ci-imagesrun for 4.22 completes without errors