volumes: return better error messages for unsupported task drivers (#…

…8030)

When an allocation runs for a task driver that can't support volume mounts,
the mounting will fail in a way that can be hard to understand. With host
volumes this usually means failing silently, whereas with CSI the operator
gets inscrutable internals exposed in the `nomad alloc status`.

This changeset adds a MountConfig field to the task driver Capabilities
response. We validate this when the `csi_hook` or `volume_hook` fires and
return a user-friendly error.

Note that we don't currently have a way to get driver capabilities up to the
server, except through attributes. Validating this when the user initially
submits the jobspec would be even better than what we're doing here (and could
be useful for all our other capabilities), but that's out of scope for this
changeset.

Also note that the MountConfig enum starts with "supports all" in order to
support community plugins in a backwards compatible way, rather than cutting
them off from volume mounting unexpectedly.

client/allocrunner/alloc_runner_hooks.go

@@ -168,7 +168,7 @@ func (ar *allocRunner) initRunnerHooks(config *clientconfig.Config) error {
 			logger:         hookLogger,
 		}),
 		newConsulSockHook(hookLogger, alloc, ar.allocDir, config.ConsulConfig),
-		newCSIHook(hookLogger, alloc, ar.rpcClient, ar.csiManager, hrs),
+		newCSIHook(ar, hookLogger, alloc, ar.rpcClient, ar.csiManager, hrs),
 	}
 
 	return nil

client/allocrunner/csi_hook.go

@@ -7,13 +7,15 @@ import (
 	hclog "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/nomad/client/pluginmanager/csimanager"
 	"github.com/hashicorp/nomad/nomad/structs"
+	"github.com/hashicorp/nomad/plugins/drivers"
 )
 
 // csiHook will wait for remote csi volumes to be attached to the host before
 // continuing.
 //
 // It is a noop for allocs that do not depend on CSI Volumes.
 type csiHook struct {
+	ar         *allocRunner
 	alloc      *structs.Allocation
 	logger     hclog.Logger
 	csimanager csimanager.Manager
@@ -88,7 +90,21 @@ func (c *csiHook) claimVolumesFromAlloc() (map[string]*volumeAndRequest, error)
 
 	// Initially, populate the result map with all of the requests
 	for alias, volumeRequest := range tg.Volumes {
+
 		if volumeRequest.Type == structs.VolumeTypeCSI {
+
+			for _, task := range tg.Tasks {
+				caps, err := c.ar.GetTaskDriverCapabilities(task.Name)
+				if err != nil {
+					return nil, fmt.Errorf("could not validate task driver capabilities: %v", err)
+				}
+
+				if caps.MountConfigs == drivers.MountConfigSupportNone {
+					return nil, fmt.Errorf(
+						"task driver %q for %q does not support CSI", task.Driver, task.Name)
+				}
+			}
+
 			result[alias] = &volumeAndRequest{request: volumeRequest}
 		}
 	}
@@ -125,8 +141,9 @@ func (c *csiHook) claimVolumesFromAlloc() (map[string]*volumeAndRequest, error)
 	return result, nil
 }
 
-func newCSIHook(logger hclog.Logger, alloc *structs.Allocation, rpcClient RPCer, csi csimanager.Manager, updater hookResourceSetter) *csiHook {
+func newCSIHook(ar *allocRunner, logger hclog.Logger, alloc *structs.Allocation, rpcClient RPCer, csi csimanager.Manager, updater hookResourceSetter) *csiHook {
 	return &csiHook{
+		ar:         ar,
 		alloc:      alloc,
 		logger:     logger.Named("csi_hook"),
 		rpcClient:  rpcClient,

client/allocrunner/taskrunner/volume_hook.go

@@ -121,6 +121,18 @@ func (h *volumeHook) prepareHostVolumes(req *interfaces.TaskPrestartRequest, vol
 		return nil, err
 	}
 
+	if len(hostVolumeMounts) > 0 {
+		caps, err := h.runner.DriverCapabilities()
+		if err != nil {
+			return nil, fmt.Errorf("could not validate task driver capabilities: %v", err)
+		}
+		if caps.MountConfigs == drivers.MountConfigSupportNone {
+			return nil, fmt.Errorf(
+				"task driver %q for %q does not support host volumes",
+				h.runner.task.Driver, h.runner.task.Name)
+		}
+	}
+
 	return hostVolumeMounts, nil
 }
 
@@ -167,6 +179,18 @@ func (h *volumeHook) prepareCSIVolumes(req *interfaces.TaskPrestartRequest, volu
 		}
 	}
 
+	if len(mounts) > 0 {
+		caps, err := h.runner.DriverCapabilities()
+		if err != nil {
+			return nil, fmt.Errorf("could not validate task driver capabilities: %v", err)
+		}
+		if caps.MountConfigs == drivers.MountConfigSupportNone {
+			return nil, fmt.Errorf(
+				"task driver %q for %q does not support CSI",
+				h.runner.task.Driver, h.runner.task.Name)
+		}
+	}
+
 	return mounts, nil
 }
 

client/allocrunner/taskrunner/volume_hook_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/plugins/drivers"
+	dtu "github.com/hashicorp/nomad/plugins/drivers/testutils"
 	"github.com/stretchr/testify/require"
 )
 
@@ -67,8 +68,11 @@ func TestVolumeHook_PartitionMountsByVolume_Works(t *testing.T) {
 }
 
 func TestVolumeHook_prepareCSIVolumes(t *testing.T) {
+
 	req := &interfaces.TaskPrestartRequest{
 		Task: &structs.Task{
+			Name:   "test",
+			Driver: "mock",
 			VolumeMounts: []*structs.VolumeMount{
 				{
 					Volume:      "foo",
@@ -85,31 +89,71 @@ func TestVolumeHook_prepareCSIVolumes(t *testing.T) {
 		},
 	}
 
-	tr := &TaskRunner{
-		allocHookResources: &cstructs.AllocHookResources{
-			CSIMounts: map[string]*csimanager.MountInfo{
-				"foo": {
-					Source: "/mnt/my-test-volume",
+	cases := []struct {
+		Name          string
+		Driver        drivers.DriverPlugin
+		Expected      []*drivers.MountConfig
+		ExpectedError string
+	}{
+		{
+			Name: "supported driver",
+			Driver: &dtu.MockDriver{
+				CapabilitiesF: func() (*drivers.Capabilities, error) {
+					return &drivers.Capabilities{
+						MountConfigs: drivers.MountConfigSupportAll,
+					}, nil
+				},
+			},
+			Expected: []*drivers.MountConfig{
+				{
+					HostPath: "/mnt/my-test-volume",
+					TaskPath: "/bar",
 				},
 			},
 		},
-	}
-
-	expected := []*drivers.MountConfig{
 		{
-			HostPath: "/mnt/my-test-volume",
-			TaskPath: "/bar",
+			Name: "unsupported driver",
+			Driver: &dtu.MockDriver{
+				CapabilitiesF: func() (*drivers.Capabilities, error) {
+					return &drivers.Capabilities{
+						MountConfigs: drivers.MountConfigSupportNone,
+					}, nil
+				},
+			},
+			ExpectedError: "task driver \"mock\" for \"test\" does not support CSI",
 		},
 	}
 
-	hook := &volumeHook{
-		logger: testlog.HCLogger(t),
-		alloc:  structs.MockAlloc(),
-		runner: tr,
+	for _, tc := range cases {
+		t.Run(tc.Name, func(t *testing.T) {
+
+			tr := &TaskRunner{
+				task:   req.Task,
+				driver: tc.Driver,
+				allocHookResources: &cstructs.AllocHookResources{
+					CSIMounts: map[string]*csimanager.MountInfo{
+						"foo": {
+							Source: "/mnt/my-test-volume",
+						},
+					},
+				},
+			}
+
+			hook := &volumeHook{
+				logger: testlog.HCLogger(t),
+				alloc:  structs.MockAlloc(),
+				runner: tr,
+			}
+			mounts, err := hook.prepareCSIVolumes(req, volumes)
+
+			if tc.ExpectedError != "" {
+				require.EqualError(t, err, tc.ExpectedError)
+			} else {
+				require.NoError(t, err)
+			}
+			require.Equal(t, tc.Expected, mounts)
+		})
 	}
-	mounts, err := hook.prepareCSIVolumes(req, volumes)
-	require.NoError(t, err)
-	require.Equal(t, expected, mounts)
 }
 
 func TestVolumeHook_Interpolation(t *testing.T) {

drivers/docker/config.go

@@ -376,6 +376,7 @@ var (
 			drivers.NetIsolationModeTask,
 		},
 		MustInitiateNetwork: true,
+		MountConfigs:        drivers.MountConfigSupportAll,
 	}
 )
 

drivers/exec/driver.go

@@ -83,6 +83,7 @@ var (
 			drivers.NetIsolationModeHost,
 			drivers.NetIsolationModeGroup,
 		},
+		MountConfigs: drivers.MountConfigSupportAll,
 	}
 )
 

drivers/java/driver.go

@@ -87,6 +87,7 @@ var (
 			drivers.NetIsolationModeHost,
 			drivers.NetIsolationModeGroup,
 		},
+		MountConfigs: drivers.MountConfigSupportNone,
 	}
 
 	_ drivers.DriverPlugin = (*Driver)(nil)
@@ -95,6 +96,7 @@ var (
 func init() {
 	if runtime.GOOS == "linux" {
 		capabilities.FSIsolation = drivers.FSIsolationChroot
+		capabilities.MountConfigs = drivers.MountConfigSupportAll
 	}
 }
 

drivers/mock/driver.go

@@ -154,9 +154,10 @@ func NewMockDriver(logger hclog.Logger) drivers.DriverPlugin {
 	logger = logger.Named(pluginName)
 
 	capabilities := &drivers.Capabilities{
-		SendSignals: true,
-		Exec:        true,
-		FSIsolation: drivers.FSIsolationNone,
+		SendSignals:  true,
+		Exec:         true,
+		FSIsolation:  drivers.FSIsolationNone,
+		MountConfigs: drivers.MountConfigSupportNone,
 	}
 
 	return &Driver{

drivers/qemu/driver.go

@@ -98,9 +98,10 @@ var (
 	// capabilities is returned by the Capabilities RPC and indicates what
 	// optional features this driver supports
 	capabilities = &drivers.Capabilities{
-		SendSignals: false,
-		Exec:        false,
-		FSIsolation: drivers.FSIsolationImage,
+		SendSignals:  false,
+		Exec:         false,
+		FSIsolation:  drivers.FSIsolationImage,
+		MountConfigs: drivers.MountConfigSupportNone,
 	}
 
 	_ drivers.DriverPlugin = (*Driver)(nil)

drivers/rawexec/driver.go

@@ -101,6 +101,7 @@ var (
 			drivers.NetIsolationModeHost,
 			drivers.NetIsolationModeGroup,
 		},
+		MountConfigs: drivers.MountConfigSupportNone,
 	}
 )
 

plugins/drivers/client.go

@@ -71,6 +71,8 @@ func (d *driverPluginClient) Capabilities() (*Capabilities, error) {
 		default:
 			caps.FSIsolation = FSIsolationNone
 		}
+
+		caps.MountConfigs = MountConfigSupport(resp.Capabilities.MountConfigs)
 	}
 
 	return caps, nil

plugins/drivers/driver.go

@@ -163,6 +163,9 @@ type Capabilities struct {
 	// MustInitiateNetwork tells Nomad that the driver must create the network
 	// namespace and that the CreateNetwork and DestroyNetwork RPCs are implemented.
 	MustInitiateNetwork bool
+
+	// MountConfigs tells Nomad which mounting config options the driver supports.
+	MountConfigs MountConfigSupport
 }
 
 func (c *Capabilities) HasNetIsolationMode(m NetIsolationMode) bool {
@@ -197,6 +200,15 @@ type NetworkIsolationSpec struct {
 	Labels map[string]string
 }
 
+// MountConfigSupport is an enum that defaults to "all" for backwards
+// compatibility with community drivers.
+type MountConfigSupport int32
+
+const (
+	MountConfigSupportAll MountConfigSupport = iota
+	MountConfigSupportNone
+)
+
 type TerminalSize struct {
 	Height int
 	Width  int