Skip to content

Commit 1f497d3

Browse files
wackxuwackxu
committed
address comments
1 parent 8d2504a commit 1f497d3

File tree

3 files changed

+81
-40
lines changed

3 files changed

+81
-40
lines changed

‎pkg/connection/connection_test.go‎

Lines changed: 67 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,11 @@ func TestGetPluginInfo(t *testing.T) {
127127

128128
func TestSupportsControllerCreateSnapshot(t *testing.T) {
129129
tests := []struct {
130-
name string
131-
output *csi.ControllerGetCapabilitiesResponse
132-
injectError bool
133-
expectError bool
130+
name string
131+
output *csi.ControllerGetCapabilitiesResponse
132+
injectError bool
133+
expectError bool
134+
expectResult bool
134135
}{
135136
{
136137
name: "success",
@@ -152,13 +153,15 @@ func TestSupportsControllerCreateSnapshot(t *testing.T) {
152153
},
153154
},
154155
},
155-
expectError: false,
156+
expectError: false,
157+
expectResult: true,
156158
},
157159
{
158-
name: "gRPC error",
159-
output: nil,
160-
injectError: true,
161-
expectError: true,
160+
name: "gRPC error",
161+
output: nil,
162+
injectError: true,
163+
expectError: true,
164+
expectResult: false,
162165
},
163166
{
164167
name: "no create snapshot",
@@ -173,7 +176,8 @@ func TestSupportsControllerCreateSnapshot(t *testing.T) {
173176
},
174177
},
175178
},
176-
expectError: false,
179+
expectError: false,
180+
expectResult: false,
177181
},
178182
{
179183
name: "empty capability",
@@ -184,14 +188,16 @@ func TestSupportsControllerCreateSnapshot(t *testing.T) {
184188
},
185189
},
186190
},
187-
expectError: false,
191+
expectError: false,
192+
expectResult: false,
188193
},
189194
{
190195
name: "no capabilities",
191196
output: &csi.ControllerGetCapabilitiesResponse{
192197
Capabilities: []*csi.ControllerServiceCapability{},
193198
},
194-
expectError: false,
199+
expectError: false,
200+
expectResult: false,
195201
},
196202
}
197203

@@ -216,22 +222,26 @@ func TestSupportsControllerCreateSnapshot(t *testing.T) {
216222
// Setup expectation
217223
controllerServer.EXPECT().ControllerGetCapabilities(gomock.Any(), in).Return(out, injectedErr).Times(1)
218224

219-
_, err = csiConn.SupportsControllerCreateSnapshot(context.Background())
225+
ok, err := csiConn.SupportsControllerCreateSnapshot(context.Background())
220226
if test.expectError && err == nil {
221227
t.Errorf("test %q: Expected error, got none", test.name)
222228
}
223229
if !test.expectError && err != nil {
224230
t.Errorf("test %q: got error: %v", test.name, err)
225231
}
232+
if err == nil && test.expectResult != ok {
233+
t.Errorf("test fail expected result %t but got %t\n", test.expectResult, ok)
234+
}
226235
}
227236
}
228237

229238
func TestSupportsControllerListSnapshots(t *testing.T) {
230239
tests := []struct {
231-
name string
232-
output *csi.ControllerGetCapabilitiesResponse
233-
injectError bool
234-
expectError bool
240+
name string
241+
output *csi.ControllerGetCapabilitiesResponse
242+
injectError bool
243+
expectError bool
244+
expectResult bool
235245
}{
236246
{
237247
name: "success",
@@ -260,13 +270,38 @@ func TestSupportsControllerListSnapshots(t *testing.T) {
260270
},
261271
},
262272
},
263-
expectError: false,
273+
expectError: false,
274+
expectResult: true,
264275
},
265276
{
266-
name: "gRPC error",
267-
output: nil,
268-
injectError: true,
269-
expectError: true,
277+
name: "have create_delete_snapshot but no list snapshot ",
278+
output: &csi.ControllerGetCapabilitiesResponse{
279+
Capabilities: []*csi.ControllerServiceCapability{
280+
{
281+
Type: &csi.ControllerServiceCapability_Rpc{
282+
Rpc: &csi.ControllerServiceCapability_RPC{
283+
Type: csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME,
284+
},
285+
},
286+
},
287+
{
288+
Type: &csi.ControllerServiceCapability_Rpc{
289+
Rpc: &csi.ControllerServiceCapability_RPC{
290+
Type: csi.ControllerServiceCapability_RPC_CREATE_DELETE_SNAPSHOT,
291+
},
292+
},
293+
},
294+
},
295+
},
296+
expectError: false,
297+
expectResult: false,
298+
},
299+
{
300+
name: "gRPC error",
301+
output: nil,
302+
injectError: true,
303+
expectError: true,
304+
expectResult: false,
270305
},
271306
{
272307
name: "no list snapshot",
@@ -281,7 +316,8 @@ func TestSupportsControllerListSnapshots(t *testing.T) {
281316
},
282317
},
283318
},
284-
expectError: false,
319+
expectError: false,
320+
expectResult: false,
285321
},
286322
{
287323
name: "empty capability",
@@ -292,14 +328,16 @@ func TestSupportsControllerListSnapshots(t *testing.T) {
292328
},
293329
},
294330
},
295-
expectError: false,
331+
expectError: false,
332+
expectResult: false,
296333
},
297334
{
298335
name: "no capabilities",
299336
output: &csi.ControllerGetCapabilitiesResponse{
300337
Capabilities: []*csi.ControllerServiceCapability{},
301338
},
302-
expectError: false,
339+
expectError: false,
340+
expectResult: false,
303341
},
304342
}
305343

@@ -324,13 +362,16 @@ func TestSupportsControllerListSnapshots(t *testing.T) {
324362
// Setup expectation
325363
controllerServer.EXPECT().ControllerGetCapabilities(gomock.Any(), in).Return(out, injectedErr).Times(1)
326364

327-
_, err = csiConn.SupportsControllerListSnapshots(context.Background())
365+
ok, err := csiConn.SupportsControllerListSnapshots(context.Background())
328366
if test.expectError && err == nil {
329367
t.Errorf("test %q: Expected error, got none", test.name)
330368
}
331369
if !test.expectError && err != nil {
332370
t.Errorf("test %q: got error: %v", test.name, err)
333371
}
372+
if err == nil && test.expectResult != ok {
373+
t.Errorf("test fail expected result %t but got %t\n", test.expectResult, ok)
374+
}
334375
}
335376
}
336377

‎pkg/controller/framework_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -195,14 +195,14 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
195195
// check the content does not exist
196196
_, found := r.contents[content.Name]
197197
if found {
198-
return true, nil, fmt.Errorf("Cannot create content %s: content already exists", content.Name)
198+
return true, nil, fmt.Errorf("cannot create content %s: content already exists", content.Name)
199199
}
200200

201201
// Store the updated object to appropriate places.
202202
r.contents[content.Name] = content
203203
r.changedObjects = append(r.changedObjects, content)
204204
r.changedSinceLastSync++
205-
glog.V(4).Infof("created content %s", content.Name)
205+
glog.V(5).Infof("created content %s", content.Name)
206206
return true, content, nil
207207

208208
case action.Matches("update", "volumesnapshotcontents"):
@@ -221,7 +221,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
221221
content = content.DeepCopy()
222222
content.ResourceVersion = strconv.Itoa(storedVer + 1)
223223
} else {
224-
return true, nil, fmt.Errorf("Cannot update content %s: content not found", content.Name)
224+
return true, nil, fmt.Errorf("cannot update content %s: content not found", content.Name)
225225
}
226226

227227
// Store the updated object to appropriate places.
@@ -247,7 +247,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
247247
snapshot = snapshot.DeepCopy()
248248
snapshot.ResourceVersion = strconv.Itoa(storedVer + 1)
249249
} else {
250-
return true, nil, fmt.Errorf("Cannot update snapshot %s: snapshot not found", snapshot.Name)
250+
return true, nil, fmt.Errorf("cannot update snapshot %s: snapshot not found", snapshot.Name)
251251
}
252252

253253
// Store the updated object to appropriate places.
@@ -265,7 +265,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
265265
return true, content, nil
266266
} else {
267267
glog.V(4).Infof("GetVolume: content %s not found", name)
268-
return true, nil, fmt.Errorf("Cannot find content %s", name)
268+
return true, nil, fmt.Errorf("cannot find content %s", name)
269269
}
270270

271271
case action.Matches("get", "volumesnapshots"):
@@ -276,7 +276,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
276276
return true, snapshot, nil
277277
} else {
278278
glog.V(4).Infof("GetSnapshot: content %s not found", name)
279-
return true, nil, fmt.Errorf("Cannot find snapshot %s", name)
279+
return true, nil, fmt.Errorf("cannot find snapshot %s", name)
280280
}
281281

282282
case action.Matches("delete", "volumesnapshotcontents"):
@@ -288,7 +288,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
288288
r.changedSinceLastSync++
289289
return true, nil, nil
290290
} else {
291-
return true, nil, fmt.Errorf("Cannot delete content %s: not found", name)
291+
return true, nil, fmt.Errorf("cannot delete content %s: not found", name)
292292
}
293293

294294
case action.Matches("delete", "volumesnapshots"):
@@ -300,7 +300,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
300300
r.changedSinceLastSync++
301301
return true, nil, nil
302302
} else {
303-
return true, nil, fmt.Errorf("Cannot delete snapshot %s: not found", name)
303+
return true, nil, fmt.Errorf("cannot delete snapshot %s: not found", name)
304304
}
305305

306306
case action.Matches("get", "persistentvolumes"):
@@ -311,7 +311,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
311311
return true, volume, nil
312312
} else {
313313
glog.V(4).Infof("GetVolume: volume %s not found", name)
314-
return true, nil, fmt.Errorf("Cannot find volume %s", name)
314+
return true, nil, fmt.Errorf("cannot find volume %s", name)
315315
}
316316

317317
case action.Matches("get", "persistentvolumeclaims"):
@@ -322,7 +322,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
322322
return true, claim, nil
323323
} else {
324324
glog.V(4).Infof("GetClaim: claim %s not found", name)
325-
return true, nil, fmt.Errorf("Cannot find claim %s", name)
325+
return true, nil, fmt.Errorf("cannot find claim %s", name)
326326
}
327327

328328
case action.Matches("get", "storageclasses"):
@@ -333,7 +333,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
333333
return true, storageClass, nil
334334
} else {
335335
glog.V(4).Infof("GetStorageClass: storageClass %s not found", name)
336-
return true, nil, fmt.Errorf("Cannot find storageClass %s", name)
336+
return true, nil, fmt.Errorf("cannot find storageClass %s", name)
337337
}
338338

339339
case action.Matches("get", "secrets"):
@@ -344,7 +344,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
344344
return true, secret, nil
345345
} else {
346346
glog.V(4).Infof("GetSecret: secret %s not found", name)
347-
return true, nil, fmt.Errorf("Cannot find secret %s", name)
347+
return true, nil, fmt.Errorf("cannot find secret %s", name)
348348
}
349349
}
350350

‎pkg/controller/snapshot_create_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ func TestCreateSnapshotSync(t *testing.T) {
271271
initialContents: nocontents,
272272
expectedContents: nocontents,
273273
initialSnapshots: newSnapshotArray("snap7-4", classGold, "", "snapuid7-4", "claim7-4", false, nil, nil, nil),
274-
expectedSnapshots: newSnapshotArray("snap7-4", classGold, "", "snapuid7-4", "claim7-4", false, newVolumeError("Failed to create snapshot: failed to retrieve PVC claim7-4 from the API server: \"Cannot find claim claim7-4\""), nil, nil),
274+
expectedSnapshots: newSnapshotArray("snap7-4", classGold, "", "snapuid7-4", "claim7-4", false, newVolumeError("Failed to create snapshot: failed to retrieve PVC claim7-4 from the API server: \"cannot find claim claim7-4\""), nil, nil),
275275
initialVolumes: newVolumeArray("volume7-4", "pv-uid7-4", "pv-handle7-4", "1Gi", "pvc-uid7-4", "claim7-4", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
276276
expectedEvents: []string{"Warning SnapshotCreationFailed"},
277277
errors: noerrors,
@@ -282,7 +282,7 @@ func TestCreateSnapshotSync(t *testing.T) {
282282
initialContents: nocontents,
283283
expectedContents: nocontents,
284284
initialSnapshots: newSnapshotArray("snap7-5", classGold, "", "snapuid7-5", "claim7-5", false, nil, nil, nil),
285-
expectedSnapshots: newSnapshotArray("snap7-5", classGold, "", "snapuid7-5", "claim7-5", false, newVolumeError("Failed to create snapshot: failed to retrieve PV volume7-5 from the API server: \"Cannot find volume volume7-5\""), nil, nil),
285+
expectedSnapshots: newSnapshotArray("snap7-5", classGold, "", "snapuid7-5", "claim7-5", false, newVolumeError("Failed to create snapshot: failed to retrieve PV volume7-5 from the API server: \"cannot find volume volume7-5\""), nil, nil),
286286
initialClaims: newClaimArray("claim7-5", "pvc-uid7-5", "1Gi", "volume7-5", v1.ClaimBound, &classEmpty),
287287
expectedEvents: []string{"Warning SnapshotCreationFailed"},
288288
errors: noerrors,

0 commit comments

Comments
 (0)