Merge pull request #13563 from calavera/fix_bind_mount_regression

Mount bind volumes coming from the old volumes configuration.

daemon/container.go

@@ -1054,6 +1054,15 @@ func (container *Container) networkMounts() []execdriver.Mount {
 	return mounts
 }
 
+func (container *Container) addBindMountPoint(name, source, destination string, rw bool) {
+	container.MountPoints[destination] = &mountPoint{
+		Name:        name,
+		Source:      source,
+		Destination: destination,
+		RW:          rw,
+	}
+}
+
 func (container *Container) addLocalMountPoint(name, destination string, rw bool) {
 	container.MountPoints[destination] = &mountPoint{
 		Name:        name,

daemon/daemon.go

@@ -213,7 +213,7 @@ func (daemon *Daemon) register(container *Container, updateSuffixarray bool) err
 	// we'll waste time if we update it for every container
 	daemon.idIndex.Add(container.ID)
 
-	if err := daemon.verifyOldVolumesInfo(container); err != nil {
+	if err := daemon.verifyVolumesInfo(container); err != nil {
 		return err
 	}
 

daemon/daemon_test.go

@@ -165,10 +165,6 @@ func TestLoadWithVolume(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if err = os.MkdirAll(volumePath, 0755); err != nil {
-		t.Fatal(err)
-	}
-
 	daemon := &Daemon{
 		repository: tmp,
 		root:       tmp,
@@ -179,7 +175,7 @@ func TestLoadWithVolume(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = daemon.verifyOldVolumesInfo(c)
+	err = daemon.verifyVolumesInfo(c)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -205,3 +201,81 @@ func TestLoadWithVolume(t *testing.T) {
 		t.Fatalf("Expected mount driver local, was %s\n", m.Driver)
 	}
 }
+
+func TestLoadWithBindMount(t *testing.T) {
+	tmp, err := ioutil.TempDir("", "docker-daemon-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","PortMapping":null,"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","ExecDriver":"native-0.2","MountLabel":"","ProcessLabel":"","AppArmorProfile":"","RestartCount":0,
+"UpdateDns":false,"Volumes":{"/vol1": "/vol1"},"VolumesRW":{"/vol1":true},"AppliedVolumesFrom":null}`
+
+	if err = ioutil.WriteFile(filepath.Join(containerPath, "config.json"), []byte(config), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	hostConfig := `{"Binds":["/vol1:/vol1"],"ContainerIDFile":"","LxcConf":[],"Memory":0,"MemorySwap":0,"CpuShares":0,"CpusetCpus":"",
+"Privileged":false,"PortBindings":{},"Links":null,"PublishAllPorts":false,"Dns":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 := &Daemon{
+		repository: tmp,
+		root:       tmp,
+	}
+
+	c, err := daemon.load(containerId)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = daemon.verifyVolumesInfo(c)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(c.MountPoints) != 1 {
+		t.Fatalf("Expected 1 volume mounted, was 0\n")
+	}
+
+	m := c.MountPoints["/vol1"]
+	if m.Name != "" {
+		t.Fatalf("Expected empty mount name, was %s\n", m.Name)
+	}
+
+	if m.Source != "/vol1" {
+		t.Fatalf("Expected mount source /vol1, was %s\n", m.Source)
+	}
+
+	if m.Destination != "/vol1" {
+		t.Fatalf("Expected mount destination /vol1, was %s\n", m.Destination)
+	}
+
+	if !m.RW {
+		t.Fatalf("Expected mount point to be RW but it was not\n")
+	}
+}

daemon/volumes.go

@@ -74,7 +74,7 @@ func parseBindMount(spec string, mountLabel string, config *runconfig.Config) (*
 		return nil, fmt.Errorf("Invalid volume specification: %s", spec)
 	}
 
-	name, source, err := parseVolumeSource(arr[0], config)
+	name, source, err := parseVolumeSource(arr[0])
 	if err != nil {
 		return nil, err
 	}
@@ -238,9 +238,9 @@ func (daemon *Daemon) registerMountPoints(container *Container, hostConfig *runc
 	return nil
 }
 
-// verifyOldVolumesInfo ports volumes configured for the containers pre docker 1.7.
+// verifyVolumesInfo ports volumes configured for the containers pre docker 1.7.
 // It reads the container configuration and creates valid mount points for the old volumes.
-func (daemon *Daemon) verifyOldVolumesInfo(container *Container) error {
+func (daemon *Daemon) verifyVolumesInfo(container *Container) error {
 	jsonPath, err := container.jsonPath()
 	if err != nil {
 		return err
@@ -268,12 +268,19 @@ func (daemon *Daemon) verifyOldVolumesInfo(container *Container) error {
 
 	for destination, hostPath := range vols.Volumes {
 		vfsPath := filepath.Join(daemon.root, "vfs", "dir")
+		rw := vols.VolumesRW != nil && vols.VolumesRW[destination]
 
 		if strings.HasPrefix(hostPath, vfsPath) {
 			id := filepath.Base(hostPath)
-
-			rw := vols.VolumesRW != nil && vols.VolumesRW[destination]
 			container.addLocalMountPoint(id, destination, rw)
+		} else { // Bind mount
+			id, source, err := parseVolumeSource(hostPath)
+			// We should not find an error here coming
+			// from the old configuration, but who knows.
+			if err != nil {
+				return err
+			}
+			container.addBindMountPoint(id, source, destination, rw)
 		}
 	}
 

daemon/volumes_experimental.go

@@ -5,7 +5,6 @@ package daemon
 import (
 	"path/filepath"
 
-	"github.com/docker/docker/runconfig"
 	"github.com/docker/docker/volume"
 	"github.com/docker/docker/volume/drivers"
 )
@@ -17,7 +16,7 @@ func getVolumeDriver(name string) (volume.Driver, error) {
 	return volumedrivers.Lookup(name)
 }
 
-func parseVolumeSource(spec string, config *runconfig.Config) (string, string, error) {
+func parseVolumeSource(spec string) (string, string, error) {
 	if !filepath.IsAbs(spec) {
 		return spec, "", nil
 	}

daemon/volumes_stubs.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"path/filepath"
 
-	"github.com/docker/docker/runconfig"
 	"github.com/docker/docker/volume"
 	"github.com/docker/docker/volume/drivers"
 )
@@ -15,7 +14,7 @@ func getVolumeDriver(_ string) (volume.Driver, error) {
 	return volumedrivers.Lookup(volume.DefaultDriverName)
 }
 
-func parseVolumeSource(spec string, _ *runconfig.Config) (string, string, error) {
+func parseVolumeSource(spec string) (string, string, error) {
 	if !filepath.IsAbs(spec) {
 		return "", "", fmt.Errorf("cannot bind mount volume: %s volume paths must be absolute.", spec)
 	}