feat(zfspv): remove finalizer that is owned by ZFS-LocalPV

We set the Finalizer to nil while handling the delete event, instead,
we should try to destroy the volume when there are no user finalizers
set. User might have added his own finalizers and we should try to destroy
the volumes util those user finalizers are removed.

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

pkg/mgmt/snapshot/snapshot.go

@@ -77,14 +77,19 @@ func (c *SnapController) syncSnap(snap *apis.ZFSSnapshot) error {
 	var err error
 	// ZFSSnapshot should be deleted. Check if deletion timestamp is set
 	if c.isDeletionCandidate(snap) {
-		err = zfs.DestroySnapshot(snap)
-		if err == nil {
-			zfs.RemoveSnapFinalizer(snap)
+		if zfs.CanDestroy(snap.Finalizers) {
+			// destroy only if other finalizers have been removed
+			err = zfs.DestroySnapshot(snap)
+			if err == nil {
+				err = zfs.RemoveSnapFinalizer(snap)
+			}
+		} else {
+			return fmt.Errorf("snapshot: destory failed, waiting for user finalizers to be removed %v", snap.Finalizers)
 		}
 	} else {
-		// if finalizer is not set then it means we are creating
+		// if status is not Ready then it means we are creating
 		// the zfs snapshot.
-		if snap.Finalizers == nil {
+		if snap.Status.State != zfs.ZFSStatusReady {
 			err = zfs.CreateSnapshot(snap)
 			if err == nil {
 				err = zfs.UpdateSnapInfo(snap)

pkg/mgmt/volume/volume.go

@@ -78,15 +78,19 @@ func (c *ZVController) syncZV(zv *apis.ZFSVolume) error {
 	var err error
 	// ZFS Volume should be deleted. Check if deletion timestamp is set
 	if c.isDeletionCandidate(zv) {
-		err = zfs.DestroyVolume(zv)
-		if err == nil {
-			zfs.RemoveZvolFinalizer(zv)
+		if zfs.CanDestroy(zv.Finalizers) {
+			// destroy only if other finalizers have been removed
+			err = zfs.DestroyVolume(zv)
+			if err == nil {
+				err = zfs.RemoveVolumeFinalizer(zv)
+			}
+		} else {
+			return fmt.Errorf("volume: destory failed, waiting for user finalizers to be removed %v", zv.Finalizers)
 		}
 	} else {
-		// if finalizer is not set then it means we are creating
-		// the volume. And if it is set then volume has already been
-		// created and this event is for property change only.
-		if zv.Finalizers != nil {
+		// if volume has already been created and its state is Ready
+		// then this event is for property change only.
+		if zfs.IsVolumeReady(zv) {
 			err = zfs.SetVolumeProp(zv)
 		} else {
 			if len(zv.Spec.SnapName) > 0 {

pkg/zfs/mount.go

@@ -134,7 +134,7 @@ func verifyMountRequest(vol *apis.ZFSVolume, mountpath string) (bool, error) {
 		vol.Spec.OwnerNodeID != NodeID {
 		return false, status.Error(codes.Internal, "verifyMount: volume is owned by different node")
 	}
-	if vol.Finalizers == nil {
+	if !IsVolumeReady(vol) {
 		return false, status.Error(codes.Internal, "verifyMount: volume is not ready to be mounted")
 	}
 

pkg/zfs/volume.go

@@ -233,10 +233,6 @@ func UpdateZvolInfo(vol *apis.ZFSVolume, status string) error {
 	finalizers := []string{}
 	labels := map[string]string{ZFSNodeKey: NodeID}
 
-	if vol.Finalizers != nil {
-		return nil
-	}
-
 	switch status {
 	case ZFSStatusReady:
 		finalizers = append(finalizers, ZFSFinalizer)
@@ -255,8 +251,8 @@ func UpdateZvolInfo(vol *apis.ZFSVolume, status string) error {
 	return err
 }
 
-// RemoveZvolFinalizer adds finalizer to ZFSVolume CR
-func RemoveZvolFinalizer(vol *apis.ZFSVolume) error {
+// RemoveVolumeFinalizer removes finalizer from ZFSVolume CR
+func RemoveVolumeFinalizer(vol *apis.ZFSVolume) error {
 	vol.Finalizers = nil
 
 	_, err := volbuilder.NewKubeclient().WithNamespace(OpenEBSNamespace).Update(vol)
@@ -290,10 +286,6 @@ func UpdateSnapInfo(snap *apis.ZFSSnapshot) error {
 	finalizers := []string{ZFSFinalizer}
 	labels := map[string]string{ZFSNodeKey: NodeID}
 
-	if snap.Finalizers != nil {
-		return nil
-	}
-
 	newSnap, err := snapbuilder.BuildFrom(snap).
 		WithFinalizer(finalizers).
 		WithLabels(labels).Build()
@@ -310,7 +302,7 @@ func UpdateSnapInfo(snap *apis.ZFSSnapshot) error {
 	return err
 }
 
-// RemoveSnapFinalizer adds finalizer to ZFSSnapshot CR
+// RemoveSnapFinalizer removes finalizer from ZFSSnapshot CR
 func RemoveSnapFinalizer(snap *apis.ZFSSnapshot) error {
 	snap.Finalizers = nil
 
@@ -358,3 +350,32 @@ func UpdateRestoreInfo(rstr *apis.ZFSRestore, status apis.ZFSRestoreStatus) erro
 	_, err = restorebuilder.NewKubeclient().WithNamespace(OpenEBSNamespace).Update(newRstr)
 	return err
 }
+
+// CanDestroy checks if there is any other finalizer present
+// apart from the one owns by ZFS node daemonset and returns
+// true if no one else has set any finalizer on it.
+func CanDestroy(finalizers []string) bool {
+	for _, fin := range finalizers {
+		if fin != ZFSFinalizer {
+			return false
+		}
+	}
+	return true
+}
+
+// IsVolumeReady returns true if volume is Ready
+func IsVolumeReady(vol *apis.ZFSVolume) bool {
+
+	if vol.Status.State == ZFSStatusReady {
+		return true
+	}
+
+	// For older volumes, there was no Status field
+	// so checking the node finalizer to make sure volume is Ready
+	for _, fin := range vol.Finalizers {
+		if fin == ZFSFinalizer {
+			return true
+		}
+	}
+	return false
+}