Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

main: add --use-librepo to support librepo downloads #51

Merged
merged 6 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions cmd/image-builder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/osbuild/images/pkg/arch"
"github.com/osbuild/images/pkg/imagefilter"
"github.com/osbuild/images/pkg/osbuild"
"github.com/osbuild/images/pkg/ostree"

"github.com/osbuild/image-builder-cli/internal/blueprintload"
Expand Down Expand Up @@ -83,6 +84,14 @@ func cmdManifestWrapper(cmd *cobra.Command, args []string, w io.Writer, archChec
if err != nil {
return nil, err
}
useLibrepo, err := cmd.Flags().GetBool("use-librepo")
if err != nil {
return nil, err
}
var rpmDownloader osbuild.RpmDownloader
if useLibrepo {
rpmDownloader = osbuild.RpmDownloaderLibrepo
}

blueprintPath, err := cmd.Flags().GetString("blueprint")
if err != nil {
Expand All @@ -101,18 +110,23 @@ func cmdManifestWrapper(cmd *cobra.Command, args []string, w io.Writer, archChec
return nil, err
}

res, err := getOneImage(dataDir, distroStr, imgTypeStr, archStr)
img, err := getOneImage(dataDir, distroStr, imgTypeStr, archStr)
if err != nil {
return nil, err
}
if archChecker != nil {
if err := archChecker(res.Arch.Name()); err != nil {
if err := archChecker(img.Arch.Name()); err != nil {
return nil, err
}
}

err = generateManifest(dataDir, blueprintPath, res, w, ostreeImgOpts)
return res, err
opts := &manifestOptions{
BlueprintPath: blueprintPath,
Ostree: ostreeImgOpts,
RpmDownloader: rpmDownloader,
}
err = generateManifest(dataDir, img, w, opts)
return img, err
}

func cmdManifest(cmd *cobra.Command, args []string) error {
Expand Down Expand Up @@ -185,6 +199,7 @@ operating sytsems like centos and RHEL with easy customizations support.`,
manifestCmd.Flags().String("ostree-ref", "", `OSTREE reference`)
manifestCmd.Flags().String("ostree-parent", "", `OSTREE parent`)
manifestCmd.Flags().String("ostree-url", "", `OSTREE url`)
manifestCmd.Flags().Bool("use-librepo", true, `use librepo to download packages (disable if you use old versions of osbuild)`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can 'just' bump the minimum required osbuild version instead of this remark; let's do that.

rootCmd.AddCommand(manifestCmd)

buildCmd := &cobra.Command{
Expand Down
53 changes: 30 additions & 23 deletions cmd/image-builder/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,29 +162,36 @@ func TestManifestIntegrationSmoke(t *testing.T) {
restore := main.MockNewRepoRegistry(testrepos.New)
defer restore()

restore = main.MockOsArgs([]string{
"manifest",
"qcow2",
"--arch=x86_64",
"--distro=centos-9",
fmt.Sprintf("--blueprint=%s", makeTestBlueprint(t, testBlueprint)),
})
defer restore()

var fakeStdout bytes.Buffer
restore = main.MockOsStdout(&fakeStdout)
defer restore()

err := main.Run()
assert.NoError(t, err)

pipelineNames, err := manifesttest.PipelineNamesFrom(fakeStdout.Bytes())
assert.NoError(t, err)
assert.Contains(t, pipelineNames, "qcow2")

// XXX: provide helpers in manifesttest to extract this in a nicer way
assert.Contains(t, fakeStdout.String(), `{"type":"org.osbuild.users","options":{"users":{"alice":{}}}}`)
assert.Contains(t, fakeStdout.String(), `"image":{"name":"registry.gitlab.com/redhat/services/products/image-builder/ci/osbuild-composer/fedora-minimal"`)
for _, useLibrepo := range []bool{false, true} {
t.Run(fmt.Sprintf("use-librepo: %v", useLibrepo), func(t *testing.T) {
restore = main.MockOsArgs([]string{
"manifest",
"qcow2",
"--arch=x86_64",
"--distro=centos-9",
fmt.Sprintf("--blueprint=%s", makeTestBlueprint(t, testBlueprint)),
fmt.Sprintf("--use-librepo=%v", useLibrepo),
})
defer restore()

var fakeStdout bytes.Buffer
restore = main.MockOsStdout(&fakeStdout)
defer restore()

err := main.Run()
assert.NoError(t, err)

pipelineNames, err := manifesttest.PipelineNamesFrom(fakeStdout.Bytes())
assert.NoError(t, err)
assert.Contains(t, pipelineNames, "qcow2")

// XXX: provide helpers in manifesttest to extract this in a nicer way
assert.Contains(t, fakeStdout.String(), `{"type":"org.osbuild.users","options":{"users":{"alice":{}}}}`)
assert.Contains(t, fakeStdout.String(), `"image":{"name":"registry.gitlab.com/redhat/services/products/image-builder/ci/osbuild-composer/fedora-minimal"`)

assert.Equal(t, strings.Contains(fakeStdout.String(), "org.osbuild.librepo"), useLibrepo)
})
}
}

func TestManifestIntegrationCrossArch(t *testing.T) {
Expand Down
20 changes: 14 additions & 6 deletions cmd/image-builder/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,34 +5,42 @@ import (

"github.com/osbuild/images/pkg/distro"
"github.com/osbuild/images/pkg/imagefilter"
"github.com/osbuild/images/pkg/osbuild"
"github.com/osbuild/images/pkg/ostree"

"github.com/osbuild/image-builder-cli/internal/blueprintload"
"github.com/osbuild/image-builder-cli/internal/manifestgen"
)

func generateManifest(dataDir, blueprintPath string, res *imagefilter.Result, output io.Writer, ostreeOpts *ostree.ImageOptions) error {
type manifestOptions struct {
BlueprintPath string
Ostree *ostree.ImageOptions
RpmDownloader osbuild.RpmDownloader
}

func generateManifest(dataDir string, img *imagefilter.Result, output io.Writer, opts *manifestOptions) error {
repos, err := newRepoRegistry(dataDir)
if err != nil {
return err
}
// XXX: add --rpmmd/cachedir option like bib
mg, err := manifestgen.New(repos, &manifestgen.Options{
Output: output,
Output: output,
RpmDownloader: opts.RpmDownloader,
})
if err != nil {
return err
}
bp, err := blueprintload.Load(blueprintPath)
bp, err := blueprintload.Load(opts.BlueprintPath)
if err != nil {
return err
}
var imgOpts *distro.ImageOptions
if ostreeOpts != nil {
if opts.Ostree != nil {
imgOpts = &distro.ImageOptions{
OSTree: ostreeOpts,
OSTree: opts.Ostree,
}
}

return mg.Generate(bp, res.Distro, res.ImgType, res.Arch, imgOpts)
return mg.Generate(bp, img.Distro, img.ImgType, img.Arch, imgOpts)
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ go 1.21.0
require (
github.com/BurntSushi/toml v1.4.0
github.com/gobwas/glob v0.2.3
github.com/osbuild/images v0.108.1-0.20250109111809-f295ad7807a3
github.com/osbuild/images v0.109.1-0.20250117082457-97ca7c62eb09
github.com/sirupsen/logrus v1.9.3
github.com/spf13/cobra v1.8.1
github.com/stretchr/testify v1.10.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ github.com/opencontainers/runtime-spec v1.2.0 h1:z97+pHb3uELt/yiAWD691HNHQIF07bE
github.com/opencontainers/runtime-spec v1.2.0/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
github.com/opencontainers/selinux v1.11.0 h1:+5Zbo97w3Lbmb3PeqQtpmTkMwsW5nRI3YaLpt7tQ7oU=
github.com/opencontainers/selinux v1.11.0/go.mod h1:E5dMC3VPuVvVHDYmi78qvhJp8+M586T4DlDRYpFkyec=
github.com/osbuild/images v0.108.1-0.20250109111809-f295ad7807a3 h1:gUuQvYWQggIrR6/tIGOiFz4L1kapAPGL1xdJ9SqvkQs=
github.com/osbuild/images v0.108.1-0.20250109111809-f295ad7807a3/go.mod h1:58tzp7jV50rjaH9gMpvmQdVati0c4TaC5Op7wmSD/tY=
github.com/osbuild/images v0.109.1-0.20250117082457-97ca7c62eb09 h1:9ABt4zMJ6tNp1+AHI4FrDq1Fxy/l5uYuDXs6EOR6QJ0=
github.com/osbuild/images v0.109.1-0.20250117082457-97ca7c62eb09/go.mod h1:58tzp7jV50rjaH9gMpvmQdVati0c4TaC5Op7wmSD/tY=
github.com/ostreedev/ostree-go v0.0.0-20210805093236-719684c64e4f h1:/UDgs8FGMqwnHagNDPGOlts35QkhAZ8by3DR7nMih7M=
github.com/ostreedev/ostree-go v0.0.0-20210805093236-719684c64e4f/go.mod h1:J6OG6YJVEWopen4avK3VNQSnALmmjvniMmni/YFYAwc=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
Expand Down
8 changes: 6 additions & 2 deletions image-builder-cli.spec
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@
%bcond_with tests
%bcond_with relax_requires

# The minimum required osbuild version
%global min_osbuild_version 129
# The minimum required osbuild version, note that this used to be 129
# but got bumped to 138 for librepo support which is not strictly
# required. So if this needs backport to places where there is no
# recent osbuild available we could simply make --use-librepo false
# and go back to 129.
Comment on lines +6 to +10
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a realistic issue? I guess it might be if you're go installing ib-cli, but in that case, you're on your own and the spec file doesn't matter.

I mean, there's no way this spec file will be used to officially build ib-cli in a distro where a new enough osbuild wont be, so I think this note isn't relevant.

I might be missing something though.

%global min_osbuild_version 138

%global goipath github.com/osbuild/image-builder-cli

Expand Down
33 changes: 20 additions & 13 deletions internal/manifestgen/manifestgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"github.com/osbuild/images/pkg/container"
"github.com/osbuild/images/pkg/distro"
"github.com/osbuild/images/pkg/dnfjson"
"github.com/osbuild/images/pkg/manifest"
"github.com/osbuild/images/pkg/osbuild"
"github.com/osbuild/images/pkg/ostree"
"github.com/osbuild/images/pkg/reporegistry"
"github.com/osbuild/images/pkg/rpmmd"
Expand All @@ -20,30 +22,26 @@ import (
// cmd/build/main.go:depsolve (and probably more places) should go
// into a common helper in "images" or images should do this on its
// own
func defaultDepsolver(cacheDir string, packageSets map[string][]rpmmd.PackageSet, d distro.Distro, arch string) (map[string][]rpmmd.PackageSpec, map[string][]rpmmd.RepoConfig, error) {
func defaultDepsolver(cacheDir string, packageSets map[string][]rpmmd.PackageSet, d distro.Distro, arch string) (map[string]dnfjson.DepsolveResult, error) {
if cacheDir == "" {
var err error
cacheDir, err = os.MkdirTemp("", "manifestgen")
if err != nil {
return nil, nil, fmt.Errorf("cannot create temporary directory: %w", err)
return nil, fmt.Errorf("cannot create temporary directory: %w", err)
}
defer os.RemoveAll(cacheDir)
}

solver := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch, d.Name(), cacheDir)
depsolvedSets := make(map[string][]rpmmd.PackageSpec)
repoSets := make(map[string][]rpmmd.RepoConfig)
depsolvedSets := make(map[string]dnfjson.DepsolveResult)
for name, pkgSet := range packageSets {
res, err := solver.Depsolve(pkgSet, sbom.StandardTypeNone)
if err != nil {
return nil, nil, fmt.Errorf("error depsolving: %w", err)
return nil, fmt.Errorf("error depsolving: %w", err)
}
depsolvedSets[name] = res.Packages
repoSets[name] = res.Repos
// the depsolve result also contains SBOM information,
// it is currently not used here though
depsolvedSets[name] = *res
}
return depsolvedSets, repoSets, nil
return depsolvedSets, nil
}

func resolveContainers(containers []container.SourceSpec, archName string) ([]container.Spec, error) {
Expand Down Expand Up @@ -85,7 +83,7 @@ func defaultCommitResolver(commitSources map[string][]ostree.SourceSpec) (map[st
}

type (
DepsolveFunc func(cacheDir string, packageSets map[string][]rpmmd.PackageSet, d distro.Distro, arch string) (map[string][]rpmmd.PackageSpec, map[string][]rpmmd.RepoConfig, error)
DepsolveFunc func(cacheDir string, packageSets map[string][]rpmmd.PackageSet, d distro.Distro, arch string) (map[string]dnfjson.DepsolveResult, error)

ContainerResolverFunc func(containerSources map[string][]container.SourceSpec, archName string) (map[string][]container.Spec, error)

Expand All @@ -100,6 +98,8 @@ type Options struct {
Depsolver DepsolveFunc
ContainerResolver ContainerResolverFunc
CommitResolver CommitResolverFunc

RpmDownloader osbuild.RpmDownloader
}

// Generator can generate an osbuild manifest from a given repository
Expand All @@ -113,6 +113,8 @@ type Generator struct {
commitResolver CommitResolverFunc

reporegistry *reporegistry.RepoRegistry

rpmDownloader osbuild.RpmDownloader
}

// New will create a new manifest generator
Expand All @@ -128,6 +130,7 @@ func New(reporegistry *reporegistry.RepoRegistry, opts *Options) (*Generator, er
depsolver: opts.Depsolver,
containerResolver: opts.ContainerResolver,
commitResolver: opts.CommitResolver,
rpmDownloader: opts.RpmDownloader,
}
if mg.out == nil {
mg.out = os.Stdout
Expand Down Expand Up @@ -165,7 +168,7 @@ func (mg *Generator) Generate(bp *blueprint.Blueprint, dist distro.Distro, imgTy
// what are these warnings?
return fmt.Errorf("warnings during manifest creation: %v", strings.Join(warnings, "\n"))
}
packageSpecs, repoConfig, err := mg.depsolver(mg.cacheDir, preManifest.GetPackageSetChains(), dist, a.Name())
depsolved, err := mg.depsolver(mg.cacheDir, preManifest.GetPackageSetChains(), dist, a.Name())
if err != nil {
return err
}
Expand All @@ -177,7 +180,11 @@ func (mg *Generator) Generate(bp *blueprint.Blueprint, dist distro.Distro, imgTy
if err != nil {
return err
}
mf, err := preManifest.Serialize(packageSpecs, containerSpecs, commitSpecs, repoConfig)

opts := &manifest.SerializeOptions{
RpmDownloader: mg.rpmDownloader,
}
mf, err := preManifest.Serialize(depsolved, containerSpecs, commitSpecs, opts)
if err != nil {
return err
}
Expand Down
Loading
Loading