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>
(cherry picked from commit 92899ffac8ca1136e807dd234e8fa1dd49db7801)
Signed-off-by: Victor Vieux <victorvieux@gmail.com>

daemon/cluster/executor/container/adapter.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"os"
 	"strings"
 	"syscall"
 	"time"
@@ -258,7 +259,28 @@ func (c *containerAdapter) create(ctx context.Context) error {
 	return nil
 }
 
+// checkMounts ensures that the provided mounts won't have any host-specific
+// problems at start up. For example, we disallow bind mounts without an
+// existing path, which slightly different from the container API.
+func (c *containerAdapter) checkMounts() error {
+	spec := c.container.spec()
+	for _, mount := range spec.Mounts {
+		switch mount.Type {
+		case api.MountTypeBind:
+			if _, err := os.Stat(mount.Source); os.IsNotExist(err) {
+				return fmt.Errorf("invalid bind mount source, source path not found: %s", mount.Source)
+			}
+		}
+	}
+
+	return nil
+}
+
 func (c *containerAdapter) start(ctx context.Context) error {
+	if err := c.checkMounts(); err != nil {
+		return err
+	}
+
 	return c.backend.ContainerStart(c.container.name(), nil, "", "")
 }
 

daemon/cluster/executor/container/validate.go

@@ -2,7 +2,6 @@ package container
 
 import (
 	"fmt"
-	"os"
 	"path/filepath"
 
 	"github.com/docker/swarmkit/api"
@@ -24,9 +23,6 @@ func validateMounts(mounts []api.Mount) error {
 			if !filepath.IsAbs(mount.Source) {
 				return fmt.Errorf("invalid bind mount source, must be an absolute path: %s", mount.Source)
 			}
-			if _, err := os.Stat(mount.Source); os.IsNotExist(err) {
-				return fmt.Errorf("invalid bind mount source, source path not found: %s", mount.Source)
-			}
 		case api.MountTypeVolume:
 			if filepath.IsAbs(mount.Source) {
 				return fmt.Errorf("invalid volume mount source, must not be an absolute path: %s", mount.Source)

daemon/cluster/executor/container/validate_test.go

@@ -42,10 +42,10 @@ func TestControllerValidateMountBind(t *testing.T) {
 	// with non-existing source
 	if _, err := newTestControllerWithMount(api.Mount{
 		Type:   api.MountTypeBind,
-		Source: "/some-non-existing-host-path/",
+		Source: testAbsNonExistent,
 		Target: testAbsPath,
-	}); err == nil || !strings.Contains(err.Error(), "invalid bind mount source") {
-		t.Fatalf("expected  error, got: %v", err)
+	}); err != nil {
+		t.Fatalf("controller should not error at creation: %v", err)
 	}
 
 	// with proper source

daemon/cluster/executor/container/validate_unix_test.go

@@ -3,5 +3,6 @@
 package container
 
 const (
-	testAbsPath = "/foo"
+	testAbsPath        = "/foo"
+	testAbsNonExistent = "/some-non-existing-host-path/"
 )

daemon/cluster/executor/container/validate_windows_test.go

@@ -1,5 +1,8 @@
+// +build windows
+
 package container
 
 const (
-	testAbsPath = `c:\foo`
+	testAbsPath        = `c:\foo`
+	testAbsNonExistent = `c:\some-non-existing-host-path\`
 )