volume status: default type to show both DHV and CSI volumes (#25185)

The `-type` option for `volume status` is a UX papercut because for many
clusters there will be only one sort of volume in use. Update the CLI so that
the default behavior is to query CSI and/or DHV.

This behavior is subtly different when the user provides an ID or not. If the
user doesn't provide an ID, we query both CSI and DHV and show both tables. If
the user provides an ID, we query DHV first and then CSI, and show only the
appropriate volume. Because DHV IDs are UUIDs, we're sure we won't have
collisions between the two. We only show errors if both queries return an error.

Fixes: https://hashicorp.atlassian.net/browse/NET-12214

command/helpers.go

@@ -753,11 +753,11 @@ func getByPrefix[T any](
 ) (*T, []*T, error) {
 	objs, _, err := queryFn(opts)
 	if err != nil {
-		return nil, nil, fmt.Errorf("Error querying %s: %s", objName, err)
+		return nil, nil, fmt.Errorf("error querying %s: %s", objName, err)
 	}
 	switch len(objs) {
 	case 0:
-		return nil, nil, fmt.Errorf("No %s with prefix or ID %q found", objName, opts.Prefix)
+		return nil, nil, fmt.Errorf("no %s with prefix or ID %q found", objName, opts.Prefix)
 	case 1:
 		return objs[0], nil, nil
 	default:

command/helpers_test.go

@@ -673,7 +673,7 @@ func TestHelperGetByPrefix(t *testing.T) {
 		{
 			name:      "query error",
 			queryErr:  errors.New("foo"),
-			expectErr: "Error querying stubs: foo",
+			expectErr: "error querying stubs: foo",
 		},
 		{
 			name: "multiple prefix matches with exact match",
@@ -697,7 +697,7 @@ func TestHelperGetByPrefix(t *testing.T) {
 			name:        "no matches",
 			queryObjs:   []*testStub{},
 			queryPrefix: "test",
-			expectErr:   "No stubs with prefix or ID \"test\" found",
+			expectErr:   "no stubs with prefix or ID \"test\" found",
 		},
 	}
 

command/scaling_policy_info_test.go

@@ -58,8 +58,8 @@ func TestScalingPolicyInfoCommand_Run(t *testing.T) {
 	if code := cmd.Run([]string{"-address=" + url, "scaling_policy_info"}); code != 1 {
 		t.Fatalf("expected cmd run exit code 1, got: %d", code)
 	}
-	if out := ui.ErrorWriter.String(); !strings.Contains(out, `No scaling policies with prefix or ID "scaling_policy_inf" found`) {
+	if out := ui.ErrorWriter.String(); !strings.Contains(out, `no scaling policies with prefix or ID "scaling_policy_inf" found`) {
-		t.Fatalf("expected 'No scaling policies with prefix' within output: %v", out)
+		t.Fatalf("expected 'no scaling policies with prefix' within output: %v", out)
 	}
 
 	// Generate a test job.

command/volume_status.go

@@ -4,6 +4,7 @@
 package command
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 
@@ -38,7 +39,8 @@ General Options:
 Status Options:
 
   -type <type>
-    List only volumes of type <type>.
+    List only volumes of type <type> (one of "host" or "csi"). If omitted, the
+    command will query for both dynamic host volumes and CSI volumes.
 
   -short
     Display short output. Used only when a single volume is being
@@ -147,17 +149,65 @@ func (c *VolumeStatusCommand) Run(args []string) int {
 		id = args[0]
 	}
 
+	opts := formatOpts{
+		verbose:  c.verbose,
+		short:    c.short,
+		length:   c.length,
+		json:     c.json,
+		template: c.template,
+	}
+
 	switch typeArg {
-	case "csi", "":
+	case "csi":
 		if nodeID != "" || nodePool != "" {
 			c.Ui.Error("-node and -node-pool can only be used with -type host")
 			return 1
 		}
-		return c.csiStatus(client, id)
+		if err := c.csiVolumeStatus(client, id, opts); err != nil {
+			c.Ui.Error(err.Error())
+			return 1
+		}
 	case "host":
-		return c.hostVolumeStatus(client, id, nodeID, nodePool)
+		if err := c.hostVolumeStatus(client, id, nodeID, nodePool, opts); err != nil {
+			c.Ui.Error(err.Error())
+			return 1
+		}
+	case "":
+		if id == "" {
+			// for list, we want to show both
+			dhvErr := c.hostVolumeList(client, nodeID, nodePool, opts)
+			if dhvErr != nil {
+				c.Ui.Error(dhvErr.Error())
+			}
+			c.Ui.Output("")
+			csiErr := c.csiVolumesList(client, opts)
+			if csiErr != nil {
+				c.Ui.Error(csiErr.Error())
+			}
+			if dhvErr == nil && csiErr == nil {
+				return 0
+			}
+			return 1
+		} else {
+			// for read, we only want to show whichever has results
+			hostErr := c.hostVolumeStatus(client, id, nodeID, nodePool, opts)
+			if hostErr != nil {
+				if !errors.Is(hostErr, hostVolumeListError) {
+					c.Ui.Error(hostErr.Error())
+					return 1 // we found a host volume but had some other error
+				}
+				csiErr := c.csiVolumeStatus(client, id, opts)
+				if csiErr != nil {
+					c.Ui.Error(hostErr.Error())
+					c.Ui.Error(csiErr.Error())
+					return 1
+				}
+			}
+		}
 	default:
 		c.Ui.Error(fmt.Sprintf("No such volume type %q", typeArg))
 		return 1
 	}
+
+	return 0
 }

command/volume_status_csi.go

@@ -11,19 +11,13 @@ import (
 	"strings"
 
 	"github.com/dustin/go-humanize"
+	multierror "github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/nomad/api"
 )
 
-func (c *VolumeStatusCommand) csiBanner() {
+func (c *VolumeStatusCommand) csiVolumeStatus(client *api.Client, id string, opts formatOpts) error {
-	if !(c.json || len(c.template) > 0) {
-		c.Ui.Output(c.Colorize().Color("[bold]Container Storage Interface[reset]"))
-	}
-}
-
-func (c *VolumeStatusCommand) csiStatus(client *api.Client, id string) int {
-	// Invoke list mode if no volume id
 	if id == "" {
-		return c.listCSIVolumes(client)
+		return c.csiVolumesList(client, opts)
 	}
 
 	// get a CSI volume that matches the given prefix or a list of all matches if an
@@ -35,71 +29,65 @@ func (c *VolumeStatusCommand) csiStatus(client *api.Client, id string) int {
 			Namespace: c.namespace,
 		})
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error listing volumes: %s", err))
+		return fmt.Errorf("Error listing CSI volumes: %w", err)
-		return 1
 	}
 	if len(possible) > 0 {
 		out, err := csiFormatVolumes(possible, c.json, c.template)
 		if err != nil {
-			c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
+			return fmt.Errorf("Error formatting: %w", err)
-			return 1
 		}
-		c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
+		return fmt.Errorf("Prefix matched multiple CSI volumes\n\n%s", out)
-		return 1
 	}
 
 	// Try querying the volume
 	vol, _, err := client.CSIVolumes().Info(volStub.ID, nil)
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error querying volume: %s", err))
+		return fmt.Errorf("Error querying CSI volume: %w", err)
-		return 1
 	}
 
 	str, err := c.formatCSIBasic(vol)
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error formatting volume: %s", err))
+		return fmt.Errorf("Error formatting CSI volume: %w", err)
-		return 1
 	}
 	c.Ui.Output(str)
-
+	return nil
-	return 0
 }
 
-func (c *VolumeStatusCommand) listCSIVolumes(client *api.Client) int {
+func (c *VolumeStatusCommand) csiVolumesList(client *api.Client, opts formatOpts) error {
+
+	if !(opts.json || len(opts.template) > 0) {
+		c.Ui.Output(c.Colorize().Color("[bold]Container Storage Interface[reset]"))
+	}
 
-	c.csiBanner()
 	vols, _, err := client.CSIVolumes().List(nil)
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error querying volumes: %s", err))
+		return fmt.Errorf("Error querying CSI volumes: %w", err)
-		return 1
 	}
 
 	if len(vols) == 0 {
-		// No output if we have no volumes
 		c.Ui.Error("No CSI volumes")
+		return nil // not an empty is not an error
 	} else {
-		str, err := csiFormatVolumes(vols, c.json, c.template)
+		str, err := csiFormatVolumes(vols, opts.json, opts.template)
 		if err != nil {
-			c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
+			return fmt.Errorf("Error formatting: %w", err)
-			return 1
 		}
 		c.Ui.Output(str)
 	}
-	if !c.verbose {
+	if !opts.verbose {
-		return 0
+		return nil
 	}
 
 	plugins, _, err := client.CSIPlugins().List(nil)
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error querying CSI plugins: %s", err))
+		return fmt.Errorf("Error querying CSI plugins: %w", err)
-		return 1
 	}
 
 	if len(plugins) == 0 {
-		return 0 // No more output if we have no plugins
+		return nil // No more output if we have no plugins
 	}
 
-	var code int
+	var mErr *multierror.Error
 	q := &api.QueryOptions{PerPage: 30} // TODO: tune page size
 
 NEXT_PLUGIN:
@@ -110,12 +98,11 @@ NEXT_PLUGIN:
 		for {
 			externalList, _, err := client.CSIVolumes().ListExternal(plugin.ID, q)
 			if err != nil && !errors.Is(err, io.EOF) {
-				c.Ui.Error(fmt.Sprintf(
+				mErr = multierror.Append(mErr, fmt.Errorf(
-					"Error querying CSI external volumes for plugin %q: %s", plugin.ID, err))
+					"Error querying CSI external volumes for plugin %q: %w", plugin.ID, err))
 				// we'll stop querying this plugin, but there may be more to
 				// query, so report and set the error code but move on to the
 				// next plugin
-				code = 1
 				continue NEXT_PLUGIN
 			}
 			if externalList == nil || len(externalList.Volumes) == 0 {
@@ -123,6 +110,7 @@ NEXT_PLUGIN:
 				// rather than an empty list
 				continue NEXT_PLUGIN
 			}
+			c.Ui.Output("") // force a newline
 			rows := []string{"External ID|Condition|Nodes"}
 			for _, v := range externalList.Volumes {
 				condition := "OK"
@@ -130,7 +118,7 @@ NEXT_PLUGIN:
 					condition = fmt.Sprintf("Abnormal (%v)", v.Status)
 				}
 				rows = append(rows, fmt.Sprintf("%s|%s|%s",
-					limit(v.ExternalID, c.length),
+					limit(v.ExternalID, opts.length),
 					limit(condition, 20),
 					strings.Join(v.PublishedExternalNodeIDs, ","),
 				))
@@ -147,7 +135,7 @@ NEXT_PLUGIN:
 		}
 	}
 
-	return code
+	return mErr.ErrorOrNil()
 }
 
 func csiFormatVolumes(vols []*api.CSIVolumeListStub, json bool, template string) (string, error) {
@@ -157,7 +145,7 @@ func csiFormatVolumes(vols []*api.CSIVolumeListStub, json bool, template string)
 	if json || len(template) > 0 {
 		out, err := Format(json, template, vols)
 		if err != nil {
-			return "", fmt.Errorf("format error: %v", err)
+			return "", fmt.Errorf("format error: %w", err)
 		}
 		return out, nil
 	}
@@ -186,7 +174,7 @@ func (c *VolumeStatusCommand) formatCSIBasic(vol *api.CSIVolume) (string, error)
 	if c.json || len(c.template) > 0 {
 		out, err := Format(c.json, c.template, vol)
 		if err != nil {
-			return "", fmt.Errorf("format error: %v", err)
+			return "", fmt.Errorf("format error: %w", err)
 		}
 		return out, nil
 	}
@@ -224,7 +212,7 @@ func (c *VolumeStatusCommand) formatCSIBasic(vol *api.CSIVolume) (string, error)
 		full = append(full, topo)
 	}
 
-	banner := "\n[bold]Capabilities[reset]"
+	banner := c.Colorize().Color("\n[bold]Capabilities[reset]")
 	caps := formatCSIVolumeCapabilities(vol.RequestedCapabilities)
 	full = append(full, banner)
 	full = append(full, caps)

command/volume_status_host.go

@@ -4,6 +4,7 @@
 package command
 
 import (
+	"errors"
 	"fmt"
 	"sort"
 	"strings"
@@ -12,83 +13,70 @@ import (
 	"github.com/hashicorp/nomad/api"
 )
 
-func (c *VolumeStatusCommand) hostVolumeStatus(client *api.Client, id, nodeID, nodePool string) int {
+// hostVolumeListError is a non-fatal error for the 'volume status' command when
+// used with the -type option unset, because we want to continue on to list CSI
+// volumes
+var hostVolumeListError = errors.New("Error listing host volumes")
+
+func (c *VolumeStatusCommand) hostVolumeStatus(client *api.Client, id, nodeID, nodePool string, opts formatOpts) error {
 	if id == "" {
-		return c.listHostVolumes(client, nodeID, nodePool)
+		return c.hostVolumeList(client, nodeID, nodePool, opts)
 	}
-
 	if nodeID != "" || nodePool != "" {
-		c.Ui.Error("-node or -node-pool options can only be used when no ID is provided")
+		return errors.New("-node or -node-pool options can only be used when no ID is provided")
-		return 1
-	}
-
-	opts := formatOpts{
-		verbose:  c.verbose,
-		short:    c.short,
-		length:   c.length,
-		json:     c.json,
-		template: c.template,
 	}
 
 	// get a host volume that matches the given prefix or a list of all matches
 	// if an exact match is not found. note we can't use the shared getByPrefix
 	// helper here because the List API doesn't match the required signature
-
 	volStub, possible, err := getHostVolumeByPrefix(client, id, c.namespace)
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error listing volumes: %s", err))
+		return fmt.Errorf("%w: %w", hostVolumeListError, err)
-		return 1
 	}
 	if len(possible) > 0 {
 		out, err := formatHostVolumes(possible, opts)
 		if err != nil {
-			c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
+			return fmt.Errorf("Error formatting: %w", err)
-			return 1
 		}
-		c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
+		return fmt.Errorf("Prefix matched multiple host volumes\n\n%s", out)
-		return 1
 	}
 
 	vol, _, err := client.HostVolumes().Get(volStub.ID, nil)
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error querying volume: %s", err))
+		return fmt.Errorf("Error querying host volume: %w", err)
-		return 1
 	}
 
 	str, err := formatHostVolume(vol, opts)
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error formatting volume: %s", err))
+		return fmt.Errorf("Error formatting host volume: %w", err)
-		return 1
 	}
 	c.Ui.Output(c.Colorize().Color(str))
-	return 0
+	return nil
 }
 
-func (c *VolumeStatusCommand) listHostVolumes(client *api.Client, nodeID, nodePool string) int {
+func (c *VolumeStatusCommand) hostVolumeList(client *api.Client, nodeID, nodePool string, opts formatOpts) error {
+	if !(opts.json || len(opts.template) > 0) {
+		c.Ui.Output(c.Colorize().Color("[bold]Dynamic Host Volumes[reset]"))
+	}
+
 	vols, _, err := client.HostVolumes().List(&api.HostVolumeListRequest{
 		NodeID:   nodeID,
 		NodePool: nodePool,
 	}, nil)
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error querying volumes: %s", err))
+		return fmt.Errorf("Error querying host volumes: %w", err)
-		return 1
 	}
-
+	if len(vols) == 0 {
-	opts := formatOpts{
+		c.Ui.Error("No dynamic host volumes")
-		verbose:  c.verbose,
+		return nil // empty is not an error
-		short:    c.short,
-		length:   c.length,
-		json:     c.json,
-		template: c.template,
 	}
 
 	str, err := formatHostVolumes(vols, opts)
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error formatting volumes: %s", err))
+		return fmt.Errorf("Error formatting host volumes: %w", err)
-		return 1
 	}
 	c.Ui.Output(c.Colorize().Color(str))
-	return 0
+	return nil
 }
 
 func getHostVolumeByPrefix(client *api.Client, prefix, ns string) (*api.HostVolumeStub, []*api.HostVolumeStub, error) {
@@ -98,7 +86,7 @@ func getHostVolumeByPrefix(client *api.Client, prefix, ns string) (*api.HostVolu
 	})
 
 	if err != nil {
-		return nil, nil, fmt.Errorf("error querying volumes: %s", err)
+		return nil, nil, fmt.Errorf("error querying volumes: %w", err)
 	}
 	switch len(vols) {
 	case 0:
@@ -127,7 +115,7 @@ func formatHostVolume(vol *api.HostVolume, opts formatOpts) (string, error) {
 	if opts.json || len(opts.template) > 0 {
 		out, err := Format(opts.json, opts.template, vol)
 		if err != nil {
-			return "", fmt.Errorf("format error: %v", err)
+			return "", fmt.Errorf("format error: %w", err)
 		}
 		return out, nil
 	}
@@ -181,7 +169,7 @@ func formatHostVolumes(vols []*api.HostVolumeStub, opts formatOpts) (string, err
 	if opts.json || len(opts.template) > 0 {
 		out, err := Format(opts.json, opts.template, vols)
 		if err != nil {
-			return "", fmt.Errorf("format error: %v", err)
+			return "", fmt.Errorf("format error: %w", err)
 		}
 		return out, nil
 	}

command/volume_status_host_test.go

@@ -161,7 +161,7 @@ capability {
 	code = cmd.Run(args)
 	must.Eq(t, 1, code)
 	must.StrContains(t, ui.ErrorWriter.String(),
-		"Error listing volumes: no volumes with prefix or ID")
+		"Error listing host volumes: no volumes with prefix or ID")
 	ui.ErrorWriter.Reset()
 
 	args = []string{"-address", url, "-type", "host", "-namespace", "prod", id}

website/content/docs/commands/volume/status.mdx

@@ -8,7 +8,7 @@ description: |
 # Command: volume status
 
 The `volume status` command displays status information for [Container
-Storage Interface (CSI)][csi] volumes.
+Storage Interface (CSI)][csi] volumes or [dynamic host volumes][dhv].
 
 ## Usage
 
@@ -16,17 +16,18 @@ Storage Interface (CSI)][csi] volumes.
 nomad volume status [options] [volume]
 ```
 
-This command accepts an optional volume ID or prefix as the sole argument. If there
+This command accepts an optional volume ID or prefix as the sole argument. If
-is an exact match based on the provided volume ID or prefix, then information about
+there is an exact match based on the provided volume ID or prefix, then
-the specific volume is queried and displayed. Otherwise, a list of matching volumes
+information about the specific volume is queried and displayed. Otherwise, a
-and information will be displayed.
+list of matching volumes and information will be displayed.
 
-If the ID is omitted, the command lists out all of the existing volumes and a few
+If the ID is omitted, the command lists out all of the existing volumes and a
-of the most useful status fields for each.
+few of the most useful status fields for each.
 
-When ACLs are enabled, this command requires a token with the
+When ACLs are enabled, this command requires a token with the appropriate
-`csi-read-volume` and `csi-list-volumes` capability for the volume's
+capability in the volume's namespace: the `csi-read-volume` and
-namespace.
+`csi-list-volumes` capability for CSI volumes, or `host-volume-read` for dynamic
+host volumes.
 
 ## General Options
 
@@ -34,9 +35,9 @@ namespace.
 
 ## Status Options
 
-- `-type`: Display only volumes of a particular type. Currently only
+- `-type`: Display only volumes of a particular type (one of `"host"` or
-  the `csi` type is supported, so this option can be omitted when
+  `"csi"`). If omitted, the command will query for both dynamic host volumes and
-  querying the status of CSI volumes.
+  CSI volumes.
 
 - `-plugin_id`: Display only volumes managed by a particular [CSI
   plugin][csi_plugin].
@@ -57,15 +58,21 @@ namespace.
 List of all volumes:
 
 ```shell-session
-$ nomad volume [-type csi] status
+$ nomad volume status
+Dynamic Host Volumes
+ID        Name             Namespace  Plugin ID            Node ID   Node Pool  State
+070dc8ca  internal-plugin  default    mkdir                c3c27514  default    ready
+7ce38017  external-plugin  default    example-plugin-mkfs  c3c27514  default    ready
+
+Container Storage Interface
 ID            Name      Namespace  Plugin ID  Schedulable  Access Mode
 ebs_prod_db1  database  default    ebs-prod   true         single-node-writer
 ```
 
-List of all volumes, with external provider info:
+List of all CSI volumes, with external provider info:
 
 ```shell-session
-$ nomad volume [-type csi] status -verbose
+$ nomad volume -type csi status -verbose
 ID            Name      Namespace  Plugin ID  Schedulable  Access Mode
 ebs_prod_db1  database  default    ebs-prod   true         single-node-writer
 
@@ -77,7 +84,7 @@ vol-cd46df    Abnormal (provider message here)  i-14a12df13
 Short view of a specific volume:
 
 ```shell-session
-$ nomad volume status [-verbose] [-plugin=ebs-prod] ebs_prod_db1
+$ nomad volume status ebs_prod_db1
 ID                   = ebs_prod_db1
 Name                 = database
 Type                 = csi
@@ -100,7 +107,7 @@ Namespace            = default
 Full status information of a volume:
 
 ```shell-session
-$ nomad volume status [-verbose] [-plugin=ebs-prod] ebs_prod_db1
+$ nomad volume status -verbose ebs_prod_db1
 ID                   = ebs_prod_db1
 Name                 = database
 Type                 = csi
@@ -127,3 +134,4 @@ b00fa322  28be17d5  write         csi         0        run
 [csi]: https://github.com/container-storage-interface/spec
 [csi_plugin]: /nomad/docs/job-specification/csi_plugin
 [`volume create`]: /nomad/docs/commands/volume/create
+[dhv]: /nomad/docs/other-specifications/volume/host