From 2f821329996071163a119dd9a93aa06a75b0edb7 Mon Sep 17 00:00:00 2001
From: prateekpandey14 <prateek.pandey@mayadata.io>
Date: Mon, 8 Jun 2020 14:46:17 +0530
Subject: [PATCH] fix(snapshot): ignore snapshot delete request for CSI volumes

Signed-off-by: prateekpandey14 <prateek.pandey@mayadata.io>
---
 pkg/apis/openebs.io/v1alpha1/cas_keys.go |  3 +++
 pkg/snapshot/v1alpha1/snapshot.go        |  9 ++++++++
 pkg/webhook/webhook.go                   | 11 ++++------
 pkg/webhook/webhook_test.go              | 28 +++++++++++++++++++++---
 4 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/pkg/apis/openebs.io/v1alpha1/cas_keys.go b/pkg/apis/openebs.io/v1alpha1/cas_keys.go
index 8691345efc..2dec183d79 100644
--- a/pkg/apis/openebs.io/v1alpha1/cas_keys.go
+++ b/pkg/apis/openebs.io/v1alpha1/cas_keys.go
@@ -102,6 +102,9 @@ const (
 
 	// PVCreatedByKey is key to fetch the details of pv creation in case of restore
 	PVCreatedByKey = "openebs.io/created-through"
+
+	// Name of the CSI driver
+	CSIDriverName = "cstor.csi.openebs.io"
 )
 
 // CASPlainKey represents a openebs key used either in resource annotation
diff --git a/pkg/snapshot/v1alpha1/snapshot.go b/pkg/snapshot/v1alpha1/snapshot.go
index c573f24640..b2426b9488 100644
--- a/pkg/snapshot/v1alpha1/snapshot.go
+++ b/pkg/snapshot/v1alpha1/snapshot.go
@@ -211,6 +211,15 @@ func (s *snapshot) Delete() (*v1alpha1.CASSnapshot, error) {
 		return nil, err
 	}
 
+	// Return success for delete snapshot request in case volume snapshot belongs to a
+	// migrated CSI based persistent volume. Later as part of migration snapshot
+	// can be migrated to CSI snapshots using a migration or manual steps
+	if pv.Spec.CSI != nil {
+		if string(pv.Spec.CSI.Driver) == string(v1alpha1.CSIDriverName) {
+			return nil, nil
+		}
+	}
+
 	storageEngine := pv.Labels[string(v1alpha1.CASTypeKey)]
 	scName := pv.Labels[string(v1alpha1.StorageClassKey)]
 	if len(scName) == 0 {
diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go
index 9eb4d625c8..20533b044a 100644
--- a/pkg/webhook/webhook.go
+++ b/pkg/webhook/webhook.go
@@ -195,7 +195,8 @@ func admissionRequired(ignoredList []string, metadata *metav1.ObjectMeta) bool {
 	switch strings.ToLower(annotations[skipValidation]) {
 	default:
 		required = true
-	case "n", "no", "false", "off":
+	case "y", "yes", "true":
+		klog.Infof("Skipping validations for %s/%s due to PVC has skip validation", metadata.Namespace, metadata.Name)
 		required = false
 	}
 	return required
@@ -245,18 +246,14 @@ func (wh *webhook) validatePVCDeleteRequest(req *v1beta1.AdmissionRequest) *v1be
 		return response
 	}
 
-	// skip pvc validation if skip-validations annotation has been set
-	if _, ok := pvc.GetAnnotations()[skipValidation]; ok {
-		klog.Infof("Skipping validations for %s/%s due to PVC has skip validation", pvc.Namespace, pvc.Name)
-		return response
-	}
-
 	// If PVC is not yet bound to PV then don't even perform any checks
 	if pvc.Spec.VolumeName == "" {
 		klog.Infof("Skipping validations for %s/%s due to PVC is not yet bound", pvc.Namespace, pvc.Name)
 		return response
 	}
 
+	// skip pvc validation if skip-validations annotation has been set to true
+	// "openebs.io/skip-validations: true"
 	if !validationRequired(ignoredNamespaces, &pvc.ObjectMeta) {
 		klog.V(4).Infof("Skipping validation for %s/%s due to policy check", pvc.Namespace, pvc.Name)
 		return response
diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go
index 2d57fb7818..cbc1ea97cf 100644
--- a/pkg/webhook/webhook_test.go
+++ b/pkg/webhook/webhook_test.go
@@ -306,12 +306,12 @@ func TestValidatePVCDeleteRequest(t *testing.T) {
 			isRequiresPVCCreation: true,
 			expectedResponse:      true,
 		},
-		"Skip pvc validaion if skipValidation annotations set even snapshot exists": {
+		"validate pvc if skipValidation annotations set to false": {
 			pvc: &corev1.PersistentVolumeClaim{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:        "PVC8",
 					Namespace:   "test",
-					Annotations: map[string]string{skipValidation: "true"},
+					Annotations: map[string]string{skipValidation: ""},
 				},
 				Spec: corev1.PersistentVolumeClaimSpec{
 					VolumeName: "PV1",
@@ -325,7 +325,29 @@ func TestValidatePVCDeleteRequest(t *testing.T) {
 				},
 			},
 			isRequiresPVCCreation: true,
-			expectedResponse:      true,
+			expectedResponse:      false,
+		},
+
+		"validate pvc if skipValidation annotations not properly set": {
+			pvc: &corev1.PersistentVolumeClaim{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "PVC8",
+					Namespace:   "test",
+					Annotations: map[string]string{skipValidation: ""},
+				},
+				Spec: corev1.PersistentVolumeClaimSpec{
+					VolumeName: "PV1",
+				},
+			},
+			snapshot: &snapshotapi.VolumeSnapshot{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "Snap1",
+					Namespace: "test",
+					Labels:    map[string]string{snapshotMetadataPVName: "PV1"},
+				},
+			},
+			isRequiresPVCCreation: true,
+			expectedResponse:      false,
 		},
 	}
 	for name, test := range tests {