From c8329b47cea3f7b2ecdb34f1949acef74c899a45 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Thu, 14 May 2020 11:47:21 -0400 Subject: [PATCH 1/3] csi: move args for ControllerValidateCapabilities into 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. --- client/csi_endpoint.go | 5 ++--- client/structs/csi.go | 17 +++++++++++++++++ plugins/csi/client.go | 23 +++++++---------------- plugins/csi/client_test.go | 9 +++++++-- plugins/csi/fake/client.go | 2 +- plugins/csi/plugin.go | 22 +++++++++++++++++++++- 6 files changed, 55 insertions(+), 23 deletions(-) diff --git a/client/csi_endpoint.go b/client/csi_endpoint.go index 71e42b1f943..6f9bf6e28fc 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 070dc2382b9..92e20212670 100644 --- a/client/structs/csi.go +++ b/client/structs/csi.go @@ -41,6 +41,23 @@ type ClientCSIControllerValidateVolumeRequest struct { 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, + }, nil +} + type ClientCSIControllerValidateVolumeResponse struct { } diff --git a/plugins/csi/client.go b/plugins/csi/client.go index 99c0cad3848..ddde111dff9 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 4b5ae1334c3..3f084e1dfae 100644 --- a/plugins/csi/client_test.go +++ b/plugins/csi/client_test.go @@ -574,11 +574,16 @@ func TestClient_RPC_ControllerValidateVolume(t *testing.T) { MountFlags: []string{"noatime", "errors=remount-ro"}, }, } + req := &ControllerValidateVolumeRequest{ + ExternalID: "volumeID", + Secrets: structs.CSISecrets{}, + Capabilities: requestedCaps, + } + 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 77fa5c51481..b54cb144d63 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 d91df8a1fdd..bb25c642131 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,6 +229,26 @@ func NewControllerCapabilitySet(resp *csipbv1.ControllerGetCapabilitiesResponse) return cs } +type ControllerValidateVolumeRequest struct { + ExternalID string + Secrets structs.CSISecrets + Capabilities *VolumeCapability +} + +func (r *ControllerValidateVolumeRequest) ToCSIRepresentation() *csipbv1.ValidateVolumeCapabilitiesRequest { + if r == nil { + return nil + } + + return &csipbv1.ValidateVolumeCapabilitiesRequest{ + VolumeId: r.ExternalID, + VolumeCapabilities: []*csipbv1.VolumeCapability{ + r.Capabilities.ToCSIRepresentation(), + }, + Secrets: r.Secrets, + } +} + type ControllerPublishVolumeRequest struct { ExternalID string NodeID string From b11f793f6193fe84ca859a24ffb8c3b3779f1693 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Thu, 14 May 2020 11:23:05 -0400 Subject: [PATCH 2/3] csi: support for VolumeContext and VolumeParameters --- api/csi.go | 2 ++ client/structs/csi.go | 16 ++++++++++++---- nomad/csi_endpoint.go | 5 +++-- nomad/mock/mock.go | 2 ++ nomad/structs/csi.go | 14 ++++++++++++++ plugins/csi/client_test.go | 2 ++ plugins/csi/plugin.go | 12 ++++++++---- 7 files changed, 43 insertions(+), 10 deletions(-) diff --git a/api/csi.go b/api/csi.go index d7e94d6f173..94a54934d27 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/structs/csi.go b/client/structs/csi.go index 92e20212670..ba6a46e0dc8 100644 --- a/client/structs/csi.go +++ b/client/structs/csi.go @@ -36,7 +36,14 @@ 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 } @@ -55,6 +62,8 @@ func (c *ClientCSIControllerValidateVolumeRequest) ToCSIRequest() (*csi.Controll ExternalID: c.VolumeID, Secrets: c.Secrets, Capabilities: caps, + Parameters: c.Parameters, + Context: c.Context, }, nil } @@ -89,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 } @@ -113,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 855e3e42512..5bf559365b9 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 85e10d09041..23738503cfd 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 f017cab0034..12126de5d9a 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_test.go b/plugins/csi/client_test.go index 3f084e1dfae..0fdfb92a9d8 100644 --- a/plugins/csi/client_test.go +++ b/plugins/csi/client_test.go @@ -578,6 +578,8 @@ func TestClient_RPC_ControllerValidateVolume(t *testing.T) { ExternalID: "volumeID", Secrets: structs.CSISecrets{}, Capabilities: requestedCaps, + Parameters: map[string]string{}, + Context: map[string]string{}, } cc.NextValidateVolumeCapabilitiesResponse = c.Response diff --git a/plugins/csi/plugin.go b/plugins/csi/plugin.go index bb25c642131..297b63f2293 100644 --- a/plugins/csi/plugin.go +++ b/plugins/csi/plugin.go @@ -233,6 +233,8 @@ type ControllerValidateVolumeRequest struct { ExternalID string Secrets structs.CSISecrets Capabilities *VolumeCapability + Parameters map[string]string + Context map[string]string } func (r *ControllerValidateVolumeRequest) ToCSIRepresentation() *csipbv1.ValidateVolumeCapabilitiesRequest { @@ -241,11 +243,13 @@ func (r *ControllerValidateVolumeRequest) ToCSIRepresentation() *csipbv1.Validat } return &csipbv1.ValidateVolumeCapabilitiesRequest{ - VolumeId: r.ExternalID, + VolumeId: r.ExternalID, + VolumeContext: r.Context, VolumeCapabilities: []*csipbv1.VolumeCapability{ r.Capabilities.ToCSIRepresentation(), }, - Secrets: r.Secrets, + Parameters: r.Parameters, + Secrets: r.Secrets, } } @@ -255,7 +259,7 @@ type ControllerPublishVolumeRequest struct { 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 { @@ -269,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, } } From d03f348a492499c603b886bb6f49579aff150d30 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Thu, 14 May 2020 09:46:00 -0400 Subject: [PATCH 3/3] documentation updates for volume context and params --- .../pages/docs/commands/volume/register.mdx | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/website/pages/docs/commands/volume/register.mdx b/website/pages/docs/commands/volume/register.mdx index 35578552103..815f80135c7 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