From 103d873ebe84cb57cbb09a599112a39c69592ac1 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Fri, 15 May 2020 08:16:01 -0400 Subject: [PATCH] csi: support for VolumeContext and VolumeParameters (#7957) The MVP for CSI in the 0.11.0 release of Nomad did not include support for opaque volume parameters or volume context. This changeset adds support for both. This also moves args for ControllerValidateCapabilities into a struct. The CSI plugin `ControllerValidateCapabilities` struct that we turn into a CSI RPC is accumulating arguments, so moving it into a request struct will reduce the churn of this internal API, make the plugin code more readable, and make this method consistent with the other plugin methods in that package. --- api/csi.go | 2 ++ client/csi_endpoint.go | 5 ++- client/structs/csi.go | 33 ++++++++++++++++--- nomad/csi_endpoint.go | 5 +-- nomad/mock/mock.go | 2 ++ nomad/structs/csi.go | 14 ++++++++ plugins/csi/client.go | 23 ++++--------- plugins/csi/client_test.go | 11 +++++-- plugins/csi/fake/client.go | 2 +- plugins/csi/plugin.go | 30 +++++++++++++++-- .../pages/docs/commands/volume/register.mdx | 24 ++++++++++++-- 11 files changed, 117 insertions(+), 34 deletions(-) diff --git a/api/csi.go b/api/csi.go index d7e94d6f1732..94a54934d27c 100644 --- a/api/csi.go +++ b/api/csi.go @@ -100,6 +100,8 @@ type CSIVolume struct { AttachmentMode CSIVolumeAttachmentMode `hcl:"attachment_mode"` MountOptions *CSIMountOptions `hcl:"mount_options"` Secrets CSISecrets `hcl:"secrets"` + Parameters map[string]string `hcl:"parameters"` + Context map[string]string `hcl:"context"` // Allocations, tracking claim status ReadAllocs map[string]*Allocation diff --git a/client/csi_endpoint.go b/client/csi_endpoint.go index 71e42b1f9435..6f9bf6e28fca 100644 --- a/client/csi_endpoint.go +++ b/client/csi_endpoint.go @@ -50,7 +50,7 @@ func (c *CSI) ControllerValidateVolume(req *structs.ClientCSIControllerValidateV } defer plugin.Close() - caps, err := csi.VolumeCapabilityFromStructs(req.AttachmentMode, req.AccessMode) + csiReq, err := req.ToCSIRequest() if err != nil { return err } @@ -60,8 +60,7 @@ func (c *CSI) ControllerValidateVolume(req *structs.ClientCSIControllerValidateV // CSI ValidateVolumeCapabilities errors for timeout, codes.Unavailable and // codes.ResourceExhausted are retried; all other errors are fatal. - return plugin.ControllerValidateCapabilities(ctx, req.VolumeID, caps, - req.Secrets, + return plugin.ControllerValidateCapabilities(ctx, csiReq, grpc_retry.WithPerRetryTimeout(CSIPluginRequestTimeout), grpc_retry.WithMax(3), grpc_retry.WithBackoff(grpc_retry.BackoffExponential(100*time.Millisecond))) diff --git a/client/structs/csi.go b/client/structs/csi.go index 070dc2382b9b..ba6a46e0dc8b 100644 --- a/client/structs/csi.go +++ b/client/structs/csi.go @@ -36,11 +36,37 @@ type ClientCSIControllerValidateVolumeRequest struct { AttachmentMode structs.CSIVolumeAttachmentMode AccessMode structs.CSIVolumeAccessMode Secrets structs.CSISecrets - // Parameters map[string]string // TODO: https://github.com/hashicorp/nomad/issues/7670 + + // Parameters as returned by storage provider in CreateVolumeResponse. + // This field is optional. + Parameters map[string]string + + // Volume context as returned by storage provider in CreateVolumeResponse. + // This field is optional. + Context map[string]string CSIControllerQuery } +func (c *ClientCSIControllerValidateVolumeRequest) ToCSIRequest() (*csi.ControllerValidateVolumeRequest, error) { + if c == nil { + return &csi.ControllerValidateVolumeRequest{}, nil + } + + caps, err := csi.VolumeCapabilityFromStructs(c.AttachmentMode, c.AccessMode) + if err != nil { + return nil, err + } + + return &csi.ControllerValidateVolumeRequest{ + ExternalID: c.VolumeID, + Secrets: c.Secrets, + Capabilities: caps, + Parameters: c.Parameters, + Context: c.Context, + }, nil +} + type ClientCSIControllerValidateVolumeResponse struct { } @@ -72,10 +98,9 @@ type ClientCSIControllerAttachVolumeRequest struct { // volume request. This field is OPTIONAL. Secrets structs.CSISecrets - // TODO https://github.com/hashicorp/nomad/issues/7771 // Volume context as returned by storage provider in CreateVolumeResponse. // This field is optional. - // VolumeContext map[string]string + VolumeContext map[string]string CSIControllerQuery } @@ -96,7 +121,7 @@ func (c *ClientCSIControllerAttachVolumeRequest) ToCSIRequest() (*csi.Controller VolumeCapability: caps, ReadOnly: c.ReadOnly, Secrets: c.Secrets, - // VolumeContext: c.VolumeContext, TODO: https://github.com/hashicorp/nomad/issues/7771 + VolumeContext: c.VolumeContext, }, nil } diff --git a/nomad/csi_endpoint.go b/nomad/csi_endpoint.go index 855e3e42512a..5bf559365b93 100644 --- a/nomad/csi_endpoint.go +++ b/nomad/csi_endpoint.go @@ -238,7 +238,8 @@ func (v *CSIVolume) controllerValidateVolume(req *structs.CSIVolumeRegisterReque AttachmentMode: vol.AttachmentMode, AccessMode: vol.AccessMode, Secrets: vol.Secrets, - // Parameters: TODO: https://github.com/hashicorp/nomad/issues/7670 + Parameters: vol.Parameters, + Context: vol.Context, } cReq.PluginID = plugin.ID cResp := &cstructs.ClientCSIControllerValidateVolumeResponse{} @@ -443,7 +444,7 @@ func (v *CSIVolume) controllerPublishVolume(req *structs.CSIVolumeClaimRequest, AccessMode: vol.AccessMode, ReadOnly: req.Claim == structs.CSIVolumeClaimRead, Secrets: vol.Secrets, - // VolumeContext: TODO https://github.com/hashicorp/nomad/issues/7771 + VolumeContext: vol.Context, } cReq.PluginID = plug.ID cResp := &cstructs.ClientCSIControllerAttachVolumeResponse{} diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go index 85e10d09041e..23738503cfdf 100644 --- a/nomad/mock/mock.go +++ b/nomad/mock/mock.go @@ -1312,6 +1312,8 @@ func CSIVolume(plugin *structs.CSIPlugin) *structs.CSIVolume { AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem, MountOptions: &structs.CSIMountOptions{}, Secrets: structs.CSISecrets{}, + Parameters: map[string]string{}, + Context: map[string]string{}, ReadAllocs: map[string]*structs.Allocation{}, WriteAllocs: map[string]*structs.Allocation{}, ReadClaims: map[string]*structs.CSIVolumeClaim{}, diff --git a/nomad/structs/csi.go b/nomad/structs/csi.go index f017cab00348..12126de5d9aa 100644 --- a/nomad/structs/csi.go +++ b/nomad/structs/csi.go @@ -236,6 +236,8 @@ type CSIVolume struct { AttachmentMode CSIVolumeAttachmentMode MountOptions *CSIMountOptions Secrets CSISecrets + Parameters map[string]string + Context map[string]string // Allocations, tracking claim status ReadAllocs map[string]*Allocation // AllocID -> Allocation @@ -300,6 +302,12 @@ func (v *CSIVolume) newStructs() { if v.Topologies == nil { v.Topologies = []*CSITopology{} } + if v.Context == nil { + v.Context = map[string]string{} + } + if v.Parameters == nil { + v.Parameters = map[string]string{} + } if v.Secrets == nil { v.Secrets = CSISecrets{} } @@ -388,6 +396,12 @@ func (v *CSIVolume) Copy() *CSIVolume { copy := *v out := © out.newStructs() + for k, v := range v.Parameters { + out.Parameters[k] = v + } + for k, v := range v.Context { + out.Context[k] = v + } for k, v := range v.Secrets { out.Secrets[k] = v } diff --git a/plugins/csi/client.go b/plugins/csi/client.go index 99c0cad3848d..ddde111dff91 100644 --- a/plugins/csi/client.go +++ b/plugins/csi/client.go @@ -294,7 +294,7 @@ func (c *client) ControllerUnpublishVolume(ctx context.Context, req *ControllerU return &ControllerUnpublishVolumeResponse{}, nil } -func (c *client) ControllerValidateCapabilities(ctx context.Context, volumeID string, capabilities *VolumeCapability, secrets structs.CSISecrets, opts ...grpc.CallOption) error { +func (c *client) ControllerValidateCapabilities(ctx context.Context, req *ControllerValidateVolumeRequest, opts ...grpc.CallOption) error { if c == nil { return fmt.Errorf("Client not initialized") } @@ -302,25 +302,16 @@ func (c *client) ControllerValidateCapabilities(ctx context.Context, volumeID st return fmt.Errorf("controllerClient not initialized") } - if volumeID == "" { - return fmt.Errorf("missing VolumeID") + if req.ExternalID == "" { + return fmt.Errorf("missing volume ID") } - if capabilities == nil { + if req.Capabilities == nil { return fmt.Errorf("missing Capabilities") } - req := &csipbv1.ValidateVolumeCapabilitiesRequest{ - VolumeId: volumeID, - VolumeCapabilities: []*csipbv1.VolumeCapability{ - capabilities.ToCSIRepresentation(), - }, - // VolumeContext: map[string]string // TODO: https://github.com/hashicorp/nomad/issues/7771 - // Parameters: map[string]string // TODO: https://github.com/hashicorp/nomad/issues/7670 - Secrets: secrets, - } - - resp, err := c.controllerClient.ValidateVolumeCapabilities(ctx, req, opts...) + creq := req.ToCSIRepresentation() + resp, err := c.controllerClient.ValidateVolumeCapabilities(ctx, creq, opts...) if err != nil { return err } @@ -338,7 +329,7 @@ func (c *client) ControllerValidateCapabilities(ctx context.Context, volumeID st // the volume is ok. confirmedCaps := resp.GetConfirmed().GetVolumeCapabilities() if confirmedCaps != nil { - for _, requestedCap := range req.VolumeCapabilities { + for _, requestedCap := range creq.VolumeCapabilities { err := compareCapabilities(requestedCap, confirmedCaps) if err != nil { return fmt.Errorf("volume capability validation failed: %v", err) diff --git a/plugins/csi/client_test.go b/plugins/csi/client_test.go index 4b5ae1334c31..0fdfb92a9d81 100644 --- a/plugins/csi/client_test.go +++ b/plugins/csi/client_test.go @@ -574,11 +574,18 @@ func TestClient_RPC_ControllerValidateVolume(t *testing.T) { MountFlags: []string{"noatime", "errors=remount-ro"}, }, } + req := &ControllerValidateVolumeRequest{ + ExternalID: "volumeID", + Secrets: structs.CSISecrets{}, + Capabilities: requestedCaps, + Parameters: map[string]string{}, + Context: map[string]string{}, + } + cc.NextValidateVolumeCapabilitiesResponse = c.Response cc.NextErr = c.ResponseErr - err := client.ControllerValidateCapabilities( - context.TODO(), "volumeID", requestedCaps, structs.CSISecrets{}) + err := client.ControllerValidateCapabilities(context.TODO(), req) if c.ExpectedErr != nil { require.Error(t, c.ExpectedErr, err, c.Name) } else { diff --git a/plugins/csi/fake/client.go b/plugins/csi/fake/client.go index 77fa5c514817..b54cb144d634 100644 --- a/plugins/csi/fake/client.go +++ b/plugins/csi/fake/client.go @@ -160,7 +160,7 @@ func (c *Client) ControllerUnpublishVolume(ctx context.Context, req *csi.Control return c.NextControllerUnpublishVolumeResponse, c.NextControllerUnpublishVolumeErr } -func (c *Client) ControllerValidateCapabilities(ctx context.Context, volumeID string, capabilities *csi.VolumeCapability, secrets structs.CSISecrets, opts ...grpc.CallOption) error { +func (c *Client) ControllerValidateCapabilities(ctx context.Context, req *csi.ControllerValidateVolumeRequest, opts ...grpc.CallOption) error { c.Mu.Lock() defer c.Mu.Unlock() diff --git a/plugins/csi/plugin.go b/plugins/csi/plugin.go index d91df8a1fdd2..297b63f22932 100644 --- a/plugins/csi/plugin.go +++ b/plugins/csi/plugin.go @@ -43,7 +43,7 @@ type CSIPlugin interface { // ControllerValidateCapabilities is used to validate that a volume exists and // supports the requested capability. - ControllerValidateCapabilities(ctx context.Context, volumeID string, capabilities *VolumeCapability, secrets structs.CSISecrets, opts ...grpc.CallOption) error + ControllerValidateCapabilities(ctx context.Context, req *ControllerValidateVolumeRequest, opts ...grpc.CallOption) error // NodeGetCapabilities is used to return the available capabilities from the // Node Service. @@ -229,13 +229,37 @@ func NewControllerCapabilitySet(resp *csipbv1.ControllerGetCapabilitiesResponse) return cs } +type ControllerValidateVolumeRequest struct { + ExternalID string + Secrets structs.CSISecrets + Capabilities *VolumeCapability + Parameters map[string]string + Context map[string]string +} + +func (r *ControllerValidateVolumeRequest) ToCSIRepresentation() *csipbv1.ValidateVolumeCapabilitiesRequest { + if r == nil { + return nil + } + + return &csipbv1.ValidateVolumeCapabilitiesRequest{ + VolumeId: r.ExternalID, + VolumeContext: r.Context, + VolumeCapabilities: []*csipbv1.VolumeCapability{ + r.Capabilities.ToCSIRepresentation(), + }, + Parameters: r.Parameters, + Secrets: r.Secrets, + } +} + type ControllerPublishVolumeRequest struct { ExternalID string NodeID string ReadOnly bool VolumeCapability *VolumeCapability Secrets structs.CSISecrets - // VolumeContext map[string]string // TODO: https://github.com/hashicorp/nomad/issues/7771 + VolumeContext map[string]string } func (r *ControllerPublishVolumeRequest) ToCSIRepresentation() *csipbv1.ControllerPublishVolumeRequest { @@ -249,7 +273,7 @@ func (r *ControllerPublishVolumeRequest) ToCSIRepresentation() *csipbv1.Controll Readonly: r.ReadOnly, VolumeCapability: r.VolumeCapability.ToCSIRepresentation(), Secrets: r.Secrets, - // VolumeContext: r.VolumeContext, https://github.com/hashicorp/nomad/issues/7771 + VolumeContext: r.VolumeContext, } } diff --git a/website/pages/docs/commands/volume/register.mdx b/website/pages/docs/commands/volume/register.mdx index 355785521039..815f80135c7c 100644 --- a/website/pages/docs/commands/volume/register.mdx +++ b/website/pages/docs/commands/volume/register.mdx @@ -49,6 +49,12 @@ mount_options { secrets { example_secret = "xyzzy" } +parameters { + skuname = "Premium_LRS" +} +context { + endpoint = "http://192.168.1.101:9425" +} ``` ## Volume Specification Parameters @@ -87,9 +93,21 @@ secrets { - `fs_type`: file system type (ex. `"ext4"`) - `mount_flags`: the flags passed to `mount` (ex. `"ro,noatime"`) -- `secrets` (map:nil) - An optional key-value map of - strings used as credentials for publishing and unpublishing volumes. - +- `secrets` (map:nil) - An optional + key-value map of strings used as credentials for publishing and + unpublishing volumes. + +- `parameters` (map:nil) - An optional + key-value map of strings passed directly to the CSI plugin to + configure the volume. The details of these parameters are specific + to each storage provider, so please see the specific plugin + documentation for more information. + +- `context` (map:nil) - An optional + key-value map of strings passed directly to the CSI plugin to + validate the volume. The details of these parameters are specific to + each storage provider, so please see the specific plugin + documentation for more information. [volume_specification]: #volume-specification [csi]: https://github.com/container-storage-interface/spec