From 6419b5eba89b89e04b02b7f4becd135dd708d833 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Tue, 17 Dec 2024 15:55:22 -0500 Subject: [PATCH] dynamic host volumes: remove multi-node access modes CSI volumes support multi-node access patterns on the same volume ID, but dynamic host volumes by nature do not. The underlying volume may actually be multi-node (ex. NFS), but Nomad is ignorant of this. Remove the CSI-specific multi-node access modes and instead include the single-node access modes intended that are currently in the alpha edition of the CSI spec but which are better suited for DHV. This PR has been extracted from #24684 to keep reviews manageable. Ref: https://github.com/hashicorp/nomad/pull/24479 Ref: https://github.com/hashicorp/nomad/pull/24684 --- nomad/structs/host_volumes.go | 15 ++++++--------- nomad/structs/host_volumes_test.go | 6 +++--- nomad/structs/volumes.go | 18 ++++++++++++------ nomad/structs/volumes_test.go | 4 ++-- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/nomad/structs/host_volumes.go b/nomad/structs/host_volumes.go index c254bf72902..440ad956512 100644 --- a/nomad/structs/host_volumes.go +++ b/nomad/structs/host_volumes.go @@ -275,9 +275,8 @@ func (hvc *HostVolumeCapability) Validate() error { switch hvc.AccessMode { case HostVolumeAccessModeSingleNodeReader, HostVolumeAccessModeSingleNodeWriter, - HostVolumeAccessModeMultiNodeReader, - HostVolumeAccessModeMultiNodeSingleWriter, - HostVolumeAccessModeMultiNodeMultiWriter: + HostVolumeAccessModeSingleNodeSingleWriter, + HostVolumeAccessModeSingleNodeMultiWriter: default: return fmt.Errorf("invalid access mode: %q", hvc.AccessMode) } @@ -302,12 +301,10 @@ type HostVolumeAccessMode string const ( HostVolumeAccessModeUnknown HostVolumeAccessMode = "" - HostVolumeAccessModeSingleNodeReader HostVolumeAccessMode = "single-node-reader-only" - HostVolumeAccessModeSingleNodeWriter HostVolumeAccessMode = "single-node-writer" - - HostVolumeAccessModeMultiNodeReader HostVolumeAccessMode = "multi-node-reader-only" - HostVolumeAccessModeMultiNodeSingleWriter HostVolumeAccessMode = "multi-node-single-writer" - HostVolumeAccessModeMultiNodeMultiWriter HostVolumeAccessMode = "multi-node-multi-writer" + HostVolumeAccessModeSingleNodeReader HostVolumeAccessMode = "single-node-reader-only" + HostVolumeAccessModeSingleNodeWriter HostVolumeAccessMode = "single-node-writer" + HostVolumeAccessModeSingleNodeSingleWriter HostVolumeAccessMode = "single-node-single-writer" + HostVolumeAccessModeSingleNodeMultiWriter HostVolumeAccessMode = "single-node-multi-writer" ) // HostVolumeStub is used for responses for the list volumes endpoint diff --git a/nomad/structs/host_volumes_test.go b/nomad/structs/host_volumes_test.go index 499bc27d1c8..2a03e838daf 100644 --- a/nomad/structs/host_volumes_test.go +++ b/nomad/structs/host_volumes_test.go @@ -45,7 +45,7 @@ func TestHostVolume_Copy(t *testing.T) { out.Constraints[0].LTarget = "${meta.node_class}" out.RequestedCapabilities = append(out.RequestedCapabilities, &HostVolumeCapability{ AttachmentMode: HostVolumeAttachmentModeBlockDevice, - AccessMode: HostVolumeAccessModeMultiNodeReader, + AccessMode: HostVolumeAccessModeSingleNodeMultiWriter, }) out.Parameters["foo"] = "baz" @@ -195,7 +195,7 @@ func TestHostVolume_CanonicalizeForUpdate(t *testing.T) { RequestedCapacityMaxBytes: 500000, RequestedCapabilities: []*HostVolumeCapability{{ AttachmentMode: HostVolumeAttachmentModeFilesystem, - AccessMode: HostVolumeAccessModeMultiNodeMultiWriter, + AccessMode: HostVolumeAccessModeSingleNodeMultiWriter, }}, } existing := &HostVolume{ @@ -240,7 +240,7 @@ func TestHostVolume_CanonicalizeForUpdate(t *testing.T) { must.Eq(t, []*HostVolumeCapability{{ AttachmentMode: HostVolumeAttachmentModeFilesystem, - AccessMode: HostVolumeAccessModeMultiNodeMultiWriter, + AccessMode: HostVolumeAccessModeSingleNodeMultiWriter, }}, vol.RequestedCapabilities) must.Eq(t, "/var/nomad/alloc_mounts/82f357d6.ext4", vol.HostPath) diff --git a/nomad/structs/volumes.go b/nomad/structs/volumes.go index b8c95fc2862..6f508ef3f27 100644 --- a/nomad/structs/volumes.go +++ b/nomad/structs/volumes.go @@ -170,16 +170,22 @@ func (v *VolumeRequest) Validate(jobType string, taskGroupCount, canaries int) e switch v.Type { case VolumeTypeHost: - if v.AttachmentMode != CSIVolumeAttachmentModeUnknown { - addErr("host volumes cannot have an attachment mode") - } - if v.AccessMode != CSIVolumeAccessModeUnknown { - addErr("host volumes cannot have an access mode") - } if v.MountOptions != nil { + // TODO(1.10.0): support mount options for dynamic host volumes addErr("host volumes cannot have mount options") } + switch v.AccessMode { + case CSIVolumeAccessModeSingleNodeReader, CSIVolumeAccessModeMultiNodeReader: + if !v.ReadOnly { + addErr("%s volumes must be read-only", v.AccessMode) + } + default: + // 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) + } + case VolumeTypeCSI: switch v.AttachmentMode { diff --git a/nomad/structs/volumes_test.go b/nomad/structs/volumes_test.go index 58585932d7c..9b697faf18c 100644 --- a/nomad/structs/volumes_test.go +++ b/nomad/structs/volumes_test.go @@ -31,9 +31,9 @@ func TestVolumeRequest_Validate(t *testing.T) { { name: "host volume with CSI volume config", expected: []string{ - "host volumes cannot have an access mode", - "host volumes cannot have an attachment mode", + "volume has an empty source", "host volumes cannot have mount options", + "single-node-reader-only volumes must be read-only", "volume cannot be per_alloc for system or sysbatch jobs", "volume cannot be per_alloc when canaries are in use", },