Skip to content

Commit

Permalink
csi: volume snapshot list plugin option is required (#12197)
Browse files Browse the repository at this point in the history
The RPC for listing volume snapshots requires a plugin ID. Update the
`volume snapshot list` command to find the specific plugin from the
provided prefix.
  • Loading branch information
tgross authored and lgfa29 committed Apr 19, 2022
1 parent b186c36 commit 185dde0
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 29 deletions.
3 changes: 3 additions & 0 deletions .changelog/12197.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
csi: Fixed a bug where `volume snapshot list` did not correctly filter by plugin IDs. The `-plugin` parameter is required.
```
38 changes: 18 additions & 20 deletions command/volume_snapshot_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ func (c *VolumeSnapshotListCommand) Help() string {
helpText := `
Usage: nomad volume snapshot list [-plugin plugin_id]
Display a list of CSI volume snapshots along with their
source volume ID as known to the external storage provider.
Display a list of CSI volume snapshots for a plugin along
with their source volume ID as known to the external
storage provider.
When ACLs are enabled, this command requires a token with the
'csi-list-volumes' capability for the plugin's namespace.
Expand All @@ -33,16 +34,16 @@ General Options:
List Options:
-plugin: Display only snapshots managed by a particular plugin. By default
this command will query all plugins for their snapshots.
-plugin: Display only snapshots managed by a particular plugin. This
parameter is required.
-secrets: A set of key/value secrets to be used when listing snapshots.
`
return strings.TrimSpace(helpText)
}

func (c *VolumeSnapshotListCommand) Synopsis() string {
return "Display a list of volume snapshots"
return "Display a list of volume snapshots for plugin"
}

func (c *VolumeSnapshotListCommand) AutocompleteFlags() complete.Flags {
Expand Down Expand Up @@ -97,15 +98,17 @@ func (c *VolumeSnapshotListCommand) Run(args []string) int {
return 1
}

// filter by plugin if a plugin ID was passed
if pluginID != "" {
plugs, _, err := client.CSIPlugins().List(&api.QueryOptions{Prefix: pluginID})
if err != nil {
c.Ui.Error(fmt.Sprintf("Error querying CSI plugins: %s", err))
return 1
}

if len(plugs) > 1 {
plugs, _, err := client.CSIPlugins().List(&api.QueryOptions{Prefix: pluginID})
if err != nil {
c.Ui.Error(fmt.Sprintf("Error querying CSI plugins: %s", err))
return 1
}
if len(plugs) == 0 {
c.Ui.Error(fmt.Sprintf("No plugins(s) with prefix or ID %q found", pluginID))
return 1
}
if len(plugs) > 1 {
if pluginID != plugs[0].ID {
out, err := c.csiFormatPlugins(plugs)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
Expand All @@ -114,13 +117,8 @@ func (c *VolumeSnapshotListCommand) Run(args []string) int {
c.Ui.Error(fmt.Sprintf("Prefix matched multiple plugins\n\n%s", out))
return 1
}
if len(plugs) == 0 {
c.Ui.Error(fmt.Sprintf("No plugins(s) with prefix or ID %q found", pluginID))
return 1
}

pluginID = plugs[0].ID
}
pluginID = plugs[0].ID

q := &api.QueryOptions{PerPage: 30} // TODO: tune page size

Expand Down
2 changes: 1 addition & 1 deletion e2e/csi/ebs.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func (tc *CSIControllerPluginEBSTest) TestSnapshot(f *framework.F) {
f.NoError(err, fmt.Sprintf("could not parse output:\n%v", out))
f.Len(snaps, 1, fmt.Sprintf("could not parse output:\n%v", out))

out, err = e2e.Command("nomad", "volume", "snapshot", "list")
out, err = e2e.Command("nomad", "volume", "snapshot", "list", "-plugin", ebsPluginID)
requireNoErrorElseDump(f, err, "could not list volume snapshots", tc.pluginJobIDs)
f.Contains(out, snaps[0]["ID"],
fmt.Sprintf("volume snapshot list did not include expected snapshot:\n%v", out))
Expand Down
15 changes: 7 additions & 8 deletions website/content/docs/commands/volume/snapshot-list.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,11 @@ Nomad.
## List Options

- `-plugin`: Display only snapshots managed by a particular [CSI
plugin][csi_plugin]. By default the `snapshot list` command will query all
plugins for their snapshots. This flag accepts a plugin ID or prefix. If
there is an exact match based on the provided plugin, then that specific
plugin will be queried. Otherwise, a list of matching plugins will be
displayed.
- `-secrets`: A list of comma separated secret key/value pairs to be passed
plugin][csi_plugin]. This flag is required and accepts a plugin ID
or prefix. If there is an exact match based on the provided plugin,
then that specific plugin will be queried. Otherwise, a list of
matching plugins will be displayed.
- `-secrets`: A list of comma separated secret key/value pairs to be passed
to the CSI driver.

When ACLs are enabled, this command requires a token with the
Expand All @@ -54,12 +53,12 @@ snap-67890 vol-fedcba 50GiB 2021-01-04T15:45:00Z true

List volume snapshots with two secret key/value pairs:
```shell-session
$ nomad volume snapshot list -secrets key1=value1,key2=val2
$ nomad volume snapshot list -plugin aws-ebs0 -secrets key1=value1,key2=val2
Snapshot ID External ID Size Creation Time Ready?
snap-12345 vol-abcdef 50GiB 2021-01-03T12:15:02Z true
```

[csi]: https://github.com/container-storage-interface/spec
[csi_plugin]: /docs/job-specification/csi_plugin
[registered]: /docs/commands/volume/register
[csi_plugins_internals]: /docs/internals/plugins/csi#csi-plugins
[csi_plugins_internals]: /docs/internals/plugins/csi#csi-plugins

0 comments on commit 185dde0

Please sign in to comment.