Merge pull request moby#5086 from Benehiko/containerd-worker-parameters

Refactor containerd `NewWorkerOpt` & containerdexecutor `New` parameters

Author: tonistiigi

cmd/buildkitd/main_containerd_worker.go

@@ -321,7 +321,25 @@ func containerdWorkerInitializer(c *cli.Context, common workerInitializerOpt) ([
 			Options: opts,
 		}
 	}
-	opt, err := containerd.NewWorkerOpt(common.config.Root, cfg.Address, snapshotter, cfg.Namespace, cfg.DefaultCgroupParent, cfg.Rootless, cfg.Labels, dns, nc, common.config.Workers.Containerd.ApparmorProfile, common.config.Workers.Containerd.SELinux, parallelismSem, common.traceSocket, runtime, ctd.WithTimeout(60*time.Second))
+
+	workerOpts := containerd.WorkerOptions{
+		Root:            common.config.Root,
+		Address:         cfg.Address,
+		SnapshotterName: snapshotter,
+		Namespace:       cfg.Namespace,
+		CgroupParent:    cfg.DefaultCgroupParent,
+		Rootless:        cfg.Rootless,
+		Labels:          cfg.Labels,
+		DNS:             dns,
+		NetworkOpt:      nc,
+		ApparmorProfile: common.config.Workers.Containerd.ApparmorProfile,
+		Selinux:         common.config.Workers.Containerd.SELinux,
+		ParallelismSem:  parallelismSem,
+		TraceSocket:     common.traceSocket,
+		Runtime:         runtime,
+	}
+
+	opt, err := containerd.NewWorkerOpt(workerOpts, ctd.WithTimeout(60*time.Second))
 	if err != nil {
 		return nil, err
 	}

executor/containerdexecutor/executor.go

@@ -60,24 +60,37 @@ type RuntimeInfo struct {
 	Options any
 }
 
+type ExecutorOptions struct {
+	Client           *containerd.Client
+	Root             string
+	CgroupParent     string
+	NetworkProviders map[pb.NetMode]network.Provider
+	DNSConfig        *oci.DNSConfig
+	ApparmorProfile  string
+	Selinux          bool
+	TraceSocket      string
+	Rootless         bool
+	Runtime          *RuntimeInfo
+}
+
 // New creates a new executor backed by connection to containerd API
