Skip to content

Commit 799fb95

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #61 from xing-yang/csi_snapshot_status
Call CreateSnapshot to check if snapshot is processed
2 parents 33190f0 + c01fb64 commit 799fb95

File tree

2 files changed

+71
-78
lines changed

2 files changed

+71
-78
lines changed

‎pkg/controller/snapshot_controller.go‎

Lines changed: 65 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -421,13 +421,65 @@ func (ctrl *csiSnapshotController) checkandBindSnapshotContent(snapshot *crdv1.V
421421
return nil
422422
}
423423

424+
func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotClass, *v1.PersistentVolume, string, map[string]string, error) {
425+
className := snapshot.Spec.VolumeSnapshotClassName
426+
glog.V(5).Infof("getCreateSnapshotInput [%s]: VolumeSnapshotClassName [%s]", snapshot.Name, *className)
427+
var class *crdv1.VolumeSnapshotClass
428+
var err error
429+
if className != nil {
430+
class, err = ctrl.GetSnapshotClass(*className)
431+
if err != nil {
432+
glog.Errorf("getCreateSnapshotInput failed to getClassFromVolumeSnapshot %s", err)
433+
return nil, nil, "", nil, err
434+
}
435+
} else {
436+
glog.Errorf("failed to getCreateSnapshotInput %s without a snapshot class", snapshot.Name)
437+
return nil, nil, "", nil, fmt.Errorf("failed to take snapshot %s without a snapshot class", snapshot.Name)
438+
}
439+
440+
volume, err := ctrl.getVolumeFromVolumeSnapshot(snapshot)
441+
if err != nil {
442+
glog.Errorf("getCreateSnapshotInput failed to get PersistentVolume object [%s]: Error: [%#v]", snapshot.Name, err)
443+
return nil, nil, "", nil, err
444+
}
445+
446+
// Create VolumeSnapshotContent name
447+
contentName := GetSnapshotContentNameForSnapshot(snapshot)
448+
449+
// Resolve snapshotting secret credentials.
450+
snapshotterSecretRef, err := GetSecretReference(class.Parameters, contentName, snapshot)
451+
if err != nil {
452+
return nil, nil, "", nil, err
453+
}
454+
snapshotterCredentials, err := GetCredentials(ctrl.client, snapshotterSecretRef)
455+
if err != nil {
456+
return nil, nil, "", nil, err
457+
}
458+
459+
return class, volume, contentName, snapshotterCredentials, nil
460+
}
461+
424462
func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshot, error) {
425-
status, _, size, err := ctrl.handler.GetSnapshotStatus(content)
463+
var err error
464+
var timestamp int64 = 0
465+
var size int64 = 0
466+
var readyToUse bool = false
467+
class, volume, _, snapshotterCredentials, err := ctrl.getCreateSnapshotInput(snapshot)
426468
if err != nil {
427-
return nil, fmt.Errorf("failed to check snapshot status %s with error %v", snapshot.Name, err)
469+
return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err)
428470
}
429-
timestamp := time.Now().UnixNano()
430-
newSnapshot, err := ctrl.updateSnapshotStatus(snapshot, status, timestamp, size, IsSnapshotBound(snapshot, content))
471+
driverName, snapshotID, timestamp, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
472+
if err != nil {
473+
glog.Errorf("checkandUpdateBoundSnapshotStatusOperation: failed to call create snapshot to check whether the snapshot is ready to use %q", err)
474+
return nil, err
475+
} else {
476+
glog.V(5).Infof("checkandUpdateBoundSnapshotStatusOperation: driver %s, snapshotId %s, timestamp %d, size %d, readyToUse %t", driverName, snapshotID, timestamp, size, readyToUse)
477+
}
478+
479+
if timestamp == 0 {
480+
timestamp = time.Now().UnixNano()
481+
}
482+
newSnapshot, err := ctrl.updateSnapshotStatus(snapshot, readyToUse, timestamp, size, IsSnapshotBound(snapshot, content))
431483
if err != nil {
432484
return nil, err
433485
}
@@ -451,51 +503,22 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
451503
return snapshot, nil
452504
}
453505

