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

store: Fix loading cross distro compose results

Browse source 
When the store is written to disk it simplifies the ImageBuild details
into a simple image type string. This works fine for composes that match
the host's distro but isn't enough detail to load composes made for
other distros, especially if the image type name isn't supported on the
host. This results in cross distro compose results being lost after a
reboot.

This fix uses the distro information from the compose's blueprint to
determine which distro the image type should be loaded from. It assumes
that the architecture matches the hosts' arch -- this is currently
always true but in the future if cross-arch builds are added it will
need to be addressed in a different way.

newComposeFromV0, newComposesFromV0, and newStoreFromV0 now take a
pointer to the full distro registry instead of an Arch, this allows them
to access the correct image types for the distro selected by the
blueprint. When loading the composes from disk the blueprint distro is
loaded from the registry before checking the image type string.

This means that we do not have to change the store version or on disk
format, the only thing changing is how it decides to populate the
ImageBuild when reloading the store.

A number of tests use a fake test distro using fake architecture names.
These tests have been adjusted to use a fake distro registry with
overridden host architecture that matches the fake one.
This commit is contained in:
Brian C. Lane 2022-10-18 14:23:54 -07:00 • committed by Ondřej Budai
parent bcb927cf63
commit 1096003598
8 changed files with 106 additions and 55 deletions
Download patch file Download diff file Expand all files Collapse all files

4
cmd/osbuild-store-dump/main.go
View file

@ -12,6 +12,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distro/fedora"
"github.com/osbuild/osbuild-composer/internal/distroregistry"
"github.com/osbuild/osbuild-composer/internal/dnfjson"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/osbuild/osbuild-composer/internal/store"
@ -142,7 +143,8 @@ func main() {
}
rpmmdCache := path.Join(homeDir, ".cache/osbuild-composer/rpmmd")
s := store.New(&cwd, a, nil)
dr, _ := distroregistry.New(d, nil)
s := store.New(&cwd, dr, nil)
if s == nil {
panic("could not create store")
}

13
internal/distro/test_distro/distro.go
View file

