Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSI: volume snapshot list plugin option is required #12197

Merged
merged 1 commit into from
Mar 7, 2022
Merged

CSI: volume snapshot list plugin option is required #12197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/12197.txt
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,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 @@ -34,8 +35,8 @@ General Options:

List Options:

-plugin: Display only snapshots managed by a particular plugin. By default
this command will query all plugins for their snapshots.
Comment on lines -37 to -38
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "by default" behavior never worked and never could work, so I'm just calling this a mulligan for the CSI beta and not a backcompat issue we need to worry about. 😀

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

-plugin: Display only snapshots managed by a particular plugin. This
parameter is required.

-secret
Secrets to pass to the plugin to list snapshots. Accepts multiple
Expand All @@ -45,7 +46,7 @@ List Options:
}

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 @@ -100,15 +101,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 @@ -117,13 +120,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

secrets := api.CSISecrets{}
for _, kv := range secretsArgs {
Expand Down
2 changes: 1 addition & 1 deletion e2e/csi/ebs.go
View file
Open in desktop
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
11 changes: 5 additions & 6 deletions website/content/docs/commands/volume/snapshot-list.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,10 @@ Nomad.
## Snapshot 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.
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.
- `-secret`: Secrets to pass to the plugin to list snapshots. Accepts
multiple flags in the form `-secret key=value`

Expand All @@ -54,7 +53,7 @@ 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 -secret key1=value1 -secret key2=val2
$ nomad volume snapshot list -plugin aws-ebs0 -secret key1=value1 -secret key2=val2
Snapshot ID External ID Size Creation Time Ready?
snap-12345 vol-abcdef 50GiB 2021-01-03T12:15:02Z true
```
Expand Down