tl;dr - runtime code is fine but tests should match reality
The Nomad Client Agent is the only consumer of the
`Node.Derive{SI,Vault}Token` RPCs, therefore tests of the RPCs should
match Nomad Client behavior.
- DeriveVaultToken code: a9ee66a6ef/client/client.go (L2904-L2917)
- DeriveSIToken code: a9ee66a6ef/client/client.go (L2988-L2997)
Both of those client code paths include the Node SecretID in both the
request's SecretID field as well as the embedded
`QueryOptions.AuthToken` field.
This patch updates server tests to match that behavior. The tests pass
either way.
When the Nomad client restarts and restores allocations, the network namespace
for an allocation may exist but no longer be correctly configured. For example,
if the host is rebooted and the task was a Docker task using a pause container,
the network namespace may be recreated by the docker daemon.
When we restore an allocation, use the CNI "check" command to verify that any
existing network namespace matches the expected configuration. This requires CNI
plugins of at least version 1.2.0 to avoid a bug in older plugin versions that
would cause the check to fail.
If the check fails, destroy the network namespace and try to recreate it from
scratch once. If that fails in the second pass, fail the restore so that the
allocation can be recreated (rather than silently having networking fail).
This should fix the gap left #24650 for Docker task drivers and any other
drivers with the `MustInitiateNetwork` capability.
Fixes: https://github.com/hashicorp/nomad/issues/24292
Ref: https://github.com/hashicorp/nomad/pull/24650
Adds an additional check in the Keyring.Delete RPC to make sure we're not
trying to delete a key that's been used to encrypt a variable. It also adds a
-force flag for the CLI/API to sidestep that check.
The recent change to collection via a "one-shot" Docker API call
did not update the stream boolean argument. This results in the
PreCPUStats values being zero and therefore breaking the CPU
calculations which rely on this data. The base fix is to update
the passed boolean parameter to match the desired non-streaming
behaviour. The non-streaming API call correctly returns the
PreCPUStats data which can be seen in the added unit test.
The most recent change also modified the behaviour of the
collectStats go routine, so that any error encountered results in
the routine exiting. In the event this was a transient error, the
container will continue to run, however, no stats will be collected
until the task is stopped and replaced. This PR reverts the
behaviour, so that an error encountered during a stats collection
run results in the error being logged but the collection process
continuing with a backoff used.
a node can have only one volume with a given name.
the scheduler prevents duplicates, but can only
do so after the server knows about the volume.
this prevents multiple concurrent creates being
called faster than the fingerprint/heartbeat interval.
users may still modify an existing volume only
if they set the `id` in the volume spec and
re-issue `nomad volume create`
if a *static* vol is added to config with a name
already being used by a dynamic volume, the
dynamic takes precedence, but log a warning.
Adds new topics to the event stream for CSI volumes and CSI plugins. We'll emit
event when either is created or deleted, and when CSI volumes are claimed.
Add a new topic to the event stream for host volumes. We'll emit events when a
dynamic host volume is registered or deregistered, and whenever a node
fingerprints with a changed volume.
Ref: https://hashicorp.atlassian.net/browse/NET-11549
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: https://github.com/hashicorp/nomad/pull/24479
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)
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: https://github.com/hashicorp/nomad/pull/24479
Ref: https://github.com/hashicorp/nomad/pull/24684
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: https://github.com/hashicorp/nomad/pull/24479
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: https://github.com/hashicorp/nomad/pull/24479
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: https://github.com/hashicorp/nomad/pull/24479
Node fingerprints include attributes for the host volume plugins, including the
built-in plugins. Add an implicit constraint on this fingerprint during volume
placement to ensure we only place volumes on hosts with the right plugins.
Ref: https://github.com/hashicorp/nomad/pull/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.
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: https://github.com/hashicorp/nomad-enterprise/pull/2087
Ref: https://github.com/hashicorp/nomad/pull/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: https://github.com/hashicorp/nomad/pull/24479
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: https://github.com/hashicorp/nomad/pull/24479
