diff --git a/client/fingerprint/dynamic_host_volumes.go b/client/fingerprint/dynamic_host_volumes.go index e15a3a8f0c6..73cdd4fe4a2 100644 --- a/client/fingerprint/dynamic_host_volumes.go +++ b/client/fingerprint/dynamic_host_volumes.go @@ -35,7 +35,7 @@ func (h *DynamicHostVolumePluginFingerprint) Fingerprint(request *FingerprintReq // always add "mkdir" plugin h.logger.Debug("detected plugin built-in", "plugin_id", hvm.HostVolumePluginMkdirID, "version", hvm.HostVolumePluginMkdirVersion) - defer response.AddAttribute("plugins.host_volume.version."+hvm.HostVolumePluginMkdirID, hvm.HostVolumePluginMkdirVersion) + defer response.AddAttribute("plugins.host_volume."+hvm.HostVolumePluginMkdirID+".version", hvm.HostVolumePluginMkdirVersion) response.Detected = true // this config value will be empty in -dev mode @@ -64,7 +64,7 @@ func (h *DynamicHostVolumePluginFingerprint) Fingerprint(request *FingerprintReq // set the attribute(s) for plugin, version := range plugins { h.logger.Debug("detected plugin", "plugin_id", plugin, "version", version) - response.AddAttribute("plugins.host_volume.version."+plugin, version) + response.AddAttribute("plugins.host_volume."+plugin+".version", version) } return nil @@ -103,14 +103,14 @@ func GetHostVolumePluginVersions(log hclog.Logger, pluginDir string) (map[string return } - version, err := p.Version(ctx) + fprint, err := p.Fingerprint(ctx) if err != nil { log.Debug("failed to get version from plugin", "error", err) return } mut.Lock() - plugins[file] = version.String() + plugins[file] = fprint.Version.String() mut.Unlock() }(file, fullPath) } diff --git a/client/fingerprint/dynamic_host_volumes_test.go b/client/fingerprint/dynamic_host_volumes_test.go index 4be9d69d305..28b331bcfc1 100644 --- a/client/fingerprint/dynamic_host_volumes_test.go +++ b/client/fingerprint/dynamic_host_volumes_test.go @@ -53,10 +53,10 @@ func TestPluginsHostVolumeFingerprint(t *testing.T) { perm os.FileMode }{ // only this first one should be detected as a valid plugin - {"happy-plugin", "#!/usr/bin/env sh\necho '0.0.1'", 0700}, - {"not-a-plugin", "#!/usr/bin/env sh\necho 'not-a-version'", 0700}, - {"unhappy-plugin", "#!/usr/bin/env sh\necho '0.0.2'; exit 1", 0700}, - {"not-executable", "hello", 0400}, + {"happy-plugin", "#!/usr/bin/env sh\necho '{\"version\": \"0.0.1\"}'", 0700}, + {"not-a-plugin", "#!/usr/bin/env sh\necho 'not a version'", 0700}, + {"unhappy-plugin", "#!/usr/bin/env sh\necho 'sad plugin is sad'; exit 1", 0700}, + {"not-executable", "do not execute me", 0400}, } for _, f := range files { must.NoError(t, os.WriteFile(filepath.Join(tmp, f.name), []byte(f.contents), f.perm)) @@ -69,8 +69,8 @@ func TestPluginsHostVolumeFingerprint(t *testing.T) { err := fp.Fingerprint(req, &resp) must.NoError(t, err) must.Eq(t, map[string]string{ - "plugins.host_volume.version.happy-plugin": "0.0.1", - "plugins.host_volume.version.mkdir": hvm.HostVolumePluginMkdirVersion, // built-in + "plugins.host_volume.mkdir.version": hvm.HostVolumePluginMkdirVersion, // built-in + "plugins.host_volume.happy-plugin.version": "0.0.1", }, resp.Attributes) // do it again after deleting our one good plugin. @@ -82,8 +82,8 @@ func TestPluginsHostVolumeFingerprint(t *testing.T) { err = fp.Fingerprint(req, &resp) must.NoError(t, err) must.Eq(t, map[string]string{ - "plugins.host_volume.version.happy-plugin": "", // empty value means removed + "plugins.host_volume.happy-plugin.version": "", // empty value means removed - "plugins.host_volume.version.mkdir": hvm.HostVolumePluginMkdirVersion, // built-in + "plugins.host_volume.mkdir.version": hvm.HostVolumePluginMkdirVersion, // built-in }, resp.Attributes) } diff --git a/client/hostvolumemanager/host_volume_plugin.go b/client/hostvolumemanager/host_volume_plugin.go index 7da5baf66d0..961466b1223 100644 --- a/client/hostvolumemanager/host_volume_plugin.go +++ b/client/hostvolumemanager/host_volume_plugin.go @@ -12,7 +12,6 @@ import ( "os" "os/exec" "path/filepath" - "strings" "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-multierror" @@ -21,8 +20,12 @@ import ( "github.com/hashicorp/nomad/helper" ) +type PluginFingerprint struct { + Version *version.Version `json:"version"` +} + type HostVolumePlugin interface { - Version(ctx context.Context) (*version.Version, error) + Fingerprint(ctx context.Context) (*PluginFingerprint, error) Create(ctx context.Context, req *cstructs.ClientHostVolumeCreateRequest) (*HostVolumePluginCreateResponse, error) Delete(ctx context.Context, req *cstructs.ClientHostVolumeDeleteRequest) error // db TODO(1.10.0): update? resize? ?? @@ -45,8 +48,11 @@ type HostVolumePluginMkdir struct { log hclog.Logger } -func (p *HostVolumePluginMkdir) Version(_ context.Context) (*version.Version, error) { - return version.NewVersion(HostVolumePluginMkdirVersion) +func (p *HostVolumePluginMkdir) Fingerprint(_ context.Context) (*PluginFingerprint, error) { + v, err := version.NewVersion(HostVolumePluginMkdirVersion) + return &PluginFingerprint{ + Version: v, + }, err } func (p *HostVolumePluginMkdir) Create(_ context.Context, @@ -134,9 +140,9 @@ type HostVolumePluginExternal struct { log hclog.Logger } -func (p *HostVolumePluginExternal) Version(ctx context.Context) (*version.Version, error) { - cmd := exec.CommandContext(ctx, p.Executable, "version") - cmd.Env = []string{"OPERATION=version"} +func (p *HostVolumePluginExternal) Fingerprint(ctx context.Context) (*PluginFingerprint, error) { + cmd := exec.CommandContext(ctx, p.Executable, "fingerprint") + cmd.Env = []string{"OPERATION=fingerprint"} stdout, stderr, err := runCommand(cmd) if err != nil { p.log.Debug("error with plugin", @@ -146,11 +152,11 @@ func (p *HostVolumePluginExternal) Version(ctx context.Context) (*version.Versio "error", err) return nil, fmt.Errorf("error getting version from plugin %q: %w", p.ID, err) } - v, err := version.NewVersion(strings.TrimSpace(string(stdout))) - if err != nil { - return nil, fmt.Errorf("error with version from plugin: %w", err) + fprint := &PluginFingerprint{} + if err := json.Unmarshal(stdout, fprint); err != nil { + return nil, fmt.Errorf("error parsing fingerprint output as json: %w", err) } - return v, nil + return fprint, nil } func (p *HostVolumePluginExternal) Create(ctx context.Context, diff --git a/client/hostvolumemanager/host_volume_plugin_test.go b/client/hostvolumemanager/host_volume_plugin_test.go index 18de2e1f381..0552810bb84 100644 --- a/client/hostvolumemanager/host_volume_plugin_test.go +++ b/client/hostvolumemanager/host_volume_plugin_test.go @@ -33,7 +33,7 @@ func TestHostVolumePluginMkdir(t *testing.T) { // contexts don't matter here, since they're thrown away by this plugin, // but sending timeout contexts anyway, in case the plugin changes later. - _, err := plug.Version(timeout(t)) + _, err := plug.Fingerprint(timeout(t)) must.NoError(t, err) t.Run("happy", func(t *testing.T) { @@ -97,9 +97,9 @@ func TestHostVolumePluginExternal(t *testing.T) { log: log, } - v, err := plug.Version(timeout(t)) + v, err := plug.Fingerprint(timeout(t)) must.NoError(t, err) - must.Eq(t, expectVersion, v) + must.Eq(t, expectVersion, v.Version) resp, err := plug.Create(timeout(t), &cstructs.ClientHostVolumeCreateRequest{ @@ -147,12 +147,12 @@ func TestHostVolumePluginExternal(t *testing.T) { log: log, } - v, err := plug.Version(timeout(t)) + v, err := plug.Fingerprint(timeout(t)) must.EqError(t, err, `error getting version from plugin "test-external-plugin-sad": exit status 1`) must.Nil(t, v) logged := getLogs() - must.StrContains(t, logged, "version: sad plugin is sad") - must.StrContains(t, logged, "version: it tells you all about it in stderr") + must.StrContains(t, logged, "fingerprint: sad plugin is sad") + must.StrContains(t, logged, "fingerprint: it tells you all about it in stderr") // reset logger log, getLogs = logRecorder(t) diff --git a/client/hostvolumemanager/test_fixtures/test_plugin.sh b/client/hostvolumemanager/test_fixtures/test_plugin.sh index b60229fd34d..e93e37bbc76 100755 --- a/client/hostvolumemanager/test_fixtures/test_plugin.sh +++ b/client/hostvolumemanager/test_fixtures/test_plugin.sh @@ -12,6 +12,8 @@ test "$1" == "$OPERATION" echo 'all operations should ignore stderr' 1>&2 case $1 in + fingerprint) + echo '{"version": "0.0.2"}' ;; create) test "$2" == "$HOST_PATH" test "$NODE_ID" == 'test-node' @@ -26,8 +28,6 @@ case $1 in test "$NODE_ID" == 'test-node' test "$PARAMETERS" == '{"key":"val"}' rm -rfv "$2" ;; - version) - echo '0.0.2' ;; *) echo "unknown operation $1" exit 1 ;; diff --git a/demo/hostvolume/_test-plugin.sh b/demo/hostvolume/_test-plugin.sh index 5ccd1f28a72..864680e237d 100755 --- a/demo/hostvolume/_test-plugin.sh +++ b/demo/hostvolume/_test-plugin.sh @@ -4,35 +4,48 @@ set -euo pipefail -if [[ $# -eq 0 || "$*" =~ -h ]]; then +help() { cat < + $0 [target dir] [uuid] -Operations: - create, delete, version - any other operation will be passed to the plugin +Args: + plugin: path to plugin executable + operation: fingerprint, create, or delete + create and delete must be idempotent. + any other operation will be passed into the plugin, + to see how it handles invalid operations. + target dir: directory to create the volume (defaults to /tmp) + uuid: volume id to use (usually assigned by Nomad; + defaults to 74564d17-ce50-0bc1-48e5-6feaa41ede48) -Environment variables: - PLUGIN: executable to run (default ./example-host-volume) - TARGET_DIR: path to place the mount dir (default /tmp, - usually {nomad data dir}/alloc_mounts) +Examples: + $0 ./example-plugin-mkfs fingerprint + $0 ./example-plugin-mkfs create + $0 ./example-plugin-mkfs create /some/other/place + $0 ./example-plugin-mkfs delete EOF +} + +if [[ $# -eq 0 || "$*" =~ -h ]]; then + help exit fi +if [ $# -lt 2 ]; then + help + exit 1 +fi -op="$1" -shift - -plugin="${PLUGIN:-./example-host-volume}" -alloc_mounts="${TARGET_DIR:-/tmp}" -uuid='74564d17-ce50-0bc1-48e5-6feaa41ede48' +plugin="$1" +op="$2" +alloc_mounts="${3:-/tmp}" +uuid="${4:-74564d17-ce50-0bc1-48e5-6feaa41ede48}" case $op in - version) - args='version' + fingerprint) + args='fingerprint' ;; create) @@ -59,4 +72,4 @@ esac export OPERATION="$op" set -x -eval "$plugin $* $args" +eval "$plugin $args" diff --git a/demo/hostvolume/example-plugin-mkfs b/demo/hostvolume/example-plugin-mkfs index 918f97748fb..5bfaa4e47fa 100755 --- a/demo/hostvolume/example-plugin-mkfs +++ b/demo/hostvolume/example-plugin-mkfs @@ -6,6 +6,11 @@ set -euo pipefail +version='0.0.1' +fingerprint() { + printf '{"version": "%s"}' "$version" +} + help() { cat < [path] + $(basename "$0") [options] [path] Options: -v|--verbose: Show shell commands (set -x) @@ -25,21 +30,19 @@ Operations: required environment: CAPACITY_MIN_BYTES delete: Unmounts and deletes the device at path (required) - version: Outputs this plugin's version + version: Outputs this plugin's version: $version + fingerprint: Outputs plugin metadata: $(fingerprint) EOF } -version() { - echo "0.0.1" -} - # parse args [ $# -eq 0 ] && { help; exit 1; } for arg in "$@"; do case $arg in -h|-help|--help) help; exit 0 ;; - version|--version) version; exit 0 ;; + fingerprint|fingerprint) fingerprint; exit 0 ;; + version|version) echo "$version"; exit 0 ;; -v|--verbose) set -x; shift; ;; esac done diff --git a/nomad/host_volume_endpoint.go b/nomad/host_volume_endpoint.go index 8f085416826..e33b0e8a42b 100644 --- a/nomad/host_volume_endpoint.go +++ b/nomad/host_volume_endpoint.go @@ -485,7 +485,7 @@ func (v *HostVolume) placeHostVolume(snap *state.StateSnapshot, vol *structs.Hos semverCache: make(map[string]scheduler.VerConstraints), } constraints := []*structs.Constraint{{ - LTarget: fmt.Sprintf("${attr.plugins.host_volume.version.%s}", vol.PluginID), + LTarget: fmt.Sprintf("${attr.plugins.host_volume.%s.version}", vol.PluginID), Operand: "is_set", }} constraints = append(constraints, vol.Constraints...) diff --git a/nomad/host_volume_endpoint_test.go b/nomad/host_volume_endpoint_test.go index 9ca0f94855b..e523ae29ae3 100644 --- a/nomad/host_volume_endpoint_test.go +++ b/nomad/host_volume_endpoint_test.go @@ -666,14 +666,14 @@ func TestHostVolumeEndpoint_placeVolume(t *testing.T) { node0, node1, node2, node3 := mock.Node(), mock.Node(), mock.Node(), mock.Node() node0.NodePool = structs.NodePoolDefault - node0.Attributes["plugins.host_volume.version.mkdir"] = "0.0.1" + node0.Attributes["plugins.host_volume.mkdir.version"] = "0.0.1" node1.NodePool = "dev" node1.Meta["rack"] = "r2" - node1.Attributes["plugins.host_volume.version.mkdir"] = "0.0.1" + node1.Attributes["plugins.host_volume.mkdir.version"] = "0.0.1" node2.NodePool = "prod" - node2.Attributes["plugins.host_volume.version.mkdir"] = "0.0.1" + node2.Attributes["plugins.host_volume.mkdir.version"] = "0.0.1" node3.NodePool = "prod" node3.Meta["rack"] = "r3" @@ -681,7 +681,7 @@ func TestHostVolumeEndpoint_placeVolume(t *testing.T) { Name: "example", Path: "/srv", }} - node3.Attributes["plugins.host_volume.version.mkdir"] = "0.0.1" + node3.Attributes["plugins.host_volume.mkdir.version"] = "0.0.1" must.NoError(t, store.UpsertNode(structs.MsgTypeTestSetup, 1000, node0)) must.NoError(t, store.UpsertNode(structs.MsgTypeTestSetup, 1000, node1)) @@ -785,7 +785,7 @@ func newMockHostVolumeClient(t *testing.T, srv *Server, pool string) (*mockHostV c1, cleanup := client.TestRPCOnlyClient(t, func(c *config.Config) { c.Node.NodePool = pool c.Node.Attributes["nomad.version"] = version.Version - c.Node.Attributes["plugins.host_volume.version.mkdir"] = "0.0.1" + c.Node.Attributes["plugins.host_volume.mkdir.version"] = "0.0.1" c.Node.Meta["rack"] = "r1" }, srv.config.RPCAddr, map[string]any{"HostVolume": mockClientEndpoint}) t.Cleanup(cleanup)