From 03a1f11f7922d5cf0f5bf69b88e1c3c8378063a1 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Fri, 4 Mar 2022 15:07:06 -0500 Subject: [PATCH] csi: get plugin ID for creating snapshot from volume, not args The `CreateSnapshot` RPC expects a plugin ID to be set by the API, but in the common case of the `nomad volume snapshot create` command, we don't ask the user for the plugin ID because it's available from the volume we're snapshotting. Change the order of the RPC so that we get the volume first and then use the volume's plugin ID for the plugin if the API didn't set the value. --- .changelog/12195.txt | 3 +++ nomad/csi_endpoint.go | 33 +++++++++++++++++++-------------- nomad/csi_endpoint_test.go | 1 - 3 files changed, 22 insertions(+), 15 deletions(-) create mode 100644 .changelog/12195.txt diff --git a/.changelog/12195.txt b/.changelog/12195.txt new file mode 100644 index 000000000000..97ec9bc751bd --- /dev/null +++ b/.changelog/12195.txt @@ -0,0 +1,3 @@ +```release-note:bug +csi: Fixed a bug where creating snapshots required a plugin ID instead of falling back to the volume's plugin ID +``` diff --git a/nomad/csi_endpoint.go b/nomad/csi_endpoint.go index 66083452fa82..78fcee973b72 100644 --- a/nomad/csi_endpoint.go +++ b/nomad/csi_endpoint.go @@ -1095,29 +1095,34 @@ func (v *CSIVolume) CreateSnapshot(args *structs.CSISnapshotCreateRequest, reply return fmt.Errorf("snapshot cannot be nil") } - plugin, err := state.CSIPluginByID(nil, snap.PluginID) + vol, err := state.CSIVolumeByID(nil, args.RequestNamespace(), snap.SourceVolumeID) if err != nil { - multierror.Append(&mErr, - fmt.Errorf("error querying plugin %q: %v", snap.PluginID, err)) + multierror.Append(&mErr, fmt.Errorf("error querying volume %q: %v", snap.SourceVolumeID, err)) continue } - if plugin == nil { - multierror.Append(&mErr, fmt.Errorf("no such plugin %q", snap.PluginID)) + if vol == nil { + multierror.Append(&mErr, fmt.Errorf("no such volume %q", snap.SourceVolumeID)) continue } - if !plugin.HasControllerCapability(structs.CSIControllerSupportsCreateDeleteSnapshot) { - multierror.Append(&mErr, - fmt.Errorf("plugin %q does not support snapshot", snap.PluginID)) - continue + + pluginID := snap.PluginID + if pluginID == "" { + pluginID = vol.PluginID } - vol, err := state.CSIVolumeByID(nil, args.RequestNamespace(), snap.SourceVolumeID) + plugin, err := state.CSIPluginByID(nil, pluginID) if err != nil { - multierror.Append(&mErr, fmt.Errorf("error querying volume %q: %v", snap.SourceVolumeID, err)) + multierror.Append(&mErr, + fmt.Errorf("error querying plugin %q: %v", pluginID, err)) continue } - if vol == nil { - multierror.Append(&mErr, fmt.Errorf("no such volume %q", snap.SourceVolumeID)) + if plugin == nil { + multierror.Append(&mErr, fmt.Errorf("no such plugin %q", pluginID)) + continue + } + if !plugin.HasControllerCapability(structs.CSIControllerSupportsCreateDeleteSnapshot) { + multierror.Append(&mErr, + fmt.Errorf("plugin %q does not support snapshot", pluginID)) continue } @@ -1127,7 +1132,7 @@ func (v *CSIVolume) CreateSnapshot(args *structs.CSISnapshotCreateRequest, reply Secrets: vol.Secrets, Parameters: snap.Parameters, } - cReq.PluginID = plugin.ID + cReq.PluginID = pluginID cResp := &cstructs.ClientCSIControllerCreateSnapshotResponse{} err = v.srv.RPC(method, cReq, cResp) if err != nil { diff --git a/nomad/csi_endpoint_test.go b/nomad/csi_endpoint_test.go index 4b8275064984..51c0d65cc2f4 100644 --- a/nomad/csi_endpoint_test.go +++ b/nomad/csi_endpoint_test.go @@ -1199,7 +1199,6 @@ func TestCSIVolumeEndpoint_CreateSnapshot(t *testing.T) { SourceVolumeID: "test-volume0", Secrets: structs.CSISecrets{"mysecret": "secretvalue"}, Parameters: map[string]string{"myparam": "paramvalue"}, - PluginID: "minnie", }}, WriteRequest: structs.WriteRequest{ Region: "global",