Partition mode

cmd/osbuild-playground/my-image.go

@@ -39,7 +39,7 @@ func (img *MyImage) InstantiateManifest(m *manifest.Manifest,
 	}
 
 	// TODO: add helper
-	pt, err := disk.NewPartitionTable(&basePT, nil, 0, false, nil, rng)
+	pt, err := disk.NewPartitionTable(&basePT, nil, 0, disk.RawPartitioningMode, nil, rng)
 	if err != nil {
 		panic(err)
 	}

pkg/disk/disk_test.go

@@ -6,8 +6,10 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/osbuild/images/pkg/blueprint"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/osbuild/images/pkg/blueprint"
 )
 
 const (
@@ -74,7 +76,7 @@ func TestDisk_DynamicallyResizePartitionTable(t *testing.T) {
 	// math/rand is good enough in this case
 	/* #nosec G404 */
 	rng := rand.New(rand.NewSource(0))
-	newpt, err := NewPartitionTable(&pt, mountpoints, 1024, false, nil, rng)
+	newpt, err := NewPartitionTable(&pt, mountpoints, 1024, RawPartitioningMode, nil, rng)
 	assert.NoError(t, err)
 	assert.GreaterOrEqual(t, newpt.Size, expectedSize)
 }
@@ -449,8 +451,12 @@ func TestCreatePartitionTable(t *testing.T) {
 	for ptName := range testPartitionTables {
 		pt := testPartitionTables[ptName]
 		for bpName, bp := range testBlueprints {
-			mpt, err := NewPartitionTable(&pt, bp, uint64(13*MiB), false, nil, rng)
-			assert.NoError(err, "Partition table generation failed: PT %q BP %q (%s)", ptName, bpName, err)
+			ptMode := RawPartitioningMode
+			if ptName == "luks+lvm" {
+				ptMode = AutoLVMPartitioningMode
+			}
+			mpt, err := NewPartitionTable(&pt, bp, uint64(13*MiB), ptMode, nil, rng)
+			require.NoError(t, err, "Partition table generation failed: PT %q BP %q (%s)", ptName, bpName, err)
 			assert.NotNil(mpt, "Partition table generation failed: PT %q BP %q (nil partition table)", ptName, bpName)
 			assert.Greater(mpt.GetSize(), sumSizes(bp))
 
@@ -474,13 +480,12 @@ func TestCreatePartitionTableLVMify(t *testing.T) {
 			pt := testPartitionTables[ptName]
 
 			if tbp != nil && (ptName == "btrfs" || ptName == "luks") {
-				assert.Panics(func() {
-					_, _ = NewPartitionTable(&pt, tbp, uint64(13*MiB), true, nil, rng)
-				}, fmt.Sprintf("PT %q BP %q: should panic", ptName, bpName))
+				_, err := NewPartitionTable(&pt, tbp, uint64(13*MiB), AutoLVMPartitioningMode, nil, rng)
+				assert.Error(err, "PT %q BP %q: should return an error with LVMPartitioningMode", ptName, bpName)
 				continue
 			}
 
-			mpt, err := NewPartitionTable(&pt, tbp, uint64(13*MiB), true, nil, rng)
+			mpt, err := NewPartitionTable(&pt, tbp, uint64(13*MiB), AutoLVMPartitioningMode, nil, rng)
 			assert.NoError(err, "PT %q BP %q: Partition table generation failed: (%s)", ptName, bpName, err)
 
 			rootPath := entityPath(mpt, "/")
@@ -502,6 +507,42 @@ func TestCreatePartitionTableLVMify(t *testing.T) {
 	}
 }
 
+func TestCreatePartitionTableLVMOnly(t *testing.T) {
+	assert := assert.New(t)
+	// math/rand is good enough in this case
+	/* #nosec G404 */
+	rng := rand.New(rand.NewSource(13))
+	for bpName, tbp := range testBlueprints {
+		for ptName := range testPartitionTables {
+			pt := testPartitionTables[ptName]
+
+			if ptName == "btrfs" || ptName == "luks" {
+				_, err := NewPartitionTable(&pt, tbp, uint64(13*MiB), LVMPartitioningMode, nil, rng)
+				assert.Error(err, "PT %q BP %q: should return an error with LVMPartitioningMode", ptName, bpName)
+				continue
+			}
+
+			mpt, err := NewPartitionTable(&pt, tbp, uint64(13*MiB), LVMPartitioningMode, nil, rng)
+			require.NoError(t, err, "PT %q BP %q: Partition table generation failed: (%s)", ptName, bpName, err)
+
+			rootPath := entityPath(mpt, "/")
+			if rootPath == nil {
+				panic(fmt.Sprintf("PT %q BP %q: no root mountpoint", ptName, bpName))
+			}
+
+			bootPath := entityPath(mpt, "/boot")
+			if tbp != nil && bootPath == nil {
+				panic(fmt.Sprintf("PT %q BP %q: no boot mountpoint", ptName, bpName))
+			}
+
+			// root should always be on a LVM
+			parent := rootPath[1]
+			_, ok := parent.(*LVMLogicalVolume)
+			assert.True(ok, "PT %q BP %q: root's parent (%q) is not an LVM logical volume", ptName, bpName, parent)
+		}
+	}
+}
+
 func TestMinimumSizes(t *testing.T) {
 	assert := assert.New(t)
 
@@ -588,7 +629,7 @@ func TestMinimumSizes(t *testing.T) {
 
 	for idx, tc := range testCases {
 		{ // without LVM
-			mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), false, nil, rng)
+			mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), RawPartitioningMode, nil, rng)
 			assert.NoError(err)
 			for mnt, minSize := range tc.ExpectedMinSizes {
 				path := entityPath(mpt, mnt)
@@ -602,7 +643,7 @@ func TestMinimumSizes(t *testing.T) {
 		}
 
 		{ // with LVM
-			mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), true, nil, rng)
+			mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), AutoLVMPartitioningMode, nil, rng)
 			assert.NoError(err)
 			for mnt, minSize := range tc.ExpectedMinSizes {
 				path := entityPath(mpt, mnt)
@@ -689,7 +730,7 @@ func TestLVMExtentAlignment(t *testing.T) {
 	}
 
 	for idx, tc := range testCases {
-		mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), true, nil, rng)
+		mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), AutoLVMPartitioningMode, nil, rng)
 		assert.NoError(err)
 		for mnt, expSize := range tc.ExpectedSizes {
 			path := entityPath(mpt, mnt)
@@ -718,7 +759,7 @@ func TestNewBootWithSizeLVMify(t *testing.T) {
 		},
 	}
 
