Skip to content

fix: use LocalTagReferencePolicy for input image tags#5139

Open
Prucek wants to merge 1 commit intoopenshift:mainfrom
Prucek:input-image-local-ref-policy
Open

fix: use LocalTagReferencePolicy for input image tags#5139
Prucek wants to merge 1 commit intoopenshift:mainfrom
Prucek:input-image-local-ref-policy

Conversation

@Prucek
Copy link
Copy Markdown
Member

@Prucek Prucek commented Apr 28, 2026

Summary

  • Switch input image tag IST reference policy from Source to Local
  • Builds now pull base images from the internal registry instead of quay-proxy
  • Avoids "manifest unknown" failures when quay.io garbage-collects a digest after import
  • Add "manifest unknown" to hintsAtInfraReason so it's classified as an infrastructure error
  • Add isNonRetriableInfraError check to skip futile retries when a manifest is permanently gone

Problem

When quay.io GCs a base image digest (e.g., ubi-minimal), builds fail with PullBuilderImageFailed: manifest unknown even though the image was already imported into the internal registry. With SourceTagReferencePolicy, builds are directed back to quay-proxy to pull, which fails. ci-operator then retries 5 times futilely.

Fix

  1. LocalTagReferencePolicy for input image tags — builds pull from the internal registry where the image is already cached from the import step. External registry GC no longer causes build failures.
  2. hintsAtInfraReason — classify "manifest unknown" as an infrastructure error so it's properly reported.
  3. isNonRetriableInfraError — detect "manifest unknown" as a permanent error and skip retries, saving minutes of futile attempts per affected image.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Updates
    • Image tag reference policy now uses local references when creating tags, affecting how images are resolved and pulled.
  • Bug Fixes
    • Builds reporting a "manifest unknown" error are no longer retried automatically; these failures are logged and handled without recreate/delete cycles.

@openshift-merge-bot
Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

📝 Walkthrough

Walkthrough

Switches created ImageStreamTag entries to use LocalTagReferencePolicy (instead of SourceTagReferencePolicy) and updates tests accordingly. Also refines build retry logic to treat failures containing the substring "manifest unknown" as non-retriable, with a new helper for that check.

Changes

Cohort / File(s) Summary
Reference Policy Type Update
pkg/steps/input_image_tag.go, pkg/steps/input_image_tag_test.go
Implementation now sets Tag.ReferencePolicy.Type to imagev1.LocalTagReferencePolicy; tests updated to assert the new policy value.
Build Retry Refinement
pkg/steps/source.go
Adds a check for the substring "manifest unknown" in b.Status.LogSnippet and treats such failures as non-retriable; introduces a helper function encapsulating that substring detection and logs an informational message.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 11 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Go Error Handling ⚠️ Warning Line 125 of pkg/steps/input_image_tag.go discards error from waitForTagInSpec() instead of wrapping with %w, violating the pattern used elsewhere in the file. Add error parameter to fmt.Errorf call on line 125 to include %w and preserve error context as shown in the resolution.
Test Coverage For New Features ⚠️ Warning New pure function isNonRetriableInfraError() lacks unit test coverage in source_test.go despite being called in critical build failure handling. Add unit tests for isNonRetriableInfraError() covering manifest unknown detection and integrate table-driven tests in TestWaitForBuild.
✅ Passed checks (11 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: switching input image tags from SourceTagReferencePolicy to LocalTagReferencePolicy.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The custom check for stable and deterministic Ginkgo test names is not applicable; repository uses standard Go testing framework with deterministic test function names.
Test Structure And Quality ✅ Passed Codebase uses standard Go testing framework with quality standards: single scenario per test, fake clients with no cleanup required, descriptive error assertions, and consistent patterns. PR changes are minimal assertion updates maintaining test integrity.
Microshift Test Compatibility ✅ Passed This pull request does not add any new Ginkgo e2e tests. The changes involve standard Go unit tests using the testing framework that verify the functionality of image tag reference policy changes. The production code changes in source.go are focused on handling manifest detection for failed builds. Since no Ginkgo e2e tests are introduced, MicroShift compatibility verification is not applicable to this PR.
Single Node Openshift (Sno) Test Compatibility ✅ Passed This pull request does not add any new Ginkgo e2e tests. All modified files are internal Go packages or standard Go unit tests.
Topology-Aware Scheduling Compatibility ✅ Passed PR modifies image tag reference policies and build error handling without introducing scheduling constraints, affinity rules, or topology changes.
Ote Binary Stdout Contract ✅ Passed The PR does not introduce any violations of the OTE Binary Stdout Contract. All modified files are library code in the steps package with no process-level entry points, and new helper functions use string checking without stdout writes.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed This PR does not add any new Ginkgo e2e tests; changes are limited to unit test modifications in the standard Go testing framework.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci Bot requested review from pruan-rht and smg247 April 28, 2026 09:26
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 28, 2026
Builds pull base images from the internal registry instead of
quay-proxy, avoiding "manifest unknown" failures when quay.io
garbage-collects a digest after import.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@deepsm007
Copy link
Copy Markdown
Contributor

/lgtm
Would be nice to have some rehearsal or runs to demonstrate the imagestreams.json from artifacts

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Apr 28, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 28, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deepsm007, Prucek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Prucek
Copy link
Copy Markdown
Member Author

Prucek commented Apr 28, 2026

/hold

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 28, 2026
@Prucek
Copy link
Copy Markdown
Member Author

Prucek commented Apr 29, 2026

/retest-required

@Prucek
Copy link
Copy Markdown
Member Author

Prucek commented Apr 29, 2026

/test e2e

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 29, 2026

@Prucek: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/breaking-changes 36ca557 link false /test breaking-changes
ci/prow/e2e 36ca557 link true /test e2e

Full PR test history. Your PR dashboard.

Details

Instructions 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.

@deepsm007
Copy link
Copy Markdown
Contributor

Line 125 of pkg/steps/input_image_tag.go discards error from waitForTagInSpec() instead of wrapping with %w, violating the pattern used elsewhere in the file. Add error parameter to fmt.Errorf call on line 125 to include %w and preserve error context as shown in the resolution.

Worth addressing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants