From c8be60f08b62daf8ba63f7d2e764c6dbafad9c97 Mon Sep 17 00:00:00 2001 From: Ivan Mikheykin Date: Mon, 25 May 2026 19:02:42 +0300 Subject: [PATCH 1/4] chore(vm): more messages about cpu and memory hotplug fails - Do not start live migration to apply cpu or memory changes with hotplug if VM is NonMigratable. - Fix sync_kvvm_test: fill PrintableStatus for kvvm as it is required to detect if disruptive changes may be applied. - Add test for Upgrade*ChangesToRestart mutators. Signed-off-by: Ivan Mikheykin --- .../pkg/controller/vm/internal/sync_kvvm.go | 11 + .../controller/vm/internal/sync_kvvm_test.go | 213 +++++++++++------- .../pkg/controller/vmchange/comparator_cpu.go | 2 + .../controller/vmchange/comparator_memory.go | 2 + .../pkg/controller/vmchange/compare_test.go | 20 ++ .../pkg/controller/vmchange/spec_changes.go | 10 + 6 files changed, 181 insertions(+), 77 deletions(-) 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 688b0f556d..f2a22041a2 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -155,6 +155,10 @@ func (h *SyncKvvmHandler) Handle(ctx context.Context, s state.VirtualMachineStat if kvvmiErr == nil && hasNonHotpluggableVolumes(kvvmi) { changes.UpgradeBlockDeviceChangesToRestart() } + // Require restart for CPU and memory changes if VM is non migratable. + if h.isVMNonMigratable(current) { + changes.UpgradeHotplugComputeChangesToRestart() + } allChanges.Add(changes.GetAll()...) } if class != nil { @@ -771,6 +775,13 @@ func (h *SyncKvvmHandler) isVMUnschedulable( return false } +func (h *SyncKvvmHandler) isVMNonMigratable( + vm *v1alpha2.VirtualMachine, +) bool { + vmMigratable, has := conditions.GetCondition(vmcondition.TypeMigratable, vm.Status.Conditions) + return has && vmMigratable.Status == metav1.ConditionFalse +} + // isPlacementPolicyChanged returns true if any of the Affinity, NodePlacement, or Toleration rules have changed. func hasNonHotpluggableVolumes(kvvmi *virtv1.VirtualMachineInstance) bool { if kvvmi == nil { 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 1e76e42ee3..0aaa490610 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 @@ -22,6 +22,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" virtv1 "kubevirt.io/api/core/v1" @@ -41,24 +42,24 @@ import ( "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" ) -var _ = Describe("SyncKvvmHandler", func() { +var _ = FDescribe("SyncKvvmHandler", func() { const ( name = "vm-sync" namespace = "default" ) var ( - ctx context.Context - fakeClient client.WithWatch - resource *reconciler.Resource[*v1alpha2.VirtualMachine, v1alpha2.VirtualMachineStatus] - vmState state.VirtualMachineState - recorder *eventrecord.EventRecorderLoggerMock + ctx context.Context + fakeClient client.WithWatch + reconcileObj *reconciler.Resource[*v1alpha2.VirtualMachine, v1alpha2.VirtualMachineStatus] + vmState state.VirtualMachineState + recorder *eventrecord.EventRecorderLoggerMock ) BeforeEach(func() { ctx = testutil.ContextBackgroundWithNoOpLogger() fakeClient = nil - resource = nil + reconcileObj = nil vmState = nil recorder = &eventrecord.EventRecorderLoggerMock{ EventFunc: func(_ client.Object, _, _, _ string) {}, @@ -69,16 +70,17 @@ var _ = Describe("SyncKvvmHandler", func() { AfterEach(func() { fakeClient = nil - resource = nil + reconcileObj = nil vmState = nil recorder = nil }) - newVM := func(phase v1alpha2.MachinePhase) *v1alpha2.VirtualMachine { + makeVM := func(phase v1alpha2.MachinePhase) *v1alpha2.VirtualMachine { vm := vmbuilder.NewEmpty(name, namespace) - vm.Status.Phase = phase + vm.Spec.VirtualMachineClassName = "vmclass" vm.Spec.CPU.Cores = 2 + vm.Spec.Memory.Size = resource.MustParse("2Gi") vm.Spec.RunPolicy = v1alpha2.ManualPolicy vm.Spec.VirtualMachineIPAddress = "test-ip" vm.Spec.OsType = v1alpha2.GenericOs @@ -86,10 +88,31 @@ var _ = Describe("SyncKvvmHandler", func() { RestartApprovalMode: v1alpha2.Manual, } + vm.Status.Phase = phase + return vm } - newKVVM := func(vm *v1alpha2.VirtualMachine) *virtv1.VirtualMachine { + // It is like mapPhases in vm/internal/util.go but reversed. + mapVMPhaseToKVVMPrintableStatus := func(phase v1alpha2.MachinePhase) virtv1.VirtualMachinePrintableStatus { + switch phase { + case v1alpha2.MachineRunning: + return virtv1.VirtualMachineStatusRunning + case v1alpha2.MachineMigrating: + return virtv1.VirtualMachineStatusMigrating + case v1alpha2.MachineStopping: + return virtv1.VirtualMachineStatusStopping + case v1alpha2.MachineStopped: + return virtv1.VirtualMachineStatusStopped + case v1alpha2.MachineStarting: + return virtv1.VirtualMachineStatusProvisioning + case v1alpha2.MachinePending: + return virtv1.VirtualMachineStatusUnknown + } + return "" + } + + makeKVVM := func(vm *v1alpha2.VirtualMachine) *virtv1.VirtualMachine { kvvm := &virtv1.VirtualMachine{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -101,11 +124,17 @@ var _ = Describe("SyncKvvmHandler", func() { } kvvm.Spec.RunStrategy = ptr.To(virtv1.RunStrategyAlways) + // Printable status is required for proper detection if changes are disruptive. + kvvm.Status.PrintableStatus = mapVMPhaseToKVVMPrintableStatus(vm.Status.Phase) + Expect(kvbuilder.SetLastAppliedSpec(kvvm, &v1alpha2.VirtualMachine{ Spec: v1alpha2.VirtualMachineSpec{ CPU: v1alpha2.CPUSpec{ Cores: vm.Spec.CPU.Cores, }, + Memory: v1alpha2.MemorySpec{ + Size: vm.Spec.Memory.Size, + }, VirtualMachineIPAddress: vm.Spec.VirtualMachineIPAddress, RunPolicy: vm.Spec.RunPolicy, OsType: vm.Spec.OsType, @@ -132,16 +161,50 @@ var _ = Describe("SyncKvvmHandler", func() { return kvvm } - newKVVMI := func() *virtv1.VirtualMachineInstance { + makeKVVMI := func() *virtv1.VirtualMachineInstance { kvvmi := newEmptyKVVMI(name, namespace) return kvvmi } + makeVMIP := func() *v1alpha2.VirtualMachineIPAddress { + return &v1alpha2.VirtualMachineIPAddress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ip", + Namespace: namespace, + }, + Spec: v1alpha2.VirtualMachineIPAddressSpec{ + Type: v1alpha2.VirtualMachineIPAddressTypeStatic, + StaticIP: "192.168.1.10", + }, + Status: v1alpha2.VirtualMachineIPAddressStatus{ + Address: "192.168.1.10", + Phase: v1alpha2.VirtualMachineIPAddressPhaseAttached, + }, + } + } + + makeVMClass := func() *v1alpha2.VirtualMachineClass { + return &v1alpha2.VirtualMachineClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmclass", + }, Spec: v1alpha2.VirtualMachineClassSpec{ + CPU: v1alpha2.CPU{ + Type: v1alpha2.CPUTypeHost, + }, + NodeSelector: v1alpha2.NodeSelector{ + MatchLabels: map[string]string{ + "node1": "node1", + }, + }, + }, + } + } + reconcile := func() { 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()) + err = reconcileObj.Update(context.Background()) Expect(err).NotTo(HaveOccurred()) } @@ -175,47 +238,33 @@ var _ = Describe("SyncKvvmHandler", func() { })).To(Succeed()) } + mutateCPUCores := func(cores int) func(fakeClient client.WithWatch, vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) { + return func(fakeClient client.WithWatch, vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) { + vm.Spec.CPU.Cores = cores + } + } + + mutateMemorySize := func(size string) func(fakeClient client.WithWatch, vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) { + memSize := resource.MustParse(size) + return func(fakeClient client.WithWatch, vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) { + vm.Spec.Memory.Size = memSize + } + } + DescribeTable("AwaitingRestart Condition Tests", func(phase v1alpha2.MachinePhase, needChange bool, expectedStatus metav1.ConditionStatus, expectedExistence bool) { - ip := &v1alpha2.VirtualMachineIPAddress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ip", - Namespace: namespace, - }, - Spec: v1alpha2.VirtualMachineIPAddressSpec{ - Type: v1alpha2.VirtualMachineIPAddressTypeStatic, - StaticIP: "192.168.1.10", - }, - Status: v1alpha2.VirtualMachineIPAddressStatus{ - Address: "192.168.1.10", - Phase: v1alpha2.VirtualMachineIPAddressPhaseAttached, - }, - } + ip := makeVMIP() + vmClass := makeVMClass() - vmClass := &v1alpha2.VirtualMachineClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: "vmclass", - }, Spec: v1alpha2.VirtualMachineClassSpec{ - CPU: v1alpha2.CPU{ - Type: v1alpha2.CPUTypeHost, - }, - NodeSelector: v1alpha2.NodeSelector{ - MatchLabels: map[string]string{ - "node1": "node1", - }, - }, - }, - } - - vm := newVM(phase) - kvvm := newKVVM(vm) - kvvmi := newKVVMI() + vm := makeVM(phase) + kvvm := makeKVVM(vm) + kvvmi := makeKVVMI() if needChange { mutateKVVM(vm, kvvm) } - fakeClient, resource, vmState = setupEnvironment(vm, kvvm, kvvmi, ip, vmClass) + fakeClient, reconcileObj, vmState = setupEnvironment(vm, kvvm, kvvmi, ip, vmClass) reconcile() @@ -248,39 +297,48 @@ var _ = Describe("SyncKvvmHandler", func() { Entry("Pending phase without changes, shouldn't have condition", v1alpha2.MachinePending, false, metav1.ConditionUnknown, false), ) - DescribeTable("ConfigurationApplied Condition Tests", - func(phase v1alpha2.MachinePhase, notReady bool, expectedStatus metav1.ConditionStatus, expectedExistence bool) { - ip := &v1alpha2.VirtualMachineIPAddress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ip", - Namespace: namespace, - }, - Spec: v1alpha2.VirtualMachineIPAddressSpec{ - Type: v1alpha2.VirtualMachineIPAddressTypeStatic, - StaticIP: "192.168.1.10", - }, - Status: v1alpha2.VirtualMachineIPAddressStatus{ - Address: "192.168.1.10", - Phase: v1alpha2.VirtualMachineIPAddressPhaseAttached, - }, + DescribeTable("AwaitingRestart Condition for NonMigratable VM", + func(phase v1alpha2.MachinePhase, mutateFn func(fakeClient client.WithWatch, vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine), expectedStatus metav1.ConditionStatus, expectedExistence bool) { + ip := makeVMIP() + vmClass := makeVMClass() + + vm := makeVM(phase) + vm.Status.Conditions = append(vm.Status.Conditions, metav1.Condition{ + Type: vmcondition.TypeMigratable.String(), + Status: metav1.ConditionFalse, + Reason: string(vmcondition.ReasonHostDevicesNotMigratable), + }) + kvvm := makeKVVM(vm) + kvvmi := makeKVVMI() + + if mutateFn != nil { + mutateFn(fakeClient, vm, kvvm) } - vmClass := &v1alpha2.VirtualMachineClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: "vmclass", - }, Spec: v1alpha2.VirtualMachineClassSpec{ - CPU: v1alpha2.CPU{ - Type: v1alpha2.CPUTypeHost, - }, - NodeSelector: v1alpha2.NodeSelector{ - MatchLabels: map[string]string{ - "node1": "node1", - }, - }, - }, + fakeClient, reconcileObj, vmState = setupEnvironment(vm, kvvm, kvvmi, ip, vmClass) + + reconcile() + + newVM := &v1alpha2.VirtualMachine{} + err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) + Expect(err).NotTo(HaveOccurred()) + + awaitCond, awaitExists := conditions.GetCondition(vmcondition.TypeAwaitingRestartToApplyConfiguration, newVM.Status.Conditions) + Expect(awaitExists).To(Equal(expectedExistence)) + if awaitExists { + Expect(awaitCond.Status).To(Equal(expectedStatus)) } + }, + Entry("should present on hotplugging cpu.cores", v1alpha2.MachineRunning, mutateCPUCores(3), metav1.ConditionTrue, true), + Entry("should present on hotplugging memory.size", v1alpha2.MachineRunning, mutateMemorySize("4Gi"), metav1.ConditionTrue, true), + ) + + DescribeTable("ConfigurationApplied Condition Tests", + func(phase v1alpha2.MachinePhase, notReady bool, expectedStatus metav1.ConditionStatus, expectedExistence bool) { + ip := makeVMIP() + vmClass := makeVMClass() - vm := newVM(phase) + vm := makeVM(phase) if notReady { vm.Status.Conditions = append(vm.Status.Conditions, metav1.Condition{ Type: vmcondition.TypeBlockDevicesReady.String(), @@ -288,9 +346,10 @@ var _ = Describe("SyncKvvmHandler", func() { Reason: "BlockDevicesNotReady", }) } - kvvm := newKVVM(vm) + kvvm := makeKVVM(vm) + kvvmi := makeKVVMI() - fakeClient, resource, vmState = setupEnvironment(vm, kvvm, ip, vmClass) + fakeClient, reconcileObj, vmState = setupEnvironment(vm, kvvm, kvvmi, ip, vmClass) reconcile() newVM := &v1alpha2.VirtualMachine{} diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparator_cpu.go b/images/virtualization-artifact/pkg/controller/vmchange/comparator_cpu.go index 71d2f919aa..4b4388d7ef 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/comparator_cpu.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparator_cpu.go @@ -28,6 +28,8 @@ type comparatorCPU struct { featureGate featuregate.FeatureGate } +const CPUPath = "cpu" + func NewComparatorCPU(featureGate featuregate.FeatureGate) VMSpecFieldComparator { return &comparatorCPU{ featureGate: featureGate, diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go b/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go index c00d33aa5c..90cda1e1b8 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go @@ -25,6 +25,8 @@ import ( "github.com/deckhouse/virtualization/api/core/v1alpha2" ) +const MemoryPath = "memory" + type comparatorMemory struct { featureGate featuregate.FeatureGate } diff --git a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go index 462f753abe..31314c99bc 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go @@ -561,6 +561,26 @@ networks: } } +// Test ApplyImmediate to Restart upgraders. +func TestUpgradeHotplugComputeChangesToRestart(t *testing.T) { + const cpuReason = "cpu restart reason" + + changes := SpecChanges{} + changes.Add( + FieldChange{Path: "cpu.cores", ActionRequired: ActionApplyImmediate, RestartMessage: cpuReason}, + FieldChange{Path: "memory.size", ActionRequired: ActionApplyImmediate}, + FieldChange{Path: "blockDeviceRefs.0", ActionRequired: ActionApplyImmediate}, + ) + + changes.UpgradeHotplugComputeChangesToRestart() + changes.UpgradeBlockDeviceChangesToRestart() + + require.Equal(t, ActionRestart, changes.GetAll()[0].ActionRequired) + require.Equal(t, cpuReason, changes.GetAll()[0].RestartMessage) + require.Equal(t, ActionRestart, changes.GetAll()[1].ActionRequired) + require.Equal(t, ActionRestart, changes.GetAll()[2].ActionRequired) +} + func loadVMSpec(t *testing.T, inYAML string) *v1alpha2.VirtualMachineSpec { t.Helper() var spec v1alpha2.VirtualMachineSpec diff --git a/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go b/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go index 9c486c7f77..a4ac5840e3 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go @@ -250,6 +250,16 @@ func (s *SpecChanges) UpgradeBlockDeviceChangesToRestart() { } } +func (s *SpecChanges) UpgradeHotplugComputeChangesToRestart() { + for i := range s.changes { + isCPUChange := strings.HasPrefix(s.changes[i].Path, CPUPath) + isMemoryChange := strings.HasPrefix(s.changes[i].Path, MemoryPath) + if (isCPUChange || isMemoryChange) && s.changes[i].ActionRequired == ActionApplyImmediate { + s.changes[i].ActionRequired = ActionRestart + } + } +} + func (s *SpecChanges) Add(changes ...FieldChange) { if s.changes == nil { s.changes = make([]FieldChange, 0) From 3b2f5a3911b7fe4924c60f0e6d2690c69bb4e2c8 Mon Sep 17 00:00:00 2001 From: Ivan Mikheykin Date: Tue, 26 May 2026 20:10:23 +0300 Subject: [PATCH 2/4] ++ remove focus for describe Signed-off-by: Ivan Mikheykin --- .../pkg/controller/vm/internal/sync_kvvm_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 0aaa490610..f9caac95ac 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 @@ -42,7 +42,7 @@ import ( "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" ) -var _ = FDescribe("SyncKvvmHandler", func() { +var _ = Describe("SyncKvvmHandler", func() { const ( name = "vm-sync" namespace = "default" From 7c6e399314ab2fafb5ccabc479dd119c58563099 Mon Sep 17 00:00:00 2001 From: Ivan Mikheykin Date: Wed, 27 May 2026 21:21:11 +0300 Subject: [PATCH 3/4] ++ more strict comparison for paths, rename public consts to private. Signed-off-by: Ivan Mikheykin --- .../pkg/controller/vmchange/comparator_block_devices.go | 6 +++--- .../pkg/controller/vmchange/comparator_cpu.go | 2 +- .../pkg/controller/vmchange/comparator_memory.go | 2 +- .../pkg/controller/vmchange/spec_changes.go | 7 ++++--- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparator_block_devices.go b/images/virtualization-artifact/pkg/controller/vmchange/comparator_block_devices.go index 1d7f792d30..4405077068 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/comparator_block_devices.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparator_block_devices.go @@ -23,7 +23,7 @@ import ( "github.com/deckhouse/virtualization/api/core/v1alpha2" ) -const BlockDevicesPath = "blockDeviceRefs" +const blockDevicesPath = "blockDeviceRefs" // compareBlockDevices returns changes between current and desired blockDevices lists. func compareBlockDevices(current, desired *v1alpha2.VirtualMachineSpec) []FieldChange { @@ -32,7 +32,7 @@ func compareBlockDevices(current, desired *v1alpha2.VirtualMachineSpec) []FieldC } fullChanges := compareEmpty( - BlockDevicesPath, + blockDevicesPath, NewValue(current.BlockDeviceRefs, len(current.BlockDeviceRefs) == 0, false), NewValue(desired.BlockDeviceRefs, len(desired.BlockDeviceRefs) == 0, false), ActionApplyImmediate, @@ -152,7 +152,7 @@ func vmdIndexedNames(vm *v1alpha2.VirtualMachineSpec) map[string]int { } func blockDevicesItemPath(idx int) string { - return fmt.Sprintf("%s.%d", BlockDevicesPath, idx) + return fmt.Sprintf("%s.%d", blockDevicesPath, idx) } func updateIndexesForAddedDevices(added map[int]struct{}, currentDevices, desiredDevices map[string]int) { diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparator_cpu.go b/images/virtualization-artifact/pkg/controller/vmchange/comparator_cpu.go index 4b4388d7ef..89d81be3a0 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/comparator_cpu.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparator_cpu.go @@ -28,7 +28,7 @@ type comparatorCPU struct { featureGate featuregate.FeatureGate } -const CPUPath = "cpu" +const cpuPath = "cpu" func NewComparatorCPU(featureGate featuregate.FeatureGate) VMSpecFieldComparator { return &comparatorCPU{ diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go b/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go index 90cda1e1b8..c442f57f3c 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparator_memory.go @@ -25,7 +25,7 @@ import ( "github.com/deckhouse/virtualization/api/core/v1alpha2" ) -const MemoryPath = "memory" +const memoryPath = "memory" type comparatorMemory struct { featureGate featuregate.FeatureGate diff --git a/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go b/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go index a4ac5840e3..9a90b50fe3 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go @@ -244,7 +244,8 @@ func (s *SpecChanges) ConvertPendingChanges() ([]apiextensionsv1.JSON, error) { func (s *SpecChanges) UpgradeBlockDeviceChangesToRestart() { for i := range s.changes { - if strings.HasPrefix(s.changes[i].Path, BlockDevicesPath) && s.changes[i].ActionRequired == ActionApplyImmediate { + isBlockDeviceChange := s.changes[i].Path == blockDevicesPath || strings.HasPrefix(s.changes[i].Path, blockDevicesPath+".") + if isBlockDeviceChange && s.changes[i].ActionRequired == ActionApplyImmediate { s.changes[i].ActionRequired = ActionRestart } } @@ -252,8 +253,8 @@ func (s *SpecChanges) UpgradeBlockDeviceChangesToRestart() { func (s *SpecChanges) UpgradeHotplugComputeChangesToRestart() { for i := range s.changes { - isCPUChange := strings.HasPrefix(s.changes[i].Path, CPUPath) - isMemoryChange := strings.HasPrefix(s.changes[i].Path, MemoryPath) + isCPUChange := s.changes[i].Path == cpuPath || strings.HasPrefix(s.changes[i].Path, cpuPath+".") + isMemoryChange := s.changes[i].Path == memoryPath || strings.HasPrefix(s.changes[i].Path, memoryPath+".") if (isCPUChange || isMemoryChange) && s.changes[i].ActionRequired == ActionApplyImmediate { s.changes[i].ActionRequired = ActionRestart } From 58b1aeda5532f9457fbc26d209de81bca3fe6f2c Mon Sep 17 00:00:00 2001 From: Ivan Mikheykin Date: Wed, 27 May 2026 21:58:27 +0300 Subject: [PATCH 4/4] ++ add feature gate with hotplug fatures to "AwaitingRestart Condition for NonMigratable VM" test Signed-off-by: Ivan Mikheykin --- .../controller/vm/internal/sync_kvvm_test.go | 41 +++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) 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 f9caac95ac..427d601606 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 @@ -24,6 +24,7 @@ import ( . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/component-base/featuregate" "k8s.io/utils/ptr" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -54,6 +55,7 @@ var _ = Describe("SyncKvvmHandler", func() { reconcileObj *reconciler.Resource[*v1alpha2.VirtualMachine, v1alpha2.VirtualMachineStatus] vmState state.VirtualMachineState recorder *eventrecord.EventRecorderLoggerMock + featureGates featuregate.FeatureGate ) BeforeEach(func() { @@ -61,6 +63,7 @@ var _ = Describe("SyncKvvmHandler", func() { fakeClient = nil reconcileObj = nil vmState = nil + featureGates = nil recorder = &eventrecord.EventRecorderLoggerMock{ EventFunc: func(_ client.Object, _, _, _ string) {}, EventfFunc: func(_ client.Object, _, _, _ string, _ ...interface{}) {}, @@ -201,7 +204,10 @@ var _ = Describe("SyncKvvmHandler", func() { } reconcile := func() { - h := NewSyncKvvmHandler(nil, fakeClient, recorder, featuregates.Default(), vmservice.NewMigrationVolumesService(fakeClient, MakeKVVMFromVMSpec, 10*time.Second)) + if featureGates == nil { + featureGates = featuregates.Default() + } + h := NewSyncKvvmHandler(nil, fakeClient, recorder, featureGates, vmservice.NewMigrationVolumesService(fakeClient, MakeKVVMFromVMSpec, 10*time.Second)) _, err := h.Handle(ctx, vmState) Expect(err).NotTo(HaveOccurred()) err = reconcileObj.Update(context.Background()) @@ -298,7 +304,7 @@ var _ = Describe("SyncKvvmHandler", func() { ) DescribeTable("AwaitingRestart Condition for NonMigratable VM", - func(phase v1alpha2.MachinePhase, mutateFn func(fakeClient client.WithWatch, vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine), expectedStatus metav1.ConditionStatus, expectedExistence bool) { + func(phase v1alpha2.MachinePhase, featureGate featuregate.FeatureGate, mutateFn func(fakeClient client.WithWatch, vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine), expectedStatus metav1.ConditionStatus, expectedExistence bool) { ip := makeVMIP() vmClass := makeVMClass() @@ -317,6 +323,8 @@ var _ = Describe("SyncKvvmHandler", func() { fakeClient, reconcileObj, vmState = setupEnvironment(vm, kvvm, kvvmi, ip, vmClass) + featureGates = featureGate + reconcile() newVM := &v1alpha2.VirtualMachine{} @@ -329,8 +337,10 @@ var _ = Describe("SyncKvvmHandler", func() { Expect(awaitCond.Status).To(Equal(expectedStatus)) } }, - Entry("should present on hotplugging cpu.cores", v1alpha2.MachineRunning, mutateCPUCores(3), metav1.ConditionTrue, true), - Entry("should present on hotplugging memory.size", v1alpha2.MachineRunning, mutateMemorySize("4Gi"), metav1.ConditionTrue, true), + Entry("should present on cpu.cores change", v1alpha2.MachineRunning, nil, mutateCPUCores(3), metav1.ConditionTrue, true), + Entry("should present on cpu.cores change when hotplug enabled", v1alpha2.MachineRunning, newFeatureGateEnableCPUHotplug(), mutateCPUCores(3), metav1.ConditionTrue, true), + Entry("should present on memory.size change", v1alpha2.MachineRunning, nil, mutateMemorySize("4Gi"), metav1.ConditionTrue, true), + Entry("should present on memory.size change when hotplug enabled", v1alpha2.MachineRunning, newFeatureGateEnableMemoryHotplug(), mutateMemorySize("4Gi"), metav1.ConditionTrue, true), ) DescribeTable("ConfigurationApplied Condition Tests", @@ -396,3 +406,26 @@ var _ = Describe("SyncKvvmHandler", func() { Entry("cpu change is not a placement policy", "cpu.cores", false), ) }) + +func newFeatureGate(enabled ...featuregate.Feature) featuregate.FeatureGate { + GinkgoHelper() + + gate, setFromMap, err := featuregates.NewUnlocked() + Expect(err).NotTo(HaveOccurred()) + featureMap := map[string]bool{} + for _, feature := range enabled { + featureMap[string(feature)] = true + } + err = setFromMap(featureMap) + Expect(err).NotTo(HaveOccurred()) + + return gate +} + +func newFeatureGateEnableCPUHotplug() featuregate.FeatureGate { + return newFeatureGate(featuregates.HotplugCPUWithLiveMigration) +} + +func newFeatureGateEnableMemoryHotplug() featuregate.FeatureGate { + return newFeatureGate(featuregates.HotplugMemoryWithLiveMigration) +}