Skip to content

Commit

Permalink
feat: implement GET_VOLUME
Browse files Browse the repository at this point in the history
Report more information in volumeCondition for GET and LIST.
  • Loading branch information
guilhem committed Nov 26, 2024
1 parent 17b7d1b commit ff7c61d
Show file tree
Hide file tree
Showing 3 changed files with 293 additions and 36 deletions.
1 change: 1 addition & 0 deletions internal/driver/capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ func ControllerServiceCapabilities() []*csi.ControllerServiceCapability {
csi.ControllerServiceCapability_RPC_CLONE_VOLUME,
csi.ControllerServiceCapability_RPC_LIST_VOLUMES,
csi.ControllerServiceCapability_RPC_VOLUME_CONDITION,
csi.ControllerServiceCapability_RPC_GET_VOLUME,
}

cc := make([]*csi.ControllerServiceCapability, 0, len(capabilities))
Expand Down
112 changes: 83 additions & 29 deletions internal/driver/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,37 +377,12 @@ func (cs *ControllerServer) ListVolumes(ctx context.Context, req *csi.ListVolume

entries := make([]*csi.ListVolumesResponse_Entry, 0, len(volumes))
for volNum := range volumes {
key := linodevolumes.CreateLinodeVolumeKey(volumes[volNum].ID, volumes[volNum].Label)

// If the volume is attached to a Linode instance, add it to the
// list. Note that in the Linode API, volumes can only be
// attached to a single Linode at a time. We are storing it in
// a []string here, since that is what the response struct
// returns. We do not need to pre-allocate the slice with
// make(), since the CSI specification says this response field
// is optional, and thus it should tolerate a nil slice.
var publishedNodeIDs []string
if volumes[volNum].LinodeID != nil {
publishedNodeIDs = append(publishedNodeIDs, strconv.Itoa(*volumes[volNum].LinodeID))
}

csiVol, publishedNodeIds, volumeCondition := getVolumeResponse(&volumes[volNum])
entries = append(entries, &csi.ListVolumesResponse_Entry{
Volume: &csi.Volume{
VolumeId: key.GetVolumeKey(),
CapacityBytes: gbToBytes(volumes[volNum].Size),
AccessibleTopology: []*csi.Topology{
{
Segments: map[string]string{
VolumeTopologyRegion: volumes[volNum].Region,
},
},
},
},
Volume: csiVol,
Status: &csi.ListVolumesResponse_VolumeStatus{
PublishedNodeIds: publishedNodeIDs,
VolumeCondition: &csi.VolumeCondition{
Abnormal: false,
},
PublishedNodeIds: publishedNodeIds,
VolumeCondition: volumeCondition,
},
})
}
Expand Down Expand Up @@ -493,3 +468,82 @@ func (cs *ControllerServer) ControllerExpandVolume(ctx context.Context, req *csi
}
return resp, nil
}

// ControllerGetVolume returns information about the specified volume.
// It checks if the volume exists and returns its ID, size, and status.
// For more details, refer to the CSI Driver Spec documentation.
func (cs *ControllerServer) ControllerGetVolume(ctx context.Context, req *csi.ControllerGetVolumeRequest) (*csi.ControllerGetVolumeResponse, error) {
log, _, done := logger.GetLogger(ctx).WithMethod("ControllerGetVolume")
defer done()

log.V(2).Info("Processing request", "req", req)

volumeID, statusErr := linodevolumes.VolumeIdAsInt("ControllerGetVolume", req)
if statusErr != nil {
return &csi.ControllerGetVolumeResponse{}, statusErr
}

// Get the volume
log.V(4).Info("Checking if volume exists", "volume_id", volumeID)
vol, err := cs.client.GetVolume(ctx, volumeID)
if linodego.IsNotFound(err) {
return &csi.ControllerGetVolumeResponse{}, errVolumeNotFound(volumeID)
} else if err != nil {
return &csi.ControllerGetVolumeResponse{}, errInternal("get volume: %v", err)
}

csiVol, publishedNodeIds, volumeCondition := getVolumeResponse(vol)
resp := &csi.ControllerGetVolumeResponse{
Volume: csiVol,
Status: &csi.ControllerGetVolumeResponse_VolumeStatus{
PublishedNodeIds: publishedNodeIds,
VolumeCondition: volumeCondition,
},
}

log.V(2).Info("Volume retrieved successfully", "volume_id", volumeID, "response", resp)
return resp, nil
}

