CSI: create volume capability validation

api/csi.go

@@ -143,6 +143,14 @@ type CSIVolume struct {
 	Secrets        CSISecrets              `hcl:"secrets"`
 	Parameters     map[string]string       `hcl:"parameters"`
 	Context        map[string]string       `hcl:"context"`
+	Capacity       int64                   `hcl:"-"`
+
+	// These fields are used as part of the volume creation request
+	RequestedCapacityMin  int64                  `hcl:"capacity_min"`
+	RequestedCapacityMax  int64                  `hcl:"capacity_max"`
+	RequestedCapabilities []*CSIVolumeCapability `hcl:"capability"`
+	CloneID               string                 `hcl:"clone_id"`
+	SnapshotID            string                 `hcl:"snapshot_id"`
 
 	// ReadAllocs is a map of allocation IDs for tracking reader claim status.
 	// The Allocation value will always be nil; clients can populate this data
@@ -176,6 +184,13 @@ type CSIVolume struct {
 	ExtraKeysHCL []string `hcl1:",unusedKeys" json:"-"`
 }
 
+// CSIVolumeCapability is a requested attachment and access mode for a
+// volume
+type CSIVolumeCapability struct {
+	AccessMode     CSIVolumeAccessMode     `hcl:"access_mode"`
+	AttachmentMode CSIVolumeAttachmentMode `hcl:"attachment_mode"`
+}
+
 type CSIVolumeIndexSort []*CSIVolumeListStub
 
 func (v CSIVolumeIndexSort) Len() int {

client/pluginmanager/csimanager/fingerprint.go

@@ -122,10 +122,18 @@ func (p *pluginFingerprinter) buildBasicFingerprint(ctx context.Context) (*struc
 }
 
 func applyCapabilitySetToControllerInfo(cs *csi.ControllerCapabilitySet, info *structs.CSIControllerInfo) {
-	info.SupportsReadOnlyAttach = cs.HasPublishReadonly
+	info.SupportsCreateDelete = cs.HasCreateDeleteVolume
 	info.SupportsAttachDetach = cs.HasPublishUnpublishVolume
 	info.SupportsListVolumes = cs.HasListVolumes
+	info.SupportsGetCapacity = cs.HasGetCapacity
+	info.SupportsCreateDeleteSnapshot = cs.HasCreateDeleteSnapshot
+	info.SupportsListSnapshots = cs.HasListSnapshots
+	info.SupportsClone = cs.HasCloneVolume
+	info.SupportsReadOnlyAttach = cs.HasPublishReadonly
+	info.SupportsExpand = cs.HasExpandVolume
 	info.SupportsListVolumesAttachedNodes = cs.HasListVolumesPublishedNodes
+	info.SupportsCondition = cs.HasVolumeCondition
+	info.SupportsGet = cs.HasGetVolume
 }
 
 func (p *pluginFingerprinter) buildControllerFingerprint(ctx context.Context, base *structs.CSIInfo) (*structs.CSIInfo, error) {

command/agent/csi_endpoint_test.go

@@ -58,7 +58,7 @@ func TestHTTP_CSIEndpointUtils(t *testing.T) {
 	require.Equal(t, "bar", tops[0].Segments["foo"])
 }
 
-func TestHTTP_CSIEndpointVolume(t *testing.T) {
+func TestHTTP_CSIEndpointRegisterVolume(t *testing.T) {
 	t.Parallel()
 	httpTest(t, nil, func(s *TestAgent) {
 		server := s.Agent.Server()
@@ -95,28 +95,36 @@ func TestHTTP_CSIEndpointVolume(t *testing.T) {
 		resp = httptest.NewRecorder()
 		_, err = s.Server.CSIVolumeSpecificRequest(resp, req)
 		require.Equal(t, CodedError(400, "detach requires node ID"), err)
+	})
+}
 
-		cArgs := structs.CSIVolumeCreateRequest{
+func TestHTTP_CSIEndpointCreateVolume(t *testing.T) {
+	t.Parallel()
+	httpTest(t, nil, func(s *TestAgent) {
+		server := s.Agent.Server()
+		cleanup := state.CreateTestCSIPlugin(server.State(), "foo")
+		defer cleanup()
+
+		args := structs.CSIVolumeCreateRequest{
 			Volumes: []*structs.CSIVolume{{
 				ID:             "baz",
 				PluginID:       "foo",
 				AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
 				AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
 			}},
 		}
-		body = encodeReq(cArgs)
-		req, err = http.NewRequest("PUT", "/v1/volumes/create", body)
+		body := encodeReq(args)
+		req, err := http.NewRequest("PUT", "/v1/volumes/create", body)
 		require.NoError(t, err)
-		resp = httptest.NewRecorder()
+		resp := httptest.NewRecorder()
 		_, err = s.Server.CSIVolumesRequest(resp, req)
-		require.NoError(t, err, "put error")
+		require.Error(t, err, "controller validate volume: No path to node")
 
 		req, err = http.NewRequest("DELETE", "/v1/volume/csi/baz", nil)
 		require.NoError(t, err)
 		resp = httptest.NewRecorder()
 		_, err = s.Server.CSIVolumeSpecificRequest(resp, req)
-		require.NoError(t, err, "delete error")
-
+		require.Error(t, err, "volume not found: baz")
 	})
 }
 

nomad/csi_endpoint.go

@@ -441,9 +441,10 @@ func (v *CSIVolume) controllerPublishVolume(req *structs.CSIVolumeClaimRequest,
 		return fmt.Errorf("%s: %s", structs.ErrUnknownAllocationPrefix, req.AllocationID)
 	}
 
-	// if no plugin was returned then controller validation is not required.
-	// Here we can return nil.
-	if plug == nil {
+	// Some plugins support controllers for create/snapshot but not attach. So
+	// if there's no plugin or the plugin doesn't attach volumes, then we can
+	// skip the controller publish workflow and return nil.
+	if plug == nil || !plug.HasControllerCapability(structs.CSIControllerSupportsAttachDetach) {
 		return nil
 	}
 
@@ -679,11 +680,23 @@ func (v *CSIVolume) controllerUnpublishVolume(vol *structs.CSIVolume, claim *str
 		return nil
 	}
 
+	state := v.srv.fsm.State()
+	ws := memdb.NewWatchSet()
+
+	plugin, err := state.CSIPluginByID(ws, vol.PluginID)
+	if err != nil {
+		return fmt.Errorf("could not query plugin: %v", err)
+	} else if plugin == nil {
+		return fmt.Errorf("no such plugin: %q", vol.PluginID)
+	}
+	if !plugin.HasControllerCapability(structs.CSIControllerSupportsAttachDetach) {
+		return nil
+	}
+
 	// we only send a controller detach if a Nomad client no longer has
 	// any claim to the volume, so we need to check the status of claimed
 	// allocations
-	state := v.srv.fsm.State()
-	vol, err := state.CSIVolumeDenormalize(memdb.NewWatchSet(), vol)
+	vol, err = state.CSIVolumeDenormalize(ws, vol)
 	if err != nil {
 		return err
 	}
@@ -834,9 +847,8 @@ func (v *CSIVolume) Create(args *structs.CSIVolumeCreateRequest, reply *structs.
 		if !plugin.ControllerRequired {
 			return fmt.Errorf("plugin has no controller")
 		}
-
-		if err := v.controllerValidateVolume(regArgs, vol, plugin); err != nil {
-			return err
+		if !plugin.HasControllerCapability(structs.CSIControllerSupportsCreateDelete) {
+			return fmt.Errorf("plugin does not support creating volumes")
 		}
 
 		validatedVols = append(validatedVols, validated{vol, plugin})

nomad/csi_endpoint_test.go

@@ -162,11 +162,6 @@ func TestCSIVolumeEndpoint_Register(t *testing.T) {
 	}
 	resp1 := &structs.CSIVolumeRegisterResponse{}
 	err := msgpackrpc.CallWithCodec(codec, "CSIVolume.Register", req1, resp1)
-	require.Error(t, err, "expected validation error")
-
-	// Fix the registration so that it passes validation
-	vols[0].AttachmentMode = structs.CSIVolumeAttachmentModeFilesystem
-	err = msgpackrpc.CallWithCodec(codec, "CSIVolume.Register", req1, resp1)
 	require.NoError(t, err)
 	require.NotEqual(t, uint64(0), resp1.Index)
 
@@ -713,6 +708,7 @@ func TestCSIVolumeEndpoint_Create(t *testing.T) {
 			Healthy:  true,
 			ControllerInfo: &structs.CSIControllerInfo{
 				SupportsAttachDetach: true,
+				SupportsCreateDelete: true,
 			},
 			RequiresControllerPlugin: true,
 		},

nomad/state/testing.go

@@ -81,6 +81,8 @@ func createTestCSIPlugin(s *StateStore, id string, requiresController bool) func
 				SupportsAttachDetach:             true,
 				SupportsListVolumes:              true,
 				SupportsListVolumesAttachedNodes: false,
+				SupportsCreateDeleteSnapshot:     true,
+				SupportsListSnapshots:            true,
 			},
 		},
 	}

nomad/structs/csi.go

@@ -590,22 +590,6 @@ func (v *CSIVolume) Validate() error {
 	if v.Namespace == "" {
 		errs = append(errs, "missing namespace")
 	}
-	if v.AccessMode == "" {
-		errs = append(errs, "missing access mode")
-	}
-	if v.AttachmentMode == "" {
-		errs = append(errs, "missing attachment mode")
-	}
-	if v.AttachmentMode == CSIVolumeAttachmentModeBlockDevice {
-		if v.MountOptions != nil {
-			if v.MountOptions.FSType != "" {
-				errs = append(errs, "mount options not allowed for block-device")
-			}
-			if v.MountOptions.MountFlags != nil && len(v.MountOptions.MountFlags) != 0 {
-				errs = append(errs, "mount options not allowed for block-device")
-			}
-		}
-	}
 	if v.SnapshotID != "" && v.CloneID != "" {
 		errs = append(errs, "only one of snapshot_id and clone_id is allowed")
 	}
@@ -862,14 +846,102 @@ func (p *CSIPlugin) Copy() *CSIPlugin {
 	return out
 }
 
+type CSIControllerCapability byte
+
+const (
+	// CSIControllerSupportsCreateDelete indicates plugin support for
+	// CREATE_DELETE_VOLUME
+	CSIControllerSupportsCreateDelete CSIControllerCapability = 0
+
+	// CSIControllerSupportsAttachDetach is true when the controller
+	// implements the methods required to attach and detach volumes. If this
+	// is false Nomad should skip the controller attachment flow.
+	CSIControllerSupportsAttachDetach CSIControllerCapability = 1
+
+	// CSIControllerSupportsListVolumes is true when the controller implements
+	// the ListVolumes RPC. NOTE: This does not guarantee that attached nodes
+	// will be returned unless SupportsListVolumesAttachedNodes is also true.
+	CSIControllerSupportsListVolumes CSIControllerCapability = 2
+
+	// CSIControllerSupportsGetCapacity indicates plugin support for
+	// GET_CAPACITY
+	CSIControllerSupportsGetCapacity CSIControllerCapability = 3
+
+	// CSIControllerSupportsCreateDeleteSnapshot indicates plugin support for
+	// CREATE_DELETE_SNAPSHOT
+	CSIControllerSupportsCreateDeleteSnapshot CSIControllerCapability = 4
+
+	// CSIControllerSupportsListSnapshots indicates plugin support for
+	// LIST_SNAPSHOTS
+	CSIControllerSupportsListSnapshots CSIControllerCapability = 5
+
+	// CSIControllerSupportsClone indicates plugin support for CLONE_VOLUME
+	CSIControllerSupportsClone CSIControllerCapability = 6
+
+	// CSIControllerSupportsReadOnlyAttach is set to true when the controller
+	// returns the ATTACH_READONLY capability.
+	CSIControllerSupportsReadOnlyAttach CSIControllerCapability = 7
+
+	// CSIControllerSupportsExpand indicates plugin support for EXPAND_VOLUME
+	CSIControllerSupportsExpand CSIControllerCapability = 8
+
+	// CSIControllerSupportsListVolumesAttachedNodes indicates whether the
+	// plugin will return attached nodes data when making ListVolume RPCs
+	// (plugin support for LIST_VOLUMES_PUBLISHED_NODES)
+	CSIControllerSupportsListVolumesAttachedNodes CSIControllerCapability = 9
+
+	// CSIControllerSupportsCondition indicates plugin support for
+	// VOLUME_CONDITION
+	CSIControllerSupportsCondition CSIControllerCapability = 10
+
+	// CSIControllerSupportsGet indicates plugin support for GET_VOLUME
+	CSIControllerSupportsGet CSIControllerCapability = 11
+)
+
+func (p *CSIPlugin) HasControllerCapability(cap CSIControllerCapability) bool {
+	if len(p.Controllers) < 1 {
+		return false
+	}
+	// we're picking the first controller because they should be uniform
+	// across the same version of the plugin
+	for _, c := range p.Controllers {
+		switch cap {
+		case CSIControllerSupportsCreateDelete:
+			return c.ControllerInfo.SupportsCreateDelete
+		case CSIControllerSupportsAttachDetach:
+			return c.ControllerInfo.SupportsAttachDetach
+		case CSIControllerSupportsListVolumes:
+			return c.ControllerInfo.SupportsListVolumes
+		case CSIControllerSupportsGetCapacity:
+			return c.ControllerInfo.SupportsGetCapacity
+		case CSIControllerSupportsCreateDeleteSnapshot:
+			return c.ControllerInfo.SupportsCreateDeleteSnapshot
+		case CSIControllerSupportsListSnapshots:
+			return c.ControllerInfo.SupportsListSnapshots
+		case CSIControllerSupportsClone:
+			return c.ControllerInfo.SupportsClone
+		case CSIControllerSupportsReadOnlyAttach:
+			return c.ControllerInfo.SupportsReadOnlyAttach
+		case CSIControllerSupportsExpand:
+			return c.ControllerInfo.SupportsExpand
+		case CSIControllerSupportsListVolumesAttachedNodes:
+			return c.ControllerInfo.SupportsListVolumesAttachedNodes
+		case CSIControllerSupportsCondition:
+			return c.ControllerInfo.SupportsCondition
+		case CSIControllerSupportsGet:
+			return c.ControllerInfo.SupportsGet
+		default:
+			return false
+		}
+	}
+	return false
+}
+
 // AddPlugin adds a single plugin running on the node. Called from state.NodeUpdate in a
 // transaction
 func (p *CSIPlugin) AddPlugin(nodeID string, info *CSIInfo) error {
 	if info.ControllerInfo != nil {
-		p.ControllerRequired = info.RequiresControllerPlugin &&
-			(info.ControllerInfo.SupportsAttachDetach ||
-				info.ControllerInfo.SupportsReadOnlyAttach)
-
+		p.ControllerRequired = info.RequiresControllerPlugin
 		prev, ok := p.Controllers[nodeID]
 		if ok {
 			if prev == nil {

nomad/structs/node.go

@@ -112,23 +112,50 @@ func (n *CSINodeInfo) Copy() *CSINodeInfo {
 // CSIControllerInfo is the fingerprinted data from a CSI Plugin that is specific to
 // the Controller API.
 type CSIControllerInfo struct {
-	// SupportsReadOnlyAttach is set to true when the controller returns the
-	// ATTACH_READONLY capability.
-	SupportsReadOnlyAttach bool
 
-	// SupportsPublishVolume is true when the controller implements the methods
-	// required to attach and detach volumes. If this is false Nomad should skip
-	// the controller attachment flow.
+	// SupportsCreateDelete indicates plugin support for CREATE_DELETE_VOLUME
+	SupportsCreateDelete bool
+
+	// SupportsPublishVolume is true when the controller implements the
+	// methods required to attach and detach volumes. If this is false Nomad
+	// should skip the controller attachment flow.
 	SupportsAttachDetach bool
 
-	// SupportsListVolumes is true when the controller implements the ListVolumes
-	// RPC. NOTE: This does not guaruntee that attached nodes will be returned
-	// unless SupportsListVolumesAttachedNodes is also true.
+	// SupportsListVolumes is true when the controller implements the
+	// ListVolumes RPC. NOTE: This does not guarantee that attached nodes will
+	// be returned unless SupportsListVolumesAttachedNodes is also true.
 	SupportsListVolumes bool
 
-	// SupportsListVolumesAttachedNodes indicates whether the plugin will return
-	// attached nodes data when making ListVolume RPCs
+	// SupportsGetCapacity indicates plugin support for GET_CAPACITY
+	SupportsGetCapacity bool
+
+	// SupportsCreateDeleteSnapshot indicates plugin support for
+	// CREATE_DELETE_SNAPSHOT
+	SupportsCreateDeleteSnapshot bool
+
+	// SupportsListSnapshots indicates plugin support for LIST_SNAPSHOTS
+	SupportsListSnapshots bool
+
+	// SupportsClone indicates plugin support for CLONE_VOLUME
+	SupportsClone bool
+
+	// SupportsReadOnlyAttach is set to true when the controller returns the
+	// ATTACH_READONLY capability.
+	SupportsReadOnlyAttach bool
+
+	// SupportsExpand indicates plugin support for EXPAND_VOLUME
+	SupportsExpand bool
+
+	// SupportsListVolumesAttachedNodes indicates whether the plugin will
+	// return attached nodes data when making ListVolume RPCs (plugin support
+	// for LIST_VOLUMES_PUBLISHED_NODES)
 	SupportsListVolumesAttachedNodes bool
+
+	// SupportsCondition indicates plugin support for VOLUME_CONDITION
+	SupportsCondition bool
+
+	// SupportsGet indicates plugin support for GET_VOLUME
+	SupportsGet bool
 }
 
 func (c *CSIControllerInfo) Copy() *CSIControllerInfo {

plugins/csi/client_test.go

@@ -222,7 +222,7 @@ func TestClient_RPC_ControllerGetCapabilities(t *testing.T) {
 					{
 						Type: &csipbv1.ControllerServiceCapability_Rpc{
 							Rpc: &csipbv1.ControllerServiceCapability_RPC{
-								Type: csipbv1.ControllerServiceCapability_RPC_GET_CAPACITY,
+								Type: csipbv1.ControllerServiceCapability_RPC_UNKNOWN,
 							},
 						},
 					},