Skip to content

AGENT-1527: Fix comparison of InternalReleaseImage to mirror#31250

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
bfournie:iri-idms
Jun 2, 2026
Merged

AGENT-1527: Fix comparison of InternalReleaseImage to mirror#31250
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
bfournie:iri-idms

Conversation

@bfournie

@bfournie bfournie commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

The IRI image should be compared against the mirrors in the IDMS, not the source.

Summary by CodeRabbit

  • Tests
    • Enhanced release image mirror validation to scan all available resources instead of a single configuration, improving validation coverage.

The IRI image should be compared against the mirrors in the IDMS,
not the source.
@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

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 1, 2026
@openshift-ci-robot

openshift-ci-robot commented Jun 1, 2026

Copy link
Copy Markdown

@bfournie: This pull request references AGENT-1527 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set.

Details

In response to this:

The IRI image should be compared against the mirrors in the IDMS, not the source.

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 openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented Jun 1, 2026

Copy link
Copy Markdown

Walkthrough

The PR modifies VerifyIDMSConfigured, a test helper method in test/extended/internalreleaseimage/helper.go, to validate release image repositories more comprehensively. Instead of fetching a single ImageDigestMirrorSets resource named image-digest-mirror, the method now lists all IDMS resources and searches through all mirror entries for an exact match to the extracted image source.

Changes

IDMS validation expansion

Layer / File(s) Summary
Expand IDMS validation to check all mirror sets
test/extended/internalreleaseimage/helper.go
VerifyIDMSConfigured now iterates through all ImageDigestMirrorSets resources and searches all configured mirrors for an exact match to the extracted release image source, replacing the single IDMS object lookup.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Test Structure And Quality ⚠️ Warning VerifyIDMSConfigured function at line 100 uses context.Background() without timeout for cluster API call, violating requirement #3 (timeouts for cluster operations). Add timeout to ImageDigestMirrorSets().List() call using context.WithTimeout or wait.PollUntilContextTimeout pattern used elsewhere in helper.
Microshift Test Compatibility ⚠️ Warning Test calls VerifyIDMSConfigured which uses ImageDigestMirrorSets (config.openshift.io API), unavailable on MicroShift. Only tagged [apigroup:machineconfiguration.openshift.io]. Add [apigroup:config.openshift.io] to test name or guard VerifyIDMSConfigured with MicroShift check.
✅ Passed checks (13 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references fixing comparison of InternalReleaseImage to mirror, which directly aligns with the core change: updating VerifyIDMSConfigured to validate the release image by checking all IDMS mirrors instead of just one.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 All 13 Ginkgo test titles in the PR (It, Describe, Context) are stable literal strings with no dynamic content, generated IDs, timestamps, or values that could change between runs.
Single Node Openshift (Sno) Test Compatibility ✅ Passed This PR modifies only a helper utility file (helper.go) and does not add any new Ginkgo e2e tests. The custom check applies only to new test declarations, so it is not applicable here.
Topology-Aware Scheduling Compatibility ✅ Passed The PR modifies only a test helper function that validates image mirror configuration, with no deployment manifests, operator code, controllers, or scheduling constraints.
Ote Binary Stdout Contract ✅ Passed Changes to helper.go are within Ginkgo test blocks and use e2e.Logf() for logging, which outputs to Ginkgo's writer, not stdout. No process-level stdout writes found.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e tests added; PR only modifies helper function. Check requires new test additions to apply.
No-Weak-Crypto ✅ Passed No weak cryptographic algorithms, custom crypto implementations, or insecure secret comparisons found. String comparison is used only for configuration values, not secrets.
Container-Privileges ✅ Passed PR modifies only Go test code without any Kubernetes manifests or container privilege configurations, making the container-privileges check inapplicable.
No-Sensitive-Data-In-Logs ✅ Passed No passwords, tokens, API keys, PII, session IDs, or customer data exposed. Logged hostnames and image paths are infrastructure metadata accessible to cluster users.

✏️ 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 andfasano and rwsu June 1, 2026 21:09
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 1, 2026
@bfournie

bfournie commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

With this fix we see the test run successfully

 STEP: Verifying image repo is present in ImageDigestMirrorSet @ 06/01/26 22:50:42.08
 I0601 22:50:42.080341 1475402 helper.go:97] Verifying image repo is present in image-digest-mirror IDMS: api-int.ostest.test.metalkube.org:22625/openshift/release-images@sha256:f72b9f8f1560ca44dec274f19560195b03e948a932cf42ef8bfc96a6c2fa7e6c
 I0601 22:50:42.085168 1475402 helper.go:106] Extracted image source: api-int.ostest.test.metalkube.org:22625/openshift/release-images
 I0601 22:50:42.085193 1475402 helper.go:114] Found IDMS match in idms-release-0: source registry.ci.openshift.org/ocp/release-5 -> mirror api-int.ostest.test.metalkube.org:22625/openshift/release-images
 I0601 22:50:42.085206 1475402 helper.go:129] Confirmed: test image repo is covered by IDMS, will be pulled from mirror registry
   STEP: Creating test namespace and pod @ 06/01/26 22:50:42.085
 I0601 22:50:42.092778 1475402 helper.go:178] Created namespace: e2e-test-41d351e2-99ac-4a31-8962-f5e742caee4c
 I0601 22:50:42.622994 1475402 helper.go:155] Created test pod: e2e-test-41d351e2-99ac-4a31-8962-f5e742caee4c/iri-registry-test-0c05776c-8fa8-4c25-93c2-4374e115d050
   STEP: Waiting for pod to complete successfully @ 06/01/26 22:50:42.623
   STEP: Saw pod success @ 06/01/26 22:50:46.648
 I0601 22:50:46.652171 1475402 internalreleaseimage.go:186] Workload successfully pulled image from mirror registry (ImageID: api-int.ostest.test.metalkube.org:22625/openshift/release-images@sha256:f72b9f8f1560ca44dec274f19560195b03e948a932cf42ef8bfc96a6c2fa7e6c)
 I0601 22:50:46.674412 1475402 helper.go:201] Deleted namespace: e2e-test-41d351e2-99ac-4a31-8962-f5e742caee4c
 • [4.639 seconds]


@bfournie

bfournie commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

/cc @pawanpinjarkar

@openshift-ci
openshift-ci Bot requested a review from pawanpinjarkar June 1, 2026 21:10

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@test/extended/internalreleaseimage/helper.go`:
- Line 97: The log message at the e2e.Logf call that currently reads "Verifying
image repo is present in image-digest-mirror IDMS: %s" is outdated; update the
string to reflect that the function now scans all ImageDigestMirrorSets (e.g.,
"Verifying image repo is present in all ImageDigestMirrorSets: %s") so
diagnostics are accurate—modify the e2e.Logf invocation that references
releaseImage accordingly.
🪄 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: Enterprise

Run ID: 16be2634-9c90-4a5e-9576-d38339d5bad3

📥 Commits

Reviewing files that changed from the base of the PR and between 76ed5a8 and 5ecd62a.

📒 Files selected for processing (1)
  • test/extended/internalreleaseimage/helper.go

// VerifyIDMSConfigured verifies that the test image repo is present in the image-digest-mirror IDMS
// VerifyIDMSConfigured verifies that the test image repo is present as a mirror in at least one IDMS
func (h *IRITestHelper) VerifyIDMSConfigured(releaseImage string) {
e2e.Logf("Verifying image repo is present in image-digest-mirror IDMS: %s", releaseImage)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update the verification log text to match the new all-IDMS scan.

The log still says image-digest-mirror IDMS, but this function now scans all ImageDigestMirrorSets. Keeping this message current will avoid misleading test diagnostics.

Suggested diff
-	e2e.Logf("Verifying image repo is present in image-digest-mirror IDMS: %s", releaseImage)
+	e2e.Logf("Verifying image repo is present as a mirror in at least one ImageDigestMirrorSet: %s", releaseImage)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
e2e.Logf("Verifying image repo is present in image-digest-mirror IDMS: %s", releaseImage)
e2e.Logf("Verifying image repo is present as a mirror in at least one ImageDigestMirrorSet: %s", releaseImage)
🤖 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 `@test/extended/internalreleaseimage/helper.go` at line 97, The log message at
the e2e.Logf call that currently reads "Verifying image repo is present in
image-digest-mirror IDMS: %s" is outdated; update the string to reflect that the
function now scans all ImageDigestMirrorSets (e.g., "Verifying image repo is
present in all ImageDigestMirrorSets: %s") so diagnostics are accurate—modify
the e2e.Logf invocation that references releaseImage accordingly.

@bfournie

bfournie commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

/test ?

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling required tests:
/test e2e-aws-csi
/test e2e-aws-ovn-fips
/test e2e-aws-ovn-microshift
/test e2e-aws-ovn-microshift-serial
/test e2e-aws-ovn-serial-1of2
/test e2e-aws-ovn-serial-2of2
/test e2e-gcp-csi
/test e2e-gcp-ovn
/test e2e-gcp-ovn-upgrade
/test e2e-metal-ipi-ovn-ipv6
/test e2e-vsphere-ovn
/test e2e-vsphere-ovn-upi

@pawanpinjarkar

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 1, 2026
@openshift-ci

openshift-ci Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bfournie, pawanpinjarkar

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

@pawanpinjarkar

Copy link
Copy Markdown
Contributor

/verified later by @pawanpinjarkar and ove periodic jobs

@openshift-ci-robot

Copy link
Copy Markdown

@pawanpinjarkar: Only users can be targets for the /verified later command.

Details

In response to this:

/verified later by @pawanpinjarkar and ove periodic jobs

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 openshift-eng/jira-lifecycle-plugin repository.

@pawanpinjarkar

Copy link
Copy Markdown
Contributor

/verified later by @pawanpinjarkar

@openshift-ci-robot

Copy link
Copy Markdown

@pawanpinjarkar: Only users can be targets for the /verified later command.

Details

In response to this:

/verified later by @pawanpinjarkar

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 openshift-eng/jira-lifecycle-plugin repository.

@pawanpinjarkar

Copy link
Copy Markdown
Contributor

/verified later @pawanpinjarkar

@openshift-ci-robot

Copy link
Copy Markdown

@pawanpinjarkar: This PR has been marked to be verified later by @pawanpinjarkar.

Details

In response to this:

/verified later @pawanpinjarkar

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jun 1, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 76ed5a8 and 2 for PR HEAD 5ecd62a in total

@openshift-ci

openshift-ci Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

@bfournie: all tests passed!

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.

@openshift-merge-bot
openshift-merge-bot Bot merged commit 293edee into openshift:main Jun 2, 2026
21 checks passed
@bfournie
bfournie deleted the iri-idms branch June 2, 2026 11:07
pawanpinjarkar pushed a commit to pawanpinjarkar/origin that referenced this pull request Jul 21, 2026
AGENT-1527: Fix comparison of InternalReleaseImage to mirror
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. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria verified-later

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants