Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

30ignition: GPT setup: wait for 'boot' labelled partition #191

Merged
merged 1 commit into from
Jun 9, 2020
Merged

30ignition: GPT setup: wait for 'boot' labelled partition #191

merged 1 commit into from
Jun 9, 2020

Conversation

travier
Copy link
Member

@travier travier commented Jun 9, 2020

Make sure that we wait for the disk with the partition labeled 'boot' to
come up. This is the only unit where it is safe to wait on a specific
disk label. Units that requires specific partitions to be available
should order themselves after ignition-diskful.target.

We used to wait for the partition labeled 'root' but this name will
change with LUKS support.

# Make sure that we wait for the disk with the partition labeled 'boot' to come
# up. This is the only unit where it is safe to wait on a specific disk label.
# Units that requires specific partitions to be available should order
# themselves after ignition-diskful.target.
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean ignition-disks.service?

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I understand, you want to order after ignition-diskful.target to work on 1rst boot available partitions and after ignition-disks.service for ignition provisioned partitions. Maybe I should clarify the comment.

Copy link
Member

Choose a reason for hiding this comment

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

I'd say it like anything that is operating on "persistent bootable disk" needs -diskful.target, as does this. We probably don't need to describe that in a comment here, the idea should be covered in the unit docs.

What I would say in this comment is two things:

  • First, we don't have any plans to support re-provisioning/re-writing the /boot partition. So the disk holding it must be the root.
  • We can't use root because RHCOS's pre-setup LUKS container doesn't allow it

This all said...I am increasingly feeling we should make this logic part of ignition-disks.service along with coreos/fedora-coreos-config#354 or so.

Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Logical change looks good to me!

travier added a commit to travier/fedora-coreos-config that referenced this pull request Jun 9, 2020
The GPT setup unit was simplified to reflect the fact that it will only
be called once for the disk with the partition labeled 'boot'.

This keeps the old unit name for now for compatibility. This will be
safe to remove once the dracut-ignition changes from [1] are available
in the latest ignition RPM.

[1] coreos/ignition-dracut#191
travier added a commit to travier/fedora-coreos-config that referenced this pull request Jun 9, 2020
The GPT setup unit is simplified to reflect the fact that this unit will
only be called once for the disk with the partition labeled 'boot'.

This keeps the old unit name for now for compatibility. This will be
safe to remove once the dracut-ignition changes from [1] are available
in the latest ignition RPM.

[1] coreos/ignition-dracut#191
This unit must the first to run when the disk holding the root partition
becomes available. To avoid relying on the name of the root partition
which is different between RHCOS LUKS setup and current FCOS setup, we
wait for the partition labeled 'boot' to become available. This is
reliable as we don't have any plan to support re-provisioning/re-writing
the /boot partition,

This is the only unit where it is safe to wait only on a specific disk
label as this will call udevadm settle after the GPT setup. Units that
requires the boot and root partitions to be available should order
themselves after this unit.
@travier
Copy link
Member Author

travier commented Jun 9, 2020

Updated the comments to add more info. Working on a graph to explain more clearly why this is required and what the boot order will look like in the end.

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

LGTM!

@jlebon jlebon merged commit bdf0a65 into coreos:master Jun 9, 2020
@travier travier deleted the gpt-boot-label branch June 10, 2020 09:38
cgwalters pushed a commit to coreos/fedora-coreos-config that referenced this pull request Jun 11, 2020
The GPT setup unit is simplified to reflect the fact that this unit will
only be called once for the disk with the partition labeled 'boot'.

This keeps the old unit name for now for compatibility. This will be
safe to remove once the dracut-ignition changes from [1] are available
in the latest ignition RPM.

[1] coreos/ignition-dracut#191
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants