particle-os/debian-forge-composer
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

distro: panic less often

Browse source 
Return errors from all distro's New() functions instead of logging and
returning nil. Also, return errors instead of panicking from
NewRegistry() and NewDefaultRegistry().
This commit is contained in:
Lars Karlitski 2020-03-02 23:59:12 +01:00
parent d1965d6268
commit 87e9c39532
19 changed files with 128 additions and 62 deletions
Download patch file Download diff file Expand all files Collapse all files

6
cmd/osbuild-composer/main.go
View file

@ -80,7 +80,11 @@ func main() {
}
rpm := rpmmd.NewRPMMD(path.Join(cacheDirectory, "rpmmd"))
distros := distro.NewDefaultRegistry([]string{"/etc/osbuild-composer", "/usr/share/osbuild-composer"})
distros, err := distro.NewDefaultRegistry([]string{"/etc/osbuild-composer", "/usr/share/osbuild-composer"})
if err != nil {
log.Fatalf("Error loading distros: %v", err)
}
distribution, err := distros.FromHost()
if err != nil {

6
cmd/osbuild-pipeline/main.go
View file

@ -64,7 +64,11 @@ func main() {
}
}
distros := distro.NewDefaultRegistry([]string{"."})
distros, err := distro.NewDefaultRegistry([]string{"."})
if err != nil {
panic(err)
}
d := distros.GetDistro(distroArg)
if d == nil {
panic("unknown distro: " + distroArg)

39
internal/distro/distro.go
View file

@ -3,6 +3,7 @@ package distro
import (
"bufio"
"errors"
"fmt"
"io"
"os"
"strings"
@ -70,41 +71,41 @@ type Registry struct {
distros map[common.Distribution]Distro
}
func NewRegistry(distros ...Distro) *Registry {
func NewRegistry(distros ...Distro) (*Registry, error) {
reg := &Registry{
distros: make(map[common.Distribution]Distro),
}
for _, distro := range distros {
distroTag := distro.Distribution()
if _, exists := reg.distros[distroTag]; exists {
panic("a distro with this name already exists: " + distro.Name())
return nil, fmt.Errorf("NewRegistry: passed two distros with the same name: %s", distro.Name())
}
reg.distros[distroTag] = distro
}
return reg
return reg, nil
}
// Create a new Registry containing all known distros.
func NewDefaultRegistry(confPaths []string) *Registry {
f30 := fedora30.New(confPaths)
if f30 == nil {
panic("Attempt to register Fedora 30 failed")
func NewDefaultRegistry(confPaths []string) (*Registry, error) {
f30, err := fedora30.New(confPaths)
if err != nil {
return nil, fmt.Errorf("error loading fedora30: %v", err)
}
f31 := fedora31.New(confPaths)
if f31 == nil {
panic("Attempt to register Fedora 31 failed")
f31, err := fedora31.New(confPaths)
if err != nil {
return nil, fmt.Errorf("error loading fedora31: %v", err)
}
f32 := fedora32.New(confPaths)
if f32 == nil {
panic("Attempt to register Fedora 32 failed")
f32, err := fedora32.New(confPaths)
if err != nil {
return nil, fmt.Errorf("error loading fedora32: %v", err)
}
el81 := rhel81.New(confPaths)
if el81 == nil {
panic("Attempt to register RHEL 8.1 failed")
el81, err := rhel81.New(confPaths)
if err != nil {
return nil, fmt.Errorf("error loading rhel81: %v", err)
}
el82 := rhel82.New(confPaths)
if el82 == nil {
panic("Attempt to register RHEL 8.2 failed")
el82, err := rhel82.New(confPaths)
if err != nil {
return nil, fmt.Errorf("error loading rhel82: %v", err)
}
return NewRegistry(f30, f31, f32, el81, el82)
}

5
internal/distro/distro_test.go
View file

@ -42,7 +42,10 @@ func TestDistro_Pipeline(t *testing.T) {
continue
}
t.Run(tt.Compose.OutputFormat, func(t *testing.T) {
distros := distro.NewDefaultRegistry([]string{"../.."})
distros, err := distro.NewDefaultRegistry([]string{"../.."})
if err != nil {
t.Fatal(err)
}
d := distros.GetDistro(tt.Compose.Distro)
if d == nil {
t.Errorf("unknown distro: %v", tt.Compose.Distro)

8
internal/distro/fedora30/distro.go
View file

@ -2,6 +2,7 @@ package fedora30
import (
"errors"
"fmt"
"log"
"sort"
"strconv"
@ -46,7 +47,7 @@ type output struct {
const Distro = common.Fedora30
const ModulePlatformID = "platform:f30"
func New(confPaths []string) *Fedora30 {
func New(confPaths []string) (*Fedora30, error) {
const GigaByte = 1024 * 1024 * 1024
r := Fedora30{
@ -65,8 +66,7 @@ func New(confPaths []string) *Fedora30 {
repoMap, err := rpmmd.LoadRepositories(confPaths, r.Name())
if err != nil {
log.Printf("Could not load repository data for %s: %s", r.Name(), err.Error())
return nil
return nil, fmt.Errorf("Could not load repository data for %s: %s", r.Name(), err.Error())
}
repos, exists := repoMap["x86_64"]
@ -307,7 +307,7 @@ func New(confPaths []string) *Fedora30 {
},
}
return &r
return &r, nil
}
func (r *Fedora30) Name() string {

11
internal/distro/fedora30/distro_test.go
View file

@ -19,7 +19,11 @@ func TestListOutputFormats(t *testing.T) {
"vmdk",
}
f30 := fedora30.New([]string{"../../../"})
f30, err := fedora30.New([]string{"../../../"})
if err != nil {
t.Fatal(err)
}
if got := f30.ListOutputFormats(); !reflect.DeepEqual(got, want) {
t.Errorf("ListOutputFormats() = %v, want %v", got, want)
}
@ -92,7 +96,10 @@ func TestFilenameFromType(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
f30 := fedora30.New([]string{"../../../"})
f30, err := fedora30.New([]string{"../../../"})
if err != nil {
t.Fatal(err)
}
got, got1, err := f30.FilenameFromType(tt.args.outputFormat)
if (err != nil) != tt.wantErr {
t.Errorf("FilenameFromType() error = %v, wantErr %v", err, tt.wantErr)

8
internal/distro/fedora31/distro.go
View file

@ -2,6 +2,7 @@ package fedora31
import (
"errors"
"fmt"
"log"
"sort"
"strconv"
@ -46,7 +47,7 @@ type output struct {
const Distro = common.Fedora31
const ModulePlatformID = "platform:f31"
func New(confPaths []string) *Fedora31 {
func New(confPaths []string) (*Fedora31, error) {
const GigaByte = 1024 * 1024 * 1024
r := Fedora31{
@ -65,8 +66,7 @@ func New(confPaths []string) *Fedora31 {
repoMap, err := rpmmd.LoadRepositories(confPaths, r.Name())
if err != nil {
log.Printf("Could not load repository data for %s: %s", r.Name(), err.Error())
return nil
return nil, fmt.Errorf("Could not load repository data for %s: %s", r.Name(), err.Error())
}
repos, exists := repoMap["x86_64"]
@ -307,7 +307,7 @@ func New(confPaths []string) *Fedora31 {
},
}
return &r
return &r, nil
}
func (r *Fedora31) Name() string {

11
internal/distro/fedora31/distro_test.go
View file

@ -19,7 +19,11 @@ func TestListOutputFormats(t *testing.T) {
"vmdk",
}
f31 := fedora31.New([]string{"../../../"})
f31, err := fedora31.New([]string{"../../../"})
if err != nil {
t.Fatal(err)
}
if got := f31.ListOutputFormats(); !reflect.DeepEqual(got, want) {
t.Errorf("ListOutputFormats() = %v, want %v", got, want)
}
@ -92,7 +96,10 @@ func TestFilenameFromType(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
f31 := fedora31.New([]string{"../../../"})
f31, err := fedora31.New([]string{"../../../"})
if err != nil {
t.Fatal(err)
}
got, got1, err := f31.FilenameFromType(tt.args.outputFormat)
if (err != nil) != tt.wantErr {
t.Errorf("FilenameFromType() error = %v, wantErr %v", err, tt.wantErr)

8
internal/distro/fedora32/distro.go
View file

@ -2,6 +2,7 @@ package fedora32
import (
"errors"
"fmt"
"log"
"sort"
"strconv"
@ -46,7 +47,7 @@ type output struct {
const Distro = common.Fedora32
const ModulePlatformID = "platform:f32"
func New(confPaths []string) *Fedora32 {
func New(confPaths []string) (*Fedora32, error) {
const GigaByte = 1024 * 1024 * 1024
r := Fedora32{
@ -65,8 +66,7 @@ func New(confPaths []string) *Fedora32 {
repoMap, err := rpmmd.LoadRepositories(confPaths, r.Name())
if err != nil {
log.Printf("Could not load repository data for %s: %s", r.Name(), err.Error())
return nil
return nil, fmt.Errorf("Could not load repository data for %s: %s", r.Name(), err.Error())
}
repos, exists := repoMap["x86_64"]
@ -307,7 +307,7 @@ func New(confPaths []string) *Fedora32 {
},
}
return &r
return &r, nil
}
func (r *Fedora32) Name() string {

11
internal/distro/fedora32/distro_test.go
View file

@ -19,7 +19,11 @@ func TestListOutputFormats(t *testing.T) {
"vmdk",
}
f32 := fedora32.New([]string{"../../../"})
f32, err := fedora32.New([]string{"../../../"})
if err != nil {
t.Fatal(err)
}
if got := f32.ListOutputFormats(); !reflect.DeepEqual(got, want) {
t.Errorf("ListOutputFormats() = %v, want %v", got, want)
}
@ -92,7 +96,10 @@ func TestFilenameFromType(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
f32 := fedora32.New([]string{"../../../"})
f32, err := fedora32.New([]string{"../../../"})
if err != nil {
t.Fatal(err)
}
got, got1, err := f32.FilenameFromType(tt.args.outputFormat)
if (err != nil) != tt.wantErr {
t.Errorf("FilenameFromType() error = %v, wantErr %v", err, tt.wantErr)

8
internal/distro/rhel81/distro.go
View file

@ -2,6 +2,7 @@ package rhel81
import (
"errors"
"fmt"
"log"
"sort"
"strconv"
@ -47,7 +48,7 @@ type output struct {
const Distro = common.RHEL81
const ModulePlatformID = "platform:el8"
func New(confPaths []string) *RHEL81 {
func New(confPaths []string) (*RHEL81, error) {
const GigaByte = 1024 * 1024 * 1024
r := RHEL81{
@ -70,8 +71,7 @@ func New(confPaths []string) *RHEL81 {
repoMap, err := rpmmd.LoadRepositories(confPaths, r.Name())
if err != nil {
log.Printf("Could not load repository data for %s: %s", r.Name(), err.Error())
return nil
return nil, fmt.Errorf("Could not load repository data for %s: %s", r.Name(), err.Error())
}
repos, exists := repoMap["x86_64"]
@ -446,7 +446,7 @@ func New(confPaths []string) *RHEL81 {
},
}
return &r
return &r, nil
}
func (r *RHEL81) Name() string {

11
internal/distro/rhel81/distro_test.go
View file

@ -19,7 +19,11 @@ func TestListOutputFormats(t *testing.T) {
"vmdk",
}
el81 := rhel81.New([]string{"../../../"})
el81, err := rhel81.New([]string{"../../../"})
if err != nil {
t.Fatal(err)
}
if got := el81.ListOutputFormats(); !reflect.DeepEqual(got, want) {
t.Errorf("ListOutputFormats() = %v, want %v", got, want)
}
@ -92,7 +96,10 @@ func TestFilenameFromType(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
el81 := rhel81.New([]string{"../../../"})
el81, err := rhel81.New([]string{"../../../"})
if err != nil {
t.Fatal(err)
}
got, got1, err := el81.FilenameFromType(tt.args.outputFormat)
if (err != nil) != tt.wantErr {
t.Errorf("FilenameFromType() error = %v, wantErr %v", err, tt.wantErr)

8
internal/distro/rhel82/distro.go
View file

@ -2,6 +2,7 @@ package rhel82
import (
"errors"
"fmt"
"log"
"sort"
"strconv"
@ -47,7 +48,7 @@ type output struct {
const Distro = common.RHEL82
const ModulePlatformID = "platform:el8"
func New(confPaths []string) *RHEL82 {
func New(confPaths []string) (*RHEL82, error) {
const GigaByte = 1024 * 1024 * 1024
r := RHEL82{
@ -70,8 +71,7 @@ func New(confPaths []string) *RHEL82 {
repoMap, err := rpmmd.LoadRepositories(confPaths, r.Name())
if err != nil {
log.Printf("Could not load repository data for %s: %s", r.Name(), err.Error())
return nil
return nil, fmt.Errorf("Could not load repository data for %s: %s", r.Name(), err.Error())
}
repos, exists := repoMap["x86_64"]
@ -446,7 +446,7 @@ func New(confPaths []string) *RHEL82 {
},
}
return &r
return &r, nil
}
func (r *RHEL82) Name() string {

11
internal/distro/rhel82/distro_test.go
View file

@ -19,7 +19,11 @@ func TestListOutputFormats(t *testing.T) {
"vmdk",
}
el82 := rhel82.New([]string{"../../../"})
el82, err := rhel82.New([]string{"../../../"})
if err != nil {
t.Fatal(err)
}
if got := el82.ListOutputFormats(); !reflect.DeepEqual(got, want) {
t.Errorf("ListOutputFormats() = %v, want %v", got, want)
}
@ -92,7 +96,10 @@ func TestFilenameFromType(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
el82 := rhel82.New([]string{"../../../"})
el82, err := rhel82.New([]string{"../../../"})
if err != nil {
t.Fatal(err)
}
got, got1, err := el82.FilenameFromType(tt.args.outputFormat)
if (err != nil) != tt.wantErr {
t.Errorf("FilenameFromType() error = %v, wantErr %v", err, tt.wantErr)

17
internal/jobqueue/api_test.go
View file

@ -34,7 +34,10 @@ func TestBasic(t *testing.T) {
for _, c := range cases {
distroStruct := test_distro.New()
registry := distro_mock.NewDefaultRegistry()
registry, err := distro_mock.NewDefaultRegistry()
if err != nil {
t.Fatal(err)
}
api := jobqueue.New(nil, store.New(nil, distroStruct, *registry))
test.TestNonJsonRoute(t, api, false, c.Method, c.Path, c.Body, c.ExpectedStatus, c.ExpectedResponse)
@ -44,11 +47,14 @@ func TestBasic(t *testing.T) {
func TestCreate(t *testing.T) {
id, _ := uuid.Parse("ffffffff-ffff-ffff-ffff-ffffffffffff")
distroStruct := test_distro.New()
registry := distro_mock.NewDefaultRegistry()
registry, err := distro_mock.NewDefaultRegistry()
if err != nil {
t.Fatal(err)
}
store := store.New(nil, distroStruct, *registry)
api := jobqueue.New(nil, store)
err := store.PushCompose(id, &blueprint.Blueprint{}, nil, nil, map[string]string{"test-repo": "test:foo"}, "x86_64", "qcow2", 0, nil)
err = store.PushCompose(id, &blueprint.Blueprint{}, nil, nil, map[string]string{"test-repo": "test:foo"}, "x86_64", "qcow2", 0, nil)
if err != nil {
t.Fatalf("error pushing compose: %v", err)
}
@ -60,7 +66,10 @@ func TestCreate(t *testing.T) {
func testUpdateTransition(t *testing.T, from, to string, expectedStatus int, expectedResponse string) {
id, _ := uuid.Parse("ffffffff-ffff-ffff-ffff-ffffffffffff")
distroStruct := test_distro.New()
registry := distro_mock.NewDefaultRegistry()
registry, err := distro_mock.NewDefaultRegistry()
if err != nil {
t.Fatal(err)
}
store := store.New(nil, distroStruct, *registry)
api := jobqueue.New(nil, store)

6
internal/jobqueue/job.go
View file

@ -46,7 +46,11 @@ func (e *TargetsError) Error() string {
}
func (job *Job) Run(uploader LocalTargetUploader) (*common.ComposeResult, error) {
distros := distro.NewDefaultRegistry([]string{"/etc/osbuild-composer", "/usr/share/osbuild-composer"})
distros, err := distro.NewDefaultRegistry([]string{"/etc/osbuild-composer", "/usr/share/osbuild-composer"})
if err != nil {
return nil, fmt.Errorf("error loading distros: %v", err)
}
d := distros.GetDistro(job.Distro)
if d == nil {
return nil, fmt.Errorf("unknown distro: %s", job.Distro)

2
internal/mocks/distro/distro_mock.go
View file

@ -5,7 +5,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/distro/fedoratest"
)
func NewDefaultRegistry() *distro.Registry {
func NewDefaultRegistry() (*distro.Registry, error) {
ftest := fedoratest.New()
if ftest == nil {
panic("Attempt to register Fedora test failed")

4
internal/mocks/rpmmd/fixtures.go
View file

@ -94,7 +94,7 @@ func createBaseStoreFixture() *store.Store {
}
d := test_distro.New()
r := distro_mock.NewDefaultRegistry()
r, _ := distro_mock.NewDefaultRegistry()
s := store.New(nil, d, *r)
s.Blueprints[bName] = b
@ -191,7 +191,7 @@ func createStoreWithoutComposesFixture() *store.Store {
}
d := test_distro.New()
r := distro_mock.NewDefaultRegistry()
r, _ := distro_mock.NewDefaultRegistry()
s := store.New(nil, d, *r)
s.Blueprints[bName] = b

10
internal/rcm/api_test.go
View file

@ -49,7 +49,10 @@ func TestBasicRcmAPI(t *testing.T) {
{"GET", "/v1/compose/7802c476-9cd1-41b7-ba81-43c1906bce73", `{"status":"RUNNING"}`, "application/json", http.StatusBadRequest, `{"error_reason":"Compose UUID does not exist"}`},
}
registry := distro_mock.NewDefaultRegistry()
registry, err := distro_mock.NewDefaultRegistry()
if err != nil {
t.Fatal(err)
}
distroStruct := fedoratest.New()
api := rcm.New(nil, store.New(nil, distroStruct, *registry), rpmmd_mock.NewRPMMDMock(rpmmd_mock.BaseFixture()))
@ -74,7 +77,10 @@ func TestBasicRcmAPI(t *testing.T) {
func TestSubmitCompose(t *testing.T) {
// Test the most basic use case: Submit a new job and get its status.
distroStruct := fedoratest.New()
registry := distro_mock.NewDefaultRegistry()
registry, err := distro_mock.NewDefaultRegistry()
if err != nil {
t.Fatal(err)
}
api := rcm.New(nil, store.New(nil, distroStruct, *registry), rpmmd_mock.NewRPMMDMock(rpmmd_mock.BaseFixture()))
var submit_reply struct {