From 19d06d5bb2e8831b61be22e9f3a0aaace247f49e Mon Sep 17 00:00:00 2001 From: Danielle Lancashire Date: Tue, 17 Dec 2019 12:37:33 +0100 Subject: [PATCH] csi: ClientCSIControllerPublish* -> ClientCSIControllerAttach* --- client/client_csi_endpoint.go | 10 ++++-- client/client_csi_endpoint_test.go | 55 +++++++++++++++++++++--------- client/structs/csi.go | 6 ++-- 3 files changed, 50 insertions(+), 21 deletions(-) diff --git a/client/client_csi_endpoint.go b/client/client_csi_endpoint.go index 73c67ab9d..dcc57fc69 100644 --- a/client/client_csi_endpoint.go +++ b/client/client_csi_endpoint.go @@ -28,9 +28,15 @@ var ( ErrPluginTypeError = errors.New("CSI Plugin loaded incorrectly") ) -// CSIControllerPublishVolume is used to attach a volume from a CSI Cluster to +// CSIControllerAttachVolume is used to attach a volume from a CSI Cluster to // the storage node provided in the request. -func (c *ClientCSI) CSIControllerPublishVolume(req *structs.ClientCSIControllerPublishVolumeRequest, resp *structs.ClientCSIControllerPublishVolumeResponse) error { +// +// The controller attachment flow currently works as follows: +// 1. Validate the volume request +// 2. Call ControllerPublishVolume on the CSI Plugin to trigger a remote attachment +// +// In the future this may be expanded to request dynamic secrets for attachement. +func (c *ClientCSI) CSIControllerAttachVolume(req *structs.ClientCSIControllerAttachVolumeRequest, resp *structs.ClientCSIControllerAttachVolumeResponse) error { defer metrics.MeasureSince([]string{"client", "csi_controller", "publish_volume"}, time.Now()) client, err := c.findControllerPlugin(req.PluginName) if err != nil { diff --git a/client/client_csi_endpoint_test.go b/client/client_csi_endpoint_test.go index f349bbe0e..8d07b88c6 100644 --- a/client/client_csi_endpoint_test.go +++ b/client/client_csi_endpoint_test.go @@ -17,47 +17,70 @@ var fakePlugin = &dynamicplugins.PluginInfo{ ConnectionInfo: &dynamicplugins.PluginConnectionInfo{}, } -func TestClientCSI_CSIControllerPublishVolume(t *testing.T) { +func TestClientCSI_CSIControllerAttachVolume(t *testing.T) { t.Parallel() cases := []struct { Name string ClientSetupFunc func(*fake.Client) - Request *structs.ClientCSIControllerPublishVolumeRequest + Request *structs.ClientCSIControllerAttachVolumeRequest ExpectedErr error - ExpectedResponse *structs.ClientCSIControllerPublishVolumeResponse + ExpectedResponse *structs.ClientCSIControllerAttachVolumeResponse }{ { Name: "returns plugin not found errors", - Request: &structs.ClientCSIControllerPublishVolumeRequest{ + Request: &structs.ClientCSIControllerAttachVolumeRequest{ PluginName: "some-garbage", }, ExpectedErr: errors.New("plugin some-garbage for type csi-controller not found"), }, { Name: "validates volumeid is not empty", - Request: &structs.ClientCSIControllerPublishVolumeRequest{ + Request: &structs.ClientCSIControllerAttachVolumeRequest{ PluginName: fakePlugin.Name, }, ExpectedErr: errors.New("VolumeID is required"), }, { Name: "validates nodeid is not empty", - Request: &structs.ClientCSIControllerPublishVolumeRequest{ + Request: &structs.ClientCSIControllerAttachVolumeRequest{ PluginName: fakePlugin.Name, VolumeID: "1234-4321-1234-4321", }, ExpectedErr: errors.New("NodeID is required"), }, + { + Name: "validates AccessMode", + Request: &structs.ClientCSIControllerAttachVolumeRequest{ + PluginName: fakePlugin.Name, + VolumeID: "1234-4321-1234-4321", + NodeID: "abcde", + AccessMode: structs.CSIVolumeAccessMode("foo"), + }, + ExpectedErr: errors.New("Unknown access mode: foo"), + }, + { + Name: "validates attachmentmode is not empty", + Request: &structs.ClientCSIControllerAttachVolumeRequest{ + PluginName: fakePlugin.Name, + VolumeID: "1234-4321-1234-4321", + NodeID: "abcde", + AccessMode: structs.CSIVolumeAccessModeMultiNodeReader, + AttachmentMode: structs.CSIVolumeAttachmentMode("bar"), + }, + ExpectedErr: errors.New("Unknown attachment mode: bar"), + }, { Name: "returns transitive errors", ClientSetupFunc: func(fc *fake.Client) { fc.NextControllerPublishVolumeErr = errors.New("hello") }, - Request: &structs.ClientCSIControllerPublishVolumeRequest{ - PluginName: fakePlugin.Name, - VolumeID: "1234-4321-1234-4321", - NodeID: "abcde", + Request: &structs.ClientCSIControllerAttachVolumeRequest{ + PluginName: fakePlugin.Name, + VolumeID: "1234-4321-1234-4321", + NodeID: "abcde", + AccessMode: structs.CSIVolumeAccessModeSingleNodeWriter, + AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem, }, ExpectedErr: errors.New("hello"), }, @@ -66,12 +89,12 @@ func TestClientCSI_CSIControllerPublishVolume(t *testing.T) { ClientSetupFunc: func(fc *fake.Client) { fc.NextControllerPublishVolumeResponse = &csi.ControllerPublishVolumeResponse{} }, - Request: &structs.ClientCSIControllerPublishVolumeRequest{ + Request: &structs.ClientCSIControllerAttachVolumeRequest{ PluginName: fakePlugin.Name, VolumeID: "1234-4321-1234-4321", NodeID: "abcde", }, - ExpectedResponse: &structs.ClientCSIControllerPublishVolumeResponse{}, + ExpectedResponse: &structs.ClientCSIControllerAttachVolumeResponse{}, }, { Name: "handles non-nil PublishContext", @@ -80,12 +103,12 @@ func TestClientCSI_CSIControllerPublishVolume(t *testing.T) { PublishContext: map[string]string{"foo": "bar"}, } }, - Request: &structs.ClientCSIControllerPublishVolumeRequest{ + Request: &structs.ClientCSIControllerAttachVolumeRequest{ PluginName: fakePlugin.Name, VolumeID: "1234-4321-1234-4321", NodeID: "abcde", }, - ExpectedResponse: &structs.ClientCSIControllerPublishVolumeResponse{ + ExpectedResponse: &structs.ClientCSIControllerAttachVolumeResponse{ PublishContext: map[string]string{"foo": "bar"}, }, }, @@ -110,8 +133,8 @@ func TestClientCSI_CSIControllerPublishVolume(t *testing.T) { err := client.dynamicRegistry.RegisterPlugin(fakePlugin) require.Nil(err) - var resp structs.ClientCSIControllerPublishVolumeResponse - err = client.ClientRPC("ClientCSI.CSIControllerPublishVolume", tc.Request, &resp) + var resp structs.ClientCSIControllerAttachVolumeResponse + err = client.ClientRPC("ClientCSI.CSIControllerAttachVolume", tc.Request, &resp) require.Equal(tc.ExpectedErr, err) if tc.ExpectedResponse != nil { require.Equal(tc.ExpectedResponse, &resp) diff --git a/client/structs/csi.go b/client/structs/csi.go index d383ccb30..0e37101c0 100644 --- a/client/structs/csi.go +++ b/client/structs/csi.go @@ -64,7 +64,7 @@ type CSIVolumeMountOptions struct { MountFlags []string } -type ClientCSIControllerPublishVolumeRequest struct { +type ClientCSIControllerAttachVolumeRequest struct { PluginName string // The ID of the volume to be used on a node. @@ -91,7 +91,7 @@ type ClientCSIControllerPublishVolumeRequest struct { ReadOnly bool } -func (c *ClientCSIControllerPublishVolumeRequest) ToCSIRequest() *csi.ControllerPublishVolumeRequest { +func (c *ClientCSIControllerAttachVolumeRequest) ToCSIRequest() *csi.ControllerPublishVolumeRequest { if c == nil { return &csi.ControllerPublishVolumeRequest{} } @@ -103,7 +103,7 @@ func (c *ClientCSIControllerPublishVolumeRequest) ToCSIRequest() *csi.Controller } } -type ClientCSIControllerPublishVolumeResponse struct { +type ClientCSIControllerAttachVolumeResponse struct { // Opaque static publish properties of the volume. SP MAY use this // field to ensure subsequent `NodeStageVolume` or `NodePublishVolume` // calls calls have contextual information.