Skip to content

Commit

Permalink
address comments from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
tgross committed Feb 21, 2025
1 parent 77728c2 commit 896ea07
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 23 deletions.
4 changes: 2 additions & 2 deletions command/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions command/volume_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package command

import (
"errors"
"fmt"
"strings"

Expand Down Expand Up @@ -191,6 +192,10 @@ func (c *VolumeStatusCommand) Run(args []string) int {
// 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())
Expand Down
20 changes: 10 additions & 10 deletions command/volume_status_csi.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,25 @@ func (c *VolumeStatusCommand) csiVolumeStatus(client *api.Client, id string, opt
Namespace: c.namespace,
})
if err != nil {
return fmt.Errorf("Error listing CSI volumes: %s", err)
return fmt.Errorf("Error listing CSI volumes: %w", err)
}
if len(possible) > 0 {
out, err := csiFormatVolumes(possible, c.json, c.template)
if err != nil {
return fmt.Errorf("Error formatting: %s", err)
return fmt.Errorf("Error formatting: %w", err)
}
return fmt.Errorf("Prefix matched multiple CSI volumes\n\n%s", out)
}

// Try querying the volume
vol, _, err := client.CSIVolumes().Info(volStub.ID, nil)
if err != nil {
return fmt.Errorf("Error querying CSI volume: %s", err)
return fmt.Errorf("Error querying CSI volume: %w", err)
}

str, err := c.formatCSIBasic(vol)
if err != nil {
return fmt.Errorf("Error formatting CSI volume: %s", err)
return fmt.Errorf("Error formatting CSI volume: %w", err)
}
c.Ui.Output(str)
return nil
Expand All @@ -61,7 +61,7 @@ func (c *VolumeStatusCommand) csiVolumesList(client *api.Client, opts formatOpts

vols, _, err := client.CSIVolumes().List(nil)
if err != nil {
return fmt.Errorf("Error querying CSI volumes: %s", err)
return fmt.Errorf("Error querying CSI volumes: %w", err)
}

if len(vols) == 0 {
Expand All @@ -70,7 +70,7 @@ func (c *VolumeStatusCommand) csiVolumesList(client *api.Client, opts formatOpts
} else {
str, err := csiFormatVolumes(vols, opts.json, opts.template)
if err != nil {
return fmt.Errorf("Error formatting: %s", err)
return fmt.Errorf("Error formatting: %w", err)
}
c.Ui.Output(str)
}
Expand All @@ -80,7 +80,7 @@ func (c *VolumeStatusCommand) csiVolumesList(client *api.Client, opts formatOpts

plugins, _, err := client.CSIPlugins().List(nil)
if err != nil {
return fmt.Errorf("Error querying CSI plugins: %s", err)
return fmt.Errorf("Error querying CSI plugins: %w", err)
}

if len(plugins) == 0 {
Expand All @@ -99,7 +99,7 @@ NEXT_PLUGIN:
externalList, _, err := client.CSIVolumes().ListExternal(plugin.ID, q)
if err != nil && !errors.Is(err, io.EOF) {
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
Expand Down Expand Up @@ -145,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
}
Expand Down Expand Up @@ -174,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
}
Expand Down
23 changes: 14 additions & 9 deletions command/volume_status_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ import (
"github.com/hashicorp/nomad/api"
)

// 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.hostVolumeList(client, nodeID, nodePool, opts)
Expand All @@ -26,24 +31,24 @@ func (c *VolumeStatusCommand) hostVolumeStatus(client *api.Client, id, nodeID, n
// helper here because the List API doesn't match the required signature
volStub, possible, err := getHostVolumeByPrefix(client, id, c.namespace)
if err != nil {
return fmt.Errorf("Error listing host volumes: %s", err)
return fmt.Errorf("%w: %w", hostVolumeListError, err)
}
if len(possible) > 0 {
out, err := formatHostVolumes(possible, opts)
if err != nil {
return fmt.Errorf("Error formatting: %s", err)
return fmt.Errorf("Error formatting: %w", err)
}
return fmt.Errorf("Prefix matched multiple host volumes\n\n%s", out)
}

vol, _, err := client.HostVolumes().Get(volStub.ID, nil)
if err != nil {
return fmt.Errorf("Error querying host volume: %s", err)
return fmt.Errorf("Error querying host volume: %w", err)
}

str, err := formatHostVolume(vol, opts)
if err != nil {
return fmt.Errorf("Error formatting host volume: %s", err)
return fmt.Errorf("Error formatting host volume: %w", err)
}
c.Ui.Output(c.Colorize().Color(str))
return nil
Expand All @@ -59,7 +64,7 @@ func (c *VolumeStatusCommand) hostVolumeList(client *api.Client, nodeID, nodePoo
NodePool: nodePool,
}, nil)
if err != nil {
return fmt.Errorf("Error querying host volumes: %s", err)
return fmt.Errorf("Error querying host volumes: %w", err)
}
if len(vols) == 0 {
c.Ui.Error("No dynamic host volumes")
Expand All @@ -68,7 +73,7 @@ func (c *VolumeStatusCommand) hostVolumeList(client *api.Client, nodeID, nodePoo

str, err := formatHostVolumes(vols, opts)
if err != nil {
return fmt.Errorf("Error formatting host volumes: %s", err)
return fmt.Errorf("Error formatting host volumes: %w", err)
}
c.Ui.Output(c.Colorize().Color(str))
return nil
Expand All @@ -81,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:
Expand Down Expand Up @@ -110,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
}
Expand Down Expand Up @@ -164,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
}
Expand Down
4 changes: 2 additions & 2 deletions website/content/docs/commands/volume/status.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -84,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
Expand All @@ -107,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
Expand Down

0 comments on commit 896ea07

Please sign in to comment.