alexlebens/external-snapshotter
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

address comments

Browse Source
This commit is contained in:
xushiwei 00425595
2018-09-03 20:29:25 +08:00
parent 8d2504ad9c
commit 1f497d324f
3 changed files with 81 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

93
pkg/connection/connection_test.go
View File

@@ -127,10 +127,11 @@ func TestGetPluginInfo(t *testing.T) {
func TestSupportsControllerCreateSnapshot(t *testing.T) { func TestSupportsControllerCreateSnapshot(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
output *csi.ControllerGetCapabilitiesResponse output *csi.ControllerGetCapabilitiesResponse
injectError bool injectError bool
expectError bool expectError bool
expectResult bool
}{ }{
{ {
name: "success", name: "success",
@@ -152,13 +153,15 @@ func TestSupportsControllerCreateSnapshot(t *testing.T) {
}, },
}, },
}, },
expectError: false, expectError: false,
expectResult: true,
}, },
{ {
name: "gRPC error", name: "gRPC error",
output: nil, output: nil,
injectError: true, injectError: true,
expectError: true, expectError: true,
expectResult: false,
}, },
{ {
name: "no create snapshot", name: "no create snapshot",
@@ -173,7 +176,8 @@ func TestSupportsControllerCreateSnapshot(t *testing.T) {
}, },
}, },
}, },
expectError: false, expectError: false,
expectResult: false,
}, },
{ {
name: "empty capability", name: "empty capability",
@@ -184,14 +188,16 @@ func TestSupportsControllerCreateSnapshot(t *testing.T) {
}, },
}, },
}, },
expectError: false, expectError: false,
expectResult: false,
}, },
{ {
name: "no capabilities", name: "no capabilities",
output: &csi.ControllerGetCapabilitiesResponse{ output: &csi.ControllerGetCapabilitiesResponse{
Capabilities: []*csi.ControllerServiceCapability{}, Capabilities: []*csi.ControllerServiceCapability{},
}, },
expectError: false, expectError: false,
expectResult: false,
}, },
} }
@@ -216,22 +222,26 @@ func TestSupportsControllerCreateSnapshot(t *testing.T) {
// Setup expectation // Setup expectation
controllerServer.EXPECT().ControllerGetCapabilities(gomock.Any(), in).Return(out, injectedErr).Times(1) controllerServer.EXPECT().ControllerGetCapabilities(gomock.Any(), in).Return(out, injectedErr).Times(1)
_, err = csiConn.SupportsControllerCreateSnapshot(context.Background()) ok, err := csiConn.SupportsControllerCreateSnapshot(context.Background())
if test.expectError && err == nil { if test.expectError && err == nil {
t.Errorf("test %q: Expected error, got none", test.name) t.Errorf("test %q: Expected error, got none", test.name)
} }
if !test.expectError && err != nil { if !test.expectError && err != nil {
t.Errorf("test %q: got error: %v", test.name, err) t.Errorf("test %q: got error: %v", test.name, err)
} }
if err == nil && test.expectResult != ok {
t.Errorf("test fail expected result %t but got %t\n", test.expectResult, ok)
}
} }
} }
func TestSupportsControllerListSnapshots(t *testing.T) { func TestSupportsControllerListSnapshots(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
output *csi.ControllerGetCapabilitiesResponse output *csi.ControllerGetCapabilitiesResponse
injectError bool injectError bool
expectError bool expectError bool
expectResult bool
}{ }{
{ {
name: "success", name: "success",
@@ -260,13 +270,38 @@ func TestSupportsControllerListSnapshots(t *testing.T) {
}, },
}, },
}, },
expectError: false, expectError: false,
expectResult: true,
}, },
{ {
name: "gRPC error", name: "have create_delete_snapshot but no list snapshot ",
output: nil, output: &csi.ControllerGetCapabilitiesResponse{
injectError: true, Capabilities: []*csi.ControllerServiceCapability{
expectError: true, {
Type: &csi.ControllerServiceCapability_Rpc{
Rpc: &csi.ControllerServiceCapability_RPC{
Type: csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME,
},
},
},
{
Type: &csi.ControllerServiceCapability_Rpc{
Rpc: &csi.ControllerServiceCapability_RPC{
Type: csi.ControllerServiceCapability_RPC_CREATE_DELETE_SNAPSHOT,
},
},
},
},
},
expectError: false,
expectResult: false,
},
{
name: "gRPC error",
output: nil,
injectError: true,
expectError: true,
expectResult: false,
}, },
{ {
name: "no list snapshot", name: "no list snapshot",
@@ -281,7 +316,8 @@ func TestSupportsControllerListSnapshots(t *testing.T) {
}, },
}, },
}, },
expectError: false, expectError: false,
expectResult: false,
}, },
{ {
name: "empty capability", name: "empty capability",
@@ -292,14 +328,16 @@ func TestSupportsControllerListSnapshots(t *testing.T) {
}, },
}, },
}, },
expectError: false, expectError: false,
expectResult: false,
}, },
{ {
name: "no capabilities", name: "no capabilities",
output: &csi.ControllerGetCapabilitiesResponse{ output: &csi.ControllerGetCapabilitiesResponse{
Capabilities: []*csi.ControllerServiceCapability{}, Capabilities: []*csi.ControllerServiceCapability{},
}, },
expectError: false, expectError: false,
expectResult: false,
}, },
} }
@@ -324,13 +362,16 @@ func TestSupportsControllerListSnapshots(t *testing.T) {
// Setup expectation // Setup expectation
controllerServer.EXPECT().ControllerGetCapabilities(gomock.Any(), in).Return(out, injectedErr).Times(1) controllerServer.EXPECT().ControllerGetCapabilities(gomock.Any(), in).Return(out, injectedErr).Times(1)
_, err = csiConn.SupportsControllerListSnapshots(context.Background()) ok, err := csiConn.SupportsControllerListSnapshots(context.Background())
if test.expectError && err == nil { if test.expectError && err == nil {
t.Errorf("test %q: Expected error, got none", test.name) t.Errorf("test %q: Expected error, got none", test.name)
} }
if !test.expectError && err != nil { if !test.expectError && err != nil {
t.Errorf("test %q: got error: %v", test.name, err) t.Errorf("test %q: got error: %v", test.name, err)
} }
if err == nil && test.expectResult != ok {
t.Errorf("test fail expected result %t but got %t\n", test.expectResult, ok)
}
} }
} }

