1
0
Fork 0
mirror of https://github.com/moby/moby.git synced 2022-11-09 12:21:53 -05:00
moby--moby/daemon/config/config_test.go
Lukas Heeren ce61a1ed98
Adding ability to change max download attempts
Moby works perfectly when you are in a situation when one has a good and stable
internet connection. Operating in area's where internet connectivity is likely
to be lost in undetermined intervals, like a satellite connection or 4G/LTE in
rural area's, can become a problem when pulling a new image. When connection is
lost while image layers are being pulled, Moby will try to reconnect up to 5 times.
If this fails, the incompletely downloaded layers are lost will need to be completely
downloaded again during the next pull request. This means that we are using more
data than we might have to.

Pulling a layer multiple times from the start can become costly over a satellite
or 4G/LTE connection. As these techniques (especially 4G) quite common in IoT and
Moby is used to run Azure IoT Edge devices, I would like to add a settable maximum
download attempts. The maximum download attempts is currently set at 5
(distribution/xfer/download.go). I would like to change this constant to a variable
that the user can set. The default will still be 5, so nothing will change from
the current version unless specified when starting the daemon with the added flag
or in the config file.

I added a default value of 5 for DefaultMaxDownloadAttempts and a settable
max-download-attempts in the daemon config file. It is also added to the config
of dockerd so it can be set with a flag when starting the daemon. This value gets
stored in the imageService of the daemon when it is initiated and can be passed
to the NewLayerDownloadManager as a parameter. It will be stored in the
LayerDownloadManager when initiated. This enables us to set the max amount of
retries in makeDownoadFunc equal to the max download attempts.

I also added some tests that are based on maxConcurrentDownloads/maxConcurrentUploads.

