Repository: kubernetes-csi/external-snapshotter
Pull request #218 (2019-12-20 17:41:53)

Fix err shadow in updateSnapshotStatus and add retry delay
Signed-off-by: Grant Griffiths <grant@portworx.com>

<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
3. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
/kind bug

**What this PR does / why we need it**:
The err for updateSnapshotStatus was being defined locally (shadowed)

**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

**Special notes for your reviewer**:

**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note
NONE
```


k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-csi/external-snapshotter/pull/218#" title="Author self-approved">ggriffiths</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **jingxu97**
You can assign the PR to them by writing `/assign @jingxu97` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-csi%2Fexternal-snapshotter).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-csi/external-snapshotter/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["jingxu97"]} -->
ggriffiths: /assign @xing-yang 

All other calls to updateSnapshotStatus look good.

Patch:
diff --git a/README.md b/README.md
index 9985fb37e..7d83293c7 100644
--- a/README.md
+++ b/README.md
@@ -1,151 +1,72 @@
-[![Build Status](https://travis-ci.org/kubernetes-csi/external-snapshotter.svg?branch=master)](https://travis-ci.org/kubernetes-csi/external-snapshotter)
-
 # CSI Snapshotter
 
-The CSI snapshotter is part of Kubernetes implementation of [Container Storage Interface (CSI)](https://github.com/container-storage-interface/spec).
-
-The volume snapshot feature supports CSI v1.0 and higher. It was introduced as an Alpha feature in Kubernetes v1.12 and has been promoted to an Beta feature in Kubernetes 1.17.
+The CSI external-snapshotter is part of Kubernetes implementation of [Container Storage Interface (CSI)](https://github.com/container-storage-interface/spec).
 
+The volume snapshot feature supports CSI v1.0 and it has been an Alpha feature in Kubernetes since v1.12.
 
 ## Overview
 
-With the promotion of Volume Snapshot to beta, the feature is now enabled by default on standard Kubernetes deployments instead of being opt-in.
-
-The move of the Kubernetes Volume Snapshot feature to beta also means:
-* A revamp of volume snapshot APIs.
-* The CSI external-snapshotter sidecar is split into two controllers, a snapshot controller and a CSI external-snapshotter sidecar.
-
-The snapshot controller is deployed by the Kubernetes distributions and is responsible for watching the VolumeSnapshot CRD objects and manges the creation and deletion lifecycle of snapshots.
-
-The CSI external-snapshotter sidecar watches Kubernetes VolumeSnapshotContent CRD objects and triggers CreateSnapshot/DeleteSnapshot against a CSI endpoint.
-
-Blog post for the beta feature can be found [here](https://kubernetes.io/blog/2019/12/09/kubernetes-1-17-feature-cis-volume-snapshot-beta)
-
-
-## Compatibility
-
-This information reflects the head of this branch.
-
-| Compatible with CSI Version                                                                | Container Image             | Min K8s Version | Snapshot CRD version |
-| ------------------------------------------------------------------------------------------ | ----------------------------| --------------- | -------------------- |
-| [CSI Spec v1.0.0](https://github.com/container-storage-interface/spec/releases/tag/v1.0.0) | quay.io/k8scsi/csi-snapshotter | 1.17         | v1beta1              |
-| [CSI Spec v1.0.0](https://github.com/container-storage-interface/spec/releases/tag/v1.0.0) | quay.io/k8scsi/snapshot-controller | 1.17     | v1beta1              |
-
-
-## Feature Status
-
-The `VolumeSnapshotDataSource` feature gate was introduced in Kubernetes 1.12 and it is enabled by default in Kubernetes 1.17 when the volume snapshot feature is promoted to beta.
-
+CSI Snapshotter is an external controller that watches Kubernetes Snapshot CRD objects and triggers CreateSnapshot/DeleteSnapshot against a CSI endpoint. Full design can be found at Kubernetes proposal at [here](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/storage/csi-snapshot.md)
 
 ## Design
 
-Both the snapshot controller and CSI external-snapshotter sidecar follow [controller](https://github.com/kubernetes/community/blob/master/contributors/devel/controllers.md) pattern and uses informers to watch for events. The snapshot controller watches for `VolumeSnapshot` and `VolumeSnapshotContent` create/update/delete events.
-
-The CSI external-snapshotter sidecar only watches for `VolumeSnapshotContent` create/update/delete events. It filters out these objects with `Driver==<CSI driver name>` specified in the associated VolumeSnapshotClass object and then processes these events in workqueues with exponential backoff.
-
-The CSI external-snapshotter sidecar talks to CSI over socket (/run/csi/socket by default, configurable by -csi-address).
-
-### Hightlights in the snapshot v1beta1 APIs
-
-* DeletionPolicy is a required field in both VolumeSnapshotClass and VolumeSnapshotContent. This way the user has to explicitly specify it, leaving no room for confusion.
-* VolumeSnapshotSpec has a required Source field. Source may be either a PersistentVolumeClaimName (if dynamically provisioning a snapshot) or VolumeSnapshotContentName (if pre-provisioning a snapshot).
-* VolumeSnapshotContentSpec has a required Source field. This Source may be either a VolumeHandle (if dynamically provisioning a snapshot) or a SnapshotHandle (if pre-provisioning volume snapshots).
-* VolumeSnapshot contains a Status to indicate the current state of the volume snapshot. It has a field BoundVolumeSnapshotContentName to indicate the VolumeSnapshot object is bound to a VolumeSnapshotContent.
-* VolumeSnapshotContent contains a Status to indicate the current state of the volume snapshot content. It has a field SnapshotHandle to indicate that the VolumeSnapshotContent represents a snapshot on the storage system.
-
-
-## Usage
-
-The Volume Snapshot feature now depends on a new, volume snapshot controller in addition to the volume snapshot CRDs. Both the volume snapshot controller and the CRDs are independent of any CSI driver. Regardless of the number CSI drivers deployed on the cluster, there must be only one instance of the volume snapshot controller running and one set of volume snapshot CRDs installed per cluster.
-
-Therefore, it is strongly recommended that Kubernetes distributors bundle and deploy the controller and CRDs as part of their Kubernetes cluster management process (independent of any CSI Driver).
+External snapshotter follows [controller](https://github.com/kubernetes/community/blob/master/contributors/devel/controllers.md) pattern and uses informers to watch for `VolumeSnapshot` and `VolumeSnapshotContent` create/update/delete events. It filters out these objects with `Snapshotter==<CSI driver name>` specified in the associated VolumeSnapshotClass object and then processes these events in workqueues with exponential backoff.
 
-If your cluster does not come pre-installed with the correct components, you may manually install these components by executing the following steps.
+### Snapshotter
 
-Install Snapshot Beta CRDs:
-* kubectl create -f config/crd
-* https://github.com/kubernetes-csi/external-snapshotter/tree/master/config/crd
-* Do this once per cluster
+Snapshotter talks to CSI over socket (/run/csi/socket by default, configurable by -csi-address). The snapshotter then:
 
-Install Common Snapshot Controller:
-* kubectl create -f deploy/kubernetes/snapshot-controller
-* https://github.com/kubernetes-csi/external-snapshotter/tree/master/deploy/kubernetes/snapshot-controller
-* Do this once per cluster
+* Discovers the supported snapshotter name by `GetDriverName` call.
 
-Install CSI Driver:
-* Follow instructions provided by your CSI Driver vendor.
-* Here is an example to install the sample hostpath CSI driver
-  * kubectl create -f deploy/kubernetes/csi-snapshotter
-  * https://github.com/kubernetes-csi/external-snapshotter/tree/master/deploy/kubernetes/csi-snapshotter
+* Uses ControllerGetCapabilities for find out if CSI driver supports `ControllerServiceCapability_RPC_CREATE_DELETE_SNAPSHOT` and `ControllerServiceCapability_RPC_LIST_SNAPSHOTS` calls. Otherwise, the controller will not start.
 
-### Snapshot controller command line options
+* Processes new/updated/deleted `VolumeSnapshots`: The snapshotter only processes `VolumeSnapshot` that has `snapshotter` specified in its `VolumeSnapshotClass` matches its driver name. The process workflow is as follows
+  * If the snapshot status is `Ready`, the controller checks whether the snapshot and its content still binds correctly. If there is any problem with the binding (e.g., snapshot points to a non-exist snapshot content), update the snapshot status and emit event.
+  * If the snapshot status is not ready, there are two cases.
+    * `SnapshotContentName` is not empty: the controller verifies whether the snapshot content exists and also binds to the snapshot. If verification passes, the controller binds the snapshot and its content objects and marks it is ready. Otherwise, it updates the error status of the snapshot.
+    * `SnapshotContentName` is set empty: the controller will first check whether there is already a content object which binds the snapshot correctly with snapshot uid (`VolumeSnapshotRef.UID`) specified. If so, the controller binds these two objects. Otherwise, the controller issues a create snapshot operation. Please note that if the error status shows that snapshot creation already failed before, it will not try to create snapshot again.
 
-#### Important optional arguments that are highly recommended to be used
-* `--leader-election`: Enables leader election. This is useful when there are multiple replicas of the same snapshot controller running for the same Kubernetes deployment. Only one of them may be active (=leader). A new leader will be re-elected when current leader dies or becomes unresponsive for ~15 seconds.
+* Processes new/updated/deleted `VolumeSnapshotContents`: The snapshotter only processes `VolumeSnapshotContent` in which the CSI driver specified in the spec matches the controller's driver name.
+  * If the `VolumeSnapshotRef` is set to nil, skip this content since it is not bound to any snapshot object.
+  * Otherwise, the controller verifies whether the content object is correctly bound to a snapshot object. In case the `VolumeSnapshotRef.UID` is set but it does not match its snapshot object or snapshot no long exists, the content object and its associated snapshot will be deleted.
 
-* `--leader-election-namespace <namespace>`: The namespace where the leader election resource exists. Defaults to the pod namespace if not set.
-
-#### Other recognized arguments
-* `--kubeconfig <path>`: Path to Kubernetes client configuration that the snapshot controller uses to connect to Kubernetes API server. When omitted, default token provided by Kubernetes will be used. This option is useful only when the snapshot controller does not run as a Kubernetes pod, e.g. for debugging.
-
-* `--resync-period <duration>`: Internal resync interval when the snapshot controller re-evaluates all existing `VolumeSnapshot` instances and tries to fulfill them, i.e. create / delete corresponding snapshots. It does not affect re-tries of failed calls! It should be used only when there is a bug in Kubernetes watch logic. Default is 60 seconds.
-
-* `--version`: Prints current snapshot controller version and quits.
-
-* All glog / klog arguments are supported, such as `-v <log level>` or `-alsologtostderr`.
-
-### CSI external snapshotter sidecar command line options
-
-#### Important optional arguments that are highly recommended to be used
-* `--csi-address <path to CSI socket>`: This is the path to the CSI driver socket inside the pod that the external-snapshotter container will use to issue CSI operations (`/run/csi/socket` is used by default).
-
-* `--leader-election`: Enables leader election. This is useful when there are multiple replicas of the same external-snapshotter running for one CSI driver. Only one of them may be active (=leader). A new leader will be re-elected when current leader dies or becomes unresponsive for ~15 seconds.
-
-* `--leader-election-namespace <namespace>`: The namespace where the leader election resource exists. Defaults to the pod namespace if not set.
-
-* `--timeout <duration>`: Timeout of all calls to CSI driver. It should be set to value that accommodates majority of `CreateSnapshot`, `DeleteSnapshot`, and `ListSnapshots` calls. 1 minute is used by default.
-
-* `snapshot-name-prefix`: Prefix to apply to the name of a created snapshot. Default is `snapshot`.
-
-* `snapshot-name-uuid-length`: Length in characters for the generated uuid of a created snapshot. Defaults behavior is to NOT truncate.
-
-#### Other recognized arguments
-* `--kubeconfig <path>`: Path to Kubernetes client configuration that the CSI external-snapshotter uses to connect to Kubernetes API server. When omitted, default token provided by Kubernetes will be used. This option is useful only when the external-snapshotter does not run as a Kubernetes pod, e.g. for debugging.
+## Usage
 
-* `--resync-period <duration>`: Internal resync interval when the CSI external-snapshotter re-evaluates all existing `VolumeSnapshotContent` instances and tries to fulfill them, i.e. update / delete corresponding snapshots. It does not affect re-tries of failed CSI calls! It should be used only when there is a bug in Kubernetes watch logic. Default is 60 seconds.
+### Running on command line
 
-* `--version`: Prints current CSI external-snapshotter version and quits.
+For debugging, it is possible to run snapshotter on command line. For example,
 
-* All glog / klog arguments are supported, such as `-v <log level>` or `-alsologtostderr`.
+```bash
+csi-snapshotter -kubeconfig ~/.kube/config -v 5 -csi-address /run/csi/socket
+```
 
+### Running in a statefulset
 
-## Upgrade from v1alpha1 to v1beta1
+It is necessary to create a new service account and give it enough privileges to run the snapshotter. We provide .yaml files that deploy for use together with the hostpath example driver. A real production deployment must customize them:
 
-The change from v1alpha1 to v1beta1 snapshot APIs is not backward compatible.
+```bash
+for i in $(find deploy/kubernetes -name '*.yaml'); do kubectl create -f $i; done
+```
 
-If you have already deployed v1alpha1 snapshot APIs and external-snapshotter sidecar controller and want to upgrade to v1beta1, you need to do the following:
-* <strong>Note: The underlying snapshots on the storage system will be deleted in the upgrade process!!!</strong>
-1. Delete volume snapshots created using v1alpha1 snapshot CRDs and external-snapshotter sidecar controller.
-2. Uninstall v1alpha1 snapshot CRDs, external-snapshotter sidecar controller, and CSI driver.
-3. Install v1beta1 snapshot CRDs, snapshot controller, CSI external-snapshotter sidecar and CSI driver.
+### Running with Leader Election
 
+If you want to run external-snapshotter with higher availability, you can enable resource based leader election. To enable this, set the following flags:
+```bash
+--leader-election=true
+```
 
 ## Testing
 
 Running Unit Tests:
 
 ```bash
-go test -timeout 30s  github.com/kubernetes-csi/external-snapshotter/pkg/common-controller
-
-go test -timeout 30s  github.com/kubernetes-csi/external-snapshotter/pkg/sidecar-controller
+go test -timeout 30s  github.com/kubernetes-csi/external-snapshotter/pkg/controller
 ```
 
-
 ## Dependency Management
 
 external-snapshotter uses [go modules](https://blog.golang.org/using-go-modules).
 
-
 ## Community, discussion, contribution, and support
 
 Learn how to engage with the Kubernetes community on the [community page](http://kubernetes.io/community/).
diff --git a/cmd/snapshot-controller/main.go b/cmd/snapshot-controller/main.go
index cbe92f0fe..88d91a7aa 100644
--- a/cmd/snapshot-controller/main.go
+++ b/cmd/snapshot-controller/main.go
@@ -46,9 +46,11 @@ const (
 
 // Command line flags
 var (
-	kubeconfig   = flag.String("kubeconfig", "", "Absolute path to the kubeconfig file. Required only when running out of cluster.")
-	resyncPeriod = flag.Duration("resync-period", 60*time.Second, "Resync interval of the controller.")
-	showVersion  = flag.Bool("version", false, "Show version.")
+	kubeconfig                      = flag.String("kubeconfig", "", "Absolute path to the kubeconfig file. Required only when running out of cluster.")
+	createSnapshotContentRetryCount = flag.Int("create-snapshotcontent-retrycount", 5, "Number of retries when we create a snapshot content object for a snapshot.")
+	createSnapshotContentInterval   = flag.Duration("create-snapshotcontent-interval", 10*time.Second, "Interval between retries when we create a snapshot content object for a snapshot.")
+	resyncPeriod                    = flag.Duration("resync-period", 60*time.Second, "Resync interval of the controller.")
+	showVersion                     = flag.Bool("version", false, "Show version.")
 
 	leaderElection          = flag.Bool("leader-election", false, "Enables leader election.")
 	leaderElectionNamespace = flag.String("leader-election-namespace", "", "The namespace where the leader election resource exists. Defaults to the pod namespace if not set.")
@@ -94,7 +96,7 @@ func main() {
 	// Add Snapshot types to the defualt Kubernetes so events can be logged for them
 	snapshotscheme.AddToScheme(scheme.Scheme)
 
-	klog.V(2).Infof("Start NewCSISnapshotController with kubeconfig [%s] resyncPeriod [%+v]", *kubeconfig, *resyncPeriod)
+	klog.V(2).Infof("Start NewCSISnapshotController with kubeconfig [%s] createSnapshotContentRetryCount [%d] createSnapshotContentInterval [%d] resyncPeriod [%+v]", *kubeconfig, *createSnapshotContentRetryCount, *createSnapshotContentInterval, *resyncPeriod)
 
 	ctrl := controller.NewCSISnapshotCommonController(
 		snapClient,
@@ -103,6 +105,8 @@ func main() {
 		factory.Snapshot().V1beta1().VolumeSnapshotContents(),
 		factory.Snapshot().V1beta1().VolumeSnapshotClasses(),
 		coreFactory.Core().V1().PersistentVolumeClaims(),
+		*createSnapshotContentRetryCount,
+		*createSnapshotContentInterval,
 		*resyncPeriod,
 	)
 
diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go
index 70e059ce8..80a41914f 100644
--- a/pkg/common-controller/snapshot_controller.go
+++ b/pkg/common-controller/snapshot_controller.go
@@ -344,6 +344,7 @@ func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.Vol
 				break
 			}
 			klog.V(4).Infof("failed to update snapshot %s status: %v", utils.SnapshotKey(snapshot), err)
+			time.Sleep(ctrl.createSnapshotContentInterval)
 		}
 
 		if err != nil {
@@ -406,6 +407,7 @@ func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.Vol
 						break
 					}
 					klog.V(4).Infof("failed to update snapshot %s status: %v", utils.SnapshotKey(snapshot), err)
+					time.Sleep(ctrl.createSnapshotContentInterval)
 				}
 
 				if err != nil {
@@ -865,16 +867,17 @@ func (ctrl *csiSnapshotCommonController) bindandUpdateVolumeSnapshot(snapshotCon
 
 	// Copy the snapshot object before updating it
 	snapshotCopy := snapshotObj.DeepCopy()
-
 	// update snapshot status
+	var updateSnapshot *crdv1.VolumeSnapshot
 	for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ {
 		klog.V(5).Infof("bindandUpdateVolumeSnapshot [%s]: trying to update snapshot status", utils.SnapshotKey(snapshotCopy))
-		updateSnapshot, err := ctrl.updateSnapshotStatus(snapshotCopy, snapshotContent)
+		updateSnapshot, err = ctrl.updateSnapshotStatus(snapshotCopy, snapshotContent)
 		if err == nil {
 			snapshotCopy = updateSnapshot
 			break
 		}
 		klog.V(4).Infof("failed to update snapshot %s status: %v", utils.SnapshotKey(snapshot), err)
+		time.Sleep(ctrl.createSnapshotContentInterval)
 	}
 
 	if err != nil {
diff --git a/pkg/common-controller/snapshot_controller_base.go b/pkg/common-controller/snapshot_controller_base.go
index 2518dad85..958f03fae 100644
--- a/pkg/common-controller/snapshot_controller_base.go
+++ b/pkg/common-controller/snapshot_controller_base.go
@@ -42,12 +42,6 @@ import (
 	"k8s.io/kubernetes/pkg/util/goroutinemap"
 )
 
-// Number of retries when we create a VolumeSnapshotContent object
-const createSnapshotContentRetryCount = 5
-
-// Interval between retries when we create a VolumeSnapshotContent object
-const createSnapshotContentInterval = 10 * time.Second
-
 type csiSnapshotCommonController struct {
 	clientset     clientset.Interface
 	client        kubernetes.Interface
@@ -83,6 +77,8 @@ func NewCSISnapshotCommonController(
 	volumeSnapshotContentInformer storageinformers.VolumeSnapshotContentInformer,
 	volumeSnapshotClassInformer storageinformers.VolumeSnapshotClassInformer,
 	pvcInformer coreinformers.PersistentVolumeClaimInformer,
+	createSnapshotContentRetryCount int,
+	createSnapshotContentInterval time.Duration,
 	resyncPeriod time.Duration,
 ) *csiSnapshotCommonController {
 	broadcaster := record.NewBroadcaster()
diff --git a/pkg/sidecar-controller/snapshot_controller.go b/pkg/sidecar-controller/snapshot_controller.go
index 87c1d0ab8..54574a9bf 100644
--- a/pkg/sidecar-controller/snapshot_controller.go
+++ b/pkg/sidecar-controller/snapshot_controller.go
@@ -23,7 +23,7 @@ import (
 
 	crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1"
 	"github.com/kubernetes-csi/external-snapshotter/pkg/utils"
-	v1 "k8s.io/api/core/v1"
+	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/util/goroutinemap"
@@ -34,20 +34,20 @@ import (
 // Design:
 //
 // This is the sidecar controller that is responsible for creating and deleting a
-// snapshot on the storage system through a csi volume driver. It watches
-// VolumeSnapshotContent objects which have been either created/deleted by the
+// snapshot on the storage infrastructure through a csi volume driver. It watches
+// the VolumeSnapshotContent object which is either created/deleted by the
 // common snapshot controller in the case of dynamic provisioning or by the admin
 // in the case of pre-provisioned snapshots.
 
-// The snapshot creation through csi driver should return a snapshot after
-// it is created successfully(however, the snapshot might not be ready to use yet
-// if there is an uploading phase). The creationTime will be updated accordingly
+// The snapshot creation through csi volume driver should return a snapshot after
+// it is created successfully (however, the snapshot might not be ready to use yet if
+// there is an uploading phase). The creationTime will be updated accordingly
 // on the status of VolumeSnapshotContent.
 // After that, the sidecar controller will keep checking the snapshot status
 // through csi snapshot calls. When the snapshot is ready to use, the sidecar
 // controller set the status "ReadyToUse" to true on the VolumeSnapshotContent object
-// to indicate the snapshot is ready to be used to restore a volume.
-// If the creation failed for any reason, the Error status is set accordingly.
+// to indicate the snapshot is ready to use. If the creation failed for any reason,
+// the Error status is set accordingly.
 
 const controllerUpdateFailMsg = "snapshot controller failed to update"
 
@@ -57,50 +57,73 @@ func (ctrl *csiSnapshotSideCarController) syncContent(content *crdv1.VolumeSnaps
 
 	var err error
 	if ctrl.shouldDelete(content) {
-		klog.V(4).Infof("VolumeSnapshotContent[%s]: the policy is %s", content.Name, content.Spec.DeletionPolicy)
-		if content.Spec.DeletionPolicy == crdv1.VolumeSnapshotContentDelete &&
-			content.Status != nil && content.Status.SnapshotHandle != nil {
-			// issue a CSI deletion call if the snapshot has not been deleted yet from
-			// underlying storage system. Note that the deletion snapshot operation will
-			// update content SnapshotHandle to nil upon a successful deletion. At this
-			// point, the finalizer on content should NOT be removed to avoid leaking.
-			return ctrl.deleteCSISnapshot(content)
-		} else {
-			// otherwise, either the snapshot has been deleted from the underlying
-			// storage system, or the deletion policy is Retain, remove the finalizer
-			// if there is one so that API server could delete the object if there is
-			// no other finalizer.
-			return ctrl.removeContentFinalizer(content)
+		switch content.Spec.DeletionPolicy {
+		case crdv1.VolumeSnapshotContentRetain:
+			klog.V(4).Infof("VolumeSnapshotContent[%s]: policy is Retain. Keep physical snapshot and remove content finalizer", content.Name)
+			// It is a deletion candidate if DeletionTimestamp is not nil and
+			// VolumeSnapshotContentFinalizer is set.
+			if utils.IsContentDeletionCandidate(content) {
+				// Volume snapshot content is a deletion candidate.
+				// Remove the content finalizer.
+				klog.V(5).Infof("syncContent: Content [%s] is a deletion candidate. Remove finalizer.", content.Name)
+				return ctrl.removeContentFinalizer(content)
+			}
+
+		case crdv1.VolumeSnapshotContentDelete:
+			klog.V(4).Infof("VolumeSnapshotContent[%s]: policy is Delete. Delete physical snapshot", content.Name)
+			err = ctrl.deleteCSISnapshot(content)
+			if err != nil {
+				return err
+			}
+			klog.V(5).Infof("syncContent: check if we should remove Finalizer for VolumeSnapshotContent[%s]", content.Name)
+			// It is a deletion candidate if DeletionTimestamp is not nil and
+			// VolumeSnapshotContentFinalizer is set.
+			if utils.IsContentDeletionCandidate(content) {
+				// Volume snapshot content is a deletion candidate.
+				// Remove the content finalizer.
+				klog.V(5).Infof("syncContent: Content [%s] is a deletion candidate. Remove finalizer.", content.Name)
+				return ctrl.removeContentFinalizer(content)
+			}
+
+		default:
+			// Unknown VolumeSnapshotDeletionPolicy
+			ctrl.eventRecorder.Event(content, v1.EventTypeWarning, "SnapshotUnknownDeletionPolicy", "Volume Snapshot Content has unrecognized deletion policy")
 		}
+		klog.V(4).Infof("VolumeSnapshotContent[%s]: the policy is %s", content.Name, content.Spec.DeletionPolicy)
 	} else {
+		var err error
+		klog.V(5).Infof("syncContent: Call CreateSnapshot for content %s", content.Name)
 		if content.Spec.Source.VolumeHandle != nil && content.Status == nil {
-			klog.V(5).Infof("syncContent: Call CreateSnapshot for content %s", content.Name)
 			if err = ctrl.createSnapshot(content); err != nil {
 				ctrl.updateContentErrorStatusWithEvent(content, v1.EventTypeWarning, "SnapshotCreationFailed", fmt.Sprintf("Failed to create snapshot with error %v", err))
 				return err
 			}
 		} else {
+
 			if err = ctrl.checkandUpdateContentStatus(content); err != nil {
 				ctrl.updateContentErrorStatusWithEvent(content, v1.EventTypeWarning, "SnapshotContentStatusUpdateFailed", fmt.Sprintf("Failed to update snapshot content status with error %v", err))
 				return err
 			}
 		}
+
+		return nil
 	}
+
 	return nil
 }
 
 // deleteCSISnapshot starts delete action.
 func (ctrl *csiSnapshotSideCarController) deleteCSISnapshot(content *crdv1.VolumeSnapshotContent) error {
 	operationName := fmt.Sprintf("delete-%s", content.Name)
-	klog.V(5).Infof("schedule to delete snapshot, operation named %s", operationName)
+	klog.V(5).Infof("Snapshotter is about to delete volume snapshot content and the operation named %s", operationName)
 	ctrl.scheduleOperation(operationName, func() error {
 		return ctrl.deleteCSISnapshotOperation(content)
 	})
 	return nil
 }
 
-// scheduleOperation starts given asynchronous operation on given snapshot. It
-// makes sure there is no running operation with the same operationName
+// scheduleOperation starts given asynchronous operation on given volume. It
+// makes sure the operation is already not running.
 func (ctrl *csiSnapshotSideCarController) scheduleOperation(operationName string, operation func() error) {
 	klog.V(5).Infof("scheduleOperation[%s]", operationName)
 
@@ -176,17 +199,17 @@ func (ctrl *csiSnapshotSideCarController) updateContentErrorStatusWithEvent(cont
 	if content.Status != nil && content.Status.Error != nil && *content.Status.Error.Message == message {
 		klog.V(4).Infof("updateContentStatusWithEvent[%s]: the same error %v is already set", content.Name, content.Status.Error)
 		return nil
+	} else if content.Status == nil {
+		content.Status = &crdv1.VolumeSnapshotContentStatus{}
 	}
 	contentClone := content.DeepCopy()
-	if contentClone.Status == nil {
-		contentClone.Status = &crdv1.VolumeSnapshotContentStatus{}
-	}
-	contentClone.Status.Error = &crdv1.VolumeSnapshotError{
+	statusError := &crdv1.VolumeSnapshotError{
 		Time: &metav1.Time{
 			Time: time.Now(),
 		},
 		Message: &message,
 	}
+	contentClone.Status.Error = statusError
 	ready := false
 	contentClone.Status.ReadyToUse = &ready
 	newContent, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().UpdateStatus(contentClone)
@@ -210,7 +233,7 @@ func (ctrl *csiSnapshotSideCarController) updateContentErrorStatusWithEvent(cont
 
 func (ctrl *csiSnapshotSideCarController) getCSISnapshotInput(content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshotClass, map[string]string, error) {
 	className := content.Spec.VolumeSnapshotClassName
-	klog.V(5).Infof("getCSISnapshotInput for content [%s]", content.Name)
+	klog.V(5).Infof("getCSISnapshotInput for content [%s]: VolumeSnapshotClassName [%s]", content.Name, *className)
 	var class *crdv1.VolumeSnapshotClass
 	var err error
 	if className != nil {
@@ -335,7 +358,6 @@ func (ctrl *csiSnapshotSideCarController) deleteCSISnapshotOperation(content *cr
 
 	_, snapshotterCredentials, err := ctrl.getCSISnapshotInput(content)
 	if err != nil {
-		ctrl.eventRecorder.Event(content, v1.EventTypeWarning, "SnapshotDeleteError", "Failed to get snapshot class or credentials")
 		return fmt.Errorf("failed to get input parameters to delete snapshot for content %s: %q", content.Name, err)
 	}
 
@@ -344,40 +366,8 @@ func (ctrl *csiSnapshotSideCarController) deleteCSISnapshotOperation(content *cr
 		ctrl.eventRecorder.Event(content, v1.EventTypeWarning, "SnapshotDeleteError", "Failed to delete snapshot")
 		return fmt.Errorf("failed to delete snapshot %#v, err: %v", content.Name, err)
 	}
-	// the snapshot has been deleted from the underlying storage system, update
-	// content status to remove snapshot handle etc.
-	newContent, err := ctrl.clearVolumeContentStatus(content.Name)
-	if err != nil {
-		ctrl.eventRecorder.Event(content, v1.EventTypeWarning, "SnapshotDeleteError", "Failed to clear content status")
-		return err
-	}
-	// update local cache
-	ctrl.updateContentInCacheStore(newContent)
-	return nil
-}
 
-// clearVolumeContentStatus resets all fields to nil related to a snapshot in
-// content.Status. On success, the latest version of the content object will be
-// returned.
-func (ctrl *csiSnapshotSideCarController) clearVolumeContentStatus(
-	contentName string) (*crdv1.VolumeSnapshotContent, error) {
-	klog.V(5).Infof("cleanVolumeSnapshotStatus content [%s]", contentName)
-	// get the latest version from API server
-	content, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Get(contentName, metav1.GetOptions{})
-	if err != nil {
-		return nil, fmt.Errorf("error get snapshot content %s from api server: %v", contentName, err)
-	}
-	if content.Status != nil {
-		content.Status.SnapshotHandle = nil
-		content.Status.ReadyToUse = nil
-		content.Status.CreationTime = nil
-		content.Status.RestoreSize = nil
-	}
-	newContent, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().UpdateStatus(content)
-	if err != nil {
-		return nil, newControllerUpdateError(contentName, err.Error())
-	}
-	return newContent, nil
+	return nil
 }
 
 func (ctrl *csiSnapshotSideCarController) updateSnapshotContentStatus(
@@ -507,13 +497,8 @@ func (ctrl *csiSnapshotSideCarController) GetCredentialsFromAnnotation(content *
 	return snapshotterCredentials, nil
 }
 
-// removeContentFinalizer removes the VolumeSnapshotContentFinalizer from a
-// content if there exists one.
+// removeContentFinalizer removes a Finalizer for VolumeSnapshotContent.
 func (ctrl csiSnapshotSideCarController) removeContentFinalizer(content *crdv1.VolumeSnapshotContent) error {
-	if !slice.ContainsString(content.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer, nil) {
-		// the finalizer does not exit, return directly
-		return nil
-	}
 	contentClone := content.DeepCopy()
 	contentClone.ObjectMeta.Finalizers = slice.RemoveString(contentClone.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer, nil)
 
@@ -522,11 +507,12 @@ func (ctrl csiSnapshotSideCarController) removeContentFinalizer(content *crdv1.V
 		return newControllerUpdateError(content.Name, err.Error())
 	}
 
-	klog.V(5).Infof("Removed protection finalizer from volume snapshot content %s", content.Name)
 	_, err = ctrl.storeContentUpdate(contentClone)
 	if err != nil {
 		klog.Errorf("failed to update content store %v", err)
 	}
+
+	klog.V(5).Infof("Removed protection finalizer from volume snapshot content %s", content.Name)
 	return nil
 }
 
@@ -538,7 +524,7 @@ func (ctrl *csiSnapshotSideCarController) shouldDelete(content *crdv1.VolumeSnap
 	if content.ObjectMeta.DeletionTimestamp == nil {
 		return false
 	}
-	// 1) shouldDelete returns true if a content is not bound
+	// 1) shouldDelete returns true if content is not bound
 	// (VolumeSnapshotRef.UID == "") for pre-provisioned snapshot
 	if content.Spec.Source.SnapshotHandle != nil && content.Spec.VolumeSnapshotRef.UID == "" {
 		return true
diff --git a/pkg/utils/util.go b/pkg/utils/util.go
index 404902fcf..b246efc4b 100644
--- a/pkg/utils/util.go
+++ b/pkg/utils/util.go
@@ -20,12 +20,8 @@ import (
 	"fmt"
 	"strings"
 
-	"os"
-	"strconv"
-	"time"
-
 	crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1"
-	v1 "k8s.io/api/core/v1"
+	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -34,6 +30,9 @@ import (
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/util/slice"
+	"os"
+	"strconv"
+	"time"
 )
 
 var (
@@ -319,6 +318,13 @@ func NoResyncPeriodFunc() time.Duration {
 	return 0
 }
 
+// isContentDeletionCandidate checks if a volume snapshot content is a deletion candidate.
+// It is a deletion candidate if DeletionTimestamp is not nil and
+// VolumeSnapshotContentFinalizer is set.
+func IsContentDeletionCandidate(content *crdv1.VolumeSnapshotContent) bool {
+	return content.ObjectMeta.DeletionTimestamp != nil && slice.ContainsString(content.ObjectMeta.Finalizers, VolumeSnapshotContentFinalizer, nil)
+}
+
 // needToAddContentFinalizer checks if a Finalizer needs to be added for the volume snapshot content.
 func NeedToAddContentFinalizer(content *crdv1.VolumeSnapshotContent) bool {
 	return content.ObjectMeta.DeletionTimestamp == nil && !slice.ContainsString(content.ObjectMeta.Finalizers, VolumeSnapshotContentFinalizer, nil)


Test patch:
diff --git a/pkg/common-controller/framework_test.go b/pkg/common-controller/framework_test.go
index 1aaf51ee7..82531eae5 100644
--- a/pkg/common-controller/framework_test.go
+++ b/pkg/common-controller/framework_test.go
@@ -761,6 +761,8 @@ func newTestController(kubeClient kubernetes.Interface, clientset clientset.Inte
 		informerFactory.Snapshot().V1beta1().VolumeSnapshotContents(),
 		informerFactory.Snapshot().V1beta1().VolumeSnapshotClasses(),
 		coreFactory.Core().V1().PersistentVolumeClaims(),
+		3,
+		5*time.Millisecond,
 		60*time.Second,
 	)
 
diff --git a/pkg/sidecar-controller/content_create_test.go b/pkg/sidecar-controller/content_create_test.go
index c2dd23139..435fa44fc 100644
--- a/pkg/sidecar-controller/content_create_test.go
+++ b/pkg/sidecar-controller/content_create_test.go
@@ -25,13 +25,12 @@ func TestSyncContent(t *testing.T) {
 	var tests []controllerTest
 
 	tests = append(tests, controllerTest{
-		name:             "Basic content update ready to use",
-		initialContents:  newContentArrayWithReadyToUse("content1-1", "snapuid1-1", "snap1-1", "sid1-1", defaultClass, "", "volume-handle-1-1", retainPolicy, nil, &defaultSize, &False, true),
-		expectedContents: newContentArrayWithReadyToUse("content1-1", "snapuid1-1", "snap1-1", "sid1-1", defaultClass, "", "volume-handle-1-1", retainPolicy, nil, &defaultSize, &True, true),
+		name:             "Basic content create ready to use",
+		initialContents:  newContentArrayWithReadyToUse("content1-1", "snapuid1-1", "snap1-1", "sid1-1", defaultClass, "", "", retainPolicy, nil, &defaultSize, &False, true),
+		expectedContents: newContentArrayWithReadyToUse("content1-1", "snapuid1-1", "snap1-1", "sid1-1", defaultClass, "", "", retainPolicy, nil, &defaultSize, &True, true),
 		expectedEvents:   noevents,
 		expectedCreateCalls: []createCall{
 			{
-				volumeHandle: "volume-handle-1-1",
 				snapshotName: "snapshot-snapuid1-1",
 				driverName:   mockDriverName,
 				snapshotId:   "snapuid1-1",
diff --git a/pkg/sidecar-controller/framework_test.go b/pkg/sidecar-controller/framework_test.go
index 009fea6e8..73b8ea357 100644
--- a/pkg/sidecar-controller/framework_test.go
+++ b/pkg/sidecar-controller/framework_test.go
@@ -553,6 +553,10 @@ func newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHa
 		Spec: crdv1.VolumeSnapshotContentSpec{
 			Driver:         mockDriverName,
 			DeletionPolicy: deletionPolicy,
+			Source: crdv1.VolumeSnapshotContentSource{
+				SnapshotHandle: &snapshotHandle,
+				VolumeHandle:   &volumeHandle,
+			},
 		},
 		Status: &crdv1.VolumeSnapshotContentStatus{
 			CreationTime: creationTime,
diff --git a/pkg/sidecar-controller/snapshot_controller_test.go b/pkg/sidecar-controller/snapshot_controller_test.go
index c3303e3bc..c4d598e97 100644
--- a/pkg/sidecar-controller/snapshot_controller_test.go
+++ b/pkg/sidecar-controller/snapshot_controller_test.go
@@ -18,7 +18,6 @@ import (
 
 	crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1"
 	"github.com/kubernetes-csi/external-snapshotter/pkg/utils"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/cache"
 )
 
@@ -90,53 +89,3 @@ func TestControllerCacheParsingError(t *testing.T) {
 		t.Errorf("Expected parsing error, got nil instead")
 	}
 }
-
-// TestShouldDelete tests logic for deleting VolumeSnapshotContent objects.
-func TestShouldDelete(t *testing.T) {
-	// Use an empty controller, since there's no struct
-	// state we need to use in this test.
-	ctrl := &csiSnapshotSideCarController{}
-
-	tests := []struct {
-		name           string
-		expectedReturn bool
-		content        *crdv1.VolumeSnapshotContent
-	}{
-		{
-			name:           "DeletionTimeStamp is nil",
-			expectedReturn: false,
-			content:        newContent("test-content", "snap-uuid", "snapName", "desiredHandle", "default", "desiredHandle", "volHandle", crdv1.VolumeSnapshotContentDelete, nil, &defaultSize, false, nil),
-		},
-		{
-			name:           "Content is not bound",
-			expectedReturn: true,
-			content:        newContent("test-content-not-bound", "", "", "snapshotHandle", "", "", "", crdv1.VolumeSnapshotContentDelete, nil, &defaultSize, false, &timeNowMetav1),
-		},
-		{
-			name:           "AnnVolumeSnapshotBeingDeleted annotation is set. ",
-			expectedReturn: true,
-			// DeletionTime means that annotation is set, and being bound means the other cases are skipped.
-			content: newContent("test-content", "snap-uuid", "snapName", "desiredHandle", "default", "desiredHandle", "volHandle", crdv1.VolumeSnapshotContentDelete, nil, &defaultSize, false, &timeNowMetav1),
-		},
-		{
-			name:           "If no other cases match, then should not delete",
-			expectedReturn: false,
-			// Use an object that does not conform to newContent's logic in order to skip the conditionals inside shouldDelete
-			content: &crdv1.VolumeSnapshotContent{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:              "test-content",
-					DeletionTimestamp: &timeNowMetav1,
-				},
-			},
-		},
-	}
-
-	for _, test := range tests {
-		result := ctrl.shouldDelete(test.content)
-
-		if result != test.expectedReturn {
-			t.Errorf("Got %t but expected %t for test: %s", result, test.expectedReturn, test.name)
-		}
-
-	}
-}
diff --git a/pkg/sidecar-controller/snapshot_delete_test.go b/pkg/sidecar-controller/snapshot_delete_test.go
index 8a8bb607d..593b71c2a 100644
--- a/pkg/sidecar-controller/snapshot_delete_test.go
+++ b/pkg/sidecar-controller/snapshot_delete_test.go
@@ -25,7 +25,7 @@ import (
 
 	crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1"
 	"github.com/kubernetes-csi/external-snapshotter/pkg/utils"
-	v1 "k8s.io/api/core/v1"
+	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -140,7 +140,7 @@ func TestDeleteSync(t *testing.T) {
 		{
 			name:             "1-1 - content non-nil DeletionTimestamp with delete policy will delete snapshot",
 			initialContents:  newContentArrayWithDeletionTimestamp("content1-1", "snapuid1-1", "snap1-1", "sid1-1", classGold, "", "snap1-1-volumehandle", deletionPolicy, nil, nil, true, &timeNowMetav1),
-			expectedContents: newContentArrayWithDeletionTimestamp("content1-1", "snapuid1-1", "snap1-1", "", classGold, "", "snap1-1-volumehandle", deletionPolicy, nil, nil, false, &timeNowMetav1),
+			expectedContents: newContentArrayWithDeletionTimestamp("content1-1", "snapuid1-1", "snap1-1", "sid1-1", classGold, "", "snap1-1-volumehandle", deletionPolicy, nil, nil, false, &timeNowMetav1),
 			expectedEvents:   noevents,
 			errors:           noerrors,
 			initialSecrets:   []*v1.Secret{secret()},
@@ -183,9 +183,9 @@ func TestDeleteSync(t *testing.T) {
 			test:                testSyncContent,
 		},
 		{
-			name:             "1-3 - delete snapshot error should result in an event, bound finalizer should remain",
+			name:             "1-3 - delete snapshot error should result in an event",
 			initialContents:  newContentArrayWithDeletionTimestamp("content1-3", "snapuid1-3", "snap1-3", "sid1-3", validSecretClass, "", "snap1-3-volumehandle", deletePolicy, nil, nil, true, &timeNowMetav1),
-			expectedContents: newContentArrayWithDeletionTimestamp("content1-3", "snapuid1-3", "snap1-3", "sid1-3", validSecretClass, "", "snap1-3-volumehandle", deletePolicy, nil, nil, true, &timeNowMetav1),
+			expectedContents: newContentArrayWithDeletionTimestamp("content1-3", "snapuid1-3", "snap1-3", "sid1-3", validSecretClass, "", "snap1-3-volumehandle", deletePolicy, nil, nil, false, &timeNowMetav1),
 			errors:           noerrors,
 			expectedCreateCalls: []createCall{
 				{
@@ -204,124 +204,235 @@ func TestDeleteSync(t *testing.T) {
 			expectedListCalls:   []listCall{{"sid1-3", true, time.Now(), 1, nil}},
 			test:                testSyncContent,
 		},
+		/*{
+			name:            "1-4 - create snapshot error should result in an event",
+			initialContents: newContentArrayWithDeletionTimestamp("content1-4", "snapuid1-4", "snap1-4", "sid1-4", classGold, "", "snap1-4-volumehandle", deletePolicy, nil, nil, true, nil),
+			//expectedContents: newContentArrayWithDeletionTimestamp("content1-4", "snapuid1-4", "snap1-4", "sid1-4", classGold, "", "snap1-4-volumehandle", deletePolicy, nil, nil, true, nil),
+			errors: []reactorError{},
+			expectedCreateCalls: []createCall{
+				{
+					snapshotName: "snapshot-snapuid1-4",
+					volumeHandle: "snap1-4-volumehandle",
+					parameters:   map[string]string{"param1": "value1"},
+					err:          fmt.Errorf("Create failed"),
+				},
+			},
+			//expectedDeleteCalls: []deleteCall{{"sid1-4", nil, nil}},
+			expectedListCalls: []listCall{{"sid1-4", true, time.Now(), 1, nil}},
+			expectedEvents:    []string{"Warning SnapshotContentCheckandUpdateFailed Failed to check and update snapshot content: Create failed"},
+			test:              testSyncContent,
+		},*/
+		/*{
+			name:                "2-1 - content with empty snapshot class will not be deleted if it is bound to a non-exist snapshot but it does not have a snapshot uid specified",
+			initialContents:     newContentArray("content2-1", "", "snap2-1", "sid2-1", "", "", "", deletionPolicy, nil, nil, true),
+			expectedContents:    newContentArray("content2-1", "", "snap2-1", "sid2-1", "", "", "", deletionPolicy, nil, nil, true),
+			expectedEvents:      noevents,
+			errors:              noerrors,
+			expectedDeleteCalls: []deleteCall{{"sid2-1", nil, nil}},
+			test:                testSyncContent,
+		},*/
+		/*{
+			name:                "1-2 - successful delete with snapshot class that has empty secret parameter",
+			initialContents:     newContentArray("content1-2", "sid1-2", "snap1-2", "sid1-2", emptySecretClass, "", "volumeHandle", deletionPolicy, nil, nil, true),
+			expectedContents:    nocontents,
+			initialSnapshots:    nosnapshots,
+			expectedSnapshots:   nosnapshots,
+			initialSecrets:      []*v1.Secret{emptySecret()},
+			expectedEvents:      noevents,
+			errors:              noerrors,
+			expectedDeleteCalls: []deleteCall{{"sid1-2", map[string]string{}, nil}},
+			test:                testSyncContent,
+		},*/
+		/*{
+			name:              "1-3 - content non-nil DeletionTimestamp with delete policy will delete snapshot",
+			initialContents:   newContentArrayWithDeletionTimestamp("content1-3", "snapuid1-1", "snap1-1", "sid1-3", classGold, "", "snap1-3-volumehandle", deletionPolicy, nil, nil, true, &timeNowMetav1),
+			expectedContents:  newContentArrayWithDeletionTimestamp("content1-3", "snapuid1-1", "snap1-1", "sid1-3", classGold, "", "snap1-3-volumehandle", deletionPolicy, nil, nil, false, &timeNowMetav1),
+			initialSnapshots:  newSnapshotArray("snap1-3", "snapuid1-3", "claim1-3", "", validSecretClass, "", &False, nil, nil, nil),
+			expectedSnapshots: newSnapshotArray("snap1-3", "snapuid1-3", "claim1-3", "", validSecretClass, "", &False, nil, nil, nil),
+			expectedEvents:    noevents,
+			errors:            noerrors,
+			initialSecrets:    []*v1.Secret{secret()},
+			expectedCreateCalls: []createCall{
+				{
+					snapshotName: "snapshot-snapuid1-1",
+					volumeHandle: "snap1-1-volumehandle",
+					parameters:   map[string]string{"param1": "value1"},
+					driverName:   mockDriverName,
+					size:         defaultSize,
+					snapshotId:   "snapuid1-1-deleted",
+					creationTime: timeNow,
+					readyToUse:   true,
+				},
+			},
+			expectedListCalls:   []listCall{{"sid1-1", true, time.Now(), 1, nil}},
+			expectedDeleteCalls: []deleteCall{{"sid1-1", nil, nil}},
+			test:                testSyncContent,
+		},*/
+
+		/*			name:              "1-3 - successful delete with snapshot class that has valid secret parameter",
+			initialContents:   newContentArray("content1-3", "sid1-3", "snap1-3", "sid1-3", validSecretClass, "", "snap1-3-volumehandle", deletionPolicy, nil, nil, true),
+			expectedContents:  newContentArray("content1-3", "sid1-3", "snap1-3", "sid1-3", validSecretClass, "", "snap1-3-volumehandle", deletionPolicy, nil, nil, false),
+			initialSnapshots:  newSnapshotArray("snapshot-snapuid1-3", "snapuid1-3", "claim1-3", "", validSecretClass, "", &False, nil, nil, nil),
+			expectedSnapshots: newSnapshotArray("snapshot-snapuid1-3", "snapuid1-3", "claim1-3", "", validSecretClass, "", &False, nil, nil, nil),
+			expectedEvents:    noevents,
+			errors:            noerrors,
+			initialSecrets:    []*v1.Secret{secret()},
+			expectedCreateCalls: []createCall{
+				{
+					snapshotName: "snap1-3",
+					volumeHandle: "snap1-3-volumehandle",
+					parameters:   map[string]string{"param1": "value1"},
+					driverName:   mockDriverName,
+					size:         defaultSize,
+					snapshotId:   "sid1-3",
+					creationTime: timeNow,
+				},
+			},
+			expectedListCalls:   []listCall{{"sid1-3", true, time.Now(), 1, nil}},
+			expectedDeleteCalls: []deleteCall{{"sid1-3", map[string]string{"param1": "value1"}, nil}},
+			test:                testSyncContent,
+		},
+		*/
 		{
-			name:             "1-4 - fail to delete with a snapshot class which has invalid secret parameter, bound finalizer should remain",
-			initialContents:  newContentArrayWithDeletionTimestamp("content1-1", "snapuid1-1", "snap1-1", "sid1-1", "invalid", "", "snap1-4-volumehandle", deletionPolicy, nil, nil, true, &timeNowMetav1),
-			expectedContents: newContentArrayWithDeletionTimestamp("content1-1", "snapuid1-1", "snap1-1", "sid1-1", "invalid", "", "snap1-4-volumehandle", deletionPolicy, nil, nil, true, &timeNowMetav1),
+			name:             "1-4 - fail delete with snapshot class that has invalid secret parameter",
+			initialContents:  newContentArrayWithDeletionTimestamp("content1-1", "snapuid1-1", "snap1-1", "sid1-1", "invalid", "", "snap1-1-volumehandle", deletionPolicy, nil, nil, true, &timeNowMetav1),
+			expectedContents: newContentArrayWithDeletionTimestamp("content1-1", "snapuid1-1", "snap1-1", "sid1-1", "invalid", "", "snap1-1-volumehandle", deletionPolicy, nil, nil, false, &timeNowMetav1),
 			expectedEvents:   noevents,
 			errors:           noerrors,
 			test:             testSyncContent,
 		},
 		{
-			name:                "1-5 - csi driver delete snapshot returns error, bound finalizer should remain",
-			initialContents:     newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "snap1-5-volumehandle", deletionPolicy, nil, &defaultSize, true, &timeNowMetav1),
-			expectedContents:    newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "snap1-5-volumehandle", deletionPolicy, nil, &defaultSize, true, &timeNowMetav1),
+			name:                "1-5 - csi driver delete snapshot returns error",
+			initialContents:     newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "", deletionPolicy, nil, &defaultSize, true, &timeNowMetav1),
+			expectedContents:    newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "", deletionPolicy, nil, &defaultSize, false, &timeNowMetav1),
 			expectedListCalls:   []listCall{{"sid1-5", true, time.Now(), 1000, nil}},
 			expectedDeleteCalls: []deleteCall{{"sid1-5", nil, errors.New("mock csi driver delete error")}},
 			expectedEvents:      []string{"Warning SnapshotDeleteError"},
 			errors:              noerrors,
 			test:                testSyncContent,
 		},
+		/*{
+			name:            "1-6 - api server delete content returns error",
+			initialContents: newContentArray("content1-6", "sid1-6", "snap1-6", "sid1-6", classGold, "", "", deletionPolicy, nil, nil, true),
+			//expectedContents:    newContentArray("content1-6", "sid1-6", "snap1-6", "sid1-6", classGold, "", "", deletionPolicy, nil, nil, true),
+			expectedDeleteCalls: []deleteCall{{"sid1-6", map[string]string{"foo": "bar"}, nil}},
+			expectedListCalls:   []listCall{{"sid1-6", false, time.Now(), 0, nil}},
+			expectedEvents:      []string{"Warning SnapshotContentObjectDeleteError"},
+			errors: []reactorError{
+				// Inject error to the first client.VolumesnapshotV1beta1().VolumeSnapshotContents().Delete call.
+				// All other calls will succeed.
+				{"delete", "volumesnapshotcontents", errors.New("mock delete error")},
+			},
+			test: testSyncContent,
+		},*/
+		/*
+			{
+				// delete success - snapshot that the content was pointing to was deleted, and another
+				// with the same name created.
+				name:                "1-7 - prebound content is deleted while the snapshot exists",
+				initialContents:     newContentArray("content1-7", "sid1-7", "snap1-7", "sid1-7", emptySecretClass, "", "", deletionPolicy, nil, nil, true),
+				expectedContents:    nocontents,
+				initialSecrets:      []*v1.Secret{secret()},
+				expectedDeleteCalls: []deleteCall{{"sid1-7", map[string]string{"foo": "bar"}, nil}},
+				expectedEvents:      noevents,
+				errors:              noerrors,
+				test:                testSyncContent,
+			},*/
 		{
-			// delete success(?) - content is deleted before deleteSnapshot starts
-			name:                "1-6 - content is deleted before deleting",
-			initialContents:     newContentArray("content1-6", "sid1-6", "snap1-6", "sid1-6", classGold, "sid1-6", "", deletionPolicy, nil, nil, true),
+			// delete success(?) - content is deleted before doDelete() starts
+			name:                "1-8 - content is deleted before deleting",
+			initialContents:     newContentArray("content1-8", "sid1-8", "snap1-8", "sid1-8", classGold, "", "", deletionPolicy, nil, nil, true),
 			expectedContents:    nocontents,
-			expectedListCalls:   []listCall{{"sid1-6", false, time.Now(), 0, nil}},
-			expectedDeleteCalls: []deleteCall{{"sid1-6", map[string]string{"foo": "bar"}, nil}},
+			expectedListCalls:   []listCall{{"sid1-8", false, time.Now(), 0, nil}},
+			expectedDeleteCalls: []deleteCall{{"sid1-8", map[string]string{"foo": "bar"}, nil}},
 			expectedEvents:      noevents,
 			errors:              noerrors,
 			test: wrapTestWithInjectedOperation(testSyncContent, func(ctrl *csiSnapshotSideCarController, reactor *snapshotReactor) {
-				// Delete the content before delete operation starts
+				// Delete the volume before delete operation starts
 				reactor.lock.Lock()
-				delete(reactor.contents, "content1-6")
+				delete(reactor.contents, "content1-8")
 				reactor.lock.Unlock()
 			}),
 		},
-		{
-			name:              "1-7 - content will not be deleted if it is bound to a snapshot correctly, snapshot uid is not specified",
-			initialContents:   newContentArrayWithReadyToUse("content1-7", "", "snap1-7", "sid1-7", validSecretClass, "sid1-7", "", deletePolicy, nil, &defaultSize, &True, true),
-			expectedContents:  newContentArrayWithReadyToUse("content1-7", "", "snap1-7", "sid1-7", validSecretClass, "sid1-7", "", deletePolicy, nil, &defaultSize, &True, true),
+		/*{
+			name:              "1-9 - content will not be deleted if it is bound to a snapshot correctly, snapshot uid is specified",
+			expectedContents:  newContentArrayWithDeletionTimestamp("content1-9", "snapuid1-9", "snap1-9", "sid1-9", classGold, "", "snap1-9-volumehandle", retainPolicy, nil, nil, false, &timeNowMetav1),
+			initialContents:   newContentArrayWithDeletionTimestamp("content1-9", "snapuid1-9", "snap1-9", "sid1-9", classGold, "", "snap1-9-volumehandle", retainPolicy, nil, nil, false, &timeNowMetav1),
 			expectedEvents:    noevents,
-			expectedListCalls: []listCall{{"sid1-7", true, time.Now(), 1000, nil}},
+			expectedListCalls: []listCall{{"sid1-9", true, time.Now(), 0, nil}},
 			initialSecrets:    []*v1.Secret{secret()},
 			errors:            noerrors,
 			test:              testSyncContent,
-		},
+		},*/
+		/*
+			{
+				name:              "1-10 - will not delete content with retain policy set which is bound to a snapshot incorrectly",
+				initialContents:   newContentArray("content1-10", "snapuid1-10-x", "snap1-10", "sid1-10", validSecretClass, "", "", retainPolicy, nil, nil, true),
+				expectedContents:  newContentArray("content1-10", "snapuid1-10-x", "snap1-10", "sid1-10", validSecretClass, "", "", retainPolicy, nil, nil, true),
+				initialSnapshots:  newSnapshotArray("snap1-10", "snapuid1-10", "claim1-10", "", validSecretClass, "content1-10", &False, nil, nil, nil),
+				expectedSnapshots: newSnapshotArray("snap1-10", "snapuid1-10", "claim1-10", "", validSecretClass, "content1-10", &False, nil, nil, nil),
+				expectedEvents:    noevents,
+				initialSecrets:    []*v1.Secret{secret()},
+				errors:            noerrors,
+				test:              testSyncContent,
+			},*/
 		{
-			name:              "1-8 - content with retain policy will not be deleted if it is bound to a non-exist snapshot and also has a snapshot uid specified",
-			initialContents:   newContentArrayWithReadyToUse("content1-8", "sid1-8", "none-existed-snapshot", "sid1-8", validSecretClass, "sid1-8", "", retainPolicy, nil, &defaultSize, &True, true),
-			expectedContents:  newContentArrayWithReadyToUse("content1-8", "sid1-8", "none-existed-snapshot", "sid1-8", validSecretClass, "sid1-8", "", retainPolicy, nil, &defaultSize, &True, true),
-			expectedEvents:    noevents,
-			expectedListCalls: []listCall{{"sid1-8", true, time.Now(), 0, nil}},
+			name:             "1-11 - content will not be deleted if it is bound to a snapshot correctly, snapsht uid is not specified",
+			initialContents:  newContentArrayWithReadyToUse("content1-11", "", "snap1-11", "sid1-11", validSecretClass, "", "", deletePolicy, nil, &defaultSize, &True, true),
+			expectedContents: newContentArrayWithReadyToUse("content1-11", "", "snap1-11", "sid1-11", validSecretClass, "", "", deletePolicy, nil, &defaultSize, &True, true),
+			expectedEvents:   noevents,
+			expectedCreateCalls: []createCall{
+				{
+					snapshotName: "snap1-11",
+					volumeHandle: "snap1-11",
+					parameters:   map[string]string{"param1": "value1"},
+					driverName:   mockDriverName,
+					size:         defaultSize,
+					snapshotId:   "snapuid1-1-deleted",
+					creationTime: timeNow,
+					readyToUse:   true,
+				},
+			},
+			expectedListCalls: []listCall{{"sid1-11", true, time.Now(), 1000, nil}},
+			initialSecrets:    []*v1.Secret{secret()},
 			errors:            noerrors,
 			test:              testSyncContent,
 		},
 		{
-			name:                "1-9 - continue deletion with snapshot class that has nonexistent secret, bound finalizer removed",
-			initialContents:     newContentArrayWithDeletionTimestamp("content1-9", "sid1-9", "snap1-9", "sid1-9", emptySecretClass, "", "snap1-9-volumehandle", deletePolicy, nil, &defaultSize, true, &timeNowMetav1),
-			expectedContents:    newContentArrayWithDeletionTimestamp("content1-9", "sid1-9", "snap1-9", "", emptySecretClass, "", "snap1-9-volumehandle", deletePolicy, nil, &defaultSize, false, &timeNowMetav1),
-			expectedEvents:      noevents,
-			expectedListCalls:   []listCall{{"sid1-9", true, time.Now(), 0, nil}},
-			errors:              noerrors,
-			initialSecrets:      []*v1.Secret{}, // secret does not exist
-			expectedDeleteCalls: []deleteCall{{"sid1-9", nil, nil}},
-			test:                testSyncContent,
-		},
-		{
-			name:              "1-10 - (dynamic)deletion of content with retain policy should not trigger CSI call, not update status, but remove bound finalizer",
-			initialContents:   newContentArrayWithDeletionTimestamp("content1-10", "sid1-10", "snap1-10", "sid1-10", emptySecretClass, "", "snap1-10-volumehandle", retainPolicy, nil, &defaultSize, true, &timeNowMetav1),
-			expectedContents:  newContentArrayWithDeletionTimestamp("content1-10", "sid1-10", "snap1-10", "sid1-10", emptySecretClass, "", "snap1-10-volumehandle", retainPolicy, nil, &defaultSize, false, &timeNowMetav1),
+			name:              "1-12 - content with retain policy will not be deleted if it is bound to a non-exist snapshot and also has a snapshot uid specified",
+			initialContents:   newContentArrayWithReadyToUse("content1-12", "sid1-12", "snap1-11", "sid1-11", validSecretClass, "", "", retainPolicy, nil, &defaultSize, &True, true),
+			expectedContents:  newContentArrayWithReadyToUse("content1-12", "sid1-12", "snap1-11", "sid1-11", validSecretClass, "", "", retainPolicy, nil, &defaultSize, &True, true),
 			expectedEvents:    noevents,
-			expectedListCalls: []listCall{{"sid1-10", true, time.Now(), 0, nil}},
+			expectedListCalls: []listCall{{"sid1-11", true, time.Now(), 0, nil}},
 			errors:            noerrors,
-			initialSecrets:    []*v1.Secret{},
 			test:              testSyncContent,
 		},
-		{
-			name:                "1-11 - (dynamic)deletion of content with deletion policy should trigger CSI call, update status, and remove bound finalizer removed.",
-			initialContents:     newContentArrayWithDeletionTimestamp("content1-11", "sid1-11", "snap1-11", "sid1-11", emptySecretClass, "", "snap1-11-volumehandle", deletePolicy, nil, &defaultSize, true, &timeNowMetav1),
-			expectedContents:    newContentArrayWithDeletionTimestamp("content1-11", "sid1-11", "snap1-11", "", emptySecretClass, "", "snap1-11-volumehandle", deletePolicy, nil, nil, false, &timeNowMetav1),
-			expectedEvents:      noevents,
-			errors:              noerrors,
-			expectedDeleteCalls: []deleteCall{{"sid1-11", nil, nil}},
-			test:                testSyncContent,
-		},
-		{
-			name:              "1-12 - (pre-provision)deletion of content with retain policy should not trigger CSI call, not update status, but remove bound finalizer",
-			initialContents:   newContentArrayWithDeletionTimestamp("content1-12", "sid1-12", "snap1-12", "sid1-12", emptySecretClass, "sid1-12", "", retainPolicy, nil, &defaultSize, true, &timeNowMetav1),
-			expectedContents:  newContentArrayWithDeletionTimestamp("content1-12", "sid1-12", "snap1-12", "sid1-12", emptySecretClass, "sid1-12", "", retainPolicy, nil, &defaultSize, false, &timeNowMetav1),
+		/*{
+			name:              "1-13 - content with empty snapshot class is not deleted when Deletion policy is not set even if it is bound to a non-exist snapshot and also has a snapshot uid specified",
+			initialContents:   newContentArray("content1-13", "sid1-13", "snap1-13", "sid1-13", validSecretClass, "", "", retainPolicy, nil, nil, true),
+			expectedContents:  newContentArray("content1-13", "sid1-13", "snap1-13", "sid1-13", validSecretClass, "", "", retainPolicy, nil, nil, true),
 			expectedEvents:    noevents,
-			expectedListCalls: []listCall{{"sid1-12", true, time.Now(), 0, nil}},
 			errors:            noerrors,
-			initialSecrets:    []*v1.Secret{},
 			test:              testSyncContent,
 		},
 		{
-			name:                "1-13 - (pre-provision)deletion of content with deletion policy should trigger CSI call, update status, and remove bound finalizer removed.",
-			initialContents:     newContentArrayWithDeletionTimestamp("content1-13", "sid1-13", "snap1-13", "sid1-13", emptySecretClass, "sid1-13", "", deletePolicy, nil, &defaultSize, true, &timeNowMetav1),
-			expectedContents:    newContentArrayWithDeletionTimestamp("content1-13", "sid1-13", "snap1-13", "", emptySecretClass, "sid1-13", "", deletePolicy, nil, nil, false, &timeNowMetav1),
-			expectedEvents:      noevents,
-			errors:              noerrors,
-			expectedDeleteCalls: []deleteCall{{"sid1-13", nil, nil}},
-			test:                testSyncContent,
-		},
+			name:              "1-14 - content will not be deleted if it is bound to a snapshot correctly, snapshot uid is specified",
+			initialContents:   newContentArray("content1-14", "snapuid1-14", "snap1-14", "sid1-14", validSecretClass, "", "", retainPolicy, nil, nil, true),
+			expectedContents:  newContentArray("content1-14", "snapuid1-14", "snap1-14", "sid1-14", validSecretClass, "", "", retainPolicy, nil, nil, true),
+			expectedEvents:    noevents,
+			initialSecrets:    []*v1.Secret{secret()},
+			errors:            noerrors,
+			test:              testSyncContent,
+		},*/
 		{
-			name:                "1-14 - (pre-provision)deletion of content with deletion policy and no snapshotclass should trigger CSI call, update status, and remove bound finalizer removed.",
-			initialContents:     newContentArrayWithDeletionTimestamp("content1-14", "sid1-14", "snap1-14", "sid1-14", "", "sid1-14", "", deletePolicy, nil, &defaultSize, true, &timeNowMetav1),
-			expectedContents:    newContentArrayWithDeletionTimestamp("content1-14", "sid1-14", "snap1-14", "", "", "sid1-14", "", deletePolicy, nil, nil, false, &timeNowMetav1),
+			name:                "1-16 - continue delete with snapshot class that has nonexistent secret",
+			initialContents:     newContentArrayWithDeletionTimestamp("content1-16", "sid1-16", "snap1-16", "sid1-16", emptySecretClass, "", "", deletePolicy, nil, &defaultSize, true, &timeNowMetav1),
+			expectedContents:    newContentArrayWithDeletionTimestamp("content1-16", "sid1-16", "snap1-16", "sid1-16", emptySecretClass, "", "", deletePolicy, nil, &defaultSize, false, &timeNowMetav1),
 			expectedEvents:      noevents,
+			expectedListCalls:   []listCall{{"sid1-16", true, time.Now(), 0, nil}},
 			errors:              noerrors,
-			expectedDeleteCalls: []deleteCall{{"sid1-14", nil, nil}},
-			test:                testSyncContent,
-		},
-		{
-			name:                "1-15 - (dynamic)deletion of content with no snapshotclass should produce error",
-			initialContents:     newContentArrayWithDeletionTimestamp("content1-15", "sid1-15", "snap1-15", "sid1-15", "", "", "snap1-15-volumehandle", deletePolicy, nil, &defaultSize, true, &timeNowMetav1),
-			expectedContents:    newContentArrayWithDeletionTimestamp("content1-15", "sid1-15", "snap1-15", "sid1-15", "", "", "snap1-15-volumehandle", deletePolicy, nil, &defaultSize, true, &timeNowMetav1),
-			expectedEvents:      []string{"Warning SnapshotDeleteError"},
-			errors:              noerrors,
-			expectedDeleteCalls: []deleteCall{{"sid1-15", nil, nil}},
+			initialSecrets:      []*v1.Secret{}, // secret does not exist
+			expectedDeleteCalls: []deleteCall{{"sid1-16", nil, nil}},
 			test:                testSyncContent,
 		},
 	}