From 8e5c906a3cd2c1290ef446a3cf6e1e8979186540 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 10 Feb 2026 11:40:48 +0100 Subject: [PATCH] daemon/internal/builder-next: add executorOpts struct The newExecutor and newExecutorGD constructors started to gain a long list of arguments, some of which were platform-specific and not used on other platforms. Add a basic struct to pass options, which also allows documenting platform-specific fields through comments, and makes it easier to maintain platform-specific stubs. Signed-off-by: Sebastiaan van Stijn --- daemon/internal/builder-next/controller.go | 55 ++++++++++--------- .../internal/builder-next/executor_linux.go | 53 ++++++------------ .../internal/builder-next/executor_nolinux.go | 6 +- daemon/internal/builder-next/executor_opts.go | 29 ++++++++++ .../internal/builder-next/executor_others.go | 6 +- .../internal/builder-next/executor_windows.go | 39 ++++--------- 6 files changed, 88 insertions(+), 100 deletions(-) create mode 100644 daemon/internal/builder-next/executor_opts.go diff --git a/daemon/internal/builder-next/controller.go b/daemon/internal/builder-next/controller.go index e356a92827..9381fc50bf 100644 --- a/daemon/internal/builder-next/controller.go +++ b/daemon/internal/builder-next/controller.go @@ -154,19 +154,19 @@ func newSnapshotterController(ctx context.Context, rt http.RoundTripper, opt Opt wo.RegistryHosts = opt.RegistryHosts wo.Labels = getLabels(opt, wo.Labels) - exec, err := newExecutor( - opt.Root, - opt.DefaultCgroupParent, - opt.NetworkController, - dnsConfig, - opt.Rootless, - opt.IdentityMapping, - opt.ApparmorProfile, - cdiManager, - opt.ContainerdAddress, - opt.ContainerdNamespace, - opt.HyperVIsolation, - ) + exec, err := newExecutor(executorOpts{ + root: opt.Root, + networkController: opt.NetworkController, + dnsConfig: dnsConfig, + cdiManager: cdiManager, + cgroupParent: opt.DefaultCgroupParent, + apparmorProfile: opt.ApparmorProfile, + rootless: opt.Rootless, + identityMapping: opt.IdentityMapping, + containerdAddr: opt.ContainerdAddress, + containerdNamespace: opt.ContainerdNamespace, + hypervIsolation: opt.HyperVIsolation, + }) if err != nil { return nil, err } @@ -354,25 +354,26 @@ func newGraphDriverController(ctx context.Context, rt http.RoundTripper, opt Opt return nil, err } - dns := getDNSConfig(opt.DNSConfig) - cdiManager, err := getCDIManager(opt) if err != nil { return nil, err } - exec, err := newExecutorGD( - root, - opt.DefaultCgroupParent, - opt.NetworkController, - dns, - opt.Rootless, - opt.IdentityMapping, - opt.ApparmorProfile, - cdiManager, - opt.ContainerdAddress, - opt.ContainerdNamespace, - ) + exec, err := newExecutorGD(executorOpts{ + root: root, + networkController: opt.NetworkController, + dnsConfig: getDNSConfig(opt.DNSConfig), + cdiManager: cdiManager, + cgroupParent: opt.DefaultCgroupParent, + apparmorProfile: opt.ApparmorProfile, + rootless: opt.Rootless, + identityMapping: opt.IdentityMapping, + + // Windows-only fields (currently not used, as newExecutorGD is not implemented on Windows) + containerdAddr: opt.ContainerdAddress, + containerdNamespace: opt.ContainerdNamespace, + hypervIsolation: opt.HyperVIsolation, + }) if err != nil { return nil, err } diff --git a/daemon/internal/builder-next/executor_linux.go b/daemon/internal/builder-next/executor_linux.go index 277c9fe745..39fb27ed01 100644 --- a/daemon/internal/builder-next/executor_linux.go +++ b/daemon/internal/builder-next/executor_linux.go @@ -8,24 +8,20 @@ import ( "github.com/containerd/log" "github.com/moby/buildkit/executor" - "github.com/moby/buildkit/executor/oci" "github.com/moby/buildkit/executor/resources" "github.com/moby/buildkit/executor/runcexecutor" - "github.com/moby/buildkit/solver/llbsolver/cdidevices" "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/network" "github.com/moby/moby/v2/daemon/internal/stringid" - "github.com/moby/moby/v2/daemon/libnetwork" - "github.com/moby/sys/user" "github.com/opencontainers/runtime-spec/specs-go" ) const networkName = "bridge" -func newExecutor(root, cgroupParent string, net *libnetwork.Controller, dnsConfig *oci.DNSConfig, rootless bool, idmap user.IdentityMapping, apparmorProfile string, cdiManager *cdidevices.Manager, _, _ string, _ bool) (executor.Executor, error) { - netRoot := filepath.Join(root, "net") +func newExecutor(opts executorOpts) (executor.Executor, error) { + netRoot := filepath.Join(opts.root, "net") networkProviders := map[pb.NetMode]network.Provider{ - pb.NetMode_UNSET: &bridgeProvider{Controller: net, Root: netRoot}, + pb.NetMode_UNSET: &bridgeProvider{Controller: opts.networkController, Root: netRoot}, pb.NetMode_HOST: network.NewHostProvider(), pb.NetMode_NONE: network.NewNoneProvider(), } @@ -43,9 +39,9 @@ func newExecutor(root, cgroupParent string, net *libnetwork.Controller, dnsConfi // Returning a non-nil but empty *IdentityMapping breaks BuildKit: // https://github.com/moby/moby/pull/39444 - pidmap := &idmap - if idmap.Empty() { - pidmap = nil + idmap := &opts.identityMapping + if opts.identityMapping.Empty() { + idmap = nil } rm, err := resources.NewMonitor() @@ -53,43 +49,30 @@ func newExecutor(root, cgroupParent string, net *libnetwork.Controller, dnsConfi return nil, err } - runcCmds := []string{"runc"} - // TODO: FIXME: testing env var, replace with something better or remove in a major version or two + runcCmds := []string{"runc"} if runcOverride := os.Getenv("DOCKER_BUILDKIT_RUNC_COMMAND"); runcOverride != "" { runcCmds = []string{runcOverride} } return runcexecutor.New(runcexecutor.Opt{ - Root: filepath.Join(root, "executor"), + Root: filepath.Join(opts.root, "executor"), CommandCandidates: runcCmds, - DefaultCgroupParent: cgroupParent, - Rootless: rootless, + DefaultCgroupParent: opts.cgroupParent, + Rootless: opts.rootless, NoPivot: os.Getenv("DOCKER_RAMDISK") != "", - IdentityMapping: pidmap, - DNS: dnsConfig, - ApparmorProfile: apparmorProfile, + IdentityMapping: idmap, + DNS: opts.dnsConfig, + ApparmorProfile: opts.apparmorProfile, ResourceMonitor: rm, - CDIManager: cdiManager, + CDIManager: opts.cdiManager, }, networkProviders) } -// newExecutorGD calls newExecutor() on Linux. -// Created for symmetry with the non-linux platforms, esp. Windows. -func newExecutorGD(root, cgroupParent string, net *libnetwork.Controller, dnsConfig *oci.DNSConfig, rootless bool, idmap user.IdentityMapping, apparmorProfile string, cdiManager *cdidevices.Manager, _, _ string) (executor.Executor, error) { - return newExecutor( - root, - cgroupParent, - net, - dnsConfig, - rootless, - idmap, - apparmorProfile, - cdiManager, - "", - "", - false, - ) +// newExecutorGD calls newExecutor() on Linux. It returns a stubExecutor on +// other platforms. +func newExecutorGD(opts executorOpts) (executor.Executor, error) { + return newExecutor(opts) } func (iface *lnInterface) Set(s *specs.Spec) error { diff --git a/daemon/internal/builder-next/executor_nolinux.go b/daemon/internal/builder-next/executor_nolinux.go index 2761fd4cad..5affd09761 100644 --- a/daemon/internal/builder-next/executor_nolinux.go +++ b/daemon/internal/builder-next/executor_nolinux.go @@ -8,11 +8,7 @@ import ( "runtime" "github.com/moby/buildkit/executor" - "github.com/moby/buildkit/executor/oci" resourcetypes "github.com/moby/buildkit/executor/resources/types" - "github.com/moby/buildkit/solver/llbsolver/cdidevices" - "github.com/moby/moby/v2/daemon/libnetwork" - "github.com/moby/sys/user" ) type stubExecutor struct{} @@ -26,6 +22,6 @@ func (w *stubExecutor) Exec(ctx context.Context, id string, process executor.Pro } // function stub created for GraphDriver -func newExecutorGD(_, _ string, _ *libnetwork.Controller, _ *oci.DNSConfig, _ bool, _ user.IdentityMapping, _ string, _ *cdidevices.Manager, _, _ string) (executor.Executor, error) { +func newExecutorGD(executorOpts) (executor.Executor, error) { return &stubExecutor{}, nil } diff --git a/daemon/internal/builder-next/executor_opts.go b/daemon/internal/builder-next/executor_opts.go new file mode 100644 index 0000000000..5e526879ae --- /dev/null +++ b/daemon/internal/builder-next/executor_opts.go @@ -0,0 +1,29 @@ +package buildkit + +import ( + "github.com/moby/buildkit/executor/oci" + "github.com/moby/buildkit/solver/llbsolver/cdidevices" + "github.com/moby/moby/v2/daemon/libnetwork" + "github.com/moby/sys/user" +) + +// executorOpts holds options for constructing an executor. It contains fields +// used on Linux, Windows, or both. +type executorOpts struct { + // common fields + root string + networkController *libnetwork.Controller + dnsConfig *oci.DNSConfig + cdiManager *cdidevices.Manager + + // linux-only fields + cgroupParent string + apparmorProfile string + rootless bool + identityMapping user.IdentityMapping + + // windows-only fields + containerdAddr string + containerdNamespace string + hypervIsolation bool +} diff --git a/daemon/internal/builder-next/executor_others.go b/daemon/internal/builder-next/executor_others.go index da8abb7d10..becb9790fc 100644 --- a/daemon/internal/builder-next/executor_others.go +++ b/daemon/internal/builder-next/executor_others.go @@ -4,12 +4,8 @@ package buildkit import ( "github.com/moby/buildkit/executor" - "github.com/moby/buildkit/executor/oci" - "github.com/moby/buildkit/solver/llbsolver/cdidevices" - "github.com/moby/moby/v2/daemon/libnetwork" - "github.com/moby/sys/user" ) -func newExecutor(_, _ string, _ *libnetwork.Controller, _ *oci.DNSConfig, _ bool, _ user.IdentityMapping, _ string, _ *cdidevices.Manager, _, _ string, _ bool) (executor.Executor, error) { +func newExecutor(executorOpts) (executor.Executor, error) { return &stubExecutor{}, nil } diff --git a/daemon/internal/builder-next/executor_windows.go b/daemon/internal/builder-next/executor_windows.go index 7a36a26b96..202dda0500 100644 --- a/daemon/internal/builder-next/executor_windows.go +++ b/daemon/internal/builder-next/executor_windows.go @@ -9,51 +9,34 @@ import ( "github.com/containerd/log" "github.com/moby/buildkit/executor" "github.com/moby/buildkit/executor/containerdexecutor" - "github.com/moby/buildkit/executor/oci" - "github.com/moby/buildkit/solver/llbsolver/cdidevices" "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/network" - "github.com/moby/moby/v2/daemon/libnetwork" - "github.com/moby/sys/user" "github.com/opencontainers/runtime-spec/specs-go" ) const networkName = "nat" -func newExecutor( - root string, - _ string, - net *libnetwork.Controller, - dns *oci.DNSConfig, - _ bool, - _ user.IdentityMapping, - _ string, - cdiManager *cdidevices.Manager, - containerdAddr string, - containerdNamespace string, - hypervIsolation bool, -) (executor.Executor, error) { - netRoot := filepath.Join(root, "net") +func newExecutor(opts executorOpts) (executor.Executor, error) { + netRoot := filepath.Join(opts.root, "net") np := map[pb.NetMode]network.Provider{ - pb.NetMode_UNSET: &bridgeProvider{Controller: net, Root: netRoot}, + pb.NetMode_UNSET: &bridgeProvider{Controller: opts.networkController, Root: netRoot}, pb.NetMode_NONE: network.NewNoneProvider(), } - opt := ctd.WithDefaultNamespace(containerdNamespace) - client, err := ctd.New(containerdAddr, opt) + opt := ctd.WithDefaultNamespace(opts.containerdNamespace) + client, err := ctd.New(opts.containerdAddr, opt) if err != nil { return nil, err } - executorOpts := containerdexecutor.ExecutorOptions{ + return containerdexecutor.New(containerdexecutor.ExecutorOptions{ Client: client, - Root: root, - DNSConfig: dns, - CDIManager: cdiManager, + Root: opts.root, + DNSConfig: opts.dnsConfig, + CDIManager: opts.cdiManager, NetworkProviders: np, - HyperVIsolation: hypervIsolation, - } - return containerdexecutor.New(executorOpts), nil + HyperVIsolation: opts.hypervIsolation, + }), nil } func (iface *lnInterface) Set(s *specs.Spec) error {