fix(vdsnapshot,vmsnapshot): unfreeze virtual machines

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

images/virtualization-artifact/pkg/controller/service/snapshot_service.go

@@ -78,7 +78,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
 	}
@@ -104,10 +104,7 @@ func (s *SnapshotService) CanUnfreeze(ctx context.Context, vdSnapshotName string
 		}
 
 		_, ok := vdByName[vdSnapshot.Spec.VirtualDiskName]
-		if ok &&
-			vdSnapshot.Status.Phase != virtv2.VirtualDiskSnapshotPhaseReady &&
-			vdSnapshot.Status.Phase != virtv2.VirtualDiskSnapshotPhaseFailed &&
-			vdSnapshot.Status.Phase != virtv2.VirtualDiskSnapshotPhaseTerminating {
+		if ok && vdSnapshot.Status.Phase == virtv2.VirtualDiskSnapshotPhaseInProgress {
 			return false, nil
 		}
 	}
@@ -121,10 +118,55 @@ func (s *SnapshotService) CanUnfreeze(ctx context.Context, vdSnapshotName string
 	}
 
 	for _, vmSnapshot := range vmSnapshots.Items {
-		if vmSnapshot.Spec.VirtualMachineName == vm.Name &&
-			vmSnapshot.Status.Phase != virtv2.VirtualMachineSnapshotPhaseReady &&
-			vmSnapshot.Status.Phase != virtv2.VirtualMachineSnapshotPhaseFailed &&
-			vmSnapshot.Status.Phase != virtv2.VirtualMachineSnapshotPhaseTerminating {
+		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
 		}
 	}
@@ -135,7 +177,7 @@ func (s *SnapshotService) CanUnfreeze(ctx context.Context, vdSnapshotName string
 func (s *SnapshotService) Unfreeze(ctx context.Context, name, namespace string) error {
 	err := s.virtClient.VirtualMachines(namespace).Unfreeze(ctx, name)
 	if err != nil {
-		return fmt.Errorf("failed to unfreeze internal virtual machine %s/%s: %w", namespace, name, err)
+		return fmt.Errorf("unfreeze virtual machine %s/%s: %w", namespace, name, err)
 	}
 
 	return nil

images/virtualization-artifact/pkg/controller/vdsnapshot/internal/deletion.go

@@ -53,24 +53,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
 			}
@@ -83,11 +83,8 @@ func (h DeletionHandler) Handle(ctx context.Context, vdSnapshot *virtv2.VirtualD
 			}
 		}
 
-		if requeue {
-			return reconcile.Result{Requeue: true}, nil
-		}
-
 		log.Info("Deletion observed: remove cleanup finalizer from VirtualDiskSnapshot")
+
 		controllerutil.RemoveFinalizer(vdSnapshot, virtv2.FinalizerVDSnapshotCleanup)
 		return reconcile.Result{}, nil
 	}

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)

images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go

@@ -82,15 +82,12 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *virtv2.Virtual
 	switch vdSnapshot.Status.Phase {
 	case "":
 		vdSnapshot.Status.Phase = virtv2.VirtualDiskSnapshotPhasePending
-
 	case virtv2.VirtualDiskSnapshotPhaseFailed:
-		vdSnapshotCondition, _ := conditions.GetCondition(vdscondition.VirtualDiskSnapshotReadyType, vdSnapshot.Status.Conditions)
-
+		readyCondition, _ := conditions.GetCondition(vdscondition.VirtualDiskSnapshotReadyType, vdSnapshot.Status.Conditions)
 		cb.
 			Status(metav1.ConditionFalse).
-			Reason(conditions.CommonReason(vdSnapshotCondition.Reason)).
-			Message(vdSnapshotCondition.Message)
-
+			Reason(conditions.CommonReason(readyCondition.Reason)).
+			Message(readyCondition.Message)
 		return reconcile.Result{}, nil
 	case virtv2.VirtualDiskSnapshotPhaseReady:
 		if vs == nil || vs.Status == nil || vs.Status.ReadyToUse == nil || !*vs.Status.ReadyToUse {
@@ -144,6 +141,15 @@ 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
+					cb.
+						Status(metav1.ConditionFalse).
+						Reason(vdscondition.Snapshotting).
+						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(cb, &vdSnapshot.Status.Phase, err)
@@ -176,6 +182,15 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *virtv2.Virtual
 			}
 		}
 
+		if vdSnapshot.Status.Phase == virtv2.VirtualDiskSnapshotPhasePending {
+			vdSnapshot.Status.Phase = virtv2.VirtualDiskSnapshotPhaseInProgress
+			cb.
+				Status(metav1.ConditionFalse).
+				Reason(vdscondition.Snapshotting).
+				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)
@@ -252,7 +267,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(cb, &vdSnapshot.Status.Phase, err)
 				return reconcile.Result{}, err

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 {

images/virtualization-artifact/pkg/controller/vm/internal/filesystem.go

@@ -18,7 +18,6 @@ package internal
 
 import (
 	"context"
-	"fmt"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -65,7 +64,7 @@ func (h *FilesystemHandler) Handle(ctx context.Context, s state.VirtualMachineSt
 	if kvvmi == nil {
 		cb.Status(metav1.ConditionFalse).
 			Reason(vmcondition.ReasonFilesystemNotReady).
-			Message(fmt.Sprintf("The internal virtual machine %q is not running.", changed.Name))
+			Message("The virtual machine is not running.")
 		return reconcile.Result{}, nil
 	}
 
@@ -78,7 +77,7 @@ func (h *FilesystemHandler) Handle(ctx context.Context, s state.VirtualMachineSt
 	if kvvmi.Status.FSFreezeStatus == "frozen" {
 		cb.Status(metav1.ConditionFalse).
 			Reason(vmcondition.ReasonFilesystemFrozen).
-			Message(fmt.Sprintf("The internal virtual machine %q is frozen.", changed.Name))
+			Message("The virtual machine is frozen.")
 		return reconcile.Result{}, nil
 	}
 

images/virtualization-artifact/pkg/controller/vm/internal/sync_power_state.go

@@ -77,7 +77,7 @@ func (h *SyncPowerStateHandler) syncPowerState(ctx context.Context, s state.Virt
 
 	kvvmi, err := s.KVVMI(ctx)
 	if err != nil {
-		return fmt.Errorf("find the internal virtual machine instance: %w", err)
+		return fmt.Errorf("find the virtual machine instance: %w", err)
 	}
 
 	if runPolicy == virtv2.AlwaysOnUnlessStoppedManually {

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)
 }