Skip to content

Commit 95fa4bb

Browse files
authored
Merge pull request #156 from hakanmemisoglu/release-1.2-list-snapshot-fix
Cherry-pick: Check if ListSnapshots is supported during GetSnapshotStatus
2 parents 7e9deea + 999884e commit 95fa4bb

File tree

2 files changed

+86
-31
lines changed

2 files changed

+86
-31
lines changed

pkg/snapshotter/snapshotter.go

+24-1
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,36 @@ func (s *snapshot) DeleteSnapshot(ctx context.Context, snapshotID string, snapsh
101101
return nil
102102
}
103103

104+
func (s *snapshot) isListSnapshotsSupported(ctx context.Context) (bool, error) {
105+
client := csi.NewControllerClient(s.conn)
106+
capRsp, err := client.ControllerGetCapabilities(ctx, &csi.ControllerGetCapabilitiesRequest{})
107+
if err != nil {
108+
return false, err
109+
}
110+
111+
for _, cap := range capRsp.Capabilities {
112+
if cap.GetRpc().GetType() == csi.ControllerServiceCapability_RPC_LIST_SNAPSHOTS {
113+
return true, nil
114+
}
115+
}
116+
117+
return false, nil
118+
}
119+
104120
func (s *snapshot) GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, time.Time, int64, error) {
105121
client := csi.NewControllerClient(s.conn)
106122

123+
// If the driver does not support ListSnapshots, assume the snapshot ID is valid.
124+
listSnapshotsSupported, err := s.isListSnapshotsSupported(ctx)
125+
if err != nil {
126+
return false, time.Time{}, 0, fmt.Errorf("failed to check if ListSnapshots is supported: %s", err.Error())
127+
}
128+
if !listSnapshotsSupported {
129+
return true, time.Time{}, 0, nil
130+
}
107131
req := csi.ListSnapshotsRequest{
108132
SnapshotId: snapshotID,
109133
}
110-
111134
rsp, err := client.ListSnapshots(ctx, &req)
112135
if err != nil {
113136
return false, time.Time{}, 0, err

pkg/snapshotter/snapshotter_test.go

+62-30
Original file line numberDiff line numberDiff line change
@@ -370,41 +370,56 @@ func TestGetSnapshotStatus(t *testing.T) {
370370
}
371371

372372
tests := []struct {
373-
name string
374-
snapshotID string
375-
input *csi.ListSnapshotsRequest
376-
output *csi.ListSnapshotsResponse
377-
injectError codes.Code
378-
expectError bool
379-
expectReady bool
380-
expectCreateAt time.Time
381-
expectSize int64
373+
name string
374+
snapshotID string
375+
listSnapshotsSupported bool
376+
input *csi.ListSnapshotsRequest
377+
output *csi.ListSnapshotsResponse
378+
injectError codes.Code
379+
expectError bool
380+
expectReady bool
381+
expectCreateAt time.Time
382+
expectSize int64
382383
}{
383384
{
384-
name: "success",
385-
snapshotID: defaultID,
386-
input: defaultRequest,
387-
output: defaultResponse,
388-
expectError: false,
389-
expectReady: true,
390-
expectCreateAt: createTime,
391-
expectSize: size,
385+
name: "success",
386+
snapshotID: defaultID,
387+
listSnapshotsSupported: true,
388+
input: defaultRequest,
389+
output: defaultResponse,
390+
expectError: false,
391+
expectReady: true,
392+
expectCreateAt: createTime,
393+
expectSize: size,
392394
},
393395
{
394-
name: "gRPC transient error",
395-
snapshotID: defaultID,
396-
input: defaultRequest,
397-
output: nil,
398-
injectError: codes.DeadlineExceeded,
399-
expectError: true,
396+
name: "ListSnapshots not supported",
397+
snapshotID: defaultID,
398+
listSnapshotsSupported: false,
399+
input: defaultRequest,
400+
output: defaultResponse,
401+
expectError: false,
402+
expectReady: true,
403+
expectCreateAt: time.Time{},
404+
expectSize: 0,
400405
},
401406
{
402-
name: "gRPC final error",
403-
snapshotID: defaultID,
404-
input: defaultRequest,
405-
output: nil,
406-
injectError: codes.NotFound,
407-
expectError: true,
407+
name: "gRPC transient error",
408+
snapshotID: defaultID,
409+
listSnapshotsSupported: true,
410+
input: defaultRequest,
411+
output: nil,
412+
injectError: codes.DeadlineExceeded,
413+
expectError: true,
414+
},
415+
{
416+
name: "gRPC final error",
417+
snapshotID: defaultID,
418+
listSnapshotsSupported: true,
419+
input: defaultRequest,
420+
output: nil,
421+
injectError: codes.NotFound,
422+
expectError: true,
408423
},
409424
}
410425

@@ -425,8 +440,25 @@ func TestGetSnapshotStatus(t *testing.T) {
425440
}
426441

427442
// Setup expectation
443+
listSnapshotsCap := &csi.ControllerServiceCapability{
444+
Type: &csi.ControllerServiceCapability_Rpc{
445+
Rpc: &csi.ControllerServiceCapability_RPC{
446+
Type: csi.ControllerServiceCapability_RPC_LIST_SNAPSHOTS,
447+
},
448+
},
449+
}
450+
451+
var controllerCapabilities []*csi.ControllerServiceCapability
452+
if test.listSnapshotsSupported {
453+
controllerCapabilities = append(controllerCapabilities, listSnapshotsCap)
454+
}
428455
if in != nil {
429-
controllerServer.EXPECT().ListSnapshots(gomock.Any(), in).Return(out, injectedErr).Times(1)
456+
controllerServer.EXPECT().ControllerGetCapabilities(gomock.Any(), gomock.Any()).Return(&csi.ControllerGetCapabilitiesResponse{
457+
Capabilities: controllerCapabilities,
458+
}, nil).Times(1)
459+
if test.listSnapshotsSupported {
460+
controllerServer.EXPECT().ListSnapshots(gomock.Any(), in).Return(out, injectedErr).Times(1)
461+
}
430462
}
431463

432464
s := NewSnapshotter(csiConn)

0 commit comments

Comments
 (0)