From 791aa3c33864420725ffa1f6864668d01c53335c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 8 Aug 2019 13:18:50 +0200 Subject: [PATCH 1/2] make.ps1: Run-IntegrationTests(): set working directory for test suite This function changed to the correct working directory before starting the tests (which is the same as on Linux), however the `ProcessStartInfo` process does not inherit this working directory, which caused Windows tests to be running with a different working directory as Linux (causing files used in tests to not be found). From the documentation; https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.processstartinfo.workingdirectory?view=netframework-4.8 > When `UseShellExecute` is `true`, the fully qualified name of the directory that contains > the process to be started. When the `UseShellExecute` property is `false`, the working > directory for the process to be started. The default is an empty string (`""`). This patch sets the `ProcessStartInfo.WorkingDirectory` to the correct working directory before starting the process. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 6ae46aeabf056180067dd6af8d5d8588d6075c31) Signed-off-by: Sebastiaan van Stijn --- hack/make.ps1 | 1 + 1 file changed, 1 insertion(+) diff --git a/hack/make.ps1 b/hack/make.ps1 index 3c51716ed39d7..bde0679c604c1 100644 --- a/hack/make.ps1 +++ b/hack/make.ps1 @@ -343,6 +343,7 @@ Function Run-IntegrationTests() { Write-Host "Running $($PWD.Path)" $pinfo = New-Object System.Diagnostics.ProcessStartInfo $pinfo.FileName = "$($PWD.Path)\test.exe" + $pinfo.WorkingDirectory = "$($PWD.Path)" $pinfo.RedirectStandardError = $true $pinfo.UseShellExecute = $false $pinfo.Arguments = $env:INTEGRATION_TESTFLAGS From ff0a0e364bc125c594f58cb219abf69a59734348 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 8 Aug 2019 10:55:12 +0200 Subject: [PATCH 2/2] Builder: fix "COPY --from" to non-existing directory on Windows This fixes a regression introduced in 6d87f19142f86b8fee75af721b583a306202f228, causing `COPY --from` to fail if the target directory does not exist: ``` FROM mcr.microsoft.com/windows/servercore:ltsc2019 as s1 RUN echo "Hello World" > /hello FROM mcr.microsoft.com/windows/servercore:ltsc2019 COPY --from=s1 /hello /hello/another/world ``` Would produce an error: ``` Step 4/4 : COPY --from=s1 /hello /hello/another/world failed to copy files: mkdir \\?: The filename, directory name, or volume label syntax is incorrect. ``` The cause for this was that Go's `os.MkdirAll()` does not support/detect volume GUID paths (`\\?\Volume{dae8d3ac-b9a1-11e9-88eb-e8554b2ba1db}\hello\another}`), and as a result attempted to create the volume as a directory (`\\?`), causing it to fail. This patch replaces `os.MkdirAll()` with our own `system.MkdirAll()` function, which is capable of detecting GUID volumes. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 5858a99267822b93e2d304d876bab84d05b227c6) Signed-off-by: Sebastiaan van Stijn --- builder/dockerfile/copy.go | 6 ++- integration/build/build_test.go | 52 +++++++++++++++++++ .../Dockerfile.TestBuildMultiStageCopy | 20 +++++++ 3 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 integration/build/testdata/Dockerfile.TestBuildMultiStageCopy diff --git a/builder/dockerfile/copy.go b/builder/dockerfile/copy.go index 4e1c7e06dc6d6..2c08b693679bc 100644 --- a/builder/dockerfile/copy.go +++ b/builder/dockerfile/copy.go @@ -556,13 +556,15 @@ func copyFile(archiver Archiver, source, dest *copyEndpoint, identity *idtools.I return errors.Wrapf(err, "failed to create new directory") } } else { + // Normal containers if identity == nil { - if err := os.MkdirAll(filepath.Dir(dest.path), 0755); err != nil { + // Use system.MkdirAll here, which is a custom version of os.MkdirAll + // modified for use on Windows to handle volume GUID paths (\\?\{dae8d3ac-b9a1-11e9-88eb-e8554b2ba1db}\path\) + if err := system.MkdirAll(filepath.Dir(dest.path), 0755, ""); err != nil { return err } } else { if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest.path), 0755, *identity); err != nil { - // Normal containers return errors.Wrapf(err, "failed to create new directory") } } diff --git a/integration/build/build_test.go b/integration/build/build_test.go index 183e7142265f8..b48921d1a4931 100644 --- a/integration/build/build_test.go +++ b/integration/build/build_test.go @@ -135,6 +135,58 @@ func buildContainerIdsFilter(buildOutput io.Reader) (filters.Args, error) { } } +// TestBuildMultiStageCopy verifies that copying between stages works correctly. +// +// Regression test for docker/for-win#4349, ENGCORE-935, where creating the target +// directory failed on Windows, because `os.MkdirAll()` was called with a volume +// GUID path (\\?\Volume{dae8d3ac-b9a1-11e9-88eb-e8554b2ba1db}\newdir\hello}), +// which currently isn't supported by Golang. +func TestBuildMultiStageCopy(t *testing.T) { + ctx := context.Background() + + dockerfile, err := ioutil.ReadFile("testdata/Dockerfile." + t.Name()) + assert.NilError(t, err) + + source := fakecontext.New(t, "", fakecontext.WithDockerfile(string(dockerfile))) + defer source.Close() + + apiclient := testEnv.APIClient() + + for _, target := range []string{"copy_to_root", "copy_to_newdir", "copy_to_newdir_nested", "copy_to_existingdir", "copy_to_newsubdir"} { + t.Run(target, func(t *testing.T) { + imgName := strings.ToLower(t.Name()) + + resp, err := apiclient.ImageBuild( + ctx, + source.AsTarReader(t), + types.ImageBuildOptions{ + Remove: true, + ForceRemove: true, + Target: target, + Tags: []string{imgName}, + }, + ) + assert.NilError(t, err) + + out := bytes.NewBuffer(nil) + assert.NilError(t, err) + _, err = io.Copy(out, resp.Body) + _ = resp.Body.Close() + if err != nil { + t.Log(out) + } + assert.NilError(t, err) + + // verify the image was successfully built + _, _, err = apiclient.ImageInspectWithRaw(ctx, imgName) + if err != nil { + t.Log(out) + } + assert.NilError(t, err) + }) + } +} + func TestBuildMultiStageParentConfig(t *testing.T) { skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.35"), "broken in earlier versions") skip.If(t, testEnv.DaemonInfo.OSType == "windows", "FIXME") diff --git a/integration/build/testdata/Dockerfile.TestBuildMultiStageCopy b/integration/build/testdata/Dockerfile.TestBuildMultiStageCopy new file mode 100644 index 0000000000000..52b0acff16baa --- /dev/null +++ b/integration/build/testdata/Dockerfile.TestBuildMultiStageCopy @@ -0,0 +1,20 @@ +FROM busybox AS base +RUN mkdir existingdir + +FROM base AS source +RUN echo "Hello World" > /hello + +FROM base AS copy_to_root +COPY --from=source /hello /hello + +FROM base AS copy_to_newdir +COPY --from=source /hello /newdir/hello + +FROM base AS copy_to_newdir_nested +COPY --from=source /hello /newdir/newsubdir/hello + +FROM base AS copy_to_existingdir +COPY --from=source /hello /existingdir/hello + +FROM base AS copy_to_newsubdir +COPY --from=source /hello /existingdir/newsubdir/hello