From c01fb649f287b3bfa723d12f7e131ec858ef08f0 Mon Sep 17 00:00:00 2001 From: Xing Yang Date: Fri, 16 Nov 2018 22:03:53 -0800 Subject: [PATCH] Call CreateSnapshot to check if snapshot is processed This PR calls CreateSnapshot to check if the snapshot post-cut processing (uploading) is complete and update status accordingly. --- pkg/controller/snapshot_controller.go | 137 ++++++++++++------------- pkg/controller/snapshot_create_test.go | 12 +-- 2 files changed, 71 insertions(+), 78 deletions(-) diff --git a/pkg/controller/snapshot_controller.go b/pkg/controller/snapshot_controller.go index a2fc74fa5..f41b8c3d2 100644 --- a/pkg/controller/snapshot_controller.go +++ b/pkg/controller/snapshot_controller.go @@ -421,13 +421,65 @@ func (ctrl *csiSnapshotController) checkandBindSnapshotContent(snapshot *crdv1.V return nil } +func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotClass, *v1.PersistentVolume, string, map[string]string, error) { + className := snapshot.Spec.VolumeSnapshotClassName + glog.V(5).Infof("getCreateSnapshotInput [%s]: VolumeSnapshotClassName [%s]", snapshot.Name, *className) + var class *crdv1.VolumeSnapshotClass + var err error + if className != nil { + class, err = ctrl.GetSnapshotClass(*className) + if err != nil { + glog.Errorf("getCreateSnapshotInput failed to getClassFromVolumeSnapshot %s", err) + return nil, nil, "", nil, err + } + } else { + glog.Errorf("failed to getCreateSnapshotInput %s without a snapshot class", snapshot.Name) + return nil, nil, "", nil, fmt.Errorf("failed to take snapshot %s without a snapshot class", snapshot.Name) + } + + volume, err := ctrl.getVolumeFromVolumeSnapshot(snapshot) + if err != nil { + glog.Errorf("getCreateSnapshotInput failed to get PersistentVolume object [%s]: Error: [%#v]", snapshot.Name, err) + return nil, nil, "", nil, err + } + + // Create VolumeSnapshotContent name + contentName := GetSnapshotContentNameForSnapshot(snapshot) + + // Resolve snapshotting secret credentials. + snapshotterSecretRef, err := GetSecretReference(class.Parameters, contentName, snapshot) + if err != nil { + return nil, nil, "", nil, err + } + snapshotterCredentials, err := GetCredentials(ctrl.client, snapshotterSecretRef) + if err != nil { + return nil, nil, "", nil, err + } + + return class, volume, contentName, snapshotterCredentials, nil +} + func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshot, error) { - status, _, size, err := ctrl.handler.GetSnapshotStatus(content) + var err error + var timestamp int64 = 0 + var size int64 = 0 + var readyToUse bool = false + class, volume, _, snapshotterCredentials, err := ctrl.getCreateSnapshotInput(snapshot) if err != nil { - return nil, fmt.Errorf("failed to check snapshot status %s with error %v", snapshot.Name, err) + return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err) } - timestamp := time.Now().UnixNano() - newSnapshot, err := ctrl.updateSnapshotStatus(snapshot, status, timestamp, size, IsSnapshotBound(snapshot, content)) + driverName, snapshotID, timestamp, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials) + if err != nil { + glog.Errorf("checkandUpdateBoundSnapshotStatusOperation: failed to call create snapshot to check whether the snapshot is ready to use %q", err) + return nil, err + } else { + glog.V(5).Infof("checkandUpdateBoundSnapshotStatusOperation: driver %s, snapshotId %s, timestamp %d, size %d, readyToUse %t", driverName, snapshotID, timestamp, size, readyToUse) + } + + if timestamp == 0 { + timestamp = time.Now().UnixNano() + } + newSnapshot, err := ctrl.updateSnapshotStatus(snapshot, readyToUse, timestamp, size, IsSnapshotBound(snapshot, content)) if err != nil { return nil, err } @@ -451,51 +503,22 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum return snapshot, nil } - className := snapshot.Spec.VolumeSnapshotClassName - glog.V(5).Infof("createSnapshotOperation [%s]: VolumeSnapshotClassName [%s]", snapshot.Name, *className) - var class *crdv1.VolumeSnapshotClass - var err error - if className != nil { - class, err = ctrl.GetSnapshotClass(*className) - if err != nil { - glog.Errorf("createSnapshotOperation failed to getClassFromVolumeSnapshot %s", err) - return nil, err - } - } else { - glog.Errorf("failed to take snapshot %s without a snapshot class", snapshot.Name) - return nil, fmt.Errorf("failed to take snapshot %s without a snapshot class", snapshot.Name) - } - - volume, err := ctrl.getVolumeFromVolumeSnapshot(snapshot) + class, volume, contentName, snapshotterCredentials, err := ctrl.getCreateSnapshotInput(snapshot) if err != nil { - glog.Errorf("createSnapshotOperation failed to get PersistentVolume object [%s]: Error: [%#v]", snapshot.Name, err) - return nil, err + return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err) } - // Create VolumeSnapshotContent name - contentName := GetSnapshotContentNameForSnapshot(snapshot) - - // Resolve snapshotting secret credentials. - snapshotterSecretRef, err := GetSecretReference(class.Parameters, contentName, snapshot) - if err != nil { - return nil, err - } - snapshotterCredentials, err := GetCredentials(ctrl.client, snapshotterSecretRef) - if err != nil { - return nil, err - } - - driverName, snapshotID, timestamp, size, csiSnapshotStatus, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials) + driverName, snapshotID, timestamp, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials) if err != nil { return nil, fmt.Errorf("failed to take snapshot of the volume, %s: %q", volume.Name, err) } - glog.V(5).Infof("Created snapshot: driver %s, snapshotId %s, timestamp %d, size %d, csiSnapshotStatus %v", driverName, snapshotID, timestamp, size, csiSnapshotStatus) + glog.V(5).Infof("Created snapshot: driver %s, snapshotId %s, timestamp %d, size %d, readyToUse %t", driverName, snapshotID, timestamp, size, readyToUse) var newSnapshot *crdv1.VolumeSnapshot // Update snapshot status with timestamp for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ { glog.V(5).Infof("createSnapshot [%s]: trying to update snapshot creation timestamp", snapshotKey(snapshot)) - newSnapshot, err = ctrl.updateSnapshotStatus(snapshot, csiSnapshotStatus, timestamp, size, false) + newSnapshot, err = ctrl.updateSnapshotStatus(snapshot, readyToUse, timestamp, size, false) if err == nil { break } @@ -682,42 +705,12 @@ func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSn status.Error = nil change = true } - if status.CreationTime == nil { - status.CreationTime = timeAt - change = true - } } - - /* TODO FIXME - switch csistatus.Type { - case csi.SnapshotStatus_READY: - if bound { - status.Ready = true - // Remove the error if checking snapshot is already bound and ready - status.Error = nil - change = true - } - if status.CreationTime == nil { - status.CreationTime = timeAt - change = true - } - case csi.SnapshotStatus_ERROR_UPLOADING: - if status.Error == nil { - status.Error = &storage.VolumeError{ - Time: *timeAt, - Message: "Failed to upload the snapshot", - } - change = true - ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotUploadError", "Failed to upload the snapshot") - - } - case csi.SnapshotStatus_UPLOADING: - if status.CreationTime == nil { - status.CreationTime = timeAt - change = true - } + if status.CreationTime == nil { + status.CreationTime = timeAt + change = true } - */ + if change { if size > 0 { status.RestoreSize = resource.NewQuantity(size, resource.BinarySI) diff --git a/pkg/controller/snapshot_create_test.go b/pkg/controller/snapshot_create_test.go index 83279aaa0..fa8b0ca81 100644 --- a/pkg/controller/snapshot_create_test.go +++ b/pkg/controller/snapshot_create_test.go @@ -215,7 +215,7 @@ func TestCreateSnapshotSync(t *testing.T) { initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-1", classNonExisting, "", "snapuid7-1", "claim7-1", false, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-1", classNonExisting, "", "snapuid7-1", "claim7-1", false, newVolumeError("Failed to create snapshot: failed to retrieve snapshot class non-existing from the informer: \"volumesnapshotclass.snapshot.storage.k8s.io \\\"non-existing\\\" not found\""), nil, nil), + expectedSnapshots: newSnapshotArray("snap7-1", classNonExisting, "", "snapuid7-1", "claim7-1", false, 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\\\"\""), nil, nil), 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"}, @@ -227,7 +227,7 @@ func TestCreateSnapshotSync(t *testing.T) { initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-2", invalidSecretClass, "", "snapuid7-2", "claim7-2", false, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-2", invalidSecretClass, "", "snapuid7-2", "claim7-2", false, newVolumeError("Failed to create snapshot: csiSnapshotterSecretName and csiSnapshotterSecretNamespace parameters must be specified together"), nil, nil), + expectedSnapshots: newSnapshotArray("snap7-2", invalidSecretClass, "", "snapuid7-2", "claim7-2", false, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-2: \"csiSnapshotterSecretName and csiSnapshotterSecretNamespace parameters must be specified together\""), nil, nil), 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"}, @@ -239,7 +239,7 @@ func TestCreateSnapshotSync(t *testing.T) { initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-3", "", "", "snapuid7-3", "claim7-3", false, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-3", "", "", "snapuid7-3", "claim7-3", false, newVolumeError("Failed to create snapshot: failed to retrieve snapshot class from the informer: \"volumesnapshotclass.snapshot.storage.k8s.io \\\"\\\" not found\""), nil, nil), + expectedSnapshots: newSnapshotArray("snap7-3", "", "", "snapuid7-3", "claim7-3", false, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-3: \"failed to retrieve snapshot class from the informer: \\\"volumesnapshotclass.snapshot.storage.k8s.io \\\\\\\"\\\\\\\" not found\\\"\""), nil, nil), initialClaims: newClaimArray("claim7-3", "pvc-uid7-3", "1Gi", "volume7-3", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume7-3", "pv-uid7-3", "pv-handle7-3", "1Gi", "pvc-uid7-3", "claim7-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialStorageClasses: []*storage.StorageClass{diffDriverStorageClass}, @@ -252,7 +252,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 get input parameters to create snapshot snap7-4: \"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, @@ -263,7 +263,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 get input parameters to create snapshot snap7-5: \"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, @@ -274,7 +274,7 @@ func TestCreateSnapshotSync(t *testing.T) { initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap7-6", classGold, "", "snapuid7-6", "claim7-6", false, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-6", classGold, "", "snapuid7-6", "claim7-6", false, newVolumeError("Failed to create snapshot: the PVC claim7-6 is not yet bound to a PV, will not attempt to take a snapshot"), nil, nil), + expectedSnapshots: newSnapshotArray("snap7-6", classGold, "", "snapuid7-6", "claim7-6", false, 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\""), nil, nil), initialClaims: newClaimArray("claim7-6", "pvc-uid7-6", "1Gi", "", v1.ClaimPending, &classEmpty), expectedEvents: []string{"Warning SnapshotCreationFailed"}, errors: noerrors,