diff --git a/pkg/controller/framework_test.go b/pkg/controller/framework_test.go index 33d04f386..4c3539e2f 100644 --- a/pkg/controller/framework_test.go +++ b/pkg/controller/framework_test.go @@ -775,7 +775,7 @@ func newTestController(kubeClient kubernetes.Interface, clientset clientset.Inte } func newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle string, - deletionPolicy crdv1.DeletionPolicy, size, creationTime *int64, + deletionPolicy crdv1.DeletionPolicy, creationTime, size *int64, withFinalizer bool) *crdv1.VolumeSnapshotContent { content := crdv1.VolumeSnapshotContent{ ObjectMeta: metav1.ObjectMeta{ @@ -830,14 +830,14 @@ func newContentArray(contentName, boundToSnapshotUID, boundToSnapshotName, snaps deletionPolicy crdv1.DeletionPolicy, size, creationTime *int64, withFinalizer bool) []*crdv1.VolumeSnapshotContent { return []*crdv1.VolumeSnapshotContent{ - newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle, deletionPolicy, size, creationTime, withFinalizer), + newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle, deletionPolicy, creationTime, size, withFinalizer), } } func newContentArrayWithReadyToUse(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle string, - deletionPolicy crdv1.DeletionPolicy, size, creationTime *int64, readyToUse *bool, + deletionPolicy crdv1.DeletionPolicy, creationTime, size *int64, readyToUse *bool, withFinalizer bool) []*crdv1.VolumeSnapshotContent { - content := newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle, deletionPolicy, size, creationTime, withFinalizer) + content := newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle, deletionPolicy, creationTime, size, withFinalizer) content.Status.ReadyToUse = readyToUse return []*crdv1.VolumeSnapshotContent{ content, @@ -881,9 +881,7 @@ func newSnapshot( snapshot.Status.BoundVolumeSnapshotContentName = &boundContentName } - if snapshotClassName != "" { - snapshot.Spec.VolumeSnapshotClassName = &snapshotClassName - } + snapshot.Spec.VolumeSnapshotClassName = &snapshotClassName if pvcName != "" { snapshot.Spec.Source = crdv1.VolumeSnapshotSource{ @@ -1406,7 +1404,6 @@ func (f *fakeSnapshotter) CreateSnapshot(ctx context.Context, snapshotName strin if err != nil { return "", "", time.Time{}, 0, false, fmt.Errorf("unexpected call") } - return call.driverName, call.snapshotId, call.creationTime, call.size, call.readyToUse, call.err } diff --git a/pkg/controller/snapshot_controller.go b/pkg/controller/snapshot_controller.go index dc07f840f..45d1c6b09 100644 --- a/pkg/controller/snapshot_controller.go +++ b/pkg/controller/snapshot_controller.go @@ -157,9 +157,6 @@ func (ctrl *csiSnapshotController) syncContent(content *crdv1.VolumeSnapshotCont ctrl.eventRecorder.Event(content, v1.EventTypeWarning, "SnapshotUnknownDeletionPolicy", "Volume Snapshot Content has unrecognized deletion policy") } return nil - // By default, we use Retain policy if it is not set by users - klog.V(4).Infof("VolumeSnapshotContent[%s]: by default the policy is Retain", content.Name) - } return nil } @@ -274,20 +271,35 @@ func (ctrl *csiSnapshotController) syncUnreadySnapshot(snapshot *crdv1.VolumeSna } return nil } else { // snapshot.Source.Spec.VolumeSnapshotContentName == nil + // find a matching volume snapshot content if contentObj := ctrl.getMatchSnapshotContent(snapshot); contentObj != nil { klog.V(5).Infof("Find VolumeSnapshotContent object %s for snapshot %s", contentObj.Name, uniqueSnapshotName) newSnapshot, err := ctrl.bindandUpdateVolumeSnapshot(contentObj, snapshot) if err != nil { return err } + if err = ctrl.checkandUpdateBoundSnapshotStatus(newSnapshot, contentObj); err != nil { + return err + } klog.V(5).Infof("bindandUpdateVolumeSnapshot %v", newSnapshot) - return nil - } else if snapshot.Status.Error == nil || isControllerUpdateFailError(snapshot.Status.Error) { // Try to create snapshot if no error status is set + } else if snapshot.Status.BoundVolumeSnapshotContentName != nil { + contentObj, found, err := ctrl.contentStore.GetByKey(*snapshot.Status.BoundVolumeSnapshotContentName) + if err != nil { + return err + } + if !found { + ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentMissing", "VolumeSnapshotContent is missing") + return fmt.Errorf("snapshot %s is bound to a non-existing content %s", uniqueSnapshotName, *snapshot.Status.BoundVolumeSnapshotContentName) + } else { + content, _ := contentObj.(*crdv1.VolumeSnapshotContent) + ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "InvalidSnapshotBinding", fmt.Sprintf("Snapshot is bound to a VolumeSnapshotContent which is bound to other Snapshot")) + return fmt.Errorf("snapshot %s is bound, but VolumeSnapshotContent %s is not bound to the VolumeSnapshot correctly", uniqueSnapshotName, content.Name) + } + } else if snapshot.Status.Error == nil || isControllerUpdateFailError(snapshot.Status.Error) { if err := ctrl.createSnapshot(snapshot); err != nil { ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotCreationFailed", fmt.Sprintf("Failed to create snapshot with error %v", err)) return err } - return nil } return nil } @@ -524,7 +536,6 @@ func (ctrl *csiSnapshotController) checkandBindSnapshotContent(snapshot *crdv1.V func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotClass, *v1.PersistentVolume, string, *v1.SecretReference, error) { className := snapshot.Spec.VolumeSnapshotClassName - klog.V(5).Infof("getCreateSnapshotInput [%s]: VolumeSnapshotClassName [%s]", snapshot.Name, *className) var class *crdv1.VolumeSnapshotClass var err error if className != nil { @@ -837,7 +848,6 @@ func (ctrl *csiSnapshotController) updateSnapshotContentStatus( if err != nil { return newControllerUpdateError(content.Name, err.Error()) } - return nil } return nil } diff --git a/pkg/controller/snapshot_create_test.go b/pkg/controller/snapshot_create_test.go index 6df796c82..2d691eccb 100644 --- a/pkg/controller/snapshot_create_test.go +++ b/pkg/controller/snapshot_create_test.go @@ -71,9 +71,9 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-1 - successful create snapshot with snapshot class gold", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-1", "snapuid6-1", "snap6-1", "sid6-1", classGold, "", "pv-handle-6-1", deletionPolicy, &defaultSize, &timeNowStamp, false), + expectedContents: newContentArrayWithReadyToUse("snapcontent-snapuid6-1", "snapuid6-1", "snap6-1", "sid6-1", classGold, "", "pv-handle6-1", deletionPolicy, &timeNowStamp, &defaultSize, &True, false), initialSnapshots: newSnapshotArray("snap6-1", "snapuid6-1", "claim6-1", "", classGold, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap6-1", "snapuid6-1", "claim6-1", "", classGold, "snapcontent-snapuid6-1", &False, metaTimeNowUnix, getSize(defaultSize), nil), + expectedSnapshots: newSnapshotArray("snap6-1", "snapuid6-1", "claim6-1", "", classGold, "snapcontent-snapuid6-1", &True, metaTimeNowUnix, getSize(defaultSize), nil), initialClaims: newClaimArray("claim6-1", "pvc-uid6-1", "1Gi", "volume6-1", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume6-1", "pv-uid6-1", "pv-handle6-1", "1Gi", "pvc-uid6-1", "claim6-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), expectedCreateCalls: []createCall{ @@ -86,7 +86,7 @@ func TestCreateSnapshotSync(t *testing.T) { size: defaultSize, snapshotId: "sid6-1", creationTime: timeNow, - readyToUse: true, + readyToUse: True, }, }, errors: noerrors, @@ -95,9 +95,9 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-2 - successful create snapshot with snapshot class silver", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-2", "snapuid6-2", "snap6-2", "sid6-2", classSilver, "", "pv-handle-6-2", deletionPolicy, &defaultSize, &timeNowStamp, false), + expectedContents: newContentArrayWithReadyToUse("snapcontent-snapuid6-2", "snapuid6-2", "snap6-2", "sid6-2", classSilver, "", "pv-handle6-2", deletionPolicy, &timeNowStamp, &defaultSize, &True, false), initialSnapshots: newSnapshotArray("snap6-2", "snapuid6-2", "claim6-2", "", classSilver, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap6-2", "snapuid6-2", "claim6-2", "", classSilver, "snapcontent-snapuid6-2", &False, metaTimeNowUnix, getSize(defaultSize), nil), + expectedSnapshots: newSnapshotArray("snap6-2", "snapuid6-2", "claim6-2", "", classSilver, "snapcontent-snapuid6-2", &True, metaTimeNowUnix, getSize(defaultSize), nil), initialClaims: newClaimArray("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume6-2", "pv-uid6-2", "pv-handle6-2", "1Gi", "pvc-uid6-2", "claim6-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), expectedCreateCalls: []createCall{ @@ -110,68 +110,16 @@ func TestCreateSnapshotSync(t *testing.T) { size: defaultSize, snapshotId: "sid6-2", creationTime: timeNow, - readyToUse: true, + readyToUse: True, }, }, errors: noerrors, test: testSyncSnapshot, }, - /*{ - name: "6-3 - successful create snapshot with snapshot class valid-secret-class", - initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-3", "sid6-3", "snapuid6-3", "snap6-3", &deletePolicy, &defaultSize, &timeNowStamp, &False), - initialSnapshots: newSnapshotArray("snap6-3", validSecretClass, "", "snapuid6-3", "claim6-3", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap6-3", validSecretClass, "snapcontent-snapuid6-3", "snapuid6-3", "claim6-3", &False, nil, metaTimeNowUnix, getSize(defaultSize)), - initialClaims: newClaimArray("claim6-3", "pvc-uid6-3", "1Gi", "volume6-3", v1.ClaimBound, &classEmpty), - initialVolumes: newVolumeArray("volume6-3", "pv-uid6-3", "pv-handle6-3", "1Gi", "pvc-uid6-3", "claim6-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - initialSecrets: []*v1.Secret{secret()}, - expectedCreateCalls: []createCall{ - { - snapshotName: "snapshot-snapuid6-3", - volume: newVolume("volume6-3", "pv-uid6-3", "pv-handle6-3", "1Gi", "pvc-uid6-3", "claim6-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - parameters: class5Parameters, - secrets: map[string]string{"foo": "bar"}, - // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid6-3", - creationTime: timeNow, - readyToUse: true, - }, - }, - errors: noerrors, - test: testSyncSnapshot, - }, - { - name: "6-4 - successful create snapshot with snapshot class empty-secret-class", - initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-4", "sid6-4", "snapuid6-4", "snap6-4", &deletePolicy, &defaultSize, &timeNowStamp, &False), - initialSnapshots: newSnapshotArray("snap6-4", emptySecretClass, "", "snapuid6-4", "claim6-4", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap6-4", emptySecretClass, "snapcontent-snapuid6-4", "snapuid6-4", "claim6-4", &False, nil, metaTimeNowUnix, getSize(defaultSize)), - initialClaims: newClaimArray("claim6-4", "pvc-uid6-4", "1Gi", "volume6-4", v1.ClaimBound, &classEmpty), - initialVolumes: newVolumeArray("volume6-4", "pv-uid6-4", "pv-handle6-4", "1Gi", "pvc-uid6-4", "claim6-4", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - initialSecrets: []*v1.Secret{emptySecret()}, - expectedCreateCalls: []createCall{ - { - snapshotName: "snapshot-snapuid6-4", - volume: newVolume("volume6-4", "pv-uid6-4", "pv-handle6-4", "1Gi", "pvc-uid6-4", "claim6-4", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - parameters: class4Parameters, - secrets: map[string]string{}, - // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid6-4", - creationTime: timeNow, - readyToUse: true, - }, - }, - errors: noerrors, - test: testSyncSnapshot, - },*/ { name: "6-5 - successful create snapshot with status uploading", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-5", "snapuid6-5", "snap6-5", "sid6-5", classGold, "", "pv-handle-6-5", deletionPolicy, &defaultSize, &timeNowStamp, false), + expectedContents: newContentArrayWithReadyToUse("snapcontent-snapuid6-5", "snapuid6-5", "snap6-5", "sid6-5", classGold, "", "pv-handle6-5", deletionPolicy, &timeNowStamp, &defaultSize, &False, false), initialSnapshots: newSnapshotArray("snap6-5", "snapuid6-5", "claim6-5", "", classGold, "", &False, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-5", "snapuid6-5", "claim6-5", "", classGold, "snapcontent-snapuid6-5", &False, metaTimeNowUnix, getSize(defaultSize), nil), initialClaims: newClaimArray("claim6-5", "pvc-uid6-5", "1Gi", "volume6-5", v1.ClaimBound, &classEmpty), @@ -186,16 +134,17 @@ func TestCreateSnapshotSync(t *testing.T) { size: defaultSize, snapshotId: "sid6-5", creationTime: timeNow, - readyToUse: true, + readyToUse: False, }, }, errors: noerrors, test: testSyncSnapshot, }, { + // TODO(xiangqian): this test does not match its name name: "6-6 - successful create snapshot with status error uploading", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-6", "snapuid6-6", "snap6-6", "sid6-6", classGold, "", "pv-handle-6-6", deletionPolicy, &defaultSize, &timeNowStamp, false), + expectedContents: newContentArrayWithReadyToUse("snapcontent-snapuid6-6", "snapuid6-6", "snap6-6", "sid6-6", classGold, "", "pv-handle6-6", deletionPolicy, &timeNowStamp, &defaultSize, &False, false), initialSnapshots: newSnapshotArray("snap6-6", "snapuid6-6", "claim6-6", "", classGold, "", &False, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-6", "snapuid6-6", "claim6-6", "", classGold, "snapcontent-snapuid6-6", &False, metaTimeNowUnix, getSize(defaultSize), nil), initialClaims: newClaimArray("claim6-6", "pvc-uid6-6", "1Gi", "volume6-6", v1.ClaimBound, &classEmpty), @@ -210,18 +159,18 @@ func TestCreateSnapshotSync(t *testing.T) { size: defaultSize, snapshotId: "sid6-6", creationTime: timeNow, - readyToUse: true, + readyToUse: False, }, }, errors: noerrors, test: testSyncSnapshot, }, { - name: "7-1 - fail create snapshot with snapshot class non-existing", + name: "7-1 - fail to create snapshot with non-existing snapshot class", initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "snapcontent-snapuid7-1", &False, nil, nil, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-1: \"failed to retrieve snapshot class non-existing from the informer: \\\"volumesnapshotclass.snapshot.storage.k8s.io \\\\\\\"non-existing\\\\\\\" not found\\\"\"")), + expectedSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", &False, nil, nil, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-1: \"failed to retrieve snapshot class non-existing from the informer: \\\"volumesnapshotclass.snapshot.storage.k8s.io \\\\\\\"non-existing\\\\\\\" not found\\\"\"")), initialClaims: newClaimArray("claim7-1", "pvc-uid7-1", "1Gi", "volume7-1", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume7-1", "pv-uid7-1", "pv-handle7-1", "1Gi", "pvc-uid7-1", "claim7-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), expectedEvents: []string{"Warning SnapshotCreationFailed"}, @@ -229,11 +178,11 @@ func TestCreateSnapshotSync(t *testing.T) { test: testSyncSnapshot, }, { - name: "7-2 - fail create snapshot with snapshot class invalid-secret-class", + name: "7-2 - fail to create snapshot with snapshot class invalid-secret-class", initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-2", "snapuid7-2", "claim7-2", "", invalidSecretClass, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-2", "snapuid7-2", "claim7-2", "", classNonExisting, "snapcontent-snapuid7-2", &False, nil, nil, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-2: \"failed to get name and namespace template from params: either name and namespace for Snapshotter secrets specified, Both must be specified\"")), + expectedSnapshots: newSnapshotArray("snap7-2", "snapuid7-2", "claim7-2", "", invalidSecretClass, "", &False, nil, nil, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-2: \"failed to get name and namespace template from params: either name and namespace for Snapshotter secrets specified, Both must be specified\"")), initialClaims: newClaimArray("claim7-2", "pvc-uid7-2", "1Gi", "volume7-2", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume7-2", "pv-uid7-2", "pv-handle7-2", "1Gi", "pvc-uid7-2", "claim7-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), expectedEvents: []string{"Warning SnapshotCreationFailed"}, @@ -241,7 +190,7 @@ func TestCreateSnapshotSync(t *testing.T) { test: testSyncSnapshot, }, { - name: "7-3 - fail create snapshot with none snapshot class ", + name: "7-3 - fail to create snapshot without snapshot class ", initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "", "", "", &False, nil, nil, nil), @@ -258,7 +207,7 @@ func TestCreateSnapshotSync(t *testing.T) { initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-4", "snapuid7-4", "claim7-4", "", classGold, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-4", "snapuid7-4", "claim7-4", "", classGold, "snapuid7-4", &False, nil, nil, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-4: \"failed to retrieve PVC claim7-4 from the lister: \\\"persistentvolumeclaim \\\\\\\"claim7-4\\\\\\\" not found\\\"\"")), + expectedSnapshots: newSnapshotArray("snap7-4", "snapuid7-4", "claim7-4", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-4: \"failed to retrieve PVC claim7-4 from the lister: \\\"persistentvolumeclaim \\\\\\\"claim7-4\\\\\\\" not found\\\"\"")), 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, @@ -269,7 +218,7 @@ func TestCreateSnapshotSync(t *testing.T) { initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-5", "snapuid7-5", "claim7-5", "", classGold, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-5", "snapuid7-5", "claim7-5", "", classGold, "snapuid7-5", &False, nil, nil, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-5: \"failed to retrieve PV volume7-5 from the API server: \\\"cannot find volume volume7-5\\\"\"")), + expectedSnapshots: newSnapshotArray("snap7-5", "snapuid7-5", "claim7-5", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-5: \"failed to retrieve PV volume7-5 from the API server: \\\"cannot find volume volume7-5\\\"\"")), initialClaims: newClaimArray("claim7-5", "pvc-uid7-5", "1Gi", "volume7-5", v1.ClaimBound, &classEmpty), expectedEvents: []string{"Warning SnapshotCreationFailed"}, errors: noerrors, @@ -280,7 +229,7 @@ func TestCreateSnapshotSync(t *testing.T) { initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-6", "snapuid7-6", "claim7-6", "", classGold, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-6", "snapuid7-6", "claim7-6", "", classGold, "snapuid7-6", &False, nil, nil, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-6: \"the PVC claim7-6 is not yet bound to a PV, will not attempt to take a snapshot\"")), + expectedSnapshots: newSnapshotArray("snap7-6", "snapuid7-6", "claim7-6", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-6: \"the PVC claim7-6 is not yet bound to a PV, will not attempt to take a snapshot\"")), initialClaims: newClaimArray("claim7-6", "pvc-uid7-6", "1Gi", "", v1.ClaimPending, &classEmpty), expectedEvents: []string{"Warning SnapshotCreationFailed"}, errors: noerrors, @@ -291,7 +240,7 @@ func TestCreateSnapshotSync(t *testing.T) { initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-7", "snapuid7-7", "claim7-7", "", classGold, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-7", "snapuid7-7", "claim7-7", "", classGold, "snapuid7-7", &False, nil, nil, newVolumeError("Failed to create snapshot: failed to take snapshot of the volume, volume7-7: \"mock create snapshot error\"")), + expectedSnapshots: newSnapshotArray("snap7-7", "snapuid7-7", "claim7-7", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot: failed to take snapshot of the volume, volume7-7: \"mock create snapshot error\"")), initialClaims: newClaimArray("claim7-7", "pvc-uid7-7", "1Gi", "volume7-7", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume7-7", "pv-uid7-7", "pv-handle7-7", "1Gi", "pvc-uid7-7", "claim7-7", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), expectedCreateCalls: []createCall{ @@ -312,7 +261,7 @@ func TestCreateSnapshotSync(t *testing.T) { initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-8", "snapuid7-8", "claim7-8", "", classGold, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-8", "snapuid7-8", "claim7-8", "", classGold, "snapuid7-8", &False, nil, nil, newVolumeError("Failed to create snapshot: snapshot controller failed to update default/snap7-8 on API server: mock update error")), + expectedSnapshots: newSnapshotArray("snap7-8", "snapuid7-8", "claim7-8", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot: snapshot controller failed to update default/snap7-8 on API server: mock update error")), initialClaims: newClaimArray("claim7-8", "pvc-uid7-8", "1Gi", "volume7-8", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume7-8", "pv-uid7-8", "pv-handle7-8", "1Gi", "pvc-uid7-8", "claim7-8", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), expectedCreateCalls: []createCall{ @@ -325,7 +274,7 @@ func TestCreateSnapshotSync(t *testing.T) { size: defaultSize, snapshotId: "sid7-8", creationTime: timeNow, - readyToUse: true, + readyToUse: True, }, }, errors: []reactorError{ @@ -339,11 +288,14 @@ func TestCreateSnapshotSync(t *testing.T) { test: testSyncSnapshot, }, { + // TODO(xiangqian): this test case needs to be revisited the scenario + // of VolumeSnapshotContent saving failure. Since there will be no content object + // in API server, it could potentially cause leaking issue name: "7-9 - fail create snapshot due to cannot save snapshot content", initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-9", "snapuid7-9", "claim7-9", "", classGold, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-9", "snapuid7-9", "claim7-9", "", classGold, "snapuid7-8", &False, metaTimeNowUnix, getSize(defaultSize), nil), + expectedSnapshots: newSnapshotArray("snap7-9", "snapuid7-9", "claim7-9", "", classGold, "snapcontent-snapuid7-9", &True, metaTimeNowUnix, getSize(defaultSize), nil), initialClaims: newClaimArray("claim7-9", "pvc-uid7-9", "1Gi", "volume7-9", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume7-9", "pv-uid7-9", "pv-handle7-9", "1Gi", "pvc-uid7-9", "claim7-9", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), expectedCreateCalls: []createCall{ @@ -356,7 +308,7 @@ func TestCreateSnapshotSync(t *testing.T) { size: defaultSize, snapshotId: "sid7-9", creationTime: timeNow, - readyToUse: true, + readyToUse: True, }, }, errors: []reactorError{ @@ -372,7 +324,7 @@ func TestCreateSnapshotSync(t *testing.T) { initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-10", "snapuid7-10", "claim7-10", "", validSecretClass, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-10", "snapuid7-10", "claim7-10", "", validSecretClass, "snapuid7-10", &False, nil, nil, newVolumeError("Failed to create snapshot: error getting secret secret in namespace default: cannot find secret secret")), + expectedSnapshots: newSnapshotArray("snap7-10", "snapuid7-10", "claim7-10", "", validSecretClass, "", &False, nil, nil, newVolumeError("Failed to create snapshot: error getting secret secret in namespace default: cannot find secret secret")), initialClaims: newClaimArray("claim7-10", "pvc-uid7-10", "1Gi", "volume7-10", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume7-10", "pv-uid7-10", "pv-handle7-10", "1Gi", "pvc-uid7-10", "claim7-10", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{}, // no initial secret created diff --git a/pkg/controller/snapshot_delete_test.go b/pkg/controller/snapshot_delete_test.go index d38d0062d..f3c157bd3 100644 --- a/pkg/controller/snapshot_delete_test.go +++ b/pkg/controller/snapshot_delete_test.go @@ -56,8 +56,9 @@ var snapshotClasses = []*crdv1.VolumeSnapshotClass{ ObjectMeta: metav1.ObjectMeta{ Name: classGold, }, - Driver: mockDriverName, - Parameters: class1Parameters, + Driver: mockDriverName, + Parameters: class1Parameters, + DeletionPolicy: crdv1.VolumeSnapshotContentDelete, }, { TypeMeta: metav1.TypeMeta{ @@ -66,8 +67,9 @@ var snapshotClasses = []*crdv1.VolumeSnapshotClass{ ObjectMeta: metav1.ObjectMeta{ Name: classSilver, }, - Driver: mockDriverName, - Parameters: class2Parameters, + Driver: mockDriverName, + Parameters: class2Parameters, + DeletionPolicy: crdv1.VolumeSnapshotContentDelete, }, { TypeMeta: metav1.TypeMeta{ @@ -76,8 +78,9 @@ var snapshotClasses = []*crdv1.VolumeSnapshotClass{ ObjectMeta: metav1.ObjectMeta{ Name: emptySecretClass, }, - Driver: mockDriverName, - Parameters: class4Parameters, + Driver: mockDriverName, + Parameters: class4Parameters, + DeletionPolicy: crdv1.VolumeSnapshotContentDelete, }, { TypeMeta: metav1.TypeMeta{ @@ -86,8 +89,9 @@ var snapshotClasses = []*crdv1.VolumeSnapshotClass{ ObjectMeta: metav1.ObjectMeta{ Name: invalidSecretClass, }, - Driver: mockDriverName, - Parameters: class3Parameters, + Driver: mockDriverName, + Parameters: class3Parameters, + DeletionPolicy: crdv1.VolumeSnapshotContentDelete, }, { TypeMeta: metav1.TypeMeta{ @@ -96,8 +100,9 @@ var snapshotClasses = []*crdv1.VolumeSnapshotClass{ ObjectMeta: metav1.ObjectMeta{ Name: validSecretClass, }, - Driver: mockDriverName, - Parameters: class5Parameters, + Driver: mockDriverName, + Parameters: class5Parameters, + DeletionPolicy: crdv1.VolumeSnapshotContentDelete, }, { TypeMeta: metav1.TypeMeta{ @@ -107,7 +112,8 @@ var snapshotClasses = []*crdv1.VolumeSnapshotClass{ Name: defaultClass, Annotations: map[string]string{IsDefaultSnapshotClassAnnotation: "true"}, }, - Driver: mockDriverName, + Driver: mockDriverName, + DeletionPolicy: crdv1.VolumeSnapshotContentDelete, }, } diff --git a/pkg/controller/snapshot_ready_test.go b/pkg/controller/snapshot_ready_test.go index ae629000a..5a6c4e5fc 100644 --- a/pkg/controller/snapshot_ready_test.go +++ b/pkg/controller/snapshot_ready_test.go @@ -47,28 +47,28 @@ func TestSync(t *testing.T) { name: "2-1 - snapshot is bound to a non-existing content", initialContents: nocontents, expectedContents: nocontents, - initialSnapshots: newSnapshotArray("snap2-1", "snapuid2-1", "claim2-1", "", validSecretClass, "content2-1", &False, nil, nil, nil), + initialSnapshots: newSnapshotArray("snap2-1", "snapuid2-1", "claim2-1", "", validSecretClass, "content2-1", &True, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap2-1", "snapuid2-1", "claim2-1", "", validSecretClass, "content2-1", &False, nil, nil, newVolumeError("VolumeSnapshotContent is missing")), expectedEvents: []string{"Warning SnapshotContentMissing"}, errors: noerrors, - test: testSyncSnapshotError, + test: testSyncSnapshot, }, { - name: "2-2 - could not bind snapshot and content, the VolumeSnapshotRef does not match", + name: "2-2 - snapshot points to a content but content does not point to snapshot(VolumeSnapshotRef does not match)", initialContents: newContentArray("content2-2", "snapuid2-2-x", "snap2-2", "sid2-2", validSecretClass, "", "", deletionPolicy, nil, nil, false), expectedContents: newContentArray("content2-2", "snapuid2-2-x", "snap2-2", "sid2-2", validSecretClass, "", "", deletionPolicy, nil, nil, false), initialSnapshots: newSnapshotArray("snap2-2", "snapuid2-2", "claim2-2", "", validSecretClass, "content2-2", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap2-2", "snapuid2-2", "claim2-2", "", validSecretClass, "content2-2", &False, nil, nil, newVolumeError("Snapshot failed to bind VolumeSnapshotContent, Could not bind snapshot snap2-2 and content content2-2, the VolumeSnapshotRef does not match")), - expectedEvents: []string{"Warning SnapshotBindFailed"}, + expectedSnapshots: newSnapshotArray("snap2-2", "snapuid2-2", "claim2-2", "", validSecretClass, "content2-2", &False, nil, nil, newVolumeError("Snapshot is bound to a VolumeSnapshotContent which is bound to other Snapshot")), + expectedEvents: []string{"Warning InvalidSnapshotBinding"}, errors: noerrors, test: testSyncSnapshotError, }, { name: "2-3 - success bind snapshot and content but not ready, no status changed", - initialContents: newContentArray("content2-3", "", "snap2-3", "sid2-3", validSecretClass, "", "", deletionPolicy, nil, nil, false), - expectedContents: newContentArray("content2-3", "snapuid2-3", "snap2-3", "sid2-3", validSecretClass, "", "", deletionPolicy, nil, nil, false), + initialContents: newContentArray("content2-3", "snapuid2-3", "snap2-3", "sid2-3", validSecretClass, "", "", deletionPolicy, nil, nil, false), + expectedContents: newContentArrayWithReadyToUse("content2-3", "snapuid2-3", "snap2-3", "sid2-3", validSecretClass, "", "", deletionPolicy, &timeNowStamp, &defaultSize, &False, false), initialSnapshots: newSnapshotArray("snap2-3", "snapuid2-3", "claim2-3", "", validSecretClass, "content2-3", &False, metaTimeNow, nil, nil), - expectedSnapshots: newSnapshotArray("snap2-3", "snapuid2-3", "claim2-3", "", validSecretClass, "content2-3", &False, metaTimeNow, nil, nil), + expectedSnapshots: newSnapshotArray("snap2-3", "snapuid2-3", "claim2-3", "", validSecretClass, "content2-3", &False, metaTimeNow, getSize(defaultSize), nil), 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()}, @@ -80,9 +80,10 @@ func TestSync(t *testing.T) { secrets: map[string]string{"foo": "bar"}, // information to return driverName: mockDriverName, + size: defaultSize, snapshotId: "sid2-3", creationTime: timeNow, - readyToUse: false, + readyToUse: False, }, }, errors: noerrors, @@ -101,9 +102,9 @@ func TestSync(t *testing.T) { { name: "2-5 - snapshot and content bound, status ready false -> true", initialContents: newContentArrayWithReadyToUse("content2-5", "snapuid2-5", "snap2-5", "sid2-5", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), - expectedContents: newContentArrayWithReadyToUse("content2-5", "snapuid2-5", "snap2-5", "sid2-5", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &True, false), + expectedContents: newContentArrayWithReadyToUse("content2-5", "snapuid2-5", "snap2-5", "sid2-5", validSecretClass, "", "", deletionPolicy, &timeNowStamp, &defaultSize, &True, false), initialSnapshots: newSnapshotArray("snap2-5", "snapuid2-5", "claim2-5", "", validSecretClass, "content2-5", &False, metaTimeNow, nil, nil), - expectedSnapshots: newSnapshotArray("snap2-5", "snapuid2-5", "claim2-5", "", validSecretClass, "content2-5", &True, metaTimeNow, nil, nil), + expectedSnapshots: newSnapshotArray("snap2-5", "snapuid2-5", "claim2-5", "", validSecretClass, "content2-5", &True, metaTimeNow, getSize(defaultSize), nil), 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()}, @@ -115,9 +116,10 @@ func TestSync(t *testing.T) { secrets: map[string]string{"foo": "bar"}, // information to return driverName: mockDriverName, + size: defaultSize, snapshotId: "sid2-5", creationTime: timeNow, - readyToUse: true, + readyToUse: True, }, }, errors: noerrors, @@ -126,11 +128,12 @@ func TestSync(t *testing.T) { { name: "2-6 - snapshot bound to prebound content correctly, status ready false -> true, ref.UID '' -> 'snapuid2-6'", initialContents: newContentArrayWithReadyToUse("content2-6", "snapuid2-6", "snap2-6", "sid2-6", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), - expectedContents: newContentArrayWithReadyToUse("content2-6", "snapuid2-6", "snap2-6", "sid2-6", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &True, false), + expectedContents: newContentArrayWithReadyToUse("content2-6", "snapuid2-6", "snap2-6", "sid2-6", validSecretClass, "", "", deletionPolicy, &timeNowStamp, &defaultSize, &True, false), initialSnapshots: newSnapshotArray("snap2-6", "snapuid2-6", "", "content2-6", validSecretClass, "content2-6", &False, metaTimeNow, nil, nil), - expectedSnapshots: newSnapshotArray("snap2-6", "snapuid2-6", "", "content2-6", validSecretClass, "content2-6", &True, metaTimeNow, nil, nil), + expectedSnapshots: newSnapshotArray("snap2-6", "snapuid2-6", "", "content2-6", validSecretClass, "content2-6", &True, metaTimeNow, getSize(defaultSize), nil), expectedListCalls: []listCall{ { + size: defaultSize, snapshotID: "sid2-6", readyToUse: true, }, @@ -164,7 +167,7 @@ func TestSync(t *testing.T) { { name: "2-8 - snapshot and content bound, apiserver update status error", initialContents: newContentArrayWithReadyToUse("content2-8", "snapuid2-8", "snap2-8", "sid2-8", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), - expectedContents: newContentArrayWithReadyToUse("content2-8", "snapuid2-8", "snap2-8", "sid2-8", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &True, false), + expectedContents: newContentArrayWithReadyToUse("content2-8", "snapuid2-8", "snap2-8", "sid2-8", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), initialSnapshots: newSnapshotArray("snap2-8", "snapuid2-8", "claim2-8", "", validSecretClass, "content2-8", &False, metaTimeNow, nil, nil), expectedSnapshots: newSnapshotArray("snap2-8", "snapuid2-8", "claim2-8", "", validSecretClass, "content2-8", &False, metaTimeNow, nil, newVolumeError("Failed to check and update snapshot: snapshot controller failed to update default/snap2-8 on API server: mock update error")), expectedEvents: []string{"Warning SnapshotCheckandUpdateFailed"}, @@ -193,11 +196,11 @@ func TestSync(t *testing.T) { test: testSyncSnapshot, }, { - name: "2-9 - bind when snapshot and content matches", + name: "2-9 - bind when snapshot and content matches, fail on status update as there is not pvc provided", initialContents: newContentArray("content2-9", "snapuid2-9", "snap2-9", "sid2-9", validSecretClass, "", "", deletionPolicy, nil, nil, false), expectedContents: newContentArray("content2-9", "snapuid2-9", "snap2-9", "sid2-9", validSecretClass, "", "", deletionPolicy, nil, nil, false), initialSnapshots: newSnapshotArray("snap2-9", "snapuid2-9", "claim2-9", "", validSecretClass, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap2-9", "snapuid2-9", "claim2-9", "", validSecretClass, "content2-9", &False, nil, nil, nil), + expectedSnapshots: newSnapshotArray("snap2-9", "snapuid2-9", "claim2-9", "", validSecretClass, "content2-9", &False, nil, nil, newVolumeError("Failed to check and update snapshot: failed to get input parameters to create snapshot snap2-9: \"failed to retrieve PVC claim2-9 from the lister: \\\"persistentvolumeclaim \\\\\\\"claim2-9\\\\\\\" not found\\\"\"")), errors: noerrors, test: testSyncSnapshot, }, @@ -215,9 +218,9 @@ func TestSync(t *testing.T) { initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap3-1", "snapuid3-1", "claim3-1", "", validSecretClass, "content3-1", &True, metaTimeNow, nil, nil), - expectedSnapshots: newSnapshotArray("snap3-1", "snapuid3-1", "claim3-1", "", validSecretClass, "content3-1", &False, metaTimeNow, nil, newVolumeError("Bound snapshot has lost reference to VolumeSnapshotContent")), + expectedSnapshots: newSnapshotArray("snap3-1", "snapuid3-1", "claim3-1", "", validSecretClass, "content3-1", &False, metaTimeNow, nil, newVolumeError("VolumeSnapshotContent is missing")), errors: noerrors, - expectedEvents: []string{"Warning SnapshotLost"}, + expectedEvents: []string{"Warning SnapshotContentMissing"}, test: testSyncSnapshot, }, {