From ed95e3d3254415bad04ad30913c7e39e89886c31 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 (cherry picked from commit f1a74a89f89affcfbe311e89aa752b3d551e0340) --- 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 | 45 +++++++------- 6 files changed, 140 insertions(+), 26 deletions(-) diff --git a/daemon/container.go b/daemon/container.go index ace100c7e..a86bc3b8a 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -153,7 +153,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 71d37cf43..f7951804a 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 d286bcfeb..eb353337d 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 ee8597363..f722ae22c 100644 --- a/integration-cli/docker_cli_inspect_test.go +++ b/integration-cli/docker_cli_inspect_test.go @@ -389,3 +389,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 3d8e4b481..b47cc4b2f 100644 --- a/opts/opts.go +++ b/opts/opts.go @@ -101,6 +101,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 7297597d5..9577b7ff6 100644 --- a/runconfig/parse.go +++ b/runconfig/parse.go @@ -361,26 +361,31 @@ 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, + // 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, } // When allocating stdin in attached mode, close stdin at client disconnect