fix(vm): fix blockdevices status and restartawaitingchanges (#183)

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

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

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
+	"slices"
 	"strings"
 	"time"
 
@@ -108,16 +109,27 @@ func (h *BlockDeviceHandler) Handle(ctx context.Context, s state.VirtualMachineS
 	}
 
 	if kvvmi != nil {
+		specDeviceMap := make(map[virtv2.BlockDeviceSpecRef]struct{}, len(changed.Spec.BlockDeviceRefs))
 		for _, ref := range changed.Spec.BlockDeviceRefs {
-			if h.findAttachedBlockDevice(changed, ref) == nil {
-				if abd := h.createAttachedBlockDevice(ref, bdState, kvvmi); abd != nil {
-					changed.Status.BlockDeviceRefs = append(
-						changed.Status.BlockDeviceRefs,
-						*abd,
-					)
-				}
+			specDeviceMap[ref] = struct{}{}
+			idx, bds := h.findAttachedBlockDevice(changed, ref)
+			newBds := h.createAttachedBlockDevice(ref, bdState, kvvmi)
+			if newBds == nil {
+				continue
+			}
+			if bds != nil {
+				changed.Status.BlockDeviceRefs[idx] = *newBds
+				continue
 			}
+			changed.Status.BlockDeviceRefs = append(
+				changed.Status.BlockDeviceRefs,
+				*newBds)
 		}
+
+		changed.Status.BlockDeviceRefs = slices.DeleteFunc(changed.Status.BlockDeviceRefs, func(ref virtv2.BlockDeviceStatusRef) bool {
+			_, ok := specDeviceMap[virtv2.BlockDeviceSpecRef{Kind: ref.Kind, Name: ref.Name}]
+			return !ok && h.findVolumeStatus(GenerateDiskName(ref.Kind, ref.Name), kvvmi) == nil
+		})
 	}
 	countBD := len(current.Spec.BlockDeviceRefs)
 	if ready, count := h.countReadyBlockDevices(current, bdState); !ready {
@@ -255,17 +267,6 @@ func (h *BlockDeviceHandler) updateFinalizers(ctx context.Context, vm *virtv2.Vi
 	return nil
 }
 
-func (h *BlockDeviceHandler) findAttachedBlockDevice(vm *virtv2.VirtualMachine, spec virtv2.BlockDeviceSpecRef) *virtv2.BlockDeviceStatusRef {
-	for i := range vm.Status.BlockDeviceRefs {
-		bda := &vm.Status.BlockDeviceRefs[i]
-		if bda.Kind == spec.Kind && bda.Name == spec.Name {
-			return bda
-		}
-	}
-
-	return nil
-}
-
 func (h *BlockDeviceHandler) createAttachedBlockDevice(spec virtv2.BlockDeviceSpecRef, state BlockDevicesState, kvvmi *virtv1.VirtualMachineInstance) *virtv2.BlockDeviceStatusRef {
 	if kvvmi == nil {
 		return nil
@@ -277,16 +278,16 @@ func (h *BlockDeviceHandler) createAttachedBlockDevice(spec virtv2.BlockDeviceSp
 			return nil
 		}
 
-		vmi, hasVMI := state.VIByName[spec.Name]
-		if !hasVMI {
+		vi, hasVI := state.VIByName[spec.Name]
+		if !hasVI {
 			return nil
 		}
 
 		return &virtv2.BlockDeviceStatusRef{
 			Kind:   virtv2.ImageDevice,
 			Name:   spec.Name,
 			Target: vs.Target,
-			Size:   vmi.Status.Capacity,
+			Size:   vi.Status.Capacity,
 		}
 
 	case virtv2.DiskDevice:
@@ -295,16 +296,16 @@ func (h *BlockDeviceHandler) createAttachedBlockDevice(spec virtv2.BlockDeviceSp
 			return nil
 		}
 
-		vmd, hasVmd := state.VDByName[spec.Name]
-		if !hasVmd {
+		vd, hasVd := state.VDByName[spec.Name]
+		if !hasVd {
 			return nil
 		}
 
 		return &virtv2.BlockDeviceStatusRef{
 			Kind:   virtv2.DiskDevice,
 			Name:   spec.Name,
 			Target: vs.Target,
-			Size:   vmd.Status.Capacity,
+			Size:   vd.Status.Capacity,
 		}
 
 	case virtv2.ClusterImageDevice:
@@ -313,21 +314,32 @@ func (h *BlockDeviceHandler) createAttachedBlockDevice(spec virtv2.BlockDeviceSp
 			return nil
 		}
 
-		cvmi, hasCvmi := state.CVIByName[spec.Name]
-		if !hasCvmi {
+		cvi, hasCvi := state.CVIByName[spec.Name]
+		if !hasCvi {
 			return nil
 		}
 
 		return &virtv2.BlockDeviceStatusRef{
 			Kind:   virtv2.ClusterImageDevice,
 			Name:   spec.Name,
 			Target: vs.Target,
-			Size:   cvmi.Status.Size.Unpacked,
+			Size:   cvi.Status.Size.Unpacked,
 		}
 	}
 	return nil
 }
 
+func (h *BlockDeviceHandler) findAttachedBlockDevice(vm *virtv2.VirtualMachine, spec virtv2.BlockDeviceSpecRef) (int, *virtv2.BlockDeviceStatusRef) {
+	for i := range vm.Status.BlockDeviceRefs {
+		bda := &vm.Status.BlockDeviceRefs[i]
+		if bda.Kind == spec.Kind && bda.Name == spec.Name {
+			return i, bda
+		}
+	}
+
+	return -1, nil
+}
+
 func (h *BlockDeviceHandler) findVolumeStatus(volumeName string, kvvmi *virtv1.VirtualMachineInstance) *virtv1.VolumeStatus {
 	if kvvmi != nil {
 		for i := range kvvmi.Status.VolumeStatus {
@@ -374,3 +386,16 @@ func (s *BlockDevicesState) Reload(ctx context.Context) error {
 	s.VDByName = vdByName
 	return nil
 }
+
+func GenerateDiskName(kind virtv2.BlockDeviceKind, name string) string {
+	switch kind {
+	case virtv2.ImageDevice:
+		return kvbuilder.GenerateVMIDiskName(name)
+	case virtv2.ClusterImageDevice:
+		return kvbuilder.GenerateCVMIDiskName(name)
+	case virtv2.DiskDevice:
+		return kvbuilder.GenerateVMDDiskName(name)
+	default:
+		return ""
+	}
+}

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

@@ -244,7 +244,7 @@ func (h *LifeCycleHandler) syncStats(current, changed *virtv2.VirtualMachine, kv
 			}
 			var empty virtv1.VirtualMachineInstanceGuestOSInfo
 			if kvvmi != nil && empty == current.Status.GuestOSInfo && empty != kvvmi.Status.GuestOSInfo && !pt.Timestamp.IsZero() {
-				launchTimeDuration.GuestOSAgentStarting = &metav1.Duration{Duration: time.Since(pt.Timestamp.Time)}
+				launchTimeDuration.GuestOSAgentStarting = &metav1.Duration{Duration: time.Now().Truncate(1 * time.Second).Sub(pt.Timestamp.Time)}
 			}
 		}
 	}

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

@@ -195,6 +195,7 @@ func (h *SyncKvvmHandler) syncKVVM(ctx context.Context, s state.VirtualMachineSt
 				Reason2(vmcondition.ReasonConfigurationNotApplied).
 				Message(fmt.Sprintf("Failed to apply configuration changes: %s", err.Error()))
 		} else {
+			changed.Status.RestartAwaitingChanges = nil
 			cb.Status(metav1.ConditionTrue).
 				Reason2(vmcondition.ReasonConfigurationApplied)
 		}
@@ -206,7 +207,7 @@ func (h *SyncKvvmHandler) syncKVVM(ctx context.Context, s state.VirtualMachineSt
 			Condition())
 		// Changes are applied, consider current spec as last applied.
 		lastAppliedSpec = &current.Spec
-	case changes != nil:
+	case !changes.IsEmpty():
 		// Delay changes propagation to KVVM until user restarts VM.
 		cb := conditions.NewConditionBuilder2(vmcondition.TypeAwaitingRestartToApplyConfiguration).
 			Generation(current.GetGeneration())
@@ -242,6 +243,7 @@ func (h *SyncKvvmHandler) syncKVVM(ctx context.Context, s state.VirtualMachineSt
 			Status(metav1.ConditionFalse).
 			Reason2(vmcondition.ReasonRestartNoNeed).
 			Condition())
+		changed.Status.RestartAwaitingChanges = nil
 	}
 	changed.Status.Conditions = mgr.Generate()
 	// Ensure power state according to the runPolicy.
@@ -395,10 +397,10 @@ func (h *SyncKvvmHandler) loadLastAppliedSpec(vm *virtv2.VirtualMachine, kvvm *v
 
 // detectSpecChanges compares KVVM generated from current VM spec with in cluster KVVM
 // to calculate changes and action needed to apply these changes.
-func (h *SyncKvvmHandler) detectSpecChanges(kvvm *virtv1.VirtualMachine, currentSpec, lastSpec *virtv2.VirtualMachineSpec) *vmchange.SpecChanges {
+func (h *SyncKvvmHandler) detectSpecChanges(kvvm *virtv1.VirtualMachine, currentSpec, lastSpec *virtv2.VirtualMachineSpec) vmchange.SpecChanges {
 	// Not applicable if KVVM is absent.
 	if kvvm == nil || lastSpec == nil {
-		return nil
+		return vmchange.SpecChanges{}
 	}
 
 	// Compare VM spec applied to the underlying KVVM
@@ -408,14 +410,14 @@ func (h *SyncKvvmHandler) detectSpecChanges(kvvm *virtv1.VirtualMachine, current
 	h.logger.Info(fmt.Sprintf("detected changes: empty %v, disruptive %v, actionType %v", specChanges.IsEmpty(), specChanges.IsDisruptive(), specChanges.ActionType()))
 	h.logger.Info(fmt.Sprintf("detected changes JSON: %s", specChanges.ToJSON()))
 
-	return &specChanges
+	return specChanges
 }
 
 // canApplyChanges returns true if changes can be applied right now.
 //
 // Wait if changes are disruptive, and approval mode is manual, and VM is still running.
-func (h *SyncKvvmHandler) canApplyChanges(vm *virtv2.VirtualMachine, kvvm *virtv1.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, pod *corev1.Pod, changes *vmchange.SpecChanges) bool {
-	if vm == nil || changes == nil {
+func (h *SyncKvvmHandler) canApplyChanges(vm *virtv2.VirtualMachine, kvvm *virtv1.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, pod *corev1.Pod, changes vmchange.SpecChanges) bool {
+	if vm == nil || changes.IsEmpty() {
 		return false
 	}
 	if vmutil.ApprovalMode(vm) == virtv2.Automatic || !changes.IsDisruptive() || kvvmi == nil {
@@ -437,7 +439,7 @@ func (h *SyncKvvmHandler) canApplyChanges(vm *virtv2.VirtualMachine, kvvm *virtv
 }
 
 // applyVMChangesToKVVM applies updates to underlying KVVM based on actions type.
-func (h *SyncKvvmHandler) applyVMChangesToKVVM(ctx context.Context, s state.VirtualMachineState, changes *vmchange.SpecChanges) error {
+func (h *SyncKvvmHandler) applyVMChangesToKVVM(ctx context.Context, s state.VirtualMachineState, changes vmchange.SpecChanges) error {
 	if changes.IsEmpty() || s.VirtualMachine().IsEmpty() {
 		return nil
 	}