Browse code

COPY file . after WORKDIR (now always created)

Signed-off-by: John Howard <jhoward@microsoft.com>

John Howard authored on 2016/11/17 08:02:27
Showing 5 changed files
... ...
@@ -129,6 +129,8 @@ type Backend interface {
129 129
 	ContainerWait(containerID string, timeout time.Duration) (int, error)
130 130
 	// ContainerUpdateCmdOnBuild updates container.Path and container.Args
131 131
 	ContainerUpdateCmdOnBuild(containerID string, cmd []string) error
132
+	// ContainerCreateWorkdir creates the workdir (currently only used on Windows)
133
+	ContainerCreateWorkdir(containerID string) error
132 134
 
133 135
 	// ContainerCopy copies/extracts a source FileInfo to a destination path inside a container
134 136
 	// specified by a container object.
... ...
@@ -18,6 +18,7 @@ import (
18 18
 
19 19
 	"github.com/Sirupsen/logrus"
20 20
 	"github.com/docker/docker/api"
21
+	"github.com/docker/docker/api/types"
21 22
 	"github.com/docker/docker/api/types/container"
22 23
 	"github.com/docker/docker/api/types/strslice"
23 24
 	"github.com/docker/docker/builder"
... ...
@@ -279,12 +280,26 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str
279 279
 		return err
280 280
 	}
281 281
 
282
-	// NOTE: You won't find the "mkdir" for the directory in here. Rather we
283
-	// just set the value in the image's runConfig.WorkingDir property
284
-	// and container.SetupWorkingDirectory() will create it automatically
285
-	// for us the next time the image is used to create a container.
282
+	// For performance reasons, we explicitly do a create/mkdir now
283
+	// This avoids having an unnecessary expensive mount/unmount calls
284
+	// (on Windows in particular) during each container create.
285
+	// Prior to 1.13, the mkdir was deferred and not executed at this step.
286
+	if b.disableCommit {
287
+		// Don't call back into the daemon if we're going through docker commit --change "WORKDIR /foo".
288
+		// We've already updated the runConfig and that's enough.
289
+		return nil
290
+	}
291
+	b.runConfig.Image = b.image
292
+	container, err := b.docker.ContainerCreate(types.ContainerCreateConfig{Config: b.runConfig}, true)
293
+	if err != nil {
294
+		return err
295
+	}
296
+	b.tmpContainers[container.ID] = struct{}{}
297
+	if err := b.docker.ContainerCreateWorkdir(container.ID); err != nil {
298
+		return err
299
+	}
286 300
 
287
-	return b.commit("", b.runConfig.Cmd, fmt.Sprintf("WORKDIR %v", b.runConfig.WorkingDir))
301
+	return b.commit(container.ID, b.runConfig.Cmd, "WORKDIR "+b.runConfig.WorkingDir)
288 302
 }
289 303
 
290 304
 // RUN some command yo
