From c296b666cf319c3c73e25b9a373b1f5226085f17 Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Tue, 5 Jul 2022 22:06:33 +0100 Subject: [PATCH] distro/fedora: drop usage of MakePackageChainSet Pass PackageSets when initialising the Manifest, and read the chains back out. This also fixes a bug where all repos were always used, rather than filtering per package set. Finally, this moves the 'chrony' inclusion from distro.go to the OSPipeline where it belongs. In doing so the logic is changed slightly, where chrony is now installed if NTP servers are configured (regardless of source), whereas in the past it was included if the timezone was set in the blueprint (which made no sense). --- internal/distro/fedora/distro.go | 84 ++++++++---------------- internal/distro/fedora/manifests.go | 86 +++++++++++-------------- internal/manifest/anaconda.go | 4 +- internal/manifest/commit_server_tree.go | 4 +- internal/manifest/os.go | 11 +++- 5 files changed, 79 insertions(+), 110 deletions(-) diff --git a/internal/distro/fedora/distro.go b/internal/distro/fedora/distro.go index 057cc345e..fb5589b26 100644 --- a/internal/distro/fedora/distro.go +++ b/internal/distro/fedora/distro.go @@ -74,9 +74,6 @@ var ( packageSets: map[string]packageSetFunc{ osPkgsKey: iotCommitPackageSet, }, - packageSetChains: map[string][]string{ - osPkgsKey: {osPkgsKey, blueprintPkgsKey}, - }, defaultImageConfig: &distro.ImageConfig{ EnabledServices: iotServices, }, @@ -100,9 +97,6 @@ var ( } }, }, - packageSetChains: map[string][]string{ - osPkgsKey: {osPkgsKey, blueprintPkgsKey}, - }, defaultImageConfig: &distro.ImageConfig{ EnabledServices: iotServices, }, @@ -141,9 +135,6 @@ var ( packageSets: map[string]packageSetFunc{ osPkgsKey: qcow2CommonPackageSet, }, - packageSetChains: map[string][]string{ - osPkgsKey: {osPkgsKey, blueprintPkgsKey}, - }, defaultImageConfig: &distro.ImageConfig{ DefaultTarget: "multi-user.target", EnabledServices: []string{ @@ -170,9 +161,6 @@ var ( packageSets: map[string]packageSetFunc{ osPkgsKey: vhdCommonPackageSet, }, - packageSetChains: map[string][]string{ - osPkgsKey: {osPkgsKey, blueprintPkgsKey}, - }, defaultImageConfig: &distro.ImageConfig{ Locale: "en_US.UTF-8", EnabledServices: []string{ @@ -202,9 +190,6 @@ var ( packageSets: map[string]packageSetFunc{ osPkgsKey: vmdkCommonPackageSet, }, - packageSetChains: map[string][]string{ - osPkgsKey: {osPkgsKey, blueprintPkgsKey}, - }, defaultImageConfig: &distro.ImageConfig{ Locale: "en_US.UTF-8", EnabledServices: []string{ @@ -231,9 +216,6 @@ var ( packageSets: map[string]packageSetFunc{ osPkgsKey: openstackCommonPackageSet, }, - packageSetChains: map[string][]string{ - osPkgsKey: {osPkgsKey, blueprintPkgsKey}, - }, defaultImageConfig: &distro.ImageConfig{ Locale: "en_US.UTF-8", EnabledServices: []string{ @@ -268,9 +250,6 @@ var ( packageSets: map[string]packageSetFunc{ osPkgsKey: ec2CommonPackageSet, }, - packageSetChains: map[string][]string{ - osPkgsKey: {osPkgsKey, blueprintPkgsKey}, - }, defaultImageConfig: defaultEc2ImageConfig, kernelOptions: defaultKernelOptions, bootable: true, @@ -289,9 +268,6 @@ var ( packageSets: map[string]packageSetFunc{ osPkgsKey: containerPackageSet, }, - packageSetChains: map[string][]string{ - osPkgsKey: {osPkgsKey, blueprintPkgsKey}, - }, defaultImageConfig: &distro.ImageConfig{ NoSElinux: true, ExcludeDocs: true, @@ -476,7 +452,7 @@ func (a *architecture) Distro() distro.Distro { return a.distro } -type manifestFunc func(m *manifest.Manifest, t *imageType, customizations *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, packageSetChains map[string][]rpmmd.PackageSet, rng *rand.Rand) error +type manifestFunc func(m *manifest.Manifest, t *imageType, customizations *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, packageSets map[string]rpmmd.PackageSet, rng *rand.Rand) error type packageSetFunc func(t *imageType) rpmmd.PackageSet @@ -488,7 +464,6 @@ type imageType struct { filename string mimeType string packageSets map[string]packageSetFunc - packageSetChains map[string][]string defaultImageConfig *distro.ImageConfig kernelOptions string defaultSize uint64 @@ -543,44 +518,39 @@ func (t *imageType) Size(size uint64) uint64 { return size } -func (t *imageType) getPackages(name string) rpmmd.PackageSet { - getter := t.packageSets[name] - if getter == nil { - return rpmmd.PackageSet{} - } - - return getter(t) -} - func (t *imageType) PackageSets(bp blueprint.Blueprint, options distro.ImageOptions, repos []rpmmd.RepoConfig) map[string][]rpmmd.PackageSet { // merge package sets that appear in the image type with the package sets // of the same name from the distro and arch - mergedSets := make(map[string]rpmmd.PackageSet) + packageSets := make(map[string]rpmmd.PackageSet) - imageSets := t.packageSets - - for name := range imageSets { - mergedSets[name] = t.getPackages(name) - } - - if _, hasPackages := imageSets[osPkgsKey]; !hasPackages { - // should this be possible?? - mergedSets[osPkgsKey] = rpmmd.PackageSet{} - } - - // do not include the kernel package, this is added in the pipelines - bpPackages := bp.GetPackagesEx(false) - timezone, _ := bp.Customizations.GetTimezoneSettings() - if timezone != nil { - bpPackages = append(bpPackages, "chrony") + for name, getter := range t.packageSets { + packageSets[name] = getter(t) } // depsolve bp packages separately // bp packages aren't restricted by exclude lists - mergedSets[blueprintPkgsKey] = rpmmd.PackageSet{Include: bpPackages} + // do not include the kernel package as it is included from the pipeline + packageSets[blueprintPkgsKey] = rpmmd.PackageSet{Include: bp.GetPackagesEx(false)} + + // amend with repository information + globalRepos := make([]rpmmd.RepoConfig, 0) + for _, repo := range repos { + if len(repo.PackageSets) > 0 { + // only apply the repo to the listed package sets + for _, psName := range repo.PackageSets { + ps := packageSets[psName] + ps.Repositories = append(ps.Repositories, repo) + packageSets[psName] = ps + } + } else { + // no package sets were listed, so apply the repo + // to all package sets + globalRepos = append(globalRepos, repo) + } + } // create a manifest object and instantiate it with the computed packageSetChains - manifest, err := t.initializeManifest(bp.Customizations, options, repos, distro.MakePackageSetChains(t, mergedSets, repos), 0) + manifest, err := t.initializeManifest(bp.Customizations, options, globalRepos, packageSets, 0) if err != nil { // TODO: handle manifest initialization errors more gracefully, we // refuse to initialize manifests with invalid config. @@ -603,7 +573,7 @@ func (t *imageType) PayloadPackageSets() []string { } func (t *imageType) PackageSetsChains() map[string][]string { - return t.packageSetChains + return make(map[string][]string) } func (t *imageType) Exports() []string { @@ -652,7 +622,7 @@ func (t *imageType) PartitionType() string { func (t *imageType) initializeManifest(customizations *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, - packageSetChains map[string][]rpmmd.PackageSet, + packageSets map[string]rpmmd.PackageSet, seed int64) (*manifest.Manifest, error) { if err := t.checkOptions(customizations, options); err != nil { @@ -665,7 +635,7 @@ func (t *imageType) initializeManifest(customizations *blueprint.Customizations, rng := rand.New(source) manifest := manifest.New() - err := t.manifest(&manifest, t, customizations, options, repos, packageSetChains, rng) + err := t.manifest(&manifest, t, customizations, options, repos, packageSets, rng) if err != nil { return nil, err } diff --git a/internal/distro/fedora/manifests.go b/internal/distro/fedora/manifests.go index b15dd4344..9f02d817c 100644 --- a/internal/distro/fedora/manifests.go +++ b/internal/distro/fedora/manifests.go @@ -14,11 +14,11 @@ func qcow2Manifest(m *manifest.Manifest, customizations *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, - packageSetChains map[string][]rpmmd.PackageSet, + packageSets map[string]rpmmd.PackageSet, rng *rand.Rand) error { buildPipeline := manifest.NewBuildPipeline(m, t.arch.distro.runner, repos) - treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSetChains[osPkgsKey], customizations, options, rng) + treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSets[osPkgsKey], packageSets[blueprintPkgsKey], customizations, options, rng) if err != nil { return err } @@ -34,11 +34,11 @@ func vhdManifest(m *manifest.Manifest, customizations *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, - packageSetChains map[string][]rpmmd.PackageSet, + packageSets map[string]rpmmd.PackageSet, rng *rand.Rand) error { buildPipeline := manifest.NewBuildPipeline(m, t.arch.distro.runner, repos) - treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSetChains[osPkgsKey], customizations, options, rng) + treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSets[osPkgsKey], packageSets[blueprintPkgsKey], customizations, options, rng) if err != nil { return err } @@ -53,11 +53,11 @@ func vmdkManifest(m *manifest.Manifest, customizations *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, - packageSetChains map[string][]rpmmd.PackageSet, + packageSets map[string]rpmmd.PackageSet, rng *rand.Rand) error { buildPipeline := manifest.NewBuildPipeline(m, t.arch.distro.runner, repos) - treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSetChains[osPkgsKey], customizations, options, rng) + treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSets[osPkgsKey], packageSets[blueprintPkgsKey], customizations, options, rng) if err != nil { return err } @@ -72,11 +72,11 @@ func openstackManifest(m *manifest.Manifest, customizations *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, - packageSetChains map[string][]rpmmd.PackageSet, + packageSets map[string]rpmmd.PackageSet, rng *rand.Rand) error { buildPipeline := manifest.NewBuildPipeline(m, t.arch.distro.runner, repos) - treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSetChains[osPkgsKey], customizations, options, rng) + treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSets[osPkgsKey], packageSets[blueprintPkgsKey], customizations, options, rng) if err != nil { return err } @@ -91,12 +91,12 @@ func ec2CommonManifest(m *manifest.Manifest, customizations *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, - packageSetChains map[string][]rpmmd.PackageSet, + packageSets map[string]rpmmd.PackageSet, rng *rand.Rand, diskfile string) error { buildPipeline := manifest.NewBuildPipeline(m, t.arch.distro.runner, repos) - treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSetChains[osPkgsKey], customizations, options, rng) + treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSets[osPkgsKey], packageSets[blueprintPkgsKey], customizations, options, rng) if err != nil { return nil } @@ -107,9 +107,9 @@ func ec2CommonManifest(m *manifest.Manifest, // ec2Manifest returns a manifest which produce uncompressed EC2 images which are expected to use RHSM for content func ec2Manifest(m *manifest.Manifest, t *imageType, customizations *blueprint.Customizations, options distro.ImageOptions, - repos []rpmmd.RepoConfig, packageSetChains map[string][]rpmmd.PackageSet, + repos []rpmmd.RepoConfig, packageSets map[string]rpmmd.PackageSet, rng *rand.Rand) error { - return ec2CommonManifest(m, t, customizations, options, repos, packageSetChains, rng, t.Filename()) + return ec2CommonManifest(m, t, customizations, options, repos, packageSets, rng, t.Filename()) } func iotInstallerManifest(m *manifest.Manifest, @@ -117,7 +117,7 @@ func iotInstallerManifest(m *manifest.Manifest, customizations *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, - packageSetChains map[string][]rpmmd.PackageSet, + packageSets map[string]rpmmd.PackageSet, rng *rand.Rand) error { buildPipeline := manifest.NewBuildPipeline(m, t.arch.distro.runner, repos) @@ -125,7 +125,7 @@ func iotInstallerManifest(m *manifest.Manifest, d := t.arch.distro ksUsers := len(customizations.GetUsers())+len(customizations.GetGroups()) > 0 - anacondaTreePipeline := anacondaTreePipeline(m, buildPipeline, repos, packageSetChains[installerPkgsKey], t.Arch().Name(), d.product, d.osVersion, "IoT", ksUsers) + anacondaTreePipeline := anacondaTreePipeline(m, buildPipeline, repos, packageSets[installerPkgsKey], t.Arch().Name(), d.product, d.osVersion, "IoT", ksUsers) isoTreePipeline := bootISOTreePipeline(m, buildPipeline, anacondaTreePipeline, options, d.vendor, d.isolabelTmpl, customizations.GetUsers(), customizations.GetGroups()) bootISOPipeline(m, buildPipeline, isoTreePipeline, t.Filename(), false) @@ -137,11 +137,11 @@ func iotCorePipelines(m *manifest.Manifest, customizations *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, - packageSetChains map[string][]rpmmd.PackageSet) (*manifest.BuildPipeline, + packageSets map[string]rpmmd.PackageSet) (*manifest.BuildPipeline, *manifest.OSTreeCommitPipeline, error) { buildPipeline := manifest.NewBuildPipeline(m, t.arch.distro.runner, repos) - treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSetChains[osPkgsKey], customizations, options, nil) + treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSets[osPkgsKey], packageSets[blueprintPkgsKey], customizations, options, nil) if err != nil { return nil, nil, err } @@ -151,10 +151,10 @@ func iotCorePipelines(m *manifest.Manifest, } func iotCommitManifest(m *manifest.Manifest, t *imageType, customizations *blueprint.Customizations, options distro.ImageOptions, - repos []rpmmd.RepoConfig, packageSetChains map[string][]rpmmd.PackageSet, + repos []rpmmd.RepoConfig, packageSets map[string]rpmmd.PackageSet, rng *rand.Rand) error { - buildPipeline, commitPipeline, err := iotCorePipelines(m, t, customizations, options, repos, packageSetChains) + buildPipeline, commitPipeline, err := iotCorePipelines(m, t, customizations, options, repos, packageSets) if err != nil { return err } @@ -164,16 +164,16 @@ func iotCommitManifest(m *manifest.Manifest, t *imageType, customizations *bluep } func iotContainerManifest(m *manifest.Manifest, t *imageType, customizations *blueprint.Customizations, - options distro.ImageOptions, repos []rpmmd.RepoConfig, packageSetChains map[string][]rpmmd.PackageSet, + options distro.ImageOptions, repos []rpmmd.RepoConfig, packageSets map[string]rpmmd.PackageSet, rng *rand.Rand) error { - buildPipeline, commitPipeline, err := iotCorePipelines(m, t, customizations, options, repos, packageSetChains) + buildPipeline, commitPipeline, err := iotCorePipelines(m, t, customizations, options, repos, packageSets) if err != nil { return err } nginxConfigPath := "/etc/nginx.conf" httpPort := "8080" - containerTreePipeline := containerTreePipeline(m, buildPipeline, commitPipeline, repos, packageSetChains[containerPkgsKey], options, customizations, nginxConfigPath, httpPort) + containerTreePipeline := containerTreePipeline(m, buildPipeline, commitPipeline, repos, packageSets[containerPkgsKey], options, customizations, nginxConfigPath, httpPort) containerPipeline(m, buildPipeline, &containerTreePipeline.BasePipeline, t, nginxConfigPath, httpPort) return nil @@ -184,11 +184,11 @@ func containerManifest(m *manifest.Manifest, customizations *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, - packageSetChains map[string][]rpmmd.PackageSet, + packageSets map[string]rpmmd.PackageSet, rng *rand.Rand) error { buildPipeline := manifest.NewBuildPipeline(m, t.arch.distro.runner, repos) - treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSetChains[osPkgsKey], customizations, options, rng) + treePipeline, err := osPipeline(m, buildPipeline, t, repos, packageSets[osPkgsKey], packageSets[blueprintPkgsKey], customizations, options, rng) if err != nil { return err } @@ -201,7 +201,8 @@ func osPipeline(m *manifest.Manifest, buildPipeline *manifest.BuildPipeline, t *imageType, repos []rpmmd.RepoConfig, - osChain []rpmmd.PackageSet, + osPackageSet rpmmd.PackageSet, + blueprintPackageSet rpmmd.PackageSet, c *blueprint.Customizations, options distro.ImageOptions, rng *rand.Rand) (*manifest.OSPipeline, error) { @@ -245,17 +246,11 @@ func osPipeline(m *manifest.Manifest, } } - if len(osChain) >= 1 { - pl.ExtraBasePackages = osChain[0].Include - pl.ExcludeBasePackages = osChain[0].Exclude - } - if len(osChain) >= 2 { - pl.UserPackages = osChain[1].Include - pl.UserRepos = osChain[1].Repositories - } - if len(osChain) > 2 { - panic("unexpected number of package sets in os chain") - } + pl.ExtraBasePackages = osPackageSet.Include + pl.ExcludeBasePackages = osPackageSet.Exclude + pl.ExtraBaseRepos = osPackageSet.Repositories + pl.UserPackages = blueprintPackageSet.Include + pl.UserRepos = blueprintPackageSet.Repositories pl.GPGKeyFiles = imageConfig.GPGKeyFiles pl.ExcludeDocs = imageConfig.ExcludeDocs @@ -350,18 +345,14 @@ func containerTreePipeline(m *manifest.Manifest, buildPipeline *manifest.BuildPipeline, commitPipeline *manifest.OSTreeCommitPipeline, repos []rpmmd.RepoConfig, - containerChain []rpmmd.PackageSet, + containerPackageSet rpmmd.PackageSet, options distro.ImageOptions, c *blueprint.Customizations, nginxConfigPath, listenPort string) *manifest.OSTreeCommitServerTreePipeline { p := manifest.NewOSTreeCommitServerTreePipeline(m, buildPipeline, repos, commitPipeline, nginxConfigPath, listenPort) - if len(containerChain) >= 1 { - p.ExtraPackages = containerChain[0].Include - } - if len(containerChain) > 2 { - panic("unexpected number of package sets in os chain") - } + p.ExtraPackages = containerPackageSet.Include + p.ExtraRepos = containerPackageSet.Repositories language, _ := c.GetPrimaryLocale() if language != nil { p.Language = *language @@ -384,16 +375,13 @@ func containerPipeline(m *manifest.Manifest, func anacondaTreePipeline(m *manifest.Manifest, buildPipeline *manifest.BuildPipeline, repos []rpmmd.RepoConfig, - chain []rpmmd.PackageSet, + installerPackageSet rpmmd.PackageSet, arch, product, osVersion, variant string, users bool) *manifest.AnacondaPipeline { p := manifest.NewAnacondaPipeline(m, buildPipeline, repos, "kernel", arch, product, osVersion) - if len(chain) >= 1 { - p.ExtraPackages = chain[0].Include - } - if len(chain) > 1 { - panic("unexpected number of package sets in installer chain") - } + p.ExtraPackages = installerPackageSet.Include + p.ExtraRepos = installerPackageSet.Repositories + p.Users = users p.Variant = variant p.Biosdevname = (arch == distro.X86_64ArchName) diff --git a/internal/manifest/anaconda.go b/internal/manifest/anaconda.go index 7ffd59c4b..343e2e59c 100644 --- a/internal/manifest/anaconda.go +++ b/internal/manifest/anaconda.go @@ -13,6 +13,8 @@ type AnacondaPipeline struct { // Packages to install in addition to the ones required by the // pipeline. ExtraPackages []string + // Extra repositories to install packages from + ExtraRepos []rpmmd.RepoConfig // Users indicate whether or not the user spoke should be enabled in // anaconda. If it is, users specified in a kickstart will be configured, // and in case no users are provided in a kickstart the user will be @@ -110,7 +112,7 @@ func (p *AnacondaPipeline) getPackageSetChain() []rpmmd.PackageSet { return []rpmmd.PackageSet{ { Include: append(packages, p.ExtraPackages...), - Repositories: p.repos, + Repositories: append(p.repos, p.ExtraRepos...), }, } } diff --git a/internal/manifest/commit_server_tree.go b/internal/manifest/commit_server_tree.go index f72b83bfd..72d2a2f5d 100644 --- a/internal/manifest/commit_server_tree.go +++ b/internal/manifest/commit_server_tree.go @@ -15,6 +15,8 @@ type OSTreeCommitServerTreePipeline struct { // Packages to install in addition to the ones required by the // pipeline. ExtraPackages []string + // Extra repositories to install packages from + ExtraRepos []rpmmd.RepoConfig // TODO: should this be configurable? Language string @@ -57,7 +59,7 @@ func (p *OSTreeCommitServerTreePipeline) getPackageSetChain() []rpmmd.PackageSet return []rpmmd.PackageSet{ { Include: append(packages, p.ExtraPackages...), - Repositories: p.repos, + Repositories: append(p.repos, p.ExtraRepos...), }, } } diff --git a/internal/manifest/os.go b/internal/manifest/os.go index f95770ffe..5a408ceb3 100644 --- a/internal/manifest/os.go +++ b/internal/manifest/os.go @@ -34,10 +34,12 @@ type OSPipeline struct { // can satisfy a dependency. Must not conflict with the included base // package set. ExcludeBasePackages []string + // Additional repos to install the base packages from. + ExtraBaseRepos []rpmmd.RepoConfig // Packages to install on top of the base packages in a seconadry dnf // transaction. UserPackages []string - // Repositories to install the user packages from. + // Additional repos to install the user packages from. UserRepos []rpmmd.RepoConfig // OSTree configuration, if nil the tree cannot be in an OSTree commit OSTree *OSPipelineOSTree @@ -153,11 +155,16 @@ func (p *OSPipeline) getPackageSetChain() []rpmmd.PackageSet { } } + if len(p.NTPServers) > 0 { + // TODO: move to base packages + userPackages = append(userPackages, "chrony") + } + chain := []rpmmd.PackageSet{ { Include: append(packages, p.ExtraBasePackages...), Exclude: p.ExcludeBasePackages, - Repositories: p.repos, + Repositories: append(p.repos, p.ExtraBaseRepos...), }, }