Skip to content

Commit

Permalink
Merge pull request moby#23002 from Microsoft/jjh/readonly
Browse files Browse the repository at this point in the history
Windows: Support RO volumes 14350+
  • Loading branch information
crosbymichael committed Jun 8, 2016
2 parents 9db33b7 + 8d174a4 commit 393e97e
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 33 deletions.
22 changes: 17 additions & 5 deletions integration-cli/docker_cli_inspect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"encoding/json"
"fmt"
"os"
"os/exec"
"strconv"
"strings"
Expand Down Expand Up @@ -209,8 +210,19 @@ func (s *DockerSuite) TestInspectContainerGraphDriver(c *check.C) {
}

func (s *DockerSuite) TestInspectBindMountPoint(c *check.C) {
testRequires(c, DaemonIsLinux)
dockerCmd(c, "run", "-d", "--name", "test", "-v", "/data:/data:ro,z", "busybox", "cat")
modifier := ",z"
prefix, slash := getPrefixAndSlashFromDaemonPlatform()
if daemonPlatform == "windows" {
modifier = ""
// TODO Windows: Temporary check - remove once TP5 support is dropped
if windowsDaemonKV < 14350 {
c.Skip("Needs later Windows build for RO volumes")
}
// Linux creates the host directory if it doesn't exist. Windows does not.
os.Mkdir(`c:\data`, os.ModeDir)
}

dockerCmd(c, "run", "-d", "--name", "test", "-v", prefix+slash+"data:"+prefix+slash+"data:ro"+modifier, "busybox", "cat")

vol := inspectFieldJSON(c, "test", "Mounts")

Expand All @@ -225,9 +237,9 @@ func (s *DockerSuite) TestInspectBindMountPoint(c *check.C) {

c.Assert(m.Name, checker.Equals, "")
c.Assert(m.Driver, checker.Equals, "")
c.Assert(m.Source, checker.Equals, "/data")
c.Assert(m.Destination, checker.Equals, "/data")
c.Assert(m.Mode, checker.Equals, "ro,z")
c.Assert(m.Source, checker.Equals, prefix+slash+"data")
c.Assert(m.Destination, checker.Equals, prefix+slash+"data")
c.Assert(m.Mode, checker.Equals, "ro"+modifier)
c.Assert(m.RW, checker.Equals, false)
}

Expand Down
45 changes: 26 additions & 19 deletions integration-cli/docker_cli_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,18 +443,20 @@ func (s *DockerSuite) TestRunCreateVolumesInSymlinkDir2(c *check.C) {
}

func (s *DockerSuite) TestRunVolumesMountedAsReadonly(c *check.C) {
// TODO Windows (Post TP5): This test cannot run on a Windows daemon as
// Windows does not support read-only bind mounts.
testRequires(c, DaemonIsLinux)
// TODO Windows: Temporary check - remove once TP5 support is dropped
if daemonPlatform == "windows" && windowsDaemonKV < 14350 {
c.Skip("Needs later Windows build for RO volumes")
}
if _, code, err := dockerCmdWithError("run", "-v", "/test:/test:ro", "busybox", "touch", "/test/somefile"); err == nil || code == 0 {
c.Fatalf("run should fail because volume is ro: exit code %d", code)
}
}

func (s *DockerSuite) TestRunVolumesFromInReadonlyModeFails(c *check.C) {
// TODO Windows (Post TP5): This test cannot run on a Windows daemon as
// Windows does not support read-only bind mounts. Modified for when ro is supported.
testRequires(c, DaemonIsLinux)
// TODO Windows: Temporary check - remove once TP5 support is dropped
if daemonPlatform == "windows" && windowsDaemonKV < 14350 {
c.Skip("Needs later Windows build for RO volumes")
}
var (
volumeDir string
fileInVol string
Expand Down Expand Up @@ -499,20 +501,23 @@ func (s *DockerSuite) TestRunVolumesFromInReadWriteMode(c *check.C) {
}

func (s *DockerSuite) TestVolumesFromGetsProperMode(c *check.C) {
// TODO Windows: This test cannot yet run on a Windows daemon as Windows does
// not support read-only bind mounts as at TP5
testRequires(c, DaemonIsLinux)
dockerCmd(c, "run", "--name", "parent", "-v", "/test:/test:ro", "busybox", "true")
prefix, slash := getPrefixAndSlashFromDaemonPlatform()

// TODO Windows: Temporary check - remove once TP5 support is dropped
if daemonPlatform == "windows" && windowsDaemonKV < 14350 {
c.Skip("Needs later Windows build for RO volumes")
}
dockerCmd(c, "run", "--name", "parent", "-v", prefix+slash+"test:"+prefix+slash+"test:ro", "busybox", "true")

// Expect this "rw" mode to be be ignored since the inherited volume is "ro"
if _, _, err := dockerCmdWithError("run", "--volumes-from", "parent:rw", "busybox", "touch", "/test/file"); err == nil {
if _, _, err := dockerCmdWithError("run", "--volumes-from", "parent:rw", "busybox", "touch", prefix+slash+"test"+slash+"file"); err == nil {
c.Fatal("Expected volumes-from to inherit read-only volume even when passing in `rw`")
}

dockerCmd(c, "run", "--name", "parent2", "-v", "/test:/test:ro", "busybox", "true")
dockerCmd(c, "run", "--name", "parent2", "-v", prefix+slash+"test:"+prefix+slash+"test:ro", "busybox", "true")

// Expect this to be read-only since both are "ro"
if _, _, err := dockerCmdWithError("run", "--volumes-from", "parent2:ro", "busybox", "touch", "/test/file"); err == nil {
if _, _, err := dockerCmdWithError("run", "--volumes-from", "parent2:ro", "busybox", "touch", prefix+slash+"test"+slash+"file"); err == nil {
c.Fatal("Expected volumes-from to inherit read-only volume even when passing in `ro`")
}
}
Expand Down Expand Up @@ -1936,6 +1941,8 @@ func (s *DockerSuite) TestRunBindMounts(c *check.C) {
testRequires(c, DaemonIsLinux, NotUserNamespace)
}

prefix, _ := getPrefixAndSlashFromDaemonPlatform()

tmpDir, err := ioutil.TempDir("", "docker-test-container")
if err != nil {
c.Fatal(err)
Expand All @@ -1944,10 +1951,10 @@ func (s *DockerSuite) TestRunBindMounts(c *check.C) {
defer os.RemoveAll(tmpDir)
writeFile(path.Join(tmpDir, "touch-me"), "", c)

// TODO Windows Post TP5. Windows does not yet support :ro binds
if daemonPlatform != "windows" {
// TODO Windows: Temporary check - remove once TP5 support is dropped
if daemonPlatform != "windows" || windowsDaemonKV >= 14350 {
// Test reading from a read-only bind mount
out, _ := dockerCmd(c, "run", "-v", fmt.Sprintf("%s:/tmp:ro", tmpDir), "busybox", "ls", "/tmp")
out, _ := dockerCmd(c, "run", "-v", fmt.Sprintf("%s:%s/tmp:ro", tmpDir, prefix), "busybox", "ls", prefix+"/tmp")
if !strings.Contains(out, "touch-me") {
c.Fatal("Container failed to read from bind mount")
}
Expand Down Expand Up @@ -3147,12 +3154,12 @@ func (s *DockerSuite) TestRunCapAddCHOWN(c *check.C) {

// https://github.com/docker/docker/pull/14498
func (s *DockerSuite) TestVolumeFromMixedRWOptions(c *check.C) {
// TODO Windows post TP5. Enable the read-only bits once they are
// supported on the platform.
prefix, slash := getPrefixAndSlashFromDaemonPlatform()

dockerCmd(c, "run", "--name", "parent", "-v", prefix+"/test", "busybox", "true")
if daemonPlatform != "windows" {

// TODO Windows: Temporary check - remove once TP5 support is dropped
if daemonPlatform != "windows" || windowsDaemonKV >= 14350 {
dockerCmd(c, "run", "--volumes-from", "parent:ro", "--name", "test-volumes-1", "busybox", "true")
}
dockerCmd(c, "run", "--volumes-from", "parent:rw", "--name", "test-volumes-2", "busybox", "true")
Expand Down
25 changes: 16 additions & 9 deletions volume/volume_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"

"github.com/Sirupsen/logrus"
"github.com/docker/docker/pkg/system"
)

// read-write modes
Expand Down Expand Up @@ -68,17 +69,23 @@ const (
// - Variation on hostdir but can be a drive followed by colon as well
// - If a path, must be absolute. Can include spaces
// - Drive cannot be c: (explicitly checked in code, not RegEx)
//
)

// RXMode is the regex expression for the mode of the mount
RXMode = `(:(?P<mode>(?i)rw))?`
// Temporarily for TP4, disabling the use of ro as it's not supported yet
// in the platform. TODO Windows: `(:(?P<mode>(?i)ro|rw))?`
// mode (optional)
// - Hopefully self explanatory in comparison to above.
// RXMode is the regex expression for the mode of the mount
var RXMode string

func init() {
osv := system.GetOSVersion()
// Read-only volumes supported from 14350 onwards (post Windows Server 2016 TP5)
// Mode (optional):
// - Hopefully self explanatory in comparison to above regex's.
// - Colon is not in the capture group
//
)
if osv.Build >= 14350 {
RXMode = `(:(?P<mode>(?i)ro|rw))?`
} else {
RXMode = `(:(?P<mode>(?i)rw))?`
}
}

// BackwardsCompatible decides whether this mount point can be
// used in old versions of Docker or not.
Expand Down

0 comments on commit 393e97e

Please sign in to comment.