fix(vdsnapshot,vmsnapshot): unfreeze virtual machines

Fix unfreezing of virtual machines Fix VolumeSnapshot deletion Improve processing of concurrent snapshots Signed-off-by: Isteb4k <[email protected]>
deckhouse · Dec 19, 2024 · e5276a3 · e5276a3
1 parent 7496c66
commit e5276a3
Show file tree

Hide file tree

Showing 10 changed files with 243 additions and 136 deletions.
diff --git a/images/virtualization-artifact/pkg/controller/service/snapshot_service.go b/images/virtualization-artifact/pkg/controller/service/snapshot_service.go
@@ -77,7 +77,7 @@ func (s *SnapshotService) Freeze(ctx context.Context, name, namespace string) er
 	return nil
 }
 
-func (s *SnapshotService) CanUnfreeze(ctx context.Context, vdSnapshotName string, vm *virtv2.VirtualMachine) (bool, error) {
+func (s *SnapshotService) CanUnfreezeWithVirtualDiskSnapshot(ctx context.Context, vdSnapshotName string, vm *virtv2.VirtualMachine) (bool, error) {
 	if vm == nil || !s.IsFrozen(vm) {
 		return false, nil
 	}
@@ -103,7 +103,7 @@ func (s *SnapshotService) CanUnfreeze(ctx context.Context, vdSnapshotName string
 		}
 
 		_, ok := vdByName[vdSnapshot.Spec.VirtualDiskName]
-		if ok {
+		if ok && vdSnapshot.Status.Phase == virtv2.VirtualDiskSnapshotPhaseInProgress {
 			return false, nil
 		}
 	}
@@ -117,7 +117,55 @@ func (s *SnapshotService) CanUnfreeze(ctx context.Context, vdSnapshotName string
 	}
 
 	for _, vmSnapshot := range vmSnapshots.Items {
-		if vmSnapshot.Spec.VirtualMachineName == vm.Name {
+		if vmSnapshot.Spec.VirtualMachineName == vm.Name && vmSnapshot.Status.Phase == virtv2.VirtualMachineSnapshotPhaseInProgress {
+			return false, nil
+		}
+	}
+
+	return true, nil
+}
+
+func (s *SnapshotService) CanUnfreezeWithVirtualMachineSnapshot(ctx context.Context, vmSnapshotName string, vm *virtv2.VirtualMachine) (bool, error) {
+	if vm == nil || !s.IsFrozen(vm) {
+		return false, nil
+	}
+
+	vdByName := make(map[string]struct{})
+	for _, bdr := range vm.Status.BlockDeviceRefs {
+		if bdr.Kind == virtv2.DiskDevice {
+			vdByName[bdr.Name] = struct{}{}
+		}
+	}
+
+	var vdSnapshots virtv2.VirtualDiskSnapshotList
+	err := s.client.List(ctx, &vdSnapshots, &client.ListOptions{
+		Namespace: vm.Namespace,
+	})
+	if err != nil {
+		return false, err
+	}
+
+	for _, vdSnapshot := range vdSnapshots.Items {
+		_, ok := vdByName[vdSnapshot.Spec.VirtualDiskName]
+		if ok && vdSnapshot.Status.Phase == virtv2.VirtualDiskSnapshotPhaseInProgress {
+			return false, nil
+		}
+	}
+
+	var vmSnapshots virtv2.VirtualMachineSnapshotList
+	err = s.client.List(ctx, &vmSnapshots, &client.ListOptions{
+		Namespace: vm.Namespace,
+	})
+	if err != nil {
+		return false, err
+	}
+
+	for _, vmSnapshot := range vmSnapshots.Items {
+		if vmSnapshot.Name == vmSnapshotName {
+			continue
+		}
+
+		if vmSnapshot.Spec.VirtualMachineName == vm.Name && vmSnapshot.Status.Phase == virtv2.VirtualMachineSnapshotPhaseInProgress {
 			return false, nil
 		}
 	}

diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/deletion.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/deletion.go
@@ -38,7 +38,7 @@ func NewDeletionHandler(snapshotter *service.SnapshotService) *DeletionHandler {
 
 func (h DeletionHandler) Handle(ctx context.Context, vdSnapshot *virtv2.VirtualDiskSnapshot) (reconcile.Result, error) {
 	if vdSnapshot.DeletionTimestamp != nil {
-		vs, err := h.snapshotter.GetVolumeSnapshot(ctx, vdSnapshot.Name, vdSnapshot.Namespace)
+		vs, err := h.snapshotter.GetVolumeSnapshot(ctx, vdSnapshot.Status.VolumeSnapshotName, vdSnapshot.Namespace)
 		if err != nil {
 			return reconcile.Result{}, err
 		}
@@ -48,24 +48,24 @@ func (h DeletionHandler) Handle(ctx context.Context, vdSnapshot *virtv2.VirtualD
 			return reconcile.Result{}, err
 		}
 
-		vm, err := getVirtualMachine(ctx, vd, h.snapshotter)
-		if err != nil {
-			return reconcile.Result{}, err
+		var vm *virtv2.VirtualMachine
+		if vd != nil {
+			vm, err = getVirtualMachine(ctx, vd, h.snapshotter)
+			if err != nil {
+				return reconcile.Result{}, err
+			}
 		}
 
-		var requeue bool
-
 		if vs != nil {
 			err = h.snapshotter.DeleteVolumeSnapshot(ctx, vs)
 			if err != nil {
 				return reconcile.Result{}, err
 			}
-			requeue = true
 		}
 
 		if vm != nil {
 			var canUnfreeze bool
-			canUnfreeze, err = h.snapshotter.CanUnfreeze(ctx, vdSnapshot.Name, vm)
+			canUnfreeze, err = h.snapshotter.CanUnfreezeWithVirtualDiskSnapshot(ctx, vdSnapshot.Name, vm)
 			if err != nil {
 				return reconcile.Result{}, err
 			}
@@ -78,10 +78,6 @@ func (h DeletionHandler) Handle(ctx context.Context, vdSnapshot *virtv2.VirtualD
 			}
 		}
 
-		if requeue {
-			return reconcile.Result{Requeue: true}, nil
-		}
-
 		controllerutil.RemoveFinalizer(vdSnapshot, virtv2.FinalizerVDSnapshotCleanup)
 		return reconcile.Result{}, nil
 	}

diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/interfaces.go
@@ -35,7 +35,7 @@ type LifeCycleSnapshotter interface {
 	Freeze(ctx context.Context, name, namespace string) error
 	IsFrozen(vm *virtv2.VirtualMachine) bool
 	CanFreeze(vm *virtv2.VirtualMachine) bool
-	CanUnfreeze(ctx context.Context, vdSnapshotName string, vm *virtv2.VirtualMachine) (bool, error)
+	CanUnfreezeWithVirtualDiskSnapshot(ctx context.Context, vdSnapshotName string, vm *virtv2.VirtualMachine) (bool, error)
 	Unfreeze(ctx context.Context, name, namespace string) error
 	CreateVolumeSnapshot(ctx context.Context, vs *vsv1.VolumeSnapshot) (*vsv1.VolumeSnapshot, error)
 	GetPersistentVolumeClaim(ctx context.Context, name, namespace string) (*corev1.PersistentVolumeClaim, error)

diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go
@@ -86,6 +86,8 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *virtv2.Virtual
 	switch vdSnapshot.Status.Phase {
 	case "":
 		vdSnapshot.Status.Phase = virtv2.VirtualDiskSnapshotPhasePending
+	case virtv2.VirtualDiskSnapshotPhaseFailed:
+		return reconcile.Result{}, nil
 	case virtv2.VirtualDiskSnapshotPhaseReady:
 		if vs == nil || vs.Status == nil || vs.Status.ReadyToUse == nil || !*vs.Status.ReadyToUse {
 			vdSnapshot.Status.Phase = virtv2.VirtualDiskSnapshotPhaseFailed
@@ -134,6 +136,14 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *virtv2.Virtual
 			if h.snapshotter.CanFreeze(vm) {
 				log.Debug("Freeze the virtual machine to take a snapshot")
 
+				if vdSnapshot.Status.Phase == virtv2.VirtualDiskSnapshotPhasePending {
+					vdSnapshot.Status.Phase = virtv2.VirtualDiskSnapshotPhaseInProgress
+					condition.Status = metav1.ConditionFalse
+					condition.Reason = vdscondition.Snapshotting
+					condition.Message = "The snapshotting process has started."
+					return reconcile.Result{Requeue: true}, nil
+				}
+
 				err = h.snapshotter.Freeze(ctx, vm.Name, vm.Namespace)
 				if err != nil {
 					setPhaseConditionToFailed(&condition, &vdSnapshot.Status.Phase, err)
@@ -164,6 +174,14 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *virtv2.Virtual
 			}
 		}
 
+		if vdSnapshot.Status.Phase == virtv2.VirtualDiskSnapshotPhasePending {
+			vdSnapshot.Status.Phase = virtv2.VirtualDiskSnapshotPhaseInProgress
+			condition.Status = metav1.ConditionFalse
+			condition.Reason = vdscondition.Snapshotting
+			condition.Message = "The snapshotting process has started."
+			return reconcile.Result{Requeue: true}, nil
+		}
+
 		log.Debug("The corresponding volume snapshot not found: create the new one")
 
 		anno := make(map[string]string)
@@ -237,7 +255,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *virtv2.Virtual
 			vdSnapshot.Status.Consistent = ptr.To(true)
 
 			var canUnfreeze bool
-			canUnfreeze, err = h.snapshotter.CanUnfreeze(ctx, vdSnapshot.Name, vm)
+			canUnfreeze, err = h.snapshotter.CanUnfreezeWithVirtualDiskSnapshot(ctx, vdSnapshot.Name, vm)
 			if err != nil {
 				setPhaseConditionToFailed(&condition, &vdSnapshot.Status.Phase, err)
 				return reconcile.Result{}, err

diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle_test.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle_test.go
@@ -165,6 +165,8 @@ var _ = Describe("LifeCycle handler", func() {
 		var vm *virtv2.VirtualMachine
 
 		BeforeEach(func() {
+			vdSnapshot.Status.Phase = virtv2.VirtualDiskSnapshotPhaseInProgress
+
 			vm = &virtv2.VirtualMachine{
 				ObjectMeta: metav1.ObjectMeta{Name: "vm"},
 				Status: virtv2.VirtualMachineStatus{
@@ -185,7 +187,7 @@ var _ = Describe("LifeCycle handler", func() {
 			snapshotter.FreezeFunc = func(_ context.Context, _, _ string) error {
 				return nil
 			}
-			snapshotter.CanUnfreezeFunc = func(_ context.Context, _ string, _ *virtv2.VirtualMachine) (bool, error) {
+			snapshotter.CanUnfreezeWithVirtualDiskSnapshotFunc = func(_ context.Context, _ string, _ *virtv2.VirtualMachine) (bool, error) {
 				return true, nil
 			}
 			snapshotter.UnfreezeFunc = func(_ context.Context, _, _ string) error {

diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/mock.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/mock.go
diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go
@@ -41,5 +41,5 @@ type Snapshotter interface {
 	Unfreeze(ctx context.Context, name, namespace string) error
 	IsFrozen(vm *virtv2.VirtualMachine) bool
 	CanFreeze(vm *virtv2.VirtualMachine) bool
-	CanUnfreeze(ctx context.Context, vdSnapshotName string, vm *virtv2.VirtualMachine) (bool, error)
+	CanUnfreezeWithVirtualMachineSnapshot(ctx context.Context, vmSnapshotName string, vm *virtv2.VirtualMachine) (bool, error)
 }