refactor: volume request modes to be generic between DHV/CSI (#24896)

When we implemented CSI, the types of the fields for access mode and attachment mode on volume requests were defined with a prefix "CSI". This gets confusing now that we have dynamic host volumes using the same fields. Fortunately the original was a typedef on string, and the Go API in the `api` package just uses strings directly, so we can change the name of the type without breaking backwards compatibility for the msgpack wire format. Update the names to `VolumeAccessMode` and `VolumeAttachmentMode`. Keep the CSI and DHV specific value constant names for these fields (they aren't currently 1:1), so that we can easily differentiate in a given bit of code which values are valid. Ref: https://github.com/hashicorp/nomad/pull/24881#discussion_r1920702890
2026-01-01 16:05:42 +03:00 · 2025-01-24 10:37:48 -05:00
parent b4d71f6693
commit 7add04eb0f
12 changed files with 76 additions and 75 deletions
--- a/client/csi_endpoint_test.go
+++ b/client/csi_endpoint_test.go
@@ -80,7 +80,7 @@ func TestCSIController_AttachVolume(t *testing.T) {
 				VolumeID:        "1234-4321-1234-4321",
 				ClientCSINodeID: "abcde",
 				AttachmentMode:  nstructs.CSIVolumeAttachmentModeFilesystem,
-				AccessMode:      nstructs.CSIVolumeAccessMode("foo"),
+				AccessMode:      nstructs.VolumeAccessMode("foo"),
 			},
 			ExpectedErr: errors.New("CSI.ControllerAttachVolume: unknown volume access mode: foo"),
 		},
@@ -93,7 +93,7 @@ func TestCSIController_AttachVolume(t *testing.T) {
 				VolumeID:        "1234-4321-1234-4321",
 				ClientCSINodeID: "abcde",
 				AccessMode:      nstructs.CSIVolumeAccessModeMultiNodeReader,
-				AttachmentMode:  nstructs.CSIVolumeAttachmentMode("bar"),
+				AttachmentMode:  nstructs.VolumeAttachmentMode("bar"),
 			},
 			ExpectedErr: errors.New("CSI.ControllerAttachVolume: unknown volume attachment mode: bar"),
 		},
@@ -217,7 +217,7 @@ func TestCSIController_ValidateVolume(t *testing.T) {
 				},
 				VolumeID: "1234-4321-1234-4321",
 				VolumeCapabilities: []*nstructs.CSIVolumeCapability{{
-					AttachmentMode: nstructs.CSIVolumeAttachmentMode("bar"),
+					AttachmentMode: nstructs.VolumeAttachmentMode("bar"),
 					AccessMode:     nstructs.CSIVolumeAccessModeMultiNodeReader,
 				}},
 			},
@@ -232,7 +232,7 @@ func TestCSIController_ValidateVolume(t *testing.T) {
 				VolumeID: "1234-4321-1234-4321",
 				VolumeCapabilities: []*nstructs.CSIVolumeCapability{{
 					AttachmentMode: nstructs.CSIVolumeAttachmentModeFilesystem,
-					AccessMode:     nstructs.CSIVolumeAccessMode("foo"),
+					AccessMode:     nstructs.VolumeAccessMode("foo"),
 				}},
 			},
 			ExpectedErr: errors.New("CSI.ControllerValidateVolume: unknown volume access mode: foo"),
@@ -395,7 +395,7 @@ func TestCSIController_CreateVolume(t *testing.T) {
 				VolumeCapabilities: []*nstructs.CSIVolumeCapability{
 					{
 						AttachmentMode: nstructs.CSIVolumeAttachmentModeFilesystem,
-						AccessMode:     nstructs.CSIVolumeAccessMode("foo"),
+						AccessMode:     nstructs.VolumeAccessMode("foo"),
 					},
 				},
 			},
@@ -411,7 +411,7 @@ func TestCSIController_CreateVolume(t *testing.T) {
 				VolumeCapabilities: []*nstructs.CSIVolumeCapability{
 					{
 						AccessMode:     nstructs.CSIVolumeAccessModeMultiNodeReader,
-						AttachmentMode: nstructs.CSIVolumeAttachmentMode("bar"),
+						AttachmentMode: nstructs.VolumeAttachmentMode("bar"),
 					},
 				},
 			},
