address pr comments

client/fingerprint/dynamic_host_volumes.go

@@ -33,8 +33,9 @@ func (h *DynamicHostVolumePluginFingerprint) Reload() {
 
 func (h *DynamicHostVolumePluginFingerprint) Fingerprint(request *FingerprintRequest, response *FingerprintResponse) error {
 	// always add "mkdir" plugin
-	h.logger.Debug("detected plugin built-in", "plugin_id", "mkdir", "version", hvm.HostVolumePluginMkdirVersion)
-	defer response.AddAttribute("plugins.dhv.version.mkdir", hvm.HostVolumePluginMkdirVersion)
+	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)
 	response.Detected = true
 
 	// this config value will be empty in -dev mode
@@ -55,22 +56,22 @@ func (h *DynamicHostVolumePluginFingerprint) Fingerprint(request *FingerprintReq
 
 	// if this was a reload, wipe what was there before
 	for k := range request.Node.Attributes {
-		if strings.HasPrefix(k, "plugins.dhv.") {
+		if strings.HasPrefix(k, "plugins.host_volume.") {
 			response.RemoveAttribute(k)
 		}
 	}
 
 	// set the attribute(s)
 	for plugin, version := range plugins {
 		h.logger.Debug("detected plugin", "plugin_id", plugin, "version", version)
-		response.AddAttribute("plugins.dhv.version."+plugin, version)
+		response.AddAttribute("plugins.host_volume.version."+plugin, version)
 	}
 
 	return nil
 }
 
 func (h *DynamicHostVolumePluginFingerprint) Periodic() (bool, time.Duration) {
-	return false, 0 // not periodic;  db TODO(1.10.0): could be?
+	return false, 0
 }
 
 // GetHostVolumePluginVersions finds all the executable files on disk
@@ -94,11 +95,11 @@ func GetHostVolumePluginVersions(log hclog.Logger, pluginDir string) (map[string
 			ctx, cancel := context.WithTimeout(context.Background(), time.Second)
 			defer cancel()
 
-			log := log.With("plugin_id", file) // db TODO: "name" like CNI, instead of "ID" ?
+			log := log.With("plugin_id", file)
 
 			p, err := hvm.NewHostVolumePluginExternal(log, file, fullPath, "")
 			if err != nil {
-				log.Debug("error getting plugin", "error", err)
+				log.Warn("error getting plugin", "error", err)
 				return
 			}
 

client/fingerprint/dynamic_host_volumes_test.go

@@ -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.dhv.version.happy-plugin": "0.0.1",
-		"plugins.dhv.version.mkdir":        hvm.HostVolumePluginMkdirVersion, // built-in
+		"plugins.host_volume.version.happy-plugin": "0.0.1",
+		"plugins.host_volume.version.mkdir":        hvm.HostVolumePluginMkdirVersion, // built-in
 	}, 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.dhv.version.happy-plugin": "", // will get cleaned up later
+		"plugins.host_volume.version.happy-plugin": "", // empty value means removed
 
-		"plugins.dhv.version.mkdir": hvm.HostVolumePluginMkdirVersion, // built-in
+		"plugins.host_volume.version.mkdir": hvm.HostVolumePluginMkdirVersion, // built-in
 	}, resp.Attributes)
 }

client/hostvolumemanager/host_volume_plugin.go

@@ -34,6 +34,7 @@ type HostVolumePluginCreateResponse struct {
 	Context   map[string]string `json:"context"` // metadata
 }
 
+const HostVolumePluginMkdirID = "mkdir"
 const HostVolumePluginMkdirVersion = "0.0.1"
 
 var _ HostVolumePlugin = &HostVolumePluginMkdir{}

client/hostvolumemanager/host_volumes.go

