diff --git a/pkg/common-controller/framework_test.go b/pkg/common-controller/framework_test.go index 254092aa..67696411 100644 --- a/pkg/common-controller/framework_test.go +++ b/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" diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go index bbbc4bc2..ee05e061 100644 --- a/pkg/common-controller/snapshot_controller.go +++ b/pkg/common-controller/snapshot_controller.go @@ -453,25 +453,25 @@ 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 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)) - return err - } + } - // Update snapshot status with BoundVolumeSnapshotContentName - klog.V(5).Infof("syncUnreadySnapshot [%s]: trying to update snapshot status", utils.SnapshotKey(snapshot)) - if _, err = ctrl.updateSnapshotStatus(snapshot, content); err != nil { - // update snapshot status failed - ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err)) - return err - } + // 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 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)) + return err + } + + // Update snapshot status with BoundVolumeSnapshotContentName + klog.V(5).Infof("syncUnreadySnapshot [%s]: trying to update snapshot status", utils.SnapshotKey(snapshot)) + if _, err = ctrl.updateSnapshotStatus(snapshot, content); err != nil { + // update snapshot status failed + ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err)) + return err } return nil } diff --git a/pkg/common-controller/snapshot_update_test.go b/pkg/common-controller/snapshot_update_test.go index 77fbf564..3aa8f2ee 100644 --- a/pkg/common-controller/snapshot_update_test.go +++ b/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)