From 4bbd2e797413f3c22270af4028cf88cfc80dad06 Mon Sep 17 00:00:00 2001 From: Chris Hunt Date: Fri, 17 Aug 2018 10:20:38 -0400 Subject: [PATCH] Remove error return on !exists On Linux, volume binding creates the directory on the host. This conditional check on Windows was preventing Windows Containers from creating the directory producing inconsistent behavior between platforms. Signed-off-by: Chris Hunt --- volume/mounts/parser_test.go | 40 ++++++++++++++++----------------- volume/mounts/validate_test.go | 4 ++-- volume/mounts/windows_parser.go | 5 +---- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/volume/mounts/parser_test.go b/volume/mounts/parser_test.go index 347f7d9c4d243..d5f924b24a5cb 100644 --- a/volume/mounts/parser_test.go +++ b/volume/mounts/parser_test.go @@ -111,16 +111,16 @@ func TestParseMountRaw(t *testing.T) { `c:\windows\:\\?\d:\`, // Long path handling (target) `\\.\pipe\foo:\\.\pipe\foo`, // named pipe `//./pipe/foo://./pipe/foo`, // named pipe forward slashes + `c:\notexist:d:`, }, invalid: map[string]string{ - ``: "invalid volume specification: ", - `.`: "invalid volume specification: ", - `..\`: "invalid volume specification: ", - `c:\:..\`: "invalid volume specification: ", - `c:\:d:\:xyzzy`: "invalid volume specification: ", - `c:`: "cannot be `c:`", - `c:\`: "cannot be `c:`", - `c:\notexist:d:`: `bind mount source path does not exist: c:\notexist`, + ``: "invalid volume specification: ", + `.`: "invalid volume specification: ", + `..\`: "invalid volume specification: ", + `c:\:..\`: "invalid volume specification: ", + `c:\:d:\:xyzzy`: "invalid volume specification: ", + `c:`: "cannot be `c:`", + `c:\`: "cannot be `c:`", `c:\windows\system32\ntdll.dll:d:`: `source path must be a directory`, `name<:d:`: `invalid volume specification`, `name>:d:`: `invalid volume specification`, @@ -178,18 +178,18 @@ func TestParseMountRaw(t *testing.T) { `c:/:/forward/slashes/are/good/too`, `c:/:/including with/spaces:ro`, `/Program Files (x86)`, // With capitals and brackets + `c:\notexist:/foo`, }, invalid: map[string]string{ - ``: "invalid volume specification: ", - `.`: "invalid volume specification: ", - `c:`: "invalid volume specification: ", - `c:\`: "invalid volume specification: ", - `../`: "invalid volume specification: ", - `c:\:../`: "invalid volume specification: ", - `c:\:/foo:xyzzy`: "invalid volume specification: ", - `/`: "destination can't be '/'", - `/..`: "destination can't be '/'", - `c:\notexist:/foo`: `bind mount source path does not exist: c:\notexist`, + ``: "invalid volume specification: ", + `.`: "invalid volume specification: ", + `c:`: "invalid volume specification: ", + `c:\`: "invalid volume specification: ", + `../`: "invalid volume specification: ", + `c:\:../`: "invalid volume specification: ", + `c:\:/foo:xyzzy`: "invalid volume specification: ", + `/`: "destination can't be '/'", + `/..`: "destination can't be '/'", `c:\windows\system32\ntdll.dll:/foo`: `source path must be a directory`, `name<:/foo`: `invalid volume specification`, `name>:/foo`: `invalid volume specification`, @@ -352,7 +352,7 @@ func TestParseMountRawSplit(t *testing.T) { {`driver/name:c:`, "", mount.TypeVolume, ``, ``, ``, "", true, true}, {`\\.\pipe\foo:\\.\pipe\bar`, "local", mount.TypeNamedPipe, `\\.\pipe\bar`, `\\.\pipe\foo`, "", "", true, false}, {`\\.\pipe\foo:c:\foo\bar`, "local", mount.TypeNamedPipe, ``, ``, "", "", true, true}, - {`c:\foo\bar:\\.\pipe\foo`, "local", mount.TypeNamedPipe, ``, ``, "", "", true, true}, + {`c:\foo\bar:\\.\pipe\foo`, "local", mount.TypeBind, `\\.\pipe\foo`, `c:\foo\bar`, "", "", true, false}, } lcowCases := []testParseMountRaw{ {`c:\:/foo`, "local", mount.TypeBind, `/foo`, `c:\`, ``, "", true, false}, @@ -366,7 +366,7 @@ func TestParseMountRawSplit(t *testing.T) { {`driver/name:/`, "", mount.TypeVolume, ``, ``, ``, "", true, true}, {`\\.\pipe\foo:\\.\pipe\bar`, "local", mount.TypeNamedPipe, `\\.\pipe\bar`, `\\.\pipe\foo`, "", "", true, true}, {`\\.\pipe\foo:/data`, "local", mount.TypeNamedPipe, ``, ``, "", "", true, true}, - {`c:\foo\bar:\\.\pipe\foo`, "local", mount.TypeNamedPipe, ``, ``, "", "", true, true}, + {`c:\foo\bar:\\.\pipe\foo`, "local", mount.TypeBind, ``, ``, "", "", true, true}, } linuxCases := []testParseMountRaw{ {"/tmp:/tmp1", "", mount.TypeBind, "/tmp1", "/tmp", "", "", true, false}, diff --git a/volume/mounts/validate_test.go b/volume/mounts/validate_test.go index 4f838560435ae..c77b6cc20ffad 100644 --- a/volume/mounts/validate_test.go +++ b/volume/mounts/validate_test.go @@ -31,7 +31,7 @@ func TestValidateMount(t *testing.T) { {mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath}, nil}, {mount.Mount{Type: "invalid", Target: testDestinationPath}, errors.New("mount type unknown")}, - {mount.Mount{Type: mount.TypeBind, Source: testSourcePath, Target: testDestinationPath}, errBindSourceDoesNotExist(testSourcePath)}, + {mount.Mount{Type: mount.TypeBind, Source: testSourcePath, Target: testDestinationPath}, nil}, } lcowCases := []struct { @@ -44,7 +44,7 @@ func TestValidateMount(t *testing.T) { {mount.Mount{Type: mount.TypeBind}, errMissingField("Target")}, {mount.Mount{Type: mount.TypeBind, Target: "/foo"}, errMissingField("Source")}, {mount.Mount{Type: mount.TypeBind, Target: "/foo", Source: "c:\\foo", VolumeOptions: &mount.VolumeOptions{}}, errExtraField("VolumeOptions")}, - {mount.Mount{Type: mount.TypeBind, Source: "c:\\foo", Target: "/foo"}, errBindSourceDoesNotExist("c:\\foo")}, + {mount.Mount{Type: mount.TypeBind, Source: "c:\\foo", Target: "/foo"}, nil}, {mount.Mount{Type: mount.TypeBind, Source: testDir, Target: "/foo"}, nil}, {mount.Mount{Type: "invalid", Target: "/foo"}, errors.New("mount type unknown")}, } diff --git a/volume/mounts/windows_parser.go b/volume/mounts/windows_parser.go index ac610440436af..2ba9f0daedaba 100644 --- a/volume/mounts/windows_parser.go +++ b/volume/mounts/windows_parser.go @@ -251,10 +251,7 @@ func (p *windowsParser) validateMountConfigReg(mnt *mount.Mount, destRegex strin if err != nil { return &errMountConfig{mnt, err} } - if !exists { - return &errMountConfig{mnt, errBindSourceDoesNotExist(mnt.Source)} - } - if !isdir { + if exists && !isdir { return &errMountConfig{mnt, fmt.Errorf("source path must be a directory")} }