From dc3cb547da337da054e969a05936c6681a0c62db Mon Sep 17 00:00:00 2001 From: Tim Gross <tgross@hashicorp.com> Date: Tue, 14 Jan 2025 14:59:32 -0500 Subject: [PATCH] dynamic host volume: set default capability We can reduce the amount of volume specification configuration many users will need by setting a default capability on a dynamic host volume if none is set. The default capability will allow using the volume in read/write mode on its node, with no further restrictions except those that might be set in the jobspec. --- command/asset/volume.host.hcl | 7 ++++--- nomad/host_volume_endpoint_test.go | 17 +++++++++++++---- nomad/mock/host_volumes.go | 10 ++-------- nomad/structs/host_volumes.go | 28 ++++++++++++++++++++-------- nomad/structs/host_volumes_test.go | 9 +++++---- 5 files changed, 44 insertions(+), 27 deletions(-) diff --git a/command/asset/volume.host.hcl b/command/asset/volume.host.hcl index 3447eef998f..4512954a76b 100644 --- a/command/asset/volume.host.hcl +++ b/command/asset/volume.host.hcl @@ -9,9 +9,10 @@ plugin_id = "plugin_id" capacity_min = "10GiB" capacity_max = "20G" -# Required (at least one): for 'nomad volume create', specify one or more -# capabilities to validate. Registering an existing volume will record but -# ignore these fields. +# Optional: for 'nomad volume create', specify one or more capabilities to +# validate. Registering an existing volume will record but ignore these fields. +# If omitted, the single-node-writer + file-system capability will be used as a +# default. capability { access_mode = "single-node-writer" attachment_mode = "file-system" diff --git a/nomad/host_volume_endpoint_test.go b/nomad/host_volume_endpoint_test.go index ae5bf08c924..793043e53b7 100644 --- a/nomad/host_volume_endpoint_test.go +++ b/nomad/host_volume_endpoint_test.go @@ -79,11 +79,13 @@ func TestHostVolumeEndpoint_CreateRegisterGetDelete(t *testing.T) { err := msgpackrpc.CallWithCodec(codec, "HostVolume.Create", req, &resp) must.EqError(t, err, "missing volume definition") - req.Volume = &structs.HostVolume{} + req.Volume = &structs.HostVolume{RequestedCapabilities: []*structs.HostVolumeCapability{ + {AttachmentMode: "foo"}}} + err = msgpackrpc.CallWithCodec(codec, "HostVolume.Create", req, &resp) must.EqError(t, err, `volume validation failed: 2 errors occurred: * missing name - * must include at least one capability block + * invalid attachment mode: "foo" `) @@ -214,7 +216,14 @@ func TestHostVolumeEndpoint_CreateRegisterGetDelete(t *testing.T) { t.Run("invalid updates", func(t *testing.T) { invalidVol1 := vol1.Copy() - invalidVol2 := &structs.HostVolume{NodeID: uuid.Generate()} + invalidVol2 := &structs.HostVolume{ + NodeID: uuid.Generate(), + RequestedCapabilities: []*structs.HostVolumeCapability{ + { + AttachmentMode: structs.HostVolumeAttachmentModeFilesystem, + AccessMode: "foo", + }}, + } createReq := &structs.HostVolumeCreateRequest{ Volume: invalidVol2, @@ -228,7 +237,7 @@ func TestHostVolumeEndpoint_CreateRegisterGetDelete(t *testing.T) { err := msgpackrpc.CallWithCodec(codec, "HostVolume.Create", createReq, &createResp) must.EqError(t, err, `volume validation failed: 2 errors occurred: * missing name - * must include at least one capability block + * invalid access mode: "foo" `, must.Sprint("initial validation failures should exit early")) diff --git a/nomad/mock/host_volumes.go b/nomad/mock/host_volumes.go index a87b084dad3..ee7dda1b3a3 100644 --- a/nomad/mock/host_volumes.go +++ b/nomad/mock/host_volumes.go @@ -23,14 +23,8 @@ func HostVolumeRequest(ns string) *structs.HostVolume { }, RequestedCapacityMinBytes: 100000, RequestedCapacityMaxBytes: 200000, - RequestedCapabilities: []*structs.HostVolumeCapability{ - { - AttachmentMode: structs.HostVolumeAttachmentModeFilesystem, - AccessMode: structs.HostVolumeAccessModeSingleNodeWriter, - }, - }, - Parameters: map[string]string{"foo": "bar"}, - State: structs.HostVolumeStatePending, + Parameters: map[string]string{"foo": "bar"}, + State: structs.HostVolumeStatePending, } return vol diff --git a/nomad/structs/host_volumes.go b/nomad/structs/host_volumes.go index 78e25d10091..18e5817dfac 100644 --- a/nomad/structs/host_volumes.go +++ b/nomad/structs/host_volumes.go @@ -148,14 +148,10 @@ func (hv *HostVolume) Validate() error { hv.RequestedCapacityMaxBytes, hv.RequestedCapacityMinBytes)) } - if len(hv.RequestedCapabilities) == 0 { - mErr = multierror.Append(mErr, errors.New("must include at least one capability block")) - } else { - for _, cap := range hv.RequestedCapabilities { - err := cap.Validate() - if err != nil { - mErr = multierror.Append(mErr, err) - } + for _, cap := range hv.RequestedCapabilities { + err := cap.Validate() + if err != nil { + mErr = multierror.Append(mErr, err) } } @@ -221,6 +217,14 @@ func (hv *HostVolume) CanonicalizeForCreate(existing *HostVolume, now time.Time) hv.CapacityBytes = 0 // returned by plugin hv.HostPath = "" // returned by plugin hv.CreateTime = now.UnixNano() + + if len(hv.RequestedCapabilities) == 0 { + hv.RequestedCapabilities = []*HostVolumeCapability{{ + AttachmentMode: HostVolumeAttachmentModeFilesystem, + AccessMode: HostVolumeAccessModeSingleNodeWriter, + }} + } + } else { if hv.PluginID == "" { hv.PluginID = existing.PluginID @@ -248,6 +252,14 @@ func (hv *HostVolume) CanonicalizeForRegister(existing *HostVolume, now time.Tim if existing == nil { hv.ID = uuid.Generate() hv.CreateTime = now.UnixNano() + + if len(hv.RequestedCapabilities) == 0 { + hv.RequestedCapabilities = []*HostVolumeCapability{{ + AttachmentMode: HostVolumeAttachmentModeFilesystem, + AccessMode: HostVolumeAccessModeSingleNodeWriter, + }} + } + } else { if hv.PluginID == "" { hv.PluginID = existing.PluginID diff --git a/nomad/structs/host_volumes_test.go b/nomad/structs/host_volumes_test.go index 9965d755552..e0e00b843fc 100644 --- a/nomad/structs/host_volumes_test.go +++ b/nomad/structs/host_volumes_test.go @@ -58,18 +58,19 @@ func TestHostVolume_Copy(t *testing.T) { func TestHostVolume_Validate(t *testing.T) { ci.Parallel(t) - invalid := &HostVolume{} + invalid := &HostVolume{RequestedCapabilities: []*HostVolumeCapability{ + {AttachmentMode: "foo"}}} err := invalid.Validate() must.EqError(t, err, `2 errors occurred: * missing name - * must include at least one capability block + * invalid attachment mode: "foo" `) - invalid = &HostVolume{Name: "example"} + invalid = &HostVolume{} err = invalid.Validate() // single error should be flattened - must.EqError(t, err, "must include at least one capability block") + must.EqError(t, err, "missing name") invalid = &HostVolume{ ID: "../../not-a-uuid",