cmd/openshift-install/upi: Add 'bootimage' subcommand

[wking]

Builds on #1528, I'll rebase after that lands.

This makes it easier for UPI folks to discover a recommended bootimage for the infrastructure they are setting up.

Fixes #1399.

I've taken a slightly different take on the CLI framework vs. what was discussed there:

• I don't actually download anything (no point in downloading AMIs, and we want to expose them too, CC @dgoodwin). Downloading should be straightforward when we spit out URI(s).
• I've used a positional platform argument (vs. --platform), because the argument is required and not optional. Without a requested platform, what would we show?

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wking

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:

• OWNERS [wking]

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

[abhinavdahiya]

* I've used a positional platform argument (vs. `--platform`), because the argument is required and not optional.  Without a requested platform, what would we show?

if the user needs the platform information, it is present in the install-config. I would expect installer to drive it self from --dir or internal state.

[cgwalters]

I guess a tricky topic is that this PR intersects the RHCOS artifact types (qemu, openstack, metal-bios, metal-uefi) with OpenShift platforms and those don't always have 1-1 mappings or name alignments.

I think we clearly want to have openstack in this list, and I'd definitely like to support downloading all of the "metal" artifacts (metal-bios, metal-uefi, and iso).

[cgwalters]

But that doesn't necessarily need to be solved in this PR, we can add to it later - only gave this a skim but LGTM.

[wking]

if the user needs the platform information, it is present in the install-config.

Hmm. I like how the current implementation is decoupled from the install-config, so you don't have to go through the hassle of reading in an install-config just to figure out the bootimage. But I agree that there's a benefit to not having to document two separate interfaces. I dunno, I could go either way here. You're pretty solidly behind the install-config approach?

I guess a tricky topic is that this PR intersects the RHCOS artifact types (qemu, openstack, metal-bios, metal-uefi) with OpenShift platforms and those don't always have 1-1 mappings or name alignments.

I think the bootimage subcommand should take OpenShift platform names. If libvirt and openstack both return the QEMU URI (should they?), that's fine. And the implementation would just be:

case libvirt.Name, openstack.Name:

which seems pretty straightforward.

For the additional objects that are so UPI that they will never have in-cluster names, I'm fine rolling forward with whatever naming RHCOS wants to use. I'm fine putting that in this PR or saving it for followup work.

I've pushed bde9ba062 -> db9069c56 to address the test failures.

[openshift-ci-robot]

@wking: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws	e791a1b	link	/test e2e-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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/test-infra repository. I understand the commands that are listed here.

[dgoodwin]

Am I correct that this solves Hive's need to know what AMI we should use for day 2 machinesets? If so that is awesome, this solves a big need for us.

Will the CLI interface need to change at all when we can pull this info from a release image?

The usage doesn't mention the positional platform argument, it probably should:

Usage:
  openshift-install user-provided-infrastructure bootimage [flags]

[wking]

Am I correct that this solves Hive's need to know what AMI we should use for day 2 machinesets?

Well... say you upgrade to a new release, and the installer associated with that new release pins a newer bootimage. You probably want to use that newer bootimage for additional Machine(Set)s, right? But pulling the installer referenced by the current release image and running this command is slightly less of a cludge than querying existing Machines, and it may be a useful bridge until the machine-os-content images pick up the bootimage metadata. And this command would be a useful shortcut if you were adding Machines within a few minutes of launching the cluster and could assume that no bootimage-bumping upgrades had happened.

The usage doesn't mention the positional platform argument...

Good catch, will fix in the next reroll.

[cgwalters]

Am I correct that this solves Hive's need to know what AMI we should use for day 2 machinesets? If so that is awesome, this solves a big need for us.

I don't think this does or should change the answer for that, which today is - it doesn't change from install time.

[abhinavdahiya]

if the user needs the platform information, it is present in the install-config.

Hmm. I like how the current implementation is decoupled from the install-config, so you don't have to go through the hassle of reading in an install-config just to figure out the bootimage. But I agree that there's a benefit to not having to document two separate interfaces. I dunno, I could go either way here. You're pretty solidly behind the install-config approach?

We need to be consistent as much we can, therefore any perplatfrom stuff needs to through install-config.

only exception that we can support is --all to this subcommand that returns the entire meta.json with all the information.

[wking]

We need to be consistent as much we can, therefore any perplatfrom stuff needs to through install-config.

So the easiest way to do that is probably to make rhcos.Image writeable and attach it to pre-cluster (#1532). I'll close this and open a new PR with that once #1532 lands.