HostVolumePlugin interface and two implementations #24497
Merged
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
approved these changes
Nov 20, 2024
| @@ -501,6 +501,7 @@ func (v *HostVolume) deleteVolume(vol *structs.HostVolume) error { | |||
| method := "ClientHostVolume.Delete" | |||
| cReq := &cstructs.ClientHostVolumeDeleteRequest{ | |||
| ID: vol.ID, | |||
| PluginID: vol.PluginID, | |||
There was a problem hiding this comment.
I suspect that once we have state tracking on the client we can slim down this RPC and drop PluginID and HostPath, but obviously fine for now.
There was a problem hiding this comment.
might need Parameters too. I can conceive of plugins that use the params in a call to some other system, or who knows.
* mkdir: HostVolumePluginMkdir: just creates a directory * example-host-volume: HostVolumePluginExternal: mkfs and mount loopback
send json PARAMETERS env var instead
gulducat
force-pushed
the
dhv-plugins
branch
from
4c68674 to
5725da4
Compare
gulducat
force-pushed
the
dhv-plugins
branch
from
5725da4 to
a85f06f
Compare
|
@gulducat if you want to push testing into it's own PR that feels reasonable for a large PR on something so "side-effect-y" but we should probably add the package to the list in |
|
I've started writing tests, but I'm also happy to open another PR for it, so we can all collectively use this purportedly-functional thing in the meantime 😜 |
tgross
added a commit
that referenced
this pull request
Nov 20, 2024
* mkdir: HostVolumePluginMkdir: just creates a directory * example-host-volume: HostVolumePluginExternal: plugin script that does mkfs and mount loopback Co-authored-by: Tim Gross <[email protected]>
tgross
added a commit
that referenced
this pull request
Dec 2, 2024
* mkdir: HostVolumePluginMkdir: just creates a directory * example-host-volume: HostVolumePluginExternal: plugin script that does mkfs and mount loopback Co-authored-by: Tim Gross <[email protected]>
tgross
added a commit
that referenced
this pull request
Dec 3, 2024
* mkdir: HostVolumePluginMkdir: just creates a directory * example-host-volume: HostVolumePluginExternal: plugin script that does mkfs and mount loopback Co-authored-by: Tim Gross <[email protected]>
tgross
added a commit
that referenced
this pull request
Dec 9, 2024
* mkdir: HostVolumePluginMkdir: just creates a directory * example-host-volume: HostVolumePluginExternal: plugin script that does mkfs and mount loopback Co-authored-by: Tim Gross <[email protected]>
tgross
added a commit
that referenced
this pull request
Dec 13, 2024
* mkdir: HostVolumePluginMkdir: just creates a directory * example-host-volume: HostVolumePluginExternal: plugin script that does mkfs and mount loopback Co-authored-by: Tim Gross <[email protected]>
gulducat
added a commit
that referenced
this pull request
Dec 18, 2024
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)
tgross
added a commit
that referenced
this pull request
Dec 19, 2024
* mkdir: HostVolumePluginMkdir: just creates a directory * example-host-volume: HostVolumePluginExternal: plugin script that does mkfs and mount loopback Co-authored-by: Tim Gross <[email protected]>
tgross
pushed a commit
that referenced
this pull request
Dec 19, 2024
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)
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.
This is functional end-to-end from CLI to Client, provided you place the
example-host-volumeplugin script in/opt/nomad/hostvolumeplugins/on the client node, or use the"mkdir"internal plugin.I have not tried this on MacOS or Windows, only my Linux laptop. Perhaps @pkazmierczak can test it on Mac for us? 🙂
There are many TODOs around, and no unit tests are present. Testing what's here is up next on my todo list, provided the overall shape of the interface here looks good to y'all.
Ref: #24479
The
_test-plugin.shscript was convenient for me sprucing up the example plugin, and gave me two ideas for future:nomadCLI subcommand might be neat to include, to ease plugin development for users_(or.or something) for library files