Skip to content

Commit

Permalink
dynamic host volumes: remove multi-node access modes (#24705)
Browse files Browse the repository at this point in the history
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: #24479
Ref: #24684
  • Loading branch information
tgross authored Dec 18, 2024
1 parent c0632a3 commit 2545932
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 20 deletions.
15 changes: 6 additions & 9 deletions nomad/structs/host_volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions nomad/structs/host_volumes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -195,7 +195,7 @@ func TestHostVolume_CanonicalizeForUpdate(t *testing.T) {
RequestedCapacityMaxBytes: 500000,
RequestedCapabilities: []*HostVolumeCapability{{
AttachmentMode: HostVolumeAttachmentModeFilesystem,
AccessMode: HostVolumeAccessModeMultiNodeMultiWriter,
AccessMode: HostVolumeAccessModeSingleNodeMultiWriter,
}},
}
existing := &HostVolume{
Expand Down Expand Up @@ -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)
Expand Down
18 changes: 12 additions & 6 deletions nomad/structs/volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions nomad/structs/volumes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand Down

0 comments on commit 2545932

Please sign in to comment.