From ccb0787e2a7627aa4b1a8bc903128ad636c88035 Mon Sep 17 00:00:00 2001 From: Mikhail Malyshev Date: Thu, 26 Feb 2026 08:20:01 +0000 Subject: [PATCH] pkg build: refactor builder parameters into BuilderConfig struct Group the four builder-related fields (name, image, config path, restart) that always travel together into a BuilderConfig struct. This simplifies: - DockerRunner interface (Build() and Builder() lose 3 params each) - buildOpts struct (4 fields -> 1) - buildArch() function signature (3 fewer params) - DiskUsage() / PruneBuilder() / getClientForPlatform() signatures - 4 WithBuildBuilder*() option functions -> 1 WithBuildBuilderConfig() Also rename the confusingly-named "builderName" local variables in buildArch() and getClientForPlatform() to "dockerContext", which better reflects their actual purpose (they hold a Docker context name, not the builder container name). No behavioral changes. Signed-off-by: Mikhail Malyshev --- src/cmd/linuxkit/pkg_build.go | 8 ++- src/cmd/linuxkit/pkg_builder.go | 8 ++- src/cmd/linuxkit/pkglib/build.go | 84 ++++++++++--------------- src/cmd/linuxkit/pkglib/build_test.go | 4 +- src/cmd/linuxkit/pkglib/docker.go | 17 ++++- src/cmd/linuxkit/pkglib/dockerdryrun.go | 4 +- src/cmd/linuxkit/pkglib/dockerimpl.go | 23 ++++--- src/cmd/linuxkit/pkglib/utils.go | 16 ++--- 8 files changed, 85 insertions(+), 79 deletions(-) diff --git a/src/cmd/linuxkit/pkg_build.go b/src/cmd/linuxkit/pkg_build.go index 79304d487..c0b719cdc 100644 --- a/src/cmd/linuxkit/pkg_build.go +++ b/src/cmd/linuxkit/pkg_build.go @@ -195,12 +195,14 @@ func pkgBuildCmd() *cobra.Command { if _, err := os.Stat(builderConfig); err != nil { return fmt.Errorf("error reading builder config file %s: %w", builderConfig, err) } - opts = append(opts, pkglib.WithBuildBuilderConfig(builderConfig)) } opts = append(opts, pkglib.WithBuildBuilders(buildersMap)) - opts = append(opts, pkglib.WithBuildBuilderImage(builderImage)) - opts = append(opts, pkglib.WithBuildBuilderRestart(builderRestart)) + opts = append(opts, pkglib.WithBuildBuilderConfig(pkglib.BuilderConfig{ + Image: builderImage, + ConfigPath: builderConfig, + Restart: builderRestart, + })) opts = append(opts, pkglib.WithProgress(progress)) if len(ssh) > 0 { opts = append(opts, pkglib.WithSSH(ssh)) diff --git a/src/cmd/linuxkit/pkg_builder.go b/src/cmd/linuxkit/pkg_builder.go index 1f3700874..dd745f957 100644 --- a/src/cmd/linuxkit/pkg_builder.go +++ b/src/cmd/linuxkit/pkg_builder.go @@ -39,13 +39,17 @@ func pkgBuilderCmd() *cobra.Command { } platformsToClean := strings.Split(platforms, ",") + bc := pkglib.BuilderConfig{ + Image: builderImage, + ConfigPath: builderConfigPath, + } switch command { case "du": - if err := pkglib.DiskUsage(buildersMap, builderImage, builderConfigPath, platformsToClean, verbose); err != nil { + if err := pkglib.DiskUsage(buildersMap, bc, platformsToClean, verbose); err != nil { return fmt.Errorf("unable to print disk usage of builder: %w", err) } case "prune": - if err := pkglib.PruneBuilder(buildersMap, builderImage, builderConfigPath, platformsToClean, verbose); err != nil { + if err := pkglib.PruneBuilder(buildersMap, bc, platformsToClean, verbose); err != nil { return fmt.Errorf("unable to prune builder: %w", err) } default: diff --git a/src/cmd/linuxkit/pkglib/build.go b/src/cmd/linuxkit/pkglib/build.go index b1df746d9..d29b808dc 100644 --- a/src/cmd/linuxkit/pkglib/build.go +++ b/src/cmd/linuxkit/pkglib/build.go @@ -24,32 +24,30 @@ import ( ) type buildOpts struct { - skipBuild bool - force bool - pull bool - ignoreCache bool - push bool - dryRun bool - preCacheImages bool - release string - manifest bool - targetDocker bool - cacheDir string - cacheProvider spec.CacheProvider - platforms []imagespec.Platform - builders map[string]string - runner DockerRunner - writer io.Writer - builderImage string - builderConfigPath string - builderRestart bool - sbomScan bool - sbomScannerImage string - dockerfile string - buildArgs []string - progress string - ssh []string - registryAuth map[string]spec.RegistryAuth + skipBuild bool + force bool + pull bool + ignoreCache bool + push bool + dryRun bool + preCacheImages bool + release string + manifest bool + targetDocker bool + cacheDir string + cacheProvider spec.CacheProvider + platforms []imagespec.Platform + builders map[string]string + runner DockerRunner + writer io.Writer + builderConfig BuilderConfig + sbomScan bool + sbomScannerImage string + dockerfile string + buildArgs []string + progress string + ssh []string + registryAuth map[string]spec.RegistryAuth } // BuildOpt allows callers to specify options to Build @@ -160,26 +158,10 @@ func WithBuildOutputWriter(w io.Writer) BuildOpt { } } -// WithBuildBuilderImage set the builder container image to use. -func WithBuildBuilderImage(image string) BuildOpt { +// WithBuildBuilderConfig set the builder configuration to use. +func WithBuildBuilderConfig(bc BuilderConfig) BuildOpt { return func(bo *buildOpts) error { - bo.builderImage = image - return nil - } -} - -// WithBuildBuilderConfig set the contents of the -func WithBuildBuilderConfig(builderConfigPath string) BuildOpt { - return func(bo *buildOpts) error { - bo.builderConfigPath = builderConfigPath - return nil - } -} - -// WithBuildBuilderRestart restart the builder container even if it already is running with the correct image version -func WithBuildBuilderRestart(restart bool) BuildOpt { - return func(bo *buildOpts) error { - bo.builderRestart = restart + bo.builderConfig = bc return nil } } @@ -322,7 +304,7 @@ func (p Pkg) Build(bos ...BuildOpt) error { case d == nil && bo.dryRun: d = NewDockerDryRunner() case d == nil: - d = NewDockerRunner(p.cache) + d = NewDockerRunner(p.cache, bo.builderConfig) } c := bo.cacheProvider @@ -492,7 +474,7 @@ func (p Pkg) Build(bos ...BuildOpt) error { // build for each arch and save in the linuxkit cache for _, platform := range platformsToBuild { - builtDescs, err := p.buildArch(ctx, d, c, bo.builderImage, bo.builderConfigPath, platform.Architecture, bo.builderRestart, writer, bo, imageBuildOpts) + builtDescs, err := p.buildArch(ctx, d, c, platform.Architecture, writer, bo, imageBuildOpts) if err != nil { return fmt.Errorf("error building for arch %s: %v", platform.Architecture, err) } @@ -645,7 +627,7 @@ func (p Pkg) Build(bos ...BuildOpt) error { // C - manifest, saved in cache as is, referenced by the index (E), and returned as a descriptor // D - attestations (if any), saved in cache as is, referenced by the index (E), and returned as a descriptor // E - index, saved in cache as is, stored in cache as tag "image:tag-arch", *not* returned as a descriptor -func (p Pkg) buildArch(ctx context.Context, d DockerRunner, c spec.CacheProvider, builderImage, builderConfigPath, arch string, restart bool, writer io.Writer, bo buildOpts, imageBuildOpts spec.ImageBuildOptions) ([]registry.Descriptor, error) { +func (p Pkg) buildArch(ctx context.Context, d DockerRunner, c spec.CacheProvider, arch string, writer io.Writer, bo buildOpts, imageBuildOpts spec.ImageBuildOptions) ([]registry.Descriptor, error) { var ( tagArch string tag = p.FullTag() @@ -674,8 +656,8 @@ func (p Pkg) buildArch(ctx context.Context, d DockerRunner, c spec.CacheProvider return nil, err } - // find the desired builder - builderName := getBuilderForPlatform(arch, bo.builders) + // find the desired docker context for the builder + dockerContext := getBuilderForPlatform(arch, bo.builders) // set the target var ( @@ -714,7 +696,7 @@ func (p Pkg) buildArch(ctx context.Context, d DockerRunner, c spec.CacheProvider imageBuildOpts.Dockerfile = bo.dockerfile - if err := d.Build(ctx, tagArch, p.path, builderName, builderImage, builderConfigPath, platform, restart, bo.preCacheImages, passCache, buildCtx.Reader(), stdout, bo.sbomScan, bo.sbomScannerImage, bo.progress, imageBuildOpts); err != nil { + if err := d.Build(ctx, tagArch, p.path, dockerContext, platform, bo.preCacheImages, passCache, buildCtx.Reader(), stdout, bo.sbomScan, bo.sbomScannerImage, bo.progress, imageBuildOpts); err != nil { stdoutCloser() if strings.Contains(err.Error(), "executor failed running [/dev/.buildkit_qemu_emulator") { return nil, fmt.Errorf("buildkit was unable to emulate %s. check binfmt has been set up and works for this platform: %v", platform, err) diff --git a/src/cmd/linuxkit/pkglib/build_test.go b/src/cmd/linuxkit/pkglib/build_test.go index 273c26acf..d4974e442 100644 --- a/src/cmd/linuxkit/pkglib/build_test.go +++ b/src/cmd/linuxkit/pkglib/build_test.go @@ -53,10 +53,10 @@ func (d *dockerMocker) ContextSupportCheck() error { } return errors.New("contexts not supported") } -func (d *dockerMocker) Builder(_ context.Context, _, _, _, _ string, _ bool) (*buildkitClient.Client, error) { +func (d *dockerMocker) Builder(_ context.Context, _, _ string) (*buildkitClient.Client, error) { return nil, fmt.Errorf("not implemented") } -func (d *dockerMocker) Build(ctx context.Context, tag, pkg, dockerContext, builderImage, builderConfigPath, platform string, builderRestart, preCacheImages bool, c spec.CacheProvider, r io.Reader, stdout io.Writer, sbomScan bool, sbomScannerImage, progress string, imageBuildOpts spec.ImageBuildOptions) error { +func (d *dockerMocker) Build(ctx context.Context, tag, pkg, dockerContext, platform string, preCacheImages bool, c spec.CacheProvider, r io.Reader, stdout io.Writer, sbomScan bool, sbomScannerImage, progress string, imageBuildOpts spec.ImageBuildOptions) error { if !d.enableBuild { return errors.New("build disabled") } diff --git a/src/cmd/linuxkit/pkglib/docker.go b/src/cmd/linuxkit/pkglib/docker.go index 5c21d086c..65c68eac0 100644 --- a/src/cmd/linuxkit/pkglib/docker.go +++ b/src/cmd/linuxkit/pkglib/docker.go @@ -18,12 +18,25 @@ import ( _ "github.com/moby/buildkit/client/connhelper/ssh" ) +// BuilderConfig holds configuration for the buildkit builder container. +type BuilderConfig struct { + // Name is the container name for the buildkit builder (e.g., "linuxkit-builder-alice"). + Name string + // Image is the container image to run (e.g., "moby/buildkit:v0.26.3"). + Image string + // ConfigPath is an optional path to a buildkitd.toml config file. + ConfigPath string + // Restart forces recreation of the builder container even if one with the + // correct name and image already exists. + Restart bool +} + type DockerRunner interface { Tag(ref, tag string) error - Build(ctx context.Context, tag, pkg, dockerContext, builderImage, builderConfigPath, platform string, restart, preCacheImages bool, c spec.CacheProvider, r io.Reader, stdout io.Writer, sbomScan bool, sbomScannerImage, platformType string, imageBuildOpts spec.ImageBuildOptions) error + Build(ctx context.Context, tag, pkg, dockerContext, platform string, preCacheImages bool, c spec.CacheProvider, r io.Reader, stdout io.Writer, sbomScan bool, sbomScannerImage, platformType string, imageBuildOpts spec.ImageBuildOptions) error Save(tgt string, refs ...string) error Load(src io.Reader) error Pull(img string) (bool, error) ContextSupportCheck() error - Builder(ctx context.Context, dockerContext, builderImage, builderConfigPath, platform string, restart bool) (*buildkitClient.Client, error) + Builder(ctx context.Context, dockerContext, platform string) (*buildkitClient.Client, error) } diff --git a/src/cmd/linuxkit/pkglib/dockerdryrun.go b/src/cmd/linuxkit/pkglib/dockerdryrun.go index c91186780..3f2bf7743 100644 --- a/src/cmd/linuxkit/pkglib/dockerdryrun.go +++ b/src/cmd/linuxkit/pkglib/dockerdryrun.go @@ -46,7 +46,7 @@ func (dr *dockerDryRunnerImpl) ContextSupportCheck() error { // 1. if dockerContext is provided, try to create a builder with that context; if it succeeds, we are done; if not, return an error. // 2. try to find an existing named runner with the pattern; if it succeeds, we are done; if not, try next. // 3. try to create a generic builder using the default context named "linuxkit". -func (dr *dockerDryRunnerImpl) Builder(ctx context.Context, dockerContext, builderImage, builderConfigPath, platform string, restart bool) (*buildkitClient.Client, error) { +func (dr *dockerDryRunnerImpl) Builder(ctx context.Context, dockerContext, platform string) (*buildkitClient.Client, error) { return nil, nil } @@ -58,7 +58,7 @@ func (dr *dockerDryRunnerImpl) Tag(ref, tag string) error { return errors.New("not implemented") } -func (dr *dockerDryRunnerImpl) Build(ctx context.Context, tag, pkg, dockerContext, builderImage, builderConfigPath, platform string, restart, preCacheImages bool, c spec.CacheProvider, stdin io.Reader, stdout io.Writer, sbomScan bool, sbomScannerImage, progressType string, imageBuildOpts spec.ImageBuildOptions) error { +func (dr *dockerDryRunnerImpl) Build(ctx context.Context, tag, pkg, dockerContext, platform string, preCacheImages bool, c spec.CacheProvider, stdin io.Reader, stdout io.Writer, sbomScan bool, sbomScannerImage, progressType string, imageBuildOpts spec.ImageBuildOptions) error { // build args var buildArgs []string for k, v := range imageBuildOpts.BuildArgs { diff --git a/src/cmd/linuxkit/pkglib/dockerimpl.go b/src/cmd/linuxkit/pkglib/dockerimpl.go index 63a34a005..51762248d 100644 --- a/src/cmd/linuxkit/pkglib/dockerimpl.go +++ b/src/cmd/linuxkit/pkglib/dockerimpl.go @@ -68,11 +68,15 @@ const ( ) type dockerRunnerImpl struct { - cache bool + cache bool + builder BuilderConfig } -func NewDockerRunner(cache bool) DockerRunner { - return &dockerRunnerImpl{cache: cache} +func NewDockerRunner(cache bool, bc BuilderConfig) DockerRunner { + if bc.Name == "" { + bc.Name = buildkitBuilderName + } + return &dockerRunnerImpl{cache: cache, builder: bc} } func isExecErrNotFound(err error) bool { @@ -219,14 +223,15 @@ func (dr *dockerRunnerImpl) ContextSupportCheck() error { // 1. if dockerContext is provided, try to create a builder with that context; if it succeeds, we are done; if not, return an error. // 2. try to find an existing named runner with the pattern; if it succeeds, we are done; if not, try next. // 3. try to create a generic builder using the default context named "linuxkit". -func (dr *dockerRunnerImpl) Builder(ctx context.Context, dockerContext, builderImage, builderConfigPath, platform string, restart bool) (*buildkitClient.Client, error) { +func (dr *dockerRunnerImpl) Builder(ctx context.Context, dockerContext, platform string) (*buildkitClient.Client, error) { + bc := dr.builder // if we were given a context, we must find a builder and use it, or create one and use it if dockerContext != "" { // does the context exist? if err := dr.command(nil, io.Discard, io.Discard, "context", "inspect", dockerContext); err != nil { return nil, fmt.Errorf("provided docker context '%s' not found", dockerContext) } - client, err := dr.builderEnsureContainer(ctx, buildkitBuilderName, builderImage, builderConfigPath, platform, dockerContext, restart) + client, err := dr.builderEnsureContainer(ctx, bc.Name, bc.Image, bc.ConfigPath, platform, dockerContext, bc.Restart) if err != nil { return nil, fmt.Errorf("error preparing builder based on context '%s': %v", dockerContext, err) } @@ -237,13 +242,13 @@ func (dr *dockerRunnerImpl) Builder(ctx context.Context, dockerContext, builderI dockerContext = fmt.Sprintf("%s-%s", "linuxkit", strings.ReplaceAll(platform, "/", "-")) if err := dr.command(nil, io.Discard, io.Discard, "context", "inspect", dockerContext); err == nil { // we found an appropriately named context, so let us try to use it or error out - if client, err := dr.builderEnsureContainer(ctx, buildkitBuilderName, builderImage, builderConfigPath, platform, dockerContext, restart); err == nil { + if client, err := dr.builderEnsureContainer(ctx, bc.Name, bc.Image, bc.ConfigPath, platform, dockerContext, bc.Restart); err == nil { return client, nil } } // create a generic builder - client, err := dr.builderEnsureContainer(ctx, buildkitBuilderName, builderImage, builderConfigPath, "", "default", restart) + client, err := dr.builderEnsureContainer(ctx, bc.Name, bc.Image, bc.ConfigPath, "", "default", bc.Restart) if err != nil { return nil, fmt.Errorf("error ensuring builder container in default context: %v", err) } @@ -507,9 +512,9 @@ func (dr *dockerRunnerImpl) Tag(ref, tag string) error { return dr.command(nil, nil, nil, "image", "tag", ref, tag) } -func (dr *dockerRunnerImpl) Build(ctx context.Context, tag, pkg, dockerContext, builderImage, builderConfigPath, platform string, restart, preCacheImages bool, c spec.CacheProvider, stdin io.Reader, stdout io.Writer, sbomScan bool, sbomScannerImage, progressType string, imageBuildOpts spec.ImageBuildOptions) error { +func (dr *dockerRunnerImpl) Build(ctx context.Context, tag, pkg, dockerContext, platform string, preCacheImages bool, c spec.CacheProvider, stdin io.Reader, stdout io.Writer, sbomScan bool, sbomScannerImage, progressType string, imageBuildOpts spec.ImageBuildOptions) error { // ensure we have a builder - client, err := dr.Builder(ctx, dockerContext, builderImage, builderConfigPath, platform, restart) + client, err := dr.Builder(ctx, dockerContext, platform) if err != nil { return fmt.Errorf("unable to ensure builder container: %v", err) } diff --git a/src/cmd/linuxkit/pkglib/utils.go b/src/cmd/linuxkit/pkglib/utils.go index 9be24882e..68c9a5896 100644 --- a/src/cmd/linuxkit/pkglib/utils.go +++ b/src/cmd/linuxkit/pkglib/utils.go @@ -93,14 +93,14 @@ func printVerbose(tw *tabwriter.Writer, du []*buildkitClient.UsageInfo) { _ = tw.Flush() } -func getClientForPlatform(ctx context.Context, buildersMap map[string]string, builderImage, builderConfigPath, platform string) (*buildkitClient.Client, error) { +func getClientForPlatform(ctx context.Context, buildersMap map[string]string, bc BuilderConfig, platform string) (*buildkitClient.Client, error) { p, err := platforms.Parse(platform) if err != nil { return nil, fmt.Errorf("failed to parse platform: %s", err) } - dr := NewDockerRunner(false) - builderName := getBuilderForPlatform(p.Architecture, buildersMap) - client, err := dr.Builder(ctx, builderName, builderImage, builderConfigPath, platform, false) + dr := NewDockerRunner(false, bc) + dockerContext := getBuilderForPlatform(p.Architecture, buildersMap) + client, err := dr.Builder(ctx, dockerContext, platform) if err != nil { return nil, fmt.Errorf("unable to ensure builder container: %v", err) } @@ -108,11 +108,11 @@ func getClientForPlatform(ctx context.Context, buildersMap map[string]string, bu } // DiskUsage of builder -func DiskUsage(buildersMap map[string]string, builderImage, builderConfigPath string, platformsToClean []string, verbose bool) error { +func DiskUsage(buildersMap map[string]string, bc BuilderConfig, platformsToClean []string, verbose bool) error { ctx, cancel := context.WithCancel(context.Background()) defer cancel() for _, platform := range platformsToClean { - client, err := getClientForPlatform(ctx, buildersMap, builderImage, builderConfigPath, platform) + client, err := getClientForPlatform(ctx, buildersMap, bc, platform) if err != nil { return fmt.Errorf("cannot get client: %s", err) } @@ -143,12 +143,12 @@ func DiskUsage(buildersMap map[string]string, builderImage, builderConfigPath st } // PruneBuilder clean build cache of builder -func PruneBuilder(buildersMap map[string]string, builderImage, builderConfigPath string, platformsToClean []string, verbose bool) error { +func PruneBuilder(buildersMap map[string]string, bc BuilderConfig, platformsToClean []string, verbose bool) error { ctx, cancel := context.WithCancel(context.Background()) defer cancel() total := int64(0) for _, platform := range platformsToClean { - client, err := getClientForPlatform(ctx, buildersMap, builderImage, builderConfigPath, platform) + client, err := getClientForPlatform(ctx, buildersMap, bc, platform) if err != nil { return fmt.Errorf("cannot get client: %s", err) }