distro: fix bootc default filesystem handling when missing

cmd/gen-manifests/main.go

@@ -31,6 +31,7 @@ import (
 	"github.com/osbuild/images/pkg/depsolvednf"
 	"github.com/osbuild/images/pkg/distro"
 	"github.com/osbuild/images/pkg/distro/bootc"
+	"github.com/osbuild/images/pkg/distro/defs"
 	"github.com/osbuild/images/pkg/distrofactory"
 	"github.com/osbuild/images/pkg/experimentalflags"
 	"github.com/osbuild/images/pkg/manifest"
@@ -534,7 +535,14 @@ func main() {
 			buildBootcRef = l[1]
 		}
 
-		distribution, err := bootc.NewBootcDistro(bootcRef)
+		distribution, err := bootc.NewBootcDistro(bootcRef, nil)
+		if err != nil && errors.Is(err, defs.ErrNoDefaultFs) {
+			// XXX: consider making this configurable but for now
+			// we just need diffable manifests
+			distribution, err = bootc.NewBootcDistro(bootcRef, &bootc.DistroOptions{
+				DefaultFs: "ext4",
+			})
+		}
 		if err != nil {
 			panic(err)
 		}
@@ -543,13 +551,6 @@ func main() {
 				panic(err)
 			}
 		}
