Merge pull request #5368 from qinqon/kv-prevent-oom-killer-failure

virt: Delete LSP at external process killing VM

Author: trozet

go-controller/pkg/kubevirt/pod.go

@@ -470,11 +470,15 @@ func DiscoverLiveMigrationStatus(client *factory.WatchFactory, pod *corev1.Pod)
 
 	targetPod := vmPods[len(vmPods)-1]
 	livingPods := filterNotComplete(vmPods)
+
+	// If there is no living pod we should state no live migration status
+	if len(livingPods) == 0 {
+		return nil, nil
+	}
+
+	// There is a living pod but is not the target one so the migration
+	// has failed.
 	if util.PodCompleted(targetPod) {
-		// if target pod failed, then there should be only one living source pod.
-		if len(livingPods) != 1 {
-			return nil, fmt.Errorf("unexpected live migration state: should have a single living pod")
-		}
 		return &LiveMigrationStatus{
 			SourcePod: livingPods[0],
 			TargetPod: targetPod,

go-controller/pkg/kubevirt/pod_test.go

@@ -98,6 +98,11 @@ var _ = Describe("Kubevirt Pod", func() {
 				pods: []corev1.Pod{successfullyMigratedKvSourcePod, failedMigrationKvTargetPod, successfulMigrationKvTargetPod},
 			},
 		),
+		Entry("returns nil when there is all the pods are completed (not running vm after migration)",
+			testParams{
+				pods: []corev1.Pod{completedKubevirtPod(t0), completedKubevirtPod(t1), completedKubevirtPod(t3)},
+			},
+		),
 		Entry("returns Migration in progress status when 2 pods are running, target pod is not yet ready",
 			testParams{
 				pods: []corev1.Pod{runningKvSourcePod, duringMigrationKvTargetPod},
@@ -148,12 +153,6 @@ var _ = Describe("Kubevirt Pod", func() {
 				},
 			},
 		),
-		Entry("returns err when kubevirt VM has several living pods and target pod failed",
-			testParams{
-				pods:          []corev1.Pod{runningKvSourcePod, successfulMigrationKvTargetPod, anotherFailedMigrationKvTargetPod},
-				expectedError: fmt.Errorf("unexpected live migration state: should have a single living pod"),
-			},
-		),
 		Entry("returns err when kubevirt VM has several living pods",
 			testParams{
 				pods:          []corev1.Pod{runningKvSourcePod, duringMigrationKvTargetPod, yetAnotherDuringMigrationKvTargetPod},

go-controller/pkg/ovn/kubevirt_test.go

@@ -146,10 +146,6 @@ var _ = Describe("OVN Kubevirt Operations", func() {
 				addressIPv6: "fd11::3",
 			},
 		}
-		logicalSwitch                            *nbdb.LogicalSwitch
-		ovnClusterRouter                         *nbdb.LogicalRouter
-		logicalRouterPort                        *nbdb.LogicalRouterPort
-		migrationSourceLSRP, migrationTargetLSRP *nbdb.LogicalSwitchPort
 
 		lrpIP = func(network string) string {
 			return strings.Split(network, "/")[0]
@@ -497,6 +493,12 @@ var _ = Describe("OVN Kubevirt Operations", func() {
 
 	Context("during execution", func() {
 		DescribeTable("reconcile migratable vm pods", func(t testData) {
+			var (
+				logicalSwitch                            *nbdb.LogicalSwitch
+				ovnClusterRouter                         *nbdb.LogicalRouter
+				logicalRouterPort                        *nbdb.LogicalRouterPort
+				migrationSourceLSRP, migrationTargetLSRP *nbdb.LogicalSwitchPort
+			)
 
 			_, parsedClusterCIDRIPv4, err := net.ParseCIDR(clusterCIDRIPv4)
 			Expect(err).ToNot(HaveOccurred())

test/e2e/kubevirt.go

@@ -50,7 +50,7 @@ import (
 	e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper"
 	testutils "k8s.io/kubernetes/test/utils"
 	utilnet "k8s.io/utils/net"
-	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 	crclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	butaneconfig "github.com/coreos/butane/config"
@@ -794,9 +794,9 @@ var _ = Describe("Kubevirt Virtual Machines", feature.VirtualMachineSupport, fun
 			}).WithPolling(time.Second).WithTimeout(time.Minute).Should(Succeed())
 		}
 
-		waitVirtualMachineInstanceReadiness = func(vmi *kubevirtv1.VirtualMachineInstance) {
+		waitVirtualMachineInstanceReadinessWith = func(vmi *kubevirtv1.VirtualMachineInstance, conditionStatus corev1.ConditionStatus) {
 			GinkgoHelper()
-			By(fmt.Sprintf("Waiting for readiness at virtual machine %s", vmi.Name))
+			By(fmt.Sprintf("Waiting for readiness=%q at virtual machine %s", conditionStatus, vmi.Name))
 			Eventually(func() []kubevirtv1.VirtualMachineInstanceCondition {
 				err := crClient.Get(context.Background(), crclient.ObjectKeyFromObject(vmi), vmi)
 				Expect(err).To(SatisfyAny(
@@ -807,10 +807,20 @@ var _ = Describe("Kubevirt Virtual Machines", feature.VirtualMachineSupport, fun
 			}).WithPolling(time.Second).WithTimeout(5 * time.Minute).Should(
 				ContainElement(SatisfyAll(
 					HaveField("Type", kubevirtv1.VirtualMachineInstanceReady),
-					HaveField("Status", corev1.ConditionTrue),
+					HaveField("Status", conditionStatus),
 				)))
 		}
 
+		waitVirtualMachineInstanceReadiness = func(vmi *kubevirtv1.VirtualMachineInstance) {
+			GinkgoHelper()
+			waitVirtualMachineInstanceReadinessWith(vmi, corev1.ConditionTrue)
+		}
+
+		waitVirtualMachineInstanceFailed = func(vmi *kubevirtv1.VirtualMachineInstance) {
+			GinkgoHelper()
+			waitVirtualMachineInstanceReadinessWith(vmi, corev1.ConditionFalse)
+		}
+
 		waitVirtualMachineAddresses = func(vmi *kubevirtv1.VirtualMachineInstance) []kubevirt.Address {
 			GinkgoHelper()
 			step := by(vmi.Name, "Wait for virtual machine to receive IPv4 address from DHCP")
@@ -903,7 +913,7 @@ var _ = Describe("Kubevirt Virtual Machines", feature.VirtualMachineSupport, fun
 							NetworkSource: networkSource,
 						},
 					},
-					TerminationGracePeriodSeconds: pointer.Int64(5),
+					TerminationGracePeriodSeconds: ptr.To(int64(5)),
 					Volumes: []kubevirtv1.Volume{
 						{
 							Name: "containerdisk",
@@ -929,7 +939,7 @@ var _ = Describe("Kubevirt Virtual Machines", feature.VirtualMachineSupport, fun
 					GenerateName: vmi.GenerateName,
 				},
 				Spec: kubevirtv1.VirtualMachineSpec{
-					Running: pointer.Bool(true),
+					RunStrategy: ptr.To(kubevirtv1.RunStrategyAlways),
 					Template: &kubevirtv1.VirtualMachineInstanceTemplateSpec{
 						ObjectMeta: metav1.ObjectMeta{
 							Annotations: vmi.Annotations,
@@ -1414,8 +1424,8 @@ fi
 					Name: "force-post-copy",
 				},
 				Spec: kvmigrationsv1alpha1.MigrationPolicySpec{
-					AllowPostCopy:           pointer.Bool(true),
-					CompletionTimeoutPerGiB: pointer.Int64(1),
+					AllowPostCopy:           ptr.To(true),
+					CompletionTimeoutPerGiB: ptr.To(int64(1)),
 					BandwidthPerMigration:   &bandwidthPerMigration,
 					Selectors: &kvmigrationsv1alpha1.Selectors{
 						VirtualMachineInstanceSelector: kvmigrationsv1alpha1.LabelSelector{
@@ -2219,15 +2229,20 @@ chpasswd: { expire: False }
 			networkData, err := staticIPsNetworkData(filterCIDRs(fr.ClientSet, vmiIPv4, vmiIPv6))
 			Expect(err).NotTo(HaveOccurred())
 
-			vmi := fedoraWithTestToolingVMI(nil /*labels*/, nil /*annotations*/, nil /*nodeSelector*/, kubevirtv1.NetworkSource{
+			vm := fedoraWithTestToolingVM(nil /*labels*/, nil /*annotations*/, nil /*nodeSelector*/, kubevirtv1.NetworkSource{
 				Multus: &kubevirtv1.MultusNetwork{
 					NetworkName: cudn.Name,
 				},
 			}, userData, networkData)
 			// Harcode mac address so it's the same after live migration
-			vmi.Spec.Domain.Devices.Interfaces[0].MacAddress = vmiMAC
-			createVirtualMachineInstance(vmi)
-
+			vm.Spec.Template.Spec.Domain.Devices.Interfaces[0].MacAddress = vmiMAC
+			createVirtualMachine(vm)
+			vmi := &kubevirtv1.VirtualMachineInstance{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: namespace,
+					Name:      vm.Name,
+				},
+			}
 			waitVirtualMachineInstanceReadiness(vmi)
 			Expect(crClient.Get(context.TODO(), crclient.ObjectKeyFromObject(vmi), vmi)).To(Succeed())
 
@@ -2253,11 +2268,39 @@ chpasswd: { expire: False }
 			by(vmi.Name, "Running live migration for virtual machine instance")
 			td(vmi)
 
-			step = by(vmi.Name, fmt.Sprintf("Login to virtual machine after virtual machine instance live migration"))
+			// Update vmi status after live migration
+			Expect(crClient.Get(context.Background(), crclient.ObjectKeyFromObject(vmi), vmi)).To(Succeed())
+
+			step = by(vmi.Name, "Login to virtual machine after virtual machine instance live migration")
 			Expect(kubevirt.LoginToFedora(vmi, "fedora", "fedora")).To(Succeed(), step)
 
 			step = by(vmi.Name, "Check east/west traffic after virtual machine instance live migration")
 			checkEastWestIperfTraffic(vmi, testPodsIPs, step)
+
+			By("Stop iperf3 traffic before force killing vm, so iperf3 server do not get stuck")
+			output, err = kubevirt.RunCommand(vmi, "killall iperf3", 5*time.Second)
+			Expect(err).ToNot(HaveOccurred(), output)
+
+			step = by(vmi.Name, fmt.Sprintf("Force kill qemu at node %q where VM is running on", vmi.Status.NodeName))
+			Expect(kubevirt.ForceKillVirtLauncherAtNode(infraprovider.Get(), vmi.Status.NodeName, vmi.Namespace, vmi.Name)).To(Succeed())
+
+			step = by(vmi.Name, "Waiting for failed restarted VMI to reach ready state")
+			waitVirtualMachineInstanceFailed(vmi)
+			waitVirtualMachineInstanceReadiness(vmi)
+			Expect(crClient.Get(context.TODO(), crclient.ObjectKeyFromObject(vmi), vmi)).To(Succeed())
+
+			step = by(vmi.Name, "Login to virtual machine after virtual machine instance force killed")
+			Expect(kubevirt.LoginToFedora(vmi, "fedora", "fedora")).To(Succeed(), step)
+
+			step = by(vmi.Name, "Restart iperf traffic after forcing a vm failure")
+			Expect(startEastWestIperfTraffic(vmi, testPodsIPs, step)).To(Succeed(), step)
+			checkEastWestIperfTraffic(vmi, testPodsIPs, step)
+
+			by(vmi.Name, "Running live migration after forcing vm failure")
+			td(vmi)
+
+			step = by(vmi.Name, "Check east/west traffic for failed virtual machine after live migration")
+			checkEastWestIperfTraffic(vmi, testPodsIPs, step)
 		},
 			Entry("after succeeded live migration", liveMigrateSucceed),
 			Entry("after failed live migration", liveMigrateFailed),

test/e2e/kubevirt/pod.go

@@ -1,6 +1,9 @@
 package kubevirt
 
 import (
+	"fmt"
+
+	infraapi "github.com/ovn-org/ovn-kubernetes/test/e2e/infraprovider/api"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
@@ -31,3 +34,17 @@ func GenerateFakeVirtLauncherPod(namespace, vmName string) *corev1.Pod {
 		},
 	}
 }
+
+func ForceKillVirtLauncherAtNode(p infraapi.Provider, nodeName, vmNamespace, vmName string) error {
+	// /usr/bin/virt-launcher --qemu-timeout 312s --name worker-dcf9j --uid bcf975f4-7bdd-4264-948b-b6080320e38a --namespace kv-live-migration-2575 --kubevirt-share-dir /var/run/kubevirt --ephemeral-disk-dir /var/run/kubevirt-ephemeral-disks --container-disk-dir /var/run/kubevirt/container-disks --grace-period-seconds 20 --hook-sidecars 0 --ovmf-path /usr/share/OVMF --run-as-nonroot
+	killScript := fmt.Sprintf(`
+pid=$(pgrep -f 'virt-launcher .*--name %s.*--namespace %s'|grep -v $$)
+ps aux |grep virt-launcher
+kill -9 $pid
+`, vmName, vmNamespace)
+	output, err := p.ExecK8NodeCommand(nodeName, []string{"bash", "-xe", "-c", killScript})
+	if err != nil {
+		return fmt.Errorf("%s:%w", output, err)
+	}
+	return nil
+}