454-
className := snapshot.Spec.VolumeSnapshotClassName
455-
glog.V(5).Infof("createSnapshotOperation [%s]: VolumeSnapshotClassName [%s]", snapshot.Name, *className)
456-
var class *crdv1.VolumeSnapshotClass
457-
var err error
458-
if className != nil {
459-
class, err = ctrl.GetSnapshotClass(*className)
460-
if err != nil {
461-
glog.Errorf("createSnapshotOperation failed to getClassFromVolumeSnapshot %s", err)
462-
return nil, err
463-
}
464-
} else {
465-
glog.Errorf("failed to take snapshot %s without a snapshot class", snapshot.Name)
466-
return nil, fmt.Errorf("failed to take snapshot %s without a snapshot class", snapshot.Name)
467-
}
468-
469-
volume, err := ctrl.getVolumeFromVolumeSnapshot(snapshot)
506+
class, volume, contentName, snapshotterCredentials, err := ctrl.getCreateSnapshotInput(snapshot)
470507
if err != nil {
471-
glog.Errorf("createSnapshotOperation failed to get PersistentVolume object [%s]: Error: [%#v]", snapshot.Name, err)
472-
return nil, err
508+
return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err)
473509
}
474510

475-
// Create VolumeSnapshotContent name
476-
contentName := GetSnapshotContentNameForSnapshot(snapshot)
477-
478-
// Resolve snapshotting secret credentials.
479-
snapshotterSecretRef, err := GetSecretReference(class.Parameters, contentName, snapshot)
480-
if err != nil {
481-
return nil, err
482-
}
483-
snapshotterCredentials, err := GetCredentials(ctrl.client, snapshotterSecretRef)
484-
if err != nil {
485-
return nil, err
486-
}
487-
488-
driverName, snapshotID, timestamp, size, csiSnapshotStatus, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
511+
driverName, snapshotID, timestamp, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
489512
if err != nil {
490513
return nil, fmt.Errorf("failed to take snapshot of the volume, %s: %q", volume.Name, err)
491514
}
492-
glog.V(5).Infof("Created snapshot: driver %s, snapshotId %s, timestamp %d, size %d, csiSnapshotStatus %v", driverName, snapshotID, timestamp, size, csiSnapshotStatus)
515+
glog.V(5).Infof("Created snapshot: driver %s, snapshotId %s, timestamp %d, size %d, readyToUse %t", driverName, snapshotID, timestamp, size, readyToUse)
493516

