diff --git a/pkg/mgmt/snapshot/snapshot.go b/pkg/mgmt/snapshot/snapshot.go index e793f7c..7378d1a 100644 --- a/pkg/mgmt/snapshot/snapshot.go +++ b/pkg/mgmt/snapshot/snapshot.go @@ -77,14 +77,20 @@ 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) + userFin := zfs.GetUserFinalizers(snap.Finalizers) + if len(userFin) == 0 { + // destroy only if other finalizers have been removed + err = zfs.DestroySnapshot(snap) + if err == nil { + err = zfs.RemoveSnapFinalizer(snap) + } + } else { + return fmt.Errorf("snapshot: can not destroy, waiting for finalizers to be removed %v", userFin) } } 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) diff --git a/pkg/mgmt/volume/volume.go b/pkg/mgmt/volume/volume.go index 0edc219..997812b 100644 --- a/pkg/mgmt/volume/volume.go +++ b/pkg/mgmt/volume/volume.go @@ -78,15 +78,20 @@ 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) + userFin := zfs.GetUserFinalizers(zv.Finalizers) + if len(userFin) == 0 { + // destroy only if other finalizers have been removed + err = zfs.DestroyVolume(zv) + if err == nil { + err = zfs.RemoveVolumeFinalizer(zv) + } + } else { + return fmt.Errorf("volume: can not destroy, waiting for finalizers to be removed %v", userFin) } } 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 { diff --git a/pkg/zfs/mount.go b/pkg/zfs/mount.go index 943ba38..9aee842 100644 --- a/pkg/zfs/mount.go +++ b/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") } diff --git a/pkg/zfs/volume.go b/pkg/zfs/volume.go index 27ec350..dcca9d7 100644 --- a/pkg/zfs/volume.go +++ b/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,34 @@ func UpdateRestoreInfo(rstr *apis.ZFSRestore, status apis.ZFSRestoreStatus) erro _, err = restorebuilder.NewKubeclient().WithNamespace(OpenEBSNamespace).Update(newRstr) return err } + +// GetUserFinalizers returns all the finalizers present on the ZFSVolume object +// execpt the one owned by ZFS node daemonset. We also need to ignore the foregroundDeletion +// finalizer as this will be present becasue of the foreground cascading deletion +func GetUserFinalizers(finalizers []string) []string { + var userFin []string + for _, fin := range finalizers { + if fin != ZFSFinalizer && + fin != "foregroundDeletion" { + userFin = append(userFin, fin) + } + } + return userFin +} + +// 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 +}