fcos: skip ErrReuseByLabel warning for boot_device.mirror disks

[dustymabe]

Config.Validate() warns when a partition on a non-boot disk uses a label without an explicit number and wipe_table is not set. However, when the disk is listed in boot_device.mirror.devices, this warning is a false positive: processBootDevice() in translate.go will auto- generate wipe_table: true for mirror disks, but Validate() runs before translation so it doesn't see the auto-generated wipe_table.

The user can't reasonably suppress this: adding explicit wipe_table: true is redundant, and adding explicit partition number fields would break the label-based merge with the auto-generated mirror partitions.

Skip the ErrReuseByLabel check for disks that are listed in boot_device.mirror.devices.

Fixes: #710

Assisted-by: <anthropic/claude-opus-4.6>

[gemini-code-assist]

Code Review

This pull request updates the configuration validation logic for FCOS (versions v1_6, v1_7, and v1_8_exp) to skip the reuse-by-label check for boot mirror devices, and adds corresponding unit tests. The reviewer suggests optimizing the map allocation for mirrorDevices across all three versions. Specifically, since reading from a nil map is safe in Go, map allocation can be avoided entirely when there are no mirror devices, and the map can be pre-allocated with the exact capacity when mirror devices are present.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

We can optimize this by avoiding map allocation entirely when there are no mirror devices (which is the most common case). In Go, reading from a nil map is safe and returns the zero value (false for bool), so the lookup !mirrorDevices[disk.Device] will still work perfectly without any changes.

Additionally, if there are mirror devices, we can pre-allocate the map with the exact capacity to avoid map resizing overhead.

	var mirrorDevices map[string]bool
	if len(conf.BootDevice.Mirror.Devices) > 0 {
		mirrorDevices = make(map[string]bool, len(conf.BootDevice.Mirror.Devices))
		for _, dev := range conf.BootDevice.Mirror.Devices {
			mirrorDevices[dev] = true
		}
	}

[gemini-code-assist]

We can optimize this by avoiding map allocation entirely when there are no mirror devices (which is the most common case). In Go, reading from a nil map is safe and returns the zero value (false for bool), so the lookup !mirrorDevices[disk.Device] will still work perfectly without any changes.

Additionally, if there are mirror devices, we can pre-allocate the map with the exact capacity to avoid map resizing overhead.

	var mirrorDevices map[string]bool
	if len(conf.BootDevice.Mirror.Devices) > 0 {
		mirrorDevices = make(map[string]bool, len(conf.BootDevice.Mirror.Devices))
		for _, dev := range conf.BootDevice.Mirror.Devices {
			mirrorDevices[dev] = true
		}
	}

[gemini-code-assist]

We can optimize this by avoiding map allocation entirely when there are no mirror devices (which is the most common case). In Go, reading from a nil map is safe and returns the zero value (false for bool), so the lookup !mirrorDevices[disk.Device] will still work perfectly without any changes.

Additionally, if there are mirror devices, we can pre-allocate the map with the exact capacity to avoid map resizing overhead.

	var mirrorDevices map[string]bool
	if len(conf.BootDevice.Mirror.Devices) > 0 {
		mirrorDevices = make(map[string]bool, len(conf.BootDevice.Mirror.Devices))
		for _, dev := range conf.BootDevice.Mirror.Devices {
			mirrorDevices[dev] = true
		}
	}

[travier]

LGTM. Not tested yet.

[travier]

This fixes one of the two cases. The second one is:

Invalid Butane config at ./modules/ROOT/pages/storage.adoc:92:
  warning at $.storage.disks.0.partitions.0.number, line 8 col 7: partitions cannot be reused by label; number must be specified except on boot disk (/dev/disk/by-id/coreos-boot-disk) or when wipe_table is true
  Config produced warnings and --strict was specified

[dustymabe]

./modules/ROOT/pages/storage.adoc

@travier I think that one is a different problem. In that case we should probably set the partition number.

This is what AI says (see below). It actually recommends setting wipe_table: true but I don't think we want to do that here because that would be destructive if people were to reprovision and actually wanted to keep the logs for whatever reason.

---

The second warning is a different case from the boot_device.mirror false
positive. It comes from the "Adding /var/log from a secondary disk" example
at line 92 of storage.adoc:

storage:
  disks:
  - device: /dev/vdb
    wipe_table: false
    partitions:
    - size_mib: 0
      start_mib: 0
      label: log

Here /dev/vdb is a regular secondary disk -- no boot_device.mirror is
involved. The partition has label: log but no explicit number, and
wipe_table is false, so ErrReuseByLabel fires.

Unlike the mirror case, I think this is a legitimate warning and it's the
docs that should be fixed. The user's intent is clearly to treat
/dev/vdb as a fresh disk (they specify start_mib: 0 to place the
partition at the beginning), but wipe_table: false contradicts that --
it tells Ignition to preserve the existing partition table and try to match
existing partitions.

Two possible docs fixes:

1. Change wipe_table: false to wipe_table: true -- this is the more
   correct fix since the example is treating the disk as fresh anyway.
   Setting wipe_table: false with start_mib: 0 is contradictory intent.

2. Add number: 1 to the partition -- this suppresses the warning but
   doesn't fix the underlying contradiction.

That said, there is a possible butane improvement here too: when a
partition specifies start_mib (i.e., the user knows the exact placement),
the warning about "reuse by label" is less relevant since the user isn't
relying on label-matching to find an existing partition -- they're defining
the layout explicitly. But that's a more nuanced validation change and
arguably not worth the complexity.

I'd recommend option 1 (change wipe_table to true) as a second commit
in the fedora-coreos-docs PR.

[travier]

Sounds good indeed. I fixed that in the docs PR.

[travier]

Gemini's suggestions looks OK to me. It should not matter but it does not make the code harder to read or understand.

[dustymabe]

Gemini's suggestions looks OK to me. It should not matter but it does not make the code harder to read or understand.

To me it makes it a little harder to read/understand. Since performance/memory use isn't really a concern here I'd prefer to leave it unless you think it is much better.

[travier]

Works for me.