Merge pull request #46008 from thaJeztah/c8d_nits

c8d: some minor cleanups

daemon/content.go

@@ -16,7 +16,7 @@ import (
 	"go.etcd.io/bbolt"
 )
 
-func (daemon *Daemon) configureLocalContentStore(ns string) (content.Store, leases.Manager, error) {
+func (daemon *Daemon) configureLocalContentStore(ns string) (*namespacedContent, *namespacedLeases, error) {
 	if err := os.MkdirAll(filepath.Join(daemon.root, "content"), 0o700); err != nil {
 		return nil, nil, errors.Wrap(err, "error creating dir for content store")
 	}
@@ -30,7 +30,15 @@ func (daemon *Daemon) configureLocalContentStore(ns string) (content.Store, leas
 	}
 	md := metadata.NewDB(db, cs, nil)
 	daemon.mdDB = db
-	return namespacedContentProvider(md.ContentStore(), ns), namespacedLeaseManager(metadata.NewLeaseManager(md), ns), nil
+	cp := &namespacedContent{
+		ns:       ns,
+		provider: md.ContentStore(),
+	}
+	lm := &namespacedLeases{
+		ns:      ns,
+		manager: metadata.NewLeaseManager(md),
+	}
+	return cp, lm, nil
 }
 
 // withDefaultNamespace sets the given namespace on the context if the current
@@ -105,14 +113,6 @@ func (cp namespacedContent) ReaderAt(ctx context.Context, desc ocispec.Descripto
 	return cp.provider.ReaderAt(withDefaultNamespace(ctx, cp.ns), desc)
 }
 
-// namespacedContentProvider sets the namespace if missing before calling the inner provider
-func namespacedContentProvider(provider content.Store, ns string) content.Store {
-	return namespacedContent{
-		ns,
-		provider,
-	}
-}
-
 type namespacedLeases struct {
 	ns      string
 	manager leases.Manager
@@ -147,11 +147,3 @@ func (nl namespacedLeases) List(ctx context.Context, filter ...string) ([]leases
 func (nl namespacedLeases) ListResources(ctx context.Context, lease leases.Lease) ([]leases.Resource, error) {
 	return nl.manager.ListResources(withDefaultNamespace(ctx, nl.ns), lease)
 }
-
-// namespacedLeaseManager sets the namespace if missing before calling the inner manager
-func namespacedLeaseManager(manager leases.Manager, ns string) leases.Manager {
-	return namespacedLeases{
-		ns,
-		manager,
-	}
-}

daemon/daemon.go

@@ -110,7 +110,7 @@ type Daemon struct {
 	PluginStore           *plugin.Store // TODO: remove
 	pluginManager         *plugin.Manager
 	linkIndex             *linkIndex
-	containerdCli         *containerd.Client
+	containerdClient      *containerd.Client
 	containerd            libcontainerdtypes.Client
 	defaultIsolation      containertypes.Isolation // Default isolation mode on Windows
 	clusterProvider       cluster.Provider
@@ -554,14 +554,14 @@ func (daemon *Daemon) restore(cfg *configStore) error {
 	}
 	group.Wait()
 
-	for c, notifier := range restartContainers {
+	for c, notifyChan := range restartContainers {
 		group.Add(1)
 		go func(c *container.Container, chNotify chan struct{}) {
 			_ = sem.Acquire(context.Background(), 1)
 
-			log := log.G(context.TODO()).WithField("container", c.ID)
+			logger := log.G(context.TODO()).WithField("container", c.ID)
 
-			log.Debug("starting container")
+			logger.Debug("starting container")
 
 			// ignore errors here as this is a best effort to wait for children to be
 			//   running before we try to start the container
@@ -579,16 +579,16 @@ func (daemon *Daemon) restore(cfg *configStore) error {
 			}
 
 			if err := daemon.prepareMountPoints(c); err != nil {
-				log.WithError(err).Error("failed to prepare mount points for container")
+				logger.WithError(err).Error("failed to prepare mount points for container")
 			}
 			if err := daemon.containerStart(context.Background(), cfg, c, "", "", true); err != nil {
-				log.WithError(err).Error("failed to start container")
+				logger.WithError(err).Error("failed to start container")
 			}
 			close(chNotify)
 
 			sem.Release(1)
 			group.Done()
-		}(c, notifier)
+		}(c, notifyChan)
 	}
 	group.Wait()
 
@@ -822,7 +822,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 	if err := initRuntimesDir(config); err != nil {
 		return nil, err
 	}
-	runtimes, err := setupRuntimes(config)
+	rts, err := setupRuntimes(config)
 	if err != nil {
 		return nil, err
 	}
@@ -831,11 +831,11 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 		PluginStore: pluginStore,
 		startupDone: make(chan struct{}),
 	}
-	configStore := &configStore{
+	cfgStore := &configStore{
 		Config:   *config,
-		Runtimes: runtimes,
+		Runtimes: rts,
 	}
-	d.configStore.Store(configStore)
+	d.configStore.Store(cfgStore)
 
 	// TEST_INTEGRATION_USE_SNAPSHOTTER is used for integration tests only.
 	if os.Getenv("TEST_INTEGRATION_USE_SNAPSHOTTER") != "" {
@@ -855,27 +855,27 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 		}
 	}()
 
-	if err := d.setGenericResources(&configStore.Config); err != nil {
+	if err := d.setGenericResources(&cfgStore.Config); err != nil {
 		return nil, err
 	}
 	// set up SIGUSR1 handler on Unix-like systems, or a Win32 global event
 	// on Windows to dump Go routine stacks
-	stackDumpDir := configStore.Root
-	if execRoot := configStore.GetExecRoot(); execRoot != "" {
+	stackDumpDir := cfgStore.Root
+	if execRoot := cfgStore.GetExecRoot(); execRoot != "" {
 		stackDumpDir = execRoot
 	}
 	d.setupDumpStackTrap(stackDumpDir)
 
-	if err := d.setupSeccompProfile(&configStore.Config); err != nil {
+	if err := d.setupSeccompProfile(&cfgStore.Config); err != nil {
 		return nil, err
 	}
 
 	// Set the default isolation mode (only applicable on Windows)
-	if err := d.setDefaultIsolation(&configStore.Config); err != nil {
+	if err := d.setDefaultIsolation(&cfgStore.Config); err != nil {
 		return nil, fmt.Errorf("error setting default isolation mode: %v", err)
 	}
 
-	if err := configureMaxThreads(&configStore.Config); err != nil {
+	if err := configureMaxThreads(&cfgStore.Config); err != nil {
 		log.G(ctx).Warnf("Failed to configure golang's threads limit: %v", err)
 	}
 
@@ -884,7 +884,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 		log.G(ctx).Errorf(err.Error())
 	}
 
-	daemonRepo := filepath.Join(configStore.Root, "containers")
+	daemonRepo := filepath.Join(cfgStore.Root, "containers")
 	if err := idtools.MkdirAllAndChown(daemonRepo, 0o710, idtools.Identity{
 		UID: idtools.CurrentIdentity().UID,
 		GID: rootIDs.GID,
@@ -896,7 +896,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 		// Note that permissions (0o700) are ignored on Windows; passing them to
 		// show intent only. We could consider using idtools.MkdirAndChown here
 		// to apply an ACL.
-		if err = os.Mkdir(filepath.Join(configStore.Root, "credentialspecs"), 0o700); err != nil && !errors.Is(err, os.ErrExist) {
+		if err = os.Mkdir(filepath.Join(cfgStore.Root, "credentialspecs"), 0o700); err != nil && !errors.Is(err, os.ErrExist) {
 			return nil, err
 		}
 	}
@@ -904,7 +904,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 	d.registryService = registryService
 	dlogger.RegisterPluginGetter(d.PluginStore)
 
-	metricsSockPath, err := d.listenMetricsSock(&configStore.Config)
+	metricsSockPath, err := d.listenMetricsSock(&cfgStore.Config)
 	if err != nil {
 		return nil, err
 	}
@@ -943,20 +943,20 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 		grpc.WithDefaultCallOptions(grpc.MaxCallSendMsgSize(defaults.DefaultMaxSendMsgSize)),
 	}
 
-	if configStore.ContainerdAddr != "" {
-		d.containerdCli, err = containerd.New(configStore.ContainerdAddr, containerd.WithDefaultNamespace(configStore.ContainerdNamespace), containerd.WithDialOpts(gopts), containerd.WithTimeout(60*time.Second))
+	if cfgStore.ContainerdAddr != "" {
+		d.containerdClient, err = containerd.New(cfgStore.ContainerdAddr, containerd.WithDefaultNamespace(cfgStore.ContainerdNamespace), containerd.WithDialOpts(gopts), containerd.WithTimeout(60*time.Second))
 		if err != nil {
-			return nil, errors.Wrapf(err, "failed to dial %q", configStore.ContainerdAddr)
+			return nil, errors.Wrapf(err, "failed to dial %q", cfgStore.ContainerdAddr)
 		}
 	}
 
 	createPluginExec := func(m *plugin.Manager) (plugin.Executor, error) {
 		var pluginCli *containerd.Client
 
-		if configStore.ContainerdAddr != "" {
-			pluginCli, err = containerd.New(configStore.ContainerdAddr, containerd.WithDefaultNamespace(configStore.ContainerdPluginNamespace), containerd.WithDialOpts(gopts), containerd.WithTimeout(60*time.Second))
+		if cfgStore.ContainerdAddr != "" {
+			pluginCli, err = containerd.New(cfgStore.ContainerdAddr, containerd.WithDefaultNamespace(cfgStore.ContainerdPluginNamespace), containerd.WithDialOpts(gopts), containerd.WithTimeout(60*time.Second))
 			if err != nil {
-				return nil, errors.Wrapf(err, "failed to dial %q", configStore.ContainerdAddr)
+				return nil, errors.Wrapf(err, "failed to dial %q", cfgStore.ContainerdAddr)
 			}
 		}
 
@@ -965,22 +965,22 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 			shimOpts interface{}
 		)
 		if runtime.GOOS != "windows" {
-			shim, shimOpts, err = runtimes.Get("")
+			shim, shimOpts, err = rts.Get("")
 			if err != nil {
 				return nil, err
 			}
 		}
-		return pluginexec.New(ctx, getPluginExecRoot(&configStore.Config), pluginCli, configStore.ContainerdPluginNamespace, m, shim, shimOpts)
+		return pluginexec.New(ctx, getPluginExecRoot(&cfgStore.Config), pluginCli, cfgStore.ContainerdPluginNamespace, m, shim, shimOpts)
 	}
 
 	// Plugin system initialization should happen before restore. Do not change order.
 	d.pluginManager, err = plugin.NewManager(plugin.ManagerConfig{
-		Root:               filepath.Join(configStore.Root, "plugins"),
-		ExecRoot:           getPluginExecRoot(&configStore.Config),
+		Root:               filepath.Join(cfgStore.Root, "plugins"),
+		ExecRoot:           getPluginExecRoot(&cfgStore.Config),
 		Store:              d.PluginStore,
 		CreateExecutor:     createPluginExec,
 		RegistryService:    registryService,
-		LiveRestoreEnabled: configStore.LiveRestoreEnabled,
+		LiveRestoreEnabled: cfgStore.LiveRestoreEnabled,
 		LogPluginEvent:     d.LogPluginEvent, // todo: make private
 		AuthzMiddleware:    authzMiddleware,
 	})
@@ -988,13 +988,13 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 		return nil, errors.Wrap(err, "couldn't create plugin manager")
 	}
 
-	d.defaultLogConfig, err = defaultLogConfig(&configStore.Config)
+	d.defaultLogConfig, err = defaultLogConfig(&cfgStore.Config)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to set log opts")
 	}
 	log.G(ctx).Debugf("Using default logging driver %s", d.defaultLogConfig.Type)
 
-	d.volumes, err = volumesservice.NewVolumeService(configStore.Root, d.PluginStore, rootIDs, d)
+	d.volumes, err = volumesservice.NewVolumeService(cfgStore.Root, d.PluginStore, rootIDs, d)
 	if err != nil {
 		return nil, err
 	}
@@ -1007,11 +1007,11 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 	// at this point.
 	//
 	// TODO(thaJeztah) add a utility to only collect the CgroupDevicesEnabled information
-	if runtime.GOOS == "linux" && !userns.RunningInUserNS() && !getSysInfo(&configStore.Config).CgroupDevicesEnabled {
+	if runtime.GOOS == "linux" && !userns.RunningInUserNS() && !getSysInfo(&cfgStore.Config).CgroupDevicesEnabled {
 		return nil, errors.New("Devices cgroup isn't mounted")
 	}
 
-	d.id, err = loadOrCreateID(filepath.Join(configStore.Root, "engine-id"))
+	d.id, err = loadOrCreateID(filepath.Join(cfgStore.Root, "engine-id"))
 	if err != nil {
 		return nil, err
 	}
@@ -1024,7 +1024,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 	d.statsCollector = d.newStatsCollector(1 * time.Second)
 
 	d.EventsService = events.New()
-	d.root = configStore.Root
+	d.root = cfgStore.Root
 	d.idMapping = idMapping
 
 	d.linkIndex = newLinkIndex()
@@ -1039,7 +1039,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 	} else if driverName != "" {
 		log.G(ctx).Infof("Setting the storage driver from the $DOCKER_DRIVER environment variable (%s)", driverName)
 	} else {
-		driverName = configStore.GraphDriver
+		driverName = cfgStore.GraphDriver
 	}
 
 	if d.UsesSnapshotter() {
@@ -1055,11 +1055,11 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 
 		// Configure and validate the kernels security support. Note this is a Linux/FreeBSD
 		// operation only, so it is safe to pass *just* the runtime OS graphdriver.
-		if err := configureKernelSecuritySupport(&configStore.Config, driverName); err != nil {
+		if err := configureKernelSecuritySupport(&cfgStore.Config, driverName); err != nil {
 			return nil, err
 		}
 		d.imageService = ctrd.NewService(ctrd.ImageServiceConfig{
-			Client:          d.containerdCli,
+			Client:          d.containerdClient,
 			Containers:      d.containers,
 			Snapshotter:     driverName,
 			RegistryHosts:   d.RegistryHosts,
@@ -1069,13 +1069,13 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 		})
 	} else {
 		layerStore, err := layer.NewStoreFromOptions(layer.StoreOptions{
-			Root:                      configStore.Root,
-			MetadataStorePathTemplate: filepath.Join(configStore.Root, "image", "%s", "layerdb"),
+			Root:                      cfgStore.Root,
+			MetadataStorePathTemplate: filepath.Join(cfgStore.Root, "image", "%s", "layerdb"),
 			GraphDriver:               driverName,
-			GraphDriverOptions:        configStore.GraphOptions,
+			GraphDriverOptions:        cfgStore.GraphOptions,
 			IDMapping:                 idMapping,
 			PluginGetter:              d.PluginStore,
-			ExperimentalEnabled:       configStore.Experimental,
+			ExperimentalEnabled:       cfgStore.Experimental,
 		})
 		if err != nil {
 			return nil, err
@@ -1083,11 +1083,11 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 
 		// Configure and validate the kernels security support. Note this is a Linux/FreeBSD
 		// operation only, so it is safe to pass *just* the runtime OS graphdriver.
-		if err := configureKernelSecuritySupport(&configStore.Config, layerStore.DriverName()); err != nil {
+		if err := configureKernelSecuritySupport(&cfgStore.Config, layerStore.DriverName()); err != nil {
 			return nil, err
 		}
 
-		imageRoot := filepath.Join(configStore.Root, "image", layerStore.DriverName())
+		imageRoot := filepath.Join(cfgStore.Root, "image", layerStore.DriverName())
 		ifs, err := image.NewFSStoreBackend(filepath.Join(imageRoot, "imagedb"))
 		if err != nil {
 			return nil, err
@@ -1137,16 +1137,14 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 		// containerd is not currently supported with Windows.
 		// So sometimes d.containerdCli will be nil
 		// In that case we'll create a local content store... but otherwise we'll use containerd
-		if d.containerdCli != nil {
-			imgSvcConfig.Leases = d.containerdCli.LeasesService()
-			imgSvcConfig.ContentStore = d.containerdCli.ContentStore()
+		if d.containerdClient != nil {
+			imgSvcConfig.Leases = d.containerdClient.LeasesService()
+			imgSvcConfig.ContentStore = d.containerdClient.ContentStore()
 		} else {
-			cs, lm, err := d.configureLocalContentStore(config.ContainerdNamespace)
+			imgSvcConfig.ContentStore, imgSvcConfig.Leases, err = d.configureLocalContentStore(config.ContainerdNamespace)
 			if err != nil {
 				return nil, err
 			}
-			imgSvcConfig.ContentStore = cs
-			imgSvcConfig.Leases = lm
 		}
 
 		// TODO: imageStore, distributionMetadataStore, and ReferenceStore are only
@@ -1161,11 +1159,11 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 
 	go d.execCommandGC()
 
-	if err := d.initLibcontainerd(ctx, &configStore.Config); err != nil {
+	if err := d.initLibcontainerd(ctx, &cfgStore.Config); err != nil {
 		return nil, err
 	}
 
-	if err := d.restore(configStore); err != nil {
+	if err := d.restore(cfgStore); err != nil {
 		return nil, err
 	}
 	close(d.startupDone)
@@ -1316,8 +1314,8 @@ func (daemon *Daemon) Shutdown(ctx context.Context) error {
 		daemon.netController.Stop()
 	}
 
-	if daemon.containerdCli != nil {
-		daemon.containerdCli.Close()
+	if daemon.containerdClient != nil {
+		daemon.containerdClient.Close()
 	}
 
 	if daemon.mdDB != nil {

daemon/daemon_unix.go

@@ -1493,7 +1493,7 @@ func (daemon *Daemon) initLibcontainerd(ctx context.Context, cfg *config.Config)
 	var err error
 	daemon.containerd, err = remote.NewClient(
 		ctx,
-		daemon.containerdCli,
+		daemon.containerdClient,
 		filepath.Join(cfg.ExecRoot, "containerd"),
 		cfg.ContainerdNamespace,
 		daemon,

daemon/daemon_windows.go

@@ -576,7 +576,7 @@ func (daemon *Daemon) initLibcontainerd(ctx context.Context, cfg *config.Config)
 	case windowsV1RuntimeName:
 		daemon.containerd, err = local.NewClient(
 			ctx,
-			daemon.containerdCli,
+			daemon.containerdClient,
 			filepath.Join(cfg.ExecRoot, "containerd"),
 			cfg.ContainerdNamespace,
 			daemon,
@@ -587,7 +587,7 @@ func (daemon *Daemon) initLibcontainerd(ctx context.Context, cfg *config.Config)
 		}
 		daemon.containerd, err = remote.NewClient(
 			ctx,
-			daemon.containerdCli,
+			daemon.containerdClient,
 			filepath.Join(cfg.ExecRoot, "containerd"),
 			cfg.ContainerdNamespace,
 			daemon,

daemon/delete.go

@@ -144,7 +144,7 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, config ty
 		container.RWLayer = nil
 	} else {
 		if daemon.UsesSnapshotter() {
-			ls := daemon.containerdCli.LeasesService()
+			ls := daemon.containerdClient.LeasesService()
 			lease := leases.Lease{
 				ID: container.ID,
 			}

daemon/exec.go

@@ -218,7 +218,7 @@ func (daemon *Daemon) ContainerExecStart(ctx context.Context, name string, optio
 
 	p := &specs.Process{}
 	if runtime.GOOS != "windows" {
-		ctr, err := daemon.containerdCli.LoadContainer(ctx, ec.Container.ID)
+		ctr, err := daemon.containerdClient.LoadContainer(ctx, ec.Container.ID)
 		if err != nil {
 			return err
 		}

daemon/exec_linux.go

@@ -55,7 +55,7 @@ func (daemon *Daemon) execSetPlatformOpt(ctx context.Context, daemonCfg *config.
 	if len(ec.User) > 0 {
 		var err error
 		if daemon.UsesSnapshotter() {
-			p.User, err = getUserFromContainerd(ctx, daemon.containerdCli, ec)
+			p.User, err = getUserFromContainerd(ctx, daemon.containerdClient, ec)
 			if err != nil {
 				return err
 			}

daemon/oci_linux.go

@@ -1116,7 +1116,7 @@ func (daemon *Daemon) createSpec(ctx context.Context, daemonCfg *configStore, c
 		snapshotKey = c.ID
 	}
 
-	return &s, coci.ApplyOpts(ctx, daemon.containerdCli, &containers.Container{
+	return &s, coci.ApplyOpts(ctx, daemon.containerdClient, &containers.Container{
 		ID:          c.ID,
 		Snapshotter: snapshotter,
 		SnapshotKey: snapshotKey,