From 2082cf738a7bc728c788471dd51ce7182b6843eb 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 | 22 +++++++++++-- 11 files changed, 116 insertions(+), 33 deletions(-) diff --git a/api/csi.go b/api/csi.go index d7e94d6f1..94a54934d 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 71e42b1f9..6f9bf6e28 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 070dc2382..ba6a46e0d 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 855e3e425..5bf559365 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 85e10d090..23738503c 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 f017cab00..12126de5d 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 99c0cad38..ddde111df 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 4b5ae1334..0fdfb92a9 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 77fa5c514..b54cb144d 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 d91df8a1f..297b63f22 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 355785521..815f80135 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