diff --git a/cmd/gen-manifests/main.go b/cmd/gen-manifests/main.go index 69d2dd4391..cfac52d059 100644 --- a/cmd/gen-manifests/main.go +++ b/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 { diff --git a/pkg/distro/bootc/bootc.go b/pkg/distro/bootc/bootc.go index 2e7d279b68..bda4eef54b 100644 --- a/pkg/distro/bootc/bootc.go +++ b/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)) } diff --git a/pkg/distro/bootc/export_test.go b/pkg/distro/bootc/export_test.go index d0413f6643..5d403984eb 100644 --- a/pkg/distro/bootc/export_test.go +++ b/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 { diff --git a/pkg/distro/bootc/image_test.go b/pkg/distro/bootc/image_test.go index 09986d45ad..0c8e6cdb06 100644 --- a/pkg/distro/bootc/image_test.go +++ b/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") diff --git a/pkg/distro/bootc/integration_test.go b/pkg/distro/bootc/integration_test.go index 38c23f1ec3..df6e1091d6 100644 --- a/pkg/distro/bootc/integration_test.go +++ b/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) diff --git a/pkg/distro/defs/loader.go b/pkg/distro/defs/loader.go index 6dc52468ad..0582c7267e 100644 --- a/pkg/distro/defs/loader.go +++ b/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 { diff --git a/pkg/distro/defs/loader_test.go b/pkg/distro/defs/loader_test.go index a1dd621a73..68360561b7 100644 --- a/pkg/distro/defs/loader_test.go +++ b/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 = ` diff --git a/pkg/osbuild/mkfs_stage.go b/pkg/osbuild/mkfs_stage.go index 85c4c49d3d..271f765ae9 100644 --- a/pkg/osbuild/mkfs_stage.go +++ b/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 { diff --git a/pkg/osbuild/mkfs_stages_test.go b/pkg/osbuild/mkfs_stages_test.go index 73bc971c8a..489dfd6e7d 100644 --- a/pkg/osbuild/mkfs_stages_test.go +++ b/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") }) }