From f1a74a89f89affcfbe311e89aa752b3d551e0340 Mon Sep 17 00:00:00 2001 From: David Calavera Date: Fri, 6 Nov 2015 17:22:48 -0500 Subject: [PATCH] Use an empty slice as default value for DNS, DNSSearch and DNSOptions So we don't print those in the client and we don't fail executing inspect templates with API field names. Make sure those fields are initialized as empty slices when a container is loaded from disk and their values are nil. Signed-off-by: David Calavera --- daemon/container.go | 23 ++++++- daemon/container_unit_test.go | 70 ++++++++++++++++++++++ daemon/daemon_test.go | 10 ++-- integration-cli/docker_cli_inspect_test.go | 8 +++ opts/opts.go | 10 ++++ runconfig/parse.go | 47 ++++++++------- 6 files changed, 141 insertions(+), 27 deletions(-) diff --git a/daemon/container.go b/daemon/container.go index fc64d0ee1a..4f7864b5d1 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -152,7 +152,28 @@ func (container *Container) readHostConfig() error { } defer f.Close() - return json.NewDecoder(f).Decode(&container.hostConfig) + if err := json.NewDecoder(f).Decode(&container.hostConfig); err != nil { + return err + } + + // Make sure the dns fields are never nil. + // New containers don't ever have those fields nil, + // but pre created containers can still have those nil values. + // See https://github.com/docker/docker/pull/17779 + // for a more detailed explanation on why we don't want that. + if container.hostConfig.DNS == nil { + container.hostConfig.DNS = make([]string, 0) + } + + if container.hostConfig.DNSSearch == nil { + container.hostConfig.DNSSearch = make([]string, 0) + } + + if container.hostConfig.DNSOptions == nil { + container.hostConfig.DNSOptions = make([]string, 0) + } + + return nil } func (container *Container) writeHostConfig() error { diff --git a/daemon/container_unit_test.go b/daemon/container_unit_test.go index 71d37cf436..f7951804a3 100644 --- a/daemon/container_unit_test.go +++ b/daemon/container_unit_test.go @@ -1,10 +1,15 @@ package daemon import ( + "io/ioutil" + "os" + "path/filepath" "testing" "github.com/docker/docker/pkg/signal" "github.com/docker/docker/runconfig" + "github.com/docker/docker/volume" + "github.com/docker/docker/volume/drivers" ) func TestGetFullName(t *testing.T) { @@ -64,3 +69,68 @@ func TestContainerStopSignal(t *testing.T) { t.Fatalf("Expected 9, got %v", s) } } + +func TestContainerInitDNS(t *testing.T) { + tmp, err := ioutil.TempDir("", "docker-container-test-") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmp) + + containerID := "d59df5276e7b219d510fe70565e0404bc06350e0d4b43fe961f22f339980170e" + containerPath := filepath.Join(tmp, containerID) + if err := os.MkdirAll(containerPath, 0755); err != nil { + t.Fatal(err) + } + + config := `{"State":{"Running":true,"Paused":false,"Restarting":false,"OOMKilled":false,"Dead":false,"Pid":2464,"ExitCode":0, +"Error":"","StartedAt":"2015-05-26T16:48:53.869308965Z","FinishedAt":"0001-01-01T00:00:00Z"}, +"ID":"d59df5276e7b219d510fe70565e0404bc06350e0d4b43fe961f22f339980170e","Created":"2015-05-26T16:48:53.7987917Z","Path":"top", +"Args":[],"Config":{"Hostname":"d59df5276e7b","Domainname":"","User":"","Memory":0,"MemorySwap":0,"CpuShares":0,"Cpuset":"", +"AttachStdin":false,"AttachStdout":false,"AttachStderr":false,"PortSpecs":null,"ExposedPorts":null,"Tty":true,"OpenStdin":true, +"StdinOnce":false,"Env":null,"Cmd":["top"],"Image":"ubuntu:latest","Volumes":null,"WorkingDir":"","Entrypoint":null, +"NetworkDisabled":false,"MacAddress":"","OnBuild":null,"Labels":{}},"Image":"07f8e8c5e66084bef8f848877857537ffe1c47edd01a93af27e7161672ad0e95", +"NetworkSettings":{"IPAddress":"172.17.0.1","IPPrefixLen":16,"MacAddress":"02:42:ac:11:00:01","LinkLocalIPv6Address":"fe80::42:acff:fe11:1", +"LinkLocalIPv6PrefixLen":64,"GlobalIPv6Address":"","GlobalIPv6PrefixLen":0,"Gateway":"172.17.42.1","IPv6Gateway":"","Bridge":"docker0","Ports":{}}, +"ResolvConfPath":"/var/lib/docker/containers/d59df5276e7b219d510fe70565e0404bc06350e0d4b43fe961f22f339980170e/resolv.conf", +"HostnamePath":"/var/lib/docker/containers/d59df5276e7b219d510fe70565e0404bc06350e0d4b43fe961f22f339980170e/hostname", +"HostsPath":"/var/lib/docker/containers/d59df5276e7b219d510fe70565e0404bc06350e0d4b43fe961f22f339980170e/hosts", +"LogPath":"/var/lib/docker/containers/d59df5276e7b219d510fe70565e0404bc06350e0d4b43fe961f22f339980170e/d59df5276e7b219d510fe70565e0404bc06350e0d4b43fe961f22f339980170e-json.log", +"Name":"/ubuntu","Driver":"aufs","MountLabel":"","ProcessLabel":"","AppArmorProfile":"","RestartCount":0, +"UpdateDns":false,"Volumes":{},"VolumesRW":{},"AppliedVolumesFrom":null}` + + if err = ioutil.WriteFile(filepath.Join(containerPath, "config.json"), []byte(config), 0644); err != nil { + t.Fatal(err) + } + + hostConfig := `{"Binds":[],"ContainerIDFile":"","Memory":0,"MemorySwap":0,"CpuShares":0,"CpusetCpus":"", +"Privileged":false,"PortBindings":{},"Links":null,"PublishAllPorts":false,"Dns":null,"DnsOptions":null,"DnsSearch":null,"ExtraHosts":null,"VolumesFrom":null, +"Devices":[],"NetworkMode":"bridge","IpcMode":"","PidMode":"","CapAdd":null,"CapDrop":null,"RestartPolicy":{"Name":"no","MaximumRetryCount":0}, +"SecurityOpt":null,"ReadonlyRootfs":false,"Ulimits":null,"LogConfig":{"Type":"","Config":null},"CgroupParent":""}` + if err = ioutil.WriteFile(filepath.Join(containerPath, "hostconfig.json"), []byte(hostConfig), 0644); err != nil { + t.Fatal(err) + } + + daemon, err := initDaemonWithVolumeStore(tmp) + if err != nil { + t.Fatal(err) + } + defer volumedrivers.Unregister(volume.DefaultDriverName) + + c, err := daemon.load(containerID) + if err != nil { + t.Fatal(err) + } + + if c.hostConfig.DNS == nil { + t.Fatal("Expected container DNS to not be nil") + } + + if c.hostConfig.DNSSearch == nil { + t.Fatal("Expected container DNSSearch to not be nil") + } + + if c.hostConfig.DNSOptions == nil { + t.Fatal("Expected container DNSOptions to not be nil") + } +} diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index 6dcce34608..919327e795 100644 --- a/daemon/daemon_test.go +++ b/daemon/daemon_test.go @@ -182,7 +182,7 @@ func TestLoadWithVolume(t *testing.T) { t.Fatal(err) } - daemon, err := initDaemonForVolumesTest(tmp) + daemon, err := initDaemonWithVolumeStore(tmp) if err != nil { t.Fatal(err) } @@ -270,7 +270,7 @@ func TestLoadWithBindMount(t *testing.T) { t.Fatal(err) } - daemon, err := initDaemonForVolumesTest(tmp) + daemon, err := initDaemonWithVolumeStore(tmp) if err != nil { t.Fatal(err) } @@ -361,7 +361,7 @@ func TestLoadWithVolume17RC(t *testing.T) { t.Fatal(err) } - daemon, err := initDaemonForVolumesTest(tmp) + daemon, err := initDaemonWithVolumeStore(tmp) if err != nil { t.Fatal(err) } @@ -466,7 +466,7 @@ func TestRemoveLocalVolumesFollowingSymlinks(t *testing.T) { t.Fatal(err) } - daemon, err := initDaemonForVolumesTest(tmp) + daemon, err := initDaemonWithVolumeStore(tmp) if err != nil { t.Fatal(err) } @@ -502,7 +502,7 @@ func TestRemoveLocalVolumesFollowingSymlinks(t *testing.T) { } } -func initDaemonForVolumesTest(tmp string) (*Daemon, error) { +func initDaemonWithVolumeStore(tmp string) (*Daemon, error) { daemon := &Daemon{ repository: tmp, root: tmp, diff --git a/integration-cli/docker_cli_inspect_test.go b/integration-cli/docker_cli_inspect_test.go index 74a7a1b9af..a035f1a15b 100644 --- a/integration-cli/docker_cli_inspect_test.go +++ b/integration-cli/docker_cli_inspect_test.go @@ -329,3 +329,11 @@ func (s *DockerSuite) TestInspectTempateError(c *check.C) { c.Assert(err, check.Not(check.IsNil)) c.Assert(out, checker.Contains, "Template parsing error") } + +func (s *DockerSuite) TestInspectJSONFields(c *check.C) { + dockerCmd(c, "run", "--name=busybox", "-d", "busybox", "top") + out, _, err := dockerCmdWithError("inspect", "--type=container", "--format='{{.HostConfig.Dns}}'", "busybox") + + c.Assert(err, check.IsNil) + c.Assert(out, checker.Equals, "[]\n") +} diff --git a/opts/opts.go b/opts/opts.go index ebfcfd1fc7..806dfff6cd 100644 --- a/opts/opts.go +++ b/opts/opts.go @@ -98,6 +98,16 @@ func (opts *ListOpts) GetAll() []string { return (*opts.values) } +// GetAllOrEmpty returns the values of the slice +// or an empty slice when there are no values. +func (opts *ListOpts) GetAllOrEmpty() []string { + v := *opts.values + if v == nil { + return make([]string, 0) + } + return v +} + // Get checks the existence of the specified key. func (opts *ListOpts) Get(key string) bool { for _, k := range *opts.values { diff --git a/runconfig/parse.go b/runconfig/parse.go index 09899e7f6a..b2451e8475 100644 --- a/runconfig/parse.go +++ b/runconfig/parse.go @@ -348,27 +348,32 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe PortBindings: portBindings, Links: flLinks.GetAll(), PublishAllPorts: *flPublishAll, - DNS: flDNS.GetAll(), - DNSSearch: flDNSSearch.GetAll(), - DNSOptions: flDNSOptions.GetAll(), - ExtraHosts: flExtraHosts.GetAll(), - VolumesFrom: flVolumesFrom.GetAll(), - NetworkMode: NetworkMode(*flNetMode), - IpcMode: ipcMode, - PidMode: pidMode, - UTSMode: utsMode, - Devices: deviceMappings, - CapAdd: stringutils.NewStrSlice(flCapAdd.GetAll()...), - CapDrop: stringutils.NewStrSlice(flCapDrop.GetAll()...), - GroupAdd: flGroupAdd.GetAll(), - RestartPolicy: restartPolicy, - SecurityOpt: flSecurityOpt.GetAll(), - ReadonlyRootfs: *flReadonlyRootfs, - Ulimits: flUlimits.GetList(), - LogConfig: LogConfig{Type: *flLoggingDriver, Config: loggingOpts}, - CgroupParent: *flCgroupParent, - VolumeDriver: *flVolumeDriver, - Isolation: IsolationLevel(*flIsolation), + // Make sure the dns fields are never nil. + // New containers don't ever have those fields nil, + // but pre created containers can still have those nil values. + // See https://github.com/docker/docker/pull/17779 + // for a more detailed explanation on why we don't want that. + DNS: flDNS.GetAllOrEmpty(), + DNSSearch: flDNSSearch.GetAllOrEmpty(), + DNSOptions: flDNSOptions.GetAllOrEmpty(), + ExtraHosts: flExtraHosts.GetAll(), + VolumesFrom: flVolumesFrom.GetAll(), + NetworkMode: NetworkMode(*flNetMode), + IpcMode: ipcMode, + PidMode: pidMode, + UTSMode: utsMode, + Devices: deviceMappings, + CapAdd: stringutils.NewStrSlice(flCapAdd.GetAll()...), + CapDrop: stringutils.NewStrSlice(flCapDrop.GetAll()...), + GroupAdd: flGroupAdd.GetAll(), + RestartPolicy: restartPolicy, + SecurityOpt: flSecurityOpt.GetAll(), + ReadonlyRootfs: *flReadonlyRootfs, + Ulimits: flUlimits.GetList(), + LogConfig: LogConfig{Type: *flLoggingDriver, Config: loggingOpts}, + CgroupParent: *flCgroupParent, + VolumeDriver: *flVolumeDriver, + Isolation: IsolationLevel(*flIsolation), } // When allocating stdin in attached mode, close stdin at client disconnect