luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

csi: volume deregistration should require exact ID (#11852) 

The command line client sends a specific volume ID, but this isn't
enforced at the API level and we were incorrectly using a prefix match
for volume deregistration, resulting in cases where a volume with a
shorter ID that's a prefix of another volume would be deregistered
instead of the intended volume.
This commit is contained in:
Tim Gross 2022-01-14 12:26:03 -05:00 committed by GitHub
parent 33f7c6cba4
commit d7756f8cdb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 12 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/11852.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
csi: Fixed a bug where deregistering volumes would attempt to deregister the wrong volume if the ID was a prefix of the intended volume
```

2
nomad/state/state_store.go
View File

@ -2375,7 +2375,7 @@ func (s *StateStore) CSIVolumeDeregister(index uint64, namespace string, ids []s
defer txn.Abort() defer txn.Abort()
for _, id := range ids { for _, id := range ids {
existing, err := txn.First("csi_volumes", "id_prefix", namespace, id) existing, err := txn.First("csi_volumes", "id", namespace, id)
if err != nil { if err != nil {
return fmt.Errorf("volume lookup failed: %s: %v", id, err) return fmt.Errorf("volume lookup failed: %s: %v", id, err)
} }

11
nomad/state/state_store_test.go
View File

@ -2995,16 +2995,21 @@ func TestStateStore_CSIVolume(t *testing.T) {
// registration is an error when the volume is in use // registration is an error when the volume is in use
index++ index++
err = state.CSIVolumeRegister(index, []*structs.CSIVolume{v0}) err = state.CSIVolumeRegister(index, []*structs.CSIVolume{v0})
require.Error(t, err, fmt.Sprintf("volume exists: %s", vol0)) require.Error(t, err, "volume re-registered while in use")
// as is deregistration // as is deregistration
index++ index++
err = state.CSIVolumeDeregister(index, ns, []string{vol0}, false) err = state.CSIVolumeDeregister(index, ns, []string{vol0}, false)
require.Error(t, err, fmt.Sprintf("volume in use: %s", vol0)) require.Error(t, err, "volume deregistered while in use")
// even if forced, because we have a non-terminal claim // even if forced, because we have a non-terminal claim
index++ index++
err = state.CSIVolumeDeregister(index, ns, []string{vol0}, true) err = state.CSIVolumeDeregister(index, ns, []string{vol0}, true)
require.Error(t, err, fmt.Sprintf("volume in use: %s", vol0)) require.Error(t, err, "volume force deregistered while in use")
// we use the ID, not a prefix
index++
err = state.CSIVolumeDeregister(index, ns, []string{"fo"}, true)
require.Error(t, err, "volume deregistered by prefix")
// release claims to unblock deregister // release claims to unblock deregister
index++ index++