luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

csi: refactor internal client field name to ExternalID (#7958) 

The CSI plugins RPCs require the use of the storage provider's volume
ID, rather than the user-defined volume ID. Although changing the RPCs
to use the field name `ExternalID` risks breaking backwards
compatibility, we can use the `ExternalID` name internally for the
client and only use `VolumeID` at the RPC boundaries.
This commit is contained in:
Tim Gross 2020-05-14 11:56:07 -04:00 committed by GitHub
parent 932710ad7d
commit 4f54a633a2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 34 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
client/pluginmanager/csimanager/volume.go
View File

@ -203,7 +203,7 @@ func (v *volumeManager) publishVolume(ctx context.Context, vol *structs.CSIVolum
// CSI NodePublishVolume errors for timeout, codes.Unavailable and
// codes.ResourceExhausted are retried; all other errors are fatal.
err = v.plugin.NodePublishVolume(ctx, &csi.NodePublishVolumeRequest{
VolumeID: vol.RemoteID(),
ExternalID: vol.RemoteID(),
PublishContext: publishContext,
StagingTargetPath: pluginStagingPath,
TargetPath: pluginTargetPath,

8
client/structs/csi.go
View File

@ -91,7 +91,7 @@ func (c *ClientCSIControllerAttachVolumeRequest) ToCSIRequest() (*csi.Controller
}
return &csi.ControllerPublishVolumeRequest{
VolumeID: c.VolumeID,
ExternalID: c.VolumeID,
NodeID: c.ClientCSINodeID,
VolumeCapability: caps,
ReadOnly: c.ReadOnly,
@ -121,7 +121,7 @@ type ClientCSIControllerAttachVolumeResponse struct {
}
type ClientCSIControllerDetachVolumeRequest struct {
// The ID of the volume to be unpublished for the node
// The external ID of the volume to be unpublished for the node
// This field is REQUIRED.
VolumeID string
@ -143,8 +143,8 @@ func (c *ClientCSIControllerDetachVolumeRequest) ToCSIRequest() *csi.ControllerU
}
return &csi.ControllerUnpublishVolumeRequest{
VolumeID: c.VolumeID,
NodeID: c.ClientCSINodeID,
ExternalID: c.VolumeID,
NodeID: c.ClientCSINodeID,
}
}

28
plugins/csi/client_test.go
View File

@ -387,13 +387,13 @@ func TestClient_RPC_ControllerPublishVolume(t *testing.T) {
{
Name: "Handles PublishContext == nil",
Request: &ControllerPublishVolumeRequest{VolumeID: "vol", NodeID: "node"},
Request: &ControllerPublishVolumeRequest{ExternalID: "vol", NodeID: "node"},
Response: &csipbv1.ControllerPublishVolumeResponse{},
ExpectedResponse: &ControllerPublishVolumeResponse{},
},
{
Name: "Handles PublishContext != nil",
Request: &ControllerPublishVolumeRequest{VolumeID: "vol", NodeID: "node"},
Request: &ControllerPublishVolumeRequest{ExternalID: "vol", NodeID: "node"},
Response: &csipbv1.ControllerPublishVolumeResponse{
PublishContext: map[string]string{
"com.hashicorp/nomad-node-id": "foobar",
@ -450,7 +450,7 @@ func TestClient_RPC_ControllerUnpublishVolume(t *testing.T) {
},
{
Name: "Handles successful response",
Request: &ControllerUnpublishVolumeRequest{VolumeID: "vol", NodeID: "node"},
Request: &ControllerUnpublishVolumeRequest{ExternalID: "vol", NodeID: "node"},
ExpectedErr: fmt.Errorf("missing NodeID"),
ExpectedResponse: &ControllerUnpublishVolumeResponse{},
},
@ -675,7 +675,7 @@ func TestClient_RPC_NodePublishVolume(t *testing.T) {
{
Name: "handles underlying grpc errors",
Request: &NodePublishVolumeRequest{
VolumeID: "foo",
ExternalID: "foo",
TargetPath: "/dev/null",
VolumeCapability: &VolumeCapability{},
},
@ -685,7 +685,7 @@ func TestClient_RPC_NodePublishVolume(t *testing.T) {
{
Name: "handles success",
Request: &NodePublishVolumeRequest{
VolumeID: "foo",
ExternalID: "foo",
TargetPath: "/dev/null",
VolumeCapability: &VolumeCapability{},
},
@ -695,10 +695,10 @@ func TestClient_RPC_NodePublishVolume(t *testing.T) {
{
Name: "Performs validation of the publish volume request",
Request: &NodePublishVolumeRequest{
VolumeID: "",
ExternalID: "",
},
ResponseErr: nil,
ExpectedErr: errors.New("missing VolumeID"),
ExpectedErr: errors.New("missing volume ID"),
},
}
@ -722,7 +722,7 @@ func TestClient_RPC_NodePublishVolume(t *testing.T) {
func TestClient_RPC_NodeUnpublishVolume(t *testing.T) {
cases := []struct {
Name string
VolumeID string
ExternalID string
TargetPath string
ResponseErr error
Response *csipbv1.NodeUnpublishVolumeResponse
@ -730,26 +730,26 @@ func TestClient_RPC_NodeUnpublishVolume(t *testing.T) {
}{
{
Name: "handles underlying grpc errors",
VolumeID: "foo",
ExternalID: "foo",
TargetPath: "/dev/null",
ResponseErr: fmt.Errorf("some grpc error"),
ExpectedErr: fmt.Errorf("some grpc error"),
},
{
Name: "handles success",
VolumeID: "foo",
ExternalID: "foo",
TargetPath: "/dev/null",
ResponseErr: nil,
ExpectedErr: nil,
},
{
Name: "Performs validation of the request args - VolumeID",
Name: "Performs validation of the request args - ExternalID",
ResponseErr: nil,
ExpectedErr: errors.New("missing VolumeID"),
ExpectedErr: errors.New("missing volume ID"),
},
{
Name: "Performs validation of the request args - TargetPath",
VolumeID: "foo",
ExternalID: "foo",
ResponseErr: nil,
ExpectedErr: errors.New("missing TargetPath"),
},
@ -763,7 +763,7 @@ func TestClient_RPC_NodeUnpublishVolume(t *testing.T) {
nc.NextErr = c.ResponseErr
nc.NextUnpublishVolumeResponse = c.Response
err := client.NodeUnpublishVolume(context.TODO(), c.VolumeID, c.TargetPath)
err := client.NodeUnpublishVolume(context.TODO(), c.ExternalID, c.TargetPath)
if c.ExpectedErr != nil {
require.Error(t, c.ExpectedErr, err)
} else {

30
plugins/csi/plugin.go
View File

@ -79,8 +79,8 @@ type CSIPlugin interface {
}
type NodePublishVolumeRequest struct {
// The ID of the volume to publish.
VolumeID string
// The external ID of the volume to publish.
ExternalID string
// If the volume was attached via a call to `ControllerPublishVolume` then
// we need to provide the returned PublishContext here.
@ -122,7 +122,7 @@ func (r *NodePublishVolumeRequest) ToCSIRepresentation() *csipbv1.NodePublishVol
}
return &csipbv1.NodePublishVolumeRequest{
VolumeId: r.VolumeID,
VolumeId: r.ExternalID,
PublishContext: r.PublishContext,
StagingTargetPath: r.StagingTargetPath,
TargetPath: r.TargetPath,
@ -133,8 +133,8 @@ func (r *NodePublishVolumeRequest) ToCSIRepresentation() *csipbv1.NodePublishVol
}
func (r *NodePublishVolumeRequest) Validate() error {
if r.VolumeID == "" {
return errors.New("missing VolumeID")
if r.ExternalID == "" {
return errors.New("missing volume ID")
}
if r.TargetPath == "" {
@ -230,7 +230,7 @@ func NewControllerCapabilitySet(resp *csipbv1.ControllerGetCapabilitiesResponse)
}
type ControllerPublishVolumeRequest struct {
VolumeID string
ExternalID string
NodeID string
ReadOnly bool
VolumeCapability *VolumeCapability
@ -244,7 +244,7 @@ func (r *ControllerPublishVolumeRequest) ToCSIRepresentation() *csipbv1.Controll
}
return &csipbv1.ControllerPublishVolumeRequest{
VolumeId: r.VolumeID,
VolumeId: r.ExternalID,
NodeId: r.NodeID,
Readonly: r.ReadOnly,
VolumeCapability: r.VolumeCapability.ToCSIRepresentation(),
@ -254,8 +254,8 @@ func (r *ControllerPublishVolumeRequest) ToCSIRepresentation() *csipbv1.Controll
}
func (r *ControllerPublishVolumeRequest) Validate() error {
if r.VolumeID == "" {
return errors.New("missing VolumeID")
if r.ExternalID == "" {
return errors.New("missing volume ID")
}
if r.NodeID == "" {
return errors.New("missing NodeID")
@ -268,9 +268,9 @@ type ControllerPublishVolumeResponse struct {
}
type ControllerUnpublishVolumeRequest struct {
VolumeID string
NodeID string
Secrets structs.CSISecrets
ExternalID string
NodeID string
Secrets structs.CSISecrets
}
func (r *ControllerUnpublishVolumeRequest) ToCSIRepresentation() *csipbv1.ControllerUnpublishVolumeRequest {
@ -279,15 +279,15 @@ func (r *ControllerUnpublishVolumeRequest) ToCSIRepresentation() *csipbv1.Contro
}
return &csipbv1.ControllerUnpublishVolumeRequest{
VolumeId: r.VolumeID,
VolumeId: r.ExternalID,
NodeId: r.NodeID,
Secrets: r.Secrets,
}
}
func (r *ControllerUnpublishVolumeRequest) Validate() error {
if r.VolumeID == "" {
return errors.New("missing VolumeID")
if r.ExternalID == "" {
return errors.New("missing ExternalID")
}
if r.NodeID == "" {
// the spec allows this but it would unpublish the