fix(vd): fix update generation in conditions

Signed-off-by: dmitry.lopatin <[email protected]>
deckhouse · Jan 14, 2025 · 4c55bf8 · 4c55bf8
1 parent c93042a
commit 4c55bf8
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 34 deletions.
diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/errors.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/errors.go
@@ -70,7 +70,7 @@ type VirtualDiskNotAllowedForUseError struct {
 }
 
 func (e VirtualDiskNotAllowedForUseError) Error() string {
-	return fmt.Sprintf("use of VirtualDisk %s is not allowed, it may be connected to a running VirtualMachine", e.name)
+	return fmt.Sprintf("the VirtualDisk %s attached to VirtualMachine", e.name)
 }
 
 func NewVirtualDiskNotAllowedForUseError(name string) error {

diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go
@@ -243,7 +243,7 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, cvi *virtv2.Cluster
 	inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions)
 	if inUseCondition.Status == metav1.ConditionTrue &&
 		inUseCondition.Reason == vdcondition.UsedForImageCreation.String() &&
-		inUseCondition.ObservedGeneration == vd.Status.ObservedGeneration {
+		inUseCondition.ObservedGeneration == vd.Generation {
 		return nil
 	}
 

diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go b/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go
@@ -65,7 +65,7 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon
 		inUseCondition = cb.Condition()
 	}
 
-	allowUseForVM, allowUseForImage := false, false
+	usedByVM, usedByImage := false, false
 
 	var vms virtv2.VirtualMachineList
 	err := h.client.List(ctx, &vms, &client.ListOptions{
@@ -78,9 +78,9 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon
 	for _, vm := range vms.Items {
 		if h.isVDAttachedToVM(vd.GetName(), vm) {
 			if vm.Status.Phase != virtv2.MachineStopped {
-				allowUseForVM = isVMCanStart(vm.Status.Conditions)
+				usedByVM = isVMCanStart(vm.Status.Conditions)
 
-				if allowUseForVM {
+				if usedByVM {
 					break
 				}
 			} else {
@@ -90,7 +90,7 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon
 				}
 
 				if kvvm != nil && kvvm.Status.StateChangeRequests != nil {
-					allowUseForVM = true
+					usedByVM = true
 					break
 				}
 
@@ -105,7 +105,7 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon
 
 				for _, pod := range podList.Items {
 					if pod.Status.Phase == corev1.PodRunning {
-						allowUseForVM = true
+						usedByVM = true
 						break
 					}
 				}
@@ -129,7 +129,7 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon
 			vi.Spec.DataSource.ObjectRef != nil &&
 			vi.Spec.DataSource.ObjectRef.Kind == virtv2.VirtualDiskKind &&
 			vi.Spec.DataSource.ObjectRef.Name == vd.Name {
-			allowUseForImage = true
+			usedByImage = true
 			break
 		}
 	}
@@ -145,46 +145,43 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon
 			cvi.Spec.DataSource.ObjectRef != nil &&
 			cvi.Spec.DataSource.ObjectRef.Kind == virtv2.VirtualDiskKind &&
 			cvi.Spec.DataSource.ObjectRef.Name == vd.Name {
-			allowUseForImage = true
+			usedByImage = true
 		}
 	}
 
 	cb := conditions.NewConditionBuilder(vdcondition.InUseType)
 	switch {
-	case allowUseForVM && inUseCondition.Status != metav1.ConditionTrue:
-		if inUseCondition.Reason != vdcondition.AttachedToVirtualMachine.String() {
-			cb.
-				Generation(vd.Generation).
-				Status(metav1.ConditionTrue).
-				Reason(vdcondition.AttachedToVirtualMachine).
-				Message("")
-			conditions.SetCondition(cb, &vd.Status.Conditions)
-			return reconcile.Result{}, nil
-		}
-	case allowUseForImage && inUseCondition.Status != metav1.ConditionTrue:
-		if inUseCondition.Reason != vdcondition.UsedForImageCreation.String() {
-			cb.
-				Generation(vd.Generation).
-				Status(metav1.ConditionTrue).
-				Reason(vdcondition.UsedForImageCreation).
-				Message("")
-			conditions.SetCondition(cb, &vd.Status.Conditions)
-			return reconcile.Result{}, nil
-		}
+	case usedByVM && inUseCondition.Status != metav1.ConditionTrue:
+		cb.
+			Generation(vd.Generation).
+			Status(metav1.ConditionTrue).
+			Reason(vdcondition.AttachedToVirtualMachine).
+			Message("")
+		conditions.SetCondition(cb, &vd.Status.Conditions)
+		return reconcile.Result{}, nil
+	case usedByImage && inUseCondition.Status != metav1.ConditionTrue:
+		cb.
+			Generation(vd.Generation).
+			Status(metav1.ConditionTrue).
+			Reason(vdcondition.UsedForImageCreation).
+			Message("")
+		conditions.SetCondition(cb, &vd.Status.Conditions)
+		return reconcile.Result{}, nil
 	default:
 		setNotInUse := false
 
-		if inUseCondition.Reason == vdcondition.AttachedToVirtualMachine.String() && !allowUseForVM {
+		if inUseCondition.Reason == vdcondition.AttachedToVirtualMachine.String() && !usedByVM {
 			setNotInUse = true
 		}
 
-		if inUseCondition.Reason == vdcondition.UsedForImageCreation.String() && !allowUseForImage {
+		if inUseCondition.Reason == vdcondition.UsedForImageCreation.String() && !usedByImage {
 			setNotInUse = true
 		}
 
 		if setNotInUse {
 			cb.Generation(vd.Generation).Status(metav1.ConditionFalse).Reason(vdcondition.NotInUse).Message("")
 			conditions.SetCondition(cb, &vd.Status.Conditions)
+			// TODO redesign the handler's operation to change the logic for updating the Reason on the fly without intermediate switching to False and remove the requeue.
 			return reconcile.Result{Requeue: true}, nil
 		}
 	}

diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/errors.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/errors.go
@@ -70,7 +70,7 @@ type VirtualDiskNotAllowedForUseError struct {
 }
 
 func (e VirtualDiskNotAllowedForUseError) Error() string {
-	return fmt.Sprintf("use of VirtualDisk %s is not allowed, it may be connected to a running VirtualMachine", e.name)
+	return fmt.Sprintf("the VirtualDisk %s attached to VirtualMachine", e.name)
 }
 
 func NewVirtualDiskNotAllowedForUseError(name string) error {

diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go
@@ -396,7 +396,7 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, vi *virtv2.VirtualI
 	inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions)
 	if inUseCondition.Status == metav1.ConditionTrue &&
 		inUseCondition.Reason == vdcondition.UsedForImageCreation.String() &&
-		inUseCondition.ObservedGeneration == vd.Status.ObservedGeneration {
+		inUseCondition.ObservedGeneration == vd.Generation {
 		return nil
 	}
 

diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go
@@ -172,7 +172,7 @@ func (h *BlockDeviceHandler) Handle(ctx context.Context, s state.VirtualMachineS
 	if !h.areVirtualDisksAllowedToUse(vds) {
 		mgr.Update(cb.Status(metav1.ConditionFalse).
 			Reason(vmcondition.ReasonBlockDevicesNotReady).
-			Message("Virtual disks are not allowed to be used in the virtual machine, check where else they are used.").Condition())
+			Message("Virtual disks cannot be used because it is being used for creating an image.").Condition())
 		changed.Status.Conditions = mgr.Generate()
 		return reconcile.Result{}, nil
 	}