-func New(client *containerd.Client, root, cgroup string, networkProviders map[pb.NetMode]network.Provider, dnsConfig *oci.DNSConfig, apparmorProfile string, selinux bool, traceSocket string, rootless bool, runtime *RuntimeInfo) executor.Executor {
+func New(executorOpts ExecutorOptions) executor.Executor {
 	// clean up old hosts/resolv.conf file. ignore errors
-	os.RemoveAll(filepath.Join(root, "hosts"))
-	os.RemoveAll(filepath.Join(root, "resolv.conf"))
+	os.RemoveAll(filepath.Join(executorOpts.Root, "hosts"))
+	os.RemoveAll(filepath.Join(executorOpts.Root, "resolv.conf"))
 
 	return &containerdExecutor{
-		client:           client,
-		root:             root,
-		networkProviders: networkProviders,
-		cgroupParent:     cgroup,
-		dnsConfig:        dnsConfig,
+		client:           executorOpts.Client,
+		root:             executorOpts.Root,
+		networkProviders: executorOpts.NetworkProviders,
+		cgroupParent:     executorOpts.CgroupParent,
+		dnsConfig:        executorOpts.DNSConfig,
 		running:          make(map[string]*containerState),
-		apparmorProfile:  apparmorProfile,
-		selinux:          selinux,
-		traceSocket:      traceSocket,
-		rootless:         rootless,
-		runtime:          runtime,
+		apparmorProfile:  executorOpts.ApparmorProfile,
+		selinux:          executorOpts.Selinux,
+		traceSocket:      executorOpts.TraceSocket,
+		rootless:         executorOpts.Rootless,
+		runtime:          executorOpts.Runtime,
 	}
 }
 

worker/containerd/containerd.go

@@ -30,22 +30,31 @@ import (
 
 type RuntimeInfo = containerdexecutor.RuntimeInfo
 
+type WorkerOptions struct {
+	Root            string
+	Address         string
+	SnapshotterName string
+	Namespace       string
+	CgroupParent    string
+	Rootless        bool
+	Labels          map[string]string
+	DNS             *oci.DNSConfig
+	NetworkOpt      netproviders.Opt
+	ApparmorProfile string
+	Selinux         bool
+	ParallelismSem  *semaphore.Weighted
+	TraceSocket     string
+	Runtime         *RuntimeInfo
+}
+
 // NewWorkerOpt creates a WorkerOpt.
 func NewWorkerOpt(
-	root string,
-	address, snapshotterName, ns, cgroupParent string,
-	rootless bool,
-	labels map[string]string,
-	dns *oci.DNSConfig,
-	nopt netproviders.Opt,
-	apparmorProfile string,
-	selinux bool,
-	parallelismSem *semaphore.Weighted,
-	traceSocket string,
-	runtime *RuntimeInfo,
+	workerOpts WorkerOptions,
 	opts ...containerd.ClientOpt,
 ) (base.WorkerOpt, error) {
-	opts = append(opts, containerd.WithDefaultNamespace(ns))
+	opts = append(opts, containerd.WithDefaultNamespace(workerOpts.Namespace))
+
+	address := workerOpts.Address
 
 	if goRuntime.GOOS == "windows" {
 		// TODO(profnandaa): once the upstream PR[1] is merged and
@@ -58,29 +67,17 @@ func NewWorkerOpt(
 		return base.WorkerOpt{}, errors.Wrapf(err, "failed to connect client to %q . make sure containerd is running", address)
 	}
 	return newContainerd(
-		root,
 		client,
-		snapshotterName,
-		ns,
-		cgroupParent,
-		rootless,
-		labels,
-		dns,
-		nopt,
-		apparmorProfile,
-		selinux,
-		parallelismSem,
-		traceSocket,
-		runtime,
+		workerOpts,
 	)
 }
 
-func newContainerd(root string, client *containerd.Client, snapshotterName, ns, cgroupParent string, rootless bool, labels map[string]string, dns *oci.DNSConfig, nopt netproviders.Opt, apparmorProfile string, selinux bool, parallelismSem *semaphore.Weighted, traceSocket string, runtime *RuntimeInfo) (base.WorkerOpt, error) {
-	if strings.Contains(snapshotterName, "/") {
-		return base.WorkerOpt{}, errors.Errorf("bad snapshotter name: %q", snapshotterName)
+func newContainerd(client *containerd.Client, workerOpts WorkerOptions) (base.WorkerOpt, error) {
+	if strings.Contains(workerOpts.SnapshotterName, "/") {
+		return base.WorkerOpt{}, errors.Errorf("bad snapshotter name: %q", workerOpts.SnapshotterName)
 	}
-	name := "containerd-" + snapshotterName
-	root = filepath.Join(root, name)
+	name := "containerd-" + workerOpts.SnapshotterName
+	root := filepath.Join(workerOpts.Root, name)
 	if err := os.MkdirAll(root, 0700); err != nil {
 		return base.WorkerOpt{}, errors.Wrapf(err, "failed to create %s", root)
 	}
@@ -97,7 +94,7 @@ func newContainerd(root string, client *containerd.Client, snapshotterName, ns,
 		return base.WorkerOpt{}, err
 	}
 
-	np, npResolvedMode, err := netproviders.Providers(nopt)
+	np, npResolvedMode, err := netproviders.Providers(workerOpts.NetworkOpt)
 	if err != nil {
 		return base.WorkerOpt{}, err
 	}
@@ -108,21 +105,21 @@ func newContainerd(root string, client *containerd.Client, snapshotterName, ns,
 	}
 	xlabels := map[string]string{
 		wlabel.Executor:       "containerd",
-		wlabel.Snapshotter:    snapshotterName,
+		wlabel.Snapshotter:    workerOpts.SnapshotterName,
 		wlabel.Hostname:       hostname,
 		wlabel.Network:        npResolvedMode,
-		wlabel.SELinuxEnabled: strconv.FormatBool(selinux),
+		wlabel.SELinuxEnabled: strconv.FormatBool(workerOpts.Selinux),
 	}
-	if apparmorProfile != "" {
-		xlabels[wlabel.ApparmorProfile] = apparmorProfile
+	if workerOpts.ApparmorProfile != "" {
+		xlabels[wlabel.ApparmorProfile] = workerOpts.ApparmorProfile
 	}
-	xlabels[wlabel.ContainerdNamespace] = ns
+	xlabels[wlabel.ContainerdNamespace] = workerOpts.Namespace
 	xlabels[wlabel.ContainerdUUID] = serverInfo.UUID
-	for k, v := range labels {
+	for k, v := range workerOpts.Labels {
 		xlabels[k] = v
 	}
 
-	lm := leaseutil.WithNamespace(client.LeasesService(), ns)
+	lm := leaseutil.WithNamespace(client.LeasesService(), workerOpts.Namespace)
 
 	gc := func(ctx context.Context) (gc.Stats, error) {
 		l, err := lm.Create(ctx)
@@ -132,7 +129,7 @@ func newContainerd(root string, client *containerd.Client, snapshotterName, ns,
 		return nil, lm.Delete(ctx, leases.Lease{ID: l.ID}, leases.SynchronousDelete)
 	}
 
-	cs := containerdsnapshot.NewContentStore(client.ContentStore(), ns)
+	cs := containerdsnapshot.NewContentStore(client.ContentStore(), workerOpts.Namespace)
 
 	resp, err := client.IntrospectionService().Plugins(context.TODO(), []string{"type==io.containerd.runtime.v1", "type==io.containerd.runtime.v2"})
 	if err != nil {
@@ -154,7 +151,7 @@ func newContainerd(root string, client *containerd.Client, snapshotterName, ns,
 		}
 	}
 
-	snap := containerdsnapshot.NewSnapshotter(snapshotterName, client.SnapshotService(snapshotterName), ns, nil)
+	snap := containerdsnapshot.NewSnapshotter(workerOpts.SnapshotterName, client.SnapshotService(workerOpts.SnapshotterName), workerOpts.Namespace, nil)
 
 	if err := cache.MigrateV2(
 		context.TODO(),
@@ -172,12 +169,25 @@ func newContainerd(root string, client *containerd.Client, snapshotterName, ns,
 		return base.WorkerOpt{}, err
 	}
 
+	executorOpts := containerdexecutor.ExecutorOptions{
+		Client:           client,
+		Root:             root,
+		CgroupParent:     workerOpts.CgroupParent,
+		ApparmorProfile:  workerOpts.ApparmorProfile,
+		DNSConfig:        workerOpts.DNS,
+		Selinux:          workerOpts.Selinux,
+		TraceSocket:      workerOpts.TraceSocket,
+		Rootless:         workerOpts.Rootless,
+		Runtime:          workerOpts.Runtime,
+		NetworkProviders: np,
+	}
+
 	opt := base.WorkerOpt{
 		ID:               id,
 		Labels:           xlabels,
 		MetadataStore:    md,
 		NetworkProviders: np,
-		Executor:         containerdexecutor.New(client, root, cgroupParent, np, dns, apparmorProfile, selinux, traceSocket, rootless, runtime),
+		Executor:         containerdexecutor.New(executorOpts),
 		Snapshotter:      snap,
 		ContentStore:     cs,
 		Applier:          winlayers.NewFileSystemApplierWithWindows(cs, df),
@@ -186,7 +196,7 @@ func newContainerd(root string, client *containerd.Client, snapshotterName, ns,
 		Platforms:        platformSpecs,
 		LeaseManager:     lm,
 		GarbageCollect:   gc,
-		ParallelismSem:   parallelismSem,
+		ParallelismSem:   workerOpts.ParallelismSem,
 		MountPoolRoot:    filepath.Join(root, "cachemounts"),
 	}
 	return opt, nil

worker/containerd/containerd_test.go

@@ -31,7 +31,23 @@ func TestContainerdWorkerIntegration(t *testing.T) {
 func newWorkerOpt(t *testing.T, addr string) base.WorkerOpt {
 	tmpdir := t.TempDir()
 	rootless := false
-	workerOpt, err := NewWorkerOpt(tmpdir, addr, "overlayfs", "buildkit-test", "", rootless, nil, nil, netproviders.Opt{Mode: "host"}, "", false, nil, "", nil)
+	options := WorkerOptions{
+		Root:            tmpdir,
+		Address:         addr,
+		SnapshotterName: "overlayfs",
+		Namespace:       "buildkit-test",
+		CgroupParent:    "",
+		Rootless:        rootless,
+		Labels:          nil,
+		DNS:             nil,
+		NetworkOpt:      netproviders.Opt{Mode: "host"},
+		ApparmorProfile: "",
+		Selinux:         false,
+		ParallelismSem:  nil,
+		TraceSocket:     "",
+		Runtime:         nil,
+	}
+	workerOpt, err := NewWorkerOpt(options)
 	require.NoError(t, err)
 	return workerOpt
 }