m

Signed-off-by: Valeriy Khorunzhin <[email protected]>

Author: Valeriy Khorunzhin

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

@@ -125,12 +125,22 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtu
 	}
 
 	virtualDiskReadyCondition, _ := conditions.GetCondition(vdscondition.VirtualDiskReadyType, vdSnapshot.Status.Conditions)
-	if vd == nil || virtualDiskReadyCondition.Status != metav1.ConditionTrue {
-		vdSnapshot.Status.Phase = v1alpha2.VirtualDiskSnapshotPhasePending
+	if vd == nil {
+		vdSnapshot.Status.Phase = v1alpha2.VirtualDiskSnapshotPhaseFailed
+		msg := fmt.Sprintf("Cannot take a snapshot right now: the virtual disk %q does not exist.", vdSnapshot.Spec.VirtualDiskName)
+		cb.
+			Status(metav1.ConditionFalse).
+			Reason(vdscondition.WaitingForTheVirtualDisk).
+			Message(msg)
+		return reconcile.Result{}, nil
+	}
+	if virtualDiskReadyCondition.Status != metav1.ConditionTrue {
+		vdSnapshot.Status.Phase = v1alpha2.VirtualDiskSnapshotPhaseFailed
+		msg := fmt.Sprintf("Cannot take a snapshot right now: the virtual disk %q is not ready.", vdSnapshot.Spec.VirtualDiskName)
 		cb.
 			Status(metav1.ConditionFalse).
 			Reason(vdscondition.WaitingForTheVirtualDisk).
-			Message(fmt.Sprintf("Waiting for the virtual disk %q to be ready for snapshotting.", vdSnapshot.Spec.VirtualDiskName))
+			Message(msg)
 		return reconcile.Result{}, nil
 	}
 
@@ -143,12 +153,22 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtu
 		}
 	}
 
