Add BMH live-iso proposal #150

hardys · 2020-11-10T17:04:35Z

Adds a proposal to expose an Ironic API that allows us to boot an arbitrary live-iso image, vs only specifying an image to be written to disk by IPA.

hardys · 2020-11-10T17:04:59Z

/cc @zaneb @dhellmann

design/baremetal-operator/bmh_boot_iso.md

dtantsur · 2020-11-16T12:37:58Z

design/baremetal-operator/bmh_boot_iso.md

+fast-track provisioning in this workflow (since it's booting two different iso images)
+so this adds a reboot into the process.  This can potentially be avoided in some cases
+where we can provide a status annotation on creation of the BMH such that inspection
+is not performed.


Unfortunately, currently fast-track does not really work with virtual media at all (reboots always happen). I'll be working on it in the coming weeks.

@dtantsur thanks, if you have a link for this please let me know, and I'll add it to the list of Ironic roadmap items

https://storyboard.openstack.org/#!/story/2008321

design/baremetal-operator/bmh_boot_iso.md

hardys · 2020-11-18T17:36:45Z

Ok thanks for the feedback all - I've pushed an update which I think addresses the feedback so far :)

dhellmann

I'm happy with the state of this. There are a few details we could fill in as updates to this PR or as separate PRs later.

/approve

dhellmann · 2020-11-19T20:10:51Z

design/baremetal-operator/bmh_live_boot.md

+  iso is booted, we don't have a way to detach the virtualmedia and will have to
+  rely e.g on efibootmgr in whatever iso gets booted to ensure the correct boot device.
+
+- The inspection of a BMH will still rely on booting IPA and there's no support for


We wouldn't do it as part of this enhancement, but should we add a flag to disable inspection to support workflows that get the hardware details another way?

Yes we could do, I was thinking we'd rely on the existing interface e.g pass some status annotation at the point of creating the BMH, but then we'd need to change behavior so it can be updated after the hardware details are collected.

A flag may be clearer though, I can either include that here or in a follow-up proposal when we have the boot-iso part working?

For the use case you're working on, the host resource needs to exist before the ISO with the alternative agent is booted, so there's no opportunity to collect valid data in advance. We could pass fake data, but that feels wrong.

How about if we say that by default if there is a live image on the host we would not perform inspection, under the assumption that we either don't care about hardware details in those cases or the details will come from somewhere else later. Then we can use other design docs to cover an API for setting hardware details after the first reconciliation and to add a flag like spec.liveImage.enableInspection to restore the ability to (optionally) perform inspection with IPA before attaching the ISO.

I'm not sure making inspection conditional on the spec.liveImage will work, because we may create the BMH resource then set the image reference later, and in that case we want the BMH to enroll and be ready to boot the iso, but not perform the existing inspection steps?

Perhaps adding an explicit flag elsewhere in the spec would be clearer?

Let's just have an annotation that causes inspection to be skipped. That's simple to do, works for all use cases, and avoids making it part of the Spec which is a bit icky.
There is a proposal I can't find right now to add an annotation to force a new inspection. Maybe we could use the same key with different values (e.g. never to skip it even the first time vs. refresh to trigger a new one).

Note that to add the data externally later on you'll need RBAC write access to the Status, which is unusual.

@zaneb I think this deserves a separate enhancement proposal - I suspect we need a way to skip inspection, but also add the data via the existing status annotation (avoiding the status RBAC issue you mention), which beyond the scope of this PR - I'll start a new one if that sounds reasonable.

I agree with moving the inspection discussion to another enhancement.

design/baremetal-operator/bmh_live_boot.md

zaneb

/approve

zaneb · 2020-12-09T17:12:56Z

design/baremetal-operator/bmh_live_boot.md

+
+Note that in this mode, `rootDeviceHints` and `userData` will not be used
+since the image won't be written to disk, and Ironic doesn't
+support passing user-data in addition to the iso attachment at present.


It's about time we added a webhook to validate this kind of stuff.

There's a period of time when validation web hooks aren't applied, between the time the CRD is installed (and the API server supports the new resource type) and when the web hook server is registered and starts.

zaneb · 2020-12-09T17:20:45Z

design/baremetal-operator/bmh_live_boot.md

+  iso is booted, we don't have a way to detach the virtualmedia and will have to
+  rely e.g on efibootmgr in whatever iso gets booted to ensure the correct boot device.
+
+- The inspection of a BMH will still rely on booting IPA and there's no support for


Let's just have an annotation that causes inspection to be skipped. That's simple to do, works for all use cases, and avoids making it part of the Spec which is a bit icky.
There is a proposal I can't find right now to add an annotation to force a new inspection. Maybe we could use the same key with different values (e.g. never to skip it even the first time vs. refresh to trigger a new one).

Note that to add the data externally later on you'll need RBAC write access to the Status, which is unusual.

hardys · 2020-12-11T09:33:22Z

Ok I added the Ironic links, and if folks agree I think the skipping inspection workflow would be best discussed in a new proposal - implementation of this doesn't directly depend on it, so I'll create a new PR where we can decide the best approach for that.

To enable testing related to metal3-io/metal3-docs#150

dhellmann · 2020-12-11T21:39:30Z

design/baremetal-operator/bmh_live_boot.md

+  iso is booted, we don't have a way to detach the virtualmedia and will have to
+  rely e.g on efibootmgr in whatever iso gets booted to ensure the correct boot device.
+
+- The inspection of a BMH will still rely on booting IPA and there's no support for


I agree with moving the inspection discussion to another enhancement.

metal3-io-bot · 2020-12-11T21:40:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhellmann, hardys, 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:

~~OWNERS~~ [dhellmann,hardys,zaneb]

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

hardys · 2020-12-16T11:39:29Z

/retitle Add BMH LiveImage proposal

Add initial support for downloading an FCOS iso, related to development of metal3-io/metal3-docs#150

hardys · 2020-12-22T15:34:19Z

I think this is now ready to merge?

We'll also need some way to disable cleaning as discussed via #151 and I'll raise a new enhancement to discuss how we can skip inspection.

dhellmann · 2021-01-04T15:28:35Z

I think this is now ready to merge?

I think so. @zaneb approved an earlier version and I'm happy with this version. It would be good to have a thumbs-up from someone else, too.

Maybe we can start the lazy consensus process this week?

hardys · 2021-01-07T16:51:28Z

Discussion on the implementation PR and slack indicates we may want to go with a different interface so I'll hold this until that direction is agreed.

/hold

dtantsur · 2021-01-11T12:06:15Z

design/baremetal-operator/bmh_live_boot.md

+
+### Risks and Mitigations
+
+- Currently there is no detachment API in ironic, so in the case where an installer


There is now! Not sure we have any documentation about it, but ping @bfournie or myself for pointers.

dtantsur · 2021-01-11T12:08:02Z

design/baremetal-operator/bmh_live_boot.md

+  avoided in some cases where we can provide a status annotation on creation of
+  the BMH such that inspection is not performed.
+
+- Similarly cleaning of a BMH will rely on IPA, so will require a reboot which may


For clarity: in-band cleaning. There is also out-of-band cleaning that is done via talking to the BMC, and we'll keep using that. It is only available as manual cleaning now, so it's fine to disable the automatic one.

dtantsur · 2021-01-11T12:12:37Z

design/baremetal-operator/bmh_live_boot.md

+
+[Allow config-drive with virtualmedia ramdisk deploy](https://storyboard.openstack.org/#!/story/2008380)
+
+[Method to detach virtualmedia after initial boot](https://storyboard.openstack.org/#!/story/2008363)


dtantsur · 2021-01-11T12:12:48Z

design/baremetal-operator/bmh_live_boot.md

+these don't block the initial implementation but may be desirable to make
+this feature more flexible:
+
+[Allow config-drive with virtualmedia ramdisk deploy](https://storyboard.openstack.org/#!/story/2008380)


On review, will try to get it merged this week.

UPD: merged

hardys · 2021-01-12T14:00:38Z

Alternative BMH API proposed via metal3-io/baremetal-operator#759 - if we're in agreement with that I'll rework this to reflect the new interface.

hardys · 2021-01-12T18:00:58Z

Updated to reflect interface in metal3-io/baremetal-operator#759

/hold cancel

hardys · 2021-01-13T10:24:16Z

/retitle Add BMH live-iso proposal

hardys · 2021-01-21T09:15:57Z

The implementation landed ref metal3-io/baremetal-operator#759 so it'd be good to merge this - @zaneb could you revisit perhaps? Thanks!

dhellmann · 2021-01-22T15:13:39Z

/retest
/lgtm

Since the code has merged, let's land this and update it in another PR if we're missing any details.

zaneb · 2021-01-22T15:21:12Z

Netlify tests are failing with make: *** No rule to make target 'build'. Stop., so I would infer that this needs to be rebased to pick up whatever tooling support was added in the repo for those jobs.
(I miss Zuul.)

hardys · 2021-01-22T16:01:03Z

Netlify tests are failing with make: *** No rule to make target 'build'. Stop., so I would infer that this needs to be rebased to pick up whatever tooling support was added in the repo for those jobs.
(I miss Zuul.)

Thanks, now rebased and the jobs completed OK

dhellmann · 2021-01-22T16:29:40Z

/lgtm

To enable testing related to metal3-io/metal3-docs#150

metal3-io-bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 10, 2020

metal3-io-bot requested review from dhellmann and zaneb November 10, 2020 17:05

dhellmann reviewed Nov 10, 2020

View reviewed changes

design/baremetal-operator/bmh_boot_iso.md Outdated Show resolved Hide resolved

design/baremetal-operator/bmh_boot_iso.md Outdated Show resolved Hide resolved

design/baremetal-operator/bmh_boot_iso.md Outdated Show resolved Hide resolved

hardys removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 11, 2020

dtantsur reviewed Nov 16, 2020

View reviewed changes

hardys force-pushed the boot_iso branch from dd64aed to a2aefda Compare November 18, 2020 17:34

metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 18, 2020

hardys force-pushed the boot_iso branch from a2aefda to e4272e6 Compare November 18, 2020 17:39

hardys mentioned this pull request Nov 19, 2020

Installer/MCO: store pointer ignition customizations in MachineConfig openshift/enhancements#540

Merged

hardys force-pushed the boot_iso branch from e4272e6 to 3315d8c Compare November 19, 2020 15:19

dhellmann reviewed Nov 19, 2020

View reviewed changes

dhellmann approved these changes Nov 19, 2020

View reviewed changes

zaneb reviewed Dec 9, 2020

View reviewed changes

hardys mentioned this pull request Dec 10, 2020

Make online status of extra workers configurable openshift-metal3/dev-scripts#1172

Merged

hardys mentioned this pull request Dec 11, 2020

Enable ramdisk deploy_interface metal3-io/ironic-image#234

Merged

hardys pushed a commit to hardys/ironic-image that referenced this pull request Dec 11, 2020

Enable ramdisk deploy_interface

575d126

To enable testing related to metal3-io/metal3-docs#150

dhellmann approved these changes Dec 11, 2020

View reviewed changes

hardys mentioned this pull request Dec 15, 2020

WIP Add LiveImage support metal3-io/baremetal-operator#753

Closed

metal3-io-bot changed the title ~~Add BMH boot-iso proposal~~ Add BMH LiveImage proposal Dec 16, 2020

hardys mentioned this pull request Dec 16, 2020

Add LiveImage support metal3-io/baremetal-operator#754

Closed

hardys pushed a commit to hardys/metal3-dev-env that referenced this pull request Dec 18, 2020

Initial support for testing with a live-iso

e7aa7c4

Add initial support for downloading an FCOS iso, related to development of metal3-io/metal3-docs#150

hardys mentioned this pull request Dec 18, 2020

Initial support for testing with a live-iso metal3-io/metal3-dev-env#579

Merged

hardys pushed a commit to hardys/metal3-dev-env that referenced this pull request Dec 18, 2020

Initial support for testing with a live-iso

9241cdb

Add initial support for downloading an FCOS iso, related to development of metal3-io/metal3-docs#150

hardys pushed a commit to hardys/metal3-dev-env that referenced this pull request Dec 21, 2020

Initial support for testing with a live-iso

d1d6bc7

Add initial support for downloading an FCOS iso, related to development of metal3-io/metal3-docs#150

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 7, 2021

dtantsur reviewed Jan 11, 2021

View reviewed changes

hardys force-pushed the boot_iso branch from 45ea3a0 to 663d206 Compare January 12, 2021 18:00

metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 12, 2021

metal3-io-bot changed the title ~~Add BMH LiveImage proposal~~ Add BMH live-iso proposal Jan 13, 2021

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 22, 2021

Add BMH boot-iso proposal

a15ef96

hardys force-pushed the boot_iso branch from 663d206 to a15ef96 Compare January 22, 2021 15:42

metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 22, 2021

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 22, 2021

metal3-io-bot merged commit 62dc0de into metal3-io:master Jan 22, 2021

derekhiggins pushed a commit to derekhiggins/ironic-image that referenced this pull request Feb 16, 2021

Enable ramdisk deploy_interface

4008a53

To enable testing related to metal3-io/metal3-docs#150


		### Risks and Mitigations

		- Currently there is no detachment API in ironic, so in the case where an installer


		[Allow config-drive with virtualmedia ramdisk deploy](https://storyboard.openstack.org/#!/story/2008380)

		[Method to detach virtualmedia after initial boot](https://storyboard.openstack.org/#!/story/2008363)

Add BMH live-iso proposal #150

Add BMH live-iso proposal #150

Conversation

hardys commented Nov 10, 2020

hardys commented Nov 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hardys commented Nov 18, 2020

dhellmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zaneb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hardys commented Dec 11, 2020

Choose a reason for hiding this comment

metal3-io-bot commented Dec 11, 2020

hardys commented Dec 16, 2020

hardys commented Dec 22, 2020

dhellmann commented Jan 4, 2021

hardys commented Jan 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtantsur Jan 11, 2021 • edited Loading

Choose a reason for hiding this comment

hardys commented Jan 12, 2021

hardys commented Jan 12, 2021

hardys commented Jan 13, 2021

hardys commented Jan 21, 2021

dhellmann commented Jan 22, 2021

zaneb commented Jan 22, 2021

hardys commented Jan 22, 2021

dhellmann commented Jan 22, 2021

dtantsur Jan 11, 2021 •

edited

Loading