Skip to content

Comments

Support the CoreOS GRUB /boot/coreos/first_boot flag file#13

Merged
pothos merged 1 commit intoflatcar-masterfrom
kai/coreos-first-boot
May 22, 2020
Merged

Support the CoreOS GRUB /boot/coreos/first_boot flag file#13
pothos merged 1 commit intoflatcar-masterfrom
kai/coreos-first-boot

Conversation

@pothos
Copy link
Member

@pothos pothos commented May 22, 2020

When a machine is migrated from CoreOS, GRUB code is not migrated and
specifies the coreos.first_boot=detected kernel parameter if the flag file
/boot/coreos/first_boot exists.
Run Ignition if the kernel parameter is present.

How to use

See flatcar/scripts#68

Testing done

See flatcar/scripts#68

When a machine is migrated from CoreOS, GRUB code is not migrated and
specifies the coreos.first_boot=detected kernel parameter if the flag file
/boot/coreos/first_boot exists.
Run Ignition if the kernel parameter is present.
pothos added a commit to flatcar-archive/coreos-overlay that referenced this pull request May 22, 2020
@pothos pothos requested a review from a team May 22, 2020 12:11
@pothos pothos marked this pull request as ready for review May 22, 2020 12:11
add_requires ignition-files.service
# Only try to mount the ESP if GRUB detected a first_boot file
if [[ $(cmdline_arg flatcar.first_boot) = "detected" ]]; then
if [[ $(cmdline_arg flatcar.first_boot) = "detected" ]] || [[ $(cmdline_arg coreos.first_boot) = "detected" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could something else be setting this or is it only the grub script from the other change? If it's just the grub script, I think it would be clearer if it set only one value (flatcar.first_boot) instead of two values depending on the directory.

If there's something else that could potentially set this value, then I guess this change would be fine, but I'd like a comment explaining where the double naming comes from.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is that migrated machines will only set coreos.first_boot always because they have the CoreOS GRUB code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should update that GRUB code then. But anyway, in that case, please add a comment explaining that the coreos flag gets set on machines that were upgraded from coreos (and let's please NOT set it ourself).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry for early merging. Yes, we could add a comment, but I didn't think about it because three lines above the two cases are also covered.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if we updated GRUB there can still be iPXE scripts that set the coreos kernel parameters.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I was mostly talking about where the split came from, which is not covered by the comment above (which is talking about the conditional, not about the fact that we take into account both variables.

I think it's important to document why we are keeping both variables and more importantly when we intend to stop supporting both (or what would need to happen for us to stop supporting both).

@pothos pothos merged commit 8043da3 into flatcar-master May 22, 2020
@pothos pothos deleted the kai/coreos-first-boot branch May 22, 2020 13:06
pothos added a commit to flatcar-archive/coreos-overlay that referenced this pull request May 22, 2020
pothos added a commit to flatcar-archive/coreos-overlay that referenced this pull request May 22, 2020
pothos added a commit to flatcar-archive/coreos-overlay that referenced this pull request May 22, 2020
pothos added a commit to flatcar-archive/coreos-overlay that referenced this pull request May 22, 2020
pothos added a commit to flatcar-archive/coreos-overlay that referenced this pull request May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants