From 68d5a9f671e0b14109a56f88a888be1dea5e4838 Mon Sep 17 00:00:00 2001 From: Steven Presti Date: Thu, 15 Jan 2026 09:17:46 -0500 Subject: [PATCH 1/4] fcos v0_7_exp: add warn on small or constrained root partition Fixes: #211, the root partition needs a certain amount of space, with recent changes butane has the opportunity to warn when those expectations are not met. Check for contstraints caused by: - Partitions with out-of-order numbers - Auto-positioned partitions (StartMiB == 0 or nil) - Explicit partition positioning --- config/common/errors.go | 2 + config/fcos/v1_7_exp/translate.go | 68 +++++++++++++++++++++++++++---- docs/release-notes.md | 4 +- 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/config/common/errors.go b/config/common/errors.go index 0f82b4006..1f0c83868 100644 --- a/config/common/errors.go +++ b/config/common/errors.go @@ -68,6 +68,8 @@ var ( // partition ErrReuseByLabel = errors.New("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") ErrWrongPartitionNumber = errors.New("incorrect partition number; a new partition will be created using reserved label") + ErrRootTooSmall = errors.New("root should have 8GiB of space assigned") + ErrRootConstrained = errors.New("root is configured too small, and has no room to expand") // MachineConfigs ErrFieldElided = errors.New("field ignored in raw mode") diff --git a/config/fcos/v1_7_exp/translate.go b/config/fcos/v1_7_exp/translate.go index bd31358eb..a380534c3 100644 --- a/config/fcos/v1_7_exp/translate.go +++ b/config/fcos/v1_7_exp/translate.go @@ -71,16 +71,70 @@ func (c Config) ToIgn3_6Unvalidated(options common.TranslateOptions) (types.Conf } r.Merge(c.processBootDevice(&ret, &ts, options)) for i, disk := range ret.Storage.Disks { - // In the boot_device.mirror case, nothing specifies partition numbers - // so match existing partitions only when `wipeTable` is false - if !util.IsTrue(disk.WipeTable) { - for j, partition := range disk.Partitions { - // check for reserved partlabels - if partition.Label != nil { + for p, partition := range disk.Partitions { + // if the partition is not nil and is root + if partition.Label != nil { + if *partition.Label == "root" { + if partition.SizeMiB == nil || *partition.SizeMiB == 0 { + // Root is set to "fill available" (0 or unspecified) + // Check if it's constrained by the next partition on disk + // Sort partitions by disk position (StartMiB) to find the next partition + var rootStart int + if partition.StartMiB != nil { + rootStart = *partition.StartMiB + } + + // Find the partition that comes immediately after root on disk + var nextPartition *types.Partition + for idx, ap := range disk.Partitions { + // Skip the root partition itself + if idx == p { + continue + } + + var apStart int + if ap.StartMiB != nil { + apStart = *ap.StartMiB + } + + // Only consider partitions that start after root + // (or auto-positioned partitions which will be placed after root) + if apStart > rootStart || (apStart == 0 && rootStart >= 0) { + if nextPartition == nil { + nextPartition = &ap + } else { + var nextStart int + if nextPartition.StartMiB != nil { + nextStart = *nextPartition.StartMiB + } + // If this partition starts before the current "next", it's closer to root + if (apStart > 0 && apStart < nextStart) || (nextStart == 0 && apStart > 0) { + nextPartition = &ap + } + } + } + } + + // Check if the next partition constrains root + if nextPartition != nil { + if nextPartition.StartMiB == nil || *nextPartition.StartMiB == 0 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrRootConstrained) + } + } + } else if *partition.SizeMiB < 8192 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "size_mib"), common.ErrRootTooSmall) + } + } + + // In the boot_device.mirror case, nothing specifies partition numbers + // so match existing partitions only when `wipeTable` is false + if !util.IsTrue(disk.WipeTable) { + // check for reserved partlabels if (*partition.Label == "BIOS-BOOT" && partition.Number != 1) || (*partition.Label == "PowerPC-PReP-boot" && partition.Number != 1) || (*partition.Label == "EFI-SYSTEM" && partition.Number != 2) || (*partition.Label == "boot" && partition.Number != 3) || (*partition.Label == "root" && partition.Number != 4) { - r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", j, "label"), common.ErrWrongPartitionNumber) + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrWrongPartitionNumber) } } + } } } diff --git a/docs/release-notes.md b/docs/release-notes.md index 5f4d4b663..9b4db79e9 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -14,6 +14,9 @@ nav_order: 9 ### Misc. changes +- Warn on root partition size is too small _(fcos 1.7.0-exp)_ +- Warn on root partition constrained by another partition _(fcos 1.7.0-exp)_ + ### Docs changes @@ -36,7 +39,6 @@ key](https://getfedora.org/security/). ### Bug fixes - Warn for `boot_device.layout` to be specified when using `boot_device.mirror` _(fcos 1.3.0-1.6.0)_ - ### Docs changes - Update `boot_device.mirror` examples to specify `boot_device.layout` From 530a65e75f30712eb19daab806ff72de27d62c1b Mon Sep 17 00:00:00 2001 From: Steven Presti Date: Wed, 28 Jan 2026 10:24:44 -0500 Subject: [PATCH 2/4] fcos v0_7_exp: add tests for partition constraint detection Add test cases for: - Root partition constrained by disk position (not partition number) - Root partition with auto-positioned following partition (StartMiB=0) - Root partition NOT constrained when next partition has explicit StartMiB - Root partition size validation (too small, exactly 8GiB) --- config/fcos/v1_7_exp/translate_test.go | 175 +++++++++++++++++++++++++ 1 file changed, 175 insertions(+) diff --git a/config/fcos/v1_7_exp/translate_test.go b/config/fcos/v1_7_exp/translate_test.go index 380a80eb5..ab9633d0a 100644 --- a/config/fcos/v1_7_exp/translate_test.go +++ b/config/fcos/v1_7_exp/translate_test.go @@ -1506,6 +1506,181 @@ func TestTranslateBootDevice(t *testing.T) { } } +// TestPartitionSorting tests that partitions are properly sorted by disk position, +// not by partition number, when detecting constraints. +func TestPartitionSorting(t *testing.T) { + tests := []struct { + name string + in Config + report report.Report + }{ + // Root partition with default size (0, fill available) followed by a partition + // with explicit StartMiB = 0 (auto-positioned). The next partition should be + // the one that comes AFTER root on disk. + { + name: "root partition constrained by disk position not partition number", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(0), // auto-positioned - will be placed after root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition followed by a partition with explicit StartMiB that would + // constrain root. + { + name: "root constrained by explicit StartMiB partition", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("var"), + StartMiB: util.IntToPtr(0), // auto-positioned - constrains root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition NOT constrained because next partition has explicit StartMiB + { + name: "root not constrained with explicit StartMiB after", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(10240), // explicit position - does NOT constrain root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + // Root partition too small (explicit size < 8192 MiB) + { + name: "root partition too small", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(4096), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootTooSmall.Error(), + Context: path.New("json", "storage", "disks", 0, "partitions", 0, "size_mib"), + }, + }, + }, + }, + // Root partition exactly 8192 MiB + { + name: "root partition exactly 8GiB", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(8192), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, translations, r := test.in.ToIgn3_6Unvalidated(common.TranslateOptions{}) + r = confutil.TranslateReportPaths(r, translations) + assert.Equal(t, test.report, r, "report mismatch") + }) + } +} + // TestTranslateGrub tests translating the Butane config Grub section. func TestTranslateGrub(t *testing.T) { // Some tests below have the same translations From 27b6e6069e50fd37f6c546a80df4e136cf00ed85 Mon Sep 17 00:00:00 2001 From: Steven Presti Date: Fri, 30 Jan 2026 15:39:10 -0500 Subject: [PATCH 3/4] fixup! fcos v0_7_exp: add warn on small or constrained root partition --- config/fcos/v1_3/translate.go | 68 +++++++++++++++++++++++++++++++---- config/fcos/v1_4/translate.go | 68 +++++++++++++++++++++++++++++++---- config/fcos/v1_5/translate.go | 68 +++++++++++++++++++++++++++++++---- config/fcos/v1_6/translate.go | 68 +++++++++++++++++++++++++++++++---- docs/release-notes.md | 4 +-- 5 files changed, 246 insertions(+), 30 deletions(-) diff --git a/config/fcos/v1_3/translate.go b/config/fcos/v1_3/translate.go index 748fb5e88..fec8d4fc8 100644 --- a/config/fcos/v1_3/translate.go +++ b/config/fcos/v1_3/translate.go @@ -70,16 +70,70 @@ func (c Config) ToIgn3_2Unvalidated(options common.TranslateOptions) (types.Conf } r.Merge(c.processBootDevice(&ret, &ts, options)) for i, disk := range ret.Storage.Disks { - // In the boot_device.mirror case, nothing specifies partition numbers - // so match existing partitions only when `wipeTable` is false - if !util.IsTrue(disk.WipeTable) { - for j, partition := range disk.Partitions { - // check for reserved partlabels - if partition.Label != nil { + for p, partition := range disk.Partitions { + // if the partition is not nil and is root + if partition.Label != nil { + if *partition.Label == "root" { + if partition.SizeMiB == nil || *partition.SizeMiB == 0 { + // Root is set to "fill available" (0 or unspecified) + // Check if it's constrained by the next partition on disk + // Sort partitions by disk position (StartMiB) to find the next partition + var rootStart int + if partition.StartMiB != nil { + rootStart = *partition.StartMiB + } + + // Find the partition that comes immediately after root on disk + var nextPartition *types.Partition + for idx, ap := range disk.Partitions { + // Skip the root partition itself + if idx == p { + continue + } + + var apStart int + if ap.StartMiB != nil { + apStart = *ap.StartMiB + } + + // Only consider partitions that start after root + // (or auto-positioned partitions which will be placed after root) + if apStart > rootStart || (apStart == 0 && rootStart >= 0) { + if nextPartition == nil { + nextPartition = &ap + } else { + var nextStart int + if nextPartition.StartMiB != nil { + nextStart = *nextPartition.StartMiB + } + // If this partition starts before the current "next", it's closer to root + if (apStart > 0 && apStart < nextStart) || (nextStart == 0 && apStart > 0) { + nextPartition = &ap + } + } + } + } + + // Check if the next partition constrains root + if nextPartition != nil { + if nextPartition.StartMiB == nil || *nextPartition.StartMiB == 0 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrRootConstrained) + } + } + } else if *partition.SizeMiB < 8192 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "size_mib"), common.ErrRootTooSmall) + } + } + + // In the boot_device.mirror case, nothing specifies partition numbers + // so match existing partitions only when `wipeTable` is false + if !util.IsTrue(disk.WipeTable) { + // check for reserved partlabels if (*partition.Label == "BIOS-BOOT" && partition.Number != 1) || (*partition.Label == "PowerPC-PReP-boot" && partition.Number != 1) || (*partition.Label == "EFI-SYSTEM" && partition.Number != 2) || (*partition.Label == "boot" && partition.Number != 3) || (*partition.Label == "root" && partition.Number != 4) { - r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", j, "label"), common.ErrWrongPartitionNumber) + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrWrongPartitionNumber) } } + } } } diff --git a/config/fcos/v1_4/translate.go b/config/fcos/v1_4/translate.go index 9723edaa0..b4dd7b805 100644 --- a/config/fcos/v1_4/translate.go +++ b/config/fcos/v1_4/translate.go @@ -70,16 +70,70 @@ func (c Config) ToIgn3_3Unvalidated(options common.TranslateOptions) (types.Conf } r.Merge(c.processBootDevice(&ret, &ts, options)) for i, disk := range ret.Storage.Disks { - // In the boot_device.mirror case, nothing specifies partition numbers - // so match existing partitions only when `wipeTable` is false - if !util.IsTrue(disk.WipeTable) { - for j, partition := range disk.Partitions { - // check for reserved partlabels - if partition.Label != nil { + for p, partition := range disk.Partitions { + // if the partition is not nil and is root + if partition.Label != nil { + if *partition.Label == "root" { + if partition.SizeMiB == nil || *partition.SizeMiB == 0 { + // Root is set to "fill available" (0 or unspecified) + // Check if it's constrained by the next partition on disk + // Sort partitions by disk position (StartMiB) to find the next partition + var rootStart int + if partition.StartMiB != nil { + rootStart = *partition.StartMiB + } + + // Find the partition that comes immediately after root on disk + var nextPartition *types.Partition + for idx, ap := range disk.Partitions { + // Skip the root partition itself + if idx == p { + continue + } + + var apStart int + if ap.StartMiB != nil { + apStart = *ap.StartMiB + } + + // Only consider partitions that start after root + // (or auto-positioned partitions which will be placed after root) + if apStart > rootStart || (apStart == 0 && rootStart >= 0) { + if nextPartition == nil { + nextPartition = &ap + } else { + var nextStart int + if nextPartition.StartMiB != nil { + nextStart = *nextPartition.StartMiB + } + // If this partition starts before the current "next", it's closer to root + if (apStart > 0 && apStart < nextStart) || (nextStart == 0 && apStart > 0) { + nextPartition = &ap + } + } + } + } + + // Check if the next partition constrains root + if nextPartition != nil { + if nextPartition.StartMiB == nil || *nextPartition.StartMiB == 0 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrRootConstrained) + } + } + } else if *partition.SizeMiB < 8192 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "size_mib"), common.ErrRootTooSmall) + } + } + + // In the boot_device.mirror case, nothing specifies partition numbers + // so match existing partitions only when `wipeTable` is false + if !util.IsTrue(disk.WipeTable) { + // check for reserved partlabels if (*partition.Label == "BIOS-BOOT" && partition.Number != 1) || (*partition.Label == "PowerPC-PReP-boot" && partition.Number != 1) || (*partition.Label == "EFI-SYSTEM" && partition.Number != 2) || (*partition.Label == "boot" && partition.Number != 3) || (*partition.Label == "root" && partition.Number != 4) { - r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", j, "label"), common.ErrWrongPartitionNumber) + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrWrongPartitionNumber) } } + } } } diff --git a/config/fcos/v1_5/translate.go b/config/fcos/v1_5/translate.go index 473a0db30..a6070c86e 100644 --- a/config/fcos/v1_5/translate.go +++ b/config/fcos/v1_5/translate.go @@ -71,16 +71,70 @@ func (c Config) ToIgn3_4Unvalidated(options common.TranslateOptions) (types.Conf } r.Merge(c.processBootDevice(&ret, &ts, options)) for i, disk := range ret.Storage.Disks { - // In the boot_device.mirror case, nothing specifies partition numbers - // so match existing partitions only when `wipeTable` is false - if !util.IsTrue(disk.WipeTable) { - for j, partition := range disk.Partitions { - // check for reserved partlabels - if partition.Label != nil { + for p, partition := range disk.Partitions { + // if the partition is not nil and is root + if partition.Label != nil { + if *partition.Label == "root" { + if partition.SizeMiB == nil || *partition.SizeMiB == 0 { + // Root is set to "fill available" (0 or unspecified) + // Check if it's constrained by the next partition on disk + // Sort partitions by disk position (StartMiB) to find the next partition + var rootStart int + if partition.StartMiB != nil { + rootStart = *partition.StartMiB + } + + // Find the partition that comes immediately after root on disk + var nextPartition *types.Partition + for idx, ap := range disk.Partitions { + // Skip the root partition itself + if idx == p { + continue + } + + var apStart int + if ap.StartMiB != nil { + apStart = *ap.StartMiB + } + + // Only consider partitions that start after root + // (or auto-positioned partitions which will be placed after root) + if apStart > rootStart || (apStart == 0 && rootStart >= 0) { + if nextPartition == nil { + nextPartition = &ap + } else { + var nextStart int + if nextPartition.StartMiB != nil { + nextStart = *nextPartition.StartMiB + } + // If this partition starts before the current "next", it's closer to root + if (apStart > 0 && apStart < nextStart) || (nextStart == 0 && apStart > 0) { + nextPartition = &ap + } + } + } + } + + // Check if the next partition constrains root + if nextPartition != nil { + if nextPartition.StartMiB == nil || *nextPartition.StartMiB == 0 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrRootConstrained) + } + } + } else if *partition.SizeMiB < 8192 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "size_mib"), common.ErrRootTooSmall) + } + } + + // In the boot_device.mirror case, nothing specifies partition numbers + // so match existing partitions only when `wipeTable` is false + if !util.IsTrue(disk.WipeTable) { + // check for reserved partlabels if (*partition.Label == "BIOS-BOOT" && partition.Number != 1) || (*partition.Label == "PowerPC-PReP-boot" && partition.Number != 1) || (*partition.Label == "EFI-SYSTEM" && partition.Number != 2) || (*partition.Label == "boot" && partition.Number != 3) || (*partition.Label == "root" && partition.Number != 4) { - r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", j, "label"), common.ErrWrongPartitionNumber) + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrWrongPartitionNumber) } } + } } } diff --git a/config/fcos/v1_6/translate.go b/config/fcos/v1_6/translate.go index 2d24cc912..ea670b832 100644 --- a/config/fcos/v1_6/translate.go +++ b/config/fcos/v1_6/translate.go @@ -71,16 +71,70 @@ func (c Config) ToIgn3_5Unvalidated(options common.TranslateOptions) (types.Conf } r.Merge(c.processBootDevice(&ret, &ts, options)) for i, disk := range ret.Storage.Disks { - // In the boot_device.mirror case, nothing specifies partition numbers - // so match existing partitions only when `wipeTable` is false - if !util.IsTrue(disk.WipeTable) { - for j, partition := range disk.Partitions { - // check for reserved partlabels - if partition.Label != nil { + for p, partition := range disk.Partitions { + // if the partition is not nil and is root + if partition.Label != nil { + if *partition.Label == "root" { + if partition.SizeMiB == nil || *partition.SizeMiB == 0 { + // Root is set to "fill available" (0 or unspecified) + // Check if it's constrained by the next partition on disk + // Sort partitions by disk position (StartMiB) to find the next partition + var rootStart int + if partition.StartMiB != nil { + rootStart = *partition.StartMiB + } + + // Find the partition that comes immediately after root on disk + var nextPartition *types.Partition + for idx, ap := range disk.Partitions { + // Skip the root partition itself + if idx == p { + continue + } + + var apStart int + if ap.StartMiB != nil { + apStart = *ap.StartMiB + } + + // Only consider partitions that start after root + // (or auto-positioned partitions which will be placed after root) + if apStart > rootStart || (apStart == 0 && rootStart >= 0) { + if nextPartition == nil { + nextPartition = &ap + } else { + var nextStart int + if nextPartition.StartMiB != nil { + nextStart = *nextPartition.StartMiB + } + // If this partition starts before the current "next", it's closer to root + if (apStart > 0 && apStart < nextStart) || (nextStart == 0 && apStart > 0) { + nextPartition = &ap + } + } + } + } + + // Check if the next partition constrains root + if nextPartition != nil { + if nextPartition.StartMiB == nil || *nextPartition.StartMiB == 0 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrRootConstrained) + } + } + } else if *partition.SizeMiB < 8192 { + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "size_mib"), common.ErrRootTooSmall) + } + } + + // In the boot_device.mirror case, nothing specifies partition numbers + // so match existing partitions only when `wipeTable` is false + if !util.IsTrue(disk.WipeTable) { + // check for reserved partlabels if (*partition.Label == "BIOS-BOOT" && partition.Number != 1) || (*partition.Label == "PowerPC-PReP-boot" && partition.Number != 1) || (*partition.Label == "EFI-SYSTEM" && partition.Number != 2) || (*partition.Label == "boot" && partition.Number != 3) || (*partition.Label == "root" && partition.Number != 4) { - r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", j, "label"), common.ErrWrongPartitionNumber) + r.AddOnWarn(path.New("json", "storage", "disks", i, "partitions", p, "label"), common.ErrWrongPartitionNumber) } } + } } } diff --git a/docs/release-notes.md b/docs/release-notes.md index 9b4db79e9..53b442e8a 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -14,8 +14,8 @@ nav_order: 9 ### Misc. changes -- Warn on root partition size is too small _(fcos 1.7.0-exp)_ -- Warn on root partition constrained by another partition _(fcos 1.7.0-exp)_ +- Warn on root partition size is too small _(fcos 1.3.0-1.7.0-exp)_ +- Warn on root partition constrained by another partition _(fcos 1.3.0-1.7.0-exp)_ ### Docs changes From 7aa07070b1c9310c23064f80d9729c8032cdcd62 Mon Sep 17 00:00:00 2001 From: Steven Presti Date: Fri, 30 Jan 2026 16:38:17 -0500 Subject: [PATCH 4/4] fixup! fcos v0_7_exp: add tests for partition constraint detection --- config/fcos/v1_3/translate_test.go | 175 +++++++++++++++++++++++++++++ config/fcos/v1_4/translate_test.go | 175 +++++++++++++++++++++++++++++ config/fcos/v1_5/translate_test.go | 175 +++++++++++++++++++++++++++++ config/fcos/v1_6/translate_test.go | 175 +++++++++++++++++++++++++++++ 4 files changed, 700 insertions(+) diff --git a/config/fcos/v1_3/translate_test.go b/config/fcos/v1_3/translate_test.go index 22dec8c89..63dcb7bcc 100644 --- a/config/fcos/v1_3/translate_test.go +++ b/config/fcos/v1_3/translate_test.go @@ -1423,3 +1423,178 @@ func TestTranslateBootDevice(t *testing.T) { }) } } + +// TestPartitionSorting tests that partitions are properly sorted by disk position, +// not by partition number, when detecting constraints. +func TestPartitionSorting(t *testing.T) { + tests := []struct { + name string + in Config + report report.Report + }{ + // Root partition with default size (0, fill available) followed by a partition + // with explicit StartMiB = 0 (auto-positioned). The next partition should be + // the one that comes AFTER root on disk. + { + name: "root partition constrained by disk position not partition number", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(0), // auto-positioned - will be placed after root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition followed by a partition with explicit StartMiB that would + // constrain root. + { + name: "root constrained by explicit StartMiB partition", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("var"), + StartMiB: util.IntToPtr(0), // auto-positioned - constrains root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition NOT constrained because next partition has explicit StartMiB + { + name: "root not constrained with explicit StartMiB after", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(10240), // explicit position - does NOT constrain root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + // Root partition too small (explicit size < 8192 MiB) + { + name: "root partition too small", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(4096), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootTooSmall.Error(), + Context: path.New("json", "storage", "disks", 0, "partitions", 0, "size_mib"), + }, + }, + }, + }, + // Root partition exactly 8192 MiB + { + name: "root partition exactly 8GiB", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(8192), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, translations, r := test.in.ToIgn3_2Unvalidated(common.TranslateOptions{}) + r = confutil.TranslateReportPaths(r, translations) + assert.Equal(t, test.report, r, "report mismatch") + }) + } +} diff --git a/config/fcos/v1_4/translate_test.go b/config/fcos/v1_4/translate_test.go index 3640c900d..21baa717d 100644 --- a/config/fcos/v1_4/translate_test.go +++ b/config/fcos/v1_4/translate_test.go @@ -1423,3 +1423,178 @@ func TestTranslateBootDevice(t *testing.T) { }) } } + +// TestPartitionSorting tests that partitions are properly sorted by disk position, +// not by partition number, when detecting constraints. +func TestPartitionSorting(t *testing.T) { + tests := []struct { + name string + in Config + report report.Report + }{ + // Root partition with default size (0, fill available) followed by a partition + // with explicit StartMiB = 0 (auto-positioned). The next partition should be + // the one that comes AFTER root on disk. + { + name: "root partition constrained by disk position not partition number", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(0), // auto-positioned - will be placed after root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition followed by a partition with explicit StartMiB that would + // constrain root. + { + name: "root constrained by explicit StartMiB partition", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("var"), + StartMiB: util.IntToPtr(0), // auto-positioned - constrains root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition NOT constrained because next partition has explicit StartMiB + { + name: "root not constrained with explicit StartMiB after", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(10240), // explicit position - does NOT constrain root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + // Root partition too small (explicit size < 8192 MiB) + { + name: "root partition too small", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(4096), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootTooSmall.Error(), + Context: path.New("json", "storage", "disks", 0, "partitions", 0, "size_mib"), + }, + }, + }, + }, + // Root partition exactly 8192 MiB + { + name: "root partition exactly 8GiB", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(8192), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, translations, r := test.in.ToIgn3_3Unvalidated(common.TranslateOptions{}) + r = confutil.TranslateReportPaths(r, translations) + assert.Equal(t, test.report, r, "report mismatch") + }) + } +} diff --git a/config/fcos/v1_5/translate_test.go b/config/fcos/v1_5/translate_test.go index 78c2e7a1f..1165ed7c2 100644 --- a/config/fcos/v1_5/translate_test.go +++ b/config/fcos/v1_5/translate_test.go @@ -1506,6 +1506,181 @@ func TestTranslateBootDevice(t *testing.T) { } } +// TestPartitionSorting tests that partitions are properly sorted by disk position, +// not by partition number, when detecting constraints. +func TestPartitionSorting(t *testing.T) { + tests := []struct { + name string + in Config + report report.Report + }{ + // Root partition with default size (0, fill available) followed by a partition + // with explicit StartMiB = 0 (auto-positioned). The next partition should be + // the one that comes AFTER root on disk. + { + name: "root partition constrained by disk position not partition number", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(0), // auto-positioned - will be placed after root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition followed by a partition with explicit StartMiB that would + // constrain root. + { + name: "root constrained by explicit StartMiB partition", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("var"), + StartMiB: util.IntToPtr(0), // auto-positioned - constrains root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition NOT constrained because next partition has explicit StartMiB + { + name: "root not constrained with explicit StartMiB after", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(10240), // explicit position - does NOT constrain root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + // Root partition too small (explicit size < 8192 MiB) + { + name: "root partition too small", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(4096), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootTooSmall.Error(), + Context: path.New("json", "storage", "disks", 0, "partitions", 0, "size_mib"), + }, + }, + }, + }, + // Root partition exactly 8192 MiB + { + name: "root partition exactly 8GiB", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(8192), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, translations, r := test.in.ToIgn3_4Unvalidated(common.TranslateOptions{}) + r = confutil.TranslateReportPaths(r, translations) + assert.Equal(t, test.report, r, "report mismatch") + }) + } +} + // TestTranslateGrub tests translating the Butane config Grub section. func TestTranslateGrub(t *testing.T) { // Some tests below have the same translations diff --git a/config/fcos/v1_6/translate_test.go b/config/fcos/v1_6/translate_test.go index 99c30253b..8281d0169 100644 --- a/config/fcos/v1_6/translate_test.go +++ b/config/fcos/v1_6/translate_test.go @@ -1506,6 +1506,181 @@ func TestTranslateBootDevice(t *testing.T) { } } +// TestPartitionSorting tests that partitions are properly sorted by disk position, +// not by partition number, when detecting constraints. +func TestPartitionSorting(t *testing.T) { + tests := []struct { + name string + in Config + report report.Report + }{ + // Root partition with default size (0, fill available) followed by a partition + // with explicit StartMiB = 0 (auto-positioned). The next partition should be + // the one that comes AFTER root on disk. + { + name: "root partition constrained by disk position not partition number", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(0), // auto-positioned - will be placed after root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition followed by a partition with explicit StartMiB that would + // constrain root. + { + name: "root constrained by explicit StartMiB partition", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("var"), + StartMiB: util.IntToPtr(0), // auto-positioned - constrains root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootConstrained.Error(), + Context: path.New("yaml", "storage", "disks", 0, "partitions", 0, "label"), + }, + }, + }, + }, + // Root partition NOT constrained because next partition has explicit StartMiB + { + name: "root not constrained with explicit StartMiB after", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(0), // fill available + StartMiB: util.IntToPtr(2048), + }, + { + Label: util.StrToPtr("data"), + StartMiB: util.IntToPtr(10240), // explicit position - does NOT constrain root + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + // Root partition too small (explicit size < 8192 MiB) + { + name: "root partition too small", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(4096), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{ + Entries: []report.Entry{ + { + Kind: report.Warn, + Message: common.ErrRootTooSmall.Error(), + Context: path.New("json", "storage", "disks", 0, "partitions", 0, "size_mib"), + }, + }, + }, + }, + // Root partition exactly 8192 MiB + { + name: "root partition exactly 8GiB", + in: Config{ + Config: base.Config{ + Storage: base.Storage{ + Disks: []base.Disk{ + { + Device: "/dev/vda", + Partitions: []base.Partition{ + { + Label: util.StrToPtr("root"), + Number: 4, + SizeMiB: util.IntToPtr(8192), + }, + }, + }, + }, + }, + }, + }, + report: report.Report{}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, translations, r := test.in.ToIgn3_5Unvalidated(common.TranslateOptions{}) + r = confutil.TranslateReportPaths(r, translations) + assert.Equal(t, test.report, r, "report mismatch") + }) + } +} + // TestTranslateGrub tests translating the Butane config Grub section. func TestTranslateGrub(t *testing.T) { // Some tests below have the same translations