Skip to content

Commit

Permalink
dynamic host volume: set default capability
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tgross committed Jan 15, 2025
1 parent a292ecc commit dc3cb54
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 27 deletions.
7 changes: 4 additions & 3 deletions command/asset/volume.host.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
17 changes: 13 additions & 4 deletions nomad/host_volume_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
`)

Expand Down Expand Up @@ -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,
Expand All @@ -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"))

Expand Down
10 changes: 2 additions & 8 deletions nomad/mock/host_volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
28 changes: 20 additions & 8 deletions nomad/structs/host_volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions nomad/structs/host_volumes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit dc3cb54

Please sign in to comment.