From 779ac270840978a3ed6f3164ec124a206955a907 Mon Sep 17 00:00:00 2001 From: Ivan Mikheykin Date: Mon, 16 Mar 2026 14:39:38 +0300 Subject: [PATCH 1/2] feat(vm): memory hotplug (phase 1) Simple implementation of the memory hotplug support. - Change memory configuration: set it in domain.memory.guest instead of setting requests and limits in domain.resources. - Support old VMs to not require reboot on module update. - Support for enabling feature gates explicitly in ModuleConfig. Dev notes: - Adding detector of kvvm.spec changes to require reboot if kvbuilder changes something. - New on-demand vmop migration prefixed as "hotplug-resource-". Signed-off-by: Ivan Mikheykin --- build/components/versions.yml | 2 +- images/virt-artifact/werf.inc.yaml | 1 + .../pkg/common/annotations/annotations.go | 7 +- .../pkg/controller/kvbuilder/kvvm.go | 75 ++++++++- .../pkg/controller/kvbuilder/kvvm_test.go | 10 +- .../pkg/controller/kvbuilder/kvvm_utils.go | 2 +- .../pkg/controller/reconciler/reconciler.go | 4 +- .../pkg/controller/vm/internal/sync_kvvm.go | 132 +++++++++------- .../controller/vm/internal/sync_kvvm_test.go | 3 +- .../pkg/controller/vm/vm_controller.go | 2 +- .../controller/vmchange/comparator_memory.go | 65 ++++++++ .../pkg/controller/vmchange/comparators.go | 7 - .../pkg/controller/vmchange/compare.go | 82 ++++++---- .../pkg/controller/vmchange/compare_test.go | 149 +++++++++++++++++- .../internal/handler/hotplug.go | 80 ++++++++++ .../internal/handler/hotplug_test.go | 119 ++++++++++++++ .../internal/watcher/kvvmi.go | 5 +- .../workload_updater_controller.go | 5 + .../pkg/featuregates/featuregate.go | 6 + openapi/config-values.yaml | 12 ++ openapi/doc-ru-config-values.yaml | 9 ++ templates/kubevirt/kubevirt.yaml | 23 ++- .../virtualization-controller/_helpers.tpl | 13 ++ .../virtualization-controller/deployment.yaml | 4 +- 24 files changed, 702 insertions(+), 115 deletions(-) create mode 100644 images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go create mode 100644 images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/hotplug.go create mode 100644 images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/hotplug_test.go diff --git a/build/components/versions.yml b/build/components/versions.yml index 35efe181a1..3b70ca0e54 100644 --- a/build/components/versions.yml +++ b/build/components/versions.yml @@ -3,7 +3,7 @@ firmware: libvirt: v10.9.0 edk2: stable202411 core: - 3p-kubevirt: v1.6.2-v12n.21 + 3p-kubevirt: dvp/set-memory-limits-while-hotplugging 3p-containerized-data-importer: v1.60.3-v12n.18 distribution: 2.8.3 package: diff --git a/images/virt-artifact/werf.inc.yaml b/images/virt-artifact/werf.inc.yaml index f30560fba6..767eff694a 100644 --- a/images/virt-artifact/werf.inc.yaml +++ b/images/virt-artifact/werf.inc.yaml @@ -15,6 +15,7 @@ secrets: shell: install: - | + echo rebuild 33 echo "Git clone {{ $gitRepoName }} repository..." git clone --depth=1 $(cat /run/secrets/SOURCE_REPO)/{{ $gitRepoUrl }} --branch {{ $tag }} /src/kubevirt diff --git a/images/virtualization-artifact/pkg/common/annotations/annotations.go b/images/virtualization-artifact/pkg/common/annotations/annotations.go index c70c6542cb..00e93d6805 100644 --- a/images/virtualization-artifact/pkg/common/annotations/annotations.go +++ b/images/virtualization-artifact/pkg/common/annotations/annotations.go @@ -90,9 +90,10 @@ const ( AnnVMRestartRequested = AnnAPIGroupV + "/vm-restart-requested" // AnnVMOPWorkloadUpdate is an annotation on vmop that represents a vmop created by workload-updater controller. - AnnVMOPWorkloadUpdate = AnnAPIGroupV + "/workload-update" - AnnVMOPWorkloadUpdateImage = AnnAPIGroupV + "/workload-update-image" - AnnVMOPWorkloadUpdateNodePlacementSum = AnnAPIGroupV + "/workload-update-node-placement-sum" + AnnVMOPWorkloadUpdate = AnnAPIGroupV + "/workload-update" + AnnVMOPWorkloadUpdateImage = AnnAPIGroupV + "/workload-update-image" + AnnVMOPWorkloadUpdateNodePlacementSum = AnnAPIGroupV + "/workload-update-node-placement-sum" + AnnVMOPWorkloadUpdateHotplugResourcesSum = AnnAPIGroupV + "/workload-update-hotplug-resources-sum" // AnnVMRestore is an annotation on a resource that indicates it was created by the vmrestore controller; the value is the UID of the `VirtualMachineRestore` resource. AnnVMRestore = AnnAPIGroupV + "/vmrestore" // AnnVMOPEvacuation is an annotation on vmop that represents a vmop created by evacuation controller diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go index 5ef04f9ed9..c1e3a1da12 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go @@ -33,6 +33,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/array" "github.com/deckhouse/virtualization-controller/pkg/common/resource_builder" "github.com/deckhouse/virtualization-controller/pkg/common/vm" + "github.com/deckhouse/virtualization-controller/pkg/featuregates" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -46,6 +47,9 @@ const ( // GenericCPUModel specifies the base CPU model for Features and Discovery CPU model types. GenericCPUModel = "qemu64" + + MaxMemorySizeForHotplug = 256 * 1024 * 1024 * 1024 // 256 Gi (safely limit to not overlap somewhat conservative 38 bit physical address space) + EnableMemoryHotplugThreshold = 1 * 1024 * 1024 * 1024 // 1 Gi (no hotplug for VMs with less than 1Gi) ) type KVVMOptions struct { @@ -280,7 +284,25 @@ func (b *KVVM) SetCPU(cores int, coreFraction string) error { return nil } +// SetMemory sets memory in kvvm. +// There are 2 possibilities to set memory: +// 1. Use domain.memory.guest field: it enabled memory hotplugging, but not set resources.limits. +// 2. Explicitly set limits and requests in domain.resources. No hotplugging in this scenario. +// +// (1) is a new approach, and (2) should be respected for Running VMs started by previous version of the controller. func (b *KVVM) SetMemory(memorySize resource.Quantity) { + // Support for VMs started with memory size in requests-limits. + // TODO delete this in the future (around 3-4 more versions after enabling memory hotplug by default). + if b.ResourceExists && isVMRunningWithMemoryResources(b.Resource) { + b.setMemoryNonHotpluggable(memorySize) + return + } + b.setMemoryHotpluggable(memorySize) +} + +// setMemoryNonHotpluggable translates memory size to requests and limits in KVVM. +// Note: this is a first implementation, memory hotplug is not compatible with this strategy. +func (b *KVVM) setMemoryNonHotpluggable(memorySize resource.Quantity) { res := &b.Resource.Spec.Template.Spec.Domain.Resources if res.Requests == nil { res.Requests = make(map[corev1.ResourceName]resource.Quantity) @@ -292,6 +314,57 @@ func (b *KVVM) SetMemory(memorySize resource.Quantity) { res.Limits[corev1.ResourceMemory] = memorySize } +// setMemoryHotpluggable translates memory size to settings in domain.memory field. +// This field is compatible with memory hotplug. +// Also, remove requests-limits for memory if any. +func (b *KVVM) setMemoryHotpluggable(memorySize resource.Quantity) { + domain := &b.Resource.Spec.Template.Spec.Domain + + currentMaxGuest := int64(-1) + if domain.Memory != nil && domain.Memory.MaxGuest != nil { + currentMaxGuest = domain.Memory.MaxGuest.Value() + } + + domain.Memory = &virtv1.Memory{ + Guest: &memorySize, + } + + // Set maxMemory to enable hotplug for mem size >= 1Gi. + hotplugThreshold := resource.NewQuantity(EnableMemoryHotplugThreshold, resource.BinarySI) + if featuregates.Default().Enabled(featuregates.HotplugMemoryWithLiveMigration) { + if memorySize.Cmp(*hotplugThreshold) >= 0 { + maxMemory := resource.NewQuantity(MaxMemorySizeForHotplug, resource.BinarySI) + domain.Memory.MaxGuest = maxMemory + } + } + // Set maxGuest to 0 if hotplug is disabled now (mem size < 1Gi) and maxGuest was previously set. + // Zero value is just a flag to patch memory and remove maxGuest before updating kvvm. + if memorySize.Cmp(*hotplugThreshold) == -1 && currentMaxGuest > 0 { + domain.Memory.MaxGuest = resource.NewQuantity(0, resource.BinarySI) + } + + // Remove memory limits and requests if set by previous implementation. + res := &b.Resource.Spec.Template.Spec.Domain.Resources + delete(res.Requests, corev1.ResourceMemory) + delete(res.Limits, corev1.ResourceMemory) +} + +func isVMRunningWithMemoryResources(kvvm *virtv1.VirtualMachine) bool { + if kvvm == nil { + return false + } + + if kvvm.Status.PrintableStatus != virtv1.VirtualMachineStatusRunning { + return false + } + + res := kvvm.Spec.Template.Spec.Domain.Resources + _, hasMemoryRequests := res.Requests[corev1.ResourceMemory] + _, hasMemoryLimits := res.Limits[corev1.ResourceMemory] + + return hasMemoryRequests && hasMemoryLimits +} + func GetCPURequest(cores int, coreFraction string) (*resource.Quantity, error) { if coreFraction == "" { return GetCPULimit(cores), nil @@ -484,7 +557,7 @@ func (b *KVVM) SetProvisioning(p *v1alpha2.Provisioning) error { } } -func (b *KVVM) SetOsType(osType v1alpha2.OsType) error { +func (b *KVVM) SetOSType(osType v1alpha2.OsType) error { switch osType { case v1alpha2.Windows: // Need for `029-use-OFVM_CODE-for-linux.patch` diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_test.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_test.go index 8a14050251..237bb4b5b4 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_test.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_test.go @@ -119,25 +119,25 @@ func TestSetAffinity(t *testing.T) { } } -func TestSetOsType(t *testing.T) { +func TestSetOSType(t *testing.T) { name := "test-name" namespace := "test-namespace" t.Run("Change from Windows to Generic should remove TPM", func(t *testing.T) { builder := NewEmptyKVVM(types.NamespacedName{Name: name, Namespace: namespace}, KVVMOptions{}) - err := builder.SetOsType(v1alpha2.Windows) + err := builder.SetOSType(v1alpha2.Windows) if err != nil { - t.Fatalf("SetOsType(Windows) failed: %v", err) + t.Fatalf("SetOSType(Windows) failed: %v", err) } if builder.Resource.Spec.Template.Spec.Domain.Devices.TPM == nil { t.Error("TPM should be present after setting Windows OS") } - err = builder.SetOsType(v1alpha2.GenericOs) + err = builder.SetOSType(v1alpha2.GenericOs) if err != nil { - t.Fatalf("SetOsType(GenericOs) failed: %v", err) + t.Fatalf("SetOSType(GenericOs) failed: %v", err) } if builder.Resource.Spec.Template.Spec.Domain.Devices.TPM != nil { diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go index 594a6d7bdb..61edd64c33 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go @@ -99,7 +99,7 @@ func ApplyVirtualMachineSpec( if err := kvvm.SetRunPolicy(vm.Spec.RunPolicy); err != nil { return err } - if err := kvvm.SetOsType(vm.Spec.OsType); err != nil { + if err := kvvm.SetOSType(vm.Spec.OsType); err != nil { return err } if err := kvvm.SetBootloader(vm.Spec.Bootloader); err != nil { diff --git a/images/virtualization-artifact/pkg/controller/reconciler/reconciler.go b/images/virtualization-artifact/pkg/controller/reconciler/reconciler.go index d839d491b0..7de44a3339 100644 --- a/images/virtualization-artifact/pkg/controller/reconciler/reconciler.go +++ b/images/virtualization-artifact/pkg/controller/reconciler/reconciler.go @@ -19,6 +19,7 @@ package reconciler import ( "context" "errors" + "fmt" "reflect" "strings" "time" @@ -102,7 +103,8 @@ handlersLoop: switch { case err == nil: // OK. case errors.Is(err, ErrStopHandlerChain): - log.Debug("Handler chain execution stopped") + msg := fmt.Sprintf("Handler %s stopped chain execution", name) + log.Debug(msg) result = MergeResults(result, res) break handlersLoop case k8serrors.IsConflict(err): 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 495d9aaa5d..6e0e9e9cfc 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -20,12 +20,15 @@ import ( "context" "errors" "fmt" + "reflect" "time" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/component-base/featuregate" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -33,6 +36,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/common/network" "github.com/deckhouse/virtualization-controller/pkg/common/object" + "github.com/deckhouse/virtualization-controller/pkg/common/patch" vmutil "github.com/deckhouse/virtualization-controller/pkg/common/vm" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/kvbuilder" @@ -52,11 +56,18 @@ type syncVolumesService interface { SyncVolumes(ctx context.Context, s state.VirtualMachineState, restartRequired bool) (reconcile.Result, error) } -func NewSyncKvvmHandler(dvcrSettings *dvcr.Settings, client client.Client, recorder eventrecord.EventRecorderLogger, syncVolumesService syncVolumesService) *SyncKvvmHandler { +func NewSyncKvvmHandler( + dvcrSettings *dvcr.Settings, + client client.Client, + recorder eventrecord.EventRecorderLogger, + featureGate featuregate.FeatureGate, + syncVolumesService syncVolumesService, +) *SyncKvvmHandler { return &SyncKvvmHandler{ dvcrSettings: dvcrSettings, client: client, recorder: recorder, + featureGate: featureGate, syncVolumesService: syncVolumesService, } } @@ -65,6 +76,7 @@ type SyncKvvmHandler struct { client client.Client recorder eventrecord.EventRecorderLogger dvcrSettings *dvcr.Settings + featureGate featuregate.FeatureGate syncVolumesService syncVolumesService } @@ -298,24 +310,14 @@ func (h *SyncKvvmHandler) syncKVVM(ctx context.Context, s state.VirtualMachineSt } return true, nil case h.isVMStopped(s.VirtualMachine().Current(), kvvm, pod): - // KVVM must be updated when the VM is stopped because all its components, - // like VirtualDisk and other resources, - // can be changed during the restoration process. + // KVVM should be updated when VM become stopped. + // It is safe to update KVVM at this point in general and also all related resources + // can be changed during the restoration process: e.g. VirtualDisks, VMIPs, etc. // For example, the PVC of the VirtualDisk will be changed, // and the volume with this PVC must be updated in the KVVM specification. - hasVMChanges, err := h.detectVMSpecChanges(ctx, s) - if err != nil { - return false, fmt.Errorf("detect changes on the stopped internal virtual machine: %w", err) - } - hasVMClassChanges, err := h.detectVMClassSpecChanges(ctx, s) + err := h.updateKVVM(ctx, s) if err != nil { - return false, fmt.Errorf("detect changes on the stopped internal virtual machine: %w", err) - } - if hasVMChanges || hasVMClassChanges { - err := h.updateKVVM(ctx, s) - if err != nil { - return false, fmt.Errorf("update stopped internal virtual machine: %w", err) - } + return false, fmt.Errorf("update internal virtual machine in 'Stopped' state: %w", err) } return true, nil case h.hasNoneDisruptiveChanges(s.VirtualMachine().Current(), kvvm, kvvmi, allChanges): @@ -370,17 +372,48 @@ func (h *SyncKvvmHandler) updateKVVM(ctx context.Context, s state.VirtualMachine return fmt.Errorf("the virtual machine is empty, please report a bug") } - kvvm, err := MakeKVVMFromVMSpec(ctx, s) + newKVVM, err := MakeKVVMFromVMSpec(ctx, s) if err != nil { - return fmt.Errorf("failed to prepare the internal virtual machine: %w", err) + return fmt.Errorf("update internal virtual machine: make kvvm from the virtual machine spec: %w", err) } - if err = h.client.Update(ctx, kvvm); err != nil { - return fmt.Errorf("failed to create the internal virtual machine: %w", err) - } + // Check for changes to skip unneeded updated. + isChanged, err := IsKVVMChanged(ctx, s, newKVVM) + if err != nil { + return fmt.Errorf("update internal virtual machine: detect changes: %w", err) + } + + if isChanged { + memory := newKVVM.Spec.Template.Spec.Domain.Memory + if memory != nil && memory.MaxGuest != nil && memory.MaxGuest.IsZero() { + // Zero maxGuest is a special value to patch KVVM to unset maxGuest. + // Set it to nil for next update call. + memory.MaxGuest = nil + + // 2 operations: remove memory.maxGuest; set memory.guest. + // Remove is not enough, remove and set are needed both to pass the kubevirt vm-validator webhook. + patchBytes, err := patch.NewJSONPatch( + patch.WithRemove("/spec/template/spec/domain/memory/maxGuest"), + patch.WithReplace("/spec/template/spec/domain/memory/guest", memory.Guest.String()), + ).Bytes() + if err != nil { + return fmt.Errorf("prepare json patch to unset memory.maxGuest: %w", err) + } + + if err = h.client.Patch(ctx, newKVVM, client.RawPatch(types.JSONPatchType, patchBytes)); err != nil { + return fmt.Errorf("patch internal virtual machine to unset memory.maxGuest: %w", err) + } + } - log.Info("Update KubeVirt VM done", "name", kvvm.Name) - log.Debug("Update KubeVirt VM done", "name", kvvm.Name, "kvvm", kvvm) + if err = h.client.Update(ctx, newKVVM); err != nil { + return fmt.Errorf("update internal virtual machine: %w", err) + } + + log.Info("Update internal virtual machine done", "name", newKVVM.Name) + log.Debug("Update internal virtual machine done", "name", newKVVM.Name, "kvvm", newKVVM) + } else { + log.Debug("Update internal virtual machine is not needed", "name", newKVVM.Name, "kvvm", newKVVM) + } return nil } @@ -407,7 +440,7 @@ func MakeKVVMFromVMSpec(ctx context.Context, s state.VirtualMachineState) (*virt bdState := NewBlockDeviceState(s) err = bdState.Reload(ctx) if err != nil { - return nil, fmt.Errorf("failed to relaod blockdevice state for the virtual machine: %w", err) + return nil, fmt.Errorf("failed to reload blockdevice state for the virtual machine: %w", err) } class, err := s.Class(ctx) if err != nil { @@ -454,6 +487,25 @@ func MakeKVVMFromVMSpec(ctx context.Context, s state.VirtualMachineState) (*virt return newKVVM, nil } +// IsKVVMChanged returns whether kvvm spec or special annotations are changed. +func IsKVVMChanged(ctx context.Context, s state.VirtualMachineState, kvvm *virtv1.VirtualMachine) (bool, error) { + currentKVVM, err := s.KVVM(ctx) + if err != nil { + return false, fmt.Errorf("get current kvvm: %w", err) + } + + isChanged := currentKVVM.Annotations[annotations.AnnVMLastAppliedSpec] != kvvm.Annotations[annotations.AnnVMLastAppliedSpec] + + if !isChanged { + isChanged = currentKVVM.Annotations[annotations.AnnVMClassLastAppliedSpec] != kvvm.Annotations[annotations.AnnVMClassLastAppliedSpec] + } + + if !isChanged { + isChanged = !reflect.DeepEqual(kvvm.Spec, currentKVVM.Spec) + } + return isChanged, nil +} + func (h *SyncKvvmHandler) loadLastAppliedSpec(vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) *v1alpha2.VirtualMachineSpec { if kvvm == nil || vm == nil { return nil @@ -526,7 +578,7 @@ func (h *SyncKvvmHandler) detectSpecChanges( // Compare VM spec applied to the underlying KVVM // with the current VM spec (maybe edited by the user). - specChanges := vmchange.CompareVMSpecs(lastSpec, currentSpec) + specChanges := vmchange.NewVMSpecComparator(h.featureGate).Compare(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())) @@ -564,36 +616,6 @@ func (h *SyncKvvmHandler) isVMStopped( return isVMStopped(kvvm) && (!isKVVMICreated(kvvm) || podStopped) } -// detectVMSpecChanges returns true and no error if specification has changes. -func (h *SyncKvvmHandler) detectVMSpecChanges(ctx context.Context, s state.VirtualMachineState) (bool, error) { - currentKvvm, err := s.KVVM(ctx) - if err != nil { - return false, err - } - - newKvvm, err := MakeKVVMFromVMSpec(ctx, s) - if err != nil { - return false, err - } - - return currentKvvm.Annotations[annotations.AnnVMLastAppliedSpec] != newKvvm.Annotations[annotations.AnnVMLastAppliedSpec], nil -} - -// detectVMClassSpecChanges returns true and no error if specification has changes. -func (h *SyncKvvmHandler) detectVMClassSpecChanges(ctx context.Context, s state.VirtualMachineState) (bool, error) { - currentKvvm, err := s.KVVM(ctx) - if err != nil { - return false, err - } - - newKvvm, err := MakeKVVMFromVMSpec(ctx, s) - if err != nil { - return false, err - } - - return currentKvvm.Annotations[annotations.AnnVMClassLastAppliedSpec] != newKvvm.Annotations[annotations.AnnVMClassLastAppliedSpec], nil -} - // 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. diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm_test.go index 6c96e15525..1e76e42ee3 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm_test.go @@ -36,6 +36,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" "github.com/deckhouse/virtualization-controller/pkg/controller/vmchange" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" + "github.com/deckhouse/virtualization-controller/pkg/featuregates" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" ) @@ -137,7 +138,7 @@ var _ = Describe("SyncKvvmHandler", func() { } reconcile := func() { - h := NewSyncKvvmHandler(nil, fakeClient, recorder, vmservice.NewMigrationVolumesService(fakeClient, MakeKVVMFromVMSpec, 10*time.Second)) + h := NewSyncKvvmHandler(nil, fakeClient, recorder, featuregates.Default(), vmservice.NewMigrationVolumesService(fakeClient, MakeKVVMFromVMSpec, 10*time.Second)) _, err := h.Handle(ctx, vmState) Expect(err).NotTo(HaveOccurred()) err = resource.Update(context.Background()) diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_controller.go b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go index 6a4e231a88..21b3ba6100 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_controller.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go @@ -76,7 +76,7 @@ func SetupController( internal.NewPodHandler(client), internal.NewSizePolicyHandler(), internal.NewNetworkInterfaceHandler(featuregates.Default()), - internal.NewSyncKvvmHandler(dvcrSettings, client, recorder, migrateVolumesService), + internal.NewSyncKvvmHandler(dvcrSettings, client, recorder, featuregates.Default(), migrateVolumesService), internal.NewSyncPowerStateHandler(client, recorder), internal.NewSyncMetadataHandler(client), internal.NewLifeCycleHandler(client, recorder), diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go b/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go new file mode 100644 index 0000000000..c863cf1b46 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go @@ -0,0 +1,65 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package vmchange + +import ( + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/component-base/featuregate" + + "github.com/deckhouse/virtualization-controller/pkg/controller/kvbuilder" + "github.com/deckhouse/virtualization-controller/pkg/featuregates" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +type comparatorMemory struct { + featureGate featuregate.FeatureGate +} + +func NewComparatorMemory(featureGate featuregate.FeatureGate) VMSpecFieldComparator { + return &comparatorMemory{ + featureGate: featureGate, + } +} + +// Compare detects changes in memory size. +// It is aware of hotplug mechanism. If hotplug is disabled it requires +// restart if memory.size is changed. If hotplug is enabled, it allows +// changing "on the fly". Also, it requires restart if hotplug boundary +// is crossed. +// Note: memory hotplug is enabled if VM has more than 1Gi of RAM. +func (c *comparatorMemory) Compare(current, desired *v1alpha2.VirtualMachineSpec) []FieldChange { + hotplugThreshold := resource.NewQuantity(kvbuilder.EnableMemoryHotplugThreshold, resource.BinarySI) + isHotpluggable := current.Memory.Size.Cmp(*hotplugThreshold) > 0 + isHotpluggableDesired := desired.Memory.Size.Cmp(*hotplugThreshold) > 0 + + actionType := ActionRestart + if isHotpluggable && isHotpluggableDesired { + actionType = ActionApplyImmediate + } + + // Restart required to decrease memory size. (current > desired) + if current.Memory.Size.Cmp(desired.Memory.Size) == 1 { + actionType = ActionRestart + } + + // Require reboot if memory hotplug is not enabled. + if !c.featureGate.Enabled(featuregates.HotplugMemoryWithLiveMigration) { + actionType = ActionRestart + } + + return compareQuantity("memory.size", current.Memory.Size, desired.Memory.Size, resource.Quantity{}, actionType) +} diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparators.go b/images/virtualization-artifact/pkg/controller/vmchange/comparators.go index 8c44383fe9..529d6bed7e 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/comparators.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparators.go @@ -17,8 +17,6 @@ limitations under the License. package vmchange import ( - "k8s.io/apimachinery/pkg/api/resource" - "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -153,11 +151,6 @@ func compareCPU(current, desired *v1alpha2.VirtualMachineSpec) []FieldChange { return nil } -// compareMemory returns changes in the memory section. -func compareMemory(current, desired *v1alpha2.VirtualMachineSpec) []FieldChange { - return compareQuantity("memory.size", current.Memory.Size, desired.Memory.Size, resource.Quantity{}, ActionRestart) -} - func compareProvisioning(current, desired *v1alpha2.VirtualMachineSpec) []FieldChange { changes := compareEmpty( "provisioning", diff --git a/images/virtualization-artifact/pkg/controller/vmchange/compare.go b/images/virtualization-artifact/pkg/controller/vmchange/compare.go index 54350e7d96..aa1f44259b 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/compare.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/compare.go @@ -17,31 +17,30 @@ limitations under the License. package vmchange import ( + "k8s.io/component-base/featuregate" + "github.com/deckhouse/virtualization/api/core/v1alpha2" ) type SpecFieldsComparator func(prev, next *v1alpha2.VirtualMachineSpec) []FieldChange -var specComparators = []SpecFieldsComparator{ - compareVirtualMachineClass, - compareRunPolicy, - compareVirtualMachineIPAddress, - compareTopologySpreadConstraints, - compareAffinity, - compareNodeSelector, - comparePriorityClassName, - compareTolerations, - compareDisruptions, - compareTerminationGracePeriodSeconds, - compareEnableParavirtualization, - compareOSType, - compareBootloader, - compareCPU, - compareMemory, - compareBlockDevices, - compareProvisioning, - compareNetworks, - compareUSBDevices, +type VMSpecFieldComparator interface { + Compare(prev, next *v1alpha2.VirtualMachineSpec) []FieldChange +} + +type vmSpecFieldsComparatorWithFn struct { + fn func(prev, next *v1alpha2.VirtualMachineSpec) []FieldChange +} + +func (v *vmSpecFieldsComparatorWithFn) Compare(prev, next *v1alpha2.VirtualMachineSpec) []FieldChange { + if v.fn == nil { + return nil + } + return v.fn(prev, next) +} + +func vmSpecFieldComparator(fn SpecFieldsComparator) VMSpecFieldComparator { + return &vmSpecFieldsComparatorWithFn{fn: fn} } type VMClassSpecFieldsComparator func(prev, next *v1alpha2.VirtualMachineClassSpec) []FieldChange @@ -51,18 +50,45 @@ var vmclassSpecComparators = []VMClassSpecFieldsComparator{ 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 +type VMSpecComparator struct { + featureGate featuregate.FeatureGate +} + +func NewVMSpecComparator(featureGate featuregate.FeatureGate) *VMSpecComparator { + return &VMSpecComparator{ + featureGate: featureGate, + } +} + +func (v *VMSpecComparator) comparators() []VMSpecFieldComparator { + return []VMSpecFieldComparator{ + vmSpecFieldComparator(compareVirtualMachineClass), + vmSpecFieldComparator(compareRunPolicy), + vmSpecFieldComparator(compareVirtualMachineIPAddress), + vmSpecFieldComparator(compareTopologySpreadConstraints), + vmSpecFieldComparator(compareAffinity), + vmSpecFieldComparator(compareNodeSelector), + vmSpecFieldComparator(comparePriorityClassName), + vmSpecFieldComparator(compareTolerations), + vmSpecFieldComparator(compareDisruptions), + vmSpecFieldComparator(compareTerminationGracePeriodSeconds), + vmSpecFieldComparator(compareEnableParavirtualization), + vmSpecFieldComparator(compareOSType), + vmSpecFieldComparator(compareBootloader), + vmSpecFieldComparator(compareCPU), + NewComparatorMemory(v.featureGate), + vmSpecFieldComparator(compareBlockDevices), + vmSpecFieldComparator(compareProvisioning), + vmSpecFieldComparator(compareNetworks), + vmSpecFieldComparator(compareUSBDevices), + } } -func CompareVMSpecs(prev, next *v1alpha2.VirtualMachineSpec) SpecChanges { +func (v *VMSpecComparator) Compare(prev, next *v1alpha2.VirtualMachineSpec) SpecChanges { specChanges := SpecChanges{} - for _, comparator := range specComparators { - changes := comparator(prev, next) + for _, comparator := range v.comparators() { + changes := comparator.Compare(prev, next) if HasChanges(changes) { specChanges.Add(changes...) } diff --git a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go index 2ab26e66a8..e428b4fa0c 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go @@ -21,8 +21,10 @@ import ( "testing" "github.com/stretchr/testify/require" + "k8s.io/component-base/featuregate" "sigs.k8s.io/yaml" + "github.com/deckhouse/virtualization-controller/pkg/featuregates" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -31,6 +33,7 @@ func TestActionRequiredOnCompare(t *testing.T) { title string currentSpec string desiredSpec string + features []featuregate.Feature assertFn func(t *testing.T, changes SpecChanges) }{ { @@ -43,6 +46,7 @@ cpu: cpu: cores: 3 `, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("cpu.cores", ChangeReplace), @@ -60,6 +64,7 @@ cpu: cores: 2 coreFraction: 40% `, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("cpu.coreFraction", ChangeReplace), @@ -77,6 +82,7 @@ cpu: cores: 6 coreFraction: 40% `, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("cpu", ChangeReplace), @@ -93,6 +99,7 @@ cpu: cores: 2 coreFraction: 100% `, + nil, assertChanges( actionRequired(ActionNone), requirePathOperation("cpu.coreFraction", ChangeAdd), @@ -109,21 +116,119 @@ cpu: cpu: cores: 2 `, + nil, assertChanges( actionRequired(ActionNone), requirePathOperation("cpu.coreFraction", ChangeRemove), ), }, { - "restart on memory.size change", + "restart on memory.size change: no hotplug", + ` +memory: + size: 256Mi +`, + ` +memory: + size: 512Mi +`, + nil, + assertChanges( + actionRequired(ActionRestart), + requirePathOperation("memory.size", ChangeReplace), + ), + }, + { + "restart on memory.size change: enable hotplug", + ` +memory: + size: 384Mi +`, ` memory: size: 2Gi +`, + nil, + assertChanges( + actionRequired(ActionRestart), + requirePathOperation("memory.size", ChangeReplace), + ), + }, + { + "restart on memory.size change: disable hotplug", + ` +memory: + size: 3Gi `, ` memory: - size: 1Gi + size: 128Mi `, + nil, + assertChanges( + actionRequired(ActionRestart), + requirePathOperation("memory.size", ChangeReplace), + ), + }, + { + "immediate apply on memory.size increase when hotplug is enabled", + ` +memory: + size: 2Gi +`, + ` +memory: + size: 4Gi +`, + []featuregate.Feature{featuregates.HotplugMemoryWithLiveMigration}, + assertChanges( + actionRequired(ActionApplyImmediate), + requirePathOperation("memory.size", ChangeReplace), + ), + }, + { + "restart on memory.size increase when hotplug is disabled", + ` +memory: + size: 2Gi +`, + ` +memory: + size: 4Gi +`, + nil, + assertChanges( + actionRequired(ActionRestart), + requirePathOperation("memory.size", ChangeReplace), + ), + }, + { + "restart on memory.size reduce when hotplug is enabled", + ` +memory: + size: 4Gi +`, + ` +memory: + size: 2Gi +`, + []featuregate.Feature{featuregates.HotplugMemoryWithLiveMigration}, + assertChanges( + actionRequired(ActionRestart), + requirePathOperation("memory.size", ChangeReplace), + ), + }, + { + "restart on memory.size reduce when hotplug is disabled", + ` +memory: + size: 4Gi +`, + ` +memory: + size: 2Gi +`, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("memory.size", ChangeReplace), @@ -137,6 +242,7 @@ blockDeviceRefs: - kind: VirtualImage name: linux `, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("blockDeviceRefs", ChangeAdd), @@ -150,6 +256,7 @@ blockDeviceRefs: name: linux `, ``, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("blockDeviceRefs", ChangeRemove), @@ -169,6 +276,7 @@ blockDeviceRefs: - kind: VirtualImage name: linux `, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("blockDeviceRefs.0", ChangeAdd), @@ -188,6 +296,7 @@ blockDeviceRefs: - kind: VirtualImage name: linux `, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("blockDeviceRefs.0", ChangeRemove), @@ -209,6 +318,7 @@ blockDeviceRefs: - kind: VirtualImage name: linux `, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("blockDeviceRefs.0", ChangeReplace), @@ -244,6 +354,7 @@ blockDeviceRefs: - kind: VirtualImage name: linux `, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("blockDeviceRefs.0", ChangeReplace), @@ -261,6 +372,7 @@ provisioning: userData: | #cloudinit `, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("provisioning", ChangeAdd), @@ -276,6 +388,7 @@ provisioning: name: cloud-init-secret `, "", + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("provisioning", ChangeRemove), @@ -296,6 +409,7 @@ provisioning: userData: | #cloudinit `, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("provisioning", ChangeReplace), @@ -317,6 +431,7 @@ provisioning: kind: Secret name: provisioning-secret `, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("provisioning.userDataRef.name", ChangeReplace), @@ -330,6 +445,7 @@ enableParavirtualization: true ` enableParavirtualization: false `, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("enableParavirtualization", ChangeReplace), @@ -343,6 +459,7 @@ enableParavirtualization: false ` enableParavirtualization: true `, + nil, assertChanges( actionRequired(ActionRestart), requirePathOperation("enableParavirtualization", ChangeReplace), @@ -364,6 +481,7 @@ networks: name: net1 id: 2 `, + nil, assertChanges( actionRequired(ActionNone), requirePathOperation("networks", ChangeReplace), @@ -377,7 +495,12 @@ networks: currentSpec := loadVMSpec(t, tt.currentSpec) desiredSpec := loadVMSpec(t, tt.desiredSpec) - changes = CompareVMSpecs(currentSpec, desiredSpec) + gate := featuregates.Default() + if len(tt.features) > 0 { + gate = newFeatureGate(t, tt.features...) + } + + changes = NewVMSpecComparator(gate).Compare(currentSpec, desiredSpec) defer func() { if t.Failed() { @@ -448,3 +571,23 @@ func changesToYAML(changes SpecChanges) string { res, _ := yaml.Marshal(status) return string(res) } + +func newFeatureGate(t *testing.T, enabled ...featuregate.Feature) featuregate.FeatureGate { + t.Helper() + + gate, setFromMap, err := featuregates.NewUnlocked() + if err != nil { + t.Fatalf("failed to create feature gate: %v", err) + } + + featureMap := map[string]bool{} + for _, feature := range enabled { + featureMap[string(feature)] = true + } + + if err = setFromMap(featureMap); err != nil { + t.Fatalf("failed to set USB feature gate: %v", err) + } + + return gate +} diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/hotplug.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/hotplug.go new file mode 100644 index 0000000000..c2bbdeb6f2 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/hotplug.go @@ -0,0 +1,80 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package handler + +import ( + "context" + + corev1 "k8s.io/api/core/v1" + virtv1 "kubevirt.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/deckhouse/virtualization-controller/pkg/common/annotations" + "github.com/deckhouse/virtualization-controller/pkg/common/object" + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/logger" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +const hotplugHandler = "HotplugHandler" + +func NewHotplugHandler(client client.Client, migration OneShotMigration) *HotplugHandler { + return &HotplugHandler{ + client: client, + oneShotMigration: migration, + } +} + +type HotplugHandler struct { + client client.Client + oneShotMigration OneShotMigration +} + +func (h *HotplugHandler) Handle(ctx context.Context, vm *v1alpha2.VirtualMachine) (reconcile.Result, error) { + if vm == nil || !vm.GetDeletionTimestamp().IsZero() { + return reconcile.Result{}, nil + } + + kvvmi := &virtv1.VirtualMachineInstance{} + if err := h.client.Get(ctx, object.NamespacedName(vm), kvvmi); err != nil { + return reconcile.Result{}, client.IgnoreNotFound(err) + } + + cond, _ := conditions.GetKVVMICondition(virtv1.VirtualMachineInstanceMemoryChange, kvvmi.Status.Conditions) + if cond.Status != corev1.ConditionTrue { + return reconcile.Result{}, nil + } + + log := logger.FromContext(ctx).With(logger.SlogHandler(hotplugHandler)) + ctx = logger.ToContext(ctx, log) + + migrate, err := h.oneShotMigration.OnceMigrate(ctx, vm, annotations.AnnVMOPWorkloadUpdateHotplugResourcesSum, getHotplugResourcesSum(vm)) + if migrate { + log.Info("The virtual machine was triggered to migrate by the hotplug resources handler.") + } + + return reconcile.Result{}, err +} + +func (h *HotplugHandler) Name() string { + return hotplugHandler +} + +func getHotplugResourcesSum(vm *v1alpha2.VirtualMachine) string { + return vm.Spec.Memory.Size.String() +} diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/hotplug_test.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/hotplug_test.go new file mode 100644 index 0000000000..71606517c8 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/hotplug_test.go @@ -0,0 +1,119 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package handler + +import ( + "context" + "errors" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + virtv1 "kubevirt.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm" + "github.com/deckhouse/virtualization-controller/pkg/common/testutil" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +var _ = Describe("TestHotplugResourcesHandler", func() { + const ( + name = "vm-hotplug-resources" + namespace = "default" + ) + + var ( + serviceCompleteErr = errors.New("service is complete") + ctx = testutil.ContextBackgroundWithNoOpLogger() + fakeClient client.Client + ) + + AfterEach(func() { + fakeClient = nil + }) + + newVMAndKVVMI := func(hasHotMemoryChange bool) (*v1alpha2.VirtualMachine, *virtv1.VirtualMachineInstance) { + vm := vmbuilder.NewEmpty(name, namespace) + kvvmi := newEmptyKVVMI(name, namespace) + + if hasHotMemoryChange { + kvvmi.Status.Conditions = append(kvvmi.Status.Conditions, virtv1.VirtualMachineInstanceCondition{ + Type: virtv1.VirtualMachineInstanceMemoryChange, + Status: corev1.ConditionTrue, + }) + } + return vm, kvvmi + } + + newOnceMigrationMock := func(shouldMigrate bool) *OneShotMigrationMock { + return &OneShotMigrationMock{ + OnceMigrateFunc: func(ctx context.Context, vm *v1alpha2.VirtualMachine, annotationKey, annotationExpectedValue string) (bool, error) { + if shouldMigrate { + return true, serviceCompleteErr + } + return false, nil + }, + } + } + + type testResourcesSettings struct { + hasHotMemoryChangeCondition bool + shouldMigrate bool + } + + DescribeTable("HotplugResourcesHandler should return serviceCompleteErr if migration executed", + func(settings testResourcesSettings) { + vm, kvvmi := newVMAndKVVMI(settings.hasHotMemoryChangeCondition) + fakeClient = setupEnvironment(vm, kvvmi) + + mockMigration := newOnceMigrationMock(settings.shouldMigrate) + + h := NewHotplugHandler(fakeClient, mockMigration) + _, err := h.Handle(ctx, vm) + + if settings.hasHotMemoryChangeCondition && !settings.shouldMigrate { + Expect(err).ToNot(HaveOccurred()) + } else if settings.hasHotMemoryChangeCondition { + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(serviceCompleteErr)) + } else { + Expect(err).NotTo(HaveOccurred()) + } + }, + Entry( + "Migration should be executed on hotMemoryChange condition", + testResourcesSettings{ + hasHotMemoryChangeCondition: true, + shouldMigrate: true, + }, + ), + Entry( + "Migration should not be executed the second time", + testResourcesSettings{ + hasHotMemoryChangeCondition: true, + shouldMigrate: false, + }, + ), + Entry( + "Migration should not be executed without hotMemoryChange condition", + testResourcesSettings{ + hasHotMemoryChangeCondition: false, + }, + ), + ) +}) diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/watcher/kvvmi.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/watcher/kvvmi.go index a473c88654..59eec3b3cd 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/internal/watcher/kvvmi.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/watcher/kvvmi.go @@ -54,8 +54,9 @@ func (w *KVVMIWatcher) Watch(mgr manager.Manager, ctr controller.Controller) err CreateFunc: func(e event.TypedCreateEvent[*virtv1.VirtualMachineInstance]) bool { return false }, DeleteFunc: func(e event.TypedDeleteEvent[*virtv1.VirtualMachineInstance]) bool { return false }, UpdateFunc: func(e event.TypedUpdateEvent[*virtv1.VirtualMachineInstance]) bool { - cond, _ := conditions.GetKVVMICondition(conditions.VirtualMachineInstanceNodePlacementNotMatched, e.ObjectNew.Status.Conditions) - return cond.Status == corev1.ConditionTrue + nodePlacementCondition, _ := conditions.GetKVVMICondition(conditions.VirtualMachineInstanceNodePlacementNotMatched, e.ObjectNew.Status.Conditions) + hotMemoryChangeCondition, _ := conditions.GetKVVMICondition(virtv1.VirtualMachineInstanceMemoryChange, e.ObjectNew.Status.Conditions) + return nodePlacementCondition.Status == corev1.ConditionTrue || hotMemoryChangeCondition.Status == corev1.ConditionTrue }, }, ), diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/workload_updater_controller.go b/images/virtualization-artifact/pkg/controller/workload-updater/workload_updater_controller.go index fc8b474683..8240277b39 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/workload_updater_controller.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/workload_updater_controller.go @@ -27,6 +27,7 @@ import ( "github.com/deckhouse/deckhouse/pkg/log" "github.com/deckhouse/virtualization-controller/pkg/controller/workload-updater/internal/handler" "github.com/deckhouse/virtualization-controller/pkg/controller/workload-updater/internal/service" + "github.com/deckhouse/virtualization-controller/pkg/featuregates" "github.com/deckhouse/virtualization-controller/pkg/logger" ) @@ -48,6 +49,10 @@ func SetupController( handler.NewFirmwareHandler(client, service.NewOneShotMigrationService(client, "firmware-update-"), firmwareImage, namespace, virtControllerName), handler.NewNodePlacementHandler(client, service.NewOneShotMigrationService(client, "nodeplacement-update-")), } + if featuregates.Default().Enabled(featuregates.HotplugMemoryWithLiveMigration) { + hotplugHandler := handler.NewHotplugHandler(client, service.NewOneShotMigrationService(client, "hotplug-resources-")) + handlers = append(handlers, hotplugHandler) + } r := NewReconciler(client, handlers) c, err := controller.New(ControllerName, mgr, controller.Options{ diff --git a/images/virtualization-artifact/pkg/featuregates/featuregate.go b/images/virtualization-artifact/pkg/featuregates/featuregate.go index 38e06075fe..8d567ae151 100644 --- a/images/virtualization-artifact/pkg/featuregates/featuregate.go +++ b/images/virtualization-artifact/pkg/featuregates/featuregate.go @@ -30,6 +30,7 @@ const ( VolumeMigration featuregate.Feature = "VolumeMigration" TargetMigration featuregate.Feature = "TargetMigration" USB featuregate.Feature = "USB" + HotplugMemoryWithLiveMigration featuregate.Feature = "HotplugMemoryWithLiveMigration" ) var featureSpecs = map[featuregate.Feature]featuregate.FeatureSpec{ @@ -57,6 +58,11 @@ var featureSpecs = map[featuregate.Feature]featuregate.FeatureSpec{ LockToDefault: true, PreRelease: featuregate.Alpha, }, + HotplugMemoryWithLiveMigration: { + Default: false, + LockToDefault: version.GetEdition() == version.EditionCE, + PreRelease: featuregate.Alpha, + }, } var ( diff --git a/openapi/config-values.yaml b/openapi/config-values.yaml index b34e7ec2c2..e46d2362fc 100644 --- a/openapi/config-values.yaml +++ b/openapi/config-values.yaml @@ -277,3 +277,15 @@ properties: enum: - "text" - "json" + featureGates: + type: array + description: | + Enable experimental or early access features. + + - `HotplugCPUWithLiveMigration` — enable live changing of cpu cores number. (Not available in CE); + - `HotplugMemoryWithLiveMigration` — enable live changing of memory size. (Not available in CE); + items: + type: string + enum: + - "HotplugCPUWithLiveMigration" + - "HotplugMemoryWithLiveMigration" diff --git a/openapi/doc-ru-config-values.yaml b/openapi/doc-ru-config-values.yaml index 442193611c..5aaf65aae6 100644 --- a/openapi/doc-ru-config-values.yaml +++ b/openapi/doc-ru-config-values.yaml @@ -174,3 +174,12 @@ properties: Работает для следующих компонентов: - `virtualization-controller` + featureGates: + type: array + description: | + Включение экспериментальных или недостаточно обкатанных возможностей. + + - `HotplugCPUWithLiveMigration` — включить изменение количества ядер процессора без перезагрузки. (Не доступно в CE); + - `HotplugMemoryWithLiveMigration` — включить изменение размера памяти без перезагрузки. (Не доступно в CE); + items: + type: string diff --git a/templates/kubevirt/kubevirt.yaml b/templates/kubevirt/kubevirt.yaml index 47c8ed8b67..55ad3fcd6c 100644 --- a/templates/kubevirt/kubevirt.yaml +++ b/templates/kubevirt/kubevirt.yaml @@ -356,11 +356,28 @@ env: patch: '{"spec":{"template":{"metadata":{"labels":{"security.deckhouse.io/security-policy-exception": "virt-handler-ds"}}}}}' type: strategic - # Change host path for directory with capabilities xml files. We have custom qemu with different - # machine types thus it conflicts with the original kubevirt. +{{ define "virt-handler-rewrite-host-path-volumes"}} +volumes: +# Directory with capabilities xml files. We have custom qemu with different +# machine types thus it conflicts with the original kubevirt. +- name: node-labeller + hostPath: + path: /var/run/d8-virtualization/node-labeller +# Other directories to communicate with virt-launcher. +# Also rewrite to prevent errors. +- name: libvirt-runtimes + hostPath: + path: /var/run/d8-virtualization/libvirt-runtimes +- name: virt-share-dir + hostPath: + path: /var/run/d8-virtualization/kubevirt +- name: virt-private-dir + hostPath: + path: /var/run/d8-virtualization/kubevirt-private +{{- end }} - resourceName: virt-handler resourceType: DaemonSet - patch: '{"spec":{"template":{"spec":{"volumes":[{"name":"node-labeller","hostPath":{"path":"/var/run/d8-virtualization/node-labeller"}}]}}}}' + patch: '{"spec":{"template":{"spec": {{include "virt-handler-rewrite-host-path-volumes" . | fromYaml | toJson }} }}}' type: strategic imagePullPolicy: IfNotPresent diff --git a/templates/virtualization-controller/_helpers.tpl b/templates/virtualization-controller/_helpers.tpl index 8db602dd1e..3d799bf0fd 100644 --- a/templates/virtualization-controller/_helpers.tpl +++ b/templates/virtualization-controller/_helpers.tpl @@ -117,3 +117,16 @@ true - name: KUBE_APISERVER_FEATURE_GATES value: {{ .Values.virtualization.internal.kubeAPIServerFeatureGates | toJson | quote }} {{- end }} + +{{- define "virtualization-controller.feature-gates-flag-args-item" }} +{{- $gates := list }} +{{- if (.Values.global.enabledModules | has "sdn") }} +{{- $gates = append $gates "SDN=true" }} +{{- end }} +{{- range $feat := .Values.virtualization.internal.moduleConfig.featureGates }} +{{- $gates = append $gates (printf "%s=true" $feat)}} +{{- end }} +{{- if gt (len $gates) 0 }} +- --feature-gates={{$gates | join ","}} +{{- end }} +{{- end }} diff --git a/templates/virtualization-controller/deployment.yaml b/templates/virtualization-controller/deployment.yaml index 1ee253d2b0..8634a067c9 100644 --- a/templates/virtualization-controller/deployment.yaml +++ b/templates/virtualization-controller/deployment.yaml @@ -81,10 +81,8 @@ spec: {{- include "helm_lib_module_container_security_context_read_only_root_filesystem_capabilities_drop_all_pss_restricted" . | nindent 10 }} image: {{ include "helm_lib_module_image" (list . "virtualizationController") }} imagePullPolicy: IfNotPresent - {{- if (.Values.global.enabledModules | has "sdn") }} args: - - --feature-gates=SDN=true - {{- end }} + {{ include "virtualization-controller.feature-gates-flag-args-item" . | nindent 12 }} volumeMounts: - mountPath: /tmp/k8s-webhook-server/serving-certs name: admission-webhook-secret From 7e478135857fc82ad872bd9c0e3ccdf88f9aa58b Mon Sep 17 00:00:00 2001 From: Ivan Mikheykin Date: Tue, 7 Apr 2026 11:33:43 +0300 Subject: [PATCH 2/2] feat(vm): hotplug cpu (phase 1) (#2147) * feat(vm): hotplug cpu (phase 1) Simple implementation of CPU hotplug. - Change cpu cores setting from domain.resources/limits to domain.cpu fields. Change cores count instead of sockets. - Support old VMs to not require reboot on module update. - Requires changes in Kubevirt: https://github.com/deckhouse/3p-kubevirt/pull/82 --------- Signed-off-by: Ivan Mikheykin --- build/components/versions.yml | 2 +- images/virt-artifact/werf.inc.yaml | 1 - .../pkg/controller/kvbuilder/kvvm.go | 117 ++++++++++++++- .../pkg/controller/vm/internal/statistic.go | 137 ++++++++++++++---- .../controller/vm/internal/statistic_test.go | 94 ++++++++++++ .../pkg/controller/vm/internal/sync_kvvm.go | 84 ++++++----- .../controller/vm/internal/sync_metadata.go | 30 ++-- .../vm/internal/sync_power_state.go | 2 +- .../pkg/controller/vm/internal/util.go | 21 ++- .../validators/cpu_count_validator.go | 14 +- .../pkg/controller/vmchange/comparator_cpu.go | 80 ++++++++++ .../controller/vmchange/comparator_memory.go | 4 +- .../pkg/controller/vmchange/comparators.go | 29 ---- .../pkg/controller/vmchange/compare.go | 2 +- .../internal/handler/hotplug.go | 10 +- .../internal/watcher/kvvmi.go | 9 +- .../workload_updater_controller.go | 4 +- .../pkg/featuregates/featuregate.go | 6 + tools/kubeconform/fixtures/module-values.yaml | 1 + 19 files changed, 507 insertions(+), 140 deletions(-) create mode 100644 images/virtualization-artifact/pkg/controller/vmchange/comparator_cpu.go diff --git a/build/components/versions.yml b/build/components/versions.yml index 3b70ca0e54..c2a2cb2bc9 100644 --- a/build/components/versions.yml +++ b/build/components/versions.yml @@ -3,7 +3,7 @@ firmware: libvirt: v10.9.0 edk2: stable202411 core: - 3p-kubevirt: dvp/set-memory-limits-while-hotplugging + 3p-kubevirt: v1.6.2-v12n.23 3p-containerized-data-importer: v1.60.3-v12n.18 distribution: 2.8.3 package: diff --git a/images/virt-artifact/werf.inc.yaml b/images/virt-artifact/werf.inc.yaml index 767eff694a..f30560fba6 100644 --- a/images/virt-artifact/werf.inc.yaml +++ b/images/virt-artifact/werf.inc.yaml @@ -15,7 +15,6 @@ secrets: shell: install: - | - echo rebuild 33 echo "Git clone {{ $gitRepoName }} repository..." git clone --depth=1 $(cat /run/secrets/SOURCE_REPO)/{{ $gitRepoUrl }} --branch {{ $tag }} /src/kubevirt diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go index c1e3a1da12..1a55e10b9b 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go @@ -20,6 +20,8 @@ import ( "fmt" "maps" "os" + "strconv" + "strings" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -52,6 +54,16 @@ const ( EnableMemoryHotplugThreshold = 1 * 1024 * 1024 * 1024 // 1 Gi (no hotplug for VMs with less than 1Gi) ) +const ( + // VCPUTopologyDynamicCoresAnnotation annotation indicates "distributed by sockets" or "dynamic cores number" VCPU topology. + VCPUTopologyDynamicCoresAnnotation = "internal.virtualization.deckhouse.io/vcpu-topology-dynamic-cores" + + CPUResourcesRequestsFractionAnnotation = "internal.virtualization.deckhouse.io/cpu-resources-requests-fraction" + + // CPUMaxCoresPerSocket is a maximum number of cores per socket. + CPUMaxCoresPerSocket = 16 +) + type KVVMOptions struct { EnableParavirtualization bool OsType v1alpha2.OsType @@ -258,6 +270,17 @@ func (b *KVVM) SetTopologySpreadConstraint(topology []corev1.TopologySpreadConst } func (b *KVVM) SetCPU(cores int, coreFraction string) error { + // Support for VMs started with cpu configuration in requests-limits. + // TODO delete this in the future (around 3-4 more versions after enabling cpu hotplug by default). + if b.ResourceExists && isVMRunningWithCPUResources(b.Resource) { + return b.setCPUNonHotpluggable(cores, coreFraction) + } + return b.setCPUHotpluggable(cores, coreFraction) +} + +// setCPUNonHotpluggable translates cpu configuration to requests and limit in KVVM. +// Note: this is a first implementation, cpu hotplug is not compatible with this strategy. +func (b *KVVM) setCPUNonHotpluggable(cores int, coreFraction string) error { domainSpec := &b.Resource.Spec.Template.Spec.Domain if domainSpec.CPU == nil { domainSpec.CPU = &virtv1.CPU{} @@ -266,6 +289,7 @@ func (b *KVVM) SetCPU(cores int, coreFraction string) error { if err != nil { return err } + cpuLimit := GetCPULimit(cores) if domainSpec.Resources.Requests == nil { domainSpec.Resources.Requests = make(map[corev1.ResourceName]resource.Quantity) @@ -284,6 +308,38 @@ func (b *KVVM) SetCPU(cores int, coreFraction string) error { return nil } +// setCPUHotpluggable translates cpu configuration to settings in domain.cpu field. +// This field is compatible with memory hotplug. +// Also, remove requests-limits for memory if any. +// Note: we swap cores and sockets to bypass vm-validation webhook. +func (b *KVVM) setCPUHotpluggable(cores int, coreFraction string) error { + domainSpec := &b.Resource.Spec.Template.Spec.Domain + if domainSpec.CPU == nil { + domainSpec.CPU = &virtv1.CPU{} + } + + fraction, err := GetCPUFraction(coreFraction) + if err != nil { + return err + } + b.SetKVVMIAnnotation(CPUResourcesRequestsFractionAnnotation, strconv.Itoa(fraction)) + + socketsNeeded, coresPerSocketNeeded := vm.CalculateCoresAndSockets(cores) + // Use "dynamic cores" hotplug strategy. + // Workaround: swap cores and sockets in domainSpec to bypass vm-validator webhook. + b.SetKVVMIAnnotation(VCPUTopologyDynamicCoresAnnotation, "") + domainSpec.CPU.Cores = uint32(socketsNeeded) + domainSpec.CPU.Sockets = uint32(coresPerSocketNeeded) + domainSpec.CPU.MaxSockets = CPUMaxCoresPerSocket + + // Remove CPU limits and requests if set by previous implementation. + res := &b.Resource.Spec.Template.Spec.Domain.Resources + delete(res.Requests, corev1.ResourceCPU) + delete(res.Limits, corev1.ResourceCPU) + + return nil +} + // SetMemory sets memory in kvvm. // There are 2 possibilities to set memory: // 1. Use domain.memory.guest field: it enabled memory hotplugging, but not set resources.limits. @@ -293,7 +349,7 @@ func (b *KVVM) SetCPU(cores int, coreFraction string) error { func (b *KVVM) SetMemory(memorySize resource.Quantity) { // Support for VMs started with memory size in requests-limits. // TODO delete this in the future (around 3-4 more versions after enabling memory hotplug by default). - if b.ResourceExists && isVMRunningWithMemoryResources(b.Resource) { + if b.ResourceExists && shouldKeepMemoryNonHotpluggable(b.Resource) { b.setMemoryNonHotpluggable(memorySize) return } @@ -349,7 +405,7 @@ func (b *KVVM) setMemoryHotpluggable(memorySize resource.Quantity) { delete(res.Limits, corev1.ResourceMemory) } -func isVMRunningWithMemoryResources(kvvm *virtv1.VirtualMachine) bool { +func isVMRunningWithCPUResources(kvvm *virtv1.VirtualMachine) bool { if kvvm == nil { return false } @@ -359,10 +415,61 @@ func isVMRunningWithMemoryResources(kvvm *virtv1.VirtualMachine) bool { } res := kvvm.Spec.Template.Spec.Domain.Resources - _, hasMemoryRequests := res.Requests[corev1.ResourceMemory] - _, hasMemoryLimits := res.Limits[corev1.ResourceMemory] + _, hasCPURequests := res.Requests[corev1.ResourceCPU] + _, hasCPULimits := res.Limits[corev1.ResourceCPU] + + return hasCPURequests && hasCPULimits +} + +func shouldKeepMemoryNonHotpluggable(kvvm *virtv1.VirtualMachine) bool { + if kvvm == nil { + return false + } + + if kvvm.Status.PrintableStatus == virtv1.VirtualMachineStatusRunning || kvvm.Status.PrintableStatus == virtv1.VirtualMachineStatusMigrating { + // Running or Migrating machines with memory resources should keep as non-hotpluggable. + // Machines without memory resources should proceed as hotpluggable. + res := kvvm.Spec.Template.Spec.Domain.Resources + _, hasMemoryRequests := res.Requests[corev1.ResourceMemory] + _, hasMemoryLimits := res.Limits[corev1.ResourceMemory] - return hasMemoryRequests && hasMemoryLimits + return hasMemoryRequests && hasMemoryLimits + } + + // Proceed as hotpluggable if machine is not Running or Migrating. + return false +} + +func GetCPUFraction(cpuFraction string) (int, error) { + if cpuFraction == "" { + return 100, nil + } + fraction := intstr.FromString(cpuFraction) + value, _, err := getIntOrPercentValueSafely(&fraction) + if err != nil { + return 0, fmt.Errorf("invalid value for cpu fraction: %w", err) + } + return value, nil +} + +func getIntOrPercentValueSafely(intOrStr *intstr.IntOrString) (int, bool, error) { + switch intOrStr.Type { + case intstr.Int: + return intOrStr.IntValue(), false, nil + case intstr.String: + s := intOrStr.StrVal + if !strings.HasSuffix(s, "%") { + return 0, false, fmt.Errorf("invalid type: string is not a percentage") + } + s = strings.TrimSuffix(intOrStr.StrVal, "%") + + v, err := strconv.Atoi(s) + if err != nil { + return 0, false, fmt.Errorf("invalid value %q: %w", intOrStr.StrVal, err) + } + return v, true, nil + } + return 0, false, fmt.Errorf("invalid type: neither int nor percentage") } func GetCPURequest(cores int, coreFraction string) (*resource.Quantity, error) { diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/statistic.go b/images/virtualization-artifact/pkg/controller/vm/internal/statistic.go index 0b0f504e15..41471b19cb 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/statistic.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/statistic.go @@ -18,6 +18,7 @@ package internal import ( "context" + "fmt" "math" "strconv" "time" @@ -30,6 +31,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/deckhouse/virtualization-controller/pkg/common/vm" + "github.com/deckhouse/virtualization-controller/pkg/controller/kvbuilder" "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -68,7 +70,9 @@ func (h *StatisticHandler) Handle(ctx context.Context, s state.VirtualMachineSta } h.syncPods(changed, pod, pods) - h.syncResources(changed, kvvmi, pod) + if err := h.syncResources(changed, kvvmi, pod); err != nil { + return reconcile.Result{}, err + } return reconcile.Result{}, nil } @@ -80,48 +84,52 @@ func (h *StatisticHandler) Name() string { func (h *StatisticHandler) syncResources(changed *v1alpha2.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, pod *corev1.Pod, -) { +) error { if changed == nil { - return + return nil } var resources v1alpha2.ResourcesStatus switch pod { case nil: var ( - cpuKVVMIRequest resource.Quantity + cpuKVVMIRequest *resource.Quantity memorySize resource.Quantity - cores int topology v1alpha2.Topology coreFraction string ) if kvvmi == nil { memorySize = changed.Spec.Memory.Size - cores = changed.Spec.CPU.Cores - coreFraction = changed.Spec.CPU.CoreFraction - sockets, coresPerSocket := vm.CalculateCoresAndSockets(cores) + sockets, coresPerSocket := vm.CalculateCoresAndSockets(changed.Spec.CPU.Cores) topology = v1alpha2.Topology{CoresPerSocket: coresPerSocket, Sockets: sockets} + coreFraction = changed.Spec.CPU.CoreFraction } else { - cpuKVVMIRequest = kvvmi.Spec.Domain.Resources.Requests[corev1.ResourceCPU] + var err error + cpuKVVMIRequest, err = h.getCoresRequestedByKVVMI(kvvmi) + if err != nil { + return err + } memorySize = kvvmi.Spec.Domain.Resources.Requests[corev1.ResourceMemory] - cores = h.getCoresByKVVMI(kvvmi) coreFraction = h.getCoreFractionByKVVMI(kvvmi) topology = h.getCurrentTopologyByKVVMI(kvvmi) } resources = v1alpha2.ResourcesStatus{ CPU: v1alpha2.CPUStatus{ - Cores: cores, - CoreFraction: coreFraction, - RequestedCores: cpuKVVMIRequest, - Topology: topology, + Cores: topology.CoresPerSocket * topology.Sockets, + CoreFraction: coreFraction, + Topology: topology, }, Memory: v1alpha2.MemoryStatus{ Size: memorySize, }, } + if cpuKVVMIRequest != nil { + resources.CPU.RequestedCores = *cpuKVVMIRequest + } + default: if kvvmi == nil { - return + return nil } var ctr corev1.Container for _, container := range pod.Spec.Containers { @@ -130,15 +138,18 @@ func (h *StatisticHandler) syncResources(changed *v1alpha2.VirtualMachine, } } - cpuKVVMIRequest := kvvmi.Spec.Domain.Resources.Requests[corev1.ResourceCPU] + coreFraction := h.getCoreFractionByKVVMI(kvvmi) + topology := h.getCurrentTopologyByKVVMI(kvvmi) + cores := topology.CoresPerSocket * topology.Sockets + + cpuFractionRequests, err := h.getCoresRequestedByKVVMI(kvvmi) + if err != nil { + return fmt.Errorf("get core fraction by kvvmi: %w", err) + } cpuPODRequest := ctr.Resources.Requests[corev1.ResourceCPU] cpuOverhead := cpuPODRequest.DeepCopy() - cpuOverhead.Sub(cpuKVVMIRequest) - - cores := h.getCoresByKVVMI(kvvmi) - coreFraction := h.getCoreFractionByKVVMI(kvvmi) - topology := h.getCurrentTopologyByKVVMI(kvvmi) + cpuOverhead.Sub(*cpuFractionRequests) memoryKVVMIRequest := kvvmi.Spec.Domain.Resources.Requests[corev1.ResourceMemory] memoryPodRequest := ctr.Resources.Requests[corev1.ResourceMemory] @@ -152,7 +163,7 @@ func (h *StatisticHandler) syncResources(changed *v1alpha2.VirtualMachine, CPU: v1alpha2.CPUStatus{ Cores: cores, CoreFraction: coreFraction, - RequestedCores: cpuKVVMIRequest, + RequestedCores: *cpuFractionRequests, RuntimeOverhead: cpuOverhead, Topology: topology, }, @@ -163,44 +174,108 @@ func (h *StatisticHandler) syncResources(changed *v1alpha2.VirtualMachine, } } changed.Status.Resources = resources + return nil } +// getCoresByKVVMI +// TODO refactor: no need to get cores from limits after enabling CPU hotplug, kvvmi.Spec.Domain.CPU should be enough. func (h *StatisticHandler) getCoresByKVVMI(kvvmi *virtv1.VirtualMachineInstance) int { if kvvmi == nil { return -1 } - cpuKVVMILimit := kvvmi.Spec.Domain.Resources.Limits[corev1.ResourceCPU] - return int(cpuKVVMILimit.Value()) + + cpuKVVMILimit, hasLimits := kvvmi.Spec.Domain.Resources.Limits[corev1.ResourceCPU] + if hasLimits { + return int(cpuKVVMILimit.Value()) + } + + return 1 } func (h *StatisticHandler) getCoreFractionByKVVMI(kvvmi *virtv1.VirtualMachineInstance) string { if kvvmi == nil { return "" } + // Fraction is stored in annotation after enabling CPU hotplug. + cpuFractionStr, hasAnno := kvvmi.Annotations[kvbuilder.CPUResourcesRequestsFractionAnnotation] + if hasAnno { + return cpuFractionStr + "%" + } + // Also support previous implementation: calculate from requests and limits values. cpuKVVMIRequest := kvvmi.Spec.Domain.Resources.Requests[corev1.ResourceCPU] return strconv.Itoa(int(cpuKVVMIRequest.MilliValue())*100/(h.getCoresByKVVMI(kvvmi)*1000)) + "%" } +func (h *StatisticHandler) getCoresRequestedByKVVMI(kvvmi *virtv1.VirtualMachineInstance) (*resource.Quantity, error) { + if kvvmi == nil { + return nil, nil + } + // Fraction is stored in annotation after enabling CPU hotplug. + cpuFractionStr, hasAnno := kvvmi.Annotations[kvbuilder.CPUResourcesRequestsFractionAnnotation] + if hasAnno { + if kvvmi.Spec.Domain.CPU == nil { + return nil, fmt.Errorf("enabled dynamic cores with annotation %s, but missing spec.domain.cpu", kvbuilder.CPUResourcesRequestsFractionAnnotation) + } + cores := kvvmi.Spec.Domain.CPU.Cores * kvvmi.Spec.Domain.CPU.Sockets + + cpuFraction, err := strconv.Atoi(cpuFractionStr) + if err != nil { + return nil, err + } + + if cpuFraction <= 0 || cpuFraction > 100 { + cpuFraction = 100 + } + if cpuFraction == 100 { + return resource.NewQuantity(int64(cores), resource.DecimalSI), nil + } + + // Use multiplier to calculate fraction of millis. + requested := cores * 1000 + // Round up, to always return integer number of millis. + value := int64(math.Ceil(float64(cpuFraction) * (float64(requested)) / 100)) + return resource.NewMilliQuantity(value, resource.DecimalSI), nil + } + + // Also support previous implementation: return cpu requests if set. + if reqCPU, hasCPURequests := kvvmi.Spec.Domain.Resources.Requests[corev1.ResourceCPU]; hasCPURequests { + return &reqCPU, nil + } + + return nil, nil +} + func (h *StatisticHandler) getCurrentTopologyByKVVMI(kvvmi *virtv1.VirtualMachineInstance) v1alpha2.Topology { if kvvmi == nil { return v1alpha2.Topology{} } + cores := -1 + sockets := -1 + if kvvmi.Status.CurrentCPUTopology != nil { - return v1alpha2.Topology{ - CoresPerSocket: int(kvvmi.Status.CurrentCPUTopology.Cores), - Sockets: int(kvvmi.Status.CurrentCPUTopology.Sockets), - } + cores = int(kvvmi.Status.CurrentCPUTopology.Cores) + sockets = int(kvvmi.Status.CurrentCPUTopology.Sockets) } if kvvmi.Spec.Domain.CPU != nil { + cores = int(kvvmi.Spec.Domain.CPU.Cores) + sockets = int(kvvmi.Spec.Domain.CPU.Sockets) + } + + if _, isDynamicCores := kvvmi.Annotations[kvbuilder.VCPUTopologyDynamicCoresAnnotation]; isDynamicCores { + // Swap cores and sockets. + cores, sockets = sockets, cores + } + + if cores > 0 && sockets > 0 { return v1alpha2.Topology{ - CoresPerSocket: int(kvvmi.Spec.Domain.CPU.Cores), - Sockets: int(kvvmi.Spec.Domain.CPU.Sockets), + CoresPerSocket: cores, + Sockets: sockets, } } - cores := h.getCoresByKVVMI(kvvmi) + cores = h.getCoresByKVVMI(kvvmi) sockets, coresPerSocket := vm.CalculateCoresAndSockets(cores) return v1alpha2.Topology{CoresPerSocket: coresPerSocket, Sockets: sockets} } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/statistic_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/statistic_test.go index 06a0cb659e..2b460b9453 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/statistic_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/statistic_test.go @@ -30,6 +30,7 @@ import ( vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm" "github.com/deckhouse/virtualization-controller/pkg/common/testutil" + "github.com/deckhouse/virtualization-controller/pkg/controller/kvbuilder" "github.com/deckhouse/virtualization-controller/pkg/controller/reconciler" "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" "github.com/deckhouse/virtualization/api/core/v1alpha2" @@ -82,6 +83,48 @@ var _ = Describe("TestStatisticHandler", func() { return kvvmi } + // Generate KVVMI with "dynamic cores" specifics: cores and sockets are intentionally + // swapped to bypass kvvm validations. + newKVVMIHotplug := func(cores, sockets, maxCores int, cpuFraction, memory, maxMemory string) *virtv1.VirtualMachineInstance { + kvvmi := newEmptyKVVMI(vmName, vmNamespace) + memoryGuest := resource.MustParse(memory) + memoryMaxGuest := resource.MustParse(maxMemory) + + kvvmi.SetAnnotations(map[string]string{ + kvbuilder.CPUResourcesRequestsFractionAnnotation: cpuFraction, + kvbuilder.VCPUTopologyDynamicCoresAnnotation: "", + }) + kvvmi.Spec = virtv1.VirtualMachineInstanceSpec{ + Domain: virtv1.DomainSpec{ + CPU: &virtv1.CPU{ + Cores: uint32(sockets), + Sockets: uint32(cores), + MaxSockets: uint32(maxCores), + Threads: 1, + }, + Memory: &virtv1.Memory{ + Guest: &memoryGuest, + MaxGuest: &memoryMaxGuest, + }, + Resources: virtv1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: memoryGuest, + }, + }, + }, + } + kvvmi.Status = virtv1.VirtualMachineInstanceStatus{ + ActivePods: map[types.UID]string{podUID: podName}, + NodeName: nodeName, + Phase: virtv1.Running, + CurrentCPUTopology: &virtv1.CPUTopology{ + Cores: uint32(sockets), + Sockets: uint32(cores), + }, + } + return kvvmi + } + newPod := func(requestCPU, limitCPU, requestMemory, limitMemory string) *corev1.Pod { pod := newEmptyPOD(podName, vmNamespace, vmName) pod.UID = podUID @@ -209,6 +252,57 @@ var _ = Describe("TestStatisticHandler", func() { TopologyCoresPerSocket: 2, TopologySockets: 1, + MemorySize: 2147483648, + MemoryRuntimeOverhead: 0, + }, + ), + Entry("Hotplug enabled: 8 cores, 100% fraction, 2 Gi", + newVM(8, ptr.To("100%"), "2Gi"), + newKVVMIHotplug(8, 1, 16, "100", "2Gi", "256Gi"), + newPod("8", "8", "2Gi", "2Gi"), + expectedValues{ + CPUCores: 8, + CPUCoreFraction: "100%", + CPURequestedCores: 8000, + CPURuntimeOverhead: 0, + + TopologyCoresPerSocket: 8, + TopologySockets: 1, + + MemorySize: 2147483648, + MemoryRuntimeOverhead: 0, + }, + ), + Entry("Hotplug enabled: 8 cores, 25% fraction, 2 Gi", + newVM(8, ptr.To("25%"), "2Gi"), + newKVVMIHotplug(8, 1, 16, "25", "2Gi", "256Gi"), + newPod("2", "8", "2Gi", "2Gi"), + expectedValues{ + CPUCores: 8, + CPUCoreFraction: "25%", + CPURequestedCores: 2000, + CPURuntimeOverhead: 0, + + TopologyCoresPerSocket: 8, + TopologySockets: 1, + + MemorySize: 2147483648, + MemoryRuntimeOverhead: 0, + }, + ), + Entry("Hotplug enabled: 1 core, 25% fraction, 2 Gi", + newVM(1, ptr.To("25%"), "2Gi"), + newKVVMIHotplug(1, 1, 16, "25", "2Gi", "256Gi"), + newPod("250m", "1", "2Gi", "2Gi"), + expectedValues{ + CPUCores: 1, + CPUCoreFraction: "25%", + CPURequestedCores: 250, + CPURuntimeOverhead: 0, + + TopologyCoresPerSocket: 1, + TopologySockets: 1, + MemorySize: 2147483648, MemoryRuntimeOverhead: 0, }, 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 6e0e9e9cfc..56e7250a2d 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -264,7 +264,7 @@ func (h *SyncKvvmHandler) Name() string { } func (h *SyncKvvmHandler) isWaiting(vm *v1alpha2.VirtualMachine) bool { - return !checkVirtualMachineConfiguration(vm) + return !virtualMachineDependenciesAreReady(vm) } func (h *SyncKvvmHandler) syncKVVM(ctx context.Context, s state.VirtualMachineState, allChanges vmchange.SpecChanges) (bool, error) { @@ -377,32 +377,20 @@ func (h *SyncKvvmHandler) updateKVVM(ctx context.Context, s state.VirtualMachine return fmt.Errorf("update internal virtual machine: make kvvm from the virtual machine spec: %w", err) } - // Check for changes to skip unneeded updated. - isChanged, err := IsKVVMChanged(ctx, s, newKVVM) + currentKVVM, err := s.KVVM(ctx) if err != nil { - return fmt.Errorf("update internal virtual machine: detect changes: %w", err) + return fmt.Errorf("get current kvvm: %w", err) } - if isChanged { - memory := newKVVM.Spec.Template.Spec.Domain.Memory - if memory != nil && memory.MaxGuest != nil && memory.MaxGuest.IsZero() { - // Zero maxGuest is a special value to patch KVVM to unset maxGuest. - // Set it to nil for next update call. - memory.MaxGuest = nil - - // 2 operations: remove memory.maxGuest; set memory.guest. - // Remove is not enough, remove and set are needed both to pass the kubevirt vm-validator webhook. - patchBytes, err := patch.NewJSONPatch( - patch.WithRemove("/spec/template/spec/domain/memory/maxGuest"), - patch.WithReplace("/spec/template/spec/domain/memory/guest", memory.Guest.String()), - ).Bytes() - if err != nil { - return fmt.Errorf("prepare json patch to unset memory.maxGuest: %w", err) - } + // Check for changes to skip unneeded updated. + isChanged := IsKVVMChanged(currentKVVM, newKVVM) - if err = h.client.Patch(ctx, newKVVM, client.RawPatch(types.JSONPatchType, patchBytes)); err != nil { - return fmt.Errorf("patch internal virtual machine to unset memory.maxGuest: %w", err) - } + if isChanged { + // Update can't handle proper reset of memory fields, so patch-after-update: + // (1) make memory copy, (2) reset memory in newKVVM and (3) patch memory field after update. + domainMemory := saveKVVMDomainMemoryForPatching(currentKVVM, newKVVM) + if domainMemory != nil { + newKVVM.Spec.Template.Spec.Domain.Memory = currentKVVM.Spec.Template.Spec.Domain.Memory } if err = h.client.Update(ctx, newKVVM); err != nil { @@ -411,6 +399,22 @@ func (h *SyncKvvmHandler) updateKVVM(ctx context.Context, s state.VirtualMachine log.Info("Update internal virtual machine done", "name", newKVVM.Name) log.Debug("Update internal virtual machine done", "name", newKVVM.Name, "kvvm", newKVVM) + + if domainMemory != nil { + jsonPatch := patch.JSONPatch{} + // Removing memory.maxGuest is not enough, replace memory.guest is needed to pass the vm-validator webhook. + jsonPatch.Append( + patch.WithRemove("/spec/template/spec/domain/memory/maxGuest"), + patch.WithReplace("/spec/template/spec/domain/memory/guest", domainMemory.Guest.String()), + ) + patchBytes, err := jsonPatch.Bytes() + if err != nil { + return fmt.Errorf("prepare json patch for internal virtual machine: %w", err) + } + if err = h.client.Patch(ctx, newKVVM, client.RawPatch(types.JSONPatchType, patchBytes)); err != nil { + return fmt.Errorf("patch internal virtual machine before update: %w", err) + } + } } else { log.Debug("Update internal virtual machine is not needed", "name", newKVVM.Name, "kvvm", newKVVM) } @@ -418,6 +422,22 @@ func (h *SyncKvvmHandler) updateKVVM(ctx context.Context, s state.VirtualMachine return nil } +// saveKVVMDomainMemoryForPatching returns copy of domain memory if maxGuest becomes 0. +// +// Note: maxGuest=0 is an invalid value for the vm-validator webhook, +// kvbuilder sets maxGuest to 0 to indicate that KVVM needs to be patched +// to clear maxGuest value: it is not possible to clear the value with the Update +// once it was set previously. +func saveKVVMDomainMemoryForPatching(prevKVVM, newKVVM *virtv1.VirtualMachine) *virtv1.Memory { + prevMemory := prevKVVM.Spec.Template.Spec.Domain.Memory + newMemory := newKVVM.Spec.Template.Spec.Domain.Memory + if newMemory != nil && newMemory.MaxGuest != nil && newMemory.MaxGuest.IsZero() && + prevMemory != nil && prevMemory.MaxGuest != nil && !prevMemory.MaxGuest.IsZero() { + return newMemory.DeepCopy() + } + return nil +} + func MakeKVVMFromVMSpec(ctx context.Context, s state.VirtualMachineState) (*virtv1.VirtualMachine, error) { if s.VirtualMachine().IsEmpty() { return nil, nil @@ -488,22 +508,16 @@ func MakeKVVMFromVMSpec(ctx context.Context, s state.VirtualMachineState) (*virt } // IsKVVMChanged returns whether kvvm spec or special annotations are changed. -func IsKVVMChanged(ctx context.Context, s state.VirtualMachineState, kvvm *virtv1.VirtualMachine) (bool, error) { - currentKVVM, err := s.KVVM(ctx) - if err != nil { - return false, fmt.Errorf("get current kvvm: %w", err) +func IsKVVMChanged(prevKVVM, newKVVM *virtv1.VirtualMachine) bool { + if prevKVVM.Annotations[annotations.AnnVMLastAppliedSpec] != newKVVM.Annotations[annotations.AnnVMLastAppliedSpec] { + return true } - isChanged := currentKVVM.Annotations[annotations.AnnVMLastAppliedSpec] != kvvm.Annotations[annotations.AnnVMLastAppliedSpec] - - if !isChanged { - isChanged = currentKVVM.Annotations[annotations.AnnVMClassLastAppliedSpec] != kvvm.Annotations[annotations.AnnVMClassLastAppliedSpec] + if prevKVVM.Annotations[annotations.AnnVMClassLastAppliedSpec] != newKVVM.Annotations[annotations.AnnVMClassLastAppliedSpec] { + return true } - if !isChanged { - isChanged = !reflect.DeepEqual(kvvm.Spec, currentKVVM.Spec) - } - return isChanged, nil + return !reflect.DeepEqual(prevKVVM.Spec, newKVVM.Spec) } func (h *SyncKvvmHandler) loadLastAppliedSpec(vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) *v1alpha2.VirtualMachineSpec { diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_metadata.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_metadata.go index df72bb67d9..40e021a30e 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_metadata.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_metadata.go @@ -34,6 +34,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/merger" "github.com/deckhouse/virtualization-controller/pkg/common/patch" commonvm "github.com/deckhouse/virtualization-controller/pkg/common/vm" + "github.com/deckhouse/virtualization-controller/pkg/controller/kvbuilder" "github.com/deckhouse/virtualization-controller/pkg/controller/netmanager" "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" "github.com/deckhouse/virtualization/api/core/v1alpha2" @@ -188,10 +189,20 @@ func (h *SyncMetadataHandler) patchLabelsAndAnnotations(ctx context.Context, obj return h.client.Patch(ctx, obj, client.RawPatch(types.JSONPatchType, bytes)) } +var annotationsToKeep = []string{ + annotations.AnnNetworksSpec, + virtv1.AllowPodBridgeNetworkLiveMigrationAnnotation, + netmanager.AnnoIPAddressCNIRequest, + virtv1.USBMigrationStrategyAnn, + kvbuilder.CPUResourcesRequestsFractionAnnotation, + kvbuilder.VCPUTopologyDynamicCoresAnnotation, +} + // updateKVVMSpecTemplateMetadataAnnotations ensures that the special network annotation is present if it exists. // It also removes well-known annotations that are dangerous to propagate. func (h *SyncMetadataHandler) updateKVVMSpecTemplateMetadataAnnotations(currAnno, newAnno map[string]string) map[string]string { res := make(map[string]string, len(newAnno)) + for k, v := range newAnno { if k == annotations.AnnVMLastAppliedSpec || k == annotations.AnnVMClassLastAppliedSpec { continue @@ -200,20 +211,11 @@ func (h *SyncMetadataHandler) updateKVVMSpecTemplateMetadataAnnotations(currAnno res[k] = v } - if v, ok := currAnno[annotations.AnnNetworksSpec]; ok { - res[annotations.AnnNetworksSpec] = v - } - - if v, ok := currAnno[virtv1.AllowPodBridgeNetworkLiveMigrationAnnotation]; ok { - res[virtv1.AllowPodBridgeNetworkLiveMigrationAnnotation] = v - } - - if v, ok := currAnno[netmanager.AnnoIPAddressCNIRequest]; ok { - res[netmanager.AnnoIPAddressCNIRequest] = v - } - - if v, ok := currAnno[virtv1.USBMigrationStrategyAnn]; ok { - res[virtv1.USBMigrationStrategyAnn] = v + // Restore annotations set by kvbuilder. + for _, keepAnno := range annotationsToKeep { + if v, ok := currAnno[keepAnno]; ok { + res[keepAnno] = v + } } return commonvm.RemoveNonPropagatableAnnotations(res) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_power_state.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_power_state.go index f3939f0ff0..fbb1143601 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_power_state.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_power_state.go @@ -127,7 +127,7 @@ func (h *SyncPowerStateHandler) syncPowerState( }) changed := s.VirtualMachine().Changed() - isConfigurationApplied := checkVirtualMachineConfiguration(changed) + isConfigurationApplied := virtualMachineDependenciesAreReady(changed) maintenance, _ := conditions.GetCondition(vmcondition.TypeMaintenance, changed.Status.Conditions) var vmAction VMAction diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/util.go b/images/virtualization-artifact/pkg/controller/vm/internal/util.go index c47ee80776..9bd9ef9639 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/util.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/util.go @@ -95,14 +95,15 @@ type PhaseGetter func(vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) var mapPhases = map[virtv1.VirtualMachinePrintableStatus]PhaseGetter{ // VirtualMachineStatusStopped indicates that the virtual machine is currently stopped and isn't expected to start. virtv1.VirtualMachineStatusStopped: func(vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) v1alpha2.MachinePhase { - if vm != nil && kvvm != nil { - if !checkVirtualMachineConfiguration(vm) && - kvvm != nil && kvvm.Annotations[annotations.AnnVMStartRequested] == "true" { - return v1alpha2.MachinePending - } + if vm == nil { + return v1alpha2.MachineStopped + } + + if !virtualMachineDependenciesAreReady(vm) && kvvm.Annotations[annotations.AnnVMStartRequested] == "true" { + return v1alpha2.MachinePending } - if vm != nil && vm.Status.Phase == v1alpha2.MachinePending && + if vm.Status.Phase == v1alpha2.MachinePending && (vm.Spec.RunPolicy == v1alpha2.AlwaysOnPolicy || vm.Spec.RunPolicy == v1alpha2.AlwaysOnUnlessStoppedManually) { return v1alpha2.MachinePending } @@ -184,6 +185,11 @@ var mapPhases = map[virtv1.VirtualMachinePrintableStatus]PhaseGetter{ virtv1.VirtualMachineStatusWaitingForVolumeBinding: func(_ *v1alpha2.VirtualMachine, _ *virtv1.VirtualMachine) v1alpha2.MachinePhase { return v1alpha2.MachinePending }, + // VirtualMachineStatusWaitingForReceiver indicates that this virtual machine is a receiver VM and + // migration should start next. + virtv1.VirtualMachineStatusWaitingForReceiver: func(_ *v1alpha2.VirtualMachine, _ *virtv1.VirtualMachine) v1alpha2.MachinePhase { + return v1alpha2.MachineMigrating + }, kvvmEmptyPhase: func(_ *v1alpha2.VirtualMachine, _ *virtv1.VirtualMachine) v1alpha2.MachinePhase { return v1alpha2.MachinePending @@ -251,7 +257,8 @@ func podFinal(pod corev1.Pod) bool { return pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed } -func checkVirtualMachineConfiguration(vm *v1alpha2.VirtualMachine) bool { +// virtualMachineDependenciesAreReady returns whether VM +func virtualMachineDependenciesAreReady(vm *v1alpha2.VirtualMachine) bool { for _, c := range vm.Status.Conditions { switch vmcondition.Type(c.Type) { case vmcondition.TypeBlockDevicesReady: diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/cpu_count_validator.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/cpu_count_validator.go index a2949bbd93..17c2de3639 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/cpu_count_validator.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/cpu_count_validator.go @@ -22,6 +22,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + commonvm "github.com/deckhouse/virtualization-controller/pkg/common/vm" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -42,16 +43,11 @@ func (v *CPUCountValidator) ValidateUpdate(_ context.Context, _, newVM *v1alpha2 func (v *CPUCountValidator) Validate(vm *v1alpha2.VirtualMachine) (admission.Warnings, error) { cores := vm.Spec.CPU.Cores - switch { - case cores <= 16: + sockets, coresPerSocket := commonvm.CalculateCoresAndSockets(cores) + + if cores == sockets*coresPerSocket { return nil, nil - case cores > 16 && cores <= 32 && cores%2 != 0: - return nil, fmt.Errorf("the requested number of cores must be a multiple of 2") - case cores > 32 && cores <= 64 && cores%4 != 0: - return nil, fmt.Errorf("the requested number of cores must be a multiple of 4") - case cores > 64 && cores%8 != 0: - return nil, fmt.Errorf("the requested number of cores must be a multiple of 8") } - return nil, nil + return nil, fmt.Errorf("the requested number of cores must be a multiple of %d", sockets) } diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparator_cpu.go b/images/virtualization-artifact/pkg/controller/vmchange/comparator_cpu.go new file mode 100644 index 0000000000..945f14a48e --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparator_cpu.go @@ -0,0 +1,80 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package vmchange + +import ( + "k8s.io/component-base/featuregate" + + "github.com/deckhouse/virtualization-controller/pkg/common/vm" + "github.com/deckhouse/virtualization-controller/pkg/featuregates" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +type comparatorCPU struct { + featureGate featuregate.FeatureGate +} + +func NewComparatorCPU(featureGate featuregate.FeatureGate) VMSpecFieldComparator { + return &comparatorCPU{ + featureGate: featureGate, + } +} + +// Compare returns changes in the cpu section. +// // It supports CPU hotplug mechanism for cores changes. +// // It requires reboot if cpu fraction is changed or if COU hotplug is disabled. +func (c *comparatorCPU) Compare(current, desired *v1alpha2.VirtualMachineSpec) []FieldChange { + // Cores can be changed "on the fly" using CPU Hotplug ... + coresChangedAction := ActionApplyImmediate + // ... but sockets count change requires a reboot. + currentSockets, _ := vm.CalculateCoresAndSockets(current.CPU.Cores) + desiredSockets, _ := vm.CalculateCoresAndSockets(desired.CPU.Cores) + if currentSockets != desiredSockets { + coresChangedAction = ActionRestart + } + + // Require reboot if CPU hotplug is not enabled. + if !c.featureGate.Enabled(featuregates.HotplugCPUWithLiveMigration) { + coresChangedAction = ActionRestart + } + + coresChanges := compareInts("cpu.cores", current.CPU.Cores, desired.CPU.Cores, 0, coresChangedAction) + fractionChanges := compareStrings("cpu.coreFraction", current.CPU.CoreFraction, desired.CPU.CoreFraction, DefaultCPUCoreFraction, ActionRestart) + + // Yield full replace if both fields changed. + if HasChanges(coresChanges) && HasChanges(fractionChanges) { + return []FieldChange{ + { + Operation: ChangeReplace, + Path: "cpu", + CurrentValue: current.CPU, + DesiredValue: desired.CPU, + ActionRequired: ActionRestart, + }, + } + } + + if HasChanges(coresChanges) { + return coresChanges + } + + if HasChanges(fractionChanges) { + return fractionChanges + } + + return nil +} diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go b/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go index c863cf1b46..c00d33aa5c 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go @@ -43,8 +43,8 @@ func NewComparatorMemory(featureGate featuregate.FeatureGate) VMSpecFieldCompara // Note: memory hotplug is enabled if VM has more than 1Gi of RAM. func (c *comparatorMemory) Compare(current, desired *v1alpha2.VirtualMachineSpec) []FieldChange { hotplugThreshold := resource.NewQuantity(kvbuilder.EnableMemoryHotplugThreshold, resource.BinarySI) - isHotpluggable := current.Memory.Size.Cmp(*hotplugThreshold) > 0 - isHotpluggableDesired := desired.Memory.Size.Cmp(*hotplugThreshold) > 0 + isHotpluggable := current.Memory.Size.Cmp(*hotplugThreshold) >= 0 + isHotpluggableDesired := desired.Memory.Size.Cmp(*hotplugThreshold) >= 0 actionType := ActionRestart if isHotpluggable && isHotpluggableDesired { diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparators.go b/images/virtualization-artifact/pkg/controller/vmchange/comparators.go index 529d6bed7e..7de8cbf481 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/comparators.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparators.go @@ -122,35 +122,6 @@ func compareBootloader(current, desired *v1alpha2.VirtualMachineSpec) []FieldCha ) } -// compareCPU returns changes in the cpu section. -func compareCPU(current, desired *v1alpha2.VirtualMachineSpec) []FieldChange { - coresChanges := compareInts("cpu.cores", current.CPU.Cores, desired.CPU.Cores, 0, ActionRestart) - fractionChanges := compareStrings("cpu.coreFraction", current.CPU.CoreFraction, desired.CPU.CoreFraction, DefaultCPUCoreFraction, ActionRestart) - - // Yield full replace if both fields changed. - if HasChanges(coresChanges) && HasChanges(fractionChanges) { - return []FieldChange{ - { - Operation: ChangeReplace, - Path: "cpu", - CurrentValue: current.CPU, - DesiredValue: desired.CPU, - ActionRequired: ActionRestart, - }, - } - } - - if HasChanges(coresChanges) { - return coresChanges - } - - if HasChanges(fractionChanges) { - return fractionChanges - } - - return nil -} - func compareProvisioning(current, desired *v1alpha2.VirtualMachineSpec) []FieldChange { changes := compareEmpty( "provisioning", diff --git a/images/virtualization-artifact/pkg/controller/vmchange/compare.go b/images/virtualization-artifact/pkg/controller/vmchange/compare.go index aa1f44259b..d734167761 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/compare.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/compare.go @@ -75,7 +75,7 @@ func (v *VMSpecComparator) comparators() []VMSpecFieldComparator { vmSpecFieldComparator(compareEnableParavirtualization), vmSpecFieldComparator(compareOSType), vmSpecFieldComparator(compareBootloader), - vmSpecFieldComparator(compareCPU), + NewComparatorCPU(v.featureGate), NewComparatorMemory(v.featureGate), vmSpecFieldComparator(compareBlockDevices), vmSpecFieldComparator(compareProvisioning), diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/hotplug.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/hotplug.go index c2bbdeb6f2..64845b963a 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/hotplug.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/handler/hotplug.go @@ -18,6 +18,7 @@ package handler import ( "context" + "fmt" corev1 "k8s.io/api/core/v1" virtv1 "kubevirt.io/api/core/v1" @@ -56,7 +57,12 @@ func (h *HotplugHandler) Handle(ctx context.Context, vm *v1alpha2.VirtualMachine } cond, _ := conditions.GetKVVMICondition(virtv1.VirtualMachineInstanceMemoryChange, kvvmi.Status.Conditions) - if cond.Status != corev1.ConditionTrue { + isMemoryHotplug := cond.Status == corev1.ConditionTrue + + cond, _ = conditions.GetKVVMICondition(virtv1.VirtualMachineInstanceVCPUChange, kvvmi.Status.Conditions) + isCPUHotplug := cond.Status == corev1.ConditionTrue + + if !isCPUHotplug && !isMemoryHotplug { return reconcile.Result{}, nil } @@ -76,5 +82,5 @@ func (h *HotplugHandler) Name() string { } func getHotplugResourcesSum(vm *v1alpha2.VirtualMachine) string { - return vm.Spec.Memory.Size.String() + return fmt.Sprintf("cpu.cores=%d,memory.size=%s", vm.Spec.CPU.Cores, vm.Spec.Memory.Size.String()) } diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/internal/watcher/kvvmi.go b/images/virtualization-artifact/pkg/controller/workload-updater/internal/watcher/kvvmi.go index 59eec3b3cd..f58f1d79be 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/internal/watcher/kvvmi.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/internal/watcher/kvvmi.go @@ -55,8 +55,15 @@ func (w *KVVMIWatcher) Watch(mgr manager.Manager, ctr controller.Controller) err DeleteFunc: func(e event.TypedDeleteEvent[*virtv1.VirtualMachineInstance]) bool { return false }, UpdateFunc: func(e event.TypedUpdateEvent[*virtv1.VirtualMachineInstance]) bool { nodePlacementCondition, _ := conditions.GetKVVMICondition(conditions.VirtualMachineInstanceNodePlacementNotMatched, e.ObjectNew.Status.Conditions) + if nodePlacementCondition.Status == corev1.ConditionTrue { + return true + } hotMemoryChangeCondition, _ := conditions.GetKVVMICondition(virtv1.VirtualMachineInstanceMemoryChange, e.ObjectNew.Status.Conditions) - return nodePlacementCondition.Status == corev1.ConditionTrue || hotMemoryChangeCondition.Status == corev1.ConditionTrue + if hotMemoryChangeCondition.Status == corev1.ConditionTrue { + return true + } + hotCPUChangeCondition, _ := conditions.GetKVVMICondition(virtv1.VirtualMachineInstanceVCPUChange, e.ObjectNew.Status.Conditions) + return hotCPUChangeCondition.Status == corev1.ConditionTrue }, }, ), diff --git a/images/virtualization-artifact/pkg/controller/workload-updater/workload_updater_controller.go b/images/virtualization-artifact/pkg/controller/workload-updater/workload_updater_controller.go index 8240277b39..150aa322a1 100644 --- a/images/virtualization-artifact/pkg/controller/workload-updater/workload_updater_controller.go +++ b/images/virtualization-artifact/pkg/controller/workload-updater/workload_updater_controller.go @@ -49,7 +49,9 @@ func SetupController( handler.NewFirmwareHandler(client, service.NewOneShotMigrationService(client, "firmware-update-"), firmwareImage, namespace, virtControllerName), handler.NewNodePlacementHandler(client, service.NewOneShotMigrationService(client, "nodeplacement-update-")), } - if featuregates.Default().Enabled(featuregates.HotplugMemoryWithLiveMigration) { + isMemoryHotplug := featuregates.Default().Enabled(featuregates.HotplugMemoryWithLiveMigration) + isCPUHotplug := featuregates.Default().Enabled(featuregates.HotplugCPUWithLiveMigration) + if isMemoryHotplug || isCPUHotplug { hotplugHandler := handler.NewHotplugHandler(client, service.NewOneShotMigrationService(client, "hotplug-resources-")) handlers = append(handlers, hotplugHandler) } diff --git a/images/virtualization-artifact/pkg/featuregates/featuregate.go b/images/virtualization-artifact/pkg/featuregates/featuregate.go index 8d567ae151..3358b0a85a 100644 --- a/images/virtualization-artifact/pkg/featuregates/featuregate.go +++ b/images/virtualization-artifact/pkg/featuregates/featuregate.go @@ -30,6 +30,7 @@ const ( VolumeMigration featuregate.Feature = "VolumeMigration" TargetMigration featuregate.Feature = "TargetMigration" USB featuregate.Feature = "USB" + HotplugCPUWithLiveMigration featuregate.Feature = "HotplugCPUWithLiveMigration" HotplugMemoryWithLiveMigration featuregate.Feature = "HotplugMemoryWithLiveMigration" ) @@ -58,6 +59,11 @@ var featureSpecs = map[featuregate.Feature]featuregate.FeatureSpec{ LockToDefault: true, PreRelease: featuregate.Alpha, }, + HotplugCPUWithLiveMigration: { + Default: false, + LockToDefault: version.GetEdition() == version.EditionCE, + PreRelease: featuregate.Alpha, + }, HotplugMemoryWithLiveMigration: { Default: false, LockToDefault: version.GetEdition() == version.EditionCE, diff --git a/tools/kubeconform/fixtures/module-values.yaml b/tools/kubeconform/fixtures/module-values.yaml index 3683e0bfae..a88e1a8a56 100644 --- a/tools/kubeconform/fixtures/module-values.yaml +++ b/tools/kubeconform/fixtures/module-values.yaml @@ -397,6 +397,7 @@ virtualization: - 10.0.10.0/24 - 10.0.20.0/24 - 10.0.30.0/24 + featureGates: [] moduleState: {} virtConfig: phase: Deployed