From b562d144ca4bdce0232282ee6c8fa31a2113b0c7 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Sat, 6 Aug 2022 22:41:27 +0200 Subject: [PATCH] distro/*: allow `/boot` to be customized Since the LVM support was added to all distros, our disk related code is adaptive, i.e. we will set the correct BLS and grub2 prefix if there a `boot` partiton is present in the layout after all customizations happen, which includes LVMification. One thing that was not yet fully working was layouts that do not yet have a `/boot` partition but allow LVMification. In that case `NewPartitionTable` and if `/boot` was the first (or only) customization, would LVMify the partition which in turn would create the `/boot` partition; but after `newPT.ensureLVM()` the call to `newPT.createFilesystem` with `/boot` would try to create another `/boot` mountpoint. In order to deal with this situation correctly we are now using a two phase approach: 1) enlarge existing mountpoints and collect new ones. 2) if there are new ones and LMVify was allowed, switch to LVM layout. Do a second pass and now create or enlarge existing partitions, handling `/boot` in the process. --- internal/client/compose_test.go | 2 +- internal/disk/disk.go | 1 + internal/disk/disk_test.go | 30 +++++++++++++++++++++++ internal/disk/partition_table.go | 34 ++++++++++++++++++++++----- internal/distro/fedora/distro_test.go | 4 ++-- internal/distro/rhel7/distro_test.go | 4 ++-- internal/distro/rhel8/distro_test.go | 4 ++-- internal/distro/rhel90/distro_test.go | 4 ++-- test/cases/filesystem.sh | 4 ++-- 9 files changed, 70 insertions(+), 17 deletions(-) diff --git a/internal/client/compose_test.go b/internal/client/compose_test.go index fb224eb47..dd9b5e874 100644 --- a/internal/client/compose_test.go +++ b/internal/client/compose_test.go @@ -553,7 +553,7 @@ func TestComposeUnsupportedMountPointV0(t *testing.T) { description="TestComposeUnsupportedMountPointV0" version="0.0.1" [[customizations.filesystem]] - mountpoint = "/boot" + mountpoint = "/etc" size = 4294967296 ` resp, err := PostTOMLBlueprintV0(testState.socket, bp) diff --git a/internal/disk/disk.go b/internal/disk/disk.go index 3a8cef90e..85680feb3 100644 --- a/internal/disk/disk.go +++ b/internal/disk/disk.go @@ -54,6 +54,7 @@ const ( var MountpointPolicies = NewPathPolicies(map[string]PathPolicy{ "/": {Exact: true}, + "/boot": {Exact: true}, "/var": {}, "/opt": {}, "/srv": {}, diff --git a/internal/disk/disk_test.go b/internal/disk/disk_test.go index 6f9b38d36..7e98ced62 100644 --- a/internal/disk/disk_test.go +++ b/internal/disk/disk_test.go @@ -617,6 +617,36 @@ func TestMinimumSizes(t *testing.T) { } } +func TestNewBootWithSizeLVMify(t *testing.T) { + pt := testPartitionTables["plain-noboot"] + assert := assert.New(t) + + // math/rand is good enough in this case + /* #nosec G404 */ + rng := rand.New(rand.NewSource(13)) + + custom := []blueprint.FilesystemCustomization{ + { + Mountpoint: "/boot", + MinSize: 700 * MiB, + }, + } + + mpt, err := NewPartitionTable(&pt, custom, uint64(3*GiB), true, rng) + assert.NoError(err) + + for idx, c := range custom { + mnt, minSize := c.Mountpoint, c.MinSize + path := entityPath(mpt, mnt) + assert.NotNil(path, "[%d] mountpoint %q not found", idx, mnt) + parent := path[1] + part, ok := parent.(*Partition) + assert.True(ok, "%q parent (%v) is not a partition", mnt, parent) + assert.GreaterOrEqual(part.GetSize(), minSize, + "[%d] %q size %d should be greater or equal to %d", idx, mnt, part.GetSize(), minSize) + } +} + func collectEntities(pt *PartitionTable) []Entity { entities := make([]Entity, 0) collector := func(ent Entity, path []Entity) error { diff --git a/internal/disk/partition_table.go b/internal/disk/partition_table.go index 93677ac5f..dcf784324 100644 --- a/internal/disk/partition_table.go +++ b/internal/disk/partition_table.go @@ -21,17 +21,33 @@ type PartitionTable struct { func NewPartitionTable(basePT *PartitionTable, mountpoints []blueprint.FilesystemCustomization, imageSize uint64, lvmify bool, rng *rand.Rand) (*PartitionTable, error) { newPT := basePT.Clone().(*PartitionTable) + newMountpoints := []blueprint.FilesystemCustomization{} + + // first pass: enlarge existing mountpoints and collect new ones for _, mnt := range mountpoints { + if path := entityPath(newPT, mnt.Mountpoint); len(path) != 0 { + size := newPT.AlignUp(clampFSSize(mnt.Mountpoint, mnt.MinSize)) + resizeEntityBranch(path, size) + } else { + newMountpoints = append(newMountpoints, mnt) + } + } + + // if there is any new mountpoint and lvmify is enabled, ensure we have LVM layout + if lvmify && len(newMountpoints) > 0 { + err := newPT.ensureLVM() + if err != nil { + return nil, err + } + } + + // second pass: deal with new mountpoints and newly created ones, after switching to + // the LVM layout, if requested, which might introduce new mount points, i.e. `/boot` + for _, mnt := range newMountpoints { size := newPT.AlignUp(clampFSSize(mnt.Mountpoint, mnt.MinSize)) if path := entityPath(newPT, mnt.Mountpoint); len(path) != 0 { resizeEntityBranch(path, size) } else { - if lvmify { - err := newPT.ensureLVM() - if err != nil { - return nil, err - } - } if err := newPT.createFilesystem(mnt.Mountpoint, size); err != nil { return nil, err } @@ -467,7 +483,13 @@ func (pt *PartitionTable) FindMountable(mountpoint string) Mountable { func clampFSSize(mountpoint string, size uint64) uint64 { // set a minimum size of 1GB for all mountpoints + // with the exception for '/boot' (= 500 MB) var minSize uint64 = 1073741824 + + if mountpoint == "/boot" { + minSize = 524288000 + } + if minSize > size { return minSize } diff --git a/internal/distro/fedora/distro_test.go b/internal/distro/fedora/distro_test.go index 4dca8ae4f..5ebd87a38 100644 --- a/internal/distro/fedora/distro_test.go +++ b/internal/distro/fedora/distro_test.go @@ -529,7 +529,7 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) { Filesystem: []blueprint.FilesystemCustomization{ { MinSize: 1024, - Mountpoint: "/boot", + Mountpoint: "/etc", }, }, }, @@ -544,7 +544,7 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) { } else if imgTypeName == "fedora-iot-installer" { continue } else { - assert.EqualError(t, err, "The following custom mountpoints are not supported [\"/boot\"]") + assert.EqualError(t, err, "The following custom mountpoints are not supported [\"/etc\"]") } } } diff --git a/internal/distro/rhel7/distro_test.go b/internal/distro/rhel7/distro_test.go index b727f941a..24bc87f8e 100644 --- a/internal/distro/rhel7/distro_test.go +++ b/internal/distro/rhel7/distro_test.go @@ -275,7 +275,7 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) { Filesystem: []blueprint.FilesystemCustomization{ { MinSize: 1024, - Mountpoint: "/boot", + Mountpoint: "/etc", }, }, }, @@ -285,7 +285,7 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) { for _, imgTypeName := range arch.ListImageTypes() { imgType, _ := arch.GetImageType(imgTypeName) _, err := imgType.Manifest(bp.Customizations, distro.ImageOptions{}, nil, nil, nil, 0) - assert.EqualError(t, err, "The following custom mountpoints are not supported [\"/boot\"]") + assert.EqualError(t, err, "The following custom mountpoints are not supported [\"/etc\"]") } } } diff --git a/internal/distro/rhel8/distro_test.go b/internal/distro/rhel8/distro_test.go index ac7827ad9..645adb311 100644 --- a/internal/distro/rhel8/distro_test.go +++ b/internal/distro/rhel8/distro_test.go @@ -614,7 +614,7 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) { Filesystem: []blueprint.FilesystemCustomization{ { MinSize: 1024, - Mountpoint: "/boot", + Mountpoint: "/etc", }, }, }, @@ -629,7 +629,7 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) { } else if imgTypeName == "edge-installer" || imgTypeName == "edge-simplified-installer" || imgTypeName == "edge-raw-image" { continue } else { - assert.EqualError(t, err, "The following custom mountpoints are not supported [\"/boot\"]") + assert.EqualError(t, err, "The following custom mountpoints are not supported [\"/etc\"]") } } } diff --git a/internal/distro/rhel90/distro_test.go b/internal/distro/rhel90/distro_test.go index c1b586469..56f3aea97 100644 --- a/internal/distro/rhel90/distro_test.go +++ b/internal/distro/rhel90/distro_test.go @@ -606,7 +606,7 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) { Filesystem: []blueprint.FilesystemCustomization{ { MinSize: 1024, - Mountpoint: "/boot", + Mountpoint: "/etc", }, }, }, @@ -621,7 +621,7 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) { } else if imgTypeName == "edge-installer" || imgTypeName == "edge-simplified-installer" || imgTypeName == "edge-raw-image" { continue } else { - assert.EqualError(t, err, "The following custom mountpoints are not supported [\"/boot\"]") + assert.EqualError(t, err, "The following custom mountpoints are not supported [\"/etc\"]") } } } diff --git a/test/cases/filesystem.sh b/test/cases/filesystem.sh index 1cb86529f..8eb22b865 100644 --- a/test/cases/filesystem.sh +++ b/test/cases/filesystem.sh @@ -242,7 +242,7 @@ mountpoint = "/etc" size = 131072000 [[customizations.filesystem]] -mountpoint = "/boot" +mountpoint = "/lost+found" size = 131072000 EOF @@ -254,7 +254,7 @@ build_image "$BLUEPRINT_FILE" rhel85-custom-filesystem-fail qcow2 true FAILED_MOUNTPOINTS=() greenprint "💬 Checking expected failures" -for MOUNTPOINT in '/etc' '/boot' ; do +for MOUNTPOINT in '/etc' '/lost+found' ; do if ! [[ $ERROR_MSG == *"$MOUNTPOINT"* ]]; then FAILED_MOUNTPOINTS+=("$MOUNTPOINT") fi