Use an empty slice as default value for DNS, DNSSearch and DNSOptions

So we don't print those <no value> 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 <[email protected]>
ramkrivas · Nov 9, 2015 · f1a74a8 · f1a74a8
1 parent cc207aa
commit f1a74a8
Show file tree

Hide file tree

Showing 6 changed files with 141 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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