-	if pvc == nil || pvc.Status.Phase != corev1.ClaimBound {
-		vdSnapshot.Status.Phase = v1alpha2.VirtualDiskSnapshotPhasePending
+	if pvc == nil {
+		vdSnapshot.Status.Phase = v1alpha2.VirtualDiskSnapshotPhaseFailed
+		msg := fmt.Sprintf("Cannot take a snapshot right now: the persistent volume claim for virtual disk %q does not exist.", vdSnapshot.Spec.VirtualDiskName)
+		cb.
+			Status(metav1.ConditionFalse).
+			Reason(vdscondition.WaitingForTheVirtualDisk).
+			Message(msg)
+		return reconcile.Result{}, nil
+	}
+	if pvc.Status.Phase != corev1.ClaimBound {
+		vdSnapshot.Status.Phase = v1alpha2.VirtualDiskSnapshotPhaseFailed
+		msg := fmt.Sprintf("Cannot take a snapshot right now: the persistent volume claim %q for virtual disk %q is not in phase %s.", pvc.Name, vdSnapshot.Spec.VirtualDiskName, corev1.ClaimBound)
 		cb.
 			Status(metav1.ConditionFalse).
 			Reason(vdscondition.WaitingForTheVirtualDisk).
-			Message("Waiting for the virtual disk's pvc to be in phase Bound.")
+			Message(msg)
 		return reconcile.Result{}, nil
 	}
 
@@ -243,27 +263,21 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtu
 			}
 
 			if vdSnapshot.Spec.RequiredConsistency {
-				vdSnapshot.Status.Phase = v1alpha2.VirtualDiskSnapshotPhasePending
+				vdSnapshot.Status.Phase = v1alpha2.VirtualDiskSnapshotPhaseFailed
 				cb.
 					Status(metav1.ConditionFalse).
 					Reason(vdscondition.PotentiallyInconsistent)
 
 				agentReadyCondition, _ := conditions.GetCondition(vmcondition.TypeAgentReady, vm.Status.Conditions)
-				switch {
-				case agentReadyCondition.Status != metav1.ConditionTrue:
+				if agentReadyCondition.Status != metav1.ConditionTrue {
 					cb.Message(fmt.Sprintf(
-						"The virtual machine %q with an attached virtual disk %q is %s: "+
-							"the snapshotting of virtual disk might result in an inconsistent snapshot: "+
-							"virtual machine agent is not ready and virtual machine cannot be frozen: "+
-							"waiting for virtual machine agent to be ready, or virtual machine will stop",
-						vm.Name, vd.Name, vm.Status.Phase,
+						"Cannot take a consistent snapshot of virtual disk %q attached to virtual machine %q: virtual machine agent is not ready and the virtual machine cannot be frozen.",
+						vd.Name, vm.Name,
 					))
-				default:
+				} else {
 					cb.Message(fmt.Sprintf(
-						"The virtual machine %q with an attached virtual disk %q is %s: "+
-							"the snapshotting of virtual disk might result in an inconsistent snapshot: "+
-							"waiting for the virtual machine to be %s or the disk to be detached",
-						vm.Name, vd.Name, vm.Status.Phase, v1alpha2.MachineStopped,
+						"Cannot take a consistent snapshot of virtual disk %q attached to virtual machine %q.",
+						vd.Name, vm.Name,
 					))
 				}
 

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

@@ -174,6 +174,36 @@ var _ = Describe("LifeCycle handler", func() {
 			Expect(ready.Reason).To(Equal(vdscondition.VirtualDiskSnapshotReady.String()))
 			Expect(ready.Message).To(BeEmpty())
 		})
+
+		It("fails when the virtual disk is missing", func() {
+			snapshotter.GetVirtualDiskFunc = func(_ context.Context, _, _ string) (*v1alpha2.VirtualDisk, error) {
+				return nil, nil
+			}
+
+			h := NewLifeCycleHandler(snapshotter)
+
+			_, err := h.Handle(testContext(), vdSnapshot)
+			Expect(err).To(BeNil())
+			Expect(vdSnapshot.Status.Phase).To(Equal(v1alpha2.VirtualDiskSnapshotPhaseFailed))
+			ready, _ := conditions.GetCondition(vdscondition.VirtualDiskSnapshotReadyType, vdSnapshot.Status.Conditions)
+			Expect(ready.Status).To(Equal(metav1.ConditionFalse))
+			Expect(ready.Reason).To(Equal(vdscondition.WaitingForTheVirtualDisk.String()))
+			Expect(ready.Message).To(ContainSubstring("does not exist"))
+		})
+
+		It("fails when the virtual disk pvc is not Bound", func() {
+			pvc.Status.Phase = corev1.ClaimPending
+
+			h := NewLifeCycleHandler(snapshotter)
+
+			_, err := h.Handle(testContext(), vdSnapshot)
+			Expect(err).To(BeNil())
+			Expect(vdSnapshot.Status.Phase).To(Equal(v1alpha2.VirtualDiskSnapshotPhaseFailed))
+			ready, _ := conditions.GetCondition(vdscondition.VirtualDiskSnapshotReadyType, vdSnapshot.Status.Conditions)
+			Expect(ready.Status).To(Equal(metav1.ConditionFalse))
+			Expect(ready.Reason).To(Equal(vdscondition.WaitingForTheVirtualDisk.String()))
+			Expect(ready.Message).To(ContainSubstring("not in phase"))
+		})
 	})
 
 	Context("The virtual disk snapshot with virtual machine", func() {
@@ -258,11 +288,11 @@ var _ = Describe("LifeCycle handler", func() {
 
 			_, err := h.Handle(testContext(), vdSnapshot)
 			Expect(err).To(BeNil())
-			Expect(vdSnapshot.Status.Phase).To(Equal(v1alpha2.VirtualDiskSnapshotPhasePending))
+			Expect(vdSnapshot.Status.Phase).To(Equal(v1alpha2.VirtualDiskSnapshotPhaseFailed))
 			ready, _ := conditions.GetCondition(vdscondition.VirtualDiskSnapshotReadyType, vdSnapshot.Status.Conditions)
 			Expect(ready.Status).To(Equal(metav1.ConditionFalse))
 			Expect(ready.Reason).To(Equal(vdscondition.PotentiallyInconsistent.String()))
-			Expect(ready.Message).ToNot(BeEmpty())
+			Expect(ready.Message).To(ContainSubstring("Cannot take a consistent snapshot"))
 		})
 
 		It("Cannot freeze virtual machine: allow potentially inconsistent", func() {

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

@@ -200,13 +200,28 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmSnapshot *v1alpha2.Virtu
 	}
 
 	virtualMachineReadyCondition, _ := conditions.GetCondition(vmscondition.VirtualMachineReadyType, vmSnapshot.Status.Conditions)
-	if vm == nil || virtualMachineReadyCondition.Status != metav1.ConditionTrue {
-		vmSnapshot.Status.Phase = v1alpha2.VirtualMachineSnapshotPhasePending
-		msg := fmt.Sprintf("Waiting for the virtual machine %q to be ready for snapshotting.", vmSnapshot.Spec.VirtualMachineName)
+	if vm == nil {
+		vmSnapshot.Status.Phase = v1alpha2.VirtualMachineSnapshotPhaseFailed
+		msg := fmt.Sprintf("Cannot take a snapshot right now: the virtual machine %q does not exist.", vmSnapshot.Spec.VirtualMachineName)
 		h.recorder.Event(
 			vmSnapshot,
-			corev1.EventTypeNormal,
-			v1alpha2.ReasonVMSnapshottingPending,
+			corev1.EventTypeWarning,
+			v1alpha2.ReasonVMSnapshottingFailed,
+			msg,
+		)
+		cb.
+			Status(metav1.ConditionFalse).
+			Reason(vmscondition.WaitingForTheVirtualMachine).
+			Message(msg)
+		return reconcile.Result{}, nil
+	}
+	if virtualMachineReadyCondition.Status != metav1.ConditionTrue {
+		vmSnapshot.Status.Phase = v1alpha2.VirtualMachineSnapshotPhaseFailed
+		msg := fmt.Sprintf("Cannot take a snapshot right now: the virtual machine %q is not ready.", vmSnapshot.Spec.VirtualMachineName)
+		h.recorder.Event(
+			vmSnapshot,
+			corev1.EventTypeWarning,
+			v1alpha2.ReasonVMSnapshottingFailed,
 			msg,
 		)
 		cb.
@@ -251,27 +266,24 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmSnapshot *v1alpha2.Virtu
 	}
 	isAwaitingConsistency := needToFreeze && !canFreeze && vmSnapshot.Spec.RequiredConsistency
 	if isAwaitingConsistency {
-		vmSnapshot.Status.Phase = v1alpha2.VirtualMachineSnapshotPhasePending
+		vmSnapshot.Status.Phase = v1alpha2.VirtualMachineSnapshotPhaseFailed
 		msg := fmt.Sprintf(
-			"The snapshotting of virtual machine %q might result in an inconsistent snapshot: "+
-				"waiting for the virtual machine to be %s",
-			vm.Name, v1alpha2.MachineStopped,
+			"Cannot take a consistent snapshot of virtual machine %q.",
+			vm.Name,
 		)
 
 		agentReadyCondition, _ := conditions.GetCondition(vmcondition.TypeAgentReady, vm.Status.Conditions)
 		if agentReadyCondition.Status != metav1.ConditionTrue {
 			msg = fmt.Sprintf(
-				"The snapshotting of virtual machine %q might result in an inconsistent snapshot: "+
-					"virtual machine agent is not ready and virtual machine cannot be frozen: "+
-					"waiting for virtual machine agent to be ready or virtual machine will stop",
+				"Cannot take a consistent snapshot of virtual machine %q: virtual machine agent is not ready and the virtual machine cannot be frozen.",
 				vm.Name,
 			)
 		}
 
 		h.recorder.Event(
 			vmSnapshot,
-			corev1.EventTypeNormal,
-			v1alpha2.ReasonVMSnapshottingPending,
+			corev1.EventTypeWarning,
+			v1alpha2.ReasonVMSnapshottingFailed,
 			msg,
 		)
 		cb.

images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go

@@ -168,6 +168,41 @@ var _ = Describe("LifeCycle handler", func() {
 		}
 	})
 
+	Context("The virtual machine is not ready", func() {
+		It("fails when the virtual machine is missing", func() {
+			snapshotter.GetVirtualMachineFunc = func(_ context.Context, _, _ string) (*v1alpha2.VirtualMachine, error) {
+				return nil, nil
+			}
+
+			h := NewLifeCycleHandler(recorder, snapshotter, storer, fakeClient)
+
+			_, err := h.Handle(testContext(), vmSnapshot)
+			Expect(err).To(BeNil())
+			Expect(vmSnapshot.Status.Phase).To(Equal(v1alpha2.VirtualMachineSnapshotPhaseFailed))
+			ready, _ := conditions.GetCondition(vmscondition.VirtualMachineSnapshotReadyType, vmSnapshot.Status.Conditions)
+			Expect(ready.Status).To(Equal(metav1.ConditionFalse))
+			Expect(ready.Reason).To(Equal(vmscondition.WaitingForTheVirtualMachine.String()))
+			Expect(ready.Message).To(ContainSubstring("does not exist"))
+		})
+
+		It("fails when the virtual machine is reported as not ready", func() {
+			cb := conditions.NewConditionBuilder(vmscondition.VirtualMachineReadyType).
+				Generation(vmSnapshot.Generation).
+				Status(metav1.ConditionFalse)
+			conditions.SetCondition(cb, &vmSnapshot.Status.Conditions)
+
+			h := NewLifeCycleHandler(recorder, snapshotter, storer, fakeClient)
+
+			_, err := h.Handle(testContext(), vmSnapshot)
+			Expect(err).To(BeNil())
+			Expect(vmSnapshot.Status.Phase).To(Equal(v1alpha2.VirtualMachineSnapshotPhaseFailed))
+			ready, _ := conditions.GetCondition(vmscondition.VirtualMachineSnapshotReadyType, vmSnapshot.Status.Conditions)
+			Expect(ready.Status).To(Equal(metav1.ConditionFalse))
+			Expect(ready.Reason).To(Equal(vmscondition.WaitingForTheVirtualMachine.String()))
+			Expect(ready.Message).To(ContainSubstring("not ready"))
+		})
+	})
+
 	Context("The block devices of the virtual machine are not in the consistent state", func() {
 		It("The BlockDevicesReady condition of the virtual machine isn't True", func() {
 			snapshotter.GetVirtualMachineFunc = func(_ context.Context, _, _ string) (*v1alpha2.VirtualMachine, error) {
@@ -274,7 +309,7 @@ var _ = Describe("LifeCycle handler", func() {
 
 			_, err := h.Handle(testContext(), vmSnapshot)
 			Expect(err).To(BeNil())
-			Expect(vmSnapshot.Status.Phase).To(Equal(v1alpha2.VirtualMachineSnapshotPhasePending))
+			Expect(vmSnapshot.Status.Phase).To(Equal(v1alpha2.VirtualMachineSnapshotPhaseFailed))
 			ready, _ := conditions.GetCondition(vmscondition.VirtualMachineSnapshotReadyType, vmSnapshot.Status.Conditions)
 			Expect(ready.Status).To(Equal(metav1.ConditionFalse))
 			Expect(ready.Reason).To(Equal(vmscondition.PotentiallyInconsistent.String()))