494517
var newSnapshot *crdv1.VolumeSnapshot
495518
// Update snapshot status with timestamp
496519
for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ {
497520
glog.V(5).Infof("createSnapshot [%s]: trying to update snapshot creation timestamp", snapshotKey(snapshot))
498-
newSnapshot, err = ctrl.updateSnapshotStatus(snapshot, csiSnapshotStatus, timestamp, size, false)
521+
newSnapshot, err = ctrl.updateSnapshotStatus(snapshot, readyToUse, timestamp, size, false)
499522
if err == nil {
500523
break
501524
}
@@ -682,42 +705,12 @@ func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSn
682705
status.Error = nil
683706
change = true
684707
}
685-
if status.CreationTime == nil {
686-
status.CreationTime = timeAt
687-
change = true
688-
}
689708
}
690-
691-
/* TODO FIXME
692-
switch csistatus.Type {
693-
case csi.SnapshotStatus_READY:
694-
if bound {
695-
status.Ready = true
696-
// Remove the error if checking snapshot is already bound and ready
697-
status.Error = nil
698-
change = true
699-
}
700-
if status.CreationTime == nil {
701-
status.CreationTime = timeAt
702-
change = true
703-
}
704-
case csi.SnapshotStatus_ERROR_UPLOADING:
705-
if status.Error == nil {
706-
status.Error = &storage.VolumeError{
707-
Time: *timeAt,
708-
Message: "Failed to upload the snapshot",
709-
}
710-
change = true
711-
ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotUploadError", "Failed to upload the snapshot")
712-
713-
}
714-
case csi.SnapshotStatus_UPLOADING:
715-
if status.CreationTime == nil {
716-
status.CreationTime = timeAt
717-
change = true
718-
}
709+
if status.CreationTime == nil {
710+
status.CreationTime = timeAt
711+
change = true
719712
}
720-
*/
713+
721714
if change {
722715
if size > 0 {
723716
status.RestoreSize = resource.NewQuantity(size, resource.BinarySI)

‎pkg/controller/snapshot_create_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ func TestCreateSnapshotSync(t *testing.T) {
215215
initialContents: nocontents,
216216
expectedContents: nocontents,
217217
initialSnapshots: newSnapshotArray("snap7-1", classNonExisting, "", "snapuid7-1", "claim7-1", false, nil, nil, nil),
218-
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),
218+
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),
219219
initialClaims: newClaimArray("claim7-1", "pvc-uid7-1", "1Gi", "volume7-1", v1.ClaimBound, &classEmpty),
220220
initialVolumes: newVolumeArray("volume7-1", "pv-uid7-1", "pv-handle7-1", "1Gi", "pvc-uid7-1", "claim7-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
221221
expectedEvents: []string{"Warning SnapshotCreationFailed"},
@@ -227,7 +227,7 @@ func TestCreateSnapshotSync(t *testing.T) {
227227
initialContents: nocontents,
228228
expectedContents: nocontents,
229229
initialSnapshots: newSnapshotArray("snap7-2", invalidSecretClass, "", "snapuid7-2", "claim7-2", false, nil, nil, nil),
230-
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),
230+
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),
231231
initialClaims: newClaimArray("claim7-2", "pvc-uid7-2", "1Gi", "volume7-2", v1.ClaimBound, &classEmpty),
232232
initialVolumes: newVolumeArray("volume7-2", "pv-uid7-2", "pv-handle7-2", "1Gi", "pvc-uid7-2", "claim7-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
233233
expectedEvents: []string{"Warning SnapshotCreationFailed"},
@@ -239,7 +239,7 @@ func TestCreateSnapshotSync(t *testing.T) {
239239
initialContents: nocontents,
240240
expectedContents: nocontents,
241241
initialSnapshots: newSnapshotArray("snap7-3", "", "", "snapuid7-3", "claim7-3", false, nil, nil, nil),
242-
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),
242+
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),
243243
initialClaims: newClaimArray("claim7-3", "pvc-uid7-3", "1Gi", "volume7-3", v1.ClaimBound, &classEmpty),
244244
initialVolumes: newVolumeArray("volume7-3", "pv-uid7-3", "pv-handle7-3", "1Gi", "pvc-uid7-3", "claim7-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
245245
initialStorageClasses: []*storage.StorageClass{diffDriverStorageClass},
@@ -252,7 +252,7 @@ func TestCreateSnapshotSync(t *testing.T) {
252252
initialContents: nocontents,
253253
expectedContents: nocontents,
254254
initialSnapshots: newSnapshotArray("snap7-4", classGold, "", "snapuid7-4", "claim7-4", false, nil, nil, nil),
255-
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),
255+
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),
256256
initialVolumes: newVolumeArray("volume7-4", "pv-uid7-4", "pv-handle7-4", "1Gi", "pvc-uid7-4", "claim7-4", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
257257
expectedEvents: []string{"Warning SnapshotCreationFailed"},
258258
errors: noerrors,
@@ -263,7 +263,7 @@ func TestCreateSnapshotSync(t *testing.T) {
263263
initialContents: nocontents,
264264
expectedContents: nocontents,
265265
initialSnapshots: newSnapshotArray("snap7-5", classGold, "", "snapuid7-5", "claim7-5", false, nil, nil, nil),
266-
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),
266+
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),
267267
initialClaims: newClaimArray("claim7-5", "pvc-uid7-5", "1Gi", "volume7-5", v1.ClaimBound, &classEmpty),
268268
expectedEvents: []string{"Warning SnapshotCreationFailed"},
269269
errors: noerrors,
@@ -274,7 +274,7 @@ func TestCreateSnapshotSync(t *testing.T) {
274274
initialContents: nocontents,
275275
expectedContents: nocontents,
276276
initialSnapshots: newSnapshotArray("snap7-6", classGold, "", "snapuid7-6", "claim7-6", false, nil, nil, nil),
277-
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),
277+
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),
278278
initialClaims: newClaimArray("claim7-6", "pvc-uid7-6", "1Gi", "", v1.ClaimPending, &classEmpty),
279279
expectedEvents: []string{"Warning SnapshotCreationFailed"},
280280
errors: noerrors,

0 commit comments

Comments
 (0)