Updated sidecar to not require VolumeSnapshotClass for snapshot deletion

[huffmanca]

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 #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?:

Allows the sidecar to delete volume snapshots if the volume snapshot class is not found.

[k8s-ci-robot]

Hi @huffmanca. Thanks for your PR.

I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[huffmanca]

@xing-yang ,

I'm submitting this as a WIP, as I can't get it to fully past the delete tests. There's an issue with the expected number of delete events on these tests, where we're expecting 0 events, and the array of events is nil. I haven't been able to trace why this is the cause. Would you have any thoughts?

[xing-yang]

/ok-to-test

[xing-yang]

@huffmanca I think it is because this PR allows snapshot to be deleted when snapshot class is not found. Check if those failed tests have a snapshot class that is not found.

[humblec]

@xing-yang is it valid to delete volumesnapshot if subjected SC not found? at times, we may require some params or data from SC to perform delete operation, Isnt it ?

[xing-yang]

@xing-yang is it valid to delete volumesnapshot if subjected SC not found? at times, we may require some params or data from SC to perform delete operation, Isnt it ?

Yes, it is valid to delete snapshot even when SC is already deleted. We only need to have secrets when deleting the snapshot which is already added as annotation to the content.

[xing-yang]

@huffmanca can you rebase your PR and see if you still have the issue with unit tests?

[k8s-ci-robot]

@huffmanca: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kubernetes-csi-external-snapshotter-1-15-on-kubernetes-1-15	3efec76	link	/test pull-kubernetes-csi-external-snapshotter-1-15-on-kubernetes-1-15

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[xing-yang]

@huffmanca, I added comments to the unit tests. It should pass after you've addressed my comments.

[huffmanca]

@xing-yang ,

I've updated the PR, and the tests pass after following your instructions. I also rebased this PR, as there's been some time since the initial submission.

With that being said, I haven't been able to reproduce the issue in the sidecar, and I'm hesitant about merging it blindly. It seems identical to the fix in the snapshot-controller, which is why it's been submitted.

[xing-yang]

@huffmanca It is a timing issue, so I think whether it happens depends on the environment. Your fix looks right to me.

[xing-yang]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: huffmanca, xing-yang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [xing-yang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment