Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions config/common/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
68 changes: 61 additions & 7 deletions config/fcos/v1_3/translate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

}
}
}
Expand Down
175 changes: 175 additions & 0 deletions config/fcos/v1_3/translate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
}
}
68 changes: 61 additions & 7 deletions config/fcos/v1_4/translate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

}
}
}
Expand Down
Loading
Loading