Repository: kubernetes-csi/external-snapshotter (go)

Issue:
VolumeSnapshot resources still remain after pass the E2E test
**What happened**:

Run storage E2E test and passed the test, but some VolumeSnapshot resources remain.

**What you expected to happen**:

VolumeSnapshot resources are deleted after E2E test.

**How to reproduce it (as minimally and precisely as possible)**:

Use https://github.com/zlabjp/kube-conformance .

Below is my testdriver.yaml

```
ShortName: ontap-block
StorageClass:
  FromName: false
  FromFile: </path/to/storage-class>
SnapshotClass:
  FromName: true
DriverInfo:
  Name: <Name My Storage Driver>
  Capabilities:
    persistence: true
    fsGroup: true
    exec: true
    block: true
    pvcDataSource: true
    controllerExpansion: true
    nodeExpansion: true
    volumeLimits: false
    singleNodeVolume: false
    snapshotDataSource: true
    topoligy: false
  SupportedSizeRange:
   Min: 1Gi
   Max: 10Gi
  RequiredAccessModes:
  - ReadWriteOnce
```

**Anything else we need to know?**:

The main cause may be [here](https://github.com/kubernetes-csi/external-snapshotter/blob/4b7aec33d4760138711058c8d94f324188b4e06b/pkg/common-controller/snapshot_controller_base.go#L217).

`snapshotWorker` calls `checkAndUpdateSnapshotClass` to check VolumeSnapshotClass.
This logic prevent deleting VolumeSnapshot when VolumeSnapshotClass have already deleted.


On the E2E side, [here](https://github.com/kubernetes/kubernetes/blob/v1.17.3/test/e2e/storage/testsuites/provisioning.go#L669) is the logic to cleanup resources.

```
	cleanupFunc := func() {
		framework.Logf("deleting snapshot %q/%q", snapshot.GetNamespace(), snapshot.GetName())
		err = dynamicClient.Resource(snapshotGVR).Namespace(updatedClaim.Namespace).Delete(snapshot.GetName(), nil)
		if err != nil && !apierrs.IsNotFound(err) {
			framework.Failf("Error deleting snapshot %q. Error: %v", snapshot.GetName(), err)
		}

		framework.Logf("deleting initClaim %q/%q", updatedClaim.Namespace, updatedClaim.Name)
		err = client.CoreV1().PersistentVolumeClaims(updatedClaim.Namespace).Delete(updatedClaim.Name, nil)
		if err != nil && !apierrs.IsNotFound(err) {
			framework.Failf("Error deleting initClaim %q. Error: %v", updatedClaim.Name, err)
		}

		framework.Logf("deleting SnapshotClass %s", snapshotClass.GetName())
		framework.ExpectNoError(dynamicClient.Resource(snapshotClassGVR).Delete(snapshotClass.GetName(), nil))
	}
```

E2E doesn't wait for finishing to delete the VolumeSnapshot, and delete SnapshotClass.
So that, SnapshotResource can't be deleted.

I think `snapshotWorker` need not to check VolumeSnapshotClass when delete VolumeSnapshot( and VolumeSnapshotContent).

**Environment**:
- Kubernetes version (use `kubectl version`): v1.17.2
- Cloud provider or hardware configuration: openstack
- OS (e.g: `cat /etc/os-release`): container-linux-2303.3.0
- Kernel (e.g. `uname -a`): 4.19.86-coreos
- Install tools: original


Pull request:
Updated sidecar to not require VolumeSnapshotClass for snapshot deletion
**What type of PR is this?**
/kind flake

**What this PR does / why we need it**:
Allows the sidecar to delete VolumeSnapshots if the VolumeSnapshotClass has already been deleted. This PR is a followup to https://github.com/kubernetes-csi/external-snapshotter/pull/275 for the sidecar.

**Which issue(s) this PR fixes**:
Fixes #264 

**Special notes for your reviewer**:

**Does this PR introduce a user-facing change?**:
```release-note
Allows the sidecar to delete volume snapshots if the volume snapshot class is not found.
```


Patch:
diff --git a/pkg/sidecar-controller/snapshot_controller.go b/pkg/sidecar-controller/snapshot_controller.go
index 96688f637..d81555c88 100644
--- a/pkg/sidecar-controller/snapshot_controller.go
+++ b/pkg/sidecar-controller/snapshot_controller.go
@@ -27,6 +27,7 @@ import (
 	codes "google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/util/slice"
@@ -329,7 +330,7 @@ func (ctrl *csiSnapshotSideCarController) deleteCSISnapshotOperation(content *cr
 	klog.V(5).Infof("deleteCSISnapshotOperation [%s] started", content.Name)
 
 	_, snapshotterCredentials, err := ctrl.getCSISnapshotInput(content)
-	if err != nil {
+	if err != nil && !errors.IsNotFound(err) {
 		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)
 	}
@@ -441,7 +442,7 @@ func (ctrl *csiSnapshotSideCarController) getSnapshotClass(className string) (*c
 	class, err := ctrl.classLister.Get(className)
 	if err != nil {
 		klog.Errorf("failed to retrieve snapshot class %s from the informer: %q", className, err)
-		return nil, fmt.Errorf("failed to retrieve snapshot class %s from the informer: %q", className, err)
+		return nil, err
 	}
 
 	return class, nil


Test patch:
diff --git a/pkg/sidecar-controller/content_create_test.go b/pkg/sidecar-controller/content_create_test.go
index babc144a8..c211c51d5 100644
--- a/pkg/sidecar-controller/content_create_test.go
+++ b/pkg/sidecar-controller/content_create_test.go
@@ -163,7 +163,7 @@ func TestSyncContent(t *testing.T) {
 					SnapshotHandle: toStringPointer("sid1-6"),
 					RestoreSize:    &defaultSize,
 					ReadyToUse:     &False,
-					Error:          newSnapshotError("Failed to check and update snapshot content: failed to get input parameters to create snapshot for content content1-6: \"failed to retrieve snapshot class bad-class from the informer: \\\"volumesnapshotclass.snapshot.storage.k8s.io \\\\\\\"bad-class\\\\\\\" not found\\\"\""),
+					Error:          newSnapshotError("Failed to check and update snapshot content: failed to get input parameters to create snapshot for content content1-6: \"volumesnapshotclass.snapshot.storage.k8s.io \\\"bad-class\\\" not found\""),
 				}),
 			expectedEvents: []string{"Warning SnapshotContentCheckandUpdateFailed"},
 			expectedCreateCalls: []createCall{
diff --git a/pkg/sidecar-controller/snapshot_delete_test.go b/pkg/sidecar-controller/snapshot_delete_test.go
index dd73ed4fb..58113c851 100644
--- a/pkg/sidecar-controller/snapshot_delete_test.go
+++ b/pkg/sidecar-controller/snapshot_delete_test.go
@@ -221,10 +221,11 @@ func TestDeleteSync(t *testing.T) {
 			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),
-			expectedEvents:   noevents,
+			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),
+			expectedEvents:      noevents,
+			expectedDeleteCalls: []deleteCall{{"sid1-1", nil, fmt.Errorf("mock csi driver delete error")}},
 			errors: []reactorError{
 				// Inject error to the first client.VolumesnapshotV1beta1().VolumeSnapshotContents().Delete call.
 				// All other calls will succeed.
@@ -336,10 +337,9 @@ func TestDeleteSync(t *testing.T) {
 			test:                testSyncContent,
 		},
 		{
-			name:                "1-15 - (dynamic)deletion of content with no snapshotclass should produce error",
+			name:                "1-15 - (dynamic)deletion of content with no snapshotclass should succeed",
 			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}},
 			test:                testSyncContent,