fix(controller): prevent zfs volume cr deletion if snapshot exists

Signed-off-by: sinhaashish <[email protected]>

ci/ci-test.sh

@@ -107,7 +107,7 @@ runTestSuite() {
 
   echo "running ginkgo test case with coverage ${coverageFile}"
 
-  if ! ginkgo -v -coverprofile="${coverageFile}" --label-filter="${labelFilter}" -covermode=atomic; then
+  if ! ginkgo -p -v -coverprofile="${coverageFile}" --label-filter="${labelFilter}" -covermode=atomic; then
 
   sudo zpool status
 

pkg/driver/controller.go

@@ -502,6 +502,18 @@ func (cs *controller) DeleteVolume(
 	unlock := cs.volumeLock.LockVolume(volumeID)
 	defer unlock()
 
+	// Fetch the list of snapshot for the given volume
+	snapList, err := zfs.GetSnapshotForVolume(volumeID)
+	if err != nil {
+		return nil, status.Errorf(
+			codes.NotFound,
+			"failed to handle delete volume request for {%s}, "+
+				"validation failed checking for snapshots. Error: %s",
+			req.VolumeId,
+			err.Error(),
+		)
+	}
+
 	// verify if the volume has already been deleted
 	vol, err := zfs.GetVolume(volumeID)
 	if vol != nil && vol.DeletionTimestamp != nil {
@@ -524,14 +536,16 @@ func (cs *controller) DeleteVolume(
 		return nil, status.Error(codes.Internal, "can not delete, volume creation is in progress")
 	}
 
-	// Delete the corresponding ZV CR
-	err = zfs.DeleteVolume(volumeID)
-	if err != nil {
-		return nil, errors.Wrapf(
-			err,
-			"failed to handle delete volume request for {%s}",
-			volumeID,
-		)
+	// Delete the corresponding ZV CR only if there are no snapshots present for the volume
+	if len(snapList.Items) == 0 {
+		err = zfs.DeleteVolume(volumeID)
+		if err != nil {
+			return nil, errors.Wrapf(
+				err,
+				"failed to handle delete volume request for {%s}",
+				volumeID,
+			)
+		}
 	}
 
 	sendEventOrIgnore("", volumeID, vol.Spec.Capacity, analytics.VolumeDeprovision)

pkg/zfs/volume.go

@@ -441,3 +441,12 @@ func IsVolumeReady(vol *apis.ZFSVolume) bool {
 
 	return false
 }
+
+// GetSnapshotForVolume fetches all the snapshots for the given volume
+func GetSnapshotForVolume(volumeID string) (*apis.ZFSSnapshotList, error) {
+	listOptions := metav1.ListOptions{
+		LabelSelector: ZFSVolKey + "=" + volumeID,
+	}
+	snapList, err := snapbuilder.NewKubeclient().WithNamespace(OpenEBSNamespace).List(listOptions)
+	return snapList, err
+}

tests/provision_test.go

@@ -24,6 +24,7 @@ var _ = Describe("[zfspv] TEST VOLUME PROVISIONING", func() {
 	Context("App is deployed with zfs driver", func() {
 		It("Running zfs volume Creation Test", volumeCreationTest)
 		It("Running zfs volume Creation Test with custom node id", Label("custom-node-id"), volumeCreationTest)
+		It("Running zfs volume Deletion Test", volumeDeletionTest)
 	})
 })
 
@@ -41,7 +42,7 @@ func exhaustiveVolumeTests(parameters map[string]string) {
 	snapshotAndCloneCreate()
 	// btrfs does not support online resize
 	if fstype != "btrfs" {
-		By("Resizing the PVC", resizeAndVerifyPVC)
+		By("Resizing the PVC", func() { resizeAndVerifyPVC(pvcNameFS) })
 	}
 	snapshotAndCloneCleanUp()
 	cleanUp()
@@ -51,57 +52,83 @@ func exhaustiveVolumeTests(parameters map[string]string) {
 func create(parameters map[string]string) {
 	By("####### Creating the storage class : " + parameters["fstype"] + " #######")
 	createFstypeStorageClass(parameters)
-	By("creating and verifying PVC bound status", createAndVerifyPVC)
-	By("Creating and deploying app pod", createDeployVerifyApp)
+	By("creating and verifying PVC bound status", func() { createAndVerifyPVC(pvcNameFS) })
+	By("Creating and deploying app pod", func() { createDeployVerifyApp(appNameFS, pvcNameFS) })
 	By("verifying ZFSVolume object", VerifyZFSVolume)
 	By("verifying storage class parameters")
 	VerifyStorageClassParams(parameters)
 }
 
 // Creates the snapshot/clone resources
 func snapshotAndCloneCreate() {
-	createSnapshot(pvcName, snapName)
-	verifySnapshotCreated(snapName)
-	createClone(clonePvcName, snapName, scObj.Name)
-	By("Creating and deploying clone app pod", createDeployVerifyCloneApp)
+	createSnapshot(pvcNameFS, snapNameFS)
+	verifySnapshotCreated(snapNameFS)
+	createClone(clonePvcNameFS, snapNameFS, scObj.Name)
+	By("Creating and deploying clone app pod", func() { createDeployVerifyCloneApp(cloneAppNameFS, clonePvcNameFS) })
 }
 
 // Removes the snapshot/clone resources
 func snapshotAndCloneCleanUp() {
-	deleteAppDeployment(cloneAppName)
-	deletePVC(clonePvcName)
-	deleteSnapshot(pvcName, snapName)
+	deleteAppDeployment(cloneAppNameFS)
+	deletePVC(clonePvcNameFS)
+	deleteSnapshot(pvcNameFS, snapNameFS)
 }
 
 // Removes the resources
 func cleanUp() {
-	deleteAppDeployment(appName)
-	deletePVC(pvcName)
+	deleteAppDeployment(appNameFS)
+	deletePVC(pvcNameFS)
 	By("Deleting storage class", deleteStorageClass)
 }
 
 func blockVolCreationTest() {
 	By("Creating default storage class", createStorageClass)
-	By("creating and verifying PVC bound status", createAndVerifyBlockPVC)
+	By("creating and verifying PVC bound status", func() { createAndVerifyPVC(pvcNameBlock) })
 
-	By("Creating and deploying app pod", createDeployVerifyBlockApp)
+	By("Creating and deploying app pod", func() { createDeployVerifyApp(appNameBlock, pvcNameBlock) })
 	By("verifying ZFSVolume object", VerifyZFSVolume)
 	By("verifying ZFSVolume property change", VerifyZFSVolumePropEdit)
-	By("Deleting application deployment")
 
-	createSnapshot(pvcName, snapName)
-	verifySnapshotCreated(snapName)
-	createClone(clonePvcName, snapName, scObj.Name)
-	By("Creating and deploying clone app pod", createDeployVerifyCloneApp)
+	createSnapshot(pvcNameBlock, snapNameBlock)
+	verifySnapshotCreated(snapNameBlock)
+	createClone(clonePvcNameBlock, snapNameBlock, scObj.Name)
+	By("Creating and deploying clone app pod", func() { createDeployVerifyCloneApp(cloneAppNameBlock, clonePvcNameBlock) })
 
 	By("Deleting clone and main application deployment")
-	deleteAppDeployment(cloneAppName)
-	deleteAppDeployment(appName)
+	deleteAppDeployment(cloneAppNameBlock)
+	deleteAppDeployment(appNameBlock)
 
 	By("Deleting snapshot, main pvc and clone pvc")
-	deletePVC(clonePvcName)
-	deleteSnapshot(pvcName, snapName)
-	deletePVC(pvcName)
+
+	deletePVC(clonePvcNameBlock)
+	deleteSnapshot(pvcNameBlock, snapNameBlock)
+	deletePVC(pvcNameBlock)
+
+	By("Deleting storage class", deleteStorageClass)
+}
+
+func blockVolDeletionTest() {
+	By("Creating default storage class", createStorageClass)
+	By("creating and verifying PVC bound status", func() { createAndVerifyPVC(pvcNameAplha) })
+	By("Creating and deploying app pod", func() { createDeployVerifyApp(appNameAlpha, pvcNameAplha) })
+	By("verifying ZFSVolume object", VerifyZFSVolume)
+	createSnapshot(pvcNameAplha, snapNameAlpha)
+	verifySnapshotCreated(snapNameAlpha)
+	By("Deleting main application deployment")
+	deleteAppDeployment(appNameAlpha)
+	zvName := getZVName(pvcNameAplha)
+	isZVPresentEventually(zvName)
+	By("Deleting main pvc")
+	deletePVC(pvcNameAplha)
+	By("Verifying ZFSVolume object after pvc deletion when snapshot is present", VerifyZFSVolume)
+	By("Creating clone from the snapshot")
+	createClone(clonePvcNameAlpha, snapNameAlpha, scObj.Name)
+	By("Creating and deploying clone app pod", func() { createDeployVerifyCloneApp(cloneAppNameAlpha, clonePvcNameAlpha) })
+	By("Deleting clone application deployment, clone pvc")
+	deleteAppDeployment(cloneAppNameAlpha)
+	deletePVC(clonePvcNameAlpha)
+	By("Verifying that ZV is present after pvc deletion ", func() { isZVPresentEventually(zvName) })
+	deleteSnapshot(pvcNameAplha, snapNameAlpha)
 
 	By("Deleting storage class", deleteStorageClass)
 }
@@ -110,3 +137,7 @@ func volumeCreationTest() {
 	By("Running volume creation test", fsVolCreationTest)
 	By("Running block volume creation test", blockVolCreationTest)
 }
+
+func volumeDeletionTest() {
+	By("Running volume deletion test", blockVolDeletionTest)
+}

tests/suite_test.go

@@ -48,11 +48,33 @@ var (
 	PodClient      *pod.KubeClient
 	scName         = "zfspv-sc"
 	ZFSProvisioner = "zfs.csi.openebs.io"
-	pvcName        = "zfspv-pvc"
-	snapName       = "zfspv-snap"
-	appName        = "busybox-zfspv"
-	clonePvcName   = "zfspv-pvc-clone"
-	cloneAppName   = "busybox-zfspv-clone"
+
+	pvcName  = "zfspv-pvc-fs"
+	snapName = "zfspv-snap"
+	appName  = "busybox-zfspv"
+
+	clonePvcName = "zfspv-pvc-clone"
+	cloneAppName = "busybox-zfspv-clone"
+
+	pvcNameFS    = "zfspv-pvc-fs"
+	pvcNameBlock = "zfspv-pvc-block"
+	pvcNameAplha = "zfspv-pvc-alpha"
+
+	appNameFS    = "busybox-zfspv-fs"
+	appNameBlock = "busybox-zfspv-block"
+	appNameAlpha = "busybox-zfspv-alpha"
+
+	snapNameFS    = "zfspv-snap-fs"
+	snapNameBlock = "zfspv-snap-block"
+	snapNameAlpha = "zfspv-snap-alpha"
+
+	clonePvcNameFS    = "zfspv-pvc-clone-fs"
+	clonePvcNameBlock = "zfspv-pvc-clone-block"
+	clonePvcNameAlpha = "zfspv-pvc-clone-alpha"
+
+	cloneAppNameFS    = "busybox-zfspv-clone-fs"
+	cloneAppNameBlock = "busybox-zfspv-clone-block"
+	cloneAppNameAlpha = "busybox-zfspv-clone-alpha"
 
 	scObj              *storagev1.StorageClass
 	deployObj          *appsv1.Deployment

tests/utils.go

@@ -349,10 +349,9 @@ func deleteStorageClass() {
 		"while deleting zfs storageclass {%s}", scObj.Name)
 }
 
-func createAndVerifyPVC() {
+func createAndVerifyPVC(pvcName string) {
 	var (
-		err     error
-		pvcName = "zfspv-pvc"
+		err error
 	)
 	ginkgo.By("building a pvc")
 	pvcObj, err = pvc.NewBuilder().
@@ -361,53 +360,11 @@ func createAndVerifyPVC() {
 		WithStorageClass(scObj.Name).
 		WithAccessModes(accessModes).
 		WithCapacity(capacity).Build()
-	gomega.Expect(err).ShouldNot(
-		gomega.HaveOccurred(),
-		"while building pvc {%s} in namespace {%s}",
-		pvcName,
-		OpenEBSNamespace,
-	)
-
-	ginkgo.By("creating above pvc")
-	pvcObj, err = PVCClient.WithNamespace(OpenEBSNamespace).Create(pvcObj)
-	gomega.Expect(err).To(
-		gomega.BeNil(),
-		"while creating pvc {%s} in namespace {%s}",
-		pvcName,
-		OpenEBSNamespace,
-	)
-
-	ginkgo.By("verifying pvc status as bound")
-
-	status := IsPVCBoundEventually(pvcName)
-	gomega.Expect(status).To(gomega.Equal(true),
-		"while checking status equal to bound")
 
-	pvcObj, err = PVCClient.WithNamespace(OpenEBSNamespace).Get(pvcObj.Name, metav1.GetOptions{})
-	gomega.Expect(err).To(
-		gomega.BeNil(),
-		"while retrieving pvc {%s} in namespace {%s}",
-		pvcName,
-		OpenEBSNamespace,
-	)
-}
-
-func createAndVerifyBlockPVC() {
-	var (
-		err     error
-		pvcName = "zfspv-pvc"
-	)
-
-	volmode := corev1.PersistentVolumeBlock
-
-	ginkgo.By("building a pvc")
-	pvcObj, err = pvc.NewBuilder().
-		WithName(pvcName).
-		WithNamespace(OpenEBSNamespace).
-		WithStorageClass(scObj.Name).
-		WithAccessModes(accessModes).
-		WithVolumeMode(&volmode).
-		WithCapacity(capacity).Build()
+	if pvcName == "zfspv-pvc-block" {
+		volmode := corev1.PersistentVolumeBlock
+		pvcObj.Spec.VolumeMode = &volmode
+	}
 	gomega.Expect(err).ShouldNot(
 		gomega.HaveOccurred(),
 		"while building pvc {%s} in namespace {%s}",
@@ -439,10 +396,9 @@ func createAndVerifyBlockPVC() {
 	)
 }
 
-func resizeAndVerifyPVC() {
+func resizeAndVerifyPVC(pvcName string) {
 	var (
-		err     error
-		pvcName = "zfspv-pvc"
+		err error
 	)
 	ginkgo.By("updating the pvc with new size")
 	pvcObj, err = PVCClient.WithNamespace(OpenEBSNamespace).Get(pvcObj.Name, metav1.GetOptions{})
@@ -476,13 +432,17 @@ func resizeAndVerifyPVC() {
 		OpenEBSNamespace,
 	)
 }
-func createDeployVerifyApp() {
+func createDeployVerifyApp(appName, pvcName string) {
 	ginkgo.By("creating and deploying app pod")
-	createAndDeployAppPod(appName, pvcName)
+	if pvcName == "zfspv-pvc-block" || pvcName == "pvc-name-for-del-test" {
+		createAndDeployBlockAppPod(appName, pvcName)
+	} else {
+		createAndDeployAppPod(appName, pvcName)
+	}
 	ginkgo.By("verifying app pod is running", func() { verifyAppPodRunning(appName) })
 }
 
-func createDeployVerifyCloneApp() {
+func createDeployVerifyCloneApp(cloneAppName, clonePvcName string) {
 	ginkgo.By("creating and deploying app pod")
 	createAndDeployAppPod(cloneAppName, clonePvcName)
 	ginkgo.By("verifying app pod is running", func() { verifyAppPodRunning(cloneAppName) })
@@ -544,7 +504,7 @@ func createAndDeployAppPod(appname string, pvcname string) {
 	)
 }
 
-func createAndDeployBlockAppPod() {
+func createAndDeployBlockAppPod(appName, pvcName string) {
 	var err error
 	labels := map[string]string{
 		"app":     "busybox",
@@ -583,7 +543,7 @@ func createAndDeployBlockAppPod() {
 				WithVolumeBuilders(
 					k8svolume.NewBuilder().
 						WithName("datavol1").
-						WithPVCSource(pvcObj.Name),
+						WithPVCSource(pvcName),
 				).
 				WithTerminationGracePeriodSeconds(5),
 		).
@@ -600,11 +560,6 @@ func createAndDeployBlockAppPod() {
 	)
 }
 
-func createDeployVerifyBlockApp() {
-	ginkgo.By("creating and deploying app pod", createAndDeployBlockAppPod)
-	ginkgo.By("verifying app pod is running", func() { verifyAppPodRunning(appName) })
-}
-
 func verifyAppPodRunning(appname string) {
 	var err error
 	gomega.Eventually(func() bool {
@@ -711,3 +666,22 @@ func getStoragClassParams() []map[string]string {
 		},
 	}
 }
+
+// isZVPresentEventually checks if the pvc is bound or not eventually
+func isZVPresentEventually(zvName string) bool {
+	return gomega.Eventually(func() bool {
+		volume, err := ZFSClient.WithNamespace(OpenEBSNamespace).
+			Get(zvName, metav1.GetOptions{})
+		gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
+		return volume.Name == zvName
+	},
+		60, 5).
+		Should(gomega.BeTrue())
+}
+
+// getZVName return the zv name
+func getZVName(pvcName string) string {
+	pvcObj, _ = PVCClient.WithNamespace(OpenEBSNamespace).Get(pvcName, metav1.GetOptions{})
+	return pvcObj.Spec.VolumeName
+
+}