-		// XXX: consider making this configurable but for now
-		// we just need diffable manifests
-		if distribution.DefaultFs() == "" {
-			if err := distribution.SetDefaultFs("ext4"); err != nil {
-				panic(err)
-			}
-		}
 		for _, archName := range arches {
 			archi, err := distribution.GetArch(archName)
 			if err != nil {

pkg/distro/bootc/bootc.go

@@ -89,15 +89,6 @@ func (d *BootcDistro) SetBuildContainerForTesting(imgref string, info *osinfo.In
 	return d.setBuildContainer(imgref, info)
 }
 
-func (d *BootcDistro) SetDefaultFs(defaultFs string) error {
-	if defaultFs == "" {
-		return nil
-	}
-
-	d.defaultFs = defaultFs
-	return nil
-}
-
 func (d *BootcDistro) DefaultFs() string {
 	return d.defaultFs
 }
@@ -523,9 +514,20 @@ func (t *BootcImageType) manifestForISO(bp *blueprint.Blueprint, options distro.
 	return &mf, nil, err
 }
 
+type DistroOptions struct {
+	// DefaultFs to use, this takes precedence over the default
+	// from the container and is required if the container does
+	// not declare a default.
+	DefaultFs string
+}
+
 // newBootcDistro returns a new instance of BootcDistro
 // from the given url
-func NewBootcDistro(imgref string) (*BootcDistro, error) {
+func NewBootcDistro(imgref string, opts *DistroOptions) (*BootcDistro, error) {
+	if opts == nil {
+		opts = &DistroOptions{}
+	}
+
 	cnt, err := bibcontainer.New(imgref)
 	if err != nil {
 		return nil, err
@@ -538,11 +540,15 @@ func NewBootcDistro(imgref string) (*BootcDistro, error) {
 	if err != nil {
 		return nil, err
 	}
-	// XXX: provide a way to set defaultfs (needed for bib)
+
 	defaultFs, err := cnt.DefaultRootfsType()
 	if err != nil {
 		return nil, err
 	}
+	if opts.DefaultFs != "" {
+		defaultFs = opts.DefaultFs
+	}
+
 	cntSize, err := getContainerSize(imgref)
 	if err != nil {
 		return nil, fmt.Errorf("cannot get container size: %w", err)
@@ -612,5 +618,5 @@ func DistroFactory(idStr string) distro.Distro {
 	}
 	imgRef := l[1]
 
-	return common.Must(NewBootcDistro(imgRef))
+	return common.Must(NewBootcDistro(imgRef, nil))
 }

pkg/distro/bootc/export_test.go

@@ -19,13 +19,17 @@ var (
 )
 
 func NewTestBootcDistro() *BootcDistro {
+	return NewTestBootcDistroWithDefaultFs("xfs")
+}
+
+func NewTestBootcDistroWithDefaultFs(defaultFs string) *BootcDistro {
 	info := &osinfo.Info{
 		OSRelease: osinfo.OSRelease{
 			ID:        "bootc-test",
 			VersionID: "1",
 		},
 	}
-	return common.Must(newBootcDistroAfterIntrospect("x86_64", info, "quay.io/example/example:ref", "xfs", 0))
+	return common.Must(newBootcDistroAfterIntrospect("x86_64", info, "quay.io/example/example:ref", defaultFs, 0))
 }
 
 func NewTestBootcImageType() *BootcImageType {

pkg/distro/bootc/image_test.go

@@ -348,9 +348,10 @@ func TestGenPartitionTableSetsRootfsForAllFilesystemsXFS(t *testing.T) {
 func TestGenPartitionTableSetsRootfsForAllFilesystemsBtrfs(t *testing.T) {
 	rng := createRand()
 
-	imgType := bootc.NewTestBootcImageType()
-	err := imgType.Arch().Distro().(*bootc.BootcDistro).SetDefaultFs("btrfs")
+	d := bootc.NewTestBootcDistroWithDefaultFs("btrfs")
+	it, err := common.Must(d.GetArch("x86_64")).GetImageType("qcow2")
 	assert.NoError(t, err)
+	imgType := it.(*bootc.BootcImageType)
 	cus := &blueprint.Customizations{}
 	rootfsMinSize := uint64(0)
 	pt, err := imgType.GenPartitionTable(cus, rootfsMinSize, rng)
@@ -359,9 +360,9 @@ func TestGenPartitionTableSetsRootfsForAllFilesystemsBtrfs(t *testing.T) {
 	mnt, _ := findMountableSizeableFor(pt, "/")
 	assert.Equal(t, "btrfs", mnt.GetFSType())
 
-	// btrfs has a default (xfs) /boot
+	// btrfs has a default (ext4) /boot
 	mnt, _ = findMountableSizeableFor(pt, "/boot")
-	assert.Equal(t, "xfs", mnt.GetFSType())
+	assert.Equal(t, "ext4", mnt.GetFSType())
 
 	// ESP is always vfat
 	mnt, _ = findMountableSizeableFor(pt, "/boot/efi")

pkg/distro/bootc/integration_test.go

@@ -66,7 +66,7 @@ func TestBuildContainerHandling(t *testing.T) {
 
 	for _, withBuildContainer := range []bool{true, false} {
 		t.Run(fmt.Sprintf("build-cnt:%v", withBuildContainer), func(t *testing.T) {
-			distri, err := bootc.NewBootcDistro(imgTag)
+			distri, err := bootc.NewBootcDistro(imgTag, nil)
 			require.NoError(t, err)
 			if withBuildContainer {
 				err = distri.SetBuildContainer(buildImgTag)
@@ -129,7 +129,7 @@ partition_table:
 
 	for _, withBuildContainer := range []bool{true, false} {
 		t.Run(fmt.Sprintf("build-cnt:%v", withBuildContainer), func(t *testing.T) {
-			distri, err := bootc.NewBootcDistro(imgTag)
+			distri, err := bootc.NewBootcDistro(imgTag, nil)
 			require.NoError(t, err)
 			if withBuildContainer {
 				err = distri.SetBuildContainer(buildImgTag)

pkg/distro/defs/loader.go

@@ -35,6 +35,7 @@ import (
 var (
 	ErrNoPartitionTableForImgType = errors.New("no partition table for image type")
 	ErrNoPartitionTableForArch    = errors.New("no partition table for arch")
+	ErrNoDefaultFs                = errors.New("no default fs set")
 )
 
 // this can be overriden in tests
@@ -512,12 +513,19 @@ func (it *ImageTypeYAML) setupDefaultFS(distroDefaultFS string) error {
 				if !ok {
 					return nil
 				}
-				if elem.Type == "" {
-					if distroDefaultFS == "" {
-						return fmt.Errorf("mount %q requires a default filesystem for the distribution but none set", mnt.GetMountpoint())
-					}
-					elem.Type = distroDefaultFS
+				if elem.Type != "" {
+					return nil
+				}
+				if distroDefaultFS == "" {
+					return fmt.Errorf("%w: mount %q requires a filesystem but none set", ErrNoDefaultFs, mnt.GetMountpoint())
+				}
+				// XXX: this is really the wrong layer, we need to move this into disk
+				// btrfs cannot be on /boot, we need to select a conservative option here instead
+				if mnt.GetMountpoint() == "/boot" && distroDefaultFS == "btrfs" {
+					elem.Type = "ext4"
+					return nil
 				}
+				elem.Type = distroDefaultFS
 				return nil
 			})
 			if err != nil {

pkg/distro/defs/loader_test.go

@@ -438,7 +438,7 @@ image_types:
 	restore := defs.MockDataFS(baseDir)
 	defer restore()
 	_, err := defs.NewDistroYAML("test-distro-1")
-	assert.EqualError(t, err, `mount "/" requires a default filesystem for the distribution but none set`)
+	assert.EqualError(t, err, `no default fs set: mount "/" requires a filesystem but none set`)
 }
 
 var fakeImageTypesYaml = `

pkg/osbuild/mkfs_stage.go

@@ -72,7 +72,7 @@ func GenFsStages(pt *disk.PartitionTable, filename string, soucePipeline string)
 
 				stages = append(stages, NewMkfsExt4Stage(options, stageDevices))
 			default:
-				panic(fmt.Sprintf("unknown fs type: %s", e.GetFSType()))
+				panic(fmt.Sprintf("unknown fs type: %s for %s", e.GetFSType(), e.GetMountpoint()))
 			}
 
 			if mkfsOptions.Geometry != nil {

pkg/osbuild/mkfs_stages_test.go

@@ -515,13 +515,14 @@ func TestGenFsStagesUnhappy(t *testing.T) {
 		Partitions: []disk.Partition{
 			{
 				Payload: &disk.Filesystem{
-					Type: "ext2",
+					Type:       "ext2",
+					Mountpoint: "/",
 				},
 			},
 		},
 	}
 
-	assert.PanicsWithValue(t, "unknown fs type: ext2", func() {
+	assert.PanicsWithValue(t, "unknown fs type: ext2 for /", func() {
 		GenFsStages(pt, "file.img", "build")
 	})
 }