func getVolumeResponse(volume *linodego.Volume) (csiVolume *csi.Volume, publishedNodeIds []string, volumeCondition *csi.VolumeCondition) {
key := linodevolumes.CreateLinodeVolumeKey(volume.ID, volume.Label)

// If the volume is attached to a Linode instance, add it to the
// list. Note that in the Linode API, volumes can only be
// attached to a single Linode at a time. We are storing it in
// a []string here, since that is what the response struct
// returns. We do not need to pre-allocate the slice with
// make(), since the CSI specification says this response field
// is optional, and thus it should tolerate a nil slice.
publishedNodeIds = []string{}
if volume.LinodeID != nil {
publishedNodeIds = append(publishedNodeIds, strconv.Itoa(*volume.LinodeID))
}

csiVolume = &csi.Volume{
VolumeId: key.GetVolumeKey(),
CapacityBytes: gbToBytes(volume.Size),
}

if volume.Region != "" {
csiVolume.AccessibleTopology = []*csi.Topology{
{
Segments: map[string]string{
VolumeTopologyRegion: volume.Region,
},
},
}
}

abnormal := false
if volume.Status != linodego.VolumeActive {
abnormal = true
}

volumeCondition = &csi.VolumeCondition{
Abnormal: abnormal,
Message: string(volume.Status),
}

return
}
216 changes: 209 additions & 7 deletions internal/driver/controllerserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func TestCreateVolume(t *testing.T) {
driver: ns.driver,
}
returnedResp, err := s.CreateVolume(context.Background(), tt.req)
if err != nil && !reflect.DeepEqual(tt.expectedError, err) {
if err != nil && !errors.Is(err, tt.expectedError) {
t.Errorf("CreateVolume error = %v, wantErr %v", err, tt.expectedError)
} else if returnedResp.GetVolume() != nil && tt.resp.GetVolume() != nil {
if returnedResp.GetVolume().GetCapacityBytes() != tt.resp.GetVolume().GetCapacityBytes() {
Expand Down Expand Up @@ -468,15 +468,17 @@ func TestListVolumes(t *testing.T) {
Region: "danmaaag",
Size: 30,
LinodeID: createLinodeID(10),
Status: linodego.VolumeActive,
},
},
},
"volume not attached": {
volumes: []linodego.Volume{
{
ID: 1,
Label: "bar",
Size: 30,
ID: 1,
Label: "bar",
Size: 30,
Status: linodego.VolumeActive,
},
},
},
Expand All @@ -487,12 +489,14 @@ func TestListVolumes(t *testing.T) {
Label: "foo",
Size: 30,
LinodeID: createLinodeID(5),
Status: linodego.VolumeActive,
},
{
ID: 2,
Label: "foo",
Size: 60,
LinodeID: createLinodeID(10),
Status: linodego.VolumeActive,
},
},
},
Expand All @@ -503,11 +507,13 @@ func TestListVolumes(t *testing.T) {
Label: "foo",
Size: 30,
LinodeID: createLinodeID(5),
Status: linodego.VolumeActive,
},
{
ID: 2,
Label: "foo",
Size: 30,
ID: 2,
Label: "foo",
Size: 30,
Status: linodego.VolumeActive,
},
},
},
Expand Down Expand Up @@ -844,3 +850,199 @@ func TestControllerMaxVolumeAttachments(t *testing.T) {
})
}
}

func Test_getVolumeResponse(t *testing.T) {
type args struct {
volume *linodego.Volume
}
tests := []struct {
name string
args args
wantCsiVolume *csi.Volume
wantPublishedNodeIds []string
wantVolumeCondition *csi.VolumeCondition
}{
{
name: "volume attached to node",
args: args{
volume: &linodego.Volume{
ID: 1,
Label: "foo",
Region: "danmaaag",
Size: 30,
LinodeID: createLinodeID(10),
Status: linodego.VolumeActive,
},
},
wantCsiVolume: &csi.Volume{
VolumeId: "1-foo",
CapacityBytes: 30 << 30,
AccessibleTopology: []*csi.Topology{
{
Segments: map[string]string{
VolumeTopologyRegion: "danmaaag",
},
},
},
},
wantPublishedNodeIds: []string{"10"},
wantVolumeCondition: &csi.VolumeCondition{
Abnormal: false,
Message: "active",
},
},
{
name: "volume not attached",
args: args{
volume: &linodego.Volume{
ID: 1,
Label: "bar",
Size: 30,
Status: linodego.VolumeActive,
},
},
wantCsiVolume: &csi.Volume{
VolumeId: "1-bar",
CapacityBytes: 30 << 30,
},
wantPublishedNodeIds: []string{},
wantVolumeCondition: &csi.VolumeCondition{
Abnormal: false,
Message: "active",
},
},
{
name: "volume attached with abnormal status",
args: args{
volume: &linodego.Volume{
ID: 1,
Label: "foo",
Region: "danmaaag",
Size: 30,
LinodeID: createLinodeID(10),
Status: linodego.VolumeContactSupport,
},
},
wantCsiVolume: &csi.Volume{
VolumeId: "1-foo",
CapacityBytes: 30 << 30,
AccessibleTopology: []*csi.Topology{
{
Segments: map[string]string{
VolumeTopologyRegion: "danmaaag",
},
},
},
},
wantPublishedNodeIds: []string{"10"},
wantVolumeCondition: &csi.VolumeCondition{
Abnormal: true,
Message: "contact_support",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotCsiVolume, gotPublishedNodeIds, gotVolumeCondition := getVolumeResponse(tt.args.volume)
if !reflect.DeepEqual(gotCsiVolume, tt.wantCsiVolume) {
t.Errorf("getVolumeResponse() gotCsiVolume = %v, want %v", gotCsiVolume, tt.wantCsiVolume)
}
if !reflect.DeepEqual(gotPublishedNodeIds, tt.wantPublishedNodeIds) {
t.Errorf("getVolumeResponse() gotPublishedNodeIds = %v, want %v", gotPublishedNodeIds, tt.wantPublishedNodeIds)
}
if !reflect.DeepEqual(gotVolumeCondition, tt.wantVolumeCondition) {
t.Errorf("getVolumeResponse() gotVolumeCondition = %v, want %v", gotVolumeCondition, tt.wantVolumeCondition)
}
})
}
}
func TestControllerGetVolume(t *testing.T) {
tests := []struct {
name string
req *csi.ControllerGetVolumeRequest
resp *csi.ControllerGetVolumeResponse
expectLinodeClientCalls func(m *mocks.MockLinodeClient)
expectedError error
}{
{
name: "volume exists",
req: &csi.ControllerGetVolumeRequest{
VolumeId: "1001",
},
resp: &csi.ControllerGetVolumeResponse{
Volume: &csi.Volume{
VolumeId: "1001-foo",
CapacityBytes: 30 << 30,
AccessibleTopology: []*csi.Topology{
{
Segments: map[string]string{
VolumeTopologyRegion: "us-east",
},
},
},
},
Status: &csi.ControllerGetVolumeResponse_VolumeStatus{
PublishedNodeIds: []string{"10"},
VolumeCondition: &csi.VolumeCondition{
Abnormal: false,
Message: "active",
},
},
},
expectLinodeClientCalls: func(m *mocks.MockLinodeClient) {
m.EXPECT().GetVolume(gomock.Any(), 597150807).Return(&linodego.Volume{
ID: 1001,
Label: "foo",
Region: "us-east",
Size: 30,
LinodeID: createLinodeID(10),
Status: linodego.VolumeActive,
}, nil)
},
expectedError: nil,
},
{
name: "volume not found",
req: &csi.ControllerGetVolumeRequest{
VolumeId: "1002",
},
resp: &csi.ControllerGetVolumeResponse{},
expectLinodeClientCalls: func(m *mocks.MockLinodeClient) {
m.EXPECT().GetVolume(gomock.Any(), 613928426).Return(nil, &linodego.Error{Code: 404})
},
expectedError: errVolumeNotFound(613928426),
},
{
name: "internal error",
req: &csi.ControllerGetVolumeRequest{
VolumeId: "1003",
},
resp: &csi.ControllerGetVolumeResponse{},
expectLinodeClientCalls: func(m *mocks.MockLinodeClient) {
m.EXPECT().GetVolume(gomock.Any(), 630706045).Return(nil, fmt.Errorf("internal error"))
},
expectedError: errInternal("get volume: %v", fmt.Errorf("internal error")),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockClient := mocks.NewMockLinodeClient(ctrl)
if tt.expectLinodeClientCalls != nil {
tt.expectLinodeClientCalls(mockClient)
}

cs := &ControllerServer{
client: mockClient,
}
resp, err := cs.ControllerGetVolume(context.Background(), tt.req)
if err != nil && !reflect.DeepEqual(tt.expectedError, err) {
t.Errorf("ControllerGetVolume error = %v, wantErr %v", err, tt.expectedError)
} else if !reflect.DeepEqual(resp, tt.resp) {
t.Errorf("ControllerGetVolume response = %v, want %v", resp, tt.resp)
}
})
}
}

0 comments on commit ff7c61d

Please sign in to comment.