Browse code

cluster/executor: check mounts at start

While it is important to not create controllers for an invalid task,
certain properties should only be checked immediately before use. Early
host validation of mounts prevents resolution of the task Executor when
the mounts are not relevant to execution flow. In this case, we have a
check for the existence of a bind mount path in a creation function that
prevents a task controller from being resolved. Such early validation
prevents one from interacting directly with a controller and result in
unnecessary error reporting.

In accordance with the above, we move the validation of the existence of
host bind mount paths to the `Controller.Start` phase. We also call
these "checks", as they are valid mounts but reference non-existent
paths.

Signed-off-by: Stephen J Day <stephen.day@docker.com>

Stephen J Day authored on 2017/02/04 11:05:20
Showing 5 changed files
... ...
@@ -6,6 +6,7 @@ import (
6 6
 	"errors"
7 7
 	"fmt"
8 8
 	"io"
9
+	"os"
9 10
 	"strings"
10 11
 	"syscall"
11 12
 	"time"
... ...
@@ -259,7 +260,28 @@ func (c *containerAdapter) create(ctx context.Context) error {
259 259
 	return nil
260 260
 }
261 261
 
262
+// checkMounts ensures that the provided mounts won't have any host-specific
263
+// problems at start up. For example, we disallow bind mounts without an
264
+// existing path, which slightly different from the container API.
265
+func (c *containerAdapter) checkMounts() error {
266
+	spec := c.container.spec()
267
+	for _, mount := range spec.Mounts {
268
+		switch mount.Type {
269
+		case api.MountTypeBind:
270
+			if _, err := os.Stat(mount.Source); os.IsNotExist(err) {
271
+				return fmt.Errorf("invalid bind mount source, source path not found: %s", mount.Source)
272
+			}
273
+		}
274
+	}
275
+
276
+	return nil
277
+}
278
+
262 279
 func (c *containerAdapter) start(ctx context.Context) error {
280
+	if err := c.checkMounts(); err != nil {
281
+		return err
282
+	}
283
+
263 284
 	return c.backend.ContainerStart(c.container.name(), nil, "", "")
264 285
 }
265 286
 
... ...
@@ -3,7 +3,6 @@ package container
3 3
 import (
4 4
 	"errors"
5 5
 	"fmt"
6
-	"os"
7 6
 	"path/filepath"
8 7
 
9 8
 	"github.com/docker/swarmkit/api"
... ...
@@ -25,9 +24,6 @@ func validateMounts(mounts []api.Mount) error {
25 25
 			if !filepath.IsAbs(mount.Source) {
26 26
 				return fmt.Errorf("invalid bind mount source, must be an absolute path: %s", mount.Source)
27 27
 			}
28
-			if _, err := os.Stat(mount.Source); os.IsNotExist(err) {
29
-				return fmt.Errorf("invalid bind mount source, source path not found: %s", mount.Source)
30
-			}
31 28
 		case api.MountTypeVolume:
32 29
 			if filepath.IsAbs(mount.Source) {
33 30
 				return fmt.Errorf("invalid volume mount source, must not be an absolute path: %s", mount.Source)
... ...
@@ -42,10 +42,10 @@ func TestControllerValidateMountBind(t *testing.T) {
42 42
 	// with non-existing source
43 43
 	if _, err := newTestControllerWithMount(api.Mount{
44 44
 		Type:   api.MountTypeBind,
45
-		Source: "/some-non-existing-host-path/",
45
+		Source: testAbsNonExistent,
46 46
 		Target: testAbsPath,
47
-	}); err == nil || !strings.Contains(err.Error(), "invalid bind mount source") {
48
-		t.Fatalf("expected  error, got: %v", err)
47
+	}); err != nil {
48
+		t.Fatalf("controller should not error at creation: %v", err)
49 49
 	}
50 50
 
51 51
 	// with proper source
... ...
@@ -3,5 +3,6 @@
3 3
 package container
4 4
 
5 5
 const (
6
-	testAbsPath = "/foo"
6
+	testAbsPath        = "/foo"
7
+	testAbsNonExistent = "/some-non-existing-host-path/"
7 8
 )
... ...
@@ -1,5 +1,8 @@
1
+// +build windows
2
+
1 3
 package container
2 4
 
3 5
 const (
4
-	testAbsPath = `c:\foo`
6
+	testAbsPath        = `c:\foo`
7
+	testAbsNonExistent = `c:\some-non-existing-host-path\`
5 8
 )