@ -9,6 +9,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/container"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distroregistry"
"github.com/osbuild/osbuild-composer/internal/osbuild"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
)
@ -344,6 +345,18 @@ func New() *TestDistro {
return newTestDistro(TestDistroName, TestDistroModulePlatformID, TestDistroReleasever)
}
func NewRegistry() *distroregistry.Registry {
td := New()
registry, err := distroregistry.New(td, td)
if err != nil {
panic(err)
}
// Override the host's architecture name with the test's name
registry.SetHostArchName(TestArchName)
return registry
}
// New2 returns new instance of TestDistro named "test-distro-2".
func New2() *TestDistro {
return newTestDistro(TestDistro2Name, TestDistro2ModulePlatformID, TestDistro2Releasever)

26
internal/store/fixtures.go
View file

@ -41,7 +41,8 @@ func FixtureBase() *Store {
},
}
d := test_distro.New()
dr := test_distro.NewRegistry()
d := dr.FromHost()
arch, err := d.GetArch(test_distro.TestArchName)
if err != nil {
panic(fmt.Sprintf("failed to get architecture %s for a test distro: %v", test_distro.TestArchName, err))
@ -54,7 +55,8 @@ func FixtureBase() *Store {
if err != nil {
panic(fmt.Sprintf("failed to create a manifest: %v", err))
}
s := New(nil, arch, nil)
s := New(nil, dr, nil)
pkgs := []rpmmd.PackageSpec{
{
@ -176,7 +178,8 @@ func FixtureFinished() *Store {
},
}
d := test_distro.New()
dr := test_distro.NewRegistry()
d := dr.FromHost()
arch, err := d.GetArch(test_distro.TestArchName)
if err != nil {
panic(fmt.Sprintf("failed to get architecture %s for a test distro: %v", test_distro.TestArchName, err))
@ -189,7 +192,8 @@ func FixtureFinished() *Store {
if err != nil {
panic(fmt.Sprintf("failed to create a manifest: %v", err))
}
s := New(nil, arch, nil)
s := New(nil, dr, nil)
pkgs := []rpmmd.PackageSpec{
{
@ -272,13 +276,14 @@ func FixtureEmpty() *Store {
Customizations: nil,
}
d := test_distro.New()
arch, err := d.GetArch(test_distro.TestArchName)
dr := test_distro.NewRegistry()
d := dr.FromHost()
_, err := d.GetArch(test_distro.TestArchName)
if err != nil {
panic(fmt.Sprintf("failed to get architecture %s for a test distro: %v", test_distro.TestArchName, err))
}
s := New(nil, arch, nil)
s := New(nil, dr, nil)
s.blueprints[bName] = b
@ -310,13 +315,14 @@ func FixtureOldChanges() *Store {
Customizations: nil,
}
d := test_distro.New()
arch, err := d.GetArch(test_distro.TestArchName)
dr := test_distro.NewRegistry()
d := dr.FromHost()
_, err := d.GetArch(test_distro.TestArchName)
if err != nil {
panic(fmt.Sprintf("failed to get architecture %s for a test distro: %v", test_distro.TestArchName, err))
}
s := New(nil, arch, nil)
s := New(nil, dr, nil)
s.PushBlueprint(b, "Initial commit")
b.Version = "0.0.1"

31
internal/store/json.go
View file

@ -2,6 +2,7 @@ package store
import (
"errors"
"fmt"
"log"
"sort"
"time"
@ -10,6 +11,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/distroregistry"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/osbuild/osbuild-composer/internal/target"
)
@ -96,11 +98,11 @@ func newWorkspaceFromV0(workspaceStruct workspaceV0) map[string]blueprint.Bluepr
return workspace
}
func newComposesFromV0(composesStruct composesV0, arch distro.Arch, log *log.Logger) map[uuid.UUID]Compose {
func newComposesFromV0(composesStruct composesV0, dr *distroregistry.Registry, log *log.Logger) map[uuid.UUID]Compose {
composes := make(map[uuid.UUID]Compose)
for composeID, composeStruct := range composesStruct {
c, err := newComposeFromV0(composeStruct, arch)
c, err := newComposeFromV0(composeStruct, dr)
if err != nil {
if log != nil {
log.Printf("ignoring compose: %v", err)
@ -143,15 +145,32 @@ func newImageBuildFromV0(imageBuildStruct imageBuildV0, arch distro.Arch) (Image
}, nil
}
func newComposeFromV0(composeStruct composeV0, arch distro.Arch) (Compose, error) {
func newComposeFromV0(composeStruct composeV0, dr *distroregistry.Registry) (Compose, error) {
if len(composeStruct.ImageBuilds) != 1 {
return Compose{}, errors.New("compose with unsupported number of image builds")
}
// Get the distro from the blueprint (empty means use host distro)
bp := composeStruct.Blueprint.DeepCopy()
distroName := bp.Distro
if len(distroName) == 0 {
distroName = dr.FromHost().Name()
}
distro := dr.GetDistro(distroName)
if distro == nil {
return Compose{}, fmt.Errorf("Unknown distro - %s", distroName)
}
// Get the host distro's architecture. This contains the distro+arch specific image types
arch, err := distro.GetArch(dr.HostArchName())
if err != nil {
return Compose{}, err
}
ib, err := newImageBuildFromV0(composeStruct.ImageBuilds[0], arch)
if err != nil {
return Compose{}, err
}
bp := composeStruct.Blueprint.DeepCopy()
pkgs := make([]rpmmd.PackageSpec, len(composeStruct.Packages))
copy(pkgs, composeStruct.Packages)
@ -234,11 +253,11 @@ func newCommitsFromV0(commitsMapStruct commitsV0, changesMapStruct changesV0) ma
return commitsMap
}
func newStoreFromV0(storeStruct storeV0, arch distro.Arch, log *log.Logger) *Store {
func newStoreFromV0(storeStruct storeV0, dr *distroregistry.Registry, log *log.Logger) *Store {
return &Store{
blueprints: newBlueprintsFromV0(storeStruct.Blueprints),
workspace: newWorkspaceFromV0(storeStruct.Workspace),
composes: newComposesFromV0(storeStruct.Composes, arch, log),
composes: newComposesFromV0(storeStruct.Composes, dr, log),
sources: newSourceConfigsFromV0(storeStruct.Sources),
blueprintsChanges: newChangesFromV0(storeStruct.Changes),
blueprintsCommits: newCommitsFromV0(storeStruct.Commits, storeStruct.Changes),

50
internal/store/json_test.go
View file

@ -15,6 +15,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distro/fedora"
"github.com/osbuild/osbuild-composer/internal/distro/test_distro"
"github.com/osbuild/osbuild-composer/internal/distroregistry"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/osbuild/osbuild-composer/internal/target"
"github.com/stretchr/testify/assert"
@ -114,13 +115,14 @@ func Test_imageTypeFromCompatString(t *testing.T) {
func TestMarshalEmpty(t *testing.T) {
d := test_distro.New()
arch, err := d.GetArch(test_distro.TestArchName)
_, err := d.GetArch(test_distro.TestArchName)
if err != nil {
panic(fmt.Sprintf("failed to get architecture %s for a test distro: %v", test_distro.TestArchName, err))
}
store1 := FixtureEmpty()
storeV0 := store1.toStoreV0()
store2 := newStoreFromV0(*storeV0, arch, nil)
dr := test_distro.NewRegistry()
store2 := newStoreFromV0(*storeV0, dr, nil)
if !reflect.DeepEqual(store1, store2) {
t.Errorf("marshal/unmarshal roundtrip not a noop for empty store:\n got: %#v\n want: %#v", store1, store2)
}
@ -128,13 +130,14 @@ func TestMarshalEmpty(t *testing.T) {
func TestMarshalFinished(t *testing.T) {
d := test_distro.New()
arch, err := d.GetArch(test_distro.TestArchName)
_, err := d.GetArch(test_distro.TestArchName)
if err != nil {
panic(fmt.Sprintf("failed to get architecture %s for a test distro: %v", test_distro.TestArchName, err))
}
store1 := FixtureFinished()
storeV0 := store1.toStoreV0()
store2 := newStoreFromV0(*storeV0, arch, nil)
dr := test_distro.NewRegistry()
store2 := newStoreFromV0(*storeV0, dr, nil)
if !reflect.DeepEqual(store1, store2) {
t.Errorf("marshal/unmarshal roundtrip not a noop for base store:\n got: %#v\n want: %#v", store2, store1)
}
@ -187,11 +190,12 @@ func TestStore_toStoreV0(t *testing.T) {
func Test_newStoreFromV0(t *testing.T) {
type args struct {
storeStruct storeV0
arch distro.Arch
registry *distroregistry.Registry
}
testDistro := test_distro.New()
testArch, _ := testDistro.GetArch(test_distro.TestArchName)
//testDistro := test_distro.New()
// testArch, _ := testDistro.GetArch(test_distro.TestArchName)
dr := test_distro.NewRegistry()
tests := []struct {
name string
@ -202,14 +206,14 @@ func Test_newStoreFromV0(t *testing.T) {
name: "empty",
args: args{
storeStruct: storeV0{},
arch: testArch,
registry: dr,
},
want: New(nil, testArch, nil),
want: New(nil, dr, nil),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := newStoreFromV0(tt.args.storeStruct, tt.args.arch, nil); !reflect.DeepEqual(got, tt.want) {
if got := newStoreFromV0(tt.args.storeStruct, tt.args.registry, nil); !reflect.DeepEqual(got, tt.want) {
t.Errorf("newStoreFromV0() =\n got: %#v\n want: %#v", got, tt.want)
}
})
@ -303,9 +307,13 @@ 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 := fedora.NewF35().GetArch(distro.X86_64ArchName)
f35 := fedora.NewF35()
registry, err := distroregistry.New(f35, f35)
assert.NoError(err)
store := newStoreFromV0(storeStruct, arch, nil)
// The test data has image types only supported on Fedora X86_64
registry.SetHostArchName(distro.X86_64ArchName)
store := newStoreFromV0(storeStruct, registry, nil)
assert.Equal(1, len(store.blueprints))
assert.Equal(1, len(store.blueprintsChanges))
assert.Equal(1, len(store.blueprintsCommits))
@ -1094,24 +1102,25 @@ func Test_newComposeFromV0(t *testing.T) {
testDistro := test_distro.New()
testArch, _ := testDistro.GetArch(test_distro.TestArchName)
testImageType, _ := testArch.GetImageType(test_distro.TestImageTypeName)
dr := test_distro.NewRegistry()
tests := []struct {
name string
compose composeV0
arch distro.Arch
registry *distroregistry.Registry
want Compose
errOk bool
}{
{
name: "empty",
compose: composeV0{},
arch: testArch,
registry: dr,
want: Compose{},
errOk: true,
},
{
name: "qcow2 compose",
arch: testArch,
registry: dr,
errOk: false,
compose: composeV0{
Blueprint: &bp,
@ -1182,7 +1191,7 @@ func Test_newComposeFromV0(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := newComposeFromV0(tt.compose, tt.arch)
got, err := newComposeFromV0(tt.compose, tt.registry)
if err != nil {
if !tt.errOk {
t.Errorf("newComposeFromV0() error = %v", err)
@ -1371,22 +1380,23 @@ func Test_newComposesFromV0(t *testing.T) {
testDistro := test_distro.New()
testArch, _ := testDistro.GetArch(test_distro.TestArchName)
testImageType, _ := testArch.GetImageType(test_distro.TestImageTypeName)
dr := test_distro.NewRegistry()
tests := []struct {
name string
arch distro.Arch
registry *distroregistry.Registry
composes composesV0
want map[uuid.UUID]Compose
}{
{
name: "empty",
arch: testArch,
registry: dr,
composes: composesV0{},
want: make(map[uuid.UUID]Compose),
},
{
name: "two composes",
arch: testArch,
registry: dr,
composes: composesV0{
uuid.MustParse("f53b49c0-d321-447e-8ab8-6e827891e3f0"): {
Blueprint: &bp,
@ -1525,7 +1535,7 @@ func Test_newComposesFromV0(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := newComposesFromV0(tt.composes, tt.arch, nil); !reflect.DeepEqual(got, tt.want) {
if got := newComposesFromV0(tt.composes, tt.registry, nil); !reflect.DeepEqual(got, tt.want) {
t.Errorf("newComposesFromV0() =\n got: %#v\n want: %#v", got, tt.want)
}
})

5
internal/store/store.go
View file

@ -20,6 +20,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/common"
"github.com/osbuild/osbuild-composer/internal/distroregistry"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/osbuild/osbuild-composer/internal/target"
@ -79,7 +80,7 @@ func (e *NoLocalTargetError) Error() string {
return e.message
}
func New(stateDir *string, arch distro.Arch, log *log.Logger) *Store {
func New(stateDir *string, dr *distroregistry.Registry, log *log.Logger) *Store {
var storeStruct storeV0
var db *jsondb.JSONDatabase
@ -91,7 +92,7 @@ func New(stateDir *string, arch distro.Arch, log *log.Logger) *Store {
}
}
store := newStoreFromV0(storeStruct, arch, log)
store := newStoreFromV0(storeStruct, dr, log)
store.stateDir = stateDir
store.db = db

5
internal/store/store_test.go
View file

@ -133,10 +133,11 @@ func (suite *storeTest) SetupSuite() {
//setup before each test
func (suite *storeTest) SetupTest() {
distro := test_distro.New()
arch, err := distro.GetArch(test_distro.TestArchName)
_, err := distro.GetArch(test_distro.TestArchName)
suite.NoError(err)
suite.dir = suite.T().TempDir()
suite.myStore = New(&suite.dir, arch, nil)
dr := test_distro.NewRegistry()
suite.myStore = New(&suite.dir, dr, nil)
}
func (suite *storeTest) TestRandomSHA1String() {

5
internal/weldr/api.go
View file

@ -164,13 +164,12 @@ func New(repoPaths []string, stateDir string, solver *dnfjson.BaseSolver, dr *di
return nil, fmt.Errorf("error loading repository definitions: %v", err)
}
var hostArch distro.Arch
hostDistro := dr.GetDistro(hostDistroName)
if hostDistro != nil {
// get canonical distro name if the host distro is supported
hostDistroName = hostDistro.Name()
hostArch, err = hostDistro.GetArch(archName)
_, err = hostDistro.GetArch(archName)
if err != nil {
return nil, fmt.Errorf("Host distro does not support host architecture: %v", err)
}
@ -185,7 +184,7 @@ func New(repoPaths []string, stateDir string, solver *dnfjson.BaseSolver, dr *di
log.Printf("host distro %q is not supported: only cross-distro builds are available", hostDistroName)
}
store := store.New(&stateDir, hostArch, logger)
store := store.New(&stateDir, dr, logger)
compatOutputDir := path.Join(stateDir, "outputs")
api := &API{