distro/fedora: implementation cleanups for newer releases

Clean up some implementation aspects of the Fedora distro definition:
 - Do not have default Fedora distro version and use `fedora` as the
   package name in all places that use it, instead of `fedora33`.
 - Fix bugs when wrong (Fedora 33) values were returned by `OSTreeRef()`
   and `Releasever()` for newer Fedora releases.
 - Test Fedora 35 in package unit tests.
 - Add unit test for `OSTreeRef()` method.
 - Use architecture name constants from `distro` package, instead of
   string literals.

Fix #1802

Signed-off-by: Tomas Hozza <thozza@redhat.com>
This commit is contained in:
Tomas Hozza 2022-02-02 14:25:08 +01:00 committed by Tomáš Hozza
parent 8b8c7bbbbe
commit b9efe82bd7
5 changed files with 138 additions and 117 deletions

View file

@ -11,7 +11,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distro/fedora33"
fedora "github.com/osbuild/osbuild-composer/internal/distro/fedora33"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/osbuild/osbuild-composer/internal/store"
"github.com/osbuild/osbuild-composer/internal/target"
@ -116,8 +116,8 @@ func main() {
awsTarget.ImageName = "My Image"
awsTarget.Created = time.Now()
d := fedora33.New()
a, err := d.GetArch("x86_64")
d := fedora.NewF35()
a, err := d.GetArch(distro.X86_64ArchName)
if err != nil {
panic(err)
}
@ -129,11 +129,11 @@ func main() {
if err != nil {
panic(err)
}
allRepos, err := rpmmd.LoadRepositories([]string{cwd}, "fedora-33")
allRepos, err := rpmmd.LoadRepositories([]string{cwd}, "fedora-35")
if err != nil {
panic(err)
}
repos := allRepos["x86_64"]
repos := allRepos[distro.X86_64ArchName]
homeDir, err := os.UserHomeDir()
if err != nil {
panic("os.UserHomeDir(): " + err.Error())

View file

@ -16,27 +16,25 @@ import (
"github.com/osbuild/osbuild-composer/internal/rpmmd"
)
const defaultName = "fedora-33"
const releaseVersion = "33"
const modulePlatformID = "platform:f33"
const ostreeRef = "fedora/33/%s/iot"
const nameTmpl = "fedora-%s"
const modulePlatformIDTmpl = "platform:f%s"
const f34Name = "fedora-34"
const f34modulePlatformID = "platform:f34"
const f34ostreeRef = "fedora/34/%s/iot"
// The second format value is intentionally escaped, because the
// format string is substituted in two steps:
// 1. on distribution level when being created
// 2. on image level
const ostreeRefTmpl = "fedora/%s/%%s/iot"
const f35Name = "fedora-35"
const f35modulePlatformID = "platform:f35"
const f35ostreeRef = "fedora/35/%s/iot"
const f36Name = "fedora-36"
const f36modulePlatformID = "platform:f36"
const f36ostreeRef = "fedora/36/%s/iot"
const f33ReleaseVersion = "33"
const f34ReleaseVersion = "34"
const f35ReleaseVersion = "35"
const f36ReleaseVersion = "36"
type distribution struct {
name string
releaseVersion string
modulePlatformID string
ostreeRef string
ostreeRefTmpl string
arches map[string]architecture
buildPackages []string
}
@ -177,7 +175,7 @@ func (t *imageType) MIMEType() string {
func (t *imageType) OSTreeRef() string {
if t.rpmOstree {
return fmt.Sprintf(ostreeRef, t.arch.name)
return fmt.Sprintf(t.Arch().Distro().OSTreeRef(), t.Arch().Name())
}
return ""
}
@ -278,7 +276,7 @@ func (d *distribution) Name() string {
}
func (d *distribution) Releasever() string {
return releaseVersion
return d.releaseVersion
}
func (d *distribution) ModulePlatformID() string {
@ -286,7 +284,7 @@ func (d *distribution) ModulePlatformID() string {
}
func (d *distribution) OSTreeRef() string {
return d.ostreeRef
return d.ostreeRefTmpl
}
func sources(packages []rpmmd.PackageSpec) *osbuild.Sources {
@ -642,29 +640,80 @@ func ostreeCommitAssembler(options distro.ImageOptions, arch distro.Arch) *osbui
}
// New creates a new distro object, defining the supported architectures and image types
func New() distro.Distro {
return newDistro(defaultName, modulePlatformID, ostreeRef)
func NewF33() distro.Distro {
return newDefaultDistro(f33ReleaseVersion)
}
func NewF34() distro.Distro {
return newDistro(f34Name, f34modulePlatformID, f34ostreeRef)
return newDefaultDistro(f34ReleaseVersion)
}
func NewF35() distro.Distro {
return newDistro(f35Name, f35modulePlatformID, f35ostreeRef)
return newDefaultDistro(f35ReleaseVersion)
}
func NewF36() distro.Distro {
return newDistro(f36Name, f36modulePlatformID, f36ostreeRef)
return newDefaultDistro(f36ReleaseVersion)
}
func NewHostDistro(name, modulePlatformID, ostreeRef string) distro.Distro {
return newDistro(name, modulePlatformID, ostreeRef)
}
func newDefaultDistro(releaseVersion string) distro.Distro {
return newDistro(
fmt.Sprintf(nameTmpl, releaseVersion),
fmt.Sprintf(modulePlatformIDTmpl, releaseVersion),
fmt.Sprintf(ostreeRefTmpl, releaseVersion),
)
}
func newDistro(name, modulePlatformID, ostreeRef string) distro.Distro {
const GigaByte = 1024 * 1024 * 1024
r := distribution{
buildPackages: []string{
"dnf",
"dosfstools",
"e2fsprogs",
"policycoreutils",
"qemu-img",
"selinux-policy-targeted",
"systemd",
"tar",
"xz",
},
name: name,
modulePlatformID: modulePlatformID,
ostreeRefTmpl: ostreeRef,
}
x86_64 := architecture{
distro: &r,
name: distro.X86_64ArchName,
bootloaderPackages: []string{
"dracut-config-generic",
"grub2-pc",
},
buildPackages: []string{
"grub2-pc",
},
legacy: "i386-pc",
}
aarch64 := architecture{
distro: &r,
name: distro.Aarch64ArchName,
bootloaderPackages: []string{
"dracut-config-generic",
"efibootmgr",
"grub2-efi-aa64",
"grub2-tools",
"shim-aa64",
},
uefi: true,
}
iotImgType := imageType{
name: "fedora-iot-commit",
filename: "commit.tar",
@ -934,35 +983,7 @@ func newDistro(name, modulePlatformID, ostreeRef string) distro.Distro {
},
}
r := distribution{
buildPackages: []string{
"dnf",
"dosfstools",
"e2fsprogs",
"policycoreutils",
"qemu-img",
"selinux-policy-targeted",
"systemd",
"tar",
"xz",
},
name: name,
modulePlatformID: modulePlatformID,
ostreeRef: ostreeRef,
}
x8664 := architecture{
distro: &r,
name: "x86_64",
bootloaderPackages: []string{
"dracut-config-generic",
"grub2-pc",
},
buildPackages: []string{
"grub2-pc",
},
legacy: "i386-pc",
}
x8664.setImageTypes(
x86_64.setImageTypes(
iotImgType,
amiImgType,
qcow2ImageType,
@ -972,18 +993,6 @@ func newDistro(name, modulePlatformID, ostreeRef string) distro.Distro {
ociImageType,
)
aarch64 := architecture{
distro: &r,
name: "aarch64",
bootloaderPackages: []string{
"dracut-config-generic",
"efibootmgr",
"grub2-efi-aa64",
"grub2-tools",
"shim-aa64",
},
uefi: true,
}
aarch64.setImageTypes(
amiImgType,
qcow2ImageType,
@ -991,7 +1000,7 @@ func newDistro(name, modulePlatformID, ostreeRef string) distro.Distro {
ociImageType,
)
r.setArches(x8664, aarch64)
r.setArches(x86_64, aarch64)
return &r
}

View file

@ -8,7 +8,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distro/distro_test_common"
"github.com/osbuild/osbuild-composer/internal/distro/fedora33"
fedora "github.com/osbuild/osbuild-composer/internal/distro/fedora33"
)
func TestFilenameFromType(t *testing.T) {
@ -60,8 +60,8 @@ func TestFilenameFromType(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
dist := fedora33.New()
arch, _ := dist.GetArch("x86_64")
dist := fedora.NewF35()
arch, _ := dist.GetArch(distro.X86_64ArchName)
imgType, err := arch.GetImageType(tt.args.outputFormat)
if (err != nil) != tt.wantErr {
t.Errorf("Arch.GetImageType() error = %v, wantErr %v", err, tt.wantErr)
@ -106,10 +106,10 @@ func TestImageType_BuildPackages(t *testing.T) {
"xz",
}
buildPackages := map[string][]string{
"x86_64": x8664BuildPackages,
"aarch64": aarch64BuildPackages,
distro.X86_64ArchName: x8664BuildPackages,
distro.Aarch64ArchName: aarch64BuildPackages,
}
d := fedora33.New()
d := fedora.NewF35()
for _, archLabel := range d.ListArches() {
archStruct, err := d.GetArch(archLabel)
if err != nil {
@ -137,13 +137,13 @@ func TestImageType_BuildPackages(t *testing.T) {
}
func TestImageType_Name(t *testing.T) {
distro := fedora33.New()
f35 := fedora.NewF35()
imgMap := []struct {
arch string
imgNames []string
}{
{
arch: "x86_64",
arch: distro.X86_64ArchName,
imgNames: []string{
"ami",
"qcow2",
@ -153,7 +153,7 @@ func TestImageType_Name(t *testing.T) {
},
},
{
arch: "aarch64",
arch: distro.Aarch64ArchName,
imgNames: []string{
"ami",
"qcow2",
@ -162,7 +162,7 @@ func TestImageType_Name(t *testing.T) {
},
}
for _, mapping := range imgMap {
arch, err := distro.GetArch(mapping.arch)
arch, err := f35.GetArch(mapping.arch)
if assert.NoError(t, err) {
for _, imgName := range mapping.imgNames {
imgType, err := arch.GetImageType(imgName)
@ -203,8 +203,8 @@ func TestImageType_Size(t *testing.T) {
},
}
distro := fedora33.New()
arch, err := distro.GetArch("x86_64")
f35 := fedora.NewF35()
arch, err := f35.GetArch(distro.X86_64ArchName)
if assert.NoError(t, err) {
for _, mapping := range sizeMap {
imgType, err := arch.GetImageType(mapping.name)
@ -282,8 +282,8 @@ func TestImageType_BasePackages(t *testing.T) {
bootable: true,
},
}
distro := fedora33.New()
arch, err := distro.GetArch("x86_64")
f35 := fedora.NewF35()
arch, err := f35.GetArch(distro.X86_64ArchName)
assert.NoError(t, err)
for _, pkgMap := range pkgMaps {
@ -307,7 +307,7 @@ func TestImageType_BasePackages(t *testing.T) {
func TestDistro_ManifestError(t *testing.T) {
// Currently, the only unsupported configuration is OSTree commit types
// with Kernel boot options
f33distro := fedora33.New()
f35distro := fedora.NewF35()
bp := blueprint.Blueprint{
Customizations: &blueprint.Customizations{
Kernel: &blueprint.KernelCustomization{
@ -316,8 +316,8 @@ func TestDistro_ManifestError(t *testing.T) {
},
}
for _, archName := range f33distro.ListArches() {
arch, _ := f33distro.GetArch(archName)
for _, archName := range f35distro.ListArches() {
arch, _ := f35distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, err := imgType.Manifest(bp.Customizations, distro.ImageOptions{}, nil, nil, 0)
@ -330,23 +330,23 @@ func TestDistro_ManifestError(t *testing.T) {
}
}
func TestFedora33_ListArches(t *testing.T) {
distro := fedora33.New()
arches := distro.ListArches()
assert.Equal(t, []string{"aarch64", "x86_64"}, arches)
func TestFedora35_ListArches(t *testing.T) {
f35 := fedora.NewF35()
arches := f35.ListArches()
assert.Equal(t, []string{distro.Aarch64ArchName, distro.X86_64ArchName}, arches)
}
func TestFedora33_GetArch(t *testing.T) {
distro := fedora33.New()
func TestFedora35_GetArch(t *testing.T) {
f35 := fedora.NewF35()
arches := []struct {
name string
errorExpected bool
}{
{
name: "x86_64",
name: distro.X86_64ArchName,
},
{
name: "aarch64",
name: distro.Aarch64ArchName,
},
{
name: "foo-arch",
@ -355,7 +355,7 @@ func TestFedora33_GetArch(t *testing.T) {
}
for _, a := range arches {
actualArch, err := distro.GetArch(a.name)
actualArch, err := f35.GetArch(a.name)
if !a.errorExpected {
assert.Equal(t, a.name, actualArch.Name())
assert.NoError(t, err)
@ -366,22 +366,34 @@ func TestFedora33_GetArch(t *testing.T) {
}
}
func TestFedora33_Name(t *testing.T) {
distro := fedora33.New()
assert.Equal(t, "fedora-33", distro.Name())
func TestFedora35_Name(t *testing.T) {
distro := fedora.NewF35()
assert.Equal(t, "fedora-35", distro.Name())
}
func TestFedora33_ModulePlatformID(t *testing.T) {
distro := fedora33.New()
assert.Equal(t, "platform:f33", distro.ModulePlatformID())
func TestFedora35_ModulePlatformID(t *testing.T) {
distro := fedora.NewF35()
assert.Equal(t, "platform:f35", distro.ModulePlatformID())
}
func TestFedora33_KernelOption(t *testing.T) {
distro_test_common.TestDistro_KernelOption(t, fedora33.New())
func TestFedora35_OSTreeRef(t *testing.T) {
f35 := fedora.NewF35()
assert.Equal(t, "fedora/35/%s/iot", f35.OSTreeRef())
x86_64, err := f35.GetArch(distro.X86_64ArchName)
assert.Nilf(t, err, "failed to get %q architecture of %q distribution", distro.X86_64ArchName, f35.Name())
ostreeImgName := "fedora-iot-commit"
ostreeImg, err := x86_64.GetImageType(ostreeImgName)
assert.Nilf(t, err, "failed to get %q image type for %q architecture of %q distribution", ostreeImgName, distro.X86_64ArchName, f35.Name())
assert.Equal(t, "fedora/35/x86_64/iot", ostreeImg.OSTreeRef())
}
func TestFedora35_KernelOption(t *testing.T) {
distro_test_common.TestDistro_KernelOption(t, fedora.NewF35())
}
func TestDistro_CustomFileSystemManifestError(t *testing.T) {
f33distro := fedora33.New()
f35distro := fedora.NewF35()
bp := blueprint.Blueprint{
Customizations: &blueprint.Customizations{
Filesystem: []blueprint.FilesystemCustomization{
@ -392,8 +404,8 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) {
},
},
}
for _, archName := range f33distro.ListArches() {
arch, _ := f33distro.GetArch(archName)
for _, archName := range f35distro.ListArches() {
arch, _ := f35distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, err := imgType.Manifest(bp.Customizations, distro.ImageOptions{}, nil, nil, 0)
@ -407,7 +419,7 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) {
}
func TestDistro_TestRootMountPoint(t *testing.T) {
f33distro := fedora33.New()
f35distro := fedora.NewF35()
bp := blueprint.Blueprint{
Customizations: &blueprint.Customizations{
Filesystem: []blueprint.FilesystemCustomization{
@ -418,8 +430,8 @@ func TestDistro_TestRootMountPoint(t *testing.T) {
},
},
}
for _, archName := range f33distro.ListArches() {
arch, _ := f33distro.GetArch(archName)
for _, archName := range f35distro.ListArches() {
arch, _ := f35distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, err := imgType.Manifest(bp.Customizations, distro.ImageOptions{}, nil, nil, 0)

View file

@ -6,7 +6,7 @@ import (
"strings"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distro/fedora33"
fedora "github.com/osbuild/osbuild-composer/internal/distro/fedora33"
"github.com/osbuild/osbuild-composer/internal/distro/rhel8"
"github.com/osbuild/osbuild-composer/internal/distro/rhel84"
"github.com/osbuild/osbuild-composer/internal/distro/rhel85"
@ -18,10 +18,10 @@ import (
// When adding support for a new distribution, add it here.
// Note that this is a constant, do not write to this array.
var supportedDistros = []supportedDistro{
{fedora33.New, fedora33.NewHostDistro},
{fedora33.NewF34, fedora33.NewHostDistro},
{fedora33.NewF35, fedora33.NewHostDistro},
{fedora33.NewF36, fedora33.NewHostDistro},
{fedora.NewF33, fedora.NewHostDistro},
{fedora.NewF34, fedora.NewHostDistro},
{fedora.NewF35, fedora.NewHostDistro},
{fedora.NewF36, fedora.NewHostDistro},
{rhel8.New, rhel8.NewHostDistro},
{rhel84.New, rhel84.NewHostDistro},
{rhel85.New, rhel85.NewHostDistro},

View file

@ -13,7 +13,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/common"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distro/fedora33"
fedora "github.com/osbuild/osbuild-composer/internal/distro/fedora33"
"github.com/osbuild/osbuild-composer/internal/distro/test_distro"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/osbuild/osbuild-composer/internal/target"
@ -303,7 +303,7 @@ func Test_upgrade(t *testing.T) {
assert.NoErrorf(err, "Could not read test-store '%s': %v", fileName, err)
err = json.Unmarshal([]byte(file), &storeStruct)
assert.NoErrorf(err, "Could not parse test-store '%s': %v", fileName, err)
arch, err := fedora33.New().GetArch("x86_64")
arch, err := fedora.NewF35().GetArch(distro.X86_64ArchName)
assert.NoError(err)
store := newStoreFromV0(storeStruct, arch, nil)
assert.Equal(1, len(store.blueprints))