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 <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2022-04-24 22:59:54 +02:00
parent 4d22584432
commit e62382d014
No known key found for this signature in database
GPG Key ID: 76698F39D527CE8C
6 changed files with 50 additions and 63 deletions

View File

@ -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")

View File

@ -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

View File

@ -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,
},
},
},

View File

@ -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,

View File

@ -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)
}
}

View File

@ -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
}