From c64681dac205c9770ebc818510663dc59099b934 Mon Sep 17 00:00:00 2001 From: Nikita Korolev <141920865+universal-itengineer@users.noreply.github.com> Date: Wed, 7 May 2025 17:34:01 +0300 Subject: [PATCH] Revert "fix(vm): add removing VM's conditions by design (#931)" This reverts commit de83ce6f0d5fa79c165fdb0a7ed2d7eaffcc05ec. Signed-off-by: Nikita Korolev --- api/core/v1alpha2/vmcondition/condition.go | 7 +- .../pkg/controller/vm/internal/agent.go | 32 +-- .../pkg/controller/vm/internal/agent_test.go | 170 ------------ .../pkg/controller/vm/internal/firmware.go | 28 +- .../controller/vm/internal/firmware_test.go | 57 +--- .../pkg/controller/vm/internal/lifecycle.go | 188 ++++++++++++- .../pkg/controller/vm/internal/migrating.go | 206 -------------- .../controller/vm/internal/migrating_test.go | 216 --------------- .../pkg/controller/vm/internal/size_policy.go | 13 +- .../vm/internal/size_policy_test.go | 130 --------- .../controller/vm/internal/snapshotting.go | 18 +- .../vm/internal/snapshotting_test.go | 145 ---------- .../pkg/controller/vm/internal/sync_kvvm.go | 51 ++-- .../controller/vm/internal/sync_kvvm_test.go | 257 ------------------ .../vm/internal/sync_power_state.go | 9 +- .../pkg/controller/vm/internal/util.go | 36 +-- .../pkg/controller/vm/vm_controller.go | 1 - 17 files changed, 267 insertions(+), 1297 deletions(-) delete mode 100644 images/virtualization-artifact/pkg/controller/vm/internal/agent_test.go delete mode 100644 images/virtualization-artifact/pkg/controller/vm/internal/migrating.go delete mode 100644 images/virtualization-artifact/pkg/controller/vm/internal/migrating_test.go delete mode 100644 images/virtualization-artifact/pkg/controller/vm/internal/size_policy_test.go delete mode 100644 images/virtualization-artifact/pkg/controller/vm/internal/snapshotting_test.go delete mode 100644 images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm_test.go diff --git a/api/core/v1alpha2/vmcondition/condition.go b/api/core/v1alpha2/vmcondition/condition.go index 6b99583c87..3fa5f134ed 100644 --- a/api/core/v1alpha2/vmcondition/condition.go +++ b/api/core/v1alpha2/vmcondition/condition.go @@ -29,6 +29,7 @@ const ( TypeRunning Type = "Running" TypeMigrating Type = "Migrating" TypeMigratable Type = "Migratable" + TypePodStarted Type = "PodStarted" TypeProvisioningReady Type = "ProvisioningReady" TypeAgentReady Type = "AgentReady" TypeAgentVersionNotSupported Type = "AgentVersionNotSupported" @@ -80,6 +81,10 @@ const ( ReasonRestartAwaitingVMClassChangesExist Reason = "RestartAwaitingVMClassChangesExist" ReasonRestartNoNeed Reason = "NoNeedRestart" + ReasonPodStarted Reason = "PodStarted" + ReasonPodNotFound Reason = "PodNotFound" + ReasonPodNotStarted Reason = "PodNotStarted" + ReasonMigratable Reason = "VirtualMachineMigratable" ReasonNotMigratable Reason = "VirtualMachineNotMigratable" @@ -89,7 +94,6 @@ const ( ReasonVmIsNotRunning Reason = "VirtualMachineNotRunning" ReasonVmIsRunning Reason = "VirtualMachineRunning" ReasonInternalVirtualMachineError Reason = "InternalVirtualMachineError" - ReasonPodNotStarted Reason = "PodNotStarted" // ReasonFilesystemFrozen indicates that virtual machine's filesystem has been successfully frozen. ReasonFilesystemFrozen Reason = "Frozen" @@ -97,6 +101,7 @@ const ( WaitingForTheSnapshotToStart Reason = "WaitingForTheSnapshotToStart" ReasonSnapshottingInProgress Reason = "SnapshottingInProgress" + ReasonSizingPolicyMatched Reason = "SizingPolicyMatched" ReasonSizingPolicyNotMatched Reason = "SizingPolicyNotMatched" ReasonVirtualMachineClassTerminating Reason = "VirtualMachineClassTerminating" ReasonVirtualMachineClassNotExists Reason = "VirtalMachineClassNotExists" diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/agent.go b/images/virtualization-artifact/pkg/controller/vm/internal/agent.go index a45e0cbdb4..1a842027cc 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/agent.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/agent.go @@ -31,6 +31,11 @@ import ( const nameAgentHandler = "AgentHandler" +var agentConditions = []vmcondition.Type{ + vmcondition.TypeAgentReady, + vmcondition.TypeAgentVersionNotSupported, +} + func NewAgentHandler() *AgentHandler { return &AgentHandler{} } @@ -44,6 +49,10 @@ func (h *AgentHandler) Handle(ctx context.Context, s state.VirtualMachineState) current := s.VirtualMachine().Current() changed := s.VirtualMachine().Changed() + if update := addAllUnknown(changed, agentConditions...); update { + return reconcile.Result{Requeue: true}, nil + } + if isDeletion(current) { return reconcile.Result{}, nil } @@ -71,14 +80,7 @@ func (h *AgentHandler) syncAgentReady(vm *virtv2.VirtualMachine, kvvmi *virtv1.V cb := conditions.NewConditionBuilder(vmcondition.TypeAgentReady).Generation(vm.GetGeneration()) - defer func() { - phase := vm.Status.Phase - if phase == virtv2.MachinePending || phase == virtv2.MachineStarting || phase == virtv2.MachineStopped { - conditions.RemoveCondition(vmcondition.TypeAgentReady, &vm.Status.Conditions) - } else { - conditions.SetCondition(cb, &vm.Status.Conditions) - } - }() + defer func() { conditions.SetCondition(cb, &vm.Status.Conditions) }() if kvvmi == nil { cb.Status(metav1.ConditionFalse). @@ -114,19 +116,7 @@ func (h *AgentHandler) syncAgentVersionNotSupport(vm *virtv2.VirtualMachine, kvv cb := conditions.NewConditionBuilder(vmcondition.TypeAgentVersionNotSupported).Generation(vm.GetGeneration()) - defer func() { - switch vm.Status.Phase { - case virtv2.MachinePending, virtv2.MachineStarting, virtv2.MachineStopped: - conditions.RemoveCondition(vmcondition.TypeAgentVersionNotSupported, &vm.Status.Conditions) - - default: - if cb.Condition().Status == metav1.ConditionTrue { - conditions.SetCondition(cb, &vm.Status.Conditions) - } else { - conditions.RemoveCondition(vmcondition.TypeAgentVersionNotSupported, &vm.Status.Conditions) - } - } - }() + defer func() { conditions.SetCondition(cb, &vm.Status.Conditions) }() if kvvmi == nil { cb.Status(metav1.ConditionFalse). diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/agent_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/agent_test.go deleted file mode 100644 index 9f3eb9f543..0000000000 --- a/images/virtualization-artifact/pkg/controller/vm/internal/agent_test.go +++ /dev/null @@ -1,170 +0,0 @@ -/* -Copyright 2025 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 internal - -import ( - "context" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/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-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/reconciler" - "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" - virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" -) - -var _ = Describe("AgentHandler Tests", func() { - const ( - name = "vm-agent" - namespace = "default" - ) - - var ( - ctx = testutil.ContextBackgroundWithNoOpLogger() - fakeClient client.WithWatch - resource *reconciler.Resource[*virtv2.VirtualMachine, virtv2.VirtualMachineStatus] - vmState state.VirtualMachineState - ) - - AfterEach(func() { - fakeClient = nil - resource = nil - vmState = nil - }) - - newVM := func(phase virtv2.MachinePhase) *virtv2.VirtualMachine { - vm := vmbuilder.NewEmpty(name, namespace) - vm.Status.Phase = phase - return vm - } - - newKVVMI := func(agentConnected bool, agentUnsupported bool) *virtv1.VirtualMachineInstance { - kvvmi := newEmptyKVVMI(name, namespace) - conditions := make([]virtv1.VirtualMachineInstanceCondition, 0) - if agentConnected { - conditions = append(conditions, virtv1.VirtualMachineInstanceCondition{ - Type: virtv1.VirtualMachineInstanceAgentConnected, - Status: corev1.ConditionTrue, - }) - } - if agentUnsupported { - conditions = append(conditions, virtv1.VirtualMachineInstanceCondition{ - Type: virtv1.VirtualMachineInstanceUnsupportedAgent, - Status: corev1.ConditionTrue, - }) - } else { - conditions = append(conditions, virtv1.VirtualMachineInstanceCondition{ - Type: virtv1.VirtualMachineInstanceUnsupportedAgent, - Status: corev1.ConditionFalse, - }) - } - - kvvmi.Status.Conditions = conditions - return kvvmi - } - - reconcile := func() { - h := NewAgentHandler() - _, err := h.Handle(ctx, vmState) - Expect(err).NotTo(HaveOccurred()) - err = resource.Update(context.Background()) - Expect(err).NotTo(HaveOccurred()) - } - - DescribeTable("AgentReady Condition Tests", - func(phase virtv2.MachinePhase, agentConnected bool, expectedStatus metav1.ConditionStatus, expectedExistence bool) { - vm := newVM(phase) - kvvmi := newKVVMI(agentConnected, false) - fakeClient, resource, vmState = setupEnvironment(vm, kvvmi) - - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - - cond, exists := conditions.GetCondition(vmcondition.TypeAgentReady, newVM.Status.Conditions) - Expect(exists).To(Equal(expectedExistence)) - if exists { - Expect(cond.Status).To(Equal(expectedStatus)) - } - }, - Entry("Should add AgentReady as True if agent is connected", virtv2.MachineRunning, true, metav1.ConditionTrue, true), - Entry("Should add AgentReady as False if agent is not connected", virtv2.MachineRunning, false, metav1.ConditionFalse, true), - - Entry("Should add AgentReady as True if agent is connected", virtv2.MachineStopping, true, metav1.ConditionTrue, true), - Entry("Should add AgentReady as False if agent is not connected", virtv2.MachineStopping, false, metav1.ConditionFalse, true), - - Entry("Should add AgentReady as True if agent is connected", virtv2.MachineMigrating, true, metav1.ConditionTrue, true), - Entry("Should add AgentReady as False if agent is not connected", virtv2.MachineMigrating, false, metav1.ConditionFalse, true), - - Entry("Should not add AgentReady if VM is in Pending phase and the agent is connected", virtv2.MachinePending, true, metav1.ConditionUnknown, false), - Entry("Should not add AgentReady if VM is in Pending phase and the agent is not connected", virtv2.MachinePending, false, metav1.ConditionUnknown, false), - - Entry("Should not add AgentReady if VM is in Starting phase and the agent is connected", virtv2.MachineStarting, true, metav1.ConditionUnknown, false), - Entry("Should not add AgentReady if VM is in Starting phase and the agent is not connected", virtv2.MachineStarting, false, metav1.ConditionUnknown, false), - - Entry("Should not add AgentReady if VM is in Stopped phase and the agent is connected", virtv2.MachineStopped, true, metav1.ConditionUnknown, false), - Entry("Should not add AgentReady if VM is in Stopped phase and the agent is not connected", virtv2.MachineStopped, false, metav1.ConditionUnknown, false), - ) - - DescribeTable("AgentVersionNotSupported Condition Tests", - func(phase virtv2.MachinePhase, agentUnsupported bool, expectedStatus metav1.ConditionStatus, expectedExistence bool) { - vm := newVM(phase) - vmi := newKVVMI(true, agentUnsupported) - fakeClient, resource, vmState = setupEnvironment(vm, vmi) - - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - - cond, exists := conditions.GetCondition(vmcondition.TypeAgentVersionNotSupported, newVM.Status.Conditions) - Expect(exists).To(Equal(expectedExistence)) - if exists { - Expect(cond.Status).To(Equal(expectedStatus)) - } - }, - Entry("Should set unsupported version condition as True in Running phase", virtv2.MachineRunning, true, metav1.ConditionTrue, true), - Entry("Should not set unsupported version condition as False in Running phase", virtv2.MachineRunning, false, metav1.ConditionUnknown, false), - - Entry("Should set unsupported version condition as True in Stopping phase", virtv2.MachineStopping, true, metav1.ConditionTrue, true), - Entry("Should set unsupported version condition as False in Stopping phase", virtv2.MachineStopping, false, metav1.ConditionUnknown, false), - - Entry("Should set unsupported version condition as True in Migrating phase", virtv2.MachineMigrating, true, metav1.ConditionTrue, true), - Entry("Should set unsupported version condition as False in Migrating phase", virtv2.MachineMigrating, false, metav1.ConditionUnknown, false), - - Entry("Should not set unsupported version condition as True in Pending phase", virtv2.MachinePending, true, metav1.ConditionUnknown, false), - Entry("Should not set unsupported version condition as False in Pending phase", virtv2.MachinePending, false, metav1.ConditionUnknown, false), - - Entry("Should not set unsupported version condition as True in Starting phase", virtv2.MachineStarting, true, metav1.ConditionUnknown, false), - Entry("Should not set unsupported version condition as False in Starting phase", virtv2.MachineStarting, false, metav1.ConditionUnknown, false), - - Entry("Should not set unsupported version condition as True in Stopped phase", virtv2.MachineStopped, true, metav1.ConditionUnknown, false), - Entry("Should not set unsupported version condition as False in Stopped phase", virtv2.MachineStopped, false, metav1.ConditionUnknown, false), - ) -}) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/firmware.go b/images/virtualization-artifact/pkg/controller/vm/internal/firmware.go index e35430533b..87356430d3 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/firmware.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/firmware.go @@ -68,24 +68,16 @@ func (h *FirmwareHandler) syncFirmwareUpToDate(vm *virtv2.VirtualMachine, kvvmi upToDate := kvvmi == nil || kvvmi.Status.LauncherContainerImageVersion == "" || kvvmi.Status.LauncherContainerImageVersion == h.image cb := conditions.NewConditionBuilder(vmcondition.TypeFirmwareUpToDate).Generation(vm.GetGeneration()) - defer func() { - switch vm.Status.Phase { - case virtv2.MachinePending, virtv2.MachineStarting, virtv2.MachineStopped: - conditions.RemoveCondition(vmcondition.TypeFirmwareUpToDate, &vm.Status.Conditions) - - default: - if cb.Condition().Status == metav1.ConditionFalse { - conditions.SetCondition(cb, &vm.Status.Conditions) - } else { - conditions.RemoveCondition(vmcondition.TypeFirmwareUpToDate, &vm.Status.Conditions) - } - } - }() - - if !upToDate { - cb.Status(metav1.ConditionFalse). - Reason(vmcondition.ReasonFirmwareOutOfDate). - Message("The VM firmware version is outdated and not recommended for use with the current version of the virtualization module, please migrate or reboot the VM to upgrade its firmware version.") + if upToDate { + cb.Status(metav1.ConditionTrue). + Reason(vmcondition.ReasonFirmwareUpToDate). + Message("") conditions.SetCondition(cb, &vm.Status.Conditions) + return } + + cb.Status(metav1.ConditionFalse). + Reason(vmcondition.ReasonFirmwareOutOfDate). + Message("The VM firmware version is outdated and not recommended for use with the current version of the virtualization module, please migrate or reboot the VM to upgrade its firmware version.") + conditions.SetCondition(cb, &vm.Status.Conditions) } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/firmware_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/firmware_test.go index b9b7af3f24..b158d53c5b 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/firmware_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/firmware_test.go @@ -73,7 +73,7 @@ var _ = Describe("TestFirmwareHandler", func() { } DescribeTable("Condition TypeFirmwareUpToDate should be in expected state", - func(vm *virtv2.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, expectedStatus metav1.ConditionStatus, expectedReason vmcondition.Reason, expectedExistence bool) { + func(vm *virtv2.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, expectedStatus metav1.ConditionStatus, expectedReason vmcondition.Reason) { fakeClient, resource, vmState = setupEnvironment(vm, kvvmi) reconcile() @@ -82,51 +82,16 @@ var _ = Describe("TestFirmwareHandler", func() { Expect(err).NotTo(HaveOccurred()) upToDate, exists := conditions.GetCondition(vmcondition.TypeFirmwareUpToDate, newVM.Status.Conditions) - Expect(exists).To(Equal(expectedExistence)) - if exists { - Expect(upToDate.Status).To(Equal(expectedStatus)) - Expect(upToDate.Reason).To(Equal(expectedReason.String())) - } + Expect(exists).To(BeTrue()) + Expect(upToDate.Status).To(Equal(expectedStatus)) + Expect(upToDate.Reason).To(Equal(expectedReason.String())) }, - Entry("Should be up to date", newVM(), newKVVMI(expectedImage), metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate, false), - Entry("Should be up to date because kvvmi is not exists", newVM(), nil, metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate, false), - Entry("Should be out of date 1", newVM(), newKVVMI("other-image-1"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), - Entry("Should be out of date 2", newVM(), newKVVMI("other-image-2"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), - Entry("Should be out of date 3", newVM(), newKVVMI("other-image-3"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), - Entry("Should be out of date 4", newVM(), newKVVMI("other-image-4"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), - Entry("Should be out of date 5", newVM(), newKVVMI("other-image-5"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true), - ) - - DescribeTable("Condition TypeFirmwareUpToDate should be in the expected state considering the VM phase", - func(vm *virtv2.VirtualMachine, phase virtv2.MachinePhase, kvvmi *virtv1.VirtualMachineInstance, expectedStatus metav1.ConditionStatus, expectedExistence bool) { - vm.Status.Phase = phase - fakeClient, resource, vmState = setupEnvironment(vm, kvvmi) - reconcile() - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - upToDate, exists := conditions.GetCondition(vmcondition.TypeFirmwareUpToDate, newVM.Status.Conditions) - Expect(exists).To(Equal(expectedExistence)) - if exists { - Expect(upToDate.Status).To(Equal(expectedStatus)) - } - }, - Entry("Running phase, condition should not be set", newVM(), virtv2.MachineRunning, newKVVMI(expectedImage), metav1.ConditionUnknown, false), - Entry("Running phase, condition should be set", newVM(), virtv2.MachineRunning, newKVVMI("other-image-1"), metav1.ConditionFalse, true), - - Entry("Migrating phase, condition should not be set", newVM(), virtv2.MachineMigrating, newKVVMI(expectedImage), metav1.ConditionUnknown, false), - Entry("Migrating phase, condition should be set", newVM(), virtv2.MachineMigrating, newKVVMI("other-image-1"), metav1.ConditionFalse, true), - - Entry("Stopping phase, condition should not be set", newVM(), virtv2.MachineStopping, newKVVMI(expectedImage), metav1.ConditionUnknown, false), - Entry("Stopping phase, condition should be set", newVM(), virtv2.MachineStopping, newKVVMI("other-image-1"), metav1.ConditionFalse, true), - - Entry("Pending phase, condition should not be set", newVM(), virtv2.MachinePending, newKVVMI(expectedImage), metav1.ConditionUnknown, false), - Entry("Pending phase, condition should not be set", newVM(), virtv2.MachinePending, newKVVMI("other-image-1"), metav1.ConditionUnknown, false), - - Entry("Starting phase, condition should not be set", newVM(), virtv2.MachineStarting, newKVVMI(expectedImage), metav1.ConditionUnknown, false), - Entry("Starting phase, condition should not be set", newVM(), virtv2.MachineStarting, newKVVMI("other-image-1"), metav1.ConditionUnknown, false), - - Entry("Stopped phase, condition should not be set", newVM(), virtv2.MachineStopped, newKVVMI(expectedImage), metav1.ConditionUnknown, false), - Entry("Stopped phase, condition should not be set", newVM(), virtv2.MachineStopped, newKVVMI("other-image-1"), metav1.ConditionUnknown, false), + Entry("Should be up to date", newVM(), newKVVMI(expectedImage), metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate), + Entry("Should be up to date because kvvmi is not exists", newVM(), nil, metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate), + Entry("Should be out of date 1", newVM(), newKVVMI("other-image-1"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate), + Entry("Should be out of date 2", newVM(), newKVVMI("other-image-2"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate), + Entry("Should be out of date 3", newVM(), newKVVMI("other-image-3"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate), + Entry("Should be out of date 4", newVM(), newKVVMI("other-image-4"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate), + Entry("Should be out of date 5", newVM(), newKVVMI("other-image-5"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate), ) }) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/lifecycle.go b/images/virtualization-artifact/pkg/controller/vm/internal/lifecycle.go index c33e233376..7166442c0a 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/lifecycle.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/lifecycle.go @@ -28,6 +28,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/deckhouse/virtualization-controller/pkg/common/annotations" + podutil "github.com/deckhouse/virtualization-controller/pkg/common/pod" + commonvmop "github.com/deckhouse/virtualization-controller/pkg/common/vmop" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" @@ -35,8 +37,16 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/logger" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" ) +var lifeCycleConditions = []vmcondition.Type{ + vmcondition.TypeRunning, + vmcondition.TypeMigrating, + vmcondition.TypeMigratable, + vmcondition.TypePodStarted, +} + const nameLifeCycleHandler = "LifeCycleHandler" func NewLifeCycleHandler(client client.Client, recorder eventrecord.EventRecorderLogger) *LifeCycleHandler { @@ -77,7 +87,7 @@ func (h *LifeCycleHandler) Handle(ctx context.Context, s state.VirtualMachineSta return reconcile.Result{}, nil } - if updated := addAllUnknown(changed, vmcondition.TypeRunning); updated || changed.Status.Phase == "" { + if updated := addAllUnknown(changed, lifeCycleConditions...); updated || changed.Status.Phase == "" { changed.Status.Phase = virtv2.MachinePending return reconcile.Result{Requeue: true}, nil } @@ -99,9 +109,18 @@ func (h *LifeCycleHandler) Handle(ctx context.Context, s state.VirtualMachineSta return reconcile.Result{}, err } + vmops, err := s.VMOPs(ctx) + if err != nil { + return reconcile.Result{}, err + } + log := logger.FromContext(ctx).With(logger.SlogHandler(nameLifeCycleHandler)) - h.syncRunning(changed, kvvm, kvvmi, pod, log) + h.syncMigrationState(changed, kvvmi) + h.syncMigrating(changed, kvvmi, vmops, log) + h.syncMigratable(changed, kvvm) + h.syncPodStarted(changed, kvvm, kvvmi, pod) + h.syncRunning(changed, kvvm, kvvmi, log) return reconcile.Result{}, nil } @@ -109,9 +128,104 @@ func (h *LifeCycleHandler) Name() string { return nameLifeCycleHandler } -func (h *LifeCycleHandler) syncRunning(vm *virtv2.VirtualMachine, kvvm *virtv1.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, pod *corev1.Pod, log *slog.Logger) { - cb := conditions.NewConditionBuilder(vmcondition.TypeRunning).Generation(vm.GetGeneration()) +func (h *LifeCycleHandler) syncMigrationState(vm *virtv2.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance) { + if kvvmi == nil || kvvmi.Status.MigrationState == nil { + vm.Status.MigrationState = nil + } else { + vm.Status.MigrationState = h.wrapMigrationState(kvvmi.Status.MigrationState) + } +} + +func (h *LifeCycleHandler) syncMigrating(vm *virtv2.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, vmops []*virtv2.VirtualMachineOperation, log *slog.Logger) { + cbMigrating := conditions.NewConditionBuilder(vmcondition.TypeMigrating).Generation(vm.GetGeneration()) + + var vmop *virtv2.VirtualMachineOperation + { + var inProgressVmops []*virtv2.VirtualMachineOperation + for _, op := range vmops { + if commonvmop.IsMigration(op) && op.Status.Phase == virtv2.VMOPPhaseInProgress { + inProgressVmops = append(inProgressVmops, op) + } + } + + switch length := len(inProgressVmops); length { + case 0: + case 1: + vmop = inProgressVmops[0] + default: + log.Error("Found vmops in progress phase. This is unexpected. Please report a bug.", slog.Int("VMOPCount", length)) + } + } + + switch { + case liveMigrationInProgress(vm.Status.MigrationState): + cbMigrating.Status(metav1.ConditionTrue).Reason(vmcondition.ReasonVmIsMigrating) + conditions.SetCondition(cbMigrating, &vm.Status.Conditions) + + case vmop != nil: + cbMigrating.Status(metav1.ConditionFalse).Reason(vmcondition.ReasonVmIsNotMigrating) + completed, _ := conditions.GetCondition(vmopcondition.TypeCompleted, vmop.Status.Conditions) + switch completed.Reason { + case vmopcondition.ReasonMigrationPending.String(): + cbMigrating.Message("Migration is awaiting start.") + case vmopcondition.ReasonMigrationPrepareTarget.String(): + cbMigrating.Message("Migration is awaiting target preparation.") + case vmopcondition.ReasonMigrationTargetReady.String(): + cbMigrating.Message("Migration is awaiting execution.") + case vmopcondition.ReasonMigrationRunning.String(): + cbMigrating.Status(metav1.ConditionTrue).Reason(vmcondition.ReasonVmIsRunning) + } + conditions.SetCondition(cbMigrating, &vm.Status.Conditions) + + case kvvmi != nil && liveMigrationFailed(vm.Status.MigrationState): + msg := kvvmi.Status.MigrationState.FailureReason + cbMigrating.Status(metav1.ConditionFalse). + Reason(vmcondition.ReasonLastMigrationFinishedWithError). + Message(msg) + conditions.SetCondition(cbMigrating, &vm.Status.Conditions) + + default: + cbMigrating.Status(metav1.ConditionFalse).Reason(vmcondition.ReasonVmIsNotMigrating) + conditions.SetCondition(cbMigrating, &vm.Status.Conditions) + } + +} + +func (h *LifeCycleHandler) syncMigratable(vm *virtv2.VirtualMachine, kvvm *virtv1.VirtualMachine) { + cbMigratable := conditions.NewConditionBuilder(vmcondition.TypeMigratable).Generation(vm.GetGeneration()) + + if kvvm != nil { + liveMigratable := service.GetKVVMCondition(string(virtv1.VirtualMachineInstanceIsMigratable), kvvm.Status.Conditions) + if liveMigratable != nil && liveMigratable.Status == corev1.ConditionFalse && liveMigratable.Reason == virtv1.VirtualMachineInstanceReasonDisksNotMigratable { + cbMigratable.Status(metav1.ConditionFalse). + Reason(vmcondition.ReasonNotMigratable). + Message("Live migration requires that all PVCs must be shared (using ReadWriteMany access mode)") + conditions.SetCondition(cbMigratable, &vm.Status.Conditions) + return + } + } + cbMigratable.Status(metav1.ConditionTrue).Reason(vmcondition.ReasonMigratable) + conditions.SetCondition(cbMigratable, &vm.Status.Conditions) +} + +func liveMigrationInProgress(migrationState *virtv2.VirtualMachineMigrationState) bool { + return migrationState != nil && migrationState.StartTimestamp != nil && migrationState.EndTimestamp == nil +} + +func liveMigrationFailed(migrationState *virtv2.VirtualMachineMigrationState) bool { + return migrationState != nil && migrationState.EndTimestamp != nil && migrationState.Result == virtv2.MigrationResultFailed +} + +func (h *LifeCycleHandler) syncPodStarted(vm *virtv2.VirtualMachine, kvvm *virtv1.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, pod *corev1.Pod) { + cb := conditions.NewConditionBuilder(vmcondition.TypePodStarted).Generation(vm.GetGeneration()) + if podutil.IsPodStarted(pod) { + cb.Status(metav1.ConditionTrue).Reason(vmcondition.ReasonPodStarted) + conditions.SetCondition(cb, &vm.Status.Conditions) + return + } + + // Try to extract error from pod. if pod != nil && pod.Status.Message != "" { cb.Status(metav1.ConditionFalse). Reason(vmcondition.ReasonPodNotStarted). @@ -121,16 +235,13 @@ func (h *LifeCycleHandler) syncRunning(vm *virtv2.VirtualMachine, kvvm *virtv1.V } if kvvm != nil { - podScheduled := service.GetKVVMCondition(string(corev1.PodScheduled), kvvm.Status.Conditions) - if podScheduled != nil && podScheduled.Status == corev1.ConditionFalse { - vm.Status.Phase = virtv2.MachinePending - if podScheduled.Message != "" { - cb.Status(metav1.ConditionFalse). - Reason(vmcondition.ReasonPodNotStarted). - Message(fmt.Sprintf("%s: %s", podScheduled.Reason, podScheduled.Message)) - conditions.SetCondition(cb, &vm.Status.Conditions) - } - + // Try to extract error from kvvm PodScheduled condition. + cond := service.GetKVVMCondition(string(corev1.PodScheduled), kvvm.Status.Conditions) + if cond != nil && cond.Status == corev1.ConditionFalse && cond.Message != "" { + cb.Status(metav1.ConditionFalse). + Reason(vmcondition.ReasonPodNotStarted). + Message(fmt.Sprintf("%s: %s", cond.Reason, cond.Message)) + conditions.SetCondition(cb, &vm.Status.Conditions) return } @@ -150,6 +261,23 @@ func (h *LifeCycleHandler) syncRunning(vm *virtv2.VirtualMachine, kvvm *virtv1.V conditions.SetCondition(cb, &vm.Status.Conditions) return } + } + + cb.Status(metav1.ConditionFalse). + Reason(vmcondition.ReasonPodNotFound). + Message("Pod of the virtual machine was not found") + conditions.SetCondition(cb, &vm.Status.Conditions) +} + +func (h *LifeCycleHandler) syncRunning(vm *virtv2.VirtualMachine, kvvm *virtv1.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, log *slog.Logger) { + cb := conditions.NewConditionBuilder(vmcondition.TypeRunning).Generation(vm.GetGeneration()) + + if kvvm != nil { + podScheduled := service.GetKVVMCondition(string(corev1.PodScheduled), kvvm.Status.Conditions) + if podScheduled != nil && podScheduled.Status == corev1.ConditionFalse { + vm.Status.Phase = virtv2.MachinePending + return + } if isInternalVirtualMachineError(kvvm.Status.PrintableStatus) { msg := fmt.Sprintf("Internal virtual machine error: %s", kvvm.Status.PrintableStatus) @@ -202,3 +330,35 @@ func (h *LifeCycleHandler) syncRunning(vm *virtv2.VirtualMachine, kvvm *virtv1.V cb.Reason(vmcondition.ReasonVmIsNotRunning).Status(metav1.ConditionFalse) conditions.SetCondition(cb, &vm.Status.Conditions) } + +func (h *LifeCycleHandler) wrapMigrationState(state *virtv1.VirtualMachineInstanceMigrationState) *virtv2.VirtualMachineMigrationState { + if state == nil { + return nil + } + return &virtv2.VirtualMachineMigrationState{ + StartTimestamp: state.StartTimestamp, + EndTimestamp: state.EndTimestamp, + Target: virtv2.VirtualMachineLocation{ + Node: state.TargetNode, + Pod: state.TargetPod, + }, + Source: virtv2.VirtualMachineLocation{ + Node: state.SourceNode, + }, + Result: h.getResult(state), + } +} + +func (h *LifeCycleHandler) getResult(state *virtv1.VirtualMachineInstanceMigrationState) virtv2.MigrationResult { + if state == nil { + return "" + } + switch { + case state.Completed && !state.Failed: + return virtv2.MigrationResultSucceeded + case state.Failed: + return virtv2.MigrationResultFailed + default: + return "" + } +} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/migrating.go b/images/virtualization-artifact/pkg/controller/vm/internal/migrating.go deleted file mode 100644 index 463fa0dc80..0000000000 --- a/images/virtualization-artifact/pkg/controller/vm/internal/migrating.go +++ /dev/null @@ -1,206 +0,0 @@ -/* -Copyright 2025 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 internal - -import ( - "context" - "log/slog" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - virtv1 "kubevirt.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - - commonvmop "github.com/deckhouse/virtualization-controller/pkg/common/vmop" - "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/service" - "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" - "github.com/deckhouse/virtualization-controller/pkg/logger" - virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" -) - -const nameMigratingHandler = "MigratingHandler" - -type MigratingHandler struct { -} - -func NewMigratingHandler() *MigratingHandler { - return &MigratingHandler{} -} - -func (h *MigratingHandler) Handle(ctx context.Context, s state.VirtualMachineState) (reconcile.Result, error) { - vm := s.VirtualMachine().Changed() - - if isDeletion(vm) { - return reconcile.Result{}, nil - } - - if vm == nil { - return reconcile.Result{}, nil - } - - kvvm, err := s.KVVM(ctx) - if err != nil { - return reconcile.Result{}, err - } - - kvvmi, err := s.KVVMI(ctx) - if err != nil { - return reconcile.Result{}, err - } - - vmops, err := s.VMOPs(ctx) - if err != nil { - return reconcile.Result{}, err - } - - log := logger.FromContext(ctx).With(logger.SlogHandler(nameLifeCycleHandler)) - vm.Status.MigrationState = h.wrapMigrationState(kvvmi) - - h.syncMigrating(vm, kvvmi, vmops, log) - h.syncMigratable(vm, kvvm) - return reconcile.Result{}, nil -} - -func (h *MigratingHandler) Name() string { - return nameMigratingHandler -} - -func (h *MigratingHandler) wrapMigrationState(kvvmi *virtv1.VirtualMachineInstance) *virtv2.VirtualMachineMigrationState { - if kvvmi == nil { - return nil - } - - migrationState := kvvmi.Status.MigrationState - - if migrationState == nil { - return nil - } - - return &virtv2.VirtualMachineMigrationState{ - StartTimestamp: migrationState.StartTimestamp, - EndTimestamp: migrationState.EndTimestamp, - Target: virtv2.VirtualMachineLocation{ - Node: migrationState.TargetNode, - Pod: migrationState.TargetPod, - }, - Source: virtv2.VirtualMachineLocation{ - Node: migrationState.SourceNode, - }, - Result: h.getMigrationResult(migrationState), - } -} - -func (h *MigratingHandler) getMigrationResult(state *virtv1.VirtualMachineInstanceMigrationState) virtv2.MigrationResult { - if state == nil { - return "" - } - switch { - case state.Completed && !state.Failed: - return virtv2.MigrationResultSucceeded - case state.Failed: - return virtv2.MigrationResultFailed - default: - return "" - } -} - -func (h *MigratingHandler) syncMigrating(vm *virtv2.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, vmops []*virtv2.VirtualMachineOperation, log *slog.Logger) { - cb := conditions.NewConditionBuilder(vmcondition.TypeMigrating).Generation(vm.GetGeneration()) - defer func() { - if cb.Condition().Status == metav1.ConditionTrue || - cb.Condition().Reason == vmcondition.ReasonLastMigrationFinishedWithError.String() || - cb.Condition().Message != "" { - conditions.SetCondition(cb, &vm.Status.Conditions) - } else { - conditions.RemoveCondition(vmcondition.TypeMigrating, &vm.Status.Conditions) - } - }() - - var vmop *virtv2.VirtualMachineOperation - { - var inProgressVmops []*virtv2.VirtualMachineOperation - for _, op := range vmops { - if commonvmop.IsMigration(op) && op.Status.Phase == virtv2.VMOPPhaseInProgress { - inProgressVmops = append(inProgressVmops, op) - } - } - - switch length := len(inProgressVmops); length { - case 0: - case 1: - vmop = inProgressVmops[0] - default: - log.Error("Found vmops in progress phase. This is unexpected. Please report a bug.", slog.Int("VMOPCount", length)) - } - } - - switch { - case liveMigrationInProgress(vm.Status.MigrationState): - cb.Status(metav1.ConditionTrue).Reason(vmcondition.ReasonVmIsMigrating) - conditions.SetCondition(cb, &vm.Status.Conditions) - - case vmop != nil: - cb.Status(metav1.ConditionFalse).Reason(vmcondition.ReasonVmIsNotMigrating) - completed, _ := conditions.GetCondition(vmopcondition.TypeCompleted, vmop.Status.Conditions) - switch completed.Reason { - case vmopcondition.ReasonMigrationPending.String(): - cb.Message("Migration is awaiting start.") - case vmopcondition.ReasonMigrationPrepareTarget.String(): - cb.Message("Migration is awaiting target preparation.") - case vmopcondition.ReasonMigrationTargetReady.String(): - cb.Message("Migration is awaiting execution.") - case vmopcondition.ReasonMigrationRunning.String(): - cb.Status(metav1.ConditionTrue).Reason(vmcondition.ReasonVmIsMigrating) - } - conditions.SetCondition(cb, &vm.Status.Conditions) - - case kvvmi != nil && liveMigrationFailed(vm.Status.MigrationState): - msg := kvvmi.Status.MigrationState.FailureReason - cb.Status(metav1.ConditionFalse). - Reason(vmcondition.ReasonLastMigrationFinishedWithError). - Message(msg) - conditions.SetCondition(cb, &vm.Status.Conditions) - } -} - -func (h *MigratingHandler) syncMigratable(vm *virtv2.VirtualMachine, kvvm *virtv1.VirtualMachine) { - cb := conditions.NewConditionBuilder(vmcondition.TypeMigratable).Generation(vm.GetGeneration()) - - if kvvm != nil { - liveMigratable := service.GetKVVMCondition(string(virtv1.VirtualMachineInstanceIsMigratable), kvvm.Status.Conditions) - if liveMigratable != nil && liveMigratable.Status == corev1.ConditionFalse && liveMigratable.Reason == virtv1.VirtualMachineInstanceReasonDisksNotMigratable { - cb.Status(metav1.ConditionFalse). - Reason(vmcondition.ReasonNotMigratable). - Message("Live migration requires that all PVCs must be shared (using ReadWriteMany access mode)") - conditions.SetCondition(cb, &vm.Status.Conditions) - return - } - } - cb.Status(metav1.ConditionTrue).Reason(vmcondition.ReasonMigratable) - conditions.SetCondition(cb, &vm.Status.Conditions) -} - -func liveMigrationInProgress(migrationState *virtv2.VirtualMachineMigrationState) bool { - return migrationState != nil && migrationState.StartTimestamp != nil && migrationState.EndTimestamp == nil -} - -func liveMigrationFailed(migrationState *virtv2.VirtualMachineMigrationState) bool { - return migrationState != nil && migrationState.EndTimestamp != nil && migrationState.Result == virtv2.MigrationResultFailed -} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/migrating_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/migrating_test.go deleted file mode 100644 index 3f1a10a146..0000000000 --- a/images/virtualization-artifact/pkg/controller/vm/internal/migrating_test.go +++ /dev/null @@ -1,216 +0,0 @@ -/* -Copyright 2025 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 internal - -import ( - "context" - "time" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - virtv1 "kubevirt.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - - vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm" - vmopbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vmop" - "github.com/deckhouse/virtualization-controller/pkg/common/testutil" - "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/reconciler" - "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" - virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition" -) - -var _ = Describe("MigratingHandler", func() { - const ( - name = "vm-migrating" - namespace = "default" - ) - - var ( - ctx = testutil.ContextBackgroundWithNoOpLogger() - fakeClient client.WithWatch - resource *reconciler.Resource[*virtv2.VirtualMachine, virtv2.VirtualMachineStatus] - vmState state.VirtualMachineState - ) - - AfterEach(func() { - fakeClient = nil - resource = nil - vmState = nil - }) - - newVM := func() *virtv2.VirtualMachine { - return vmbuilder.NewEmpty(name, namespace) - } - - newKVVMI := func(migrationState *virtv1.VirtualMachineInstanceMigrationState) *virtv1.VirtualMachineInstance { - kvvmi := newEmptyKVVMI(name, namespace) - kvvmi.Status.MigrationState = migrationState - return kvvmi - } - - newVMOP := func(phase virtv2.VMOPPhase, reason string) *virtv2.VirtualMachineOperation { - vmop := vmopbuilder.New( - vmopbuilder.WithGenerateName("test-vmop-"), - vmopbuilder.WithNamespace(namespace), - vmopbuilder.WithVirtualMachine(name), - vmopbuilder.WithType(virtv2.VMOPTypeMigrate), - ) - vmop.Status.Phase = phase - vmop.Status.Conditions = []metav1.Condition{ - { - Type: vmopcondition.TypeCompleted.String(), - Status: metav1.ConditionFalse, - Reason: reason, - }, - } - return vmop - } - - reconcile := func() { - h := NewMigratingHandler() - _, err := h.Handle(ctx, vmState) - Expect(err).NotTo(HaveOccurred()) - err = resource.Update(context.Background()) - Expect(err).NotTo(HaveOccurred()) - } - - Describe("Condition presence and absence scenarios", func() { - It("Should display migrating condition when migration is in progress", func() { - vm := newVM() - migrationState := &virtv1.VirtualMachineInstanceMigrationState{ - StartTimestamp: &metav1.Time{Time: time.Now()}, - EndTimestamp: nil, - } - kvvmi := newKVVMI(migrationState) - fakeClient, resource, vmState = setupEnvironment(vm, kvvmi) - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - - cond, exists := conditions.GetCondition(vmcondition.TypeMigrating, newVM.Status.Conditions) - Expect(exists).To(BeTrue()) - Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - Expect(cond.Reason).To(Equal(vmcondition.ReasonVmIsMigrating.String())) - }) - - It("Should display condition for last unsuccessful migration", func() { - vm := newVM() - migrationState := &virtv1.VirtualMachineInstanceMigrationState{ - StartTimestamp: &metav1.Time{Time: time.Now()}, - EndTimestamp: &metav1.Time{Time: time.Now()}, - Failed: true, - FailureReason: "Network issues", - } - kvvmi := newKVVMI(migrationState) - fakeClient, resource, vmState = setupEnvironment(vm, kvvmi) - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - - cond, exists := conditions.GetCondition(vmcondition.TypeMigrating, newVM.Status.Conditions) - Expect(exists).To(BeTrue()) - Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - Expect(cond.Reason).To(Equal(vmcondition.ReasonLastMigrationFinishedWithError.String())) - Expect(cond.Message).To(Equal("Network issues")) - }) - - It("Should remove migrating condition when migration is not in progress", func() { - vm := newVM() - vm.Status.Conditions = []metav1.Condition{ - { - Type: vmcondition.TypeMigrating.String(), - Status: metav1.ConditionTrue, - }, - } - kvvmi := newKVVMI(nil) - fakeClient, resource, vmState = setupEnvironment(vm, kvvmi) - reconcile() - - newVM := &virtv2.VirtualMachine{} - - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - - _, exists := conditions.GetCondition(vmcondition.TypeMigrating, newVM.Status.Conditions) - Expect(exists).To(BeFalse()) - }) - - It("Should set condition when vmop is in progress with pending reason", func() { - vm := newVM() - kvvmi := newKVVMI(nil) - vmop := newVMOP(virtv2.VMOPPhaseInProgress, vmopcondition.ReasonMigrationPending.String()) - fakeClient, resource, vmState = setupEnvironment(vm, kvvmi, vmop) - - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - - cond, exists := conditions.GetCondition(vmcondition.TypeMigrating, newVM.Status.Conditions) - Expect(exists).To(BeTrue()) - Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - Expect(cond.Reason).To(Equal(vmcondition.ReasonVmIsNotMigrating.String())) - Expect(cond.Message).To(Equal("Migration is awaiting start.")) - }) - - It("Should set condition when vmop is in progress with target ready reason", func() { - vm := newVM() - kvvmi := newKVVMI(nil) - vmop := newVMOP(virtv2.VMOPPhaseInProgress, vmopcondition.ReasonMigrationTargetReady.String()) - fakeClient, resource, vmState = setupEnvironment(vm, kvvmi, vmop) - - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - - cond, exists := conditions.GetCondition(vmcondition.TypeMigrating, newVM.Status.Conditions) - Expect(exists).To(BeTrue()) - Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - Expect(cond.Reason).To(Equal(vmcondition.ReasonVmIsNotMigrating.String())) - Expect(cond.Message).To(Equal("Migration is awaiting execution.")) - }) - - It("Should set condition when vmop is in progress with running reason", func() { - vm := newVM() - kvvmi := newKVVMI(nil) - vmop := newVMOP(virtv2.VMOPPhaseInProgress, vmopcondition.ReasonMigrationRunning.String()) - fakeClient, resource, vmState = setupEnvironment(vm, kvvmi, vmop) - - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - - cond, exists := conditions.GetCondition(vmcondition.TypeMigrating, newVM.Status.Conditions) - Expect(exists).To(BeTrue()) - Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - Expect(cond.Reason).To(Equal(vmcondition.ReasonVmIsMigrating.String())) - }) - }) -}) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/size_policy.go b/images/virtualization-artifact/pkg/controller/vm/internal/size_policy.go index 1523d81cc6..eea7e32471 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/size_policy.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/size_policy.go @@ -53,13 +53,7 @@ func (h *SizePolicyHandler) Handle(ctx context.Context, s state.VirtualMachineSt Status(metav1.ConditionUnknown). Generation(current.GetGeneration()) - defer func() { - if cb.Condition().Status == metav1.ConditionFalse { - conditions.SetCondition(cb, &changed.Status.Conditions) - } else { - conditions.RemoveCondition(vmcondition.TypeSizingPolicyMatched, &changed.Status.Conditions) - } - }() + defer func() { conditions.SetCondition(cb, &changed.Status.Conditions) }() if isDeletion(current) { return reconcile.Result{}, nil @@ -81,7 +75,10 @@ func (h *SizePolicyHandler) Handle(ctx context.Context, s state.VirtualMachineSt Status(metav1.ConditionFalse) default: err = h.service.CheckVMMatchedSizePolicy(changed, vmClass) - if err != nil { + if err == nil { + cb.Reason(vmcondition.ReasonSizingPolicyMatched). + Status(metav1.ConditionTrue) + } else { cb.Message(fmt.Sprintf("Size policy matching errors: %s.", err.Error())). Reason(vmcondition.ReasonSizingPolicyNotMatched). Status(metav1.ConditionFalse) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/size_policy_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/size_policy_test.go deleted file mode 100644 index 63b6aac1e5..0000000000 --- a/images/virtualization-artifact/pkg/controller/vm/internal/size_policy_test.go +++ /dev/null @@ -1,130 +0,0 @@ -/* -Copyright 2025 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 internal - -import ( - "context" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/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-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/reconciler" - "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" - virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" -) - -var _ = Describe("SizePolicyHandler", func() { - const ( - name = "vm-size" - namespace = "default" - vmClassName = "vmclass" - ) - - var ( - ctx = testutil.ContextBackgroundWithNoOpLogger() - fakeClient client.WithWatch - resource *reconciler.Resource[*virtv2.VirtualMachine, virtv2.VirtualMachineStatus] - vmState state.VirtualMachineState - ) - - AfterEach(func() { - fakeClient = nil - resource = nil - vmState = nil - }) - - newVM := func(vmClassName string) *virtv2.VirtualMachine { - vm := vmbuilder.NewEmpty(name, namespace) - if vmClassName != "" { - vm.Spec.VirtualMachineClassName = vmClassName - } - - return vm - } - - reconcile := func() { - h := NewSizePolicyHandler() - _, err := h.Handle(ctx, vmState) - Expect(err).NotTo(HaveOccurred()) - err = resource.Update(context.Background()) - Expect(err).NotTo(HaveOccurred()) - } - - Describe("Condition presence and absence scenarios", func() { - It("Should add condition if it was absent and size policy does not match", func() { - vm := newVM("") - fakeClient, resource, vmState = setupEnvironment(vm) - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - cond, exists := conditions.GetCondition(vmcondition.TypeSizingPolicyMatched, newVM.Status.Conditions) - Expect(exists).To(BeTrue()) - Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - }) - - It("Should remove condition if it was present and size policy matches now", func() { - vm := newVM(vmClassName) - vm.Status.Conditions = []metav1.Condition{ - { - Type: vmcondition.TypeSizingPolicyMatched.String(), - Status: metav1.ConditionFalse, - }, - } - - vmClass := &virtv2.VirtualMachineClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: vmClassName, - }, - } - fakeClient, resource, vmState = setupEnvironment(vm, vmClass) - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - _, exists := conditions.GetCondition(vmcondition.TypeSizingPolicyMatched, newVM.Status.Conditions) - Expect(exists).To(BeFalse()) - }) - - It("Should not add condition if it was absent and size policy matches", func() { - vm := newVM(vmClassName) - - vmClass := &virtv2.VirtualMachineClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: vmClassName, - }, - } - fakeClient, resource, vmState = setupEnvironment(vm, vmClass) - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - _, exists := conditions.GetCondition(vmcondition.TypeSizingPolicyMatched, newVM.Status.Conditions) - Expect(exists).To(BeFalse()) - }) - }) -}) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/snapshotting.go b/images/virtualization-artifact/pkg/controller/vm/internal/snapshotting.go index b621c7283d..ffe8de7971 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/snapshotting.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/snapshotting.go @@ -46,6 +46,10 @@ func (h *SnapshottingHandler) Handle(ctx context.Context, s state.VirtualMachine vm := s.VirtualMachine().Changed() + if update := addAllUnknown(vm, vmcondition.TypeSnapshotting); update { + return reconcile.Result{Requeue: true}, nil + } + if isDeletion(vm) { return reconcile.Result{}, nil } @@ -56,17 +60,11 @@ func (h *SnapshottingHandler) Handle(ctx context.Context, s state.VirtualMachine return reconcile.Result{}, err } - cb := conditions.NewConditionBuilder(vmcondition.TypeSnapshotting). - Status(metav1.ConditionUnknown). - Generation(vm.GetGeneration()) + cb := conditions.NewConditionBuilder(vmcondition.TypeSnapshotting).Generation(vm.GetGeneration()) - defer func() { - if cb.Condition().Status == metav1.ConditionTrue { - conditions.SetCondition(cb, &vm.Status.Conditions) - } else { - conditions.RemoveCondition(vmcondition.TypeSnapshotting, &vm.Status.Conditions) - } - }() + defer func() { conditions.SetCondition(cb, &vm.Status.Conditions) }() + + cb.Status(metav1.ConditionUnknown) for _, vmSnapshot := range vmSnapshots.Items { if vmSnapshot.Spec.VirtualMachineName != vm.Name { diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/snapshotting_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/snapshotting_test.go deleted file mode 100644 index a0c78e637c..0000000000 --- a/images/virtualization-artifact/pkg/controller/vm/internal/snapshotting_test.go +++ /dev/null @@ -1,145 +0,0 @@ -/* -Copyright 2025 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 internal - -import ( - "context" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/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-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/reconciler" - "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" - virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" -) - -var _ = Describe("SnapshottingHandler", func() { - const ( - name = "vm-snapshot" - namespace = "default" - ) - - var ( - ctx = testutil.ContextBackgroundWithNoOpLogger() - fakeClient client.WithWatch - resource *reconciler.Resource[*virtv2.VirtualMachine, virtv2.VirtualMachineStatus] - vmState state.VirtualMachineState - ) - - AfterEach(func() { - fakeClient = nil - resource = nil - vmState = nil - }) - - newVM := func() *virtv2.VirtualMachine { - return vmbuilder.NewEmpty(name, namespace) - } - - newVMSnapshot := func(vmName string, phase virtv2.VirtualMachineSnapshotPhase) *virtv2.VirtualMachineSnapshot { - return &virtv2.VirtualMachineSnapshot{ - ObjectMeta: metav1.ObjectMeta{ - Name: vmName + "-snapshot", - Namespace: namespace, - }, - Spec: virtv2.VirtualMachineSnapshotSpec{ - VirtualMachineName: vmName, - }, - Status: virtv2.VirtualMachineSnapshotStatus{ - Phase: phase, - }, - } - } - - reconcile := func() { - h := NewSnapshottingHandler(fakeClient) - _, err := h.Handle(ctx, vmState) - Expect(err).NotTo(HaveOccurred()) - err = resource.Update(context.Background()) - Expect(err).NotTo(HaveOccurred()) - } - - Describe("Condition presence and absence scenarios", func() { - It("Should add condition if snapshot is in progress", func() { - vm := newVM() - snapshot := newVMSnapshot(vm.Name, virtv2.VirtualMachineSnapshotPhaseInProgress) - fakeClient, resource, vmState = setupEnvironment(vm, snapshot) - - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - - cond, exists := conditions.GetCondition(vmcondition.TypeSnapshotting, newVM.Status.Conditions) - Expect(exists).To(BeTrue()) - Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - }) - - It("Should not add condition if snapshot is ready", func() { - vm := newVM() - snapshot := newVMSnapshot(vm.Name, virtv2.VirtualMachineSnapshotPhaseReady) - fakeClient, resource, vmState = setupEnvironment(vm, snapshot) - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - - _, exists := conditions.GetCondition(vmcondition.TypeSnapshotting, newVM.Status.Conditions) - Expect(exists).To(BeFalse()) - }) - - It("Should not add condition if no snapshots exist", func() { - vm := newVM() - fakeClient, resource, vmState = setupEnvironment(vm) - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - - _, exists := conditions.GetCondition(vmcondition.TypeSnapshotting, newVM.Status.Conditions) - Expect(exists).To(BeFalse()) - }) - - It("Should remove condition if it was present but no snapshots in progress", func() { - vm := newVM() - vm.Status.Conditions = []metav1.Condition{ - { - Type: vmcondition.TypeSnapshotting.String(), - Status: metav1.ConditionTrue, - }, - } - fakeClient, resource, vmState = setupEnvironment(vm) - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - - _, exists := conditions.GetCondition(vmcondition.TypeSnapshotting, newVM.Status.Conditions) - Expect(exists).To(BeFalse()) - }) - }) -}) 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 090275218b..ce062d48de 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -80,24 +80,8 @@ func (h *SyncKvvmHandler) Handle(ctx context.Context, s state.VirtualMachineStat Reason(vmcondition.ReasonRestartNoNeed) defer func() { - switch changed.Status.Phase { - case virtv2.MachinePending, virtv2.MachineStarting, virtv2.MachineStopped: - conditions.RemoveCondition(vmcondition.TypeConfigurationApplied, &changed.Status.Conditions) - conditions.RemoveCondition(vmcondition.TypeAwaitingRestartToApplyConfiguration, &changed.Status.Conditions) - - default: - if cbConfApplied.Condition().Status == metav1.ConditionFalse { - conditions.SetCondition(cbConfApplied, &changed.Status.Conditions) - } else { - conditions.RemoveCondition(vmcondition.TypeConfigurationApplied, &changed.Status.Conditions) - } - - if cbAwaitingRestart.Condition().Status == metav1.ConditionTrue { - conditions.SetCondition(cbAwaitingRestart, &changed.Status.Conditions) - } else { - conditions.RemoveCondition(vmcondition.TypeAwaitingRestartToApplyConfiguration, &changed.Status.Conditions) - } - } + conditions.SetCondition(cbConfApplied, &changed.Status.Conditions) + conditions.SetCondition(cbAwaitingRestart, &changed.Status.Conditions) }() if isDeletion(current) { @@ -223,7 +207,36 @@ func (h *SyncKvvmHandler) Name() string { } func (h *SyncKvvmHandler) isWaiting(vm *virtv2.VirtualMachine) bool { - return !checkVirtualMachineConfiguration(vm) + for _, c := range vm.Status.Conditions { + switch vmcondition.Type(c.Type) { + case vmcondition.TypeBlockDevicesReady: + if c.Status != metav1.ConditionTrue && c.Reason != vmcondition.ReasonWaitingForProvisioningToPVC.String() { + return true + } + + case vmcondition.TypeSnapshotting: + if c.Status == metav1.ConditionTrue && c.Reason == vmcondition.ReasonSnapshottingInProgress.String() { + return true + } + + case vmcondition.TypeIPAddressReady: + if c.Status != metav1.ConditionTrue && c.Reason != vmcondition.ReasonIPAddressNotAssigned.String() { + return true + } + + case vmcondition.TypeProvisioningReady, + vmcondition.TypeClassReady: + if c.Status != metav1.ConditionTrue { + return true + } + + case vmcondition.TypeSizingPolicyMatched: + if c.Status != metav1.ConditionTrue { + return true + } + } + } + return false } func (h *SyncKvvmHandler) syncKVVM(ctx context.Context, s state.VirtualMachineState, allChanges vmchange.SpecChanges) (bool, error) { 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 deleted file mode 100644 index 443ecc4820..0000000000 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm_test.go +++ /dev/null @@ -1,257 +0,0 @@ -/* -Copyright 2025 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 internal - -import ( - "context" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/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-controller/pkg/controller/conditions" - "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-controller/pkg/eventrecord" - virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" -) - -var _ = Describe("SyncKvvmHandler", func() { - const ( - name = "vm-sync" - namespace = "default" - ) - - var ( - ctx context.Context - fakeClient client.WithWatch - resource *reconciler.Resource[*virtv2.VirtualMachine, virtv2.VirtualMachineStatus] - vmState state.VirtualMachineState - recorder *eventrecord.EventRecorderLoggerMock - ) - - BeforeEach(func() { - ctx = testutil.ContextBackgroundWithNoOpLogger() - fakeClient = nil - resource = nil - vmState = nil - recorder = &eventrecord.EventRecorderLoggerMock{ - EventFunc: func(_ client.Object, _, _, _ string) {}, - EventfFunc: func(_ client.Object, _, _, _ string, _ ...interface{}) {}, - WithLoggingFunc: func(logger eventrecord.InfoLogger) eventrecord.EventRecorderLogger { return recorder }, - } - }) - - AfterEach(func() { - fakeClient = nil - resource = nil - vmState = nil - recorder = nil - }) - - newVM := func(phase virtv2.MachinePhase) *virtv2.VirtualMachine { - vm := vmbuilder.NewEmpty(name, namespace) - vm.Status.Phase = phase - return vm - } - - newKVVM := func() *virtv1.VirtualMachine { - return &virtv1.VirtualMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - }, - Spec: virtv1.VirtualMachineSpec{ - Template: &virtv1.VirtualMachineInstanceTemplateSpec{}, - }, - } - } - - newKVVMI := func() *virtv1.VirtualMachineInstance { - kvvmi := newEmptyKVVMI(name, namespace) - return kvvmi - } - - reconcile := func() { - h := NewSyncKvvmHandler(nil, fakeClient, recorder) - _, err := h.Handle(ctx, vmState) - Expect(err).NotTo(HaveOccurred()) - err = resource.Update(context.Background()) - Expect(err).NotTo(HaveOccurred()) - } - - mutateVM := func(vm *virtv2.VirtualMachine) { - vm.Spec.VirtualMachineClassName = "vmclass" - vm.Spec.CPU.Cores = 2 - vm.Spec.RunPolicy = virtv2.ManualPolicy - vm.Spec.VirtualMachineIPAddress = "test-ip" - vm.Spec.OsType = virtv2.GenericOs - vm.Spec.Disruptions = &virtv2.Disruptions{ - RestartApprovalMode: virtv2.Manual, - } - } - - mutateKVVM := func(kvvm *virtv1.VirtualMachine) { - kvbuilder.SetLastAppliedSpec(kvvm, &virtv2.VirtualMachine{ - Spec: virtv2.VirtualMachineSpec{ - CPU: virtv2.CPUSpec{ - Cores: 1, - }, - VirtualMachineIPAddress: "test-ip", - RunPolicy: virtv2.ManualPolicy, - OsType: virtv2.GenericOs, - VirtualMachineClassName: "vmclass", - }, - }) - - kvbuilder.SetLastAppliedClassSpec(kvvm, &virtv2.VirtualMachineClass{ - Spec: virtv2.VirtualMachineClassSpec{ - CPU: virtv2.CPU{ - Type: virtv2.CPUTypeHost, - }, - NodeSelector: virtv2.NodeSelector{ - MatchLabels: map[string]string{ - "test": "test", - }, - }, - }, - }) - } - - DescribeTable("AwaitingRestart Condition Tests", - func(phase virtv2.MachinePhase, needChange bool, expectedStatus metav1.ConditionStatus, expectedExistence bool) { - vm := newVM(phase) - kvvm := newKVVM() - kvvmi := newKVVMI() - - ip := &virtv2.VirtualMachineIPAddress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ip", - Namespace: namespace, - }, - Spec: virtv2.VirtualMachineIPAddressSpec{ - Type: virtv2.VirtualMachineIPAddressTypeStatic, - StaticIP: "192.168.1.10", - }, - Status: virtv2.VirtualMachineIPAddressStatus{ - Address: "192.168.1.10", - Phase: virtv2.VirtualMachineIPAddressPhaseAttached, - }, - } - - vmClass := &virtv2.VirtualMachineClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: "vmclass", - }, Spec: virtv2.VirtualMachineClassSpec{ - CPU: virtv2.CPU{ - Type: virtv2.CPUTypeHost, - }, - NodeSelector: virtv2.NodeSelector{ - MatchLabels: map[string]string{ - "test2": "test2", - }, - }, - }, - } - - if needChange { - mutateVM(vm) - mutateKVVM(kvvm) - } - - fakeClient, resource, vmState = setupEnvironment(vm, kvvm, kvvmi, ip, vmClass) - reconcile() - - newVM := &virtv2.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("Running phase with changes", virtv2.MachineRunning, true, metav1.ConditionTrue, true), - Entry("Running phase without changes", virtv2.MachineRunning, false, metav1.ConditionUnknown, false), - - Entry("Migrating phase with changes, condition should exist", virtv2.MachineMigrating, true, metav1.ConditionTrue, true), - Entry("Migrating phase without changes, condition should not exist", virtv2.MachineMigrating, false, metav1.ConditionUnknown, false), - - Entry("Stopping phase with changes, condition should exist", virtv2.MachineStopping, true, metav1.ConditionTrue, true), - Entry("Stopping phase without changes, condition should not exist", virtv2.MachineStopping, false, metav1.ConditionUnknown, false), - - Entry("Stopped phase with changes, shouldn't have condition", virtv2.MachineStopped, true, metav1.ConditionUnknown, false), - Entry("Stopped phase without changes, shouldn't have condition", virtv2.MachineStopped, false, metav1.ConditionUnknown, false), - - Entry("Starting phase with changes, shouldn't have condition", virtv2.MachineStarting, true, metav1.ConditionUnknown, false), - Entry("Starting phase without changes, shouldn't have condition", virtv2.MachineStarting, false, metav1.ConditionUnknown, false), - - Entry("Pending phase with changes, shouldn't have condition", virtv2.MachinePending, true, metav1.ConditionUnknown, false), - Entry("Pending phase without changes, shouldn't have condition", virtv2.MachinePending, false, metav1.ConditionUnknown, false), - ) - - DescribeTable("ConfigurationApplied Condition Tests", - func(phase virtv2.MachinePhase, notReady bool, expectedStatus metav1.ConditionStatus, expectedExistence bool) { - vm := newVM(phase) - if notReady { - vm.Status.Conditions = append(vm.Status.Conditions, metav1.Condition{ - Type: vmcondition.TypeBlockDevicesReady.String(), - Status: metav1.ConditionFalse, - Reason: "BlockDevicesNotReady", - }) - } - kvvm := newKVVM() - - fakeClient, resource, vmState = setupEnvironment(vm, kvvm) - reconcile() - - newVM := &virtv2.VirtualMachine{} - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM) - Expect(err).NotTo(HaveOccurred()) - - confAppliedCond, confAppliedExists := conditions.GetCondition(vmcondition.TypeConfigurationApplied, newVM.Status.Conditions) - Expect(confAppliedExists).To(Equal(expectedExistence)) - if confAppliedExists { - Expect(confAppliedCond.Status).To(Equal(expectedStatus)) - } - }, - Entry("Running phase with changes applied", virtv2.MachineRunning, false, metav1.ConditionUnknown, false), - Entry("Running phase with changes not applied", virtv2.MachineRunning, true, metav1.ConditionFalse, true), - - Entry("Migrating phase with changes applied, condition should not exist", virtv2.MachineMigrating, false, metav1.ConditionUnknown, false), - Entry("Migrating phase with changes not applied, condition should exist", virtv2.MachineMigrating, true, metav1.ConditionFalse, true), - - Entry("Stopping phase with changes applied, condition should not exist", virtv2.MachineStopping, false, metav1.ConditionUnknown, false), - Entry("Stopping phase with changes not applied, condition should exist", virtv2.MachineStopping, true, metav1.ConditionFalse, true), - - Entry("Stopped phase with changes applied, condition should not exist", virtv2.MachineStopped, false, metav1.ConditionUnknown, false), - Entry("Stopped phase with changes not applied, condition should not exist", virtv2.MachineStopped, true, metav1.ConditionUnknown, false), - - Entry("Starting phase with changes applied, condition should not exist", virtv2.MachineStarting, false, metav1.ConditionUnknown, false), - Entry("Starting phase with changes not applied, condition should not exist", virtv2.MachineStarting, true, metav1.ConditionUnknown, false), - - Entry("Pending phase with changes applied, condition should not exist", virtv2.MachinePending, false, metav1.ConditionUnknown, false), - Entry("Pending phase with changes not applied, condition should not exist", virtv2.MachinePending, true, metav1.ConditionUnknown, false), - ) -}) 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 277b2356d9..d52c77791e 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 @@ -22,18 +22,21 @@ import ( corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/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" kvvmutil "github.com/deckhouse/virtualization-controller/pkg/common/kvvm" + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/kvbuilder" "github.com/deckhouse/virtualization-controller/pkg/controller/powerstate" "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization-controller/pkg/logger" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" ) const nameSyncPowerStateHandler = "SyncPowerStateHandler" @@ -123,7 +126,11 @@ func (h *SyncPowerStateHandler) syncPowerState( shutdownInfo = s.ShutdownInfo }) - isConfigurationApplied := checkVirtualMachineConfiguration(s.VirtualMachine().Changed()) + appliedCondition, _ := conditions.GetCondition(vmcondition.TypeConfigurationApplied, + s.VirtualMachine().Changed().Status.Conditions) + isConfigurationApplied := appliedCondition.Status == metav1.ConditionTrue && + appliedCondition.ObservedGeneration == s.VirtualMachine().Changed().Generation + var vmAction VMAction switch runPolicy { case virtv2.AlwaysOffPolicy: diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/util.go b/images/virtualization-artifact/pkg/controller/vm/internal/util.go index 6ef4c9529d..1b40a2560f 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/util.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/util.go @@ -96,7 +96,8 @@ 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 *virtv2.VirtualMachine, kvvm *virtv1.VirtualMachine) virtv2.MachinePhase { if vm != nil && kvvm != nil { - if !checkVirtualMachineConfiguration(vm) && + confAppliedCondition, _ := conditions.GetCondition(vmcondition.TypeConfigurationApplied, vm.Status.Conditions) + if confAppliedCondition.Status == metav1.ConditionFalse && kvvm != nil && kvvm.Annotations[annotations.AnnVmStartRequested] == "true" { return virtv2.MachinePending } @@ -244,36 +245,3 @@ func isInternalVirtualMachineError(phase virtv1.VirtualMachinePrintableStatus) b func podFinal(pod corev1.Pod) bool { return pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed } - -func checkVirtualMachineConfiguration(vm *virtv2.VirtualMachine) bool { - for _, c := range vm.Status.Conditions { - switch vmcondition.Type(c.Type) { - case vmcondition.TypeBlockDevicesReady: - if c.Status != metav1.ConditionTrue && c.Reason != vmcondition.ReasonWaitingForProvisioningToPVC.String() { - return false - } - - case vmcondition.TypeSnapshotting: - if c.Status == metav1.ConditionTrue && c.Reason == vmcondition.ReasonSnapshottingInProgress.String() { - return false - } - - case vmcondition.TypeIPAddressReady: - if c.Status != metav1.ConditionTrue && c.Reason != vmcondition.ReasonIPAddressNotAssigned.String() { - return false - } - - case vmcondition.TypeProvisioningReady, - vmcondition.TypeClassReady: - if c.Status != metav1.ConditionTrue { - return false - } - - case vmcondition.TypeSizingPolicyMatched: - if c.Status != metav1.ConditionTrue { - return false - } - } - } - return true -} diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_controller.go b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go index a1ae262722..fc29247373 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_controller.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go @@ -68,7 +68,6 @@ func SetupController( internal.NewSyncPowerStateHandler(client, recorder), internal.NewSyncMetadataHandler(client), internal.NewLifeCycleHandler(client, recorder), - internal.NewMigratingHandler(), internal.NewFirmwareHandler(firmwareImage), internal.NewEvictHandler(), internal.NewStatisticHandler(client),