virt: Delete LSP at external process killing VM

When something like OOM killer kill the running virt-launcher pod code
enter on [1] this prevent ovnk from deleting the LSP so when VM is
restarted the traffic is blocked.

This change just return nil virtual machine status, since there is no
live migration going on.

[1] https://github.com/openshift/ovn-kubernetes/blob/release-4.18/go-controller/pkg/kubevirt/pod.go#L475

Signed-off-by: Enrique Llorente <[email protected]>

Author: qinqon

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(selectCIDRs(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
+}