From ad1dc4283f74ee17f210e5b0c6ab7afa38113790 Mon Sep 17 00:00:00 2001 From: Yaroslav Borbat Date: Mon, 6 Jul 2026 10:52:27 +0300 Subject: [PATCH] fix(vm): fix VM stuck until the child KVVMI in Failed phase is deleted manually Signed-off-by: Yaroslav Borbat --- .../pkg/common/kvvm/kvvm.go | 10 +- .../pkg/common/kvvm/kvvm_test.go | 101 ++++++++++++++++++ .../pkg/common/kvvm/suite_test.go | 29 +++++ .../controller/powerstate/shutdown_reason.go | 12 ++- .../powerstate/shutdown_reason_test.go | 53 ++++++++- .../vm/internal/sync_power_state.go | 21 ++-- .../vm/internal/sync_power_state_test.go | 42 ++++++++ .../pkg/controller/vm/vm_controller.go | 6 +- 8 files changed, 261 insertions(+), 13 deletions(-) create mode 100644 images/virtualization-artifact/pkg/common/kvvm/kvvm_test.go create mode 100644 images/virtualization-artifact/pkg/common/kvvm/suite_test.go diff --git a/images/virtualization-artifact/pkg/common/kvvm/kvvm.go b/images/virtualization-artifact/pkg/common/kvvm/kvvm.go index 43b2348bde..c652db754e 100644 --- a/images/virtualization-artifact/pkg/common/kvvm/kvvm.go +++ b/images/virtualization-artifact/pkg/common/kvvm/kvvm.go @@ -69,9 +69,13 @@ func GetVMPod(kvvmi *virtv1.VirtualMachineInstance, podList *corev1.PodList) *co return nil } + nodeName := kvvmi.Status.NodeName + var pods []corev1.Pod for _, pod := range podList.Items { - if pod.Spec.NodeName != kvvmi.Status.NodeName { + // if KVVMI completed, nodeName can be empty + kvvmCompletedWithEmptyNodeName := kvvmiCompleted(kvvmi) && nodeName == "" + if !kvvmCompletedWithEmptyNodeName && nodeName != pod.Spec.NodeName { continue } if _, exists := kvvmi.Status.ActivePods[pod.GetUID()]; !exists { @@ -127,3 +131,7 @@ func RemoveStartAnnotation(ctx context.Context, cl client.Client, kvvm *virtv1.V func RemoveRestartAnnotation(ctx context.Context, cl client.Client, kvvm *virtv1.VirtualMachine) error { return object.RemoveAnnotation(ctx, cl, kvvm, annotations.AnnVMRestartRequested) } + +func kvvmiCompleted(kvvmi *virtv1.VirtualMachineInstance) bool { + return kvvmi.Status.Phase == virtv1.Succeeded || kvvmi.Status.Phase == virtv1.Failed +} diff --git a/images/virtualization-artifact/pkg/common/kvvm/kvvm_test.go b/images/virtualization-artifact/pkg/common/kvvm/kvvm_test.go new file mode 100644 index 0000000000..94c900dcd5 --- /dev/null +++ b/images/virtualization-artifact/pkg/common/kvvm/kvvm_test.go @@ -0,0 +1,101 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kvvm + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + virtv1 "kubevirt.io/api/core/v1" +) + +var _ = Describe("GetVMPod", func() { + const ( + podName = "virt-launcher-test" + nodeName = "worker-1" + ) + + var ( + podUID types.UID = "pod-uid" + kvvmi *virtv1.VirtualMachineInstance + pod corev1.Pod + ) + + BeforeEach(func() { + kvvmi = &virtv1.VirtualMachineInstance{ + Status: virtv1.VirtualMachineInstanceStatus{ + Phase: virtv1.Running, + NodeName: nodeName, + ActivePods: map[types.UID]string{ + podUID: podName, + }, + }, + } + pod = corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + UID: podUID, + }, + Spec: corev1.PodSpec{ + NodeName: nodeName, + }, + } + }) + + It("returns pod when node names match", func() { + result := GetVMPod(kvvmi, &corev1.PodList{Items: []corev1.Pod{pod}}) + Expect(result).NotTo(BeNil()) + Expect(result.Name).To(Equal(podName)) + }) + + It("skips pod on different node when kvvmi is running", func() { + pod.Spec.NodeName = "worker-2" + + result := GetVMPod(kvvmi, &corev1.PodList{Items: []corev1.Pod{pod}}) + Expect(result).To(BeNil()) + }) + + It("returns pod when kvvmi is completed and nodeName is empty", func() { + kvvmi.Status.Phase = virtv1.Failed + kvvmi.Status.NodeName = "" + pod.Spec.NodeName = "worker-2" + + result := GetVMPod(kvvmi, &corev1.PodList{Items: []corev1.Pod{pod}}) + Expect(result).NotTo(BeNil()) + Expect(result.Name).To(Equal(podName)) + }) + + It("returns pod when kvvmi is succeeded and nodeName is empty", func() { + kvvmi.Status.Phase = virtv1.Succeeded + kvvmi.Status.NodeName = "" + pod.Spec.NodeName = "worker-2" + + result := GetVMPod(kvvmi, &corev1.PodList{Items: []corev1.Pod{pod}}) + Expect(result).NotTo(BeNil()) + Expect(result.Name).To(Equal(podName)) + }) + + It("skips pod on different node when kvvmi is running with empty nodeName", func() { + kvvmi.Status.NodeName = "" + pod.Spec.NodeName = "worker-2" + + result := GetVMPod(kvvmi, &corev1.PodList{Items: []corev1.Pod{pod}}) + Expect(result).To(BeNil()) + }) +}) diff --git a/images/virtualization-artifact/pkg/common/kvvm/suite_test.go b/images/virtualization-artifact/pkg/common/kvvm/suite_test.go new file mode 100644 index 0000000000..341f8876b3 --- /dev/null +++ b/images/virtualization-artifact/pkg/common/kvvm/suite_test.go @@ -0,0 +1,29 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kvvm + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestKVVM(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Common KVVM Suite") +} diff --git a/images/virtualization-artifact/pkg/controller/powerstate/shutdown_reason.go b/images/virtualization-artifact/pkg/controller/powerstate/shutdown_reason.go index 4f10d73b4f..1528441126 100644 --- a/images/virtualization-artifact/pkg/controller/powerstate/shutdown_reason.go +++ b/images/virtualization-artifact/pkg/controller/powerstate/shutdown_reason.go @@ -47,7 +47,7 @@ const ( // {"event":"SHUTDOWN","details":"{\"guest\":true,\"reason\":\"guest-reset\"}"} // {"event":"SHUTDOWN","details":"{\"guest\":false,\"reason\":\"host-signal\"}"} func ShutdownReason(vm *v1alpha2.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, kvPods *corev1.PodList) ShutdownInfo { - if kvvmi == nil || kvvmi.Status.Phase != virtv1.Succeeded { + if kvvmi == nil || !kvvmiCompleted(kvvmi) { return ShutdownInfo{} } if kvPods == nil || len(kvPods.Items) == 0 { @@ -59,7 +59,7 @@ func ShutdownReason(vm *v1alpha2.VirtualMachine, kvvmi *virtv1.VirtualMachineIns } // Power events are not available in Running state, only Completed Pod has termination message. - if activePod.Status.Phase != corev1.PodSucceeded { + if !podCompleted(activePod) { return ShutdownInfo{} } @@ -106,3 +106,11 @@ type ShutdownInfo struct { PodCompleted bool Pod corev1.Pod } + +func kvvmiCompleted(kvvmi *virtv1.VirtualMachineInstance) bool { + return kvvmi.Status.Phase == virtv1.Succeeded || kvvmi.Status.Phase == virtv1.Failed +} + +func podCompleted(pod corev1.Pod) bool { + return pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed +} diff --git a/images/virtualization-artifact/pkg/controller/powerstate/shutdown_reason_test.go b/images/virtualization-artifact/pkg/controller/powerstate/shutdown_reason_test.go index 4fe35524d9..63ef7b589b 100644 --- a/images/virtualization-artifact/pkg/controller/powerstate/shutdown_reason_test.go +++ b/images/virtualization-artifact/pkg/controller/powerstate/shutdown_reason_test.go @@ -49,7 +49,7 @@ var _ = Describe("ShutdownReason", func() { }) }) - Context("when kvvmi is not in Succeeded phase", func() { + Context("when kvvmi is not completed", func() { BeforeEach(func() { kvvmi = &virtv1.VirtualMachineInstance{ Status: virtv1.VirtualMachineInstanceStatus{ @@ -64,6 +64,55 @@ var _ = Describe("ShutdownReason", func() { }) }) + Context("when kvvmi is in Failed phase", func() { + BeforeEach(func() { + kvvmi = &virtv1.VirtualMachineInstance{ + Status: virtv1.VirtualMachineInstanceStatus{ + Phase: virtv1.Failed, + }, + } + vm.Status = v1alpha2.VirtualMachineStatus{ + VirtualMachinePods: []v1alpha2.VirtualMachinePod{ + { + Name: "test-pod", + Active: true, + }, + }, + } + }) + + It("should return ShutdownInfo when pod is in Failed phase", func() { + pods = &corev1.PodList{ + Items: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-namespace", + }, + Status: corev1.PodStatus{ + Phase: corev1.PodFailed, + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "test-compute", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `{"event":"SHUTDOWN","details":"{\"guest\":true,\"reason\":\"guest-shutdown\"}"}`, + }, + }, + }, + }, + }, + }, + }, + } + + result := ShutdownReason(vm, kvvmi, pods) + Expect(result.PodCompleted).To(BeTrue()) + Expect(result.Reason).To(Equal(GuestShutdownReason)) + Expect(result.Pod.Name).To(Equal("test-pod")) + }) + }) + Context("when kvPods is nil", func() { BeforeEach(func() { kvvmi = &virtv1.VirtualMachineInstance{ @@ -169,7 +218,7 @@ var _ = Describe("ShutdownReason", func() { }) }) - Context("when pod is not in Succeeded phase", func() { + Context("when pod is not completed", func() { BeforeEach(func() { kvvmi = &virtv1.VirtualMachineInstance{ Status: virtv1.VirtualMachineInstanceStatus{ 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 fbb1143601..b80f9760df 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 @@ -209,27 +209,30 @@ func (h *SyncPowerStateHandler) handleManualPolicy( isConfigurationApplied bool, shutdownInfo powerstate.ShutdownInfo, ) VMAction { - if kvvmi == nil || kvvmi.DeletionTimestamp != nil { + if kvvmi == nil || kvvmi.DeletionTimestamp != nil || kvvmiCompleted(kvvmi) { if h.checkNeedStartVM(ctx, s, kvvm, isConfigurationApplied, v1alpha2.ManualPolicy) { return Start } - return Nothing + + if !kvvmiCompleted(kvvmi) { + return Nothing + } } if kvvm.Annotations[annotations.AnnVMRestartRequested] == "true" && kvvmi.Status.Phase == virtv1.Running { h.recordRestartEventf(ctx, s.VirtualMachine().Current(), "Restart initiated "+ "by VirtualMachineOparation for Manual runPolicy") return Restart - } else if kvvmi.Status.Phase == virtv1.Succeeded && shutdownInfo.PodCompleted { + } else if kvvmiCompleted(kvvmi) && shutdownInfo.PodCompleted { if shutdownInfo.Reason == powerstate.GuestResetReason { h.recordRestartEventf(ctx, s.VirtualMachine().Current(), "Restart initiated by inside "+ "the guest VirtualMachine for Manual runPolicy") return Restart - } else { - h.recordStopEventf(ctx, s.VirtualMachine().Current(), "Stop initiated from inside "+ - "the guest VirtualMachine") - return Stop } + + h.recordStopEventf(ctx, s.VirtualMachine().Current(), "Stop initiated from inside "+ + "the guest VirtualMachine") + return Stop } return Nothing @@ -521,3 +524,7 @@ func (h *SyncPowerStateHandler) recordRestartEventf(ctx context.Context, obj cli messageFmt, ) } + +func kvvmiCompleted(kvvmi *virtv1.VirtualMachineInstance) bool { + return kvvmi != nil && (kvvmi.Status.Phase == virtv1.Succeeded || kvvmi.Status.Phase == virtv1.Failed) +} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_power_state_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_power_state_test.go index cb58c52ea9..024b228437 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_power_state_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_power_state_test.go @@ -188,6 +188,48 @@ var _ = Describe("Test action getters for different run policy", func() { Expect(action).To(Equal(Stop)) }) + It("should return stop action on failed phase with pod completed", func() { + kvvmi.Status.Phase = virtv1.Failed + + action := handler.handleManualPolicy( + ctx, vmState, kvvm, kvvmi, true, powerstate.ShutdownInfo{PodCompleted: true}, + ) + + Expect(action).To(Equal(Stop)) + }) + + It("should return restart action on failed phase with pod completed and guest reset reason", func() { + kvvmi.Status.Phase = virtv1.Failed + shutdownInfo := powerstate.ShutdownInfo{PodCompleted: true, Reason: powerstate.GuestResetReason} + + action := handler.handleManualPolicy( + ctx, vmState, kvvm, kvvmi, true, shutdownInfo, + ) + + Expect(action).To(Equal(Restart)) + }) + + It("should return start action on failed phase when start requested", func() { + setupKVVMAnnotations(kvvm, annotations.AnnVMStartRequested) + kvvmi.Status.Phase = virtv1.Failed + + action := handler.handleManualPolicy( + ctx, vmState, kvvm, kvvmi, true, powerstate.ShutdownInfo{}, + ) + + Expect(action).To(Equal(Start)) + }) + + It("should return nothing action on failed phase without pod completed", func() { + kvvmi.Status.Phase = virtv1.Failed + + action := handler.handleManualPolicy( + ctx, vmState, kvvm, kvvmi, true, powerstate.ShutdownInfo{}, + ) + + Expect(action).To(Equal(Nothing)) + }) + It("should return restart action", func() { setupKVVMAnnotations(kvvm, annotations.AnnVMRestartRequested) kvvmi.Status.Phase = virtv1.Running diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_controller.go b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go index ebc466e99e..4f131f9e01 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_controller.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go @@ -75,18 +75,22 @@ func SetupController( internal.NewAgentHandler(), internal.NewFilesystemHandler(), internal.NewSnapshottingHandler(client), + // StatisticHandler should be executed before PodHandler. + // PodHandler needs to get pods from virtual machine status. + // If StatisticHandler is executed after PodHandler, PodHandler will always get old pods (from previous reconciling) + internal.NewStatisticHandler(client), internal.NewPodHandler(client), internal.NewSizePolicyHandler(), internal.NewNetworkInterfaceHandler(featuregates.Default()), internal.NewSyncKvvmHandler(dvcrSettings, client, recorder, featuregates.Default(), migrateVolumesService), internal.NewHotplugHandler(attachmentService), + // SyncPowerStateHandler should be executed after PodHandler, because PodHandler store SharedShutdownInfo, which is used by SyncPowerStateHandler. internal.NewSyncPowerStateHandler(client, recorder), internal.NewSyncMetadataHandler(client), internal.NewLifeCycleHandler(client, recorder), internal.NewMigratingHandler(migrateVolumesService), internal.NewFirmwareHandler(firmwareImage), internal.NewEvictHandler(), - internal.NewStatisticHandler(client), } r := NewReconciler(client, handlers...)