From e62382d01430ee5e26236f55799e160baefef7d4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 24 Apr 2022 22:59:54 +0200 Subject: [PATCH] daemon/config: remove uses of pointers for ints Use the default (0) value to indicate "not set", which simplifies working with these configuration options, preventing the need to use intermediate variables etc. While changing this code, also making some small cleanups, such as replacing "fmt.Sprintf()" for "strconv" variants. Signed-off-by: Sebastiaan van Stijn --- cmd/dockerd/config.go | 11 +++----- daemon/config/config.go | 18 ++++++------- daemon/config/config_test.go | 18 +++++-------- daemon/daemon.go | 6 ++--- daemon/images/service.go | 10 ++++---- daemon/reload.go | 50 +++++++++++++++++------------------- 6 files changed, 50 insertions(+), 63 deletions(-) diff --git a/cmd/dockerd/config.go b/cmd/dockerd/config.go index f39ce8ed54..7b74250fad 100644 --- a/cmd/dockerd/config.go +++ b/cmd/dockerd/config.go @@ -12,7 +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 maxConcurrentDownloads, maxConcurrentUploads, maxDownloadAttempts int var err error conf.Pidfile, err = getDefaultPidFile() if err != nil { @@ -60,9 +59,9 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error { flags.Var(opts.NewNamedMapOpts("log-opts", conf.LogConfig.Config, nil), "log-opt", "Default log driver options for containers") flags.StringVar(&conf.CorsHeaders, "api-cors-header", "", "Set CORS headers in the Engine API") - flags.IntVar(&maxConcurrentDownloads, "max-concurrent-downloads", config.DefaultMaxConcurrentDownloads, "Set the max concurrent downloads for each pull") - flags.IntVar(&maxConcurrentUploads, "max-concurrent-uploads", config.DefaultMaxConcurrentUploads, "Set the max concurrent uploads for each push") - flags.IntVar(&maxDownloadAttempts, "max-download-attempts", config.DefaultDownloadAttempts, "Set the max download attempts for each pull") + flags.IntVar(&conf.MaxConcurrentDownloads, "max-concurrent-downloads", config.DefaultMaxConcurrentDownloads, "Set the max concurrent downloads for each pull") + flags.IntVar(&conf.MaxConcurrentUploads, "max-concurrent-uploads", config.DefaultMaxConcurrentUploads, "Set the max concurrent uploads for each push") + flags.IntVar(&conf.MaxDownloadAttempts, "max-download-attempts", config.DefaultDownloadAttempts, "Set the max download attempts for each pull") flags.IntVar(&conf.ShutdownTimeout, "shutdown-timeout", config.DefaultShutdownTimeout, "Set the default shutdown timeout") flags.StringVar(&conf.SwarmDefaultAdvertiseAddr, "swarm-default-advertise-addr", "", "Set default address or interface for swarm advertised address") @@ -70,10 +69,6 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error { flags.StringVar(&conf.MetricsAddress, "metrics-addr", "", "Set default address and port to serve the metrics api on") flags.Var(opts.NewNamedListOptsRef("node-generic-resources", &conf.NodeGenericResources, opts.ValidateSingleGenericResource), "node-generic-resource", "Advertise user-defined resource") - conf.MaxConcurrentDownloads = &maxConcurrentDownloads - conf.MaxConcurrentUploads = &maxConcurrentUploads - conf.MaxDownloadAttempts = &maxDownloadAttempts - flags.StringVar(&conf.ContainerdNamespace, "containerd-namespace", config.DefaultContainersNamespace, "Containerd namespace to use") flags.StringVar(&conf.ContainerdPluginNamespace, "containerd-plugins-namespace", config.DefaultPluginNamespace, "Containerd namespace to use for plugins") flags.StringVar(&conf.DefaultRuntime, "default-runtime", config.StockRuntimeName, "Default OCI runtime for containers") diff --git a/daemon/config/config.go b/daemon/config/config.go index 339acf2fc3..9c17efa389 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -200,15 +200,15 @@ type CommonConfig struct { // MaxConcurrentDownloads is the maximum number of downloads that // may take place at a time for each pull. - MaxConcurrentDownloads *int `json:"max-concurrent-downloads,omitempty"` + MaxConcurrentDownloads int `json:"max-concurrent-downloads,omitempty"` // MaxConcurrentUploads is the maximum number of uploads that // may take place at a time for each push. - MaxConcurrentUploads *int `json:"max-concurrent-uploads,omitempty"` + MaxConcurrentUploads int `json:"max-concurrent-uploads,omitempty"` // MaxDownloadAttempts is the maximum number of attempts that // may take place at a time for each push. - MaxDownloadAttempts *int `json:"max-download-attempts,omitempty"` + MaxDownloadAttempts int `json:"max-download-attempts,omitempty"` // ShutdownTimeout is the timeout value (in seconds) the daemon will wait for the container // to stop when daemon is being shutdown @@ -602,14 +602,14 @@ func Validate(config *Config) error { } // TODO(thaJeztah) Validations below should not accept "0" to be valid; see Validate() for a more in-depth description of this problem - if config.MaxConcurrentDownloads != nil && *config.MaxConcurrentDownloads < 0 { - return fmt.Errorf("invalid max concurrent downloads: %d", *config.MaxConcurrentDownloads) + if config.MaxConcurrentDownloads < 0 { + return fmt.Errorf("invalid max concurrent downloads: %d", config.MaxConcurrentDownloads) } - if config.MaxConcurrentUploads != nil && *config.MaxConcurrentUploads < 0 { - return fmt.Errorf("invalid max concurrent uploads: %d", *config.MaxConcurrentUploads) + if config.MaxConcurrentUploads < 0 { + return fmt.Errorf("invalid max concurrent uploads: %d", config.MaxConcurrentUploads) } - if config.MaxDownloadAttempts != nil && *config.MaxDownloadAttempts < 0 { - return fmt.Errorf("invalid max download attempts: %d", *config.MaxDownloadAttempts) + if config.MaxDownloadAttempts < 0 { + return fmt.Errorf("invalid max download attempts: %d", config.MaxDownloadAttempts) } // validate that "default" runtime is not reset diff --git a/daemon/config/config_test.go b/daemon/config/config_test.go index a982ea2920..6da6ec2aba 100644 --- a/daemon/config/config_test.go +++ b/daemon/config/config_test.go @@ -213,8 +213,6 @@ func TestFindConfigurationConflictsWithMergedValues(t *testing.T) { } func TestValidateConfigurationErrors(t *testing.T) { - intPtr := func(i int) *int { return &i } - testCases := []struct { name string config *Config @@ -286,7 +284,7 @@ func TestValidateConfigurationErrors(t *testing.T) { name: "negative max-concurrent-downloads", config: &Config{ CommonConfig: CommonConfig{ - MaxConcurrentDownloads: intPtr(-10), + MaxConcurrentDownloads: -10, }, }, expectedErr: "invalid max concurrent downloads: -10", @@ -295,7 +293,7 @@ func TestValidateConfigurationErrors(t *testing.T) { name: "negative max-concurrent-uploads", config: &Config{ CommonConfig: CommonConfig{ - MaxConcurrentUploads: intPtr(-10), + MaxConcurrentUploads: -10, }, }, expectedErr: "invalid max concurrent uploads: -10", @@ -304,7 +302,7 @@ func TestValidateConfigurationErrors(t *testing.T) { name: "negative max-download-attempts", config: &Config{ CommonConfig: CommonConfig{ - MaxDownloadAttempts: intPtr(-10), + MaxDownloadAttempts: -10, }, }, expectedErr: "invalid max download attempts: -10", @@ -315,7 +313,7 @@ func TestValidateConfigurationErrors(t *testing.T) { name: "zero max-download-attempts", config: &Config{ CommonConfig: CommonConfig{ - MaxDownloadAttempts: intPtr(0), + MaxDownloadAttempts: 0, }, }, expectedErr: "invalid max download attempts: 0", @@ -367,8 +365,6 @@ func TestValidateConfigurationErrors(t *testing.T) { } func TestValidateConfiguration(t *testing.T) { - intPtr := func(i int) *int { return &i } - testCases := []struct { name string config *Config @@ -405,7 +401,7 @@ func TestValidateConfiguration(t *testing.T) { name: "with max-concurrent-downloads", config: &Config{ CommonConfig: CommonConfig{ - MaxConcurrentDownloads: intPtr(4), + MaxConcurrentDownloads: 4, }, }, }, @@ -413,7 +409,7 @@ func TestValidateConfiguration(t *testing.T) { name: "with max-concurrent-uploads", config: &Config{ CommonConfig: CommonConfig{ - MaxConcurrentUploads: intPtr(4), + MaxConcurrentUploads: 4, }, }, }, @@ -421,7 +417,7 @@ func TestValidateConfiguration(t *testing.T) { name: "with max-download-attempts", config: &Config{ CommonConfig: CommonConfig{ - MaxDownloadAttempts: intPtr(4), + MaxDownloadAttempts: 4, }, }, }, diff --git a/daemon/daemon.go b/daemon/daemon.go index 9e87a56e40..1bf2cf67bf 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1038,9 +1038,9 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S EventsService: d.EventsService, ImageStore: imageStore, LayerStore: layerStore, - MaxConcurrentDownloads: *config.MaxConcurrentDownloads, - MaxConcurrentUploads: *config.MaxConcurrentUploads, - MaxDownloadAttempts: *config.MaxDownloadAttempts, + MaxConcurrentDownloads: config.MaxConcurrentDownloads, + MaxConcurrentUploads: config.MaxConcurrentUploads, + MaxDownloadAttempts: config.MaxDownloadAttempts, ReferenceStore: rs, RegistryService: registryService, TrustKey: trustKey, diff --git a/daemon/images/service.go b/daemon/images/service.go index 6216d413b3..b1fdcc11ba 100644 --- a/daemon/images/service.go +++ b/daemon/images/service.go @@ -271,11 +271,11 @@ func (i *ImageService) ImageDiskUsage(ctx context.Context) ([]*types.ImageSummar // UpdateConfig values // // called from reload.go -func (i *ImageService) UpdateConfig(maxDownloads, maxUploads *int) { - if i.downloadManager != nil && maxDownloads != nil { - i.downloadManager.SetConcurrency(*maxDownloads) +func (i *ImageService) UpdateConfig(maxDownloads, maxUploads int) { + if i.downloadManager != nil && maxDownloads != 0 { + i.downloadManager.SetConcurrency(maxDownloads) } - if i.uploadManager != nil && maxUploads != nil { - i.uploadManager.SetConcurrency(*maxUploads) + if i.uploadManager != nil && maxUploads != 0 { + i.uploadManager.SetConcurrency(maxUploads) } } diff --git a/daemon/reload.go b/daemon/reload.go index 368ec53ff2..c8e256557d 100644 --- a/daemon/reload.go +++ b/daemon/reload.go @@ -3,6 +3,7 @@ package daemon // import "github.com/docker/docker/daemon" import ( "encoding/json" "fmt" + "strconv" "github.com/docker/docker/daemon/config" "github.com/sirupsen/logrus" @@ -86,52 +87,47 @@ func (daemon *Daemon) reloadDebug(conf *config.Config, attributes map[string]str daemon.configStore.Debug = conf.Debug } // prepare reload event attributes with updatable configurations - attributes["debug"] = fmt.Sprintf("%t", daemon.configStore.Debug) + attributes["debug"] = strconv.FormatBool(daemon.configStore.Debug) } // reloadMaxConcurrentDownloadsAndUploads updates configuration with max concurrent // download and upload options and updates the passed attributes func (daemon *Daemon) reloadMaxConcurrentDownloadsAndUploads(conf *config.Config, attributes map[string]string) { - // If no value is set for max-concurrent-downloads we assume it is the default value // We always "reset" as the cost is lightweight and easy to maintain. - maxConcurrentDownloads := config.DefaultMaxConcurrentDownloads - if conf.IsValueSet("max-concurrent-downloads") && conf.MaxConcurrentDownloads != nil { - maxConcurrentDownloads = *conf.MaxConcurrentDownloads - } - daemon.configStore.MaxConcurrentDownloads = &maxConcurrentDownloads - logrus.Debugf("Reset Max Concurrent Downloads: %d", *daemon.configStore.MaxConcurrentDownloads) + daemon.configStore.MaxConcurrentDownloads = config.DefaultMaxConcurrentDownloads + daemon.configStore.MaxConcurrentUploads = config.DefaultMaxConcurrentUploads - // If no value is set for max-concurrent-upload we assume it is the default value - // We always "reset" as the cost is lightweight and easy to maintain. - maxConcurrentUploads := config.DefaultMaxConcurrentUploads - if conf.IsValueSet("max-concurrent-uploads") && conf.MaxConcurrentUploads != nil { - maxConcurrentUploads = *conf.MaxConcurrentUploads + if conf.IsValueSet("max-concurrent-downloads") && conf.MaxConcurrentDownloads != 0 { + daemon.configStore.MaxConcurrentDownloads = conf.MaxConcurrentDownloads + } + if conf.IsValueSet("max-concurrent-uploads") && conf.MaxConcurrentUploads != 0 { + daemon.configStore.MaxConcurrentUploads = conf.MaxConcurrentUploads } - daemon.configStore.MaxConcurrentUploads = &maxConcurrentUploads - logrus.Debugf("Reset Max Concurrent Uploads: %d", *daemon.configStore.MaxConcurrentUploads) - if daemon.imageService != nil { - daemon.imageService.UpdateConfig(&maxConcurrentDownloads, &maxConcurrentUploads) + daemon.imageService.UpdateConfig( + daemon.configStore.MaxConcurrentDownloads, + daemon.configStore.MaxConcurrentUploads, + ) } // prepare reload event attributes with updatable configurations - attributes["max-concurrent-downloads"] = fmt.Sprintf("%d", *daemon.configStore.MaxConcurrentDownloads) - // prepare reload event attributes with updatable configurations - attributes["max-concurrent-uploads"] = fmt.Sprintf("%d", *daemon.configStore.MaxConcurrentUploads) + attributes["max-concurrent-downloads"] = strconv.Itoa(daemon.configStore.MaxConcurrentDownloads) + attributes["max-concurrent-uploads"] = strconv.Itoa(daemon.configStore.MaxConcurrentUploads) + logrus.Debug("Reset Max Concurrent Downloads: ", attributes["max-concurrent-downloads"]) + logrus.Debug("Reset Max Concurrent Uploads: ", attributes["max-concurrent-uploads"]) } // reloadMaxDownloadAttempts updates configuration with max concurrent // download attempts when a connection is lost and updates the passed attributes func (daemon *Daemon) reloadMaxDownloadAttempts(conf *config.Config, attributes map[string]string) { // We always "reset" as the cost is lightweight and easy to maintain. - maxDownloadAttempts := config.DefaultDownloadAttempts - if conf.IsValueSet("max-download-attempts") && conf.MaxDownloadAttempts != nil { - maxDownloadAttempts = *conf.MaxDownloadAttempts + daemon.configStore.MaxDownloadAttempts = config.DefaultDownloadAttempts + if conf.IsValueSet("max-download-attempts") && conf.MaxDownloadAttempts != 0 { + daemon.configStore.MaxDownloadAttempts = conf.MaxDownloadAttempts } - daemon.configStore.MaxDownloadAttempts = &maxDownloadAttempts // prepare reload event attributes with updatable configurations - attributes["max-download-attempts"] = fmt.Sprintf("%d", *daemon.configStore.MaxDownloadAttempts) + attributes["max-download-attempts"] = strconv.Itoa(daemon.configStore.MaxDownloadAttempts) logrus.Debug("Reset Max Download Attempts: ", attributes["max-download-attempts"]) } @@ -145,7 +141,7 @@ func (daemon *Daemon) reloadShutdownTimeout(conf *config.Config, attributes map[ } // prepare reload event attributes with updatable configurations - attributes["shutdown-timeout"] = fmt.Sprintf("%d", daemon.configStore.ShutdownTimeout) + attributes["shutdown-timeout"] = strconv.Itoa(daemon.configStore.ShutdownTimeout) } // reloadLabels updates configuration with engine labels @@ -254,7 +250,7 @@ func (daemon *Daemon) reloadLiveRestore(conf *config.Config, attributes map[stri } // prepare reload event attributes with updatable configurations - attributes["live-restore"] = fmt.Sprintf("%t", daemon.configStore.LiveRestoreEnabled) + attributes["live-restore"] = strconv.FormatBool(daemon.configStore.LiveRestoreEnabled) return nil }