--- a/client/pluginmanager/csimanager/interface.go
+++ b/client/pluginmanager/csimanager/interface.go
@@ -29,8 +29,8 @@ func (mi *MountInfo) Copy() *MountInfo {

 type UsageOptions struct {
 	ReadOnly       bool
-	AttachmentMode structs.CSIVolumeAttachmentMode
-	AccessMode     structs.CSIVolumeAccessMode
+	AttachmentMode structs.VolumeAttachmentMode
+	AccessMode     structs.VolumeAccessMode
 	MountOptions   *structs.CSIMountOptions
 }

--- a/client/structs/csi.go
+++ b/client/structs/csi.go
@@ -65,8 +65,8 @@ type ClientCSIControllerValidateVolumeRequest struct {

 	// COMPAT(1.1.1): the AttachmentMode and AccessMode fields are deprecated
 	// and replaced by the VolumeCapabilities field above
-	AttachmentMode structs.CSIVolumeAttachmentMode
-	AccessMode     structs.CSIVolumeAccessMode
+	AttachmentMode structs.VolumeAttachmentMode
+	AccessMode     structs.VolumeAccessMode

 	// Parameters as returned by storage provider in CreateVolumeResponse.
 	// This field is optional.
@@ -117,10 +117,10 @@ type ClientCSIControllerAttachVolumeRequest struct {

 	// AttachmentMode indicates how the volume should be attached and mounted into
 	// a task.
-	AttachmentMode structs.CSIVolumeAttachmentMode
+	AttachmentMode structs.VolumeAttachmentMode

 	// AccessMode indicates the desired concurrent access model for the volume
-	AccessMode structs.CSIVolumeAccessMode
+	AccessMode structs.VolumeAccessMode

 	// MountOptions is an optional field that contains additional configuration
 	// when providing an AttachmentMode of CSIVolumeAttachmentModeFilesystem
@@ -449,8 +449,8 @@ type ClientCSINodeDetachVolumeRequest struct {

 	// These fields should match the original volume request so that
 	// we can find the mount points on the client
-	AttachmentMode structs.CSIVolumeAttachmentMode
-	AccessMode     structs.CSIVolumeAccessMode
+	AttachmentMode structs.VolumeAttachmentMode
+	AccessMode     structs.VolumeAccessMode
 	ReadOnly       bool
 }

--- a/command/agent/job_endpoint.go
+++ b/command/agent/job_endpoint.go
@@ -1336,8 +1336,8 @@ func ApiTgToStructsTG(job *structs.Job, taskGroup *api.TaskGroup, tg *structs.Ta
 				ReadOnly:       v.ReadOnly,
 				Sticky:         v.Sticky,
 				Source:         v.Source,
-				AttachmentMode: structs.CSIVolumeAttachmentMode(v.AttachmentMode),
-				AccessMode:     structs.CSIVolumeAccessMode(v.AccessMode),
+				AttachmentMode: structs.VolumeAttachmentMode(v.AttachmentMode),
+				AccessMode:     structs.VolumeAccessMode(v.AccessMode),
 				PerAlloc:       v.PerAlloc,
 			}

--- a/nomad/structs/csi.go
+++ b/nomad/structs/csi.go
@@ -117,33 +117,25 @@ func (t *TaskCSIPluginConfig) Copy() *TaskCSIPluginConfig {
 // CSIVolumeCapability is the requested attachment and access mode for a
 // volume
 type CSIVolumeCapability struct {
-	AttachmentMode CSIVolumeAttachmentMode
-	AccessMode     CSIVolumeAccessMode
+	AttachmentMode VolumeAttachmentMode
+	AccessMode     VolumeAccessMode
 }

-// CSIVolumeAttachmentMode chooses the type of storage api that will be used to
-// interact with the device.
-type CSIVolumeAttachmentMode string
-
 const (
-	CSIVolumeAttachmentModeUnknown     CSIVolumeAttachmentMode = ""
-	CSIVolumeAttachmentModeBlockDevice CSIVolumeAttachmentMode = "block-device"
-	CSIVolumeAttachmentModeFilesystem  CSIVolumeAttachmentMode = "file-system"
+	CSIVolumeAttachmentModeUnknown     VolumeAttachmentMode = ""
+	CSIVolumeAttachmentModeBlockDevice VolumeAttachmentMode = "block-device"
+	CSIVolumeAttachmentModeFilesystem  VolumeAttachmentMode = "file-system"
 )

-// CSIVolumeAccessMode indicates how a volume should be used in a storage topology
-// e.g whether the provider should make the volume available concurrently.
-type CSIVolumeAccessMode string
-
 const (
-	CSIVolumeAccessModeUnknown CSIVolumeAccessMode = ""
+	CSIVolumeAccessModeUnknown VolumeAccessMode = ""

-	CSIVolumeAccessModeSingleNodeReader CSIVolumeAccessMode = "single-node-reader-only"
-	CSIVolumeAccessModeSingleNodeWriter CSIVolumeAccessMode = "single-node-writer"
+	CSIVolumeAccessModeSingleNodeReader VolumeAccessMode = "single-node-reader-only"
+	CSIVolumeAccessModeSingleNodeWriter VolumeAccessMode = "single-node-writer"

-	CSIVolumeAccessModeMultiNodeReader       CSIVolumeAccessMode = "multi-node-reader-only"
-	CSIVolumeAccessModeMultiNodeSingleWriter CSIVolumeAccessMode = "multi-node-single-writer"
-	CSIVolumeAccessModeMultiNodeMultiWriter  CSIVolumeAccessMode = "multi-node-multi-writer"
+	CSIVolumeAccessModeMultiNodeReader       VolumeAccessMode = "multi-node-reader-only"
+	CSIVolumeAccessModeMultiNodeSingleWriter VolumeAccessMode = "multi-node-single-writer"
+	CSIVolumeAccessModeMultiNodeMultiWriter  VolumeAccessMode = "multi-node-multi-writer"
 )

 // CSIMountOptions contain optional additional configuration that can be used
@@ -238,8 +230,8 @@ type CSIVolumeClaim struct {
 	NodeID         string
 	ExternalNodeID string
 	Mode           CSIVolumeClaimMode
-	AccessMode     CSIVolumeAccessMode
-	AttachmentMode CSIVolumeAttachmentMode
+	AccessMode     VolumeAccessMode
+	AttachmentMode VolumeAttachmentMode
 	State          CSIVolumeClaimState
 }

@@ -273,8 +265,8 @@ type CSIVolume struct {
 	// could support. This value cannot be set by the user.
 	Topologies []*CSITopology

-	AccessMode     CSIVolumeAccessMode     // *current* access mode
-	AttachmentMode CSIVolumeAttachmentMode // *current* attachment mode
+	AccessMode     VolumeAccessMode     // *current* access mode
+	AttachmentMode VolumeAttachmentMode // *current* attachment mode
 	MountOptions   *CSIMountOptions

 	Secrets    CSISecrets
@@ -352,8 +344,8 @@ type CSIVolListStub struct {
 	Name                string
 	ExternalID          string
 	Topologies          []*CSITopology
-	AccessMode          CSIVolumeAccessMode
-	AttachmentMode      CSIVolumeAttachmentMode
+	AccessMode          VolumeAccessMode
+	AttachmentMode      VolumeAttachmentMode
 	CurrentReaders      int
 	CurrentWriters      int
 	Schedulable         bool
@@ -933,8 +925,8 @@ type CSIVolumeClaimRequest struct {
 	NodeID         string
 	ExternalNodeID string
 	Claim          CSIVolumeClaimMode
-	AccessMode     CSIVolumeAccessMode
-	AttachmentMode CSIVolumeAttachmentMode
+	AccessMode     VolumeAccessMode
+	AttachmentMode VolumeAttachmentMode
 	State          CSIVolumeClaimState
 	Timestamp      int64 // UnixNano
 	WriteRequest
--- a/nomad/structs/funcs.go
+++ b/nomad/structs/funcs.go
@@ -173,7 +173,7 @@ func AllocsFit(node *Node, allocs []*Allocation, netIdx *NetworkIndex, checkDevi
 			for _, volReq := range group.Volumes {
 				hostVolumeClaims[volReq.Source]++
 				if volReq.AccessMode ==
-					CSIVolumeAccessMode(HostVolumeAccessModeSingleNodeSingleWriter) {
+					HostVolumeAccessModeSingleNodeSingleWriter {
 					exclusiveHostVolumeClaims = append(exclusiveHostVolumeClaims, volReq.Source)
 				}
 			}
--- a/nomad/structs/funcs_test.go
+++ b/nomad/structs/funcs_test.go
@@ -549,7 +549,7 @@ func TestAllocsFit_ExclusiveVolumes(t *testing.T) {
 		Job: &Job{TaskGroups: []*TaskGroup{{Name: "group", Volumes: map[string]*VolumeRequest{
 			"foo": {
 				Source:     "example",
-				AccessMode: CSIVolumeAccessMode(HostVolumeAccessModeSingleNodeSingleWriter),
+				AccessMode: HostVolumeAccessModeSingleNodeSingleWriter,
 			},
 		}}}},
 		AllocatedResources: &AllocatedResources{
@@ -566,7 +566,7 @@ func TestAllocsFit_ExclusiveVolumes(t *testing.T) {
 		Cpu:    AllocatedCpuResources{CpuShares: 500},
 		Memory: AllocatedMemoryResources{MemoryMB: 500},
 	}
-	a2.Job.TaskGroups[0].Volumes["foo"].AccessMode = CSIVolumeAccessModeMultiNodeReader
+	a2.Job.TaskGroups[0].Volumes["foo"].AccessMode = HostVolumeAccessModeSingleNodeMultiWriter

 	// Should fit one allocation
 	fit, _, _, err := AllocsFit(n, []*Allocation{a1}, nil, true)
--- a/nomad/structs/host_volumes.go
+++ b/nomad/structs/host_volumes.go
@@ -289,8 +289,8 @@ func (hv *HostVolume) GetID() string {

 // HostVolumeCapability is the requested attachment and access mode for a volume
 type HostVolumeCapability struct {
-	AttachmentMode HostVolumeAttachmentMode
-	AccessMode     HostVolumeAccessMode
+	AttachmentMode VolumeAttachmentMode
+	AccessMode     VolumeAccessMode
 }

 func (hvc *HostVolumeCapability) Copy() *HostVolumeCapability {
@@ -326,27 +326,23 @@ func (hvc *HostVolumeCapability) Validate() error {
 	return nil
 }

-// HostVolumeAttachmentMode chooses the type of storage API that will be used to
+// HostVolumeAttachmentModes choose the type of storage API that will be used to
 // interact with the device.
-type HostVolumeAttachmentMode string
-
 const (
-	HostVolumeAttachmentModeUnknown     HostVolumeAttachmentMode = ""
-	HostVolumeAttachmentModeBlockDevice HostVolumeAttachmentMode = "block-device"
-	HostVolumeAttachmentModeFilesystem  HostVolumeAttachmentMode = "file-system"
+	HostVolumeAttachmentModeUnknown     VolumeAttachmentMode = ""
+	HostVolumeAttachmentModeBlockDevice VolumeAttachmentMode = "block-device"
+	HostVolumeAttachmentModeFilesystem  VolumeAttachmentMode = "file-system"
 )

-// HostVolumeAccessMode indicates how Nomad should make the volume available to
+// HostVolumeAccessModes indicate how Nomad should make the volume available to
 // concurrent allocations.
-type HostVolumeAccessMode string
-
 const (
-	HostVolumeAccessModeUnknown HostVolumeAccessMode = ""
+	HostVolumeAccessModeUnknown VolumeAccessMode = ""

-	HostVolumeAccessModeSingleNodeReader       HostVolumeAccessMode = "single-node-reader-only"
-	HostVolumeAccessModeSingleNodeWriter       HostVolumeAccessMode = "single-node-writer"
-	HostVolumeAccessModeSingleNodeSingleWriter HostVolumeAccessMode = "single-node-single-writer"
-	HostVolumeAccessModeSingleNodeMultiWriter  HostVolumeAccessMode = "single-node-multi-writer"
+	HostVolumeAccessModeSingleNodeReader       VolumeAccessMode = "single-node-reader-only"
+	HostVolumeAccessModeSingleNodeWriter       VolumeAccessMode = "single-node-writer"
+	HostVolumeAccessModeSingleNodeSingleWriter VolumeAccessMode = "single-node-single-writer"
+	HostVolumeAccessModeSingleNodeMultiWriter  VolumeAccessMode = "single-node-multi-writer"
 )

 // HostVolumeStub is used for responses for the list volumes endpoint
--- a/nomad/structs/volumes.go
+++ b/nomad/structs/volumes.go
@@ -111,8 +111,8 @@ type VolumeRequest struct {
 	Source         string
 	ReadOnly       bool
 	Sticky         bool
-	AccessMode     CSIVolumeAccessMode
-	AttachmentMode CSIVolumeAttachmentMode
+	AccessMode     VolumeAccessMode
+	AttachmentMode VolumeAttachmentMode
 	MountOptions   *CSIMountOptions
 	PerAlloc       bool
 }
@@ -178,14 +178,19 @@ func (v *VolumeRequest) Validate(jobType string, taskGroupCount, canaries int) e
 		}

 		switch v.AccessMode {
-		case CSIVolumeAccessModeSingleNodeReader, CSIVolumeAccessModeMultiNodeReader:
+		case HostVolumeAccessModeSingleNodeReader:
 			if !v.ReadOnly {
 				addErr("%s volumes must be read-only", v.AccessMode)
 			}
-		default:
+		case HostVolumeAccessModeSingleNodeWriter,
+			HostVolumeAccessModeSingleNodeSingleWriter,
+			HostVolumeAccessModeSingleNodeMultiWriter,
+			HostVolumeAccessModeUnknown:
 			// dynamic host volumes are all "per node" so there's no way to
 			// validate that other access modes work for a given volume until we
 			// have access to other allocations (in the scheduler)
+		default:
+			addErr("host volumes cannot be mounted with %s access mode")
 		}

 	case VolumeTypeCSI:
@@ -265,6 +270,14 @@ func CopyMapVolumeRequest(s map[string]*VolumeRequest) map[string]*VolumeRequest
 	return c
 }

+// VolumeAttachmentMode chooses the type of storage api that will be used to
+// interact with the device.
+type VolumeAttachmentMode string
+
+// VolumeAccessMode indicates how a volume should be used in a storage topology
+// e.g whether the provider should make the volume available concurrently.
+type VolumeAccessMode string
+
 // VolumeMount represents the relationship between a destination path in a task
 // and the task group volume that should be mounted there.
 type VolumeMount struct {
--- a/plugins/csi/plugin.go
+++ b/plugins/csi/plugin.go
@@ -948,7 +948,7 @@ type VolumeCapability struct {
 	MountVolume *structs.CSIMountOptions
 }

-func VolumeCapabilityFromStructs(sAccessType structs.CSIVolumeAttachmentMode, sAccessMode structs.CSIVolumeAccessMode, sMountOptions *structs.CSIMountOptions) (*VolumeCapability, error) {
+func VolumeCapabilityFromStructs(sAccessType structs.VolumeAttachmentMode, sAccessMode structs.VolumeAccessMode, sMountOptions *structs.CSIMountOptions) (*VolumeCapability, error) {
 	var accessType VolumeAccessType
 	switch sAccessType {
 	case structs.CSIVolumeAttachmentModeBlockDevice:
--- a/scheduler/feasible.go
+++ b/scheduler/feasible.go
@@ -214,8 +214,8 @@ func (h *HostVolumeChecker) hasVolumes(n *structs.Node) bool {
 				return false
 			}
 			if !h.hostVolumeIsAvailable(vol,
-				structs.HostVolumeAccessMode(req.AccessMode),
-				structs.HostVolumeAttachmentMode(req.AttachmentMode),
+				req.AccessMode,
+				req.AttachmentMode,
 				req.ReadOnly,
 				proposed,
 			) {
@@ -245,8 +245,8 @@ func (h *HostVolumeChecker) hasVolumes(n *structs.Node) bool {
 // hostVolumeIsAvailable determines if a dynamic host volume is available for a request
 func (h *HostVolumeChecker) hostVolumeIsAvailable(
 	vol *structs.HostVolume,
-	reqAccess structs.HostVolumeAccessMode,
-	reqAttach structs.HostVolumeAttachmentMode,
+	reqAccess structs.VolumeAccessMode,
+	reqAttach structs.VolumeAttachmentMode,
 	readOnly bool,
 	proposed []*structs.Allocation) bool {

--- a/scheduler/feasible_test.go
+++ b/scheduler/feasible_test.go
@@ -484,13 +484,13 @@ func TestDynamicHostVolumeIsAvailable(t *testing.T) {

 	allCaps := []*structs.HostVolumeCapability{}

-	for _, accessMode := range []structs.HostVolumeAccessMode{
+	for _, accessMode := range []structs.VolumeAccessMode{
 		structs.HostVolumeAccessModeSingleNodeReader,
 		structs.HostVolumeAccessModeSingleNodeWriter,
 		structs.HostVolumeAccessModeSingleNodeSingleWriter,
 		structs.HostVolumeAccessModeSingleNodeMultiWriter,
 	} {
-		for _, attachMode := range []structs.HostVolumeAttachmentMode{
+		for _, attachMode := range []structs.VolumeAttachmentMode{
 			structs.HostVolumeAttachmentModeFilesystem,
 			structs.HostVolumeAttachmentModeBlockDevice,
 		} {
@@ -537,8 +537,8 @@ func TestDynamicHostVolumeIsAvailable(t *testing.T) {
 		name        string
 		hasProposed []*structs.Allocation
 		hasCaps     []*structs.HostVolumeCapability
-		wantAccess  structs.HostVolumeAccessMode
-		wantAttach  structs.HostVolumeAttachmentMode
+		wantAccess  structs.VolumeAccessMode
+		wantAttach  structs.VolumeAttachmentMode
 		readOnly    bool
 		expect      bool
 	}{