From 50d469fe45a8ae77cbc37e8ae2e7bd8300d23a70 Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Fri, 19 Jun 2020 15:12:04 +0200 Subject: [PATCH] distro: replace BasePackages() with Packages() Rather than getting a set of base packages from the ImageType, and then appending the requested packages from the blueprint, pass the blueprint into the new Packages() function, and return the full set of packages to be depsolved. This allows us to also append packages based on other customizations too, and use that to append chrony when the timezone is set. This matches the behavior anaconda had, and there was a TODO item to do this, which had been overlooked. Fixes #787. Signed-off-by: Tom Gundersen --- cmd/osbuild-dnf-json-tests/main_test.go | 3 ++- cmd/osbuild-pipeline/main.go | 15 +-------------- cmd/osbuild-store-dump/main.go | 2 +- internal/distro/distro.go | 2 +- internal/distro/fedora31/distro.go | 9 ++++++--- internal/distro/fedora31/distro_test.go | 3 ++- internal/distro/fedora32/distro.go | 9 ++++++--- internal/distro/fedora32/distro_test.go | 3 ++- internal/distro/fedoratest/distro.go | 2 +- internal/distro/rhel8/distro.go | 9 ++++++--- internal/distro/rhel8/distro_test.go | 3 ++- internal/distro/test_distro/distro.go | 2 +- internal/rcm/api.go | 3 ++- internal/weldr/api.go | 6 ++---- 14 files changed, 35 insertions(+), 36 deletions(-) diff --git a/cmd/osbuild-dnf-json-tests/main_test.go b/cmd/osbuild-dnf-json-tests/main_test.go index 152fa5fbe..6c0b3bc44 100644 --- a/cmd/osbuild-dnf-json-tests/main_test.go +++ b/cmd/osbuild-dnf-json-tests/main_test.go @@ -14,6 +14,7 @@ import ( "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/distro/fedora31" "github.com/osbuild/osbuild-composer/internal/distro/fedora32" @@ -83,7 +84,7 @@ func TestCrossArchDepsolve(t *testing.T) { _, _, err = rpm.Depsolve(buildPackages, []string{}, repos[archStr], distroStruct.ModulePlatformID(), archStr) assert.NoError(t, err) - basePackagesInclude, basePackagesExclude := imgType.BasePackages() + basePackagesInclude, basePackagesExclude := imgType.Packages(blueprint.Blueprint{}) _, _, err = rpm.Depsolve(basePackagesInclude, basePackagesExclude, repos[archStr], distroStruct.ModulePlatformID(), archStr) assert.NoError(t, err) }) diff --git a/cmd/osbuild-pipeline/main.go b/cmd/osbuild-pipeline/main.go index 4009974eb..c221d4fcd 100644 --- a/cmd/osbuild-pipeline/main.go +++ b/cmd/osbuild-pipeline/main.go @@ -114,20 +114,7 @@ func main() { } } - packages := make([]string, len(composeRequest.Blueprint.Packages)) - for i, pkg := range composeRequest.Blueprint.Packages { - packages[i] = pkg.Name - // If a package has version "*" the package name suffix must be equal to "-*-*.*" - // Using just "-*" would find any other package containing the package name - if pkg.Version != "" && pkg.Version != "*" { - packages[i] += "-" + pkg.Version - } else if pkg.Version == "*" { - packages[i] += "-*-*.*" - } - } - - pkgs, excludePkgs := imageType.BasePackages() - packages = append(pkgs, packages...) + packages, excludePkgs := imageType.Packages(composeRequest.Blueprint) home, err := os.UserHomeDir() if err != nil { diff --git a/cmd/osbuild-store-dump/main.go b/cmd/osbuild-store-dump/main.go index 9c0871f88..508e6fae9 100644 --- a/cmd/osbuild-store-dump/main.go +++ b/cmd/osbuild-store-dump/main.go @@ -17,7 +17,7 @@ import ( ) func getManifest(bp blueprint.Blueprint, t distro.ImageType, a distro.Arch, d distro.Distro, rpmmd rpmmd.RPMMD, repos []rpmmd.RepoConfig) distro.Manifest { - packages, excludePackages := t.BasePackages() + packages, excludePackages := t.Packages(bp) pkgs, _, err := rpmmd.Depsolve(packages, excludePackages, repos, d.ModulePlatformID(), a.Name()) if err != nil { panic(err) diff --git a/internal/distro/distro.go b/internal/distro/distro.go index 9de18c82d..4e649bc69 100644 --- a/internal/distro/distro.go +++ b/internal/distro/distro.go @@ -70,7 +70,7 @@ type ImageType interface { // Returns the default packages to include and exclude when making the image // type. - BasePackages() ([]string, []string) + Packages(bp blueprint.Blueprint) ([]string, []string) // Returns the build packages for the output type. BuildPackages() []string diff --git a/internal/distro/fedora31/distro.go b/internal/distro/fedora31/distro.go index 3a1338429..ef7cca93a 100644 --- a/internal/distro/fedora31/distro.go +++ b/internal/distro/fedora31/distro.go @@ -155,8 +155,12 @@ func (t *imageType) Size(size uint64) uint64 { return size } -func (t *imageType) BasePackages() ([]string, []string) { - packages := t.packages +func (t *imageType) Packages(bp blueprint.Blueprint) ([]string, []string) { + packages := append(t.packages, bp.GetPackages()...) + timezone, _ := bp.Customizations.GetTimezoneSettings() + if timezone != nil { + packages = append(packages, "chrony") + } if t.bootable { packages = append(packages, t.arch.bootloaderPackages...) } @@ -439,7 +443,6 @@ func (t *imageType) pipeline(c *blueprint.Customizations, repos []rpmmd.RepoConf timezone, ntpServers := c.GetTimezoneSettings() - // TODO install chrony when this is set? if timezone != nil { p.AddStage(osbuild.NewTimezoneStage(&osbuild.TimezoneStageOptions{*timezone})) } diff --git a/internal/distro/fedora31/distro_test.go b/internal/distro/fedora31/distro_test.go index 6d259fd0e..41ac43452 100644 --- a/internal/distro/fedora31/distro_test.go +++ b/internal/distro/fedora31/distro_test.go @@ -3,6 +3,7 @@ package fedora31_test import ( "testing" + "github.com/osbuild/osbuild-composer/internal/blueprint" "github.com/osbuild/osbuild-composer/internal/distro/distro_test_common" "github.com/osbuild/osbuild-composer/internal/distro/fedora31" "github.com/stretchr/testify/assert" @@ -227,7 +228,7 @@ func TestImageType_BasePackages(t *testing.T) { for _, pkgMap := range pkgMaps { imgType, err := arch.GetImageType(pkgMap.name) assert.NoError(t, err) - basePackages, excludedPackages := imgType.BasePackages() + basePackages, excludedPackages := imgType.Packages(blueprint.Blueprint{}) assert.Equalf( t, append(pkgMap.basePackages, pkgMap.bootloaderPackages...), diff --git a/internal/distro/fedora32/distro.go b/internal/distro/fedora32/distro.go index cf659edd6..7588542f7 100644 --- a/internal/distro/fedora32/distro.go +++ b/internal/distro/fedora32/distro.go @@ -159,8 +159,12 @@ func (t *imageType) Size(size uint64) uint64 { return size } -func (t *imageType) BasePackages() ([]string, []string) { - packages := t.packages +func (t *imageType) Packages(bp blueprint.Blueprint) ([]string, []string) { + packages := append(t.packages, bp.GetPackages()...) + timezone, _ := bp.Customizations.GetTimezoneSettings() + if timezone != nil { + packages = append(packages, "chrony") + } if t.bootable { packages = append(packages, t.arch.bootloaderPackages...) } @@ -243,7 +247,6 @@ func (t *imageType) pipeline(c *blueprint.Customizations, options distro.ImageOp timezone, ntpServers := c.GetTimezoneSettings() - // TODO install chrony when this is set? if timezone != nil { p.AddStage(osbuild.NewTimezoneStage(&osbuild.TimezoneStageOptions{Zone: *timezone})) } diff --git a/internal/distro/fedora32/distro_test.go b/internal/distro/fedora32/distro_test.go index 3e2164676..158a98b3e 100644 --- a/internal/distro/fedora32/distro_test.go +++ b/internal/distro/fedora32/distro_test.go @@ -3,6 +3,7 @@ package fedora32_test import ( "testing" + "github.com/osbuild/osbuild-composer/internal/blueprint" "github.com/osbuild/osbuild-composer/internal/distro/distro_test_common" "github.com/osbuild/osbuild-composer/internal/distro/fedora32" "github.com/stretchr/testify/assert" @@ -272,7 +273,7 @@ func TestImageType_BasePackages(t *testing.T) { for _, pkgMap := range pkgMaps { imgType, err := arch.GetImageType(pkgMap.name) assert.NoError(t, err) - basePackages, excludedPackages := imgType.BasePackages() + basePackages, excludedPackages := imgType.Packages(blueprint.Blueprint{}) assert.Equalf( t, append(pkgMap.basePackages, pkgMap.bootloaderPackages...), diff --git a/internal/distro/fedoratest/distro.go b/internal/distro/fedoratest/distro.go index 72bd3dc59..8cb644915 100644 --- a/internal/distro/fedoratest/distro.go +++ b/internal/distro/fedoratest/distro.go @@ -83,7 +83,7 @@ func (t *imageType) Size(size uint64) uint64 { return size } -func (t *imageType) BasePackages() ([]string, []string) { +func (t *imageType) Packages(bp blueprint.Blueprint) ([]string, []string) { return nil, nil } diff --git a/internal/distro/rhel8/distro.go b/internal/distro/rhel8/distro.go index a43f4fceb..a853c8cd0 100644 --- a/internal/distro/rhel8/distro.go +++ b/internal/distro/rhel8/distro.go @@ -161,8 +161,12 @@ func (t *imageType) Size(size uint64) uint64 { return size } -func (t *imageType) BasePackages() ([]string, []string) { - packages := t.packages +func (t *imageType) Packages(bp blueprint.Blueprint) ([]string, []string) { + packages := append(t.packages, bp.GetPackages()...) + timezone, _ := bp.Customizations.GetTimezoneSettings() + if timezone != nil { + packages = append(packages, "chrony") + } if t.bootable { packages = append(packages, t.arch.bootloaderPackages...) } @@ -264,7 +268,6 @@ func (t *imageType) pipeline(c *blueprint.Customizations, options distro.ImageOp timezone, ntpServers := c.GetTimezoneSettings() - // TODO install chrony when this is set? if timezone != nil { p.AddStage(osbuild.NewTimezoneStage(&osbuild.TimezoneStageOptions{Zone: *timezone})) } diff --git a/internal/distro/rhel8/distro_test.go b/internal/distro/rhel8/distro_test.go index b8a8c601e..7196e66b8 100644 --- a/internal/distro/rhel8/distro_test.go +++ b/internal/distro/rhel8/distro_test.go @@ -3,6 +3,7 @@ package rhel8_test import ( "testing" + "github.com/osbuild/osbuild-composer/internal/blueprint" "github.com/osbuild/osbuild-composer/internal/distro/distro_test_common" "github.com/osbuild/osbuild-composer/internal/distro/rhel8" "github.com/stretchr/testify/assert" @@ -333,7 +334,7 @@ func TestImageType_BasePackages(t *testing.T) { for _, pkgMap := range pkgMaps { imgType, err := arch.GetImageType(pkgMap.name) assert.NoError(t, err) - basePackages, excludedPackages := imgType.BasePackages() + basePackages, excludedPackages := imgType.Packages(blueprint.Blueprint{}) assert.Equalf( t, append(pkgMap.basePackages, pkgMap.bootloaderPackages...), diff --git a/internal/distro/test_distro/distro.go b/internal/distro/test_distro/distro.go index 9cd4909bd..a0d2c9dd9 100644 --- a/internal/distro/test_distro/distro.go +++ b/internal/distro/test_distro/distro.go @@ -67,7 +67,7 @@ func (t *TestImageType) Size(size uint64) uint64 { return 0 } -func (t *TestImageType) BasePackages() ([]string, []string) { +func (t *TestImageType) Packages(bp blueprint.Blueprint) ([]string, []string) { return nil, nil } diff --git a/internal/rcm/api.go b/internal/rcm/api.go index e2bd77148..0cca0860e 100644 --- a/internal/rcm/api.go +++ b/internal/rcm/api.go @@ -9,6 +9,7 @@ import ( "net" "net/http" + "github.com/osbuild/osbuild-composer/internal/blueprint" "github.com/osbuild/osbuild-composer/internal/rpmmd" "github.com/osbuild/osbuild-composer/internal/worker" @@ -82,7 +83,7 @@ func notFoundHandler(writer http.ResponseWriter, request *http.Request) { // Depsolves packages and build packages for building an image for a given // distro, in the given architecture func depsolve(rpmmd rpmmd.RPMMD, distro distro.Distro, imageType distro.ImageType, repos []rpmmd.RepoConfig, arch distro.Arch) ([]rpmmd.PackageSpec, []rpmmd.PackageSpec, error) { - specs, excludeSpecs := imageType.BasePackages() + specs, excludeSpecs := imageType.Packages(blueprint.Blueprint{}) packages, _, err := rpmmd.Depsolve(specs, excludeSpecs, repos, distro.ModulePlatformID(), arch.Name()) if err != nil { return nil, nil, fmt.Errorf("RPMMD.Depsolve: %v", err) diff --git a/internal/weldr/api.go b/internal/weldr/api.go index 6cda1ba37..7fffe1349 100644 --- a/internal/weldr/api.go +++ b/internal/weldr/api.go @@ -2544,15 +2544,13 @@ func (api *API) allRepositories() []rpmmd.RepoConfig { func (api *API) depsolveBlueprint(bp *blueprint.Blueprint, imageType distro.ImageType) ([]rpmmd.PackageSpec, []rpmmd.PackageSpec, error) { repos := api.allRepositories() - specs := bp.GetPackages() + specs := bp.GetPackages() excludeSpecs := []string{} if imageType != nil { // When the output type is known, include the base packages in the depsolve // transaction. - packages, excludePackages := imageType.BasePackages() - specs = append(specs, packages...) - excludeSpecs = append(excludePackages, excludeSpecs...) + specs, excludeSpecs = imageType.Packages(*bp) } packages, _, err := api.rpmmd.Depsolve(specs, excludeSpecs, repos, api.distro.ModulePlatformID(), api.arch.Name())