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

Jump to bottom

CSI: volume cli prefix matching should accept exact match #12051

Merged
merged 1 commit into from
Feb 11, 2022
Merged

CSI: volume cli prefix matching should accept exact match #12051

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/12051.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 detach`, `volume deregister`, and `volume status` commands did not accept an exact ID if multiple volumes matched the prefix
```
22 changes: 12 additions & 10 deletions command/volume_deregister.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,20 +99,22 @@ func (c *VolumeDeregisterCommand) Run(args []string) int {
c.Ui.Error(fmt.Sprintf("Error querying volumes: %s", err))
return 1
}
if len(vols) > 1 {
sort.Slice(vols, func(i, j int) bool { return vols[i].ID < vols[j].ID })
out, err := csiFormatSortedVolumes(vols, fullId)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
return 1
}
c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
return 1
}
if len(vols) == 0 {
c.Ui.Error(fmt.Sprintf("No volumes(s) with prefix or ID %q found", volID))
return 1
}
if len(vols) > 1 {
if (volID != vols[0].ID) || (c.allNamespaces() && vols[0].ID == vols[1].ID) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just checking my understanding of the state store, when looking by ID prefix, vols would be sorted by prefix length, so if the first element is not an exact match, we don't need to check the rest because it would be a longer value. Is this right? 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly. This property arises from go-immutable-radix.

sort.Slice(vols, func(i, j int) bool { return vols[i].ID < vols[j].ID })
out, err := csiFormatSortedVolumes(vols, fullId)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
return 1
}
c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
return 1
}
}
volID = vols[0].ID

// Confirm the -force flag
Expand Down
22 changes: 12 additions & 10 deletions command/volume_detach.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,20 +121,22 @@ func (c *VolumeDetachCommand) Run(args []string) int {
c.Ui.Error(fmt.Sprintf("Error querying volumes: %s", err))
return 1
}
if len(vols) > 1 {
sort.Slice(vols, func(i, j int) bool { return vols[i].ID < vols[j].ID })
out, err := csiFormatSortedVolumes(vols, fullId)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
return 1
}
c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
return 1
}
if len(vols) == 0 {
c.Ui.Error(fmt.Sprintf("No volumes(s) with prefix or ID %q found", volID))
return 1
}
if len(vols) > 1 {
if (volID != vols[0].ID) || (c.allNamespaces() && vols[0].ID == vols[1].ID) {
sort.Slice(vols, func(i, j int) bool { return vols[i].ID < vols[j].ID })
out, err := csiFormatSortedVolumes(vols, fullId)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
return 1
}
c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
return 1
}
}
volID = vols[0].ID

vol, _, err := client.CSIVolumes().Info(volID, nil)
Expand Down
20 changes: 11 additions & 9 deletions command/volume_status_csi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,21 @@ func (c *VolumeStatusCommand) csiStatus(client *api.Client, id string) int {
c.Ui.Error(fmt.Sprintf("Error querying volumes: %s", err))
return 1
}
if len(vols) > 1 {
out, err := c.csiFormatVolumes(vols)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
return 1
}
c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
return 1
}
if len(vols) == 0 {
c.Ui.Error(fmt.Sprintf("No volumes(s) with prefix or ID %q found", id))
return 1
}
if len(vols) > 1 {
if (id != vols[0].ID) || (c.allNamespaces() && vols[0].ID == vols[1].ID) {
out, err := c.csiFormatVolumes(vols)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
return 1
}
c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
return 1
}
}
id = vols[0].ID

// Try querying the volume
Expand Down