Skip to content
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.

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

Add ignition.platform.id=metal for bare metal #339

Merged
merged 1 commit into from Apr 9, 2019

Conversation

@cgwalters
Copy link
Member

cgwalters commented Feb 9, 2019

@dustymabe

This comment has been minimized.

Copy link
Member

dustymabe commented Feb 11, 2019

This works since we have a separate kickstart for metal. Do we plan for that to always be the case? Or would we possibly use the "golden image" one day for bare metal too? If so it may be better to use gf-oemid like we do for qemu?

Copy link
Member

dustymabe left a comment

One comment/question about approach, otherwise LGTM. Can you make sure that ignition won't barf if it encounters an OEM ID it doesn't know about?

@cgwalters

This comment has been minimized.

Copy link
Member Author

cgwalters commented Feb 11, 2019

Do we plan for that to always be the case?

No - we've talked repeatedly about not using Anaconda right?

If so it may be better to use gf-oemid like we do for qemu?

gf-oemid will fail on EFI layout right now; also doesn't really gain us anything either.

Can you make sure that ignition won't barf if it encounters an OEM ID it doesn't know about?

Looking at the code, it does panic today. So you're definitely right we should fix that before merging.
(Which then argues a bit for adding the parsing support to Ignition itself, but we can continue with the approach from coreos/ignition-dracut#42 for now)

@cgwalters cgwalters changed the title Add coreos.oem.id=metal for bare metal WIP: Add coreos.oem.id=metal for bare metal Feb 11, 2019
@dustymabe dustymabe mentioned this pull request Feb 13, 2019
@dustymabe

This comment has been minimized.

Copy link
Member

dustymabe commented Feb 27, 2019

FYI the 'metal' platform id was added coreos/ignition#724 . Will be in the next ignition release.

@jlebon

This comment has been minimized.

Copy link
Member

jlebon commented Apr 8, 2019

@cgwalters I think we can just s/coreos.oem/ignition.platform/ and get this in now that coreos/ignition#724 is fixed right?

We need this for bare-metal in FCOS (see coreos/coreos-installer#22 (comment)).

For RHCOS, I think this is working right now because we're always outputting an id even if it's the wrong one: https://github.com/coreos/ignition-dracut/blob/79f5d7adfbb83f088be947f6497f04fbc9de939c/dracut/30ignition/ignition-generator#L98.

@jlebon

This comment has been minimized.

Copy link
Member

jlebon commented Apr 8, 2019

For FCOS, I was playing with:

commit afe77b7313c2fafd3820901c5192249613a0b8e3
Date:   Mon Apr 8 16:05:13 2019 -0400

    dracut/30ignition: default to `metal` platform if none given

    The `metal` platform id is essentially a "no-op" in Ignition. IOW, if
    no specific platform id is given, let's just assume that the config is
    baked in.

diff --git a/dracut/30ignition/ignition-generator b/dracut/30ignition/ignition-generator
index 7d56030..439d784 100755
--- a/dracut/30ignition/ignition-generator
+++ b/dracut/30ignition/ignition-generator
@@ -39,4 +39,4 @@ if $(cmdline_bool 'ignition.firstboot' 0); then
     add_requires ignition-complete.target
 fi

-echo "PLATFORM_ID=$(cmdline_arg ignition.platform.id)" > /run/ignition.env
+echo "PLATFORM_ID=$(cmdline_arg ignition.platform.id metal)" > /run/ignition.env

but I'm thinking now it's probably cleaner to just keep erroring the way we do now (though we could improve that error), instead of having this sort of "default" logic.

@jlebon jlebon force-pushed the cgwalters:its-so-metal branch from a7f0482 to 46ec3e0 Apr 9, 2019
@jlebon jlebon force-pushed the cgwalters:its-so-metal branch from 46ec3e0 to cab596c Apr 9, 2019
@jlebon jlebon changed the title WIP: Add coreos.oem.id=metal for bare metal Add ignition.platform.id=metal for bare metal Apr 9, 2019
@jlebon jlebon removed WIP hold labels Apr 9, 2019
@jlebon

This comment has been minimized.

Copy link
Member

jlebon commented Apr 9, 2019

OK, updated! ⬆️
Should be ready to go now.

Copy link
Member

bgilbert left a comment

LGTM

@cgwalters

This comment has been minimized.

Copy link
Member Author

cgwalters commented Apr 9, 2019

LGTM

@cgwalters cgwalters merged commit 9aab362 into coreos:master Apr 9, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jcordes73

This comment has been minimized.

Copy link

jcordes73 commented Apr 10, 2019

How is this supposed to be used? I tried with setting coreos.oem.id=metal and it didn't work for PXE-boot. However after installation I added ignition.platform.id=metal to the kernel boot parameters and this seems to be picked up. I'm using build 29.936.

@jlebon

This comment has been minimized.

Copy link
Member

jlebon commented Apr 10, 2019

This is the id that gets embedded in the raw metal images so that Ignition will know to not try to look elsewhere for an Ignition config and just use the one that was specified at install time. IOW, you shouldn't actually have to type out ignition.platform.id=metal anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.