dynamic host volumes: basic CLI CRUD #24382
Conversation
tgross
force-pushed
the
dynamic-host-volumes
branch
from
1d8323b to
dae28c9
Compare
tgross
force-pushed
the
dynamic-host-volumes
branch
from
dae28c9 to
39d63c8
Compare
tgross
force-pushed
the
dynamic-host-volumes
branch
from
40f6063 to
da738e7
Compare
tgross
force-pushed
the
dynamic-host-volumes
branch
from
7f7f726 to
f650d18
Compare
| // HostPath is the path on disk where the volume's mount point was | ||
| // created. We record this to make debugging easier. | ||
| HostPath string `mapstructure:"host_path" hcl:"host_path"` |
There was a problem hiding this comment.
users can't (shouldn't be able to) override this, right? so why hcl tag?
...on second thought, I guess it must be for Register, where the volume will have been created out-of-band.
There was a problem hiding this comment.
Right, only for Register.
| HostVolumeStatePending HostVolumeState = "pending" | ||
| HostVolumeStateReady HostVolumeState = "ready" | ||
| HostVolumeStateDeleted HostVolumeState = "deleted" |
There was a problem hiding this comment.
what if a node becomes unavailable, temporary or otherwise? would it go back to pending, or might we want unknown or similar?
There was a problem hiding this comment.
We didn't really cover that feature in the RFC. I think we'll need to figure out whether there's any notion of liveness when we start on the client-side work.
| func parseConstraints(result *[]*api.Constraint, list *ast.ObjectList) error { | ||
| for _, o := range list.Elem().Items { | ||
| valid := []string{ | ||
| "attribute", | ||
| "distinct_hosts", |
There was a problem hiding this comment.
I'm a little surprised this isn't already a thing somewhere.
There was a problem hiding this comment.
It was! We ripped it out with the rest of HCL1 jobspec parsing. Because everything other than jobspecs is still HCL1, I needed to resurrect this from the code we removed.
| if exactMatchesCount == 1 { | ||
| return match, nil, nil | ||
| } | ||
| return nil, vols, nil |
There was a problem hiding this comment.
nil pointer and nil error still makes me twitchy, and it'd be pretty easy to handle a consistent slice at the call site instead. i.e. I'd prefer the return signature of this method be ([]*api.HostVolumeStub, error)
There was a problem hiding this comment.
If you do that, the caller has to length-check the return value for (0, 1, many), which just repeats the logic we've done here and doesn't improve the ergonomics much.
Think of it not as nil, nil but as (obj, nil), nil or (nil, obj), nil. This return value is the same as what we do in all the other places in the CLI where we do prefix-matching to find a single object. (There's even a generic helper for this getByPrefix[T], but we can't use it here because the query function doesn't quite match the type param there.)
| if code != 0 { | ||
| return code | ||
| switch typeArg { | ||
| case "csi", "": |
There was a problem hiding this comment.
tangentially related display comment:
nomad volume status with no args currently shows
Container Storage Interface
No CSI volumes
the bold heading pushes me towards showing all (both) types of volumes with respective headers, unless a -type flag is provided to request a specific single type.
There was a problem hiding this comment.
Yeah I don't love how the "bare" version works... I thought I had a TODO here to refactor the whole way we've got volume status split out but I must've dropped it. Will leave a note for myself on that.
The `volume status :id` command outputs the namespace for a CSI volume twice. Drop the second output. Ref: #24382 (comment)
The `volume status :id` command outputs the namespace for a CSI volume twice. Drop the second output. Ref: #24382 (comment)
This changeset implements a first pass at the CLI for Dynamic Host Volumes. Ref: https://hashicorp.atlassian.net/browse/NET-11549
This changeset implements a first pass at the CLI for Dynamic Host Volumes. Ref: https://hashicorp.atlassian.net/browse/NET-11549
This changeset implements a first pass at the CLI for Dynamic Host Volumes. Ref: https://hashicorp.atlassian.net/browse/NET-11549
This changeset implements a first pass at the CLI for Dynamic Host Volumes. Ref: https://hashicorp.atlassian.net/browse/NET-11549
This changeset implements a first pass at the CLI for Dynamic Host Volumes. Ref: https://hashicorp.atlassian.net/browse/NET-11549
This changeset implements a first pass at the CLI for Dynamic Host Volumes. Ref: https://hashicorp.atlassian.net/browse/NET-11549
This changeset implements a first pass at the CLI for Dynamic Host Volumes. Ref: https://hashicorp.atlassian.net/browse/NET-11549
This changeset implements a first pass at the CLI for Dynamic Host Volumes.
Ref: https://hashicorp.atlassian.net/browse/NET-11549