Merge pull request moby#21223 from cpuguy83/add_nocp_to_vols

tiborvass · tiborvass · commit de9ff4bdc0e1 · 2016-03-22T13:45:12.000-04:00
Add explicit flags for volume cp/no-cp
diff --git a/container/container.go b/container/container.go
@@ -552,6 +552,7 @@ func (container *Container) AddMountPointWithVolume(destination string, vol volu
 		Destination: destination,
 		RW:          rw,
 		Volume:      vol,
+		CopyData:    volume.DefaultCopyMode,
 	}
 }
 
diff --git a/container/container_unix.go b/container/container_unix.go
@@ -185,12 +185,8 @@ func (container *Container) CopyImagePathContent(v volume.Volume, destination st
 	if err != nil {
 		return err
 	}
-
-	if err := copyExistingContents(rootfs, path); err != nil {
-		return err
-	}
-
-	return v.Unmount()
+	defer v.Unmount()
+	return copyExistingContents(rootfs, path)
 }
 
 // ShmResourcePath returns path to shm
diff --git a/daemon/create_unix.go b/daemon/create_unix.go
@@ -63,8 +63,7 @@ func (daemon *Daemon) createContainerPlatformSpecificSettings(container *contain
 // this is only called when the container is created.
 func (daemon *Daemon) populateVolumes(c *container.Container) error {
 	for _, mnt := range c.MountPoints {
-		// skip binds and volumes referenced by other containers (ie, volumes-from)
-		if mnt.Driver == "" || mnt.Volume == nil || len(daemon.volumes.Refs(mnt.Volume)) > 1 {
+		if !mnt.CopyData || mnt.Volume == nil {
 			continue
 		}
 
diff --git a/daemon/volumes.go b/daemon/volumes.go
@@ -131,6 +131,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
 				bind = setBindModeIfNull(bind)
 			}
 		}
+
 		if label.RelabelNeeded(bind.Mode) {
 			if err := label.Relabel(bind.Source, container.MountLabel, label.IsShared(bind.Mode)); err != nil {
 				return err
diff --git a/docs/reference/api/docker_remote_api.md b/docs/reference/api/docker_remote_api.md
@@ -126,6 +126,7 @@ This section lists each version from latest to oldest.  Each listing includes a
 * `POST /containers/create` now takes `PidsLimit` field, if the kernel is >= 4.3 and the pids cgroup is supported.
 * `GET /containers/(id or name)/stats` now returns `pids_stats`, if the kernel is >= 4.3 and the pids cgroup is supported.
 * `POST /containers/create` now allows you to override usernamespaces remapping and use privileged options for the container.
+* `POST /containers/create` now allows specifying `nocopy` for named volumes, which disables automatic copying from the container path to the volume.
 * `POST /auth` now returns an `IdentityToken` when supported by a registry.
 * `POST /containers/create` with both `Hostname` and `Domainname` fields specified will result in the container's hostname being set to `Hostname`, rather than `Hostname.Domainname`.
 
diff --git a/docs/reference/commandline/create.md b/docs/reference/commandline/create.md
@@ -90,10 +90,10 @@ Creates a new container.
       --uts=""                      UTS namespace to use
       -v, --volume=[host-src:]container-dest[:<options>]
                                     Bind mount a volume. The comma-delimited
-                                    `options` are [rw|ro], [z|Z], or
-                                    [[r]shared|[r]slave|[r]private]. The
-                                    'host-src' is an absolute path or a name
-                                    value.
+                                    `options` are [rw|ro], [z|Z],
+                                    [[r]shared|[r]slave|[r]private], and
+                                    [nocopy]. The 'host-src' is an absolute path
+                                    or a name value.
       --volume-driver=""            Container's volume driver
       --volumes-from=[]             Mount volumes from the specified container(s)
       -w, --workdir=""              Working directory inside the container
diff --git a/docs/reference/commandline/run.md b/docs/reference/commandline/run.md
@@ -92,10 +92,10 @@ parent = "smn_cli"
       --uts=""                      UTS namespace to use
       -v, --volume=[host-src:]container-dest[:<options>]
                                     Bind mount a volume. The comma-delimited
-                                    `options` are [rw|ro], [z|Z], or
-                                    [[r]shared|[r]slave|[r]private]. The
-                                    'host-src' is an absolute path or a name
-                                    value.
+                                    `options` are [rw|ro], [z|Z],
+                                    [[r]shared|[r]slave|[r]private], and
+                                    [nocopy]. The 'host-src' is an absolute path
+                                    or a name value.
       --volume-driver=""            Container's volume driver
       --volumes-from=[]             Mount volumes from the specified container(s)
       -w, --workdir=""              Working directory inside the container
diff --git a/docs/reference/run.md b/docs/reference/run.md
@@ -1400,13 +1400,18 @@ The example below mounts an empty tmpfs into the container with the `rw`,
 ### VOLUME (shared filesystems)
 
     -v, --volume=[host-src:]container-dest[:<options>]: Bind mount a volume.
-    The comma-delimited `options` are [rw|ro], [z|Z], or
-    [[r]shared|[r]slave|[r]private]. The 'host-src' is an absolute path or a
-    name value.
+    The comma-delimited `options` are [rw|ro], [z|Z],
+    [[r]shared|[r]slave|[r]private], and [nocopy].
+    The 'host-src' is an absolute path or a name value.
 
     If neither 'rw' or 'ro' is specified then the volume is mounted in
     read-write mode.
 
+    The `nocopy` modes is used to disable automatic copying requested volume
+    path in the container to the volume storage location.
+    For named volumes, `copy` is the default mode. Copy modes are not supported
+    for bind-mounted volumes.
+
     --volumes-from="": Mount all volumes from the given container(s)
 
 > **Note**:
diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go
@@ -4249,6 +4249,44 @@ func (s *DockerSuite) TestRunVolumeWithOneCharacter(c *check.C) {
 	testRequires(c, DaemonIsLinux)
 
 	out, _ := dockerCmd(c, "run", "-v", "/tmp/q:/foo", "busybox", "sh", "-c", "find /foo")
-	fmt.Printf("OUTPUT: %+v", out)
 	c.Assert(strings.TrimSpace(out), checker.Equals, "/foo")
 }
+
+func (s *DockerSuite) TestRunVolumeCopyFlag(c *check.C) {
+	testRequires(c, DaemonIsLinux) // Windows does not support copying data from image to the volume
+	_, err := buildImage("volumecopy",
+		`FROM busybox
+		RUN mkdir /foo && echo hello > /foo/bar
+		CMD cat /foo/bar`,
+		true,
+	)
+	c.Assert(err, checker.IsNil)
+
+	dockerCmd(c, "volume", "create", "--name=test")
+
+	// test with the nocopy flag
+	out, _, err := dockerCmdWithError("run", "-v", "test:/foo:nocopy", "volumecopy")
+	c.Assert(err, checker.NotNil, check.Commentf(out))
+	// test default behavior which is to copy for non-binds
+	out, _ = dockerCmd(c, "run", "-v", "test:/foo", "volumecopy")
+	c.Assert(strings.TrimSpace(out), checker.Equals, "hello")
+	// error out when the volume is already populated
+	out, _, err = dockerCmdWithError("run", "-v", "test:/foo:copy", "volumecopy")
+	c.Assert(err, checker.NotNil, check.Commentf(out))
+	// do not error out when copy isn't explicitly set even though it's already populated
+	out, _ = dockerCmd(c, "run", "-v", "test:/foo", "volumecopy")
+	c.Assert(strings.TrimSpace(out), checker.Equals, "hello")
+
+	// do not allow copy modes on volumes-from
+	dockerCmd(c, "run", "--name=test", "-v", "/foo", "busybox", "true")
+	out, _, err = dockerCmdWithError("run", "--volumes-from=test:copy", "busybox", "true")
+	c.Assert(err, checker.NotNil, check.Commentf(out))
+	out, _, err = dockerCmdWithError("run", "--volumes-from=test:nocopy", "busybox", "true")
+	c.Assert(err, checker.NotNil, check.Commentf(out))
+
+	// do not allow copy modes on binds
+	out, _, err = dockerCmdWithError("run", "-v", "/foo:/bar:copy", "busybox", "true")
+	c.Assert(err, checker.NotNil, check.Commentf(out))
+	out, _, err = dockerCmdWithError("run", "-v", "/foo:/bar:nocopy", "busybox", "true")
+	c.Assert(err, checker.NotNil, check.Commentf(out))
+}
diff --git a/man/docker-create.1.md b/man/docker-create.1.md
@@ -434,6 +434,10 @@ change propagation properties of source mount. Say `/` is source mount for
 > is `slave`, you may not be able to use the `shared` or `rshared` propagation on
 > a volume.
 
+
+To disable automatic copying of data from the container path to the volume, use
+the `nocopy` flag. The `nocopy` flag can be set on bind mounts and named volumes.
+
 **--volume-driver**=""
    Container's volume driver. This driver creates volumes specified either from
    a Dockerfile's `VOLUME` instruction or from the `docker run -v` flag.
diff --git a/man/docker-run.1.md b/man/docker-run.1.md
@@ -531,6 +531,7 @@ any options, the systems uses the following options:
    * [rw|ro]
    * [z|Z]
    * [`[r]shared`|`[r]slave`|`[r]private`]
+   * [nocopy]
 
 The `CONTAINER-DIR` must be an absolute path such as `/src/docs`. The `HOST-DIR`
 can be an absolute path or a `name` value. A `name` value must start with an
@@ -603,6 +604,9 @@ change propagation properties of source mount. Say `/` is source mount for
 > is `slave`, you may not be able to use the `shared` or `rshared` propagation on
 > a volume.
 
+To disable automatic copying of data from the container path to the volume, use
+the `nocopy` flag. The `nocopy` flag can be set on bind mounts and named volumes.
+
 **--volume-driver**=""
    Container's volume driver. This driver creates volumes specified either from
    a Dockerfile's `VOLUME` instruction or from the `docker run -v` flag.
diff --git a/volume/volume.go b/volume/volume.go
@@ -60,6 +60,11 @@ type MountPoint struct {
 	// Note Propagation is not used on Windows
 	Propagation string // Mount propagation string
 	Named       bool   // specifies if the mountpoint was specified by name
+
+	// Specifies if data should be copied from the container before the first mount
+	// Use a pointer here so we can tell if the user set this value explicitly
+	// This allows us to error out when the user explicitly enabled copy but we can't copy due to the volume being populated
+	CopyData bool `json:"-"`
 }
 
 // Setup sets up a mount point by either mounting the volume if it is
@@ -115,6 +120,10 @@ func ParseVolumesFrom(spec string) (string, string, error) {
 		if HasPropagation(mode) {
 			return "", "", errInvalidMode(mode)
 		}
+		// Do not allow copy modes on volumes-from
+		if _, isSet := getCopyMode(mode); isSet {
+			return "", "", errInvalidMode(mode)
+		}
 	}
 	return id, mode, nil
 }
diff --git a/volume/volume_copy.go b/volume/volume_copy.go
@@ -0,0 +1,28 @@
+package volume
+
+import "strings"
+
+const (
+	// DefaultCopyMode is the copy mode used by default for normal/named volumes
+	DefaultCopyMode = true
+)
+
+// {<copy mode>=isEnabled}
+var copyModes = map[string]bool{
+	"nocopy": false,
+}
+
+func copyModeExists(mode string) bool {
+	_, exists := copyModes[mode]
+	return exists
+}
+
+// GetCopyMode gets the copy mode from the mode string for mounts
+func getCopyMode(mode string) (bool, bool) {
+	for _, o := range strings.Split(mode, ",") {
+		if isEnabled, exists := copyModes[o]; exists {
+			return isEnabled, true
+		}
+	}
+	return DefaultCopyMode, false
+}
diff --git a/volume/volume_unix.go b/volume/volume_unix.go
@@ -110,6 +110,13 @@ func ParseMountSpec(spec, volumeDriver string) (*MountPoint, error) {
 		mp.Source = filepath.Clean(source)
 	}
 
+	copyData, isSet := getCopyMode(mp.Mode)
+	// do not allow copy modes on binds
+	if len(name) == 0 && isSet {
+		return nil, errInvalidMode(mp.Mode)
+	}
+
+	mp.CopyData = copyData
 	mp.Name = name
 
 	return mp, nil
@@ -137,23 +144,25 @@ func ValidMountMode(mode string) bool {
 	rwModeCount := 0
 	labelModeCount := 0
 	propagationModeCount := 0
+	copyModeCount := 0
 
 	for _, o := range strings.Split(mode, ",") {
-		if rwModes[o] {
+		switch {
+		case rwModes[o]:
 			rwModeCount++
-			continue
-		} else if labelModes[o] {
+		case labelModes[o]:
 			labelModeCount++
-			continue
-		} else if propagationModes[o] {
+		case propagationModes[o]:
 			propagationModeCount++
-			continue
+		case copyModeExists(o):
+			copyModeCount++
+		default:
+			return false
 		}
-		return false
 	}
 
 	// Only one string for each mode is allowed.
-	if rwModeCount > 1 || labelModeCount > 1 || propagationModeCount > 1 {
+	if rwModeCount > 1 || labelModeCount > 1 || propagationModeCount > 1 || copyModeCount > 1 {
 		return false
 	}
 	return true

Original file line number	Diff line number	Diff line change
`@@ -552,6 +552,7 @@ func (container *Container) AddMountPointWithVolume(destination string, vol volu`
`552`	`552`	`Destination: destination,`
`553`	`553`	`RW: rw,`
`554`	`554`	`Volume: vol,`
	`555`	`+ CopyData: volume.DefaultCopyMode,`
`555`	`556`	`}`
`556`	`557`	`}`
`557`	`558`
Original file line number	Diff line number	Diff line change
`@@ -185,12 +185,8 @@ func (container *Container) CopyImagePathContent(v volume.Volume, destination st`
`185`	`185`	`if err != nil {`
`186`	`186`	`return err`
`187`	`187`	`}`
`188`		`-`
`189`		`- if err := copyExistingContents(rootfs, path); err != nil {`
`190`		`- return err`
`191`		`- }`
`192`		`-`
`193`		`- return v.Unmount()`
	`188`	`+ defer v.Unmount()`
	`189`	`+ return copyExistingContents(rootfs, path)`
`194`	`190`	`}`
`195`	`191`
`196`	`192`	`// ShmResourcePath returns path to shm`
Original file line number	Diff line number	Diff line change
`@@ -63,8 +63,7 @@ func (daemon Daemon) createContainerPlatformSpecificSettings(container contain`
`63`	`63`	`// this is only called when the container is created.`
`64`	`64`	`func (daemon Daemon) populateVolumes(c container.Container) error {`
`65`	`65`	`for _, mnt := range c.MountPoints {`
`66`		`- // skip binds and volumes referenced by other containers (ie, volumes-from)`
`67`		`- if mnt.Driver == "" \|\| mnt.Volume == nil \|\| len(daemon.volumes.Refs(mnt.Volume)) > 1 {`
	`66`	`+ if !mnt.CopyData \|\| mnt.Volume == nil {`
`68`	`67`	`continue`
`69`	`68`	`}`
`70`	`69`
Original file line number	Diff line number	Diff line change
`@@ -131,6 +131,7 @@ func (daemon Daemon) registerMountPoints(container container.Container, hostCo`
`131`	`131`	`bind = setBindModeIfNull(bind)`
`132`	`132`	`}`
`133`	`133`	`}`
	`134`	`+`
`134`	`135`	`if label.RelabelNeeded(bind.Mode) {`
`135`	`136`	`if err := label.Relabel(bind.Source, container.MountLabel, label.IsShared(bind.Mode)); err != nil {`
`136`	`137`	`return err`