Identify and match existing partitions based on label

[dustymabe]

Feature Request

Currently if I have a butane config like:

variant: fcos
version: 1.3.0
storage:
  disks:
    - device: /dev/sda
      wipe_table: false
      partitions:
        - number: 0
          label: root
          size_mib: 12000
          resize: true
        - number: 0
          label: var-home
          size_mib: 10240
  filesystems:
    - path: /var/home
      device: /dev/disk/by-partlabel/var-home
      format: xfs
      wipe_filesystem: false
      label: var-home
      with_mount_unit: true

I assume that the label: root entry will match the existing partition with the root label and apply specified changes to that partition.

What I end up with is two partitions with a root partition label and my actual root partition is the minimal 1.8G:

[dustymabe@media images]$ ssh core@192.168.122.31
Warning: Permanently added '192.168.122.31' (ECDSA) to the list of known hosts.
Fedora CoreOS 34.20210427.3.0

############################################################################
WARNING: The root filesystem is too small. It is strongly recommended to
allocate at least 8 GiB of space to allow for upgrades. From June 2021, this
condition will trigger a failure in some cases. For more information, see:
https://docs.fedoraproject.org/en-US/fedora-coreos/storage/

You may delete this warning using:
sudo rm /etc/motd.d/60-coreos-rootfs-size.motd
############################################################################

Tracker: https://github.com/coreos/fedora-coreos-tracker
Discuss: https://discussion.fedoraproject.org/c/server/coreos/

[core@localhost ~]$ 
[core@localhost ~]$ lsblk
NAME   MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
sda      8:0    0   40G  0 disk 
├─sda1   8:1    0    1M  0 part 
├─sda2   8:2    0  127M  0 part 
├─sda3   8:3    0  384M  0 part /boot
├─sda4   8:4    0  1.8G  0 part /sysroot
├─sda5   8:5    0 11.7G  0 part 
└─sda6   8:6    0   10G  0 part /var/home
[core@localhost ~]$ 
[core@localhost ~]$ sudo blkid
/dev/sda3: LABEL="boot" UUID="faa1e08b-44cb-4b22-9fe6-0ec9c7809e1c" BLOCK_SIZE="1024" TYPE="ext4" PARTLABEL="boot" PARTUUID="e694bc20-c397-4c1c-8a9a-a2e585878d18"
/dev/sda4: LABEL="root" UUID="621d6ca4-75e2-4a88-ae98-6d100840ddfc" BLOCK_SIZE="512" TYPE="xfs" PARTLABEL="root" PARTUUID="b081bc27-7fe4-473c-b1ad-7d19dc575d95"
/dev/sda1: PARTLABEL="BIOS-BOOT" PARTUUID="67c1015f-668a-48ce-aea4-e34836df4582"
/dev/sda2: SEC_TYPE="msdos" LABEL_FATBOOT="EFI-SYSTEM" LABEL="EFI-SYSTEM" UUID="24F3-E9D4" BLOCK_SIZE="512" TYPE="vfat" PARTLABEL="EFI-SYSTEM" PARTUUID="6b80bc4c-caad-4eaa-b57d-e9ec319cddc5"
/dev/sda5: PARTLABEL="root" PARTUUID="2bb02c19-239f-42e1-bcbc-981ad69a7138"
/dev/sda6: LABEL="var-home" UUID="d6b014ed-81cd-4cba-9811-a496e1ca844f" BLOCK_SIZE="512" TYPE="xfs" PARTLABEL="var-home" PARTUUID="a64dda2d-910a-4be0-890a-91e0ae9092c9"

We should probably try to match entries to existing labels. Also maybe we should just enforce that partition labels are unique such that there can't be more than one partition with that label.

Other Information

[core@localhost ~]$ rpm-ostree status
State: idle
AutomaticUpdatesDriver: Zincati
  DriverState: active; trying to stage 34.20210518.3.0 (1 failed deployment attempt)
Deployments:
● ostree://fedora:fedora/x86_64/coreos/stable
                   Version: 34.20210427.3.0 (2021-05-18T08:56:57Z)
                    Commit: b4b2199ec09b9e4200024b52062b119035a06b3ffc27b4268c5b8c3aa6fcde17
              GPGSignature: Valid signature by 8C5BA6990BDB26E19F2A1A801161AE6945719A39
[core@localhost ~]$ rpm -q ignition
ignition-2.9.0-4.fc34.x86_64

[bgilbert]

I've made the same mistake, multiple times. The current behavior is described by the spec, though it requires some close reading:

• partitions (list of objects): the list of partitions and their configuration for this particular disk. Every partition must have a unique number, or if 0 is specified, a unique label.
  • label (string): the PARTLABEL for the partition.
  • number (integer): the partition number, which dictates it's position in the partition table (one-indexed). If zero, use the next available partition slot.

udev assumes that every partition label and every filesystem label is unique within the system, and FCOS inherits that assumption for system partitions. However, the GPT spec has no such requirement for partition labels. Ignition currently follows the GPT spec in allowing duplicate partlabels.

