Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

csi: refactor internal client field name to ExternalID #7958

Merged
merged 1 commit into from
May 14, 2020
Merged

csi: refactor internal client field name to ExternalID #7958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/pluginmanager/csimanager/volume.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions client/structs/csi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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

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

Expand Down
28 changes: 14 additions & 14 deletions plugins/csi/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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{},
},
Expand Down Expand Up @@ -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{},
},
Expand All @@ -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{},
},
Expand All @@ -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"),
},
}

Expand All @@ -722,34 +722,34 @@ 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
ExpectedErr error
}{
{
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"),
},
Expand All @@ -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 {
Expand Down
30 changes: 15 additions & 15 deletions plugins/csi/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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 == "" {
Expand Down Expand Up @@ -230,7 +230,7 @@ func NewControllerCapabilitySet(resp *csipbv1.ControllerGetCapabilitiesResponse)
}

type ControllerPublishVolumeRequest struct {
VolumeID string
ExternalID string
NodeID string
ReadOnly bool
VolumeCapability *VolumeCapability
Expand All @@ -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(),
Expand All @@ -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")
Expand All @@ -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 {
Expand All @@ -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
Expand Down