Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@wackxu
wackxu committed Sep 3, 2018
1 parent 8d2504a commit 1f497d3
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 40 deletions.
93 changes: 67 additions & 26 deletions pkg/connection/connection_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,11 @@ func TestGetPluginInfo(t *testing.T) {

func TestSupportsControllerCreateSnapshot(t *testing.T) {
tests := []struct {
name string
output *csi.ControllerGetCapabilitiesResponse
injectError bool
expectError bool
name string
output *csi.ControllerGetCapabilitiesResponse
injectError bool
expectError bool
expectResult bool
}{
{
name: "success",
Expand All @@ -152,13 +153,15 @@ func TestSupportsControllerCreateSnapshot(t *testing.T) {
},
},
},
expectError: false,
expectError: false,
expectResult: true,
},
{
name: "gRPC error",
output: nil,
injectError: true,
expectError: true,
name: "gRPC error",
output: nil,
injectError: true,
expectError: true,
expectResult: false,
},
{
name: "no create snapshot",
Expand All @@ -173,7 +176,8 @@ func TestSupportsControllerCreateSnapshot(t *testing.T) {
},
},
},
expectError: false,
expectError: false,
expectResult: false,
},
{
name: "empty capability",
Expand All @@ -184,14 +188,16 @@ func TestSupportsControllerCreateSnapshot(t *testing.T) {
},
},
},
expectError: false,
expectError: false,
expectResult: false,
},
{
name: "no capabilities",
output: &csi.ControllerGetCapabilitiesResponse{
Capabilities: []*csi.ControllerServiceCapability{},
},
expectError: false,
expectError: false,
expectResult: false,
},
}

Expand All @@ -216,22 +222,26 @@ func TestSupportsControllerCreateSnapshot(t *testing.T) {
// Setup expectation
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 {
t.Errorf("test %q: Expected error, got none", test.name)
}
if !test.expectError && err != nil {
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) {
tests := []struct {
name string
output *csi.ControllerGetCapabilitiesResponse
injectError bool
expectError bool
name string
output *csi.ControllerGetCapabilitiesResponse
injectError bool
expectError bool
expectResult bool
}{
{
name: "success",
Expand Down Expand Up @@ -260,13 +270,38 @@ func TestSupportsControllerListSnapshots(t *testing.T) {
},
},
},
expectError: false,
expectError: false,
expectResult: true,
},
{
name: "gRPC error",
output: nil,
injectError: true,
expectError: true,
name: "have create_delete_snapshot but no list snapshot ",
output: &csi.ControllerGetCapabilitiesResponse{
Capabilities: []*csi.ControllerServiceCapability{
{
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",
Expand All @@ -281,7 +316,8 @@ func TestSupportsControllerListSnapshots(t *testing.T) {
},
},
},
expectError: false,
expectError: false,
expectResult: false,
},
{
name: "empty capability",
Expand All @@ -292,14 +328,16 @@ func TestSupportsControllerListSnapshots(t *testing.T) {
},
},
},
expectError: false,
expectError: false,
expectResult: false,
},
{
name: "no capabilities",
output: &csi.ControllerGetCapabilitiesResponse{
Capabilities: []*csi.ControllerServiceCapability{},
},
expectError: false,
expectError: false,
expectResult: false,
},
}

Expand All @@ -324,13 +362,16 @@ func TestSupportsControllerListSnapshots(t *testing.T) {
// Setup expectation
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 {
t.Errorf("test %q: Expected error, got none", test.name)
}
if !test.expectError && err != nil {
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)
}
}
}

Expand Down
24 changes: 12 additions & 12 deletions pkg/controller/framework_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,14 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
// check the content does not exist
_, found := r.contents[content.Name]
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.
r.contents[content.Name] = content
r.changedObjects = append(r.changedObjects, content)
r.changedSinceLastSync++
glog.V(4).Infof("created content %s", content.Name)
glog.V(5).Infof("created content %s", content.Name)
return true, content, nil

case action.Matches("update", "volumesnapshotcontents"):
Expand All @@ -221,7 +221,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
content = content.DeepCopy()
content.ResourceVersion = strconv.Itoa(storedVer + 1)
} 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.
Expand All @@ -247,7 +247,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
snapshot = snapshot.DeepCopy()
snapshot.ResourceVersion = strconv.Itoa(storedVer + 1)
} 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.
Expand All @@ -265,7 +265,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, content, nil
} else {
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"):
Expand All @@ -276,7 +276,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, snapshot, nil
} else {
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"):
Expand All @@ -288,7 +288,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
r.changedSinceLastSync++
return true, nil, nil
} 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"):
Expand All @@ -300,7 +300,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
r.changedSinceLastSync++
return true, nil, nil
} 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"):
Expand All @@ -311,7 +311,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, volume, nil
} else {
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"):
Expand All @@ -322,7 +322,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, claim, nil
} else {
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"):
Expand All @@ -333,7 +333,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, storageClass, nil
} else {
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"):
Expand All @@ -344,7 +344,7 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, secret, nil
} else {
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)
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/snapshot_create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func TestCreateSnapshotSync(t *testing.T) {
initialContents: nocontents,
expectedContents: nocontents,
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),
expectedEvents: []string{"Warning SnapshotCreationFailed"},
errors: noerrors,
Expand All @@ -282,7 +282,7 @@ func TestCreateSnapshotSync(t *testing.T) {
initialContents: nocontents,
expectedContents: nocontents,
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),
expectedEvents: []string{"Warning SnapshotCreationFailed"},
errors: noerrors,
Expand Down

0 comments on commit 1f497d3

Please sign in to comment.