distro: move Registry to its own distroregistry package

My goal is to add a method to distroregistry to return Registry with
all supported distributions. This way, all supported distributions
would be defined only on one place.

To achieve this, the Registry must live outside the distro package
because the distro implementation depends on it and this would create
a circular dependency unsupported by Go.

Signed-off-by: Ondřej Budai <ondrej@budai.cz>
This commit is contained in:
Ondřej Budai 2021-03-11 12:30:58 +01:00 committed by Ondřej Budai
parent b5a1d89e45
commit dd4db353e2
10 changed files with 109 additions and 86 deletions

View file

@ -14,6 +14,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/cloudapi"
"github.com/osbuild/osbuild-composer/internal/common"
"github.com/osbuild/osbuild-composer/internal/distroregistry"
"github.com/osbuild/osbuild-composer/internal/jobqueue/fsjobqueue"
"github.com/osbuild/osbuild-composer/internal/kojiapi"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
@ -21,7 +22,6 @@ import (
"github.com/osbuild/osbuild-composer/internal/weldr"
"github.com/osbuild/osbuild-composer/internal/worker"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distro/fedora32"
"github.com/osbuild/osbuild-composer/internal/distro/fedora33"
"github.com/osbuild/osbuild-composer/internal/distro/rhel8"
@ -33,7 +33,7 @@ type Composer struct {
stateDir string
cacheDir string
logger *log.Logger
distros *distro.Registry
distros *distroregistry.Registry
rpm rpmmd.RPMMD
@ -63,7 +63,7 @@ func NewComposer(config *ComposerConfigFile, stateDir, cacheDir string, logger *
return nil, err
}
c.distros, err = distro.NewRegistry(fedora32.New(), fedora33.New(), rhel8.New(), rhel84.New(), rhel84.NewCentos())
c.distros, err = distroregistry.New(fedora32.New(), fedora33.New(), rhel8.New(), rhel84.New(), rhel84.NewCentos())
if err != nil {
return nil, fmt.Errorf("Error loading distros: %v", err)
}

View file

@ -13,6 +13,7 @@ import (
"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/distroregistry"
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/distro"
@ -67,7 +68,7 @@ func main() {
}
}
distros, err := distro.NewRegistry(fedora32.New(), fedora33.New(), rhel8.New(), rhel84.New(), rhel84.NewCentos())
distros, err := distroregistry.New(fedora32.New(), fedora33.New(), rhel8.New(), rhel84.New(), rhel84.NewCentos())
if err != nil {
panic(err)
}

View file

@ -15,6 +15,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distroregistry"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/osbuild/osbuild-composer/internal/target"
"github.com/osbuild/osbuild-composer/internal/worker"
@ -31,11 +32,11 @@ const (
type Server struct {
workers *worker.Server
rpmMetadata rpmmd.RPMMD
distros *distro.Registry
distros *distroregistry.Registry
}
// NewServer creates a new cloud server
func NewServer(workers *worker.Server, rpmMetadata rpmmd.RPMMD, distros *distro.Registry) *Server {
func NewServer(workers *worker.Server, rpmMetadata rpmmd.RPMMD, distros *distroregistry.Registry) *Server {
server := &Server{
workers: workers,
rpmMetadata: rpmMetadata,

View file

@ -8,7 +8,6 @@ import (
"io"
"io/ioutil"
"os"
"sort"
"strings"
"github.com/osbuild/osbuild-composer/internal/blueprint"
@ -125,57 +124,6 @@ func (m *Manifest) UnmarshalJSON(payload []byte) error {
return nil
}
type Registry struct {
distros map[string]Distro
}
func NewRegistry(distros ...Distro) (*Registry, error) {
reg := &Registry{
distros: make(map[string]Distro),
}
for _, distro := range distros {
name := distro.Name()
if _, exists := reg.distros[name]; exists {
return nil, fmt.Errorf("NewRegistry: passed two distros with the same name: %s", distro.Name())
}
reg.distros[name] = distro
}
return reg, nil
}
func (r *Registry) GetDistro(name string) Distro {
distro, ok := r.distros[name]
if !ok {
return nil
}
return distro
}
// List returns the names of all distros in a Registry, sorted alphabetically.
func (r *Registry) List() []string {
list := []string{}
for _, distro := range r.distros {
list = append(list, distro.Name())
}
sort.Strings(list)
return list
}
func (r *Registry) FromHost() (Distro, bool, bool, error) {
name, beta, isStream, err := GetHostDistroName()
if err != nil {
return nil, false, false, err
}
d := r.GetDistro(name)
if d == nil {
return nil, false, false, errors.New("unknown distro: " + name)
}
return d, beta, isStream, nil
}
func GetHostDistroName() (string, bool, bool, error) {
f, err := os.Open("/etc/os-release")
if err != nil {

View file

@ -3,9 +3,6 @@ package distro_test
import (
"testing"
"github.com/stretchr/testify/require"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distro/distro_test_common"
"github.com/osbuild/osbuild-composer/internal/distro/fedora32"
"github.com/osbuild/osbuild-composer/internal/distro/fedora33"
@ -21,19 +18,3 @@ func TestDistro_Manifest(t *testing.T) {
fedora32.New(), fedora33.New(), rhel8.New(), rhel84.New(), rhel84.NewCentos(),
)
}
// Test that all distros are registered properly and that Registry.List() works.
func TestDistro_RegistryList(t *testing.T) {
expected := []string{
"centos-8",
"fedora-32",
"fedora-33",
"rhel-8",
"rhel-84",
}
distros, err := distro.NewRegistry(fedora32.New(), fedora33.New(), rhel8.New(), rhel84.New(), rhel84.NewCentos())
require.NoError(t, err)
require.Equalf(t, expected, distros.List(), "unexpected list of distros")
}

View file

@ -9,11 +9,13 @@ import (
"testing"
"github.com/google/go-cmp/cmp"
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distroregistry"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
)
const RandomTestSeed = 0
@ -64,7 +66,7 @@ func TestDistro_Manifest(t *testing.T, pipelinePath string, prefix string, distr
}
}
t.Run(path.Base(fileName), func(t *testing.T) {
distros, err := distro.NewRegistry(distros...)
distros, err := distroregistry.New(distros...)
require.NoError(t, err)
d := distros.GetDistro(tt.ComposeRequest.Distro)
if d == nil {

View file

@ -0,0 +1,60 @@
package distroregistry
import (
"errors"
"fmt"
"sort"
"github.com/osbuild/osbuild-composer/internal/distro"
)
type Registry struct {
distros map[string]distro.Distro
}
func New(distros ...distro.Distro) (*Registry, error) {
reg := &Registry{
distros: make(map[string]distro.Distro),
}
for _, d := range distros {
name := d.Name()
if _, exists := reg.distros[name]; exists {
return nil, fmt.Errorf("New: passed two distros with the same name: %s", d.Name())
}
reg.distros[name] = d
}
return reg, nil
}
func (r *Registry) GetDistro(name string) distro.Distro {
d, ok := r.distros[name]
if !ok {
return nil
}
return d
}
// List returns the names of all distros in a Registry, sorted alphabetically.
func (r *Registry) List() []string {
list := []string{}
for _, d := range r.distros {
list = append(list, d.Name())
}
sort.Strings(list)
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
}
d := r.GetDistro(name)
if d == nil {
return nil, false, false, errors.New("unknown distro: " + name)
}
return d, beta, isStream, nil
}

View file

@ -0,0 +1,29 @@
package distroregistry_test
import (
"testing"
"github.com/stretchr/testify/require"
"github.com/osbuild/osbuild-composer/internal/distro/fedora32"
"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/distroregistry"
)
// Test that all distros are registered properly and that Registry.List() works.
func TestRegistry_List(t *testing.T) {
expected := []string{
"centos-8",
"fedora-32",
"fedora-33",
"rhel-8",
"rhel-84",
}
distros, err := distroregistry.New(fedora32.New(), fedora33.New(), rhel8.New(), rhel84.New(), rhel84.NewCentos())
require.NoError(t, err)
require.Equalf(t, expected, distros.List(), "unexpected list of distros")
}

View file

@ -17,6 +17,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distroregistry"
"github.com/osbuild/osbuild-composer/internal/kojiapi/api"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/osbuild/osbuild-composer/internal/worker"
@ -27,11 +28,11 @@ type Server struct {
logger *log.Logger
workers *worker.Server
rpmMetadata rpmmd.RPMMD
distros *distro.Registry
distros *distroregistry.Registry
}
// NewServer creates a new koji server
func NewServer(logger *log.Logger, workers *worker.Server, rpmMetadata rpmmd.RPMMD, distros *distro.Registry) *Server {
func NewServer(logger *log.Logger, workers *worker.Server, rpmMetadata rpmmd.RPMMD, distros *distroregistry.Registry) *Server {
s := &Server{
logger: logger,
workers: workers,

View file

@ -1,14 +1,14 @@
package distro_mock
import (
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distro/fedoratest"
"github.com/osbuild/osbuild-composer/internal/distroregistry"
)
func NewDefaultRegistry() (*distro.Registry, error) {
func NewDefaultRegistry() (*distroregistry.Registry, error) {
ftest := fedoratest.New()
if ftest == nil {
panic("Attempt to register Fedora test failed")
}
return distro.NewRegistry(ftest)
return distroregistry.New(ftest)
}