24
pkg/controller/framework_test.go
View File

@@ -195,14 +195,14 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
// check the content does not exist // check the content does not exist
_, found := r.contents[content.Name] _, found := r.contents[content.Name]
if found { if found {
return true, nil, fmt.Errorf("Cannot create content %s: content already exists", content.Name) return true, nil, fmt.Errorf("cannot create content %s: content already exists", content.Name)
} }
// Store the updated object to appropriate places. // Store the updated object to appropriate places.
r.contents[content.Name] = content r.contents[content.Name] = content
r.changedObjects = append(r.changedObjects, content) r.changedObjects = append(r.changedObjects, content)
r.changedSinceLastSync++ r.changedSinceLastSync++
glog.V(4).Infof("created content %s", content.Name) glog.V(5).Infof("created content %s", content.Name)
return true, content, nil return true, content, nil
case action.Matches("update", "volumesnapshotcontents"): case action.Matches("update", "volumesnapshotcontents"):
@@ -221,7 +221,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
content = content.DeepCopy() content = content.DeepCopy()
content.ResourceVersion = strconv.Itoa(storedVer + 1) content.ResourceVersion = strconv.Itoa(storedVer + 1)
} else { } else {
return true, nil, fmt.Errorf("Cannot update content %s: content not found", content.Name) return true, nil, fmt.Errorf("cannot update content %s: content not found", content.Name)
} }
// Store the updated object to appropriate places. // Store the updated object to appropriate places.
@@ -247,7 +247,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
snapshot = snapshot.DeepCopy() snapshot = snapshot.DeepCopy()
snapshot.ResourceVersion = strconv.Itoa(storedVer + 1) snapshot.ResourceVersion = strconv.Itoa(storedVer + 1)
} else { } else {
return true, nil, fmt.Errorf("Cannot update snapshot %s: snapshot not found", snapshot.Name) return true, nil, fmt.Errorf("cannot update snapshot %s: snapshot not found", snapshot.Name)
} }
// Store the updated object to appropriate places. // Store the updated object to appropriate places.
@@ -265,7 +265,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, content, nil return true, content, nil
} else { } else {
glog.V(4).Infof("GetVolume: content %s not found", name) glog.V(4).Infof("GetVolume: content %s not found", name)
return true, nil, fmt.Errorf("Cannot find content %s", name) return true, nil, fmt.Errorf("cannot find content %s", name)
} }
case action.Matches("get", "volumesnapshots"): case action.Matches("get", "volumesnapshots"):
@@ -276,7 +276,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, snapshot, nil return true, snapshot, nil
} else { } else {
glog.V(4).Infof("GetSnapshot: content %s not found", name) glog.V(4).Infof("GetSnapshot: content %s not found", name)
return true, nil, fmt.Errorf("Cannot find snapshot %s", name) return true, nil, fmt.Errorf("cannot find snapshot %s", name)
} }
case action.Matches("delete", "volumesnapshotcontents"): case action.Matches("delete", "volumesnapshotcontents"):
@@ -288,7 +288,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
r.changedSinceLastSync++ r.changedSinceLastSync++
return true, nil, nil return true, nil, nil
} else { } else {
return true, nil, fmt.Errorf("Cannot delete content %s: not found", name) return true, nil, fmt.Errorf("cannot delete content %s: not found", name)
} }
case action.Matches("delete", "volumesnapshots"): case action.Matches("delete", "volumesnapshots"):
@@ -300,7 +300,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
r.changedSinceLastSync++ r.changedSinceLastSync++
return true, nil, nil return true, nil, nil
} else { } else {
return true, nil, fmt.Errorf("Cannot delete snapshot %s: not found", name) return true, nil, fmt.Errorf("cannot delete snapshot %s: not found", name)
} }
case action.Matches("get", "persistentvolumes"): case action.Matches("get", "persistentvolumes"):
@@ -311,7 +311,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, volume, nil return true, volume, nil
} else { } else {
glog.V(4).Infof("GetVolume: volume %s not found", name) glog.V(4).Infof("GetVolume: volume %s not found", name)
return true, nil, fmt.Errorf("Cannot find volume %s", name) return true, nil, fmt.Errorf("cannot find volume %s", name)
} }
case action.Matches("get", "persistentvolumeclaims"): case action.Matches("get", "persistentvolumeclaims"):
@@ -322,7 +322,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, claim, nil return true, claim, nil
} else { } else {
glog.V(4).Infof("GetClaim: claim %s not found", name) glog.V(4).Infof("GetClaim: claim %s not found", name)
return true, nil, fmt.Errorf("Cannot find claim %s", name) return true, nil, fmt.Errorf("cannot find claim %s", name)
} }
case action.Matches("get", "storageclasses"): case action.Matches("get", "storageclasses"):
@@ -333,7 +333,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, storageClass, nil return true, storageClass, nil
} else { } else {
glog.V(4).Infof("GetStorageClass: storageClass %s not found", name) glog.V(4).Infof("GetStorageClass: storageClass %s not found", name)
return true, nil, fmt.Errorf("Cannot find storageClass %s", name) return true, nil, fmt.Errorf("cannot find storageClass %s", name)
} }
case action.Matches("get", "secrets"): case action.Matches("get", "secrets"):
@@ -344,7 +344,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, secret, nil return true, secret, nil
} else { } else {
glog.V(4).Infof("GetSecret: secret %s not found", name) glog.V(4).Infof("GetSecret: secret %s not found", name)
return true, nil, fmt.Errorf("Cannot find secret %s", name) return true, nil, fmt.Errorf("cannot find secret %s", name)
} }
} }

