feat(vm): add specchanges for vmclass

api/core/v1alpha2/events.go

@@ -28,6 +28,9 @@ const (
 	// ReasonVMLastAppliedSpecInvalid is event reason that JSON in last-applied-spec annotation is invalid.
 	ReasonVMLastAppliedSpecInvalid = "VMLastAppliedSpecInvalid"
 
+	// ReasonVMClassLastAppliedSpecInvalid is event reason that JSON in last-applied-spec annotation is invalid.
+	ReasonVMClassLastAppliedSpecInvalid = "VMClassLastAppliedSpecInvalid"
+
 	// ReasonErrVmNotSynced is event reason that vm is not synced.
 	ReasonErrVmNotSynced = "VirtualMachineNotSynced"
 

api/core/v1alpha2/virtual_machine_class.go

@@ -208,7 +208,7 @@ type VirtualMachineClassStatus struct {
 	// +kubebuilder:example={"maxAllocatableResources: {\"cpu\": 1, \"memory\": \"10Gi\"}"}
 	MaxAllocatableResources corev1.ResourceList `json:"maxAllocatableResources,omitempty"`
 	// The latest detailed observations of the VirtualMachineClass resource.
-	Conditions              []metav1.Condition  `json:"conditions,omitempty"`
+	Conditions []metav1.Condition `json:"conditions,omitempty"`
 	// The generation last processed by the controller.
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 }

api/core/v1alpha2/vmcondition/condition.go

@@ -25,6 +25,7 @@ func (t Type) String() string {
 const (
 	TypeIPAddressReady                      Type = "VirtualMachineIPAddressReady"
 	TypeClassReady                          Type = "VirtualMachineClassReady"
+	TypeClassChanged                        Type = "VirtualMachineClassChanged"
 	TypeBlockDevicesReady                   Type = "BlockDevicesReady"
 	TypeRunning                             Type = "Running"
 	TypeMigrating                           Type = "Migrating"
@@ -56,6 +57,7 @@ const (
 
 	ReasonClassReady    Reason = "VirtualMachineClassReady"
 	ReasonClassNotReady Reason = "VirtualMachineClassNotReady"
+	ReasonClassChanged  Reason = "VirtualMachineClassChanged"
 
 	ReasonIPAddressReady        Reason = "VirtualMachineIPAddressReady"
 	ReasonIPAddressNotReady     Reason = "VirtualMachineIPAddressNotReady"

images/virtualization-artifact/pkg/common/annotations/annotations.go

@@ -53,6 +53,9 @@ const (
 	// AnnVMLastAppliedSpec is an annotation on KVVM. It contains a JSON with VM spec.
 	AnnVMLastAppliedSpec = AnnAPIGroup + "/vm.last-applied-spec"
 
+	// AnnVMClassLastAppliedSpec is an annotation on KVVM. It contains a JSON with VM spec.
+	AnnVMClassLastAppliedSpec = AnnAPIGroup + "/vmclass.last-applied-spec"
+
 	// LastPropagatedVMAnnotationsAnnotation is a marshalled map of previously applied virtual machine annotations.
 	LastPropagatedVMAnnotationsAnnotation = AnnAPIGroup + "/last-propagated-vm-annotations"
 	// LastPropagatedVMLabelsAnnotation is a marshalled map of previously applied virtual machine labels.

images/virtualization-artifact/pkg/controller/conditions/builder.go

@@ -39,6 +39,10 @@ func SetCondition(c Conder, conditions *[]metav1.Condition) {
 	meta.SetStatusCondition(conditions, c.Condition())
 }
 
+func RemoveCondition(conditionType Stringer, conditions *[]metav1.Condition) {
+	meta.RemoveStatusCondition(conditions, conditionType.String())
+}
+
 func GetCondition(condType Stringer, conditions []metav1.Condition) (metav1.Condition, bool) {
 	for _, condition := range conditions {
 		if condition.Type == condType.String() {

images/virtualization-artifact/pkg/controller/kvbuilder/last_applied_spec.go

@@ -52,3 +52,29 @@ func SetLastAppliedSpec(kvvm *virtv1.VirtualMachine, vm *v1alpha2.VirtualMachine
 	annotations.AddAnnotation(kvvm, annotations.AnnVMLastAppliedSpec, string(lastApplied))
 	return nil
 }
+
+// LoadLastAppliedClassSpec loads VMClass spec from JSON in the last-applied-spec annotation.
+func LoadLastAppliedClassSpec(kvvm *virtv1.VirtualMachine) (*v1alpha2.VirtualMachineClassSpec, error) {
+	lastSpecJSON := kvvm.GetAnnotations()[common.AnnVMClassLastAppliedSpec]
+	if strings.TrimSpace(lastSpecJSON) == "" {
+		return nil, nil
+	}
+
+	var spec v1alpha2.VirtualMachineClassSpec
+	err := json.Unmarshal([]byte(lastSpecJSON), &spec)
+	if err != nil {
+		return nil, fmt.Errorf("load spec from JSON: %w", err)
+	}
+	return &spec, nil
+}
+
+// SetLastAppliedClassSpec updates the last-applied-spec annotation with VMClass spec JSON.
+func SetLastAppliedClassSpec(kvvm *virtv1.VirtualMachine, class *v1alpha2.VirtualMachineClass) error {
+	lastApplied, err := json.Marshal(class.Spec)
+	if err != nil {
+		return fmt.Errorf("convert spec to JSON: %w", err)
+	}
+
+	common.AddAnnotation(kvvm, common.AnnVMClassLastAppliedSpec, string(lastApplied))
+	return nil
+}

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

@@ -99,13 +99,37 @@ func (h *SyncKvvmHandler) Handle(ctx context.Context, s state.VirtualMachineStat
 			Message(service.CapitalizeFirstLetter(err.Error()) + ".")
 		return reconcile.Result{}, err
 	}
+	class, err := s.Class(ctx)
+	if err != nil {
+		return reconcile.Result{}, err
+	}
 
 	// 1. Set RestartAwaitingChanges.
-	var lastAppliedSpec *virtv2.VirtualMachineSpec
-	var changes vmchange.SpecChanges
+	var (
+		lastAppliedSpec *virtv2.VirtualMachineSpec
+		changes         vmchange.SpecChanges
+		allChanges      vmchange.SpecChanges
+	)
 	if kvvm != nil {
 		lastAppliedSpec = h.loadLastAppliedSpec(current, kvvm)
+		lastClassAppliedSpec := h.loadClassLastAppliedSpec(class, kvvm)
 		changes = h.detectSpecChanges(ctx, kvvm, &current.Spec, lastAppliedSpec)
+		if !changes.IsEmpty() {
+			allChanges.Add(changes.GetAll()...)
+		}
+		classChanges := h.detectClassSpecChanges(ctx, &class.Spec, lastClassAppliedSpec)
+		if !classChanges.IsEmpty() {
+			allChanges.Add(classChanges.GetAll()...)
+			cbClassChanged := conditions.NewConditionBuilder(vmcondition.TypeClassChanged).
+				Generation(current.GetGeneration()).
+				Status(metav1.ConditionTrue).
+				Reason(vmcondition.ReasonClassChanged)
+			conditions.SetCondition(cbClassChanged, &changed.Status.Conditions)
+		} else {
+			conditions.RemoveCondition(vmcondition.TypeClassChanged, &changed.Status.Conditions)
+		}
+	} else {
+		conditions.RemoveCondition(vmcondition.TypeClassChanged, &changed.Status.Conditions)
 	}
 
 	if kvvm == nil || changes.IsEmpty() {
@@ -138,7 +162,7 @@ func (h *SyncKvvmHandler) Handle(ctx context.Context, s state.VirtualMachineStat
 	var errs error
 
 	// 3. Create or update KVVM.
-	synced, kvvmSyncErr := h.syncKVVM(ctx, s, changes)
+	synced, kvvmSyncErr := h.syncKVVM(ctx, s, allChanges)
 	if kvvmSyncErr != nil {
 		errs = errors.Join(errs, fmt.Errorf("failed to sync the internal virtual machine: %w", kvvmSyncErr))
 	}
@@ -163,7 +187,7 @@ func (h *SyncKvvmHandler) Handle(ctx context.Context, s state.VirtualMachineStat
 			Status(metav1.ConditionFalse).
 			Reason(vmcondition.ReasonConfigurationNotApplied).
 			Message(service.CapitalizeFirstLetter(errs.Error()) + ".")
-	case len(changed.Status.RestartAwaitingChanges) > 0:
+	case len(changed.Status.RestartAwaitingChanges) > 0 || h.classChanged(changed):
 		h.recorder.Event(current, corev1.EventTypeNormal, virtv2.ReasonErrRestartAwaitingChanges, "The virtual machine configuration successfully synced")
 		cbConfApplied.
 			Status(metav1.ConditionFalse).
@@ -187,6 +211,14 @@ func (h *SyncKvvmHandler) Name() string {
 	return nameSyncKvvmHandler
 }
 
+func (h *SyncKvvmHandler) classChanged(vm *virtv2.VirtualMachine) bool {
+	if vm == nil {
+		return false
+	}
+	c, _ := conditions.GetCondition(vmcondition.TypeClassChanged, vm.Status.Conditions)
+	return c.Status == metav1.ConditionTrue
+}
+
 func (h *SyncKvvmHandler) isWaiting(vm *virtv2.VirtualMachine) bool {
 	for _, c := range vm.Status.Conditions {
 		switch vmcondition.Type(c.Type) {
@@ -225,7 +257,7 @@ func (h *SyncKvvmHandler) isWaiting(vm *virtv2.VirtualMachine) bool {
 	return false
 }
 
-func (h *SyncKvvmHandler) syncKVVM(ctx context.Context, s state.VirtualMachineState, changes vmchange.SpecChanges) (bool, error) {
+func (h *SyncKvvmHandler) syncKVVM(ctx context.Context, s state.VirtualMachineState, allChanges vmchange.SpecChanges) (bool, error) {
 	if s.VirtualMachine().IsEmpty() {
 		return false, fmt.Errorf("the virtual machine is empty, please report a bug")
 	}
@@ -254,15 +286,15 @@ func (h *SyncKvvmHandler) syncKVVM(ctx context.Context, s state.VirtualMachineSt
 	}
 
 	switch {
-	case h.canApplyChanges(s.VirtualMachine().Current(), kvvm, kvvmi, pod, changes):
+	case h.canApplyChanges(s.VirtualMachine().Current(), kvvm, kvvmi, pod, allChanges):
 		// No need to wait, apply changes to KVVM immediately.
-		err = h.applyVMChangesToKVVM(ctx, s, changes)
+		err = h.applyVMChangesToKVVM(ctx, s, allChanges)
 		if err != nil {
 			return false, fmt.Errorf("apply changes to the internal virtual machine: %w", err)
 		}
 
 		return true, nil
-	case changes.IsEmpty():
+	case allChanges.IsEmpty():
 		return true, nil
 	default:
 		// Delay changes propagation to KVVM until user restarts VM.
@@ -375,7 +407,12 @@ func (h *SyncKvvmHandler) makeKVVMFromVMSpec(ctx context.Context, s state.Virtua
 
 	err = kvbuilder.SetLastAppliedSpec(newKVVM, current)
 	if err != nil {
-		return nil, fmt.Errorf("set last applied spec on the internal virtual machine: %w", err)
+		return nil, fmt.Errorf("set vm last applied spec on the internal virtual machine: %w", err)
+	}
+
+	err = kvbuilder.SetLastAppliedClassSpec(newKVVM, class)
+	if err != nil {
+		return nil, fmt.Errorf("set vmclass last applied spec on the internal virtual machine: %w", err)
 	}
 
 	return newKVVM, nil
@@ -417,9 +454,33 @@ func (h *SyncKvvmHandler) loadLastAppliedSpec(vm *virtv2.VirtualMachine, kvvm *v
 	return lastSpec
 }
 
+func (h *SyncKvvmHandler) loadClassLastAppliedSpec(class *virtv2.VirtualMachineClass, kvvm *virtv1.VirtualMachine) *virtv2.VirtualMachineClassSpec {
+	if kvvm == nil || class == nil {
+		return nil
+	}
+
+	lastSpec, err := kvbuilder.LoadLastAppliedClassSpec(kvvm)
+	// TODO Add smarter handler for empty/invalid annotation.
+	if lastSpec == nil && err == nil {
+		h.recorder.Event(class, corev1.EventTypeWarning, virtv2.ReasonVMClassLastAppliedSpecInvalid, "Could not find last applied spec. Possible old VMClass or partial backup restore. Restart or recreate VM to adopt it.")
+		lastSpec = &virtv2.VirtualMachineClassSpec{}
+	}
+	if err != nil {
+		msg := fmt.Sprintf("Could not restore last applied spec: %v. Possible old VMClass or partial backup restore. Restart or recreate VM to adopt it.", err)
+		h.recorder.Event(class, corev1.EventTypeWarning, virtv2.ReasonVMClassLastAppliedSpecInvalid, msg)
+		lastSpec = &virtv2.VirtualMachineClassSpec{}
+	}
+
+	return lastSpec
+}
+
 // 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(ctx context.Context, kvvm *virtv1.VirtualMachine, currentSpec, lastSpec *virtv2.VirtualMachineSpec) vmchange.SpecChanges {
+func (h *SyncKvvmHandler) detectSpecChanges(
+	ctx context.Context,
+	kvvm *virtv1.VirtualMachine,
+	currentSpec, lastSpec *virtv2.VirtualMachineSpec,
+) vmchange.SpecChanges {
 	log := logger.FromContext(ctx)
 
 	// Not applicable if KVVM is absent.
@@ -429,18 +490,35 @@ func (h *SyncKvvmHandler) detectSpecChanges(ctx context.Context, kvvm *virtv1.Vi
 
 	// Compare VM spec applied to the underlying KVVM
 	// with the current VM spec (maybe edited by the user).
-	specChanges := vmchange.CompareSpecs(lastSpec, currentSpec)
+	specChanges := vmchange.CompareVMSpecs(lastSpec, currentSpec)
+
+	log.Info(fmt.Sprintf("detected VM changes: empty %v, disruptive %v, actionType %v", specChanges.IsEmpty(), specChanges.IsDisruptive(), specChanges.ActionType()))
+	log.Info(fmt.Sprintf("detected VM changes JSON: %s", specChanges.ToJSON()))
+
+	return specChanges
+}
+
+func (h *SyncKvvmHandler) detectClassSpecChanges(ctx context.Context, currentClassSpec, lastClassSpec *virtv2.VirtualMachineClassSpec) vmchange.SpecChanges {
+	log := logger.FromContext(ctx)
+
+	specChanges := vmchange.CompareClassSpecs(currentClassSpec, lastClassSpec)
 
-	log.Info(fmt.Sprintf("detected changes: empty %v, disruptive %v, actionType %v", specChanges.IsEmpty(), specChanges.IsDisruptive(), specChanges.ActionType()))
-	log.Info(fmt.Sprintf("detected changes JSON: %s", specChanges.ToJSON()))
+	log.Info(fmt.Sprintf("detected VMClass changes: empty %v, disruptive %v, actionType %v", specChanges.IsEmpty(), specChanges.IsDisruptive(), specChanges.ActionType()))
+	log.Info(fmt.Sprintf("detected VMClass changes JSON: %s", specChanges.ToJSON()))
 
 	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 {
+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
 	}
@@ -515,22 +593,35 @@ func (h *SyncKvvmHandler) applyVMChangesToKVVM(ctx context.Context, s state.Virt
 	case vmchange.ActionNone:
 		log.Info("No changes to underlying KVVM, update last-applied-spec annotation", "vm.name", current.GetName())
 
-		if err := h.updateKVVMLastAppliedSpec(ctx, current, kvvm); err != nil {
+		class, err := s.Class(ctx)
+		if err != nil {
+			return fmt.Errorf("failed to get vmclass: %w", err)
+		}
+		if err = h.updateKVVMLastAppliedSpec(ctx, current, kvvm, class); err != nil {
 			return fmt.Errorf("unable to update last-applied-spec on KVVM: %w", err)
 		}
 	}
 	return nil
 }
 
 // updateKVVMLastAppliedSpec updates last-applied-spec annotation on KubeVirt VirtualMachine.
-func (h *SyncKvvmHandler) updateKVVMLastAppliedSpec(ctx context.Context, vm *virtv2.VirtualMachine, kvvm *virtv1.VirtualMachine) error {
+func (h *SyncKvvmHandler) updateKVVMLastAppliedSpec(
+	ctx context.Context,
+	vm *virtv2.VirtualMachine,
+	kvvm *virtv1.VirtualMachine,
+	class *virtv2.VirtualMachineClass,
+) error {
 	if vm == nil || kvvm == nil {
 		return nil
 	}
 
 	err := kvbuilder.SetLastAppliedSpec(kvvm, vm)
 	if err != nil {
-		return fmt.Errorf("set last applied spec on KubeVirt VM '%s': %w", kvvm.GetName(), err)
+		return fmt.Errorf("set vm last applied spec on KubeVirt VM '%s': %w", kvvm.GetName(), err)
+	}
+	err = kvbuilder.SetLastAppliedClassSpec(kvvm, class)
+	if err != nil {
+		return fmt.Errorf("set vmclass last applied spec on KubeVirt VM '%s': %w", kvvm.GetName(), err)
 	}
 
 	if err := h.client.Update(ctx, kvvm); err != nil {

images/virtualization-artifact/pkg/controller/vm/internal/watcher/vmclass_watcher.go

@@ -18,8 +18,8 @@ package watcher
 
 import (
 	"context"
-	"reflect"
 
+	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -79,7 +79,9 @@ func (w VirtualMachineClassWatcher) Watch(mgr manager.Manager, ctr controller.Co
 				if !oldOk || !newOk {
 					return false
 				}
-				return !reflect.DeepEqual(oldVMC.Spec.SizingPolicies, newVMC.Spec.SizingPolicies)
+				return !equality.Semantic.DeepEqual(oldVMC.Spec.SizingPolicies, newVMC.Spec.SizingPolicies) ||
+					!equality.Semantic.DeepEqual(oldVMC.Spec.Tolerations, oldVMC.Spec.Tolerations) ||
+					!equality.Semantic.DeepEqual(oldVMC.Spec.NodeSelector, newVMC.Spec.NodeSelector)
 			},
 		},
 	)

images/virtualization-artifact/pkg/controller/vmchange/compare.go

@@ -42,7 +42,21 @@ var specComparators = []SpecFieldsComparator{
 	compareProvisioning,
 }
 
-func CompareSpecs(prev, next *v1alpha2.VirtualMachineSpec) SpecChanges {
+type VmClassSpecFieldsComparator func(prev, next *v1alpha2.VirtualMachineClassSpec) []FieldChange
+
+var vmclassSpecComparators = []VmClassSpecFieldsComparator{
+	compareVmClassNodeSelector,
+	compareVmClassTolerations,
+}
+
+func CompareSpecs(prev, next *v1alpha2.VirtualMachineSpec, prevClass, nextClass *v1alpha2.VirtualMachineClassSpec) SpecChanges {
+	specChanges := CompareVMSpecs(prev, next)
+	specClassChanges := CompareClassSpecs(prevClass, nextClass)
+	specChanges.Add(specClassChanges.GetAll()...)
+	return specChanges
+}
+
+func CompareVMSpecs(prev, next *v1alpha2.VirtualMachineSpec) SpecChanges {
 	specChanges := SpecChanges{}
 
 	for _, comparator := range specComparators {
@@ -54,3 +68,16 @@ func CompareSpecs(prev, next *v1alpha2.VirtualMachineSpec) SpecChanges {
 
 	return specChanges
 }
+
+func CompareClassSpecs(prevClass, nextClass *v1alpha2.VirtualMachineClassSpec) SpecChanges {
+	var specChanges SpecChanges
+
+	for _, comparator := range vmclassSpecComparators {
+		changes := comparator(prevClass, nextClass)
+		if HasChanges(changes) {
+			specChanges.Add(changes...)
+		}
+	}
+
+	return specChanges
+}

images/virtualization-artifact/pkg/controller/vmchange/compare_test.go

@@ -356,7 +356,7 @@ enableParavirtualization: true
 			currentSpec := loadVMSpec(t, tt.currentSpec)
 			desiredSpec := loadVMSpec(t, tt.desiredSpec)
 
-			changes = CompareSpecs(currentSpec, desiredSpec)
+			changes = CompareVMSpecs(currentSpec, desiredSpec)
 
 			defer func() {
 				if t.Failed() {