(Actually, Ignition only allows creating multiple new partitions with the same label if the partition number is 0 for no more than one of them. That's not for GPT reasons, but to ensure each partition section has a unique key for config merging.)

We could decide to be more opinionated in this area, but it'd be a breaking change that would need to wait for spec 4.0. We'd need to decide what to do with existing partition tables that have duplicate labels; it'd probably make the most sense to error out. Also, we'd only be able to enforce uniqueness on a single disk; I don't think there's a technical justification for enforcing uniqueness across the entire system, nor is there a way to ensure we've seen every disk that could possibly show up later. Since we can't solve systemwide uniqueness for udev, IMO we should think carefully about whether we'd gain enough from solving only per-disk uniqueness.

In the short term, it'd be good to document this behavior more explicitly. Also, Butane should probably check for filesystems with partition 0 and a label from the shipped OS image.

[bgilbert]

Butane RFE: coreos/butane#243

[dustymabe]

I've made the same mistake, multiple times. The current behavior is described by the spec, though it requires some close reading:

I read that spec too over the weekend. This part is interesting:

• partitions (list of objects): the list of partitions and their configuration for this particular disk. Every partition must have a unique number, or if 0 is specified, a unique label.

Maybe it's just me, but the Every partition must have a unique number, or if 0 is specified, a unique label implies that if a partition doesn't have a unique label (in this case I'm specifying a partlabel of root and one already exists) then I'll see an error if the interpretation of Ignition is that it needs to create a new partition (rather than what I expected, which is for it to match the existing partition)).

udev assumes that every partition label and every filesystem label is unique within the system, and FCOS inherits that assumption for system partitions. However, the GPT spec has no such requirement for partition labels. Ignition currently follows the GPT spec in allowing duplicate partlabels.

Yeah. I guess I'm interested in the uses. Can we think of a good use of multiple same labeled partitions? If not I say we default to requiring unique labels and then allowing an override (unique_label: false) or something.

We could decide to be more opinionated in this area, but it'd be a breaking change that would need to wait for spec 4.0. We'd need to decide what to do with existing partition tables that have duplicate labels; it'd probably make the most sense to error out.

I'm good with waiting til spec 4.0 and also good with erroring out. Maybe unique_label: false would allow people to get around the error.

Also, we'd only be able to enforce uniqueness on a single disk; I don't think there's a technical justification for enforcing uniqueness across the entire system, nor is there a way to ensure we've seen every disk that could possibly show up later. Since we can't solve systemwide uniqueness for udev, IMO we should think carefully about whether we'd gain enough from solving only per-disk uniqueness.

I'm good with single disk enforcement.

[jlebon]

I've also hit this multiple times and think it's reasonable to tighten this up. It also leads to documentation to expose implementation details: coreos/fedora-coreos-docs#248 (comment)

[cgwalters]

(Coming here from coreos/fedora-coreos-tracker#855 )

It looks to me like we need a "do what I really want" flag like unique_label: true as proposed above, and we start generating that in newer FCOS butane specs by default, and add it to our docs?

Since we can't solve systemwide uniqueness for udev, IMO we should think carefully about whether we'd gain enough from solving only per-disk uniqueness.

In this case though, we're going to mount something from /dev/disk/by-label/root, so there can only be one at that time, right?

Also, Butane should probably check for filesystems with partition 0 and a label from the shipped OS image.

Right, and in that case inject unique_label: true or whatever the flag is?

[bgilbert]

Maybe it's just me, but the Every partition must have a unique number, or if 0 is specified, a unique label implies that if a partition doesn't have a unique label (in this case I'm specifying a partlabel of root and one already exists) then I'll see an error if the interpretation of Ignition is that it needs to create a new partition (rather than what I expected, which is for it to match the existing partition).

Okay, I see what you mean. The spec is written from the perspective of the config contents, and what it actually means is: "Every specified partition must have a unique number, or ... a unique label".

It looks to me like we need a "do what I really want" flag like unique_label: true as proposed above, and we start generating that in newer FCOS butane specs by default, and add it to our docs?

We've generally tried to avoid flags, and implementing one here would be a bit messy (because the lookup key needs to change depending on a flag field in the parent struct). But we may not have a choice. If we made the change in spec 4.0 and removed the old behavior, we'd be removing functionality present in spec 3.x, which would make old configs non-translatable. And I don't think we want to go there again.

But you're right that we could hardwire the flag in Butane. That'd involve a major bump of the Butane spec, but there's no requirement to translate Butane configs between versions (since Butane carries parallel implementations) so that'd be relatively painless. (Except that we can't bump the major version of the openshift variant, sigh.) And then we wouldn't need a major bump in Ignition.

In this case though, we're going to mount something from /dev/disk/by-label/root, so there can only be one at that time, right?

There's nothing preventing multiple partitions from having the same label and racing with each other.

Also, Butane should probably check for filesystems with partition 0 and a label from the shipped OS image.

Right, and in that case inject unique_label: true or whatever the flag is?

I meant that it could warn the user that they're probably not doing what they intend.