Merge pull request #43945 from thaJeztah/libcontainer_supervisor_cleanups

libcontainerd/supervisor: refactoring and cleanup in preparation of custom containerd.toml

cmd/dockerd/daemon.go

@@ -167,7 +167,7 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
 	}
 
 	ctx, cancel := context.WithCancel(context.Background())
-	waitForContainerDShutdown, err := cli.initContainerD(ctx)
+	waitForContainerDShutdown, err := cli.initContainerd(ctx)
 	if waitForContainerDShutdown != nil {
 		defer waitForContainerDShutdown(10 * time.Second)
 	}
@@ -589,13 +589,13 @@ func (cli *DaemonCli) getContainerdDaemonOpts() ([]supervisor.DaemonOpt, error)
 		return nil, err
 	}
 
-	if cli.Config.Debug {
+	if cli.Debug {
 		opts = append(opts, supervisor.WithLogLevel("debug"))
-	} else if cli.Config.LogLevel != "" {
-		opts = append(opts, supervisor.WithLogLevel(cli.Config.LogLevel))
+	} else {
+		opts = append(opts, supervisor.WithLogLevel(cli.LogLevel))
 	}
 
-	if !cli.Config.CriContainerd {
+	if !cli.CriContainerd {
 		// CRI support in the managed daemon is currently opt-in.
 		//
 		// It's disabled by default, originally because it was listening on

cmd/dockerd/daemon_unix.go

@@ -62,6 +62,9 @@ func getDaemonConfDir(_ string) (string, error) {
 
 func (cli *DaemonCli) getPlatformContainerdDaemonOpts() ([]supervisor.DaemonOpt, error) {
 	opts := []supervisor.DaemonOpt{
+		// TODO(thaJeztah) change this to use /proc/self/oom_score_adj instead,
+		// which would allow us to set the correct score even if dockerd's score
+		// was set through other means (such as systemd or "manually").
 		supervisor.WithOOMScore(cli.Config.OOMScoreAdjust),
 	}
 
@@ -129,33 +132,34 @@ func newCgroupParent(config *config.Config) string {
 	return cgroupParent
 }
 
-func (cli *DaemonCli) initContainerD(ctx context.Context) (func(time.Duration) error, error) {
-	var waitForShutdown func(time.Duration) error
-	if cli.Config.ContainerdAddr == "" {
-		systemContainerdAddr, ok, err := systemContainerdRunning(honorXDG)
-		if err != nil {
-			return nil, errors.Wrap(err, "could not determine whether the system containerd is running")
-		}
-		if !ok {
-			logrus.Debug("Containerd not running, starting daemon managed containerd")
-			opts, err := cli.getContainerdDaemonOpts()
-			if err != nil {
-				return nil, errors.Wrap(err, "failed to generate containerd options")
-			}
-
-			r, err := supervisor.Start(ctx, filepath.Join(cli.Config.Root, "containerd"), filepath.Join(cli.Config.ExecRoot, "containerd"), opts...)
-			if err != nil {
-				return nil, errors.Wrap(err, "failed to start containerd")
-			}
-			logrus.Debug("Started daemon managed containerd")
-			cli.Config.ContainerdAddr = r.Address()
-
-			// Try to wait for containerd to shutdown
-			waitForShutdown = r.WaitTimeout
-		} else {
-			cli.Config.ContainerdAddr = systemContainerdAddr
-		}
+func (cli *DaemonCli) initContainerd(ctx context.Context) (func(time.Duration) error, error) {
+	if cli.ContainerdAddr != "" {
+		// use system containerd at the given address.
+		return nil, nil
 	}
 
-	return waitForShutdown, nil
+	systemContainerdAddr, ok, err := systemContainerdRunning(honorXDG)
+	if err != nil {
+		return nil, errors.Wrap(err, "could not determine whether the system containerd is running")
+	}
+	if ok {
+		// detected a system containerd at the given address.
+		cli.ContainerdAddr = systemContainerdAddr
+		return nil, nil
+	}
+
+	logrus.Info("containerd not running, starting managed containerd")
+	opts, err := cli.getContainerdDaemonOpts()
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to generate containerd options")
+	}
+
+	r, err := supervisor.Start(ctx, filepath.Join(cli.Root, "containerd"), filepath.Join(cli.ExecRoot, "containerd"), opts...)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to start containerd")
+	}
+	cli.ContainerdAddr = r.Address()
+
+	// Try to wait for containerd to shutdown
+	return r.WaitTimeout, nil
 }

cmd/dockerd/daemon_windows.go

@@ -93,8 +93,8 @@ func newCgroupParent(config *config.Config) string {
 	return ""
 }
 
-func (cli *DaemonCli) initContainerD(_ context.Context) (func(time.Duration) error, error) {
-	system.InitContainerdRuntime(cli.Config.ContainerdAddr)
+func (cli *DaemonCli) initContainerd(_ context.Context) (func(time.Duration) error, error) {
+	system.InitContainerdRuntime(cli.ContainerdAddr)
 	return nil, nil
 }
 

libcontainerd/supervisor/remote_daemon.go

@@ -2,18 +2,17 @@ package supervisor // import "github.com/docker/docker/libcontainerd/supervisor"
 
 import (
 	"context"
-	"fmt"
 	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strconv"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/services/server/config"
+	"github.com/containerd/containerd/sys"
 	"github.com/docker/docker/pkg/system"
 	"github.com/pelletier/go-toml"
 	"github.com/pkg/errors"
@@ -31,18 +30,28 @@ const (
 )
 
 type remote struct {
-	sync.RWMutex
 	config.Config
 
+	// configFile is the location where the generated containerd configuration
+	// file is saved.
+	configFile string
+
 	daemonPid int
+	pidFile   string
 	logger    *logrus.Entry
 
 	daemonWaitCh  chan struct{}
 	daemonStartCh chan error
 	daemonStopCh  chan struct{}
 
-	rootDir  string
 	stateDir string
+
+	// oomScore adjusts the OOM score for the containerd process.
+	oomScore int
+
+	// logLevel overrides the containerd logging-level through the --log-level
+	// command-line option.
+	logLevel string
 }
 
 // Daemon represents a running containerd daemon
@@ -57,14 +66,15 @@ type DaemonOpt func(c *remote) error
 // Start starts a containerd daemon and monitors it
 func Start(ctx context.Context, rootDir, stateDir string, opts ...DaemonOpt) (Daemon, error) {
 	r := &remote{
-		rootDir:  rootDir,
 		stateDir: stateDir,
 		Config: config.Config{
 			Version: 2,
 			Root:    filepath.Join(rootDir, "daemon"),
 			State:   filepath.Join(stateDir, "daemon"),
 		},
+		configFile:    filepath.Join(stateDir, configFile),
 		daemonPid:     -1,
+		pidFile:       filepath.Join(stateDir, pidFile),
 		logger:        logrus.WithField("module", "libcontainerd"),
 		daemonStartCh: make(chan error, 1),
 		daemonStopCh:  make(chan struct{}),
@@ -114,8 +124,7 @@ func (r *remote) Address() string {
 	return r.GRPC.Address
 }
 func (r *remote) getContainerdPid() (int, error) {
-	pidFile := filepath.Join(r.stateDir, pidFile)
-	f, err := os.OpenFile(pidFile, os.O_RDWR, 0600)
+	f, err := os.OpenFile(r.pidFile, os.O_RDWR, 0600)
 	if err != nil {
 		if os.IsNotExist(err) {
 			return -1, nil
@@ -144,17 +153,16 @@ func (r *remote) getContainerdPid() (int, error) {
 }
 
 func (r *remote) getContainerdConfig() (string, error) {
-	path := filepath.Join(r.stateDir, configFile)
-	f, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)
+	f, err := os.OpenFile(r.configFile, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)
 	if err != nil {
-		return "", errors.Wrapf(err, "failed to open containerd config file at %s", path)
+		return "", errors.Wrapf(err, "failed to open containerd config file (%s)", r.configFile)
 	}
 	defer f.Close()
 
 	if err := toml.NewEncoder(f).Encode(r); err != nil {
-		return "", errors.Wrapf(err, "failed to write containerd config file (%s)", path)
+		return "", errors.Wrapf(err, "failed to write containerd config file (%s)", r.configFile)
 	}
-	return path, nil
+	return r.configFile, nil
 }
 
 func (r *remote) startContainerd() error {
@@ -165,8 +173,7 @@ func (r *remote) startContainerd() error {
 
 	if pid != -1 {
 		r.daemonPid = pid
-		logrus.WithField("pid", pid).
-			Infof("libcontainerd: %s is still running", binaryName)
+		r.logger.WithField("pid", pid).Infof("%s is still running", binaryName)
 		return nil
 	}
 
@@ -177,8 +184,8 @@ func (r *remote) startContainerd() error {
 
 	args := []string{"--config", configFile}
 
-	if r.Debug.Level != "" {
-		args = append(args, "--log-level", r.Debug.Level)
+	if r.logLevel != "" {
+		args = append(args, "--log-level", r.logLevel)
 	}
 
 	cmd := exec.Command(binaryName, args...)
@@ -208,18 +215,33 @@ func (r *remote) startContainerd() error {
 
 	r.daemonPid = cmd.Process.Pid
 
-	err = os.WriteFile(filepath.Join(r.stateDir, pidFile), []byte(fmt.Sprintf("%d", r.daemonPid)), 0660)
+	if err := r.adjustOOMScore(); err != nil {
+		r.logger.WithError(err).Warn("failed to adjust OOM score")
+	}
+
+	err = os.WriteFile(r.pidFile, []byte(strconv.Itoa(r.daemonPid)), 0660)
 	if err != nil {
 		system.KillProcess(r.daemonPid)
 		return errors.Wrap(err, "libcontainerd: failed to save daemon pid to disk")
 	}
 
-	logrus.WithField("pid", r.daemonPid).
-		Infof("libcontainerd: started new %s process", binaryName)
+	r.logger.WithField("pid", r.daemonPid).WithField("address", r.Address()).Infof("started new %s process", binaryName)
 
 	return nil
 }
 
+func (r *remote) adjustOOMScore() error {
+	if r.oomScore == 0 || r.daemonPid <= 1 {
+		// no score configured, or daemonPid contains an invalid PID (we don't
+		// expect containerd to be running as PID 1 :)).
+		return nil
+	}
+	if err := sys.SetOOMScore(r.daemonPid, r.oomScore); err != nil {
+		return errors.Wrap(err, "failed to adjust OOM score for containerd process")
+	}
+	return nil
+}
+
 func (r *remote) monitorDaemon(ctx context.Context) {
 	var (
 		transientFailureCount = 0
@@ -236,7 +258,7 @@ func (r *remote) monitorDaemon(ctx context.Context) {
 		}
 
 		// cleanup some files
-		os.Remove(filepath.Join(r.stateDir, pidFile))
+		_ = os.Remove(r.pidFile)
 
 		r.platformCleanup()
 
@@ -289,7 +311,7 @@ func (r *remote) monitorDaemon(ctx context.Context) {
 				delay = 100 * time.Millisecond
 				continue
 			}
-			logrus.WithField("address", r.GRPC.Address).Debug("Created containerd monitoring client")
+			r.logger.WithField("address", r.GRPC.Address).Debug("created containerd monitoring client")
 		}
 
 		if client != nil {

libcontainerd/supervisor/remote_daemon_linux.go

@@ -55,5 +55,5 @@ func (r *remote) killDaemon() {
 }
 
 func (r *remote) platformCleanup() {
-	os.Remove(filepath.Join(r.stateDir, sockFile))
+	_ = os.Remove(r.Address())
 }

libcontainerd/supervisor/remote_daemon_options.go

@@ -1,10 +1,14 @@
 package supervisor // import "github.com/docker/docker/libcontainerd/supervisor"
 
-// WithLogLevel defines which log level to starts containerd with.
-// This only makes sense if WithStartDaemon() was set to true.
+// WithLogLevel defines which log level to start containerd with.
 func WithLogLevel(lvl string) DaemonOpt {
 	return func(r *remote) error {
-		r.Debug.Level = lvl
+		if lvl == "info" {
+			// both dockerd and containerd default log-level is "info",
+			// so don't pass the default.
+			lvl = ""
+		}
+		r.logLevel = lvl
 		return nil
 	}
 }

libcontainerd/supervisor/remote_daemon_options_linux.go

@@ -3,7 +3,7 @@ package supervisor // import "github.com/docker/docker/libcontainerd/supervisor"
 // WithOOMScore defines the oom_score_adj to set for the containerd process.
 func WithOOMScore(score int) DaemonOpt {
 	return func(r *remote) error {
-		r.OOMScore = score
+		r.oomScore = score
 		return nil
 	}
 }