Make the Distroregistry FromHost() return distro with correct name

Composer does not have 1:1 mapping of what can be the Host Distro name
and the names of supported distributions held in the Distroregistry.

The fact that the host distro `Name()` method as passed to the Weldr API
does not return the same name as what is used as distro name for
repository definitions. This makes it hard to use `distro.Distro` and
`distro.Arch` directly and rely on the values returned by them as their
name.

Add `New*HostDistro()` to all distro definitions, accepting the name
that should be returned by the distro's `Name()` method. This is useful
mainly if the host distro is Beta or Stream variant of the distro.

Change the distroregistry.Registry to contain host distro as a separate
value set when creating it using `New()` function. This value is
returned by `Registry.FromHost()` method. Determining the host distro is
handled by the `NewDefault()` function. Move the distro name mangling to
distroregistry package. Add relevant unit tests.

Signed-off-by: Tomas Hozza <thozza@redhat.com>
This commit is contained in:
Tomas Hozza 2021-05-06 14:53:42 +02:00 committed by Ondřej Budai
parent dda9cce03e
commit fba9fe1072
10 changed files with 231 additions and 65 deletions

View file

@ -11,7 +11,6 @@ import (
"net/http"
"os"
"path"
"strings"
"github.com/osbuild/osbuild-composer/internal/cloudapi"
"github.com/osbuild/osbuild-composer/internal/common"
@ -76,9 +75,9 @@ func NewComposer(config *ComposerConfigFile, stateDir, cacheDir string, logger *
func (c *Composer) InitWeldr(repoPaths []string, weldrListener net.Listener) error {
archName := common.CurrentArch()
hostDistro, beta, isStream, err := c.distros.FromHost()
if err != nil {
return err
hostDistro := c.distros.FromHost()
if hostDistro == nil {
return fmt.Errorf("host distro is not supported")
}
arch, err := hostDistro.GetArch(archName)
@ -86,21 +85,7 @@ func (c *Composer) InitWeldr(repoPaths []string, weldrListener net.Listener) err
return fmt.Errorf("Host distro does not support host architecture: %v", err)
}
// TODO: refactor to be more generic
name := hostDistro.Name()
if strings.HasPrefix(name, "rhel-8") {
name = "rhel-8"
}
if beta {
name += "-beta"
}
// override repository for centos stream, remove when CentOS 8 is EOL
if isStream && name == "centos-8" {
name = "centos-stream-8"
}
repos, err := rpmmd.LoadRepositories(repoPaths, name)
repos, err := rpmmd.LoadRepositories(repoPaths, hostDistro.Name())
if err != nil {
return fmt.Errorf("Error loading repositories for %s: %v", hostDistro.Name(), err)
}

View file

@ -16,11 +16,12 @@ import (
"github.com/osbuild/osbuild-composer/internal/rpmmd"
)
const name = "fedora-32"
const defaultName = "fedora-32"
const modulePlatformID = "platform:f32"
const ostreeRef = "fedora/32/%s/iot"
type distribution struct {
name string
arches map[string]architecture
buildPackages []string
}
@ -246,7 +247,7 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
}
func (d *distribution) Name() string {
return name
return d.name
}
func (d *distribution) ModulePlatformID() string {
@ -573,6 +574,14 @@ 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)
}
func NewHostDistro(name string) distro.Distro {
return newDistro(name)
}
func newDistro(name string) distro.Distro {
const GigaByte = 1024 * 1024 * 1024
iotImgType := imageType{
@ -799,6 +808,7 @@ func New() distro.Distro {
"tar",
"xz",
},
name: name,
}
x8664 := architecture{
distro: &r,

View file

@ -16,11 +16,12 @@ import (
"github.com/osbuild/osbuild-composer/internal/rpmmd"
)
const name = "fedora-33"
const defaultName = "fedora-33"
const modulePlatformID = "platform:f33"
const ostreeRef = "fedora/33/%s/iot"
type distribution struct {
name string
arches map[string]architecture
buildPackages []string
}
@ -246,7 +247,7 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
}
func (d *distribution) Name() string {
return name
return d.name
}
func (d *distribution) ModulePlatformID() string {
@ -585,6 +586,14 @@ 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)
}
func NewHostDistro(name string) distro.Distro {
return newDistro(name)
}
func newDistro(name string) distro.Distro {
const GigaByte = 1024 * 1024 * 1024
iotImgType := imageType{
@ -834,6 +843,7 @@ func New() distro.Distro {
"tar",
"xz",
},
name: name,
}
x8664 := architecture{
distro: &r,

View file

@ -16,11 +16,12 @@ import (
"github.com/osbuild/osbuild-composer/internal/rpmmd"
)
const name = "rhel-8"
const defaultName = "rhel-8"
const modulePlatformID = "platform:el8"
const ostreeRef = "rhel/8/%s/edge"
type distribution struct {
name string
arches map[string]architecture
buildPackages []string
}
@ -248,7 +249,7 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
}
func (d *distribution) Name() string {
return name
return d.name
}
func (d *distribution) ModulePlatformID() string {
@ -672,6 +673,14 @@ 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)
}
func NewHostDistro(name string) distro.Distro {
return newDistro(name)
}
func newDistro(name string) distro.Distro {
const GigaByte = 1024 * 1024 * 1024
edgeImgTypeX86_64 := imageType{
@ -1064,6 +1073,7 @@ func New() distro.Distro {
"xfsprogs",
"xz",
},
name: name,
}
x8664 := architecture{
distro: &r,

View file

@ -19,12 +19,13 @@ import (
"github.com/osbuild/osbuild-composer/internal/rpmmd"
)
const name = "rhel-84"
const centosName = "centos-8"
const defaultName = "rhel-84"
const defaultCentosName = "centos-8"
const modulePlatformID = "platform:el8"
const ostreeRef = "rhel/8/%s/edge"
type distribution struct {
name string
arches map[string]architecture
buildPackages []string
isCentos bool
@ -277,10 +278,7 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
}
func (d *distribution) Name() string {
if d.isCentos {
return centosName
}
return name
return d.name
}
func (d *distribution) ModulePlatformID() string {
@ -822,14 +820,22 @@ func removePackage(packages []string, packageToRemove string) []string {
// New creates a new distro object, defining the supported architectures and image types
func New() distro.Distro {
return newDistro(false)
return newDistro(defaultName, false)
}
func NewCentos() distro.Distro {
return newDistro(true)
return newDistro(defaultCentosName, true)
}
func newDistro(isCentos bool) distro.Distro {
func NewHostDistro(name string) distro.Distro {
return newDistro(name, false)
}
func NewCentosHostDistro(name string) distro.Distro {
return newDistro(name, true)
}
func newDistro(name string, isCentos bool) distro.Distro {
const GigaByte = 1024 * 1024 * 1024
edgeImgTypeX86_64 := imageType{
@ -1237,6 +1243,7 @@ func newDistro(isCentos bool) distro.Distro {
"xz",
},
isCentos: isCentos,
name: name,
}
x8664 := architecture{
distro: &r,

View file

@ -14,17 +14,18 @@ import (
"github.com/osbuild/osbuild-composer/internal/rpmmd"
)
const name = "rhel-85"
const defaultName = "rhel-85"
const osVersion = "8.5"
const modulePlatformID = "platform:el8"
const ostreeRef = "rhel/8/%s/edge"
type distribution struct {
name string
arches map[string]distro.Arch
}
func (d *distribution) Name() string {
return name
return d.name
}
func (d *distribution) ModulePlatformID() string {
@ -277,7 +278,17 @@ func (t *imageType) checkOptions(customizations *blueprint.Customizations, optio
// New creates a new distro object, defining the supported architectures and image types
func New() distro.Distro {
rd := new(distribution)
return newDistro(defaultName)
}
func NewHostDistro(name string) distro.Distro {
return newDistro(name)
}
func newDistro(name string) distro.Distro {
rd := &distribution{
name: name,
}
x86_64 := architecture{
name: "x86_64",

View file

@ -19,10 +19,11 @@ import (
"github.com/osbuild/osbuild-composer/internal/rpmmd"
)
const name = "rhel-90"
const defaultName = "rhel-90"
const modulePlatformID = "platform:el9"
type distribution struct {
name string
arches map[string]architecture
buildPackages []string
}
@ -252,7 +253,7 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
}
func (d *distribution) Name() string {
return name
return d.name
}
func (d *distribution) ModulePlatformID() string {
@ -742,6 +743,14 @@ func newRandomUUIDFromReader(r io.Reader) (uuid.UUID, error) {
}
func New() distro.Distro {
return newDistro(defaultName)
}
func NewHostDistro(name string) distro.Distro {
return newDistro(name)
}
func newDistro(name string) distro.Distro {
const GigaByte = 1024 * 1024 * 1024
qcow2ImageType := imageType{
@ -839,6 +848,7 @@ func New() distro.Distro {
"xfsprogs",
"xz",
},
name: name,
}
x8664 := architecture{
distro: &r,

View file

@ -1,9 +1,9 @@
package distroregistry
import (
"errors"
"fmt"
"sort"
"strings"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distro/fedora32"
@ -16,23 +16,30 @@ 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 = []func() distro.Distro{
fedora32.New,
fedora33.New,
rhel8.New,
rhel84.New,
rhel84.NewCentos,
rhel85.New,
rhel90.New,
var supportedDistros = []supportedDistro{
{fedora32.New, fedora32.NewHostDistro},
{fedora33.New, fedora33.NewHostDistro},
{rhel8.New, rhel8.NewHostDistro},
{rhel84.New, rhel84.NewHostDistro},
{rhel84.NewCentos, rhel84.NewCentosHostDistro},
{rhel85.New, rhel85.NewHostDistro},
{rhel90.New, rhel90.NewHostDistro},
}
type supportedDistro struct {
defaultDistro func() distro.Distro
hostDistro func(name string) distro.Distro
}
type Registry struct {
distros map[string]distro.Distro
hostDistro distro.Distro
}
func New(distros ...distro.Distro) (*Registry, error) {
func New(hostDistro distro.Distro, distros ...distro.Distro) (*Registry, error) {
reg := &Registry{
distros: make(map[string]distro.Distro),
hostDistro: hostDistro,
}
for _, d := range distros {
name := d.Name()
@ -49,11 +56,27 @@ func New(distros ...distro.Distro) (*Registry, error) {
// supportedDistros variable.
func NewDefault() *Registry {
var distros []distro.Distro
for _, distroInitializer := range supportedDistros {
distros = append(distros, distroInitializer())
var hostDistro distro.Distro
// First determine the name of the Host Distro
// If there was an error, then the hostDistroName will be an empty string
// and as a result, the hostDistro will have a nil value when calling New().
// Getting the host distro later using FromHost() will return nil as well.
hostDistroName, hostDistroIsBeta, hostDistroIsStream, _ := distro.GetHostDistroName()
for _, supportedDistro := range supportedDistros {
distro := supportedDistro.defaultDistro()
if distro.Name() == hostDistroName {
hostDistro = supportedDistro.hostDistro(
mangleHostDistroName(distro.Name(), hostDistroIsBeta, hostDistroIsStream),
)
}
registry, err := New(distros...)
distros = append(distros, distro)
}
registry, err := New(hostDistro, distros...)
if err != nil {
panic(fmt.Sprintf("two supported distros have the same name, this is a programming error: %v", err))
}
@ -80,16 +103,26 @@ func (r *Registry) List() []string {
return list
}
func (r *Registry) FromHost() (distro.Distro, bool, bool, error) {
name, beta, isStream, err := distro.GetHostDistroName()
if err != nil {
return nil, false, false, err
func mangleHostDistroName(name string, isBeta, isStream bool) string {
hostDistroName := name
if strings.HasPrefix(hostDistroName, "rhel-8") {
hostDistroName = "rhel-8"
}
if isBeta {
hostDistroName += "-beta"
}
d := r.GetDistro(name)
if d == nil {
return nil, false, false, errors.New("unknown distro: " + name)
// override repository for centos stream, remove when CentOS 8 is EOL
if isStream && hostDistroName == "centos-8" {
hostDistroName = "centos-stream-8"
}
return d, beta, isStream, nil
return hostDistroName
}
// FromHost returns a distro instance, that is specific to the host.
// Its name may differ from other supported distros, if the host version
// is e.g. a Beta or a Stream.
func (r *Registry) FromHost() distro.Distro {
return r.hostDistro
}

View file

@ -5,6 +5,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distro/rhel8"
)
@ -12,8 +13,8 @@ import (
func TestRegistry_List(t *testing.T) {
// build expected distros
var expected []string
for _, distroInitializer := range supportedDistros {
d := distroInitializer()
for _, supportedDistro := range supportedDistros {
d := supportedDistro.defaultDistro()
expected = append(expected, d.Name())
}
@ -34,3 +35,92 @@ func TestRegistry_GetDistro(t *testing.T) {
require.Nil(t, distros.GetDistro("toucan-os"))
})
}
func TestRegistry_mangleHostDistroName(t *testing.T) {
type args struct {
name string
isBeta bool
isStream bool
}
tests := []struct {
name string
args args
want string
}{
{"fedora-32", args{"fedora-32", false, false}, "fedora-32"},
{"fedora-32 beta", args{"fedora-32", true, false}, "fedora-32-beta"},
{"fedora-32 stream", args{"fedora-32", false, true}, "fedora-32"},
{"fedora-32 beta stream", args{"fedora-32", true, true}, "fedora-32-beta"},
{"fedora-33", args{"fedora-33", false, false}, "fedora-33"},
{"fedora-33 beta", args{"fedora-33", true, false}, "fedora-33-beta"},
{"fedora-33 stream", args{"fedora-33", false, true}, "fedora-33"},
{"fedora-33 beta stream", args{"fedora-33", true, true}, "fedora-33-beta"},
{"rhel-8", args{"rhel-8", false, false}, "rhel-8"},
{"rhel-8 beta", args{"rhel-8", true, false}, "rhel-8-beta"},
{"rhel-8 stream", args{"rhel-8", false, true}, "rhel-8"},
{"rhel-8 beta stream", args{"rhel-8", true, true}, "rhel-8-beta"},
{"rhel-84", args{"rhel-84", false, false}, "rhel-8"},
{"rhel-84 beta", args{"rhel-84", true, false}, "rhel-8-beta"},
{"rhel-84 stream", args{"rhel-84", false, true}, "rhel-8"},
{"rhel-84 beta stream", args{"rhel-84", true, true}, "rhel-8-beta"},
{"centos-8", args{"centos-8", false, false}, "centos-8"},
{"centos-8 beta", args{"centos-8", true, false}, "centos-8-beta"},
{"centos-8 stream", args{"centos-8", false, true}, "centos-stream-8"},
{"centos-8 beta stream", args{"centos-8", true, true}, "centos-8-beta"},
{"rhel-90", args{"rhel-90", false, false}, "rhel-90"},
{"rhel-90 beta", args{"rhel-90", true, false}, "rhel-90-beta"},
{"rhel-90 stream", args{"rhel-90", false, true}, "rhel-90"},
{"rhel-90 beta stream", args{"rhel-90", true, true}, "rhel-90-beta"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mangledName := mangleHostDistroName(tt.args.name, tt.args.isBeta, tt.args.isStream)
require.Equalf(
t,
tt.want,
mangledName,
"mangleHostDistroName() name:%s, isBeta:%s, isStream:%s =\nExpected: %s\nGot: %s\n",
tt.args.name,
tt.args.isBeta,
tt.args.isStream,
tt.want,
mangledName,
)
})
}
}
func TestRegistry_FromHost(t *testing.T) {
// expected distros
var distros []distro.Distro
for _, supportedDistro := range supportedDistros {
distros = append(distros, supportedDistro.defaultDistro())
}
t.Run("host distro is nil", func(t *testing.T) {
registry, err := New(nil, distros...)
require.Nil(t, err)
require.NotNil(t, registry)
require.Nil(t, registry.FromHost())
})
t.Run("host distro not nil", func(t *testing.T) {
mangledName := mangleHostDistroName("rhel-8", true, false)
hostDistro := rhel8.NewHostDistro(mangledName)
registry, err := New(hostDistro, distros...)
require.Nil(t, err)
require.NotNil(t, registry)
gotDistro := registry.FromHost()
require.NotNil(t, gotDistro)
require.Equal(t, gotDistro.Name(), mangledName)
})
}

View file

@ -10,5 +10,5 @@ func NewDefaultRegistry() (*distroregistry.Registry, error) {
if testDistro == nil {
panic("Attempt to register test distro failed")
}
return distroregistry.New(testDistro)
return distroregistry.New(nil, testDistro)
}