CSI: capability block is required for volume registration

hashicorp · Apr 8, 2021 · 45f0a3a · 45f0a3a
1 parent f133a89
commit 45f0a3a
Show file tree

Hide file tree

Showing 8 changed files with 136 additions and 63 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,9 +1,13 @@
 ## 1.1.0 (Unreleased)
 
 FEATURES:
- * **Consul Namespaces (Enterprise)**: Adds support for Consul Namespaces [[GH-10235](https://github.com/hashicorp/nomad/pull/10235)]
+ * **Consul Namespaces (Enterprise)**: Added support for Consul Namespaces [[GH-10235](https://github.com/hashicorp/nomad/pull/10235)]
+ * **CSI Volume Create and Snapshot**: Added support for creating, deleting, listing, and snapshotting volumes managed by Container Storage Interface (CSI) plugins. [[GH-8212](https://github.com/hashicorp/nomad/issues/8212)]
  * **Licensing (Enterprise)**: Support loading Enterprise license from disk or environment. [[GH-10216](https://github.com/hashicorp/nomad/issues/10216)]
- * **Readiness Checks**: Adds `service` and `check` `on_update` configuration to support liveness and readiness checks. [[GH-9955](https://github.com/hashicorp/nomad/issues/9955)]
+ * **Readiness Checks**: Added `service` and `check` `on_update` configuration to support liveness and readiness checks. [[GH-9955](https://github.com/hashicorp/nomad/issues/9955)]
+
+__BACKWARDS INCOMPATIBILITIES:__
+ * csi: The `attachment_mode` and `access_mode` field are required for `volume` blocks in job specifications. Registering a volume requires at least one `capability` block with the `attachment_mode` and `access_mode` fields set. [[GH-10330](https://github.com/hashicorp/nomad/issues/10330)]
 
 IMPROVEMENTS:
  * api: Removed unimplemented `CSIVolumes.PluginList` API. [[GH-10158](https://github.com/hashicorp/nomad/issues/10158)]

diff --git a/command/agent/csi_endpoint_test.go b/command/agent/csi_endpoint_test.go
@@ -67,10 +67,12 @@ func TestHTTP_CSIEndpointRegisterVolume(t *testing.T) {
 
 		args := structs.CSIVolumeRegisterRequest{
 			Volumes: []*structs.CSIVolume{{
-				ID:             "bar",
-				PluginID:       "foo",
-				AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
-				AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+				ID:       "bar",
+				PluginID: "foo",
+				RequestedCapabilities: []*structs.CSIVolumeCapability{{
+					AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
+					AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+				}},
 			}},
 		}
 		body := encodeReq(args)
@@ -107,10 +109,12 @@ func TestHTTP_CSIEndpointCreateVolume(t *testing.T) {
 
 		args := structs.CSIVolumeCreateRequest{
 			Volumes: []*structs.CSIVolume{{
-				ID:             "baz",
-				PluginID:       "foo",
-				AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
-				AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+				ID:       "baz",
+				PluginID: "foo",
+				RequestedCapabilities: []*structs.CSIVolumeCapability{{
+					AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
+					AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+				}},
 			}},
 		}
 		body := encodeReq(args)

diff --git a/nomad/core_sched_test.go b/nomad/core_sched_test.go
@@ -2293,12 +2293,14 @@ func TestCoreScheduler_CSIVolumeClaimGC(t *testing.T) {
 
 	// Register a volume
 	vols := []*structs.CSIVolume{{
-		ID:             volID,
-		Namespace:      ns,
-		PluginID:       pluginID,
-		AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
-		AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
-		Topologies:     []*structs.CSITopology{},
+		ID:         volID,
+		Namespace:  ns,
+		PluginID:   pluginID,
+		Topologies: []*structs.CSITopology{},
+		RequestedCapabilities: []*structs.CSIVolumeCapability{{
+			AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
+			AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+		}},
 	}}
 	volReq := &structs.CSIVolumeRegisterRequest{Volumes: vols}
 	volReq.Namespace = ns

diff --git a/nomad/csi_endpoint_test.go b/nomad/csi_endpoint_test.go
@@ -36,12 +36,14 @@ func TestCSIVolumeEndpoint_Get(t *testing.T) {
 
 	// Create the volume
 	vols := []*structs.CSIVolume{{
-		ID:             id0,
-		Namespace:      ns,
-		AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
-		AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
-		PluginID:       "minnie",
-		Secrets:        structs.CSISecrets{"mysecret": "secretvalue"},
+		ID:        id0,
+		Namespace: ns,
+		PluginID:  "minnie",
+		Secrets:   structs.CSISecrets{"mysecret": "secretvalue"},
+		RequestedCapabilities: []*structs.CSIVolumeCapability{{
+			AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
+			AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+		}},
 	}}
 	err := state.CSIVolumeRegister(999, vols)
 	require.NoError(t, err)
@@ -84,12 +86,14 @@ func TestCSIVolumeEndpoint_Get_ACL(t *testing.T) {
 
 	// Create the volume
 	vols := []*structs.CSIVolume{{
-		ID:             id0,
-		Namespace:      ns,
-		AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
-		AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
-		PluginID:       "minnie",
-		Secrets:        structs.CSISecrets{"mysecret": "secretvalue"},
+		ID:        id0,
+		Namespace: ns,
+		PluginID:  "minnie",
+		Secrets:   structs.CSISecrets{"mysecret": "secretvalue"},
+		RequestedCapabilities: []*structs.CSIVolumeCapability{{
+			AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
+			AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+		}},
 	}}
 	err := state.CSIVolumeRegister(999, vols)
 	require.NoError(t, err)
@@ -143,13 +147,17 @@ func TestCSIVolumeEndpoint_Register(t *testing.T) {
 		ID:             id0,
 		Namespace:      "notTheNamespace",
 		PluginID:       "minnie",
-		AccessMode:     structs.CSIVolumeAccessModeMultiNodeReader,
-		AttachmentMode: structs.CSIVolumeAttachmentModeBlockDevice,
+		AccessMode:     structs.CSIVolumeAccessModeSingleNodeReader, // legacy field ignored
+		AttachmentMode: structs.CSIVolumeAttachmentModeBlockDevice,  // legacy field ignored
 		MountOptions: &structs.CSIMountOptions{
 			FSType: "ext4", MountFlags: []string{"sensitive"}},
 		Secrets:    structs.CSISecrets{"mysecret": "secretvalue"},
 		Parameters: map[string]string{"myparam": "paramvalue"},
 		Context:    map[string]string{"mycontext": "contextvalue"},
+		RequestedCapabilities: []*structs.CSIVolumeCapability{{
+			AccessMode:     structs.CSIVolumeAccessModeMultiNodeReader,
+			AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+		}},
 	}}
 
 	// Create the register request
@@ -262,15 +270,17 @@ func TestCSIVolumeEndpoint_Claim(t *testing.T) {
 	require.NoError(t, err)
 
 	vols := []*structs.CSIVolume{{
-		ID:             id0,
-		Namespace:      structs.DefaultNamespace,
-		PluginID:       "minnie",
-		AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
-		AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+		ID:        id0,
+		Namespace: structs.DefaultNamespace,
+		PluginID:  "minnie",
 		Topologies: []*structs.CSITopology{{
 			Segments: map[string]string{"foo": "bar"},
 		}},
 		Secrets: structs.CSISecrets{"mysecret": "secretvalue"},
+		RequestedCapabilities: []*structs.CSIVolumeCapability{{
+			AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
+			AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+		}},
 	}}
 	index++
 	err = state.CSIVolumeRegister(index, vols)
@@ -406,9 +416,11 @@ func TestCSIVolumeEndpoint_ClaimWithController(t *testing.T) {
 		Namespace:          ns,
 		PluginID:           "minnie",
 		ControllerRequired: true,
-		AccessMode:         structs.CSIVolumeAccessModeMultiNodeSingleWriter,
-		AttachmentMode:     structs.CSIVolumeAttachmentModeFilesystem,
 		Secrets:            structs.CSISecrets{"mysecret": "secretvalue"},
+		RequestedCapabilities: []*structs.CSIVolumeCapability{{
+			AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
+			AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+		}},
 	}}
 	err = state.CSIVolumeRegister(1003, vols)
 	require.NoError(t, err)
@@ -510,11 +522,13 @@ func TestCSIVolumeEndpoint_Unpublish(t *testing.T) {
 			vol := &structs.CSIVolume{
 				ID:                 volID,
 				Namespace:          ns,
-				AccessMode:         structs.CSIVolumeAccessModeMultiNodeSingleWriter,
-				AttachmentMode:     structs.CSIVolumeAttachmentModeFilesystem,
 				PluginID:           "minnie",
 				Secrets:            structs.CSISecrets{"mysecret": "secretvalue"},
 				ControllerRequired: true,
+				RequestedCapabilities: []*structs.CSIVolumeCapability{{
+					AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
+					AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+				}},
 			}
 
 			index++
@@ -608,18 +622,22 @@ func TestCSIVolumeEndpoint_List(t *testing.T) {
 	id0 := uuid.Generate()
 	id1 := uuid.Generate()
 	vols := []*structs.CSIVolume{{
-		ID:             id0,
-		Namespace:      structs.DefaultNamespace,
-		AccessMode:     structs.CSIVolumeAccessModeMultiNodeReader,
-		AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
-		PluginID:       "minnie",
-		Secrets:        structs.CSISecrets{"mysecret": "secretvalue"},
+		ID:        id0,
+		Namespace: structs.DefaultNamespace,
+		PluginID:  "minnie",
+		Secrets:   structs.CSISecrets{"mysecret": "secretvalue"},
+		RequestedCapabilities: []*structs.CSIVolumeCapability{{
+			AccessMode:     structs.CSIVolumeAccessModeMultiNodeReader,
+			AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+		}},
 	}, {
-		ID:             id1,
-		Namespace:      structs.DefaultNamespace,
-		AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
-		AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
-		PluginID:       "adam",
+		ID:        id1,
+		Namespace: structs.DefaultNamespace,
+		PluginID:  "adam",
+		RequestedCapabilities: []*structs.CSIVolumeCapability{{
+			AccessMode:     structs.CSIVolumeAccessModeMultiNodeSingleWriter,
+			AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+		}},
 	}}
 	err = state.CSIVolumeRegister(1002, vols)
 	require.NoError(t, err)
@@ -730,13 +748,19 @@ func TestCSIVolumeEndpoint_Create(t *testing.T) {
 		Name:           "vol",
 		Namespace:      "notTheNamespace", // overriden by WriteRequest
 		PluginID:       "minnie",
-		AccessMode:     structs.CSIVolumeAccessModeMultiNodeReader, // ignored in create
-		AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,  // ignored in create
+		AccessMode:     structs.CSIVolumeAccessModeSingleNodeReader, // legacy field ignored
+		AttachmentMode: structs.CSIVolumeAttachmentModeBlockDevice,  // legacy field ignored
 		MountOptions: &structs.CSIMountOptions{
 			FSType: "ext4", MountFlags: []string{"sensitive"}}, // ignored in create
 		Secrets:    structs.CSISecrets{"mysecret": "secretvalue"},
 		Parameters: map[string]string{"myparam": "paramvalue"},
 		Context:    map[string]string{"mycontext": "contextvalue"}, // dropped by create
+		RequestedCapabilities: []*structs.CSIVolumeCapability{
+			{
+				AccessMode:     structs.CSIVolumeAccessModeMultiNodeReader,
+				AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
+			},
+		},
 	}}
 
 	// Create the create request
@@ -773,6 +797,7 @@ func TestCSIVolumeEndpoint_Create(t *testing.T) {
 	require.Equal(t, "csi.CSIOptions(FSType: ext4, MountFlags: [REDACTED])",
 		vol.MountOptions.String())
 	require.Equal(t, ns, vol.Namespace)
+	require.Len(t, vol.RequestedCapabilities, 1)
 
 	// these fields are set from the plugin and should have been written to raft
 	require.Equal(t, "vol-12345", vol.ExternalID)

diff --git a/nomad/structs/csi.go b/nomad/structs/csi.go
@@ -664,6 +664,9 @@ func (v *CSIVolume) Validate() error {
 	if v.SnapshotID != "" && v.CloneID != "" {
 		errs = append(errs, "only one of snapshot_id and clone_id is allowed")
 	}
+	if len(v.RequestedCapabilities) == 0 {
+		errs = append(errs, "must include at least one capability block")
+	}
 
 	// TODO: Volume Topologies are optional - We should check to see if the plugin
 	//       the volume is being registered with requires them.

diff --git a/website/content/docs/commands/volume/register.mdx b/website/content/docs/commands/volume/register.mdx
@@ -77,6 +77,24 @@ context {
 - `plugin_id` `(string: <required>)` - The ID of the [CSI plugin][csi_plugin]
   that manages this volume.
 
+- `capability` `(Capability: <required>)` - Option for validating the
+  capbility of a volume. You must provide at least one `capability` block, and
+  you must provide a block for each capability you intend to use in a job's
+  [`volume`] block. Each `capability` block must have the following fields:
+
+  - `access_mode` `(string: <required>)` - Defines whether a volume should be
+    available concurrently. Can be one of `"single-node-reader-only"`,
+    `"single-node-writer"`, `"multi-node-reader-only"`,
+    `"multi-node-single-writer"`, or `"multi-node-multi-writer"`. Most CSI
+    plugins support only single-node modes. Consult the documentation of the
+    storage provider and CSI plugin.
+
+  - `attachment_mode` `(string: <required>)` - The storage API that will be used
+    by the volume. Most storage providers will support `"file-system"`, to mount
+    volumes using the CSI filesystem API. Some storage providers will support
+    `"block-device"`, which will mount the volume with the CSI block device API
+    within the container.
+
 - `secrets` <code>(map<string|string>:nil)</code> - An optional
   key-value map of strings used as credentials for publishing and
   unpublishing volumes.
@@ -98,8 +116,7 @@ context {
 Note that several fields used in the [`volume create`] command are set
 automatically by the plugin when `volume create` is successful and cannot be
 set on a pre-existing volume. You should not set the `snapshot_id`,
-`clone_id`, `capacity_min`, `capacity_max`, or `capability` fields described
-on that page.
+`clone_id`, `capacity_min`, or `capacity_max` fields described on that page.
 
 [csi]: https://github.com/container-storage-interface/spec
 [csi_plugins_internals]: /docs/internals/plugins/csi#csi-plugins

diff --git a/website/content/docs/job-specification/volume.mdx b/website/content/docs/job-specification/volume.mdx
@@ -78,17 +78,19 @@ the [volume_mount][volume_mount] stanza in the `task` configuration.
 The following fields are only valid for volumes with `type = "csi"`:
 
 - `access_mode` `(string: <required>)` - Defines whether a volume should be
-  available concurrently. Can be one of `"single-node-reader-only"`,
-  `"single-node-writer"`, `"multi-node-reader-only"`,
-  `"multi-node-single-writer"`, or `"multi-node-multi-writer"`. Most CSI
-  plugins support only single-node modes. Consult the documentation of the
-  storage provider and CSI plugin.
+  available concurrently. The `access_mode` and `attachment_mode` together
+  must exactly match one of the volume's `capability` blocks. Can be one of
+  `"single-node-reader-only"`, `"single-node-writer"`,
+  `"multi-node-reader-only"`, `"multi-node-single-writer"`, or
+  `"multi-node-multi-writer"`. Most CSI plugins support only single-node
+  modes. Consult the documentation of the storage provider and CSI plugin.
 
 - `attachment_mode` `(string: <required>)` - The storage API that will be used
-  by the volume. Most storage providers will support `"file-system"`, to mount
-  volumes using the CSI filesystem API. Some storage providers will support
-  `"block-device"`, which will mount the volume with the CSI block device API
-  within the container.
+  by the volume. The `access_mode` and `attachment_mode` together must exactly
+  match one of the volume's `capability` blocks. Most storage providers will
+  support `"file-system"`, to mount volumes using the CSI filesystem API. Some
+  storage providers will support `"block-device"`, which will mount the volume
+  with the CSI block device API within the container.
 
 - `per_alloc` `(bool: false)` - Specifies that the `source` of the volume
   should have the suffix `[n]`, where `n` is the allocation index. This allows

diff --git a/website/content/docs/upgrade/upgrade-specific.mdx b/website/content/docs/upgrade/upgrade-specific.mdx
@@ -22,6 +22,19 @@ The Nomad agent metrics API now respects the
 configuration value. If this value is set to `false`, which is the default value,
 calling `/v1/metrics?format=prometheus` will now result in a response error.
 
+#### CSI volumes
+
+The volume specification for CSI volumes has been updated to support volume
+creation. The `access_mode` and `attachment_mode` fields have been moved to a
+`capability` block that can be repeated. Existing registered volumes will be
+automatically modified the next time that a volume claim is updated. Volume
+specification files for new volumes should be updated to the format described
+in the [`volume create`] and [`volume register`] commands.
+
+The [`volume`] block has an `access_mode` and `attachment_mode` field that are
+required for CSI volumes. Jobs that use CSI volumes should be updated with
+these fields.
+
 #### Connect native tasks
 
 Connect native tasks running in host networking mode will now have `CONSUL_HTTP_ADDR`
@@ -1024,3 +1037,6 @@ deleted and then Nomad 0.3.0 can be launched.
 [node drain]: https://www.nomadproject.io/docs/upgrade#5-upgrade-clients
 [`template.disable_file_sandbox`]: /docs/configuration/client#template-parameters
 [pki]: https://www.vaultproject.io/docs/secrets/pki
+[`volume create`]: /docs/commands/volume/create
+[`volume register`]: /docs/commands/volume/register
+[`volume`]: /docs/job-specification/volume