291 305
new file mode 100644
... ...
@@ -0,0 +1,21 @@
0
+package daemon
1
+
2
+// ContainerCreateWorkdir creates the working directory. This is solves the
3
+// issue arising from https://github.com/docker/docker/issues/27545,
4
+// which was initially fixed by https://github.com/docker/docker/pull/27884. But that fix
5
+// was too expensive in terms of performance on Windows. Instead,
6
+// https://github.com/docker/docker/pull/28514 introduces this new functionality
7
+// where the builder calls into the backend here to create the working directory.
8
+func (daemon *Daemon) ContainerCreateWorkdir(cID string) error {
9
+	container, err := daemon.GetContainer(cID)
10
+	if err != nil {
11
+		return err
12
+	}
13
+	err = daemon.Mount(container)
14
+	if err != nil {
15
+		return err
16
+	}
17
+	defer daemon.Unmount(container)
18
+	rootUID, rootGID := daemon.GetRemappedUIDGID()
19
+	return container.SetupWorkingDirectory(rootUID, rootGID)
20
+}
... ...
@@ -1878,8 +1878,8 @@ func (s *DockerSuite) TestBuildWindowsAddCopyPathProcessing(c *check.C) {
1878 1878
 			WORKDIR /wc2
1879 1879
 			ADD wc2 c:/wc2
1880 1880
 			WORKDIR c:/
1881
-			RUN sh -c "[ $(cat c:/wc1) = 'hellowc1' ]"
1882
-			RUN sh -c "[ $(cat c:/wc2) = 'worldwc2' ]"
1881
+			RUN sh -c "[ $(cat c:/wc1/wc1) = 'hellowc1' ]"
1882
+			RUN sh -c "[ $(cat c:/wc2/wc2) = 'worldwc2' ]"
1883 1883
 
1884 1884
 			# Trailing slash on COPY/ADD, Windows-style path.
1885 1885
 			WORKDIR /wd1
... ...
@@ -7283,3 +7283,31 @@ func (s *DockerSuite) TestBuildWindowsUser(c *check.C) {
7283 7283
 	}
7284 7284
 	c.Assert(strings.ToLower(out), checker.Contains, "username=user")
7285 7285
 }
7286
+
7287
+// Verifies if COPY file . when WORKDIR is set to a non-existing directory,
7288
+// the directory is created and the file is copied into the directory,
7289
+// as opposed to the file being copied as a file with the name of the
7290
+// directory. Fix for 27545 (found on Windows, but regression good for Linux too).
7291
+// Note 27545 was reverted in 28505, but a new fix was added subsequently in 28514.
7292
+func (s *DockerSuite) TestBuildCopyFileDotWithWorkdir(c *check.C) {
7293
+	name := "testbuildcopyfiledotwithworkdir"
7294
+	ctx, err := fakeContext(`FROM busybox 
7295
+WORKDIR /foo 
7296
+COPY file . 
7297
+RUN ["cat", "/foo/file"] 
7298
+`,
7299
+		map[string]string{})
7300
+
7301
+	if err != nil {
7302
+		c.Fatal(err)
7303
+	}
7304
+	defer ctx.Close()
7305
+
7306
+	if err := ctx.Add("file", "content"); err != nil {
7307
+		c.Fatal(err)
7308
+	}
7309
+
7310
+	if _, err = buildImageFromContext(name, ctx, true); err != nil {
7311
+		c.Fatal(err)
7312
+	}
7313
+}
... ...
@@ -104,7 +104,6 @@ func (s *DockerSuite) TestCommitWithHostBindMount(c *check.C) {
104 104
 }
105 105
 
106 106
 func (s *DockerSuite) TestCommitChange(c *check.C) {
107
-	testRequires(c, DaemonIsLinux)
108 107
 	dockerCmd(c, "run", "--name", "test", "busybox", "true")
109 108
 
110 109
 	imageID, _ := dockerCmd(c, "commit",
... ...
@@ -122,12 +121,14 @@ func (s *DockerSuite) TestCommitChange(c *check.C) {
122 122
 		"test", "test-commit")
123 123
 	imageID = strings.TrimSpace(imageID)
124 124
 
125
+	prefix, slash := getPrefixAndSlashFromDaemonPlatform()
126
+	prefix = strings.ToUpper(prefix) // Force C: as that's how WORKDIR is normalised on Windows
125 127
 	expected := map[string]string{
126 128
 		"Config.ExposedPorts": "map[8080/tcp:{}]",
127 129
 		"Config.Env":          "[DEBUG=true test=1 PATH=/foo]",
128 130
 		"Config.Labels":       "map[foo:bar]",
129 131
 		"Config.Cmd":          "[/bin/sh]",
130
-		"Config.WorkingDir":   "/opt",
132
+		"Config.WorkingDir":   prefix + slash + "opt",
131 133
 		"Config.Entrypoint":   "[/bin/sh]",
132 134
 		"Config.User":         "testuser",
133 135
 		"Config.Volumes":      "map[/var/lib/docker:{}]",