[WIP] use machine-config-osimagestream to avoid hard-coding image tag names

[cheesesashimi]

No description provided.

[coderabbitai]

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Excluded labels (none allowed) (1)

• do-not-merge/work-in-progress

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2a5a13e3-106b-44d4-9e63-531c153d4a6c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign zaneb for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[pablintino]

Well, we don't want the default OS image, we need to return the OS image install-config points too.
We need to pass it by using

installer/pkg/asset/ignition/bootstrap/common.go

Line 79 in bb827bf

type bootstrapTemplateData struct {

We can use this PR to remove the StreamTag field we won't use anymore.

[cheesesashimi]

I removed the StreamTag field and tried to just use the OSImageStream field. Unless this is explicitly set, it is empty. I pushed up a change that branches based upon whether this is set and will call default-node-image when that is the case. However, I don't think that is the right path forward because what should be the source of truth for the default value? Should it be the OSImageStream code in the MCO repo? Or should it be the code here?

[pablintino]

Can't we rely on the defaults? (Just asking not a blocking question or ask for change)

[cheesesashimi]

In #10406, I don't think we can rely on the defaults since machine-config-osimagestream will be running containerized. However, I think we can rely on the defaults here. I need to look at https://github.com/containers/image to see if there is a SetDefaultValues() function someplace for types.SystemContext. If not, then I'll put these values into the machine-config-osimagestream entrypoint as the default values.

[patrickdillon]

What's bad about hardcoding the images? Is it because there are potentially more streams than rhcos9 & rhcos10, and we don't want to teach the installer about all of those?

[patrickdillon]

Can we use podman run via

installer/data/data/bootstrap/files/usr/local/bin/bootkube.sh.template

Lines 24 to 28 in ac9a370

	bootkube_podman_run() {
	# we run all commands in the host-network to prevent IP conflicts with
	# end-user infrastructure.
	podman run --quiet --net=host --rm --log-driver=k8s-file "${@}"
	}

If so, would be a little cleaner, but I'm not sure we can. Just an idea/suggestion.

[cheesesashimi]

So, I tried to do this in a different PR. While I eventually got it to work, it seemed really brittle.