You can pull this version and test in a development container. Either create a config
`file /etc/docker/daemon.json` with `{"max-download-attempts"=3}``, or use
`dockerd --max-download-attempts=3 -D &` to start up the dockerd. Start downloading
a container and disconnect from the internet whilst downloading. The result would
be that it stops pulling after three attempts.

Signed-off-by: Lukas Heeren <lukas-heeren@hotmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-09-19 13:51:40 +02:00

572 lines
15 KiB
Go

package config // import "github.com/docker/docker/daemon/config"
import (
"io/ioutil"
"os"
"strings"
"testing"
"github.com/docker/docker/daemon/discovery"
"github.com/docker/docker/opts"
"github.com/spf13/pflag"
"gotest.tools/assert"
is "gotest.tools/assert/cmp"
"gotest.tools/fs"
"gotest.tools/skip"
)
func TestDaemonConfigurationNotFound(t *testing.T) {
_, err := MergeDaemonConfigurations(&Config{}, nil, "/tmp/foo-bar-baz-docker")
if err == nil || !os.IsNotExist(err) {
t.Fatalf("expected does not exist error, got %v", err)
}
}
func TestDaemonBrokenConfiguration(t *testing.T) {
f, err := ioutil.TempFile("", "docker-config-")
if err != nil {
t.Fatal(err)
}
configFile := f.Name()
f.Write([]byte(`{"Debug": tru`))
f.Close()
_, err = MergeDaemonConfigurations(&Config{}, nil, configFile)
if err == nil {
t.Fatalf("expected error, got %v", err)
}
}
func TestParseClusterAdvertiseSettings(t *testing.T) {
_, err := ParseClusterAdvertiseSettings("something", "")
if err != discovery.ErrDiscoveryDisabled {
t.Fatalf("expected discovery disabled error, got %v\n", err)
}
_, err = ParseClusterAdvertiseSettings("", "something")
if err == nil {
t.Fatalf("expected discovery store error, got %v\n", err)
}
_, err = ParseClusterAdvertiseSettings("etcd", "127.0.0.1:8080")
if err != nil {
t.Fatal(err)
}
}
func TestFindConfigurationConflicts(t *testing.T) {
config := map[string]interface{}{"authorization-plugins": "foobar"}
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
flags.String("authorization-plugins", "", "")
assert.Check(t, flags.Set("authorization-plugins", "asdf"))
assert.Check(t, is.ErrorContains(findConfigurationConflicts(config, flags), "authorization-plugins: (from flag: asdf, from file: foobar)"))
}
func TestFindConfigurationConflictsWithNamedOptions(t *testing.T) {
config := map[string]interface{}{"hosts": []string{"qwer"}}
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
var hosts []string
flags.VarP(opts.NewNamedListOptsRef("hosts", &hosts, opts.ValidateHost), "host", "H", "Daemon socket(s) to connect to")
assert.Check(t, flags.Set("host", "tcp://127.0.0.1:4444"))
assert.Check(t, flags.Set("host", "unix:///var/run/docker.sock"))
assert.Check(t, is.ErrorContains(findConfigurationConflicts(config, flags), "hosts"))
}
func TestDaemonConfigurationMergeConflicts(t *testing.T) {
f, err := ioutil.TempFile("", "docker-config-")
if err != nil {
t.Fatal(err)
}
configFile := f.Name()
f.Write([]byte(`{"debug": true}`))
f.Close()
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
flags.Bool("debug", false, "")
flags.Set("debug", "false")
_, err = MergeDaemonConfigurations(&Config{}, flags, configFile)
if err == nil {
t.Fatal("expected error, got nil")
}
if !strings.Contains(err.Error(), "debug") {
t.Fatalf("expected debug conflict, got %v", err)
}
}
func TestDaemonConfigurationMergeConcurrent(t *testing.T) {
f, err := ioutil.TempFile("", "docker-config-")
if err != nil {
t.Fatal(err)
}
configFile := f.Name()
f.Write([]byte(`{"max-concurrent-downloads": 1}`))
f.Close()
_, err = MergeDaemonConfigurations(&Config{}, nil, configFile)
if err != nil {
t.Fatal("expected error, got nil")
}
}
func TestDaemonConfigurationMergeConcurrentError(t *testing.T) {
f, err := ioutil.TempFile("", "docker-config-")
if err != nil {
t.Fatal(err)
}
configFile := f.Name()
f.Write([]byte(`{"max-concurrent-downloads": -1}`))
f.Close()
_, err = MergeDaemonConfigurations(&Config{}, nil, configFile)
if err == nil {
t.Fatalf("expected no error, got error %v", err)
}
}
func TestDaemonConfigurationMergeConflictsWithInnerStructs(t *testing.T) {
f, err := ioutil.TempFile("", "docker-config-")
if err != nil {
t.Fatal(err)
}
configFile := f.Name()
f.Write([]byte(`{"tlscacert": "/etc/certificates/ca.pem"}`))
f.Close()
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
flags.String("tlscacert", "", "")
flags.Set("tlscacert", "~/.docker/ca.pem")
_, err = MergeDaemonConfigurations(&Config{}, flags, configFile)
if err == nil {
t.Fatal("expected error, got nil")
}
if !strings.Contains(err.Error(), "tlscacert") {
t.Fatalf("expected tlscacert conflict, got %v", err)
}
}
func TestFindConfigurationConflictsWithUnknownKeys(t *testing.T) {
config := map[string]interface{}{"tls-verify": "true"}
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
flags.Bool("tlsverify", false, "")
err := findConfigurationConflicts(config, flags)
if err == nil {
t.Fatal("expected error, got nil")
}
if !strings.Contains(err.Error(), "the following directives don't match any configuration option: tls-verify") {
t.Fatalf("expected tls-verify conflict, got %v", err)
}
}
func TestFindConfigurationConflictsWithMergedValues(t *testing.T) {
var hosts []string
config := map[string]interface{}{"hosts": "tcp://127.0.0.1:2345"}
flags := pflag.NewFlagSet("base", pflag.ContinueOnError)
flags.VarP(opts.NewNamedListOptsRef("hosts", &hosts, nil), "host", "H", "")
err := findConfigurationConflicts(config, flags)
if err != nil {
t.Fatal(err)
}
flags.Set("host", "unix:///var/run/docker.sock")
err = findConfigurationConflicts(config, flags)
if err == nil {
t.Fatal("expected error, got nil")
}
if !strings.Contains(err.Error(), "hosts: (from flag: [unix:///var/run/docker.sock], from file: tcp://127.0.0.1:2345)") {
t.Fatalf("expected hosts conflict, got %v", err)
}
}
func TestValidateReservedNamespaceLabels(t *testing.T) {
for _, validLabels := range [][]string{
nil, // no error if there are no labels
{ // no error if there aren't any reserved namespace labels
"hello=world",
"label=me",
},
{ // only reserved namespaces that end with a dot are invalid
"com.dockerpsychnotreserved.label=value",
"io.dockerproject.not=reserved",
"org.docker.not=reserved",
},
} {
assert.Check(t, ValidateReservedNamespaceLabels(validLabels))
}
for _, invalidLabel := range []string{
"com.docker.feature=enabled",
"io.docker.configuration=0",
"org.dockerproject.setting=on",
// casing doesn't matter
"COM.docker.feature=enabled",
"io.DOCKER.CONFIGURATION=0",
"Org.Dockerproject.Setting=on",
} {
err := ValidateReservedNamespaceLabels([]string{
"valid=label",
invalidLabel,
"another=valid",
})
assert.Check(t, is.ErrorContains(err, invalidLabel))
}
}
func TestValidateConfigurationErrors(t *testing.T) {
intPtr := func(i int) *int { return &i }
testCases := []struct {
name string
config *Config
expectedErr string
}{
{
name: "single label without value",
config: &Config{
CommonConfig: CommonConfig{
Labels: []string{"one"},
},
},
expectedErr: "bad attribute format: one",
},
{
name: "multiple label without value",
config: &Config{
CommonConfig: CommonConfig{
Labels: []string{"foo=bar", "one"},
},
},
expectedErr: "bad attribute format: one",
},
{
name: "single DNS, invalid IP-address",
config: &Config{
CommonConfig: CommonConfig{
DNSConfig: DNSConfig{
DNS: []string{"1.1.1.1o"},
},
},
},
expectedErr: "1.1.1.1o is not an ip address",
},
{
name: "multiple DNS, invalid IP-address",
config: &Config{
CommonConfig: CommonConfig{
DNSConfig: DNSConfig{
DNS: []string{"2.2.2.2", "1.1.1.1o"},
},
},
},
expectedErr: "1.1.1.1o is not an ip address",
},
{
name: "single DNSSearch",
config: &Config{
CommonConfig: CommonConfig{
DNSConfig: DNSConfig{
DNSSearch: []string{"123456"},
},
},
},
expectedErr: "123456 is not a valid domain",
},
{
name: "multiple DNSSearch",
config: &Config{
CommonConfig: CommonConfig{
DNSConfig: DNSConfig{
DNSSearch: []string{"a.b.c", "123456"},
},
},
},
expectedErr: "123456 is not a valid domain",
},
{
name: "negative max-concurrent-downloads",
config: &Config{
CommonConfig: CommonConfig{
MaxConcurrentDownloads: intPtr(-10),
},
},
expectedErr: "invalid max concurrent downloads: -10",
},
{
name: "negative max-concurrent-uploads",
config: &Config{
CommonConfig: CommonConfig{
MaxConcurrentUploads: intPtr(-10),
},
},
expectedErr: "invalid max concurrent uploads: -10",
},
{
name: "negative max-download-attempts",
config: &Config{
CommonConfig: CommonConfig{
MaxDownloadAttempts: intPtr(-10),
},
},
expectedErr: "invalid max download attempts: -10",
},
{
name: "zero max-download-attempts",
config: &Config{
CommonConfig: CommonConfig{
MaxDownloadAttempts: intPtr(0),
},
},
expectedErr: "invalid max download attempts: 0",
},
{
name: "generic resource without =",
config: &Config{
CommonConfig: CommonConfig{
NodeGenericResources: []string{"foo"},
},
},
expectedErr: "could not parse GenericResource: incorrect term foo, missing '=' or malformed expression",
},
{
name: "generic resource mixed named and discrete",
config: &Config{
CommonConfig: CommonConfig{
NodeGenericResources: []string{"foo=bar", "foo=1"},
},
},
expectedErr: "could not parse GenericResource: mixed discrete and named resources in expression 'foo=[bar 1]'",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
err := Validate(tc.config)
assert.Error(t, err, tc.expectedErr)
})
}
}
func TestValidateConfiguration(t *testing.T) {
intPtr := func(i int) *int { return &i }
testCases := []struct {
name string
config *Config
}{
{
name: "with label",
config: &Config{
CommonConfig: CommonConfig{
Labels: []string{"one=two"},
},
},
},
{
name: "with dns",
config: &Config{
CommonConfig: CommonConfig{
DNSConfig: DNSConfig{
DNS: []string{"1.1.1.1"},
},
},
},
},
{
name: "with dns-search",
config: &Config{
CommonConfig: CommonConfig{
DNSConfig: DNSConfig{
DNSSearch: []string{"a.b.c"},
},
},
},
},
{
name: "with max-concurrent-downloads",
config: &Config{
CommonConfig: CommonConfig{
MaxConcurrentDownloads: intPtr(4),
},
},
},
{
name: "with max-concurrent-uploads",
config: &Config{
CommonConfig: CommonConfig{
MaxConcurrentUploads: intPtr(4),
},
},
},
{
name: "with max-download-attempts",
config: &Config{
CommonConfig: CommonConfig{
MaxDownloadAttempts: intPtr(4),
},
},
},
{
name: "with multiple node generic resources",
config: &Config{
CommonConfig: CommonConfig{
NodeGenericResources: []string{"foo=bar", "foo=baz"},
},
},
},
{
name: "with node generic resources",
config: &Config{
CommonConfig: CommonConfig{
NodeGenericResources: []string{"foo=1"},
},
},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
err := Validate(tc.config)
assert.NilError(t, err)
})
}
}
func TestModifiedDiscoverySettings(t *testing.T) {
cases := []struct {
current *Config
modified *Config
expected bool
}{
{
current: discoveryConfig("foo", "bar", map[string]string{}),
modified: discoveryConfig("foo", "bar", map[string]string{}),
expected: false,
},
{
current: discoveryConfig("foo", "bar", map[string]string{"foo": "bar"}),
modified: discoveryConfig("foo", "bar", map[string]string{"foo": "bar"}),
expected: false,
},
{
current: discoveryConfig("foo", "bar", map[string]string{}),
modified: discoveryConfig("foo", "bar", nil),
expected: false,
},
{
current: discoveryConfig("foo", "bar", nil),
modified: discoveryConfig("foo", "bar", map[string]string{}),
expected: false,
},
{
current: discoveryConfig("foo", "bar", nil),
modified: discoveryConfig("baz", "bar", nil),
expected: true,
},
{
current: discoveryConfig("foo", "bar", nil),
modified: discoveryConfig("foo", "baz", nil),
expected: true,
},
{
current: discoveryConfig("foo", "bar", nil),
modified: discoveryConfig("foo", "bar", map[string]string{"foo": "bar"}),
expected: true,
},
}
for _, c := range cases {
got := ModifiedDiscoverySettings(c.current, c.modified.ClusterStore, c.modified.ClusterAdvertise, c.modified.ClusterOpts)
if c.expected != got {
t.Fatalf("expected %v, got %v: current config %v, new config %v", c.expected, got, c.current, c.modified)
}
}
}
func discoveryConfig(backendAddr, advertiseAddr string, opts map[string]string) *Config {
return &Config{
CommonConfig: CommonConfig{
ClusterStore: backendAddr,
ClusterAdvertise: advertiseAddr,
ClusterOpts: opts,
},
}
}
// TestReloadSetConfigFileNotExist tests that when `--config-file` is set
// and it doesn't exist the `Reload` function returns an error.
func TestReloadSetConfigFileNotExist(t *testing.T) {
configFile := "/tmp/blabla/not/exists/config.json"
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
flags.String("config-file", "", "")
flags.Set("config-file", configFile)
err := Reload(configFile, flags, func(c *Config) {})
assert.Check(t, is.ErrorContains(err, "unable to configure the Docker daemon with file"))
}
// TestReloadDefaultConfigNotExist tests that if the default configuration file
// doesn't exist the daemon still will be reloaded.
func TestReloadDefaultConfigNotExist(t *testing.T) {
skip.If(t, os.Getuid() != 0, "skipping test that requires root")
reloaded := false
configFile := "/etc/docker/daemon.json"
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
flags.String("config-file", configFile, "")
err := Reload(configFile, flags, func(c *Config) {
reloaded = true
})
assert.Check(t, err)
assert.Check(t, reloaded)
}
// TestReloadBadDefaultConfig tests that when `--config-file` is not set
// and the default configuration file exists and is bad return an error
func TestReloadBadDefaultConfig(t *testing.T) {
f, err := ioutil.TempFile("", "docker-config-")
if err != nil {
t.Fatal(err)
}
configFile := f.Name()
f.Write([]byte(`{wrong: "configuration"}`))
f.Close()
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
flags.String("config-file", configFile, "")
err = Reload(configFile, flags, func(c *Config) {})
assert.Check(t, is.ErrorContains(err, "unable to configure the Docker daemon with file"))
}
func TestReloadWithConflictingLabels(t *testing.T) {
tempFile := fs.NewFile(t, "config", fs.WithContent(`{"labels":["foo=bar","foo=baz"]}`))
defer tempFile.Remove()
configFile := tempFile.Path()
var lbls []string
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
flags.String("config-file", configFile, "")
flags.StringSlice("labels", lbls, "")
err := Reload(configFile, flags, func(c *Config) {})
assert.Check(t, is.ErrorContains(err, "conflict labels for foo=baz and foo=bar"))
}
func TestReloadWithDuplicateLabels(t *testing.T) {
tempFile := fs.NewFile(t, "config", fs.WithContent(`{"labels":["foo=the-same","foo=the-same"]}`))
defer tempFile.Remove()
configFile := tempFile.Path()
var lbls []string
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
flags.String("config-file", configFile, "")
flags.StringSlice("labels", lbls, "")
err := Reload(configFile, flags, func(c *Config) {})
assert.Check(t, err)
}