CSI: don't overwrite context with empty value from request (#24922)

When a volume is updated, we merge the new definition to the old. But the
volume's context comes from the plugin and is likely not present in the user's
volume specification. Which means that if the user re-submits the volume
specification to make an adjustment to the volume, we wipe out the context field
which might be required for subsequent operations by the CSI plugin. This was
discovered to be a problem with the Terraform provider and fixed there, but it's
also a problem for users of the `volume create` and `volume register` commands.

Update the merge so that we only overwrite the value of the context if it's been
explictly set by the user. We still need to support user-driven updates to
context for the `volume register` workflow.

Ref: https://github.com/hashicorp/terraform-provider-nomad/pull/503
Fixes: https://github.com/democratic-csi/democratic-csi/issues/438
This commit is contained in:
Tim Gross
2025-01-23 14:06:32 -05:00
committed by GitHub
parent 5befea62b7
commit c1dc9ed75d
3 changed files with 25 additions and 3 deletions

3
.changelog/24922.txt Normal file
View File

@@ -0,0 +1,3 @@
```release-note:bug
csi: Fixed a bug where volume context from the plugin would be erased on volume updates
```

View File

@@ -846,9 +846,13 @@ func (v *CSIVolume) Merge(other *CSIVolume) error {
"volume parameters cannot be updated"))
}
// Context is mutable and will be used during controller
// validation
v.Context = other.Context
// Context is mutable and will be used during controller validation, but we
// need to ensure we don't remove context that's been previously stored
// server-side if the user has submitted an update without adding it to the
// spec manually (which we should not require)
if len(other.Context) != 0 {
v.Context = other.Context
}
return errs.ErrorOrNil()
}

View File

@@ -711,6 +711,13 @@ func TestCSIVolume_Merge(t *testing.T) {
{Segments: map[string]string{"rack": "R2"}},
},
},
Context: map[string]string{
// a typical context for democratic-csi
"provisioner_driver": "nfs-client",
"node_attach_driver": "nfs",
"server": "192.168.1.170",
"share": "/srv/nfs_data/v/csi-volume-nfs",
},
},
update: &CSIVolume{
Topologies: []*CSITopology{
@@ -745,6 +752,14 @@ func TestCSIVolume_Merge(t *testing.T) {
test.Sprint("should add 2 requested capabilities"))
test.Eq(t, []string{"noatime", "another"}, v.MountOptions.MountFlags,
test.Sprint("should add mount flag"))
test.Eq(t, map[string]string{
"provisioner_driver": "nfs-client",
"node_attach_driver": "nfs",
"server": "192.168.1.170",
"share": "/srv/nfs_data/v/csi-volume-nfs",
}, v.Context,
test.Sprint("context should not be overwritten with empty update"))
},
},
}