Merge pull request #30732 from stevvooe/validate-mounts-at-start

cluster/executor: check mounts at start
Upstream-commit: 41864ca0ecc5c515f870cae862c286bb38cc1bf6
Component: engine

components/engine/daemon/cluster/executor/container/adapter.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"os"
 	"strings"
 	"syscall"
 	"time"
@@ -259,7 +260,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, "", "")
 }
 

components/engine/daemon/cluster/executor/container/validate.go

@@ -3,7 +3,6 @@ package container
 import (
 	"errors"
 	"fmt"
-	"os"
 	"path/filepath"
 
 	"github.com/docker/swarmkit/api"
@@ -25,9 +24,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)

components/engine/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

components/engine/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/"
 )

components/engine/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\`
 )