Ver Fonte

daemon/config: set default MTU when initializing config

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn há 3 anos atrás
pai
commit
a0d0db126c
4 ficheiros alterados com 75 adições e 25 exclusões
  1. 1 1
      cmd/dockerd/config.go
  2. 1 0
      daemon/config/config.go
  3. 73 14
      daemon/config/config_test.go
  4. 0 10
      daemon/daemon.go

+ 1 - 1
cmd/dockerd/config.go

@@ -44,7 +44,7 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
 	flags.StringVar(&conf.ContainerdAddr, "containerd", "", "containerd grpc address")
 	flags.StringVar(&conf.ContainerdAddr, "containerd", "", "containerd grpc address")
 	flags.BoolVar(&conf.CriContainerd, "cri-containerd", false, "start containerd with cri")
 	flags.BoolVar(&conf.CriContainerd, "cri-containerd", false, "start containerd with cri")
 
 
-	flags.IntVar(&conf.Mtu, "mtu", 0, "Set the containers network MTU")
+	flags.IntVar(&conf.Mtu, "mtu", conf.Mtu, "Set the containers network MTU")
 	flags.IntVar(&conf.NetworkControlPlaneMTU, "network-control-plane-mtu", config.DefaultNetworkMtu, "Network Control plane MTU")
 	flags.IntVar(&conf.NetworkControlPlaneMTU, "network-control-plane-mtu", config.DefaultNetworkMtu, "Network Control plane MTU")
 	flags.IntVar(&conf.NetworkDiagnosticPort, "network-diagnostic-port", 0, "TCP port number of the network diagnostic server")
 	flags.IntVar(&conf.NetworkDiagnosticPort, "network-diagnostic-port", 0, "TCP port number of the network diagnostic server")
 	_ = flags.MarkHidden("network-diagnostic-port")
 	_ = flags.MarkHidden("network-diagnostic-port")

+ 1 - 0
daemon/config/config.go

@@ -299,6 +299,7 @@ func New() *Config {
 			LogConfig: LogConfig{
 			LogConfig: LogConfig{
 				Config: make(map[string]string),
 				Config: make(map[string]string),
 			},
 			},
+			Mtu: DefaultNetworkMtu,
 		},
 		},
 	}
 	}
 }
 }

+ 73 - 14
daemon/config/config_test.go

@@ -2,11 +2,15 @@ package config // import "github.com/docker/docker/daemon/config"
 
 
 import (
 import (
 	"os"
 	"os"
+	"reflect"
 	"strings"
 	"strings"
 	"testing"
 	"testing"
 
 
 	"github.com/docker/docker/libnetwork/ipamutils"
 	"github.com/docker/docker/libnetwork/ipamutils"
 	"github.com/docker/docker/opts"
 	"github.com/docker/docker/opts"
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/imdario/mergo"
 	"github.com/spf13/pflag"
 	"github.com/spf13/pflag"
 	"gotest.tools/v3/assert"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	is "gotest.tools/v3/assert/cmp"
@@ -215,6 +219,7 @@ func TestFindConfigurationConflictsWithMergedValues(t *testing.T) {
 func TestValidateConfigurationErrors(t *testing.T) {
 func TestValidateConfigurationErrors(t *testing.T) {
 	testCases := []struct {
 	testCases := []struct {
 		name        string
 		name        string
+		field       string
 		config      *Config
 		config      *Config
 		expectedErr string
 		expectedErr string
 	}{
 	}{
@@ -319,7 +324,8 @@ func TestValidateConfigurationErrors(t *testing.T) {
 		// TODO(thaJeztah) temporarily excluding this test as it assumes defaults are set before validating and applying updated configs
 		// TODO(thaJeztah) temporarily excluding this test as it assumes defaults are set before validating and applying updated configs
 		/*
 		/*
 			{
 			{
-				name: "zero max-download-attempts",
+				name:  "zero max-download-attempts",
+				field: "MaxDownloadAttempts",
 				config: &Config{
 				config: &Config{
 					CommonConfig: CommonConfig{
 					CommonConfig: CommonConfig{
 						MaxDownloadAttempts: 0,
 						MaxDownloadAttempts: 0,
@@ -367,19 +373,45 @@ func TestValidateConfigurationErrors(t *testing.T) {
 	}
 	}
 	for _, tc := range testCases {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
-			err := Validate(tc.config)
+			cfg := New()
+			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)
 			assert.Error(t, err, tc.expectedErr)
 			assert.Error(t, err, tc.expectedErr)
 		})
 		})
 	}
 	}
 }
 }
 
 
+func withForceOverwrite(fieldName string) func(config *mergo.Config) {
+	return mergo.WithTransformers(overwriteTransformer{fieldName: fieldName})
+}
+
+type overwriteTransformer struct {
+	fieldName string
+}
+
+func (tf overwriteTransformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error {
+	if typ == reflect.TypeOf(CommonConfig{}) {
+		return func(dst, src reflect.Value) error {
+			dst.FieldByName(tf.fieldName).Set(src.FieldByName(tf.fieldName))
+			return nil
+		}
+	}
+	return nil
+}
+
 func TestValidateConfiguration(t *testing.T) {
 func TestValidateConfiguration(t *testing.T) {
 	testCases := []struct {
 	testCases := []struct {
 		name   string
 		name   string
+		field  string
 		config *Config
 		config *Config
 	}{
 	}{
 		{
 		{
-			name: "with label",
+			name:  "with label",
+			field: "Labels",
 			config: &Config{
 			config: &Config{
 				CommonConfig: CommonConfig{
 				CommonConfig: CommonConfig{
 					Labels: []string{"one=two"},
 					Labels: []string{"one=two"},
@@ -387,7 +419,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 			},
 		},
 		},
 		{
 		{
-			name: "with dns",
+			name:  "with dns",
+			field: "DNSConfig",
 			config: &Config{
 			config: &Config{
 				CommonConfig: CommonConfig{
 				CommonConfig: CommonConfig{
 					DNSConfig: DNSConfig{
 					DNSConfig: DNSConfig{
@@ -397,7 +430,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 			},
 		},
 		},
 		{
 		{
-			name: "with dns-search",
+			name:  "with dns-search",
+			field: "DNSConfig",
 			config: &Config{
 			config: &Config{
 				CommonConfig: CommonConfig{
 				CommonConfig: CommonConfig{
 					DNSConfig: DNSConfig{
 					DNSConfig: DNSConfig{
@@ -407,7 +441,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 			},
 		},
 		},
 		{
 		{
-			name: "with mtu",
+			name:  "with mtu",
+			field: "Mtu",
 			config: &Config{
 			config: &Config{
 				CommonConfig: CommonConfig{
 				CommonConfig: CommonConfig{
 					Mtu: 1234,
 					Mtu: 1234,
@@ -415,7 +450,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 			},
 		},
 		},
 		{
 		{
-			name: "with max-concurrent-downloads",
+			name:  "with max-concurrent-downloads",
+			field: "MaxConcurrentDownloads",
 			config: &Config{
 			config: &Config{
 				CommonConfig: CommonConfig{
 				CommonConfig: CommonConfig{
 					MaxConcurrentDownloads: 4,
 					MaxConcurrentDownloads: 4,
@@ -423,7 +459,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 			},
 		},
 		},
 		{
 		{
-			name: "with max-concurrent-uploads",
+			name:  "with max-concurrent-uploads",
+			field: "MaxConcurrentUploads",
 			config: &Config{
 			config: &Config{
 				CommonConfig: CommonConfig{
 				CommonConfig: CommonConfig{
 					MaxConcurrentUploads: 4,
 					MaxConcurrentUploads: 4,
@@ -431,7 +468,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 			},
 		},
 		},
 		{
 		{
-			name: "with max-download-attempts",
+			name:  "with max-download-attempts",
+			field: "MaxDownloadAttempts",
 			config: &Config{
 			config: &Config{
 				CommonConfig: CommonConfig{
 				CommonConfig: CommonConfig{
 					MaxDownloadAttempts: 4,
 					MaxDownloadAttempts: 4,
@@ -439,7 +477,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 			},
 		},
 		},
 		{
 		{
-			name: "with multiple node generic resources",
+			name:  "with multiple node generic resources",
+			field: "NodeGenericResources",
 			config: &Config{
 			config: &Config{
 				CommonConfig: CommonConfig{
 				CommonConfig: CommonConfig{
 					NodeGenericResources: []string{"foo=bar", "foo=baz"},
 					NodeGenericResources: []string{"foo=bar", "foo=baz"},
@@ -447,7 +486,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 			},
 		},
 		},
 		{
 		{
-			name: "with node generic resources",
+			name:  "with node generic resources",
+			field: "NodeGenericResources",
 			config: &Config{
 			config: &Config{
 				CommonConfig: CommonConfig{
 				CommonConfig: CommonConfig{
 					NodeGenericResources: []string{"foo=1"},
 					NodeGenericResources: []string{"foo=1"},
@@ -455,7 +495,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 			},
 		},
 		},
 		{
 		{
-			name: "with hosts",
+			name:  "with hosts",
+			field: "Hosts",
 			config: &Config{
 			config: &Config{
 				CommonConfig: CommonConfig{
 				CommonConfig: CommonConfig{
 					Hosts: []string{"tcp://127.0.0.1:2375"},
 					Hosts: []string{"tcp://127.0.0.1:2375"},
@@ -463,7 +504,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 			},
 		},
 		},
 		{
 		{
-			name: "with log-level warn",
+			name:  "with log-level warn",
+			field: "LogLevel",
 			config: &Config{
 			config: &Config{
 				CommonConfig: CommonConfig{
 				CommonConfig: CommonConfig{
 					LogLevel: "warn",
 					LogLevel: "warn",
@@ -473,12 +515,29 @@ func TestValidateConfiguration(t *testing.T) {
 	}
 	}
 	for _, tc := range testCases {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
-			err := Validate(tc.config)
+			// Start with a config with all defaults set, so that we only
+			cfg := New()
+			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)
 			assert.NilError(t, err)
 			assert.NilError(t, err)
 		})
 		})
 	}
 	}
 }
 }
 
 
+func field(field string) cmp.Option {
+	tmp := reflect.TypeOf(Config{})
+	ignoreFields := make([]string, 0, tmp.NumField())
+	for i := 0; i < tmp.NumField(); i++ {
+		if tmp.Field(i).Name != field {
+			ignoreFields = append(ignoreFields, tmp.Field(i).Name)
+		}
+	}
+	return cmpopts.IgnoreFields(Config{}, ignoreFields...)
+}
+
 // TestReloadSetConfigFileNotExist tests that when `--config-file` is set
 // TestReloadSetConfigFileNotExist tests that when `--config-file` is set
 // and it doesn't exist the `Reload` function returns an error.
 // and it doesn't exist the `Reload` function returns an error.
 func TestReloadSetConfigFileNotExist(t *testing.T) {
 func TestReloadSetConfigFileNotExist(t *testing.T) {

+ 0 - 10
daemon/daemon.go

@@ -703,8 +703,6 @@ func (daemon *Daemon) IsSwarmCompatible() error {
 // NewDaemon sets up everything for the daemon to be able to service
 // NewDaemon sets up everything for the daemon to be able to service
 // requests from the webserver.
 // requests from the webserver.
 func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.Store) (daemon *Daemon, err error) {
 func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.Store) (daemon *Daemon, err error) {
-	setDefaultMtu(config)
-
 	registryService, err := registry.NewService(config.ServiceOptions)
 	registryService, err := registry.NewService(config.ServiceOptions)
 	if err != nil {
 	if err != nil {
 		return nil, err
 		return nil, err
@@ -1349,14 +1347,6 @@ func (daemon *Daemon) setGenericResources(conf *config.Config) error {
 	return nil
 	return nil
 }
 }
 
 
-func setDefaultMtu(conf *config.Config) {
-	// do nothing if the config does not have the default 0 value.
-	if conf.Mtu != 0 {
-		return
-	}
-	conf.Mtu = config.DefaultNetworkMtu
-}
-
 // IsShuttingDown tells whether the daemon is shutting down or not
 // IsShuttingDown tells whether the daemon is shutting down or not
 func (daemon *Daemon) IsShuttingDown() bool {
 func (daemon *Daemon) IsShuttingDown() bool {
 	return daemon.shutdown
 	return daemon.shutdown