Call dynamic VS content creation unconditionally

Irrespective of any error on the Volume Snapshot
object, initiate dynamic VolumeSnapshotContent object
creation. If any required parameters are not found, (e.g. missing
VS class), the VS content object creation would fail gracefully.

pkg/common-controller/framework_test.go

@@ -1145,6 +1145,14 @@ func testUpdateSnapshotClass(ctrl *csiSnapshotCommonController, reactor *snapsho
 	return err
 }
 
+func testNewSnapshotContentCreation(ctrl *csiSnapshotCommonController, reactor *snapshotReactor, test controllerTest) error {
+	if err := ctrl.syncUnreadySnapshot(test.initialSnapshots[0]); err != nil {
+		return fmt.Errorf("syncUnreadySnapshot failed: %v", err)
+	}
+
+	return nil
+}
+
 var (
 	classEmpty         string
 	classGold          = "gold"

pkg/common-controller/snapshot_controller.go

@@ -453,12 +453,13 @@ func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.Vol
 		}
 		klog.V(5).Infof("bindandUpdateVolumeSnapshot %v", newSnapshot)
 		return nil
-	} else if snapshot.Status == nil || snapshot.Status.Error == nil || isControllerUpdateFailError(snapshot.Status.Error) {
+	}
+
+	// If we reach here, it is a dynamically provisioned snapshot, and the volumeSnapshotContent object is not yet created.
 	if snapshot.Spec.Source.PersistentVolumeClaimName == nil {
 		ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotPVCSourceMissing", fmt.Sprintf("PVC source for snapshot %s is missing", uniqueSnapshotName))
 		return fmt.Errorf("expected PVC source for snapshot %s but got nil", uniqueSnapshotName)
 	}
-		var err error
 	var content *crdv1.VolumeSnapshotContent
 	if content, err = ctrl.createSnapshotContent(snapshot); err != nil {
 		ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentCreationFailed", fmt.Sprintf("Failed to create snapshot content with error %v", err))
@@ -472,7 +473,6 @@ func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.Vol
 		ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err))
 		return err
 	}
-	}
 	return nil
 }
 

pkg/common-controller/snapshot_update_test.go

@@ -510,6 +510,48 @@ func TestSync(t *testing.T) {
 			expectSuccess:     true,
 			test:              testUpdateSnapshotErrorStatus,
 		},
+		{
+			// Snapshot status nil, no initial content, new content should be created.
+			name:              "8-1 - Snapshot status nil, no initial snapshot content, new content should be created",
+			initialContents:   nocontents,
+			expectedContents:  withContentAnnotations(newContentArrayNoStatus("snapcontent-snapuid8-1", "snapuid8-1", "snap8-1", "sid8-1", validSecretClass, "", "pv-handle8-1", deletionPolicy, nil, nil, false, false), map[string]string{utils.AnnDeletionSecretRefName: "secret", utils.AnnDeletionSecretRefNamespace: "default"}),
+			initialSnapshots:  newSnapshotArray("snap8-1", "snapuid8-1", "claim8-1", "", validSecretClass, "", nil, nil, nil, nil, true, false, nil),
+			expectedSnapshots: newSnapshotArray("snap8-1", "snapuid8-1", "claim8-1", "", validSecretClass, "snapcontent-snapuid8-1", &False, nil, nil, nil, false, false, nil),
+			initialClaims:     newClaimArray("claim8-1", "pvc-uid8-1", "1Gi", "volume8-1", v1.ClaimBound, &classEmpty),
+			initialVolumes:    newVolumeArray("volume8-1", "pv-uid8-1", "pv-handle8-1", "1Gi", "pvc-uid8-1", "claim8-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
+			initialSecrets:    []*v1.Secret{secret()},
+			errors:            noerrors,
+			expectSuccess:     true,
+			test:              testNewSnapshotContentCreation,
+		},
+		{
+			// Snapshot status with nil error, no initial content, new content should be created.
+			name:              "8-2 - Snapshot status with nil error, no initial snapshot content, new content should be created",
+			initialContents:   nocontents,
+			expectedContents:  withContentAnnotations(newContentArrayNoStatus("snapcontent-snapuid8-2", "snapuid8-2", "snap8-2", "sid8-2", validSecretClass, "", "pv-handle8-2", deletionPolicy, nil, nil, false, false), map[string]string{utils.AnnDeletionSecretRefName: "secret", utils.AnnDeletionSecretRefNamespace: "default"}),
+			initialSnapshots:  newSnapshotArray("snap8-2", "snapuid8-2", "claim8-2", "", validSecretClass, "", nil, nil, nil, nil, false, false, nil),
+			expectedSnapshots: newSnapshotArray("snap8-2", "snapuid8-2", "claim8-2", "", validSecretClass, "snapcontent-snapuid8-2", &False, nil, nil, nil, false, false, nil),
+			initialClaims:     newClaimArray("claim8-2", "pvc-uid8-2", "1Gi", "volume8-2", v1.ClaimBound, &classEmpty),
+			initialVolumes:    newVolumeArray("volume8-2", "pv-uid8-2", "pv-handle8-2", "1Gi", "pvc-uid8-2", "claim8-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
+			initialSecrets:    []*v1.Secret{secret()},
+			errors:            noerrors,
+			expectSuccess:     true,
+			test:              testNewSnapshotContentCreation,
+		},
+		{
+			// Snapshot status with error, no initial content, new content should be created, snapshot error should be cleared.
+			name:              "8-3 - Snapshot status with error, no initial content, new content should be created, snapshot error should be cleared",
+			initialContents:   nocontents,
+			expectedContents:  withContentAnnotations(newContentArrayNoStatus("snapcontent-snapuid8-3", "snapuid8-3", "snap8-3", "sid8-3", validSecretClass, "", "pv-handle8-3", deletionPolicy, nil, nil, false, false), map[string]string{utils.AnnDeletionSecretRefName: "secret", utils.AnnDeletionSecretRefNamespace: "default"}),
+			initialSnapshots:  newSnapshotArray("snap8-3", "snapuid8-3", "claim8-3", "", validSecretClass, "", nil, nil, nil, snapshotErr, false, false, nil),
+			expectedSnapshots: newSnapshotArray("snap8-3", "snapuid8-3", "claim8-3", "", validSecretClass, "snapcontent-snapuid8-3", &False, nil, nil, nil, false, false, nil),
+			initialClaims:     newClaimArray("claim8-3", "pvc-uid8-3", "1Gi", "volume8-3", v1.ClaimBound, &classEmpty),
+			initialVolumes:    newVolumeArray("volume8-3", "pv-uid8-3", "pv-handle8-3", "1Gi", "pvc-uid8-3", "claim8-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
+			initialSecrets:    []*v1.Secret{secret()},
+			errors:            noerrors,
+			expectSuccess:     true,
+			test:              testNewSnapshotContentCreation,
+		},
 	}
 
 	runSyncTests(t, tests, snapshotClasses)