-	mpt, err := NewPartitionTable(&pt, custom, uint64(3*GiB), true, nil, rng)
+	mpt, err := NewPartitionTable(&pt, custom, uint64(3*GiB), AutoLVMPartitioningMode, nil, rng)
 	assert.NoError(err)
 
 	for idx, c := range custom {
@@ -1058,7 +1099,7 @@ func TestMinimumSizesWithRequiredSizes(t *testing.T) {
 
 	for idx, tc := range testCases {
 		{ // without LVM
-			mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), false, map[string]uint64{"/": 1 * GiB, "/usr": 3 * GiB}, rng)
+			mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), RawPartitioningMode, map[string]uint64{"/": 1 * GiB, "/usr": 3 * GiB}, rng)
 			assert.NoError(err)
 			for mnt, minSize := range tc.ExpectedMinSizes {
 				path := entityPath(mpt, mnt)
@@ -1072,7 +1113,7 @@ func TestMinimumSizesWithRequiredSizes(t *testing.T) {
 		}
 
 		{ // with LVM
-			mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), true, map[string]uint64{"/": 1 * GiB, "/usr": 3 * GiB}, rng)
+			mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), AutoLVMPartitioningMode, map[string]uint64{"/": 1 * GiB, "/usr": 3 * GiB}, rng)
 			assert.NoError(err)
 			for mnt, minSize := range tc.ExpectedMinSizes {
 				path := entityPath(mpt, mnt)

pkg/disk/partition_table.go

@@ -6,6 +6,7 @@ import (
 	"path/filepath"
 
 	"github.com/google/uuid"
+
 	"github.com/osbuild/images/pkg/blueprint"
 )
 
@@ -20,14 +21,46 @@ type PartitionTable struct {
 	StartOffset  uint64 // Starting offset of the first partition in the table (Mb)
 }
 
-func NewPartitionTable(basePT *PartitionTable, mountpoints []blueprint.FilesystemCustomization, imageSize uint64, lvmify bool, requiredSizes map[string]uint64, rng *rand.Rand) (*PartitionTable, error) {
+type PartitioningMode string
+
+const (
+	// AutoLVMPartitioningMode creates a LVM layout if the filesystem
+	// contains a mountpoint that's not defined in the base partition table
+	// of the specified image type. In the other case, a raw layout is used.
+	AutoLVMPartitioningMode PartitioningMode = "auto-lvm"
+
+	// LVMPartitioningMode always creates an LVM layout.
+	LVMPartitioningMode PartitioningMode = "lvm"
+
+	// RawPartitioningMode always creates a raw layout.
+	RawPartitioningMode PartitioningMode = "raw"
+
+	// DefaultPartitioningMode is AutoLVMPartitioningMode and is the empty state
+	DefaultPartitioningMode PartitioningMode = ""
+)
+
+func NewPartitionTable(basePT *PartitionTable, mountpoints []blueprint.FilesystemCustomization, imageSize uint64, mode PartitioningMode, requiredSizes map[string]uint64, rng *rand.Rand) (*PartitionTable, error) {
 	newPT := basePT.Clone().(*PartitionTable)
 
+	if basePT.features().LVM && mode == RawPartitioningMode {
+		return nil, fmt.Errorf("raw partitioning mode set for a base partition table with LVM, this is unsupported")
+	}
+
 	// first pass: enlarge existing mountpoints and collect new ones
 	newMountpoints, _ := newPT.applyCustomization(mountpoints, false)
 
-	// if there is any new mountpoint and lvmify is enabled, ensure we have LVM layout
-	if lvmify && len(newMountpoints) > 0 {
+	var ensureLVM bool
+	switch mode {
+	case LVMPartitioningMode:
+		ensureLVM = true
+	case RawPartitioningMode:
+		ensureLVM = false
+	case DefaultPartitioningMode, AutoLVMPartitioningMode:
+		ensureLVM = len(newMountpoints) > 0
+	default:
+		return nil, fmt.Errorf("unsupported partitioning mode %q", mode)
+	}
+	if ensureLVM {
 		err := newPT.ensureLVM()
 		if err != nil {
 			return nil, err
@@ -626,63 +659,75 @@ func (pt *PartitionTable) ensureLVM() error {
 		}
 
 	} else {
-		panic("unsupported parent for LVM")
+		return fmt.Errorf("Unsupported parent for LVM")
 	}
 
 	return nil
 }
 
-func (pt *PartitionTable) GetBuildPackages() []string {
-	packages := []string{}
+type partitionTableFeatures struct {
+	LVM   bool
+	Btrfs bool
+	XFS   bool
+	FAT   bool
+	EXT4  bool
+	LUKS  bool
+}
 
-	hasLVM := false
-	hasBtrfs := false
-	hasXFS := false
-	hasFAT := false
-	hasEXT4 := false
-	hasLUKS := false
+// features examines all of the PartitionTable entities
+// and returns a struct with flags set for each feature used
+func (pt *PartitionTable) features() partitionTableFeatures {
+	var ptFeatures partitionTableFeatures
 
 	introspectPT := func(e Entity, path []Entity) error {
 		switch ent := e.(type) {
 		case *LVMLogicalVolume:
-			hasLVM = true
+			ptFeatures.LVM = true
 		case *Btrfs:
-			hasBtrfs = true
+			ptFeatures.Btrfs = true
 		case *Filesystem:
 			switch ent.GetFSType() {
 			case "vfat":
-				hasFAT = true
+				ptFeatures.FAT = true
 			case "btrfs":
-				hasBtrfs = true
+				ptFeatures.Btrfs = true
 			case "xfs":
-				hasXFS = true
+				ptFeatures.XFS = true
 			case "ext4":
-				hasEXT4 = true
+				ptFeatures.EXT4 = true
 			}
 		case *LUKSContainer:
-			hasLUKS = true
+			ptFeatures.LUKS = true
 		}
 		return nil
 	}
 	_ = pt.ForEachEntity(introspectPT)
 
-	// TODO: LUKS
-	if hasLVM {
+	return ptFeatures
+}
+
+// GetBuildPackages returns an array of packages needed to support the features used in the PartitionTable.
+func (pt *PartitionTable) GetBuildPackages() []string {
+	packages := []string{}
+
+	features := pt.features()
+
+	if features.LVM {
 		packages = append(packages, "lvm2")
 	}
-	if hasBtrfs {
+	if features.Btrfs {
 		packages = append(packages, "btrfs-progs")
 	}
-	if hasXFS {
+	if features.XFS {
 		packages = append(packages, "xfsprogs")
 	}
-	if hasFAT {
+	if features.FAT {
 		packages = append(packages, "dosfstools")
 	}
-	if hasEXT4 {
+	if features.EXT4 {
 		packages = append(packages, "e2fsprogs")
 	}
-	if hasLUKS {
+	if features.LUKS {
 		packages = append(packages,
 			"clevis",
 			"clevis-luks",

pkg/disk/partition_table_test.go

@@ -0,0 +1,26 @@
+package disk
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPartitionTableFeatures(t *testing.T) {
+	type testCase struct {
+		partitionType    string
+		expectedFeatures partitionTableFeatures
+	}
+	testCases := []testCase{
+		{"plain", partitionTableFeatures{XFS: true, FAT: true}},
+		{"luks", partitionTableFeatures{XFS: true, FAT: true, LUKS: true}},
+		{"luks+lvm", partitionTableFeatures{XFS: true, FAT: true, LUKS: true, LVM: true}},
+		{"btrfs", partitionTableFeatures{XFS: true, FAT: true, Btrfs: true}},
+	}
+
+	for _, tc := range testCases {
+		pt := testPartitionTables[tc.partitionType]
+		assert.Equal(t, tc.expectedFeatures, pt.features())
+
+	}
+}

pkg/distro/distro.go

@@ -130,10 +130,11 @@ type ImageType interface {
 
 // The ImageOptions specify options for a specific image build
 type ImageOptions struct {
-	Size         uint64
-	OSTree       *ostree.ImageOptions
-	Subscription *subscription.ImageOptions
-	Facts        *facts.ImageOptions
+	Size             uint64
+	OSTree           *ostree.ImageOptions
+	Subscription     *subscription.ImageOptions
+	Facts            *facts.ImageOptions
+	PartitioningMode disk.PartitioningMode
 }
 
 type BasePartitionTableMap map[string]disk.PartitionTable

pkg/distro/fedora/imagetype.go

@@ -136,9 +136,18 @@ func (t *imageType) getPartitionTable(
 
 	imageSize := t.Size(options.Size)
 
-	lvmify := !t.rpmOstree
+	partitioningMode := options.PartitioningMode
+	if t.rpmOstree {
+		// IoT supports only LVM, force it.
+		// Raw is not supported, return an error if it is requested
+		// TODO Need a central location for logic like this
+		if partitioningMode == disk.RawPartitioningMode {
+			return nil, fmt.Errorf("partitioning mode raw not supported for %s on %s", t.Name(), t.arch.Name())
+		}
+		partitioningMode = disk.AutoLVMPartitioningMode
+	}
 
-	return disk.NewPartitionTable(&basePartitionTable, mountpoints, imageSize, lvmify, t.requiredPartitionSizes, rng)
+	return disk.NewPartitionTable(&basePartitionTable, mountpoints, imageSize, partitioningMode, t.requiredPartitionSizes, rng)
 }
 
 func (t *imageType) getDefaultImageConfig() *distro.ImageConfig {

pkg/distro/rhel7/imagetype.go

@@ -125,7 +125,7 @@ func (t *imageType) getPartitionTable(
 
 	imageSize := t.Size(options.Size)
 
-	return disk.NewPartitionTable(&basePartitionTable, mountpoints, imageSize, true, nil, rng)
+	return disk.NewPartitionTable(&basePartitionTable, mountpoints, imageSize, options.PartitioningMode, nil, rng)
 }
 
 func (t *imageType) getDefaultImageConfig() *distro.ImageConfig {