MCO-2133: Select bootimages based on OSImageStream#10321
MCO-2133: Select bootimages based on OSImageStream#10321pablintino wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@pablintino: This pull request references MCO-2133 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 story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
[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 |
pablintino
force-pushed
the
mco-2133
branch
2 times, most recently
from
8f70a1a to
15030dd
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
| type CoreOSBuildFetcher func(ctx context.Context) (*stream.Stream, error) | ||
|
|
||
| func DefaultCoreOSStreamGetter(ctx context.Context) (*stream.Stream, error) { | ||
| return rhcos.FetchCoreOSBuild(ctx, rhcos.DefaultOSImageStream) |
There was a problem hiding this comment.
Not sure if I'm missing something, but my expectations for this asset weres to modify setStreamGetter method to take into account the OSImageStream (by reading it from the agent.OptionalInstallConfig{}, already listed as a dependency). Otherwise the live ISO will remain pinned to rhcos9.
There was a problem hiding this comment.
I haven't covered the agent cause I saw that I'd need to cover the case where the install-config is not provided but the AgentConfig is given, that means I need to add a field to it and so on. To avoid making the change bigger and bigger I thought about leaving the agent side consistent and ready for a follow up.
Anyways, if you think that for now supporting the intall-config.yaml only is enought, cool, I can incorporate this simple change to this PR and do the rest as a follow up.
pablintino
force-pushed
the
mco-2133
branch
2 times, most recently
from
8cac45c to
1812d64
Compare
There was a problem hiding this comment.
Simplest ever approach, should this be ok for now? I'm not too familiar with the concept of marketplace, so not really sure.
There was a problem hiding this comment.
This is the fresh file @ravanelli provided, thanks!
There was a problem hiding this comment.
Reviewers: Please take a look at this change. It's the safest approach I could come up with. Basically, the structure of the ConfigMap is preserved and the stream field will continue to be reported, but from now on, it will use the default stream. A new streams field with a JSON map<streamName, streamJson> is now present. The format for SCOS remains unchanged, and the marketplace file is merged only if the file exists, which is only true for rhcos-9 for now.
cc:@djoshy
|
|
||
| // DefaultOSImageStream is the OS image stream used when the install-config | ||
| // does not specify one. | ||
| const DefaultOSImageStream = types.OSImageStreamRHCOS9 |
There was a problem hiding this comment.
I've hard-coded this default value since it'll be simple to open a PR and change it during the 5.0 branchout. I can modify this code to use an init function that sets the default based on the compiled version, but I'd prefer the current approach. It's simpler and more reliable than trying to guess the version from the version string.
There was a problem hiding this comment.
The purpose of these changes is to replicate what I did in the regular stream.go file. Of course, in SCOS everything is hard-coded to use the scos.json file and nothing changes.
Consume the osImageStream install-config field through all code paths that consume embedded bootimage metadata, allowing the installer to load per-stream files (rhel-9.json, rhel-10.json) instead of a single rhcos.json. Defaults to RHEL 9 when the field is not set. Signed-off-by: Pablo Rodriguez Nava <git@amail.pablintino.com>
|
@pablintino: 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. |
3 participants
Consume the osImageStream install-config field through all code paths that consume embedded bootimage metadata, allowing the installer to load per-stream files (rhel-9.json, rhel-10.json) instead of a single rhcos.json. Defaults to RHEL 9 when the field is not set.