Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #95: Call GetSnapshotStatus to check snapshot status #96

Closed
wants to merge 1 commit into from
Closed

Fix #95: Call GetSnapshotStatus to check snapshot status #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions pkg/controller/snapshot_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ import (
// this snapshot. After that, the controller will keep checking the snapshot status
// though csi snapshot calls. When the snapshot is ready to use, the controller set
// the status "Bound" to true to indicate the snapshot is bound and ready to use.
// If the createtion failed for any reason, the Error status is set accordingly.
// If the creation failed for any reason, the Error status is set accordingly.
// In alpha version, the controller not retry to create the snapshot after it failed.
// In the future version, a retry policy will be added.

Expand Down Expand Up @@ -560,16 +560,14 @@ func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(sn
var timestamp int64
var size int64
var readyToUse = false
class, volume, _, snapshotterCredentials, err := ctrl.getCreateSnapshotInput(snapshot)
if err != nil {
return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err)
}
driverName, snapshotID, timestamp, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)

readyToUse, timestamp, size, err = ctrl.handler.GetSnapshotStatus(content)
if err != nil {
glog.Errorf("checkandUpdateBoundSnapshotStatusOperation: failed to call create snapshot to check whether the snapshot is ready to use %q", err)
glog.Errorf("checkandUpdateBoundSnapshotStatusOperation: failed to call list snapshot to check whether the"+
" snapshot is ready to use %q", err)
return nil, err
}
glog.V(5).Infof("checkandUpdateBoundSnapshotStatusOperation: driver %s, snapshotId %s, timestamp %d, size %d, readyToUse %t", driverName, snapshotID, timestamp, size, readyToUse)
glog.V(5).Infof("checkandUpdateBoundSnapshotStatusOperation: timestamp %d, size %d, readyToUse %t", timestamp, size, readyToUse)

