Dynamic Host Volumes #24479
Merged
Dynamic Host Volumes #24479
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tgross
force-pushed
the
dynamic-host-volumes
branch
from
a3784ca to
83de356
Compare
tgross
added a commit
that referenced
this pull request
Nov 20, 2024
When making a request to create a dynamic host volumes, users can pass a node pool and constraints instead of a specific node ID. This changeset implements a node scheduling logic by instantiating a filter by node pool and constraint checker borrowed from the scheduler package. Because host volumes with the same name can't land on the same host, we don't need to support `distinct_hosts`/`distinct_property`; this would be challenging anyways without building out a much larger node iteration mechanism to keep track of usage across multiple hosts. Ref: #24479
tgross
added a commit
that referenced
this pull request
Nov 21, 2024
When making a request to create a dynamic host volumes, users can pass a node pool and constraints instead of a specific node ID. This changeset implements a node scheduling logic by instantiating a filter by node pool and constraint checker borrowed from the scheduler package. Because host volumes with the same name can't land on the same host, we don't need to support `distinct_hosts`/`distinct_property`; this would be challenging anyways without building out a much larger node iteration mechanism to keep track of usage across multiple hosts. Ref: #24479
tgross
added a commit
that referenced
this pull request
Nov 21, 2024
When dynamic host volumes are created, they're written to the state store in a "pending" state. Once the client fingerprints the volume it's eligible for scheduling, so we mark the state as ready at that point. Because the fingerprint could potentially be returned before the RPC handler has a chance to write to the state store, this changeset adds test coverage to verify that upserts of pending volumes check the node for a previously-fingerprinted volume as well. Ref: #24479
tgross
added a commit
that referenced
this pull request
Nov 21, 2024
When dynamic host volumes are created, they're written to the state store in a "pending" state. Once the client fingerprints the volume it's eligible for scheduling, so we mark the state as ready at that point. Because the fingerprint could potentially be returned before the RPC handler has a chance to write to the state store, this changeset adds test coverage to verify that upserts of pending volumes check the node for a previously-fingerprinted volume as well. Ref: #24479
When creating a dynamic host volumes, set up an optional monitor that waits for the node to fingerprint the volume as healthy. Ref: #24479
Add support for dynamic host volumes to the search endpoint. Like many other objects with UUID identifiers, we're not supporting fuzzy search here, just prefix search on the fuzzy search endpoint. Because the search endpoint only returns IDs, we need to seperate CSI volumes and host volumes for it to be useful. The new context is called `"host_volumes"` to disambiguate it from `"volumes"`. In future versions of Nomad we should consider deprecating the `"volumes"` context in lieu of a `"csi_volumes"` context. Ref: #24479
Most Nomad upsert RPCs accept a single object with the notable exception of CSI. But in CSI we don't actually expose this to users except through the Go API. It deeply complicates how we present errors to users, especially once Sentinel policy enforcement enters the mix. Refactor the `HostVolume.Create` and `HostVolume.Register` RPCs to take a single volume instead of a slice of volumes. Add a stub function for Enterprise policy enforcement. This requires splitting out placement from the `createVolume` function so that we can ensure we've completed placement before trying to enforce policy. Ref: #24479
Adds dynamic host volumes to argument autocomplete for the `volume status` and `volume delete` commands. Adds flag autocompletion for those commands plus `volume create`. Ref: #24479
…24563) This adapts the shell script for darwin, making it easier to test.
…24586) In #24528 we added monitoring to the CLI for dynamic host volume creation. But when the volume's namespace is set by the volume specification instead of the `-namespace` flag, the API client doesn't have the right namespace and gets a 404 when setting up the monitoring. The specification always overrides the `-namespace` flag, so use that when available for all subsequent API calls. Ref: #24479
The create/register volume RPCs support a policy override flag for soft-mandatory Sentinel policies, but the CLI and Go API were missing support for it. Also add support for Sentinel warnings to the Go API and CLI. Ref: #24479
When we add a Sentinel scope for dynamic host volumes, having a default `-scope` value for `sentinel apply` risks accidentally adding policies for volumes to the job scope. This would immediately prevent any job from being submitted. Forcing the administrator to pass a `-scope` will prevent accidental misuse. Ref: hashicorp/nomad-enterprise#2087 Ref: #24479
store dynamic host volume creations in client state, so they can be "restored" on agent restart. restore works by repeating the same Create operation as initial creation, and expecting the plugin to be idempotent. this is (potentially) especially important after host restarts, which may have dropped mount points or such.
string instead of []string
…24612) The List Volumes API was originally written for CSI but assumed we'd have future volume types, dispatched on a query parameter. Dynamic host volumes uses this, but the resulting code has host volumes concerns comingled in the CSI volumes endpoint. Refactor this so that we have a top-level `GET /v1/volumes` route that's shared between CSI and DHV, and have it dispatch to the appropriate handler in the type-specific endpoints. Ref: #24479
and expand the demo a bit
Static host volumes have a simple readonly toggle, but dynamic host volumes have a more complex set of capabilities similar to CSI volumes. Update the feasibility checker to account for these capabilities and volume readiness. Also fixes a minor bug in the state store where a soft-delete (not yet implemented) could cause a volume to be marked ready again. This is needed to support testing the readiness checking in the scheduler. Ref: #24479
Adds a `-type` flag to the `volume init` command that generates an example volume specification with only those fields relevant to dynamic host volumes. This changeset also moves the string literals into uses of `go:embed` Ref: #24479
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
Instead of a plugin `version` subcommand that responds with a string (established in #24497), respond to a `fingerprint` command with a data structure that we may extend in the future (such as plugin capabilities, like size constraint support?). In the immediate term, it's still just the version: `{"version": "0.0.1"}` In addition to leaving the door open for future expansion, I think it will also avoid false positives detecting executables that just happen to respond to a `version` command. This also reverses the ordering of the fingerprint string parts from `plugins.host_volume.version.mkdir` (which aligned with CNI) to `plugins.host_volume.mkdir.version` (makes more sense to me)
…24684) When we feasibility check a dynamic host volume against a volume request, we check the attachment mode and access mode. This only ensures that the capabilities match, but doesn't enforce the semantics of the capabilities against other claims that may be made on the allocation. Add support for checking the requested capability against other allocations that the volume claimed. Ref: #24479
tgross
force-pushed
the
dynamic-host-volumes
branch
from
6ea52e4 to
ad1e597
Compare
|
Ready for merge. All commits on this branch have been reviewed under other PRs. |
philrenaud
approved these changes
Dec 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feature integration branch for dynamic host volumes.
Closes: #15489
Ref: https://hashicorp.atlassian.net/browse/NET-11259