Skip to content

Commit

Permalink
csi: support for VolumeContext and VolumeParameters (#7957)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tgross committed May 15, 2020
1 parent f2210ce commit 103d873
Show file tree
Hide file tree
Showing 11 changed files with 117 additions and 34 deletions.
2 changes: 2 additions & 0 deletions api/csi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions client/csi_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)))
Expand Down
33 changes: 29 additions & 4 deletions client/structs/csi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
}

Expand Down
5 changes: 3 additions & 2 deletions nomad/csi_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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{}
Expand Down
2 changes: 2 additions & 0 deletions nomad/mock/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down
14 changes: 14 additions & 0 deletions nomad/structs/csi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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{}
}
Expand Down Expand Up @@ -388,6 +396,12 @@ func (v *CSIVolume) Copy() *CSIVolume {
copy := *v
out := &copy
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
}
Expand Down
23 changes: 7 additions & 16 deletions plugins/csi/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,33 +294,24 @@ 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")
}
if c.controllerClient == nil {
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
}
Expand All @@ -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)
Expand Down
11 changes: 9 additions & 2 deletions plugins/csi/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion plugins/csi/fake/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
30 changes: 27 additions & 3 deletions plugins/csi/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
}
}

Expand Down
24 changes: 21 additions & 3 deletions website/pages/docs/commands/volume/register.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -87,9 +93,21 @@ secrets {
- `fs_type`: file system type (ex. `"ext4"`)
- `mount_flags`: the flags passed to `mount` (ex. `"ro,noatime"`)

- `secrets` <code>(map:nil)</code> - An optional key-value map of
strings used as credentials for publishing and unpublishing volumes.

- `secrets` <code>(map<string|string>:nil)</code> - An optional
key-value map of strings used as credentials for publishing and
unpublishing volumes.

- `parameters` <code>(map<string|string>:nil)</code> - 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` <code>(map<string|string>:nil)</code> - 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
Expand Down

0 comments on commit 103d873

Please sign in to comment.