if timestamp == 0 {
timestamp = time.Now().UnixNano()
Expand Down
68 changes: 28 additions & 40 deletions pkg/controller/snapshot_ready_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controller

import (
"errors"
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -72,17 +73,12 @@ func TestSync(t *testing.T) {
initialClaims: newClaimArray("claim2-3", "pvc-uid2-3", "1Gi", "volume2-3", v1.ClaimBound, &classEmpty),
initialVolumes: newVolumeArray("volume2-3", "pv-uid2-3", "pv-handle2-3", "1Gi", "pvc-uid2-3", "claim2-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
initialSecrets: []*v1.Secret{secret()},
expectedCreateCalls: []createCall{
expectedListCalls: []listCall{
{
snapshotName: "snapshot-snapuid2-3",
volume: newVolume("volume2-3", "pv-uid2-3", "pv-handle2-3", "1Gi", "pvc-uid2-3", "claim2-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
parameters: class5Parameters,
secrets: map[string]string{"foo": "bar"},
snapshotID: "sid2-3",
// information to return
driverName: mockDriverName,
snapshotId: "sid2-3",
timestamp: timeNow,
readyToUse: false,
createTime: timeNow,
},
},
errors: noerrors,
Expand All @@ -107,42 +103,38 @@ func TestSync(t *testing.T) {
initialClaims: newClaimArray("claim2-5", "pvc-uid2-5", "1Gi", "volume2-5", v1.ClaimBound, &classEmpty),
initialVolumes: newVolumeArray("volume2-5", "pv-uid2-5", "pv-handle2-5", "1Gi", "pvc-uid2-5", "claim2-5", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
initialSecrets: []*v1.Secret{secret()},
expectedCreateCalls: []createCall{
expectedListCalls: []listCall{
{
snapshotName: "snapshot-snapuid2-5",
volume: newVolume("volume2-5", "pv-uid2-5", "pv-handle2-5", "1Gi", "pvc-uid2-5", "claim2-5", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
parameters: class5Parameters,
secrets: map[string]string{"foo": "bar"},
snapshotID: "sid2-5",
// information to return
driverName: mockDriverName,
snapshotId: "sid2-5",
timestamp: timeNow,
readyToUse: true,
createTime: timeNow,
},
},

errors: noerrors,
test: testSyncSnapshot,
},
{
name: "2-7 - snapshot and content bound, csi driver get status error",
initialContents: newContentArray("content2-7", validSecretClass, "sid2-7", "vuid2-7", "volume2-7", "snapuid2-7", "snap2-7", &deletePolicy, nil, nil, false),
expectedContents: newContentArray("content2-7", validSecretClass, "sid2-7", "vuid2-7", "volume2-7", "snapuid2-7", "snap2-7", &deletePolicy, nil, nil, false),
initialSnapshots: newSnapshotArray("snap2-7", validSecretClass, "content2-7", "snapuid2-7", "claim2-7", false, nil, metaTimeNow, nil),
expectedSnapshots: newSnapshotArray("snap2-7", validSecretClass, "content2-7", "snapuid2-7", "claim2-7", false, newVolumeError("Failed to check and update snapshot: mock create snapshot error"), metaTimeNow, nil),
expectedEvents: []string{"Warning SnapshotCheckandUpdateFailed"},
initialClaims: newClaimArray("claim2-7", "pvc-uid2-7", "1Gi", "volume2-7", v1.ClaimBound, &classEmpty),
initialVolumes: newVolumeArray("volume2-7", "pv-uid2-7", "pv-handle2-7", "1Gi", "pvc-uid2-7", "claim2-7", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
initialSecrets: []*v1.Secret{secret()},
expectedCreateCalls: []createCall{
name: "2-7 - snapshot and content bound, csi driver get status error",
initialContents: newContentArray("content2-7", validSecretClass, "sid2-7", "vuid2-7", "volume2-7", "snapuid2-7", "snap2-7", &deletePolicy, nil, nil, false),
expectedContents: newContentArray("content2-7", validSecretClass, "sid2-7", "vuid2-7", "volume2-7", "snapuid2-7", "snap2-7", &deletePolicy, nil, nil, false),
initialSnapshots: newSnapshotArray("snap2-7", validSecretClass, "content2-7", "snapuid2-7", "claim2-7", false, nil, metaTimeNow, nil),
expectedSnapshots: newSnapshotArray("snap2-7", validSecretClass, "content2-7", "snapuid2-7", "claim2-7",
false, newVolumeError("Failed to check and update snapshot: "+fmt.Sprintf(
"failed to list snapshot content %s: %q", "content2-7", "mock list snapshot error")), metaTimeNow, nil),
expectedEvents: []string{"Warning SnapshotCheckandUpdateFailed"},
initialClaims: newClaimArray("claim2-7", "pvc-uid2-7", "1Gi", "volume2-7", v1.ClaimBound, &classEmpty),
initialVolumes: newVolumeArray("volume2-7", "pv-uid2-7", "pv-handle2-7", "1Gi", "pvc-uid2-7", "claim2-7", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
initialSecrets: []*v1.Secret{secret()},
expectedListCalls: []listCall{
{
snapshotName: "snapshot-snapuid2-7",
volume: newVolume("volume2-7", "pv-uid2-7", "pv-handle2-7", "1Gi", "pvc-uid2-7", "claim2-7", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
parameters: class5Parameters,
secrets: map[string]string{"foo": "bar"},
snapshotID: "sid2-7",
// information to return
err: errors.New("mock create snapshot error"),
err: errors.New("mock list snapshot error"),
},
},

errors: noerrors,
test: testSyncSnapshot,
},
Expand All @@ -156,20 +148,16 @@ func TestSync(t *testing.T) {
initialClaims: newClaimArray("claim2-8", "pvc-uid2-8", "1Gi", "volume2-8", v1.ClaimBound, &classEmpty),
initialVolumes: newVolumeArray("volume2-8", "pv-uid2-8", "pv-handle2-8", "1Gi", "pvc-uid2-8", "claim2-8", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
initialSecrets: []*v1.Secret{secret()},
expectedCreateCalls: []createCall{
expectedListCalls: []listCall{
{
snapshotName: "snapshot-snapuid2-8",
volume: newVolume("volume2-8", "pv-uid2-8", "pv-handle2-8", "1Gi", "pvc-uid2-8", "claim2-8", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
parameters: class5Parameters,
secrets: map[string]string{"foo": "bar"},
snapshotID: "sid2-8",
// information to return
driverName: mockDriverName,
size: defaultSize,
snapshotId: "sid2-8",
timestamp: timeNow,
readyToUse: true,
createTime: timeNow,
size: defaultSize,
},
},

errors: []reactorError{
// Inject error to the first client.VolumesnapshotV1alpha1().VolumeSnapshots().Update call.
// All other calls will succeed.
Expand Down