Skip to content

METAL-1564: Use available images #147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Oct 9, 2025
Merged

METAL-1564: Use available images #147

merged 6 commits into from
Oct 9, 2025
+460 −78

Conversation

zaneb
Copy link
Member

@zaneb zaneb commented Sep 8, 2025

Instead of hard-coding which OS images we expect to be available, work with whatever images are provided. Ensure that the DEPLOY_ISO and DEPLOY_INITRD env vars continue to work as they did prior to 583e13f.

zaneb added 2 commits September 8, 2025 21:12
@zaneb
Allow custom filenames for arch-specific artifacts
447e8f9 
Take the base filename from the DEPLOY_ISO/DEPLOY_INITRD environment
variables instead of hardcoding it.

Assisted-By: Cursor
@zaneb
Allow . as arch separator
0ad81e9 
Using a . instead of an _ is the standard for e.g. CoreOS images on the
mirror. ABI uses agent.x86_64.iso. Since x86_64 already has an
underscore in it, it may parse ambiguously in other contexts. Allow both
so as not to break any existing code in development.

Assisted-By: Cursor
@openshift-ci-robot
Copy link

openshift-ci-robot commented Sep 8, 2025 •
edited by openshift-ci bot
Loading

@zaneb: This pull request references METAL-1564 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 task to target the "4.21.0" version, but no target version was set.

In response to this:

Instead of hard-coding which OS images we expect to be available, work with whatever images are provided. Ensure that the DEPLOY_ISO and DEPLOY_INITRD env vars continue to work as they did prior to 583e13f.

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 jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 8, 2025
@openshift-ci openshift-ci bot requested review from dtantsur and honza September 8, 2025 23:00
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Sep 8, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zaneb

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

The pull request process is described here

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 8, 2025
@zaneb zaneb mentioned this pull request Sep 8, 2025
Open
zaneb added 4 commits September 9, 2025 17:37
@zaneb
Always find DEPLOY_ISO and DEPLOY_INITRD
3d1b0b9 
A regression from 583e13f meant that
the DEPLOY_ISO and DEPLOY_INITRD files would not be found unless they
happened to be in the directory specified as IMAGE_SHARED_DIR (by
default /shared/html/images).

This change means that these files will always be found if specified,
and that the directory/directories that contain them will be searched
for arch-specific images unless the search directory is explicitly
overridden by IMAGE_SHARED_DIR.

Assisted-By: Cursor
@zaneb
Rename ironicImage to osImage
1310fa5 
The input images here are nothing to do with ironic, so choose a less
confusing name.

Assisted-By: Cursor
@zaneb
Make use of host-architecture image
0323263 
If no architecture-specific images are provided, fall back to the host
architecture image when the requested architecture is the host
architecture.

Assisted-By: Cursor
@zaneb
Implement SupportsArchitecture()
4872cf8 
If we don't have images available for the requested architecture, return
false from SupportsArchitecture(). This ensures that the correct error
message is set in the controller, and avoids a segfault when we try to
call ServeImage() with an invalid architecture.

Assisted-By: Cursor
@zaneb zaneb force-pushed the supports-architecture branch from c117ec7 to 4872cf8 Compare September 9, 2025 05:42
@zaneb
Copy link
Member Author

zaneb commented Sep 9, 2025

/retest-required

3 similar comments
@zaneb
Copy link
Member Author

zaneb commented Sep 10, 2025

/retest-required

@zaneb
Copy link
Member Author

zaneb commented Sep 11, 2025

/retest-required

@zaneb
Copy link
Member Author

zaneb commented Sep 12, 2025

/retest-required

@honza
Copy link
Member

honza commented Sep 22, 2025

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 22, 2025
@honza
Copy link
Member

honza commented Sep 22, 2025

/retest-required

1 similar comment
@zaneb
Copy link
Member Author

zaneb commented Sep 25, 2025

/retest-required

@dtantsur
Copy link
Member

This needs a verified label. Is it possible for QE to test this change? Otherwise, we need to delay testing or self-verify.

@honza
Copy link
Member

honza commented Sep 25, 2025

I think we can say it's verified by CI. Not sure why the ipv4 job is stubborn: baremetalhosts are provisioned, no errors in ICC.

@zaneb
Copy link
Member Author

zaneb commented Sep 26, 2025

/retest-required

@zaneb
Copy link
Member Author

zaneb commented Sep 26, 2025

This is blocking OCPBUGS-61477, so we will need to find QE resources to test this. However, it might make sense to test it together with openshift/machine-os-images#67. Testing it on its own tells us we haven't broken anything but only by reverting back to one ISO per arch can we see that it has achieved its goal.

@sgoveas
Copy link

sgoveas commented Oct 8, 2025

/verified by sgoveas

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Oct 8, 2025
@openshift-ci-robot
Copy link

@sgoveas: This PR has been marked as verified by sgoveas.

In response to this:

/verified by sgoveas

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
Copy link

/retest-required

Remaining retests: 0 against base HEAD 0fc97ae and 2 for PR HEAD 4872cf8 in total

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Oct 9, 2025 •
edited
Loading

@zaneb: The following test 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/okd-scos-e2e-aws-ovn 4872cf8 link false /test okd-scos-e2e-aws-ovn

Full PR test history. Your PR dashboard.

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.

@honza
Copy link
Member

honza commented Oct 9, 2025

/retest-required

@openshift-merge-bot openshift-merge-bot bot merged commit d958f0b into openshift:main Oct 9, 2025
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtantsur dtantsur Awaiting requested review from dtantsur

@honza honza Awaiting requested review from honza

Assignees

@honza honza

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
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zaneb @openshift-ci-robot @honza @dtantsur @sgoveas