From b28e66cf4fd2fb7c7f14d889614f57f3457f585e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 6 Jun 2022 21:11:08 +0200 Subject: [PATCH] daemon/config: New(): initialize config with platform-specific defaults This centralizes more defaults, to be part of the config struct that's created, instead of interweaving the defaults with other code in various places. Signed-off-by: Sebastiaan van Stijn --- cmd/dockerd/config.go | 14 ------ cmd/dockerd/config_unix.go | 69 +++------------------------- cmd/dockerd/config_unix_test.go | 5 +- cmd/dockerd/config_windows.go | 15 ------ cmd/dockerd/docker.go | 6 ++- daemon/config/config.go | 18 ++++++-- daemon/config/config_linux.go | 50 ++++++++++++++++++++ daemon/config/config_linux_test.go | 3 +- daemon/config/config_test.go | 10 ++-- daemon/config/config_windows.go | 10 ++++ daemon/config/config_windows_test.go | 3 +- daemon/runtime_unix_test.go | 5 +- 12 files changed, 102 insertions(+), 106 deletions(-) diff --git a/cmd/dockerd/config.go b/cmd/dockerd/config.go index a1c5a3c42b..a4bb8699f1 100644 --- a/cmd/dockerd/config.go +++ b/cmd/dockerd/config.go @@ -12,20 +12,6 @@ const defaultTrustKeyFile = "key.json" // installCommonConfigFlags adds flags to the pflag.FlagSet to configure the daemon func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error { - var err error - conf.Pidfile, err = getDefaultPidFile() - if err != nil { - return err - } - conf.Root, err = getDefaultDataRoot() - if err != nil { - return err - } - conf.ExecRoot, err = getDefaultExecRoot() - if err != nil { - return err - } - var ( allowNonDistributable = opts.NewNamedListOptsRef("allow-nondistributable-artifacts", &conf.AllowNondistributableArtifacts, registry.ValidateIndexName) registryMirrors = opts.NewNamedListOptsRef("registry-mirrors", &conf.Mirrors, registry.ValidateMirror) diff --git a/cmd/dockerd/config_unix.go b/cmd/dockerd/config_unix.go index 1c750eb20e..89148d81c4 100644 --- a/cmd/dockerd/config_unix.go +++ b/cmd/dockerd/config_unix.go @@ -5,18 +5,13 @@ package main import ( "net" - "os/exec" "path/filepath" - "github.com/containerd/cgroups" - "github.com/docker/docker/api/types" "github.com/docker/docker/daemon/config" "github.com/docker/docker/opts" "github.com/docker/docker/pkg/homedir" "github.com/docker/docker/registry" "github.com/docker/docker/rootless" - units "github.com/docker/go-units" - "github.com/pkg/errors" "github.com/spf13/pflag" ) @@ -27,12 +22,6 @@ func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) error { return err } - conf.Ulimits = make(map[string]*units.Ulimit) - - // Set default value for `--default-shm-size` - conf.ShmSize = opts.MemBytes(config.DefaultShmSize) - conf.Runtimes = make(map[string]types.Runtime) - // Then platform-specific install flags flags.Var(opts.NewNamedRuntimeOpt("runtimes", &conf.Runtimes, config.StockRuntimeName), "add-runtime", "Register an additional OCI compatible runtime") flags.StringVarP(&conf.SocketGroup, "group", "G", "docker", "Group for the unix socket") @@ -53,16 +42,7 @@ func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) error { flags.BoolVar(&conf.BridgeConfig.InterContainerCommunication, "icc", true, "Enable inter-container communication") flags.IPVar(&conf.BridgeConfig.DefaultIP, "ip", net.IPv4zero, "Default IP when binding container ports") flags.BoolVar(&conf.BridgeConfig.EnableUserlandProxy, "userland-proxy", true, "Use userland proxy for loopback traffic") - defaultUserlandProxyPath := "" - if rootless.RunningWithRootlessKit() { - var err error - // use rootlesskit-docker-proxy for exposing the ports in RootlessKit netns to the initial namespace. - defaultUserlandProxyPath, err = exec.LookPath(rootless.RootlessKitDockerProxyBinary) - if err != nil { - return errors.Wrapf(err, "running with RootlessKit, but %s not installed", rootless.RootlessKitDockerProxyBinary) - } - } - flags.StringVar(&conf.BridgeConfig.UserlandProxyPath, "userland-proxy-path", defaultUserlandProxyPath, "Path to the userland proxy binary") + flags.StringVar(&conf.BridgeConfig.UserlandProxyPath, "userland-proxy-path", conf.BridgeConfig.UserlandProxyPath, "Path to the userland proxy binary") flags.StringVar(&conf.CgroupParent, "cgroup-parent", "", "Set parent cgroup for all containers") flags.StringVar(&conf.RemappedRoot, "userns-remap", "", "User/Group setting for user namespaces") flags.BoolVar(&conf.LiveRestoreEnabled, "live-restore", false, "Enable live restore of docker when containers are still running") @@ -71,19 +51,15 @@ func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) error { flags.StringVar(&conf.InitPath, "init-path", "", "Path to the docker-init binary") flags.Int64Var(&conf.CPURealtimePeriod, "cpu-rt-period", 0, "Limit the CPU real-time period in microseconds for the parent cgroup for all containers (not supported with cgroups v2)") flags.Int64Var(&conf.CPURealtimeRuntime, "cpu-rt-runtime", 0, "Limit the CPU real-time runtime in microseconds for the parent cgroup for all containers (not supported with cgroups v2)") - flags.StringVar(&conf.SeccompProfile, "seccomp-profile", config.SeccompProfileDefault, `Path to seccomp profile. Use "unconfined" to disable the default seccomp profile`) + flags.StringVar(&conf.SeccompProfile, "seccomp-profile", conf.SeccompProfile, `Path to seccomp profile. Use "unconfined" to disable the default seccomp profile`) flags.Var(&conf.ShmSize, "default-shm-size", "Default shm size for containers") flags.BoolVar(&conf.NoNewPrivileges, "no-new-privileges", false, "Set no-new-privileges by default for new containers") - flags.StringVar(&conf.IpcMode, "default-ipc-mode", string(config.DefaultIpcMode), `Default mode for containers ipc ("shareable" | "private")`) + flags.StringVar(&conf.IpcMode, "default-ipc-mode", conf.IpcMode, `Default mode for containers ipc ("shareable" | "private")`) flags.Var(&conf.NetworkConfig.DefaultAddressPools, "default-address-pool", "Default address pools for node specific local networks") // rootless needs to be explicitly specified for running "rootful" dockerd in rootless dockerd (#38702) - // Note that defaultUserlandProxyPath and honorXDG are configured according to the value of rootless.RunningWithRootlessKit, not the value of --rootless. - flags.BoolVar(&conf.Rootless, "rootless", rootless.RunningWithRootlessKit(), "Enable rootless mode; typically used with RootlessKit") - defaultCgroupNamespaceMode := config.DefaultCgroupNamespaceMode - if cgroups.Mode() != cgroups.Unified { - defaultCgroupNamespaceMode = config.DefaultCgroupV1NamespaceMode - } - flags.StringVar(&conf.CgroupNamespaceMode, "default-cgroupns-mode", string(defaultCgroupNamespaceMode), `Default mode for containers cgroup namespace ("host" | "private")`) + // Note that conf.BridgeConfig.UserlandProxyPath and honorXDG are configured according to the value of rootless.RunningWithRootlessKit, not the value of --rootless. + flags.BoolVar(&conf.Rootless, "rootless", conf.Rootless, "Enable rootless mode; typically used with RootlessKit") + flags.StringVar(&conf.CgroupNamespaceMode, "default-cgroupns-mode", conf.CgroupNamespaceMode, `Default mode for containers cgroup namespace ("host" | "private")`) return nil } @@ -97,36 +73,3 @@ func configureCertsDir() { } } } - -func getDefaultPidFile() (string, error) { - if !honorXDG { - return "/var/run/docker.pid", nil - } - runtimeDir, err := homedir.GetRuntimeDir() - if err != nil { - return "", err - } - return filepath.Join(runtimeDir, "docker.pid"), nil -} - -func getDefaultDataRoot() (string, error) { - if !honorXDG { - return "/var/lib/docker", nil - } - dataHome, err := homedir.GetDataHome() - if err != nil { - return "", err - } - return filepath.Join(dataHome, "docker"), nil -} - -func getDefaultExecRoot() (string, error) { - if !honorXDG { - return "/var/run/docker", nil - } - runtimeDir, err := homedir.GetRuntimeDir() - if err != nil { - return "", err - } - return filepath.Join(runtimeDir, "docker"), nil -} diff --git a/cmd/dockerd/config_unix_test.go b/cmd/dockerd/config_unix_test.go index 1ffe4c1fbf..b07ce8a24b 100644 --- a/cmd/dockerd/config_unix_test.go +++ b/cmd/dockerd/config_unix_test.go @@ -15,8 +15,9 @@ import ( func TestDaemonParseShmSize(t *testing.T) { flags := pflag.NewFlagSet("test", pflag.ContinueOnError) - conf := &config.Config{} - err := installConfigFlags(conf, flags) + conf, err := config.New() + assert.NilError(t, err) + err = installConfigFlags(conf, flags) assert.NilError(t, err) // By default `--default-shm-size=64M` assert.Check(t, is.Equal(int64(64*1024*1024), conf.ShmSize.Value())) diff --git a/cmd/dockerd/config_windows.go b/cmd/dockerd/config_windows.go index df96861066..d8bebf4486 100644 --- a/cmd/dockerd/config_windows.go +++ b/cmd/dockerd/config_windows.go @@ -1,25 +1,10 @@ package main import ( - "os" - "path/filepath" - "github.com/docker/docker/daemon/config" "github.com/spf13/pflag" ) -func getDefaultPidFile() (string, error) { - return "", nil -} - -func getDefaultDataRoot() (string, error) { - return filepath.Join(os.Getenv("programdata"), "docker"), nil -} - -func getDefaultExecRoot() (string, error) { - return filepath.Join(os.Getenv("programdata"), "docker", "exec-root"), nil -} - // installConfigFlags adds flags to the pflag.FlagSet to configure the daemon func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) error { // First handle install flags which are consistent cross-platform diff --git a/cmd/dockerd/docker.go b/cmd/dockerd/docker.go index 4b6d55d653..90df537bdd 100644 --- a/cmd/dockerd/docker.go +++ b/cmd/dockerd/docker.go @@ -21,7 +21,11 @@ var ( ) func newDaemonCommand() (*cobra.Command, error) { - opts := newDaemonOptions(config.New()) + cfg, err := config.New() + if err != nil { + return nil, err + } + opts := newDaemonOptions(cfg) cmd := &cobra.Command{ Use: "dockerd [OPTIONS]", diff --git a/daemon/config/config.go b/daemon/config/config.go index 645beb8c03..482bbe2d62 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -275,9 +275,10 @@ func (conf *Config) IsValueSet(name string) bool { return ok } -// New returns a new fully initialized Config struct -func New() *Config { - return &Config{ +// New returns a new fully initialized Config struct with default values set. +func New() (*Config, error) { + // platform-agnostic default values for the Config. + cfg := &Config{ CommonConfig: CommonConfig{ ShutdownTimeout: DefaultShutdownTimeout, LogConfig: LogConfig{ @@ -295,6 +296,12 @@ func New() *Config { DefaultRuntime: StockRuntimeName, }, } + + if err := setPlatformDefaults(cfg); err != nil { + return nil, err + } + + return cfg, nil } // GetConflictFreeLabels validates Labels for conflict @@ -329,7 +336,10 @@ func Reload(configFile string, flags *pflag.FlagSet, reload func(*Config)) error if flags.Changed("config-file") || !os.IsNotExist(err) { return errors.Wrapf(err, "unable to configure the Docker daemon with file %s", configFile) } - newConfig = New() + newConfig, err = New() + if err != nil { + return err + } } // Check if duplicate label-keys with different values are found diff --git a/daemon/config/config_linux.go b/daemon/config/config_linux.go index 27d1c11806..70d1efda8d 100644 --- a/daemon/config/config_linux.go +++ b/daemon/config/config_linux.go @@ -3,11 +3,17 @@ package config // import "github.com/docker/docker/daemon/config" import ( "fmt" "net" + "os/exec" + "path/filepath" + "github.com/containerd/cgroups" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/opts" + "github.com/docker/docker/pkg/homedir" + "github.com/docker/docker/rootless" units "github.com/docker/go-units" + "github.com/pkg/errors" ) const ( @@ -161,3 +167,47 @@ func (conf *Config) ValidatePlatformConfig() error { func (conf *Config) IsRootless() bool { return conf.Rootless } + +func setPlatformDefaults(cfg *Config) error { + cfg.Ulimits = make(map[string]*units.Ulimit) + cfg.ShmSize = opts.MemBytes(DefaultShmSize) + cfg.SeccompProfile = SeccompProfileDefault + cfg.IpcMode = string(DefaultIpcMode) + cfg.Runtimes = make(map[string]types.Runtime) + + if cgroups.Mode() != cgroups.Unified { + cfg.CgroupNamespaceMode = string(DefaultCgroupV1NamespaceMode) + } else { + cfg.CgroupNamespaceMode = string(DefaultCgroupNamespaceMode) + } + + if rootless.RunningWithRootlessKit() { + cfg.Rootless = true + + var err error + // use rootlesskit-docker-proxy for exposing the ports in RootlessKit netns to the initial namespace. + cfg.BridgeConfig.UserlandProxyPath, err = exec.LookPath(rootless.RootlessKitDockerProxyBinary) + if err != nil { + return errors.Wrapf(err, "running with RootlessKit, but %s not installed", rootless.RootlessKitDockerProxyBinary) + } + + dataHome, err := homedir.GetDataHome() + if err != nil { + return err + } + runtimeDir, err := homedir.GetRuntimeDir() + if err != nil { + return err + } + + cfg.Root = filepath.Join(dataHome, "docker") + cfg.ExecRoot = filepath.Join(runtimeDir, "docker") + cfg.Pidfile = filepath.Join(runtimeDir, "docker.pid") + } else { + cfg.Root = "/var/lib/docker" + cfg.ExecRoot = "/var/run/docker" + cfg.Pidfile = "/var/run/docker.pid" + } + + return nil +} diff --git a/daemon/config/config_linux_test.go b/daemon/config/config_linux_test.go index a9f571f223..7b8d776281 100644 --- a/daemon/config/config_linux_test.go +++ b/daemon/config/config_linux_test.go @@ -69,7 +69,8 @@ func TestDaemonConfigurationMerge(t *testing.T) { file := fs.NewFile(t, "docker-config", fs.WithContent(configFileData)) defer file.Remove() - conf := New() + conf, err := New() + assert.NilError(t, err) flags := pflag.NewFlagSet("test", pflag.ContinueOnError) flags.BoolVarP(&conf.Debug, "debug", "D", false, "") diff --git a/daemon/config/config_test.go b/daemon/config/config_test.go index ae571c0182..882dcaedff 100644 --- a/daemon/config/config_test.go +++ b/daemon/config/config_test.go @@ -338,13 +338,14 @@ func TestValidateConfigurationErrors(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - cfg := New() + cfg, err := New() + assert.NilError(t, err) if tc.field != "" { assert.Check(t, mergo.Merge(cfg, tc.config, mergo.WithOverride, withForceOverwrite(tc.field))) } else { assert.Check(t, mergo.Merge(cfg, tc.config, mergo.WithOverride)) } - err := Validate(cfg) + err = Validate(cfg) assert.Error(t, err, tc.expectedErr) }) } @@ -481,12 +482,13 @@ func TestValidateConfiguration(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { // Start with a config with all defaults set, so that we only - cfg := New() + cfg, err := New() + assert.NilError(t, err) assert.Check(t, mergo.Merge(cfg, tc.config, mergo.WithOverride)) // Check that the override happened :) assert.Check(t, is.DeepEqual(cfg, tc.config, field(tc.field))) - err := Validate(cfg) + err = Validate(cfg) assert.NilError(t, err) }) } diff --git a/daemon/config/config_windows.go b/daemon/config/config_windows.go index 05b3b667bc..7958abaac1 100644 --- a/daemon/config/config_windows.go +++ b/daemon/config/config_windows.go @@ -1,6 +1,9 @@ package config // import "github.com/docker/docker/daemon/config" import ( + "os" + "path/filepath" + "github.com/docker/docker/api/types" ) @@ -62,3 +65,10 @@ func (conf *Config) ValidatePlatformConfig() error { func (conf *Config) IsRootless() bool { return false } + +func setPlatformDefaults(cfg *Config) error { + cfg.Root = filepath.Join(os.Getenv("programdata"), "docker") + cfg.ExecRoot = filepath.Join(os.Getenv("programdata"), "docker", "exec-root") + cfg.Pidfile = filepath.Join(cfg.Root, "docker.pid") + return nil +} diff --git a/daemon/config/config_windows_test.go b/daemon/config/config_windows_test.go index 94b198b944..69c3c97372 100644 --- a/daemon/config/config_windows_test.go +++ b/daemon/config/config_windows_test.go @@ -23,7 +23,8 @@ func TestDaemonConfigurationMerge(t *testing.T) { f.Close() - conf := New() + conf, err := New() + assert.NilError(t, err) flags := pflag.NewFlagSet("test", pflag.ContinueOnError) flags.BoolVarP(&conf.Debug, "debug", "D", false, "") diff --git a/daemon/runtime_unix_test.go b/daemon/runtime_unix_test.go index 9d98b0da15..f4b55ac6e4 100644 --- a/daemon/runtime_unix_test.go +++ b/daemon/runtime_unix_test.go @@ -24,7 +24,10 @@ func TestGetRuntime(t *testing.T) { const configuredRtName = "my/custom.shim.v1" configuredRuntime := types.Runtime{Path: "/bin/true"} - d := &Daemon{configStore: config.New()} + cfg, err := config.New() + assert.NilError(t, err) + + d := &Daemon{configStore: cfg} d.configStore.Root = t.TempDir() assert.Assert(t, os.Mkdir(filepath.Join(d.configStore.Root, "runtimes"), 0700)) d.configStore.Runtimes = map[string]types.Runtime{