Merge pull request #2234 from chewi/partx-race

prestist · web-flow · commit 9d28e049d173 · 2026-07-10T12:56:24.000-04:00
Fix disk partitioning race condition and using partition number 0
diff --git a/config/shared/errors/errors.go b/config/shared/errors/errors.go
@@ -85,7 +85,6 @@ var (
 	ErrUnrecognizedRaidLevel            = errors.New("unrecognized raid level")
 	ErrRaidDevicesRequired              = errors.New("raid devices required")
 	ErrShouldNotExistWithOthers         = errors.New("shouldExist specified false with other options also specified")
-	ErrZeroesWithShouldNotExist         = errors.New("shouldExist is false for a partition and other partition(s) has start or size 0")
 	ErrNeedLabelOrNumber                = errors.New("a partition number >= 1 or a label must be specified")
 	ErrDuplicateLabels                  = errors.New("cannot use the same partition label twice")
 	ErrInvalidProxy                     = errors.New("proxies must be http(s)")
diff --git a/config/v3_0/types/disk.go b/config/v3_0/types/disk.go
@@ -16,7 +16,6 @@ package types
 
 import (
 	"github.com/coreos/ignition/v2/config/shared/errors"
-	"github.com/coreos/ignition/v2/config/util"
 
 	"github.com/coreos/vcontext/path"
 	"github.com/coreos/vcontext/report"
@@ -39,9 +38,6 @@ func (n Disk) Validate(c path.ContextPath) (r report.Report) {
 	if overlaps, p := n.partitionsOverlap(); overlaps {
 		r.AddOnError(c.Append("partitions", p), errors.ErrPartitionsOverlap)
 	}
-	if n.partitionsMixZeroesAndNonexistence() {
-		r.AddOnError(c.Append("partitions"), errors.ErrZeroesWithShouldNotExist)
-	}
 	if collides, p := n.partitionLabelsCollide(); collides {
 		r.AddOnError(c.Append("partitions", p), errors.ErrDuplicateLabels)
 	}
@@ -123,13 +119,3 @@ func (n Disk) partitionsOverlap() (bool, int) {
 	}
 	return false, 0
 }
-
-func (n Disk) partitionsMixZeroesAndNonexistence() bool {
-	hasZero := false
-	hasShouldNotExist := false
-	for _, p := range n.Partitions {
-		hasShouldNotExist = hasShouldNotExist || util.IsFalse(p.ShouldExist)
-		hasZero = hasZero || (p.Number == 0)
-	}
-	return hasZero && hasShouldNotExist
-}
diff --git a/config/v3_1/types/disk.go b/config/v3_1/types/disk.go
@@ -16,7 +16,6 @@ package types
 
 import (
 	"github.com/coreos/ignition/v2/config/shared/errors"
-	"github.com/coreos/ignition/v2/config/util"
 
 	"github.com/coreos/vcontext/path"
 	"github.com/coreos/vcontext/report"
@@ -39,9 +38,6 @@ func (n Disk) Validate(c path.ContextPath) (r report.Report) {
 	if overlaps, p := n.partitionsOverlap(); overlaps {
 		r.AddOnError(c.Append("partitions", p), errors.ErrPartitionsOverlap)
 	}
-	if n.partitionsMixZeroesAndNonexistence() {
-		r.AddOnError(c.Append("partitions"), errors.ErrZeroesWithShouldNotExist)
-	}
 	if collides, p := n.partitionLabelsCollide(); collides {
 		r.AddOnError(c.Append("partitions", p), errors.ErrDuplicateLabels)
 	}
@@ -123,13 +119,3 @@ func (n Disk) partitionsOverlap() (bool, int) {
 	}
 	return false, 0
 }
-
-func (n Disk) partitionsMixZeroesAndNonexistence() bool {
-	hasZero := false
-	hasShouldNotExist := false
-	for _, p := range n.Partitions {
-		hasShouldNotExist = hasShouldNotExist || util.IsFalse(p.ShouldExist)
-		hasZero = hasZero || (p.Number == 0)
-	}
-	return hasZero && hasShouldNotExist
-}
diff --git a/config/v3_2/types/disk.go b/config/v3_2/types/disk.go
@@ -16,7 +16,6 @@ package types
 
 import (
 	"github.com/coreos/ignition/v2/config/shared/errors"
-	"github.com/coreos/ignition/v2/config/util"
 
 	"github.com/coreos/vcontext/path"
 	"github.com/coreos/vcontext/report"
@@ -39,9 +38,6 @@ func (n Disk) Validate(c path.ContextPath) (r report.Report) {
 	if overlaps, p := n.partitionsOverlap(); overlaps {
 		r.AddOnError(c.Append("partitions", p), errors.ErrPartitionsOverlap)
 	}
-	if n.partitionsMixZeroesAndNonexistence() {
-		r.AddOnError(c.Append("partitions"), errors.ErrZeroesWithShouldNotExist)
-	}
 	if collides, p := n.partitionLabelsCollide(); collides {
 		r.AddOnError(c.Append("partitions", p), errors.ErrDuplicateLabels)
 	}
@@ -123,13 +119,3 @@ func (n Disk) partitionsOverlap() (bool, int) {
 	}
 	return false, 0
 }
-
-func (n Disk) partitionsMixZeroesAndNonexistence() bool {
-	hasZero := false
-	hasShouldNotExist := false
-	for _, p := range n.Partitions {
-		hasShouldNotExist = hasShouldNotExist || util.IsFalse(p.ShouldExist)
-		hasZero = hasZero || (p.Number == 0)
-	}
-	return hasZero && hasShouldNotExist
-}
diff --git a/config/v3_3/types/disk.go b/config/v3_3/types/disk.go
@@ -16,7 +16,6 @@ package types
 
 import (
 	"github.com/coreos/ignition/v2/config/shared/errors"
-	"github.com/coreos/ignition/v2/config/util"
 
 	"github.com/coreos/vcontext/path"
 	"github.com/coreos/vcontext/report"
@@ -39,9 +38,6 @@ func (n Disk) Validate(c path.ContextPath) (r report.Report) {
 	if overlaps, p := n.partitionsOverlap(); overlaps {
 		r.AddOnError(c.Append("partitions", p), errors.ErrPartitionsOverlap)
 	}
-	if n.partitionsMixZeroesAndNonexistence() {
-		r.AddOnError(c.Append("partitions"), errors.ErrZeroesWithShouldNotExist)
-	}
 	if collides, p := n.partitionLabelsCollide(); collides {
 		r.AddOnError(c.Append("partitions", p), errors.ErrDuplicateLabels)
 	}
@@ -123,13 +119,3 @@ func (n Disk) partitionsOverlap() (bool, int) {
 	}
 	return false, 0
 }
-
-func (n Disk) partitionsMixZeroesAndNonexistence() bool {
-	hasZero := false
-	hasShouldNotExist := false
-	for _, p := range n.Partitions {
-		hasShouldNotExist = hasShouldNotExist || util.IsFalse(p.ShouldExist)
-		hasZero = hasZero || (p.Number == 0)
-	}
-	return hasZero && hasShouldNotExist
-}
diff --git a/config/v3_4/types/disk.go b/config/v3_4/types/disk.go
@@ -16,7 +16,6 @@ package types
 
 import (
 	"github.com/coreos/ignition/v2/config/shared/errors"
-	"github.com/coreos/ignition/v2/config/util"
 
 	"github.com/coreos/vcontext/path"
 	"github.com/coreos/vcontext/report"
@@ -39,9 +38,6 @@ func (n Disk) Validate(c path.ContextPath) (r report.Report) {
 	if overlaps, p := n.partitionsOverlap(); overlaps {
 		r.AddOnError(c.Append("partitions", p), errors.ErrPartitionsOverlap)
 	}
-	if n.partitionsMixZeroesAndNonexistence() {
-		r.AddOnError(c.Append("partitions"), errors.ErrZeroesWithShouldNotExist)
-	}
 	if collides, p := n.partitionLabelsCollide(); collides {
 		r.AddOnError(c.Append("partitions", p), errors.ErrDuplicateLabels)
 	}
@@ -123,13 +119,3 @@ func (n Disk) partitionsOverlap() (bool, int) {
 	}
 	return false, 0
 }
-
-func (n Disk) partitionsMixZeroesAndNonexistence() bool {
-	hasZero := false
-	hasShouldNotExist := false
-	for _, p := range n.Partitions {
-		hasShouldNotExist = hasShouldNotExist || util.IsFalse(p.ShouldExist)
-		hasZero = hasZero || (p.Number == 0)
-	}
-	return hasZero && hasShouldNotExist
-}
diff --git a/config/v3_5/types/disk.go b/config/v3_5/types/disk.go
@@ -16,7 +16,6 @@ package types
 
 import (
 	"github.com/coreos/ignition/v2/config/shared/errors"
-	"github.com/coreos/ignition/v2/config/util"
 
 	"github.com/coreos/vcontext/path"
 	"github.com/coreos/vcontext/report"
@@ -39,9 +38,6 @@ func (n Disk) Validate(c path.ContextPath) (r report.Report) {
 	if overlaps, p := n.partitionsOverlap(); overlaps {
 		r.AddOnError(c.Append("partitions", p), errors.ErrPartitionsOverlap)
 	}
-	if n.partitionsMixZeroesAndNonexistence() {
-		r.AddOnError(c.Append("partitions"), errors.ErrZeroesWithShouldNotExist)
-	}
 	if collides, p := n.partitionLabelsCollide(); collides {
 		r.AddOnError(c.Append("partitions", p), errors.ErrDuplicateLabels)
 	}
@@ -123,13 +119,3 @@ func (n Disk) partitionsOverlap() (bool, int) {
 	}
 	return false, 0
 }
-
-func (n Disk) partitionsMixZeroesAndNonexistence() bool {
-	hasZero := false
-	hasShouldNotExist := false
-	for _, p := range n.Partitions {
-		hasShouldNotExist = hasShouldNotExist || util.IsFalse(p.ShouldExist)
-		hasZero = hasZero || (p.Number == 0)
-	}
-	return hasZero && hasShouldNotExist
-}
diff --git a/config/v3_6/types/disk.go b/config/v3_6/types/disk.go
@@ -16,7 +16,6 @@ package types
 
 import (
 	"github.com/coreos/ignition/v2/config/shared/errors"
-	"github.com/coreos/ignition/v2/config/util"
 
 	"github.com/coreos/vcontext/path"
 	"github.com/coreos/vcontext/report"
@@ -39,9 +38,6 @@ func (n Disk) Validate(c path.ContextPath) (r report.Report) {
 	if overlaps, p := n.partitionsOverlap(); overlaps {
 		r.AddOnError(c.Append("partitions", p), errors.ErrPartitionsOverlap)
 	}
-	if n.partitionsMixZeroesAndNonexistence() {
-		r.AddOnError(c.Append("partitions"), errors.ErrZeroesWithShouldNotExist)
-	}
 	if collides, p := n.partitionLabelsCollide(); collides {
 		r.AddOnError(c.Append("partitions", p), errors.ErrDuplicateLabels)
 	}
@@ -123,13 +119,3 @@ func (n Disk) partitionsOverlap() (bool, int) {
 	}
 	return false, 0
 }
-
-func (n Disk) partitionsMixZeroesAndNonexistence() bool {
-	hasZero := false
-	hasShouldNotExist := false
-	for _, p := range n.Partitions {
-		hasShouldNotExist = hasShouldNotExist || util.IsFalse(p.ShouldExist)
-		hasZero = hasZero || (p.Number == 0)
-	}
-	return hasZero && hasShouldNotExist
-}
diff --git a/config/v3_7_experimental/types/disk.go b/config/v3_7_experimental/types/disk.go
@@ -16,7 +16,6 @@ package types
 
 import (
 	"github.com/coreos/ignition/v2/config/shared/errors"
-	"github.com/coreos/ignition/v2/config/util"
 
 	"github.com/coreos/vcontext/path"
 	"github.com/coreos/vcontext/report"
@@ -39,9 +38,6 @@ func (n Disk) Validate(c path.ContextPath) (r report.Report) {
 	if overlaps, p := n.partitionsOverlap(); overlaps {
 		r.AddOnError(c.Append("partitions", p), errors.ErrPartitionsOverlap)
 	}
-	if n.partitionsMixZeroesAndNonexistence() {
-		r.AddOnError(c.Append("partitions"), errors.ErrZeroesWithShouldNotExist)
-	}
 	if collides, p := n.partitionLabelsCollide(); collides {
 		r.AddOnError(c.Append("partitions", p), errors.ErrDuplicateLabels)
 	}
@@ -123,13 +119,3 @@ func (n Disk) partitionsOverlap() (bool, int) {
 	}
 	return false, 0
 }
-
-func (n Disk) partitionsMixZeroesAndNonexistence() bool {
-	hasZero := false
-	hasShouldNotExist := false
-	for _, p := range n.Partitions {
-		hasShouldNotExist = hasShouldNotExist || util.IsFalse(p.ShouldExist)
-		hasZero = hasZero || (p.Number == 0)
-	}
-	return hasZero && hasShouldNotExist
-}
diff --git a/docs/release-notes.md b/docs/release-notes.md
@@ -15,6 +15,7 @@ Starting with this release, ignition-validate binaries are signed with the
 
 - Support reading configs from `/run/ignition` and `/etc/ignition/` in addition to `/usr/lib/ignition/`, searched in descending priority order ([#2221](https://github.com/coreos/ignition/pull/2221))
 - Add support for `virtiofs`
+- Allow deleting a disk partition while creating another partition with number 0. ([#2234](https://github.com/coreos/ignition/pull/2234))
 
 ### Changes
 
@@ -24,6 +25,9 @@ Starting with this release, ignition-validate binaries are signed with the
 
 ### Bug fixes
 
+- Fix giving disk partition number 0 to get the next available slot. This caused the disks stage to fail since version 2.20.0. ([#2234](https://github.com/coreos/ignition/pull/2234))
+- Fix disk partitioning race condition where the kernel is already aware of the changes before running `partx`, causing a fatal error. ([#2234](https://github.com/coreos/ignition/pull/2234))
+
 
 ## Ignition 2.26.0 (2026-02-17)
 
diff --git a/internal/distro/distro.go b/internal/distro/distro.go
@@ -38,6 +38,7 @@ var (
 	groupaddCmd  = "groupadd"
 	groupmodCmd  = "groupmod"
 	groupdelCmd  = "groupdel"
+	dmsetupCmd   = "dmsetup"
 	mdadmCmd     = "mdadm"
 	mountCmd     = "mount"
 	partxCmd     = "partx"
@@ -109,6 +110,7 @@ func SystemConfigDirs() []string {
 func GroupaddCmd() string  { return groupaddCmd }
 func GroupmodCmd() string  { return groupmodCmd }
 func GroupdelCmd() string  { return groupdelCmd }
+func DmsetupCmd() string   { return dmsetupCmd }
 func MdadmCmd() string     { return mdadmCmd }
 func MountCmd() string     { return mountCmd }
 func PartxCmd() string     { return partxCmd }
diff --git a/internal/exec/stages/disks/partitions.go b/internal/exec/stages/disks/partitions.go
diff --git a/internal/exec/stages/disks/partitions_test.go b/internal/exec/stages/disks/partitions_test.go
diff --git a/tests/positive/partitions/complex-mb.go b/tests/positive/partitions/complex-mb.go