@@ -38,9 +38,9 @@ func NewHostVolumeManager(logger hclog.Logger, pluginDir, sharedMountDir string)
 func (hvm *HostVolumeManager) getPlugin(id string) (HostVolumePlugin, error) {
 	log := hvm.log.With("plugin_id", id)
 
-	if id == "mkdir" { // db TODO: constant?
+	if id == HostVolumePluginMkdirID {
 		return &HostVolumePluginMkdir{
-			ID:         "mkdir",
+			ID:         HostVolumePluginMkdirID,
 			TargetPath: hvm.sharedMountDir,
 			log:        log,
 		}, nil

command/agent/agent.go

@@ -726,7 +726,7 @@ func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) {
 		conf.AllocDir = filepath.Join(agentConfig.DataDir, "alloc")
 		dataParent := filepath.Dir(agentConfig.DataDir)
 		conf.AllocMountsDir = filepath.Join(dataParent, "alloc_mounts")
-		conf.DynamicHostVolumePluginPath = filepath.Join(dataParent, "plugins-dhv")
+		conf.DynamicHostVolumePluginPath = filepath.Join(dataParent, "plugins-host-volume")
 	}
 	if agentConfig.Client.StateDir != "" {
 		conf.StateDir = agentConfig.Client.StateDir
@@ -737,8 +737,8 @@ func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) {
 	if agentConfig.Client.AllocMountsDir != "" {
 		conf.AllocMountsDir = agentConfig.Client.AllocMountsDir
 	}
-	if agentConfig.Client.DHVPluginDir != "" {
-		conf.DynamicHostVolumePluginPath = agentConfig.Client.DHVPluginDir
+	if agentConfig.Client.HostVolumePluginDir != "" {
+		conf.DynamicHostVolumePluginPath = agentConfig.Client.HostVolumePluginDir
 	}
 	if agentConfig.Client.NetworkInterface != "" {
 		conf.NetworkInterface = agentConfig.Client.NetworkInterface

command/agent/command.go

@@ -111,7 +111,7 @@ func (c *Command) readConfig() *Config {
 	flags.StringVar(&cmdConfig.Client.StateDir, "state-dir", "", "")
 	flags.StringVar(&cmdConfig.Client.AllocDir, "alloc-dir", "", "")
 	flags.StringVar(&cmdConfig.Client.AllocMountsDir, "alloc-mounts-dir", "", "")
-	flags.StringVar(&cmdConfig.Client.DHVPluginDir, "dhv-plugin-dir", "", "")
+	flags.StringVar(&cmdConfig.Client.HostVolumePluginDir, "host-volume-plugin-dir", "", "")
 	flags.StringVar(&cmdConfig.Client.NodeClass, "node-class", "", "")
 	flags.StringVar(&cmdConfig.Client.NodePool, "node-pool", "", "")
 	flags.StringVar(&servers, "servers", "", "")
@@ -385,12 +385,12 @@ func (c *Command) IsValidConfig(config, cmdConfig *Config) bool {
 
 	// Verify the paths are absolute.
 	dirs := map[string]string{
-		"data-dir":         config.DataDir,
-		"plugin-dir":       config.PluginDir,
-		"alloc-dir":        config.Client.AllocDir,
-		"alloc-mounts-dir": config.Client.AllocMountsDir,
-		"dhv-plugin-dir":   config.Client.DHVPluginDir,
-		"state-dir":        config.Client.StateDir,
+		"data-dir":               config.DataDir,
+		"plugin-dir":             config.PluginDir,
+		"alloc-dir":              config.Client.AllocDir,
+		"alloc-mounts-dir":       config.Client.AllocMountsDir,
+		"host-volume-plugin-dir": config.Client.HostVolumePluginDir,
+		"state-dir":              config.Client.StateDir,
 	}
 	for k, dir := range dirs {
 		if dir == "" {
@@ -737,6 +737,7 @@ func (c *Command) AutocompleteFlags() complete.Flags {
 		"-region":                      complete.PredictAnything,
 		"-data-dir":                    complete.PredictDirs("*"),
 		"-plugin-dir":                  complete.PredictDirs("*"),
+		"-host-volume-plugin-dir":      complete.PredictDirs("*"),
 		"-dc":                          complete.PredictAnything,
 		"-log-level":                   complete.PredictAnything,
 		"-json-logs":                   complete.PredictNothing,
@@ -1570,9 +1571,9 @@ Client Options:
     The default speed for network interfaces in MBits if the link speed can not
     be determined dynamically.
 
-  -dhv-plugin-dir
-    Directory containing dynamic host volume plugins. Defaults to "plugins-dhv"
-    next to "-state-dir".
+  -host-volume-plugin-dir
+    Directory containing dynamic host volume plugins. The default is
+    "plugins-host-volume" next to "-state-dir".
 
 ACL Options:
 

command/agent/config.go

@@ -229,9 +229,9 @@ type ClientConfig struct {
 	// AllocMountsDir is the directory for storing mounts into allocation data
 	AllocMountsDir string `hcl:"alloc_mounts_dir"`
 
-	// DHVPluginDir is the directory containing dynamic host volume plugins
+	// HostVolumePluginDir directory contains dynamic host volume plugins
 	// db TODO(1.10.0): document default directory is alongside alloc_mounts
-	DHVPluginDir string `hcl:"dhv_plugin_dir"` // db TODO(1.10.0): is this a good name?
+	HostVolumePluginDir string `hcl:"host_volume_plugin_dir"`
 
 	// Servers is a list of known server addresses. These are as "host:port"
 	Servers []string `hcl:"servers"`
@@ -2315,8 +2315,8 @@ func (a *ClientConfig) Merge(b *ClientConfig) *ClientConfig {
 	if b.AllocMountsDir != "" {
 		result.AllocMountsDir = b.AllocMountsDir
 	}
-	if b.DHVPluginDir != "" {
-		result.DHVPluginDir = b.DHVPluginDir
+	if b.HostVolumePluginDir != "" {
+		result.HostVolumePluginDir = b.HostVolumePluginDir
 	}
 	if b.NodeClass != "" {
 		result.NodeClass = b.NodeClass