diff --git a/api/core/v1alpha2/events.go b/api/core/v1alpha2/events.go index 8050fb86f..cae81b51a 100644 --- a/api/core/v1alpha2/events.go +++ b/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" diff --git a/api/core/v1alpha2/virtual_machine_class.go b/api/core/v1alpha2/virtual_machine_class.go index 5e063582a..7c5d5e2d2 100644 --- a/api/core/v1alpha2/virtual_machine_class.go +++ b/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"` } diff --git a/api/core/v1alpha2/vmcondition/condition.go b/api/core/v1alpha2/vmcondition/condition.go index e99b6416a..f4c6ff843 100644 --- a/api/core/v1alpha2/vmcondition/condition.go +++ b/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" diff --git a/images/virtualization-artifact/pkg/common/annotations/annotations.go b/images/virtualization-artifact/pkg/common/annotations/annotations.go index a7221cd17..d69bae09c 100644 --- a/images/virtualization-artifact/pkg/common/annotations/annotations.go +++ b/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. diff --git a/images/virtualization-artifact/pkg/controller/conditions/builder.go b/images/virtualization-artifact/pkg/controller/conditions/builder.go index ea23639b8..cc131c0ef 100644 --- a/images/virtualization-artifact/pkg/controller/conditions/builder.go +++ b/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() { diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/last_applied_spec.go b/images/virtualization-artifact/pkg/controller/kvbuilder/last_applied_spec.go index 1a20be659..8b7d2578e 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/last_applied_spec.go +++ b/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 +} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go index c4bf24dc0..7a7e9f5c7 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/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, ¤t.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,10 +490,21 @@ 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 } @@ -440,7 +512,13 @@ func (h *SyncKvvmHandler) detectSpecChanges(ctx context.Context, kvvm *virtv1.Vi // 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,7 +593,11 @@ 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) } } @@ -523,14 +605,23 @@ func (h *SyncKvvmHandler) applyVMChangesToKVVM(ctx context.Context, s state.Virt } // 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 { diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/watcher/vmclass_watcher.go b/images/virtualization-artifact/pkg/controller/vm/internal/watcher/vmclass_watcher.go index 48a7bb9f0..aeb792378 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/watcher/vmclass_watcher.go +++ b/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) }, }, ) diff --git a/images/virtualization-artifact/pkg/controller/vmchange/compare.go b/images/virtualization-artifact/pkg/controller/vmchange/compare.go index 194ca7f6e..6c1c3881a 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/compare.go +++ b/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 +} diff --git a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go index 7c807afb7..cca57372e 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go +++ b/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() { diff --git a/images/virtualization-artifact/pkg/controller/vmchange/vmclass_change.go b/images/virtualization-artifact/pkg/controller/vmchange/vmclass_change.go new file mode 100644 index 000000000..8d2defb44 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmchange/vmclass_change.go @@ -0,0 +1,42 @@ +package vmchange + +import ( + "fmt" + "reflect" + + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +func makePathWithClass(path string) string { + return fmt.Sprintf("VirtualMachineClass:%s", path) +} + +func compareVmClassNodeSelector(current, desired *v1alpha2.VirtualMachineClassSpec) []FieldChange { + isEmpty := func(nodeSelector v1alpha2.NodeSelector) bool { + return len(nodeSelector.MatchExpressions) == 0 && len(nodeSelector.MatchLabels) == 0 + } + + currentValue := NewValue(current.NodeSelector, isEmpty(current.NodeSelector), false) + desiredValue := NewValue(desired.NodeSelector, isEmpty(desired.NodeSelector), false) + + return compareValues( + makePathWithClass("spec.nodeSelector"), + currentValue, + desiredValue, + reflect.DeepEqual(current.NodeSelector, desired.NodeSelector), + ActionRestart, + ) +} + +func compareVmClassTolerations(current, desired *v1alpha2.VirtualMachineClassSpec) []FieldChange { + currentValue := NewValue(current.Tolerations, len(current.Tolerations) == 0, false) + desiredValue := NewValue(desired.Tolerations, len(desired.Tolerations) == 0, false) + + return compareValues( + makePathWithClass("spec.tolerations"), + currentValue, + desiredValue, + reflect.DeepEqual(current.Tolerations, desired.Tolerations), + ActionRestart, + ) +}