4
pkg/controller/snapshot_create_test.go
View File

@@ -271,7 +271,7 @@ func TestCreateSnapshotSync(t *testing.T) {
initialContents: nocontents, initialContents: nocontents,
expectedContents: nocontents, expectedContents: nocontents,
initialSnapshots: newSnapshotArray("snap7-4", classGold, "", "snapuid7-4", "claim7-4", false, nil, nil, nil), initialSnapshots: newSnapshotArray("snap7-4", classGold, "", "snapuid7-4", "claim7-4", false, nil, nil, nil),
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), 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),
initialVolumes: newVolumeArray("volume7-4", "pv-uid7-4", "pv-handle7-4", "1Gi", "pvc-uid7-4", "claim7-4", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialVolumes: newVolumeArray("volume7-4", "pv-uid7-4", "pv-handle7-4", "1Gi", "pvc-uid7-4", "claim7-4", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
expectedEvents: []string{"Warning SnapshotCreationFailed"}, expectedEvents: []string{"Warning SnapshotCreationFailed"},
errors: noerrors, errors: noerrors,
@@ -282,7 +282,7 @@ func TestCreateSnapshotSync(t *testing.T) {
initialContents: nocontents, initialContents: nocontents,
expectedContents: nocontents, expectedContents: nocontents,
initialSnapshots: newSnapshotArray("snap7-5", classGold, "", "snapuid7-5", "claim7-5", false, nil, nil, nil), initialSnapshots: newSnapshotArray("snap7-5", classGold, "", "snapuid7-5", "claim7-5", false, nil, nil, nil),
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), 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),
initialClaims: newClaimArray("claim7-5", "pvc-uid7-5", "1Gi", "volume7-5", v1.ClaimBound, &classEmpty), initialClaims: newClaimArray("claim7-5", "pvc-uid7-5", "1Gi", "volume7-5", v1.ClaimBound, &classEmpty),
expectedEvents: []string{"Warning SnapshotCreationFailed"}, expectedEvents: []string{"Warning SnapshotCreationFailed"},
errors: noerrors, errors: noerrors,