From e1af61f5d465357cec25c4374d5a2b432468265c Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Thu, 3 Jun 2021 15:47:55 -0400 Subject: [PATCH] csi: accept list of caps during validation in volume register When `nomad volume create` was introduced in Nomad 1.1.0, we changed the volume spec to take a list of capabilities rather than a single capability, to meet the requirements of the CSI spec. When a volume is registered via `nomad volume register`, we should be using the same fields to validate the volume with the controller plugin. --- CHANGELOG.md | 1 + client/csi_endpoint_test.go | 16 ++++++++++------ client/structs/csi.go | 29 +++++++++++++++++++---------- nomad/csi_endpoint.go | 11 +++++------ plugins/csi/client_test.go | 4 ++-- plugins/csi/plugin.go | 19 +++++++++++-------- 6 files changed, 48 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 363b91e07fd6..79e35aae262c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ BUG FIXES: * cli: Fixed a bug where `quota status` and `namespace status` commands may panic if the CLI targets a pre-1.1.0 cluster [[GH-10620](https://github.com/hashicorp/nomad/pull/10620)] * cli: Fixed a bug where `alloc exec` may fail with "unexpected EOF" without returning the exit code after a command [[GH-10657](https://github.com/hashicorp/nomad/issues/10657)] * csi: Fixed a bug where `mount_options` were not passed to CSI controller plugins for validation during volume creation and mounting. [[GH-10643](https://github.com/hashicorp/nomad/issues/10643)] +* csi: Fixed a bug where `capability` blocks were not passed to CSI controller plugins for validation for `nomad volume register` commands. [[GH-10703](https://github.com/hashicorp/nomad/issues/10703)] * drivers/exec: Fixed a bug where `exec` and `java` tasks inherit the Nomad agent's `oom_score_adj` value [[GH-10698](https://github.com/hashicorp/nomad/issues/10698)] * quotas (Enterprise): Fixed a bug where stopped allocations for a failed deployment can be double-credited to quota limits, resulting in a quota limit bypass. [[GH-10694](https://github.com/hashicorp/nomad/issues/10694)] * ui: Fixed a bug where exec would not work across regions. [[GH-10539](https://github.com/hashicorp/nomad/issues/10539)] diff --git a/client/csi_endpoint_test.go b/client/csi_endpoint_test.go index 84012124cae3..f5bc07b44dc2 100644 --- a/client/csi_endpoint_test.go +++ b/client/csi_endpoint_test.go @@ -206,9 +206,11 @@ func TestCSIController_ValidateVolume(t *testing.T) { CSIControllerQuery: structs.CSIControllerQuery{ PluginID: fakePlugin.Name, }, - VolumeID: "1234-4321-1234-4321", - AttachmentMode: nstructs.CSIVolumeAttachmentMode("bar"), - AccessMode: nstructs.CSIVolumeAccessModeMultiNodeReader, + VolumeID: "1234-4321-1234-4321", + VolumeCapabilities: []*nstructs.CSIVolumeCapability{{ + AttachmentMode: nstructs.CSIVolumeAttachmentMode("bar"), + AccessMode: nstructs.CSIVolumeAccessModeMultiNodeReader, + }}, }, ExpectedErr: errors.New("CSI.ControllerValidateVolume: unknown volume attachment mode: bar"), }, @@ -218,9 +220,11 @@ func TestCSIController_ValidateVolume(t *testing.T) { CSIControllerQuery: structs.CSIControllerQuery{ PluginID: fakePlugin.Name, }, - VolumeID: "1234-4321-1234-4321", - AttachmentMode: nstructs.CSIVolumeAttachmentModeFilesystem, - AccessMode: nstructs.CSIVolumeAccessMode("foo"), + VolumeID: "1234-4321-1234-4321", + VolumeCapabilities: []*nstructs.CSIVolumeCapability{{ + AttachmentMode: nstructs.CSIVolumeAttachmentModeFilesystem, + AccessMode: nstructs.CSIVolumeAccessMode("foo"), + }}, }, ExpectedErr: errors.New("CSI.ControllerValidateVolume: unknown volume access mode: foo"), }, diff --git a/client/structs/csi.go b/client/structs/csi.go index 7ad853a87622..d4846f7e95cc 100644 --- a/client/structs/csi.go +++ b/client/structs/csi.go @@ -54,10 +54,14 @@ func (c *CSIControllerQuery) SetControllerNodeID(nodeID string) { type ClientCSIControllerValidateVolumeRequest struct { VolumeID string // note: this is the external ID + VolumeCapabilities []*structs.CSIVolumeCapability + MountOptions *structs.CSIMountOptions + Secrets structs.CSISecrets + + // COMPAT(1.1.1): the AttachmentMode and AccessMode fields are deprecated + // and replaced by the VolumeCapabilities field above AttachmentMode structs.CSIVolumeAttachmentMode AccessMode structs.CSIVolumeAccessMode - MountOptions *structs.CSIMountOptions - Secrets structs.CSISecrets // Parameters as returned by storage provider in CreateVolumeResponse. // This field is optional. @@ -75,18 +79,23 @@ func (c *ClientCSIControllerValidateVolumeRequest) ToCSIRequest() (*csi.Controll return &csi.ControllerValidateVolumeRequest{}, nil } - caps, err := csi.VolumeCapabilityFromStructs(c.AttachmentMode, c.AccessMode, c.MountOptions) - if err != nil { - return nil, err - } - - return &csi.ControllerValidateVolumeRequest{ + creq := &csi.ControllerValidateVolumeRequest{ ExternalID: c.VolumeID, Secrets: c.Secrets, - Capabilities: caps, + Capabilities: []*csi.VolumeCapability{}, Parameters: c.Parameters, Context: c.Context, - }, nil + } + + for _, cap := range c.VolumeCapabilities { + ccap, err := csi.VolumeCapabilityFromStructs( + cap.AttachmentMode, cap.AccessMode, c.MountOptions) + if err != nil { + return nil, err + } + creq.Capabilities = append(creq.Capabilities, ccap) + } + return creq, nil } type ClientCSIControllerValidateVolumeResponse struct { diff --git a/nomad/csi_endpoint.go b/nomad/csi_endpoint.go index 6a9560a3af01..f642441a724c 100644 --- a/nomad/csi_endpoint.go +++ b/nomad/csi_endpoint.go @@ -249,12 +249,11 @@ func (v *CSIVolume) controllerValidateVolume(req *structs.CSIVolumeRegisterReque method := "ClientCSI.ControllerValidateVolume" cReq := &cstructs.ClientCSIControllerValidateVolumeRequest{ - VolumeID: vol.RemoteID(), - AttachmentMode: vol.AttachmentMode, - AccessMode: vol.AccessMode, - Secrets: vol.Secrets, - Parameters: vol.Parameters, - Context: vol.Context, + VolumeID: vol.RemoteID(), + VolumeCapabilities: vol.RequestedCapabilities, + Secrets: vol.Secrets, + Parameters: vol.Parameters, + Context: vol.Context, } cReq.PluginID = plugin.ID cResp := &cstructs.ClientCSIControllerValidateVolumeResponse{} diff --git a/plugins/csi/client_test.go b/plugins/csi/client_test.go index 4233024b25a4..40da479b7dc3 100644 --- a/plugins/csi/client_test.go +++ b/plugins/csi/client_test.go @@ -664,14 +664,14 @@ func TestClient_RPC_ControllerValidateVolume(t *testing.T) { _, cc, _, client := newTestClient() defer client.Close() - requestedCaps := &VolumeCapability{ + requestedCaps := []*VolumeCapability{{ AccessType: tc.AccessType, AccessMode: tc.AccessMode, MountVolume: &structs.CSIMountOptions{ // should be ignored FSType: "ext4", MountFlags: []string{"noatime", "errors=remount-ro"}, }, - } + }} req := &ControllerValidateVolumeRequest{ ExternalID: "volumeID", Secrets: structs.CSISecrets{}, diff --git a/plugins/csi/plugin.go b/plugins/csi/plugin.go index e211c5a44c80..7019127c54e7 100644 --- a/plugins/csi/plugin.go +++ b/plugins/csi/plugin.go @@ -349,7 +349,7 @@ func NewControllerCapabilitySet(resp *csipbv1.ControllerGetCapabilitiesResponse) type ControllerValidateVolumeRequest struct { ExternalID string Secrets structs.CSISecrets - Capabilities *VolumeCapability + Capabilities []*VolumeCapability Parameters map[string]string Context map[string]string } @@ -359,14 +359,17 @@ func (r *ControllerValidateVolumeRequest) ToCSIRepresentation() *csipbv1.Validat return nil } + caps := make([]*csipbv1.VolumeCapability, 0, len(r.Capabilities)) + for _, cap := range r.Capabilities { + caps = append(caps, cap.ToCSIRepresentation()) + } + return &csipbv1.ValidateVolumeCapabilitiesRequest{ - VolumeId: r.ExternalID, - VolumeContext: r.Context, - VolumeCapabilities: []*csipbv1.VolumeCapability{ - r.Capabilities.ToCSIRepresentation(), - }, - Parameters: r.Parameters, - Secrets: r.Secrets, + VolumeId: r.ExternalID, + VolumeContext: r.Context, + VolumeCapabilities: caps, + Parameters: r.Parameters, + Secrets: r.Secrets, } }