From fafab3e81fbce2533669cc7ce7c38981712831de Mon Sep 17 00:00:00 2001 From: Ram Lavi Date: Tue, 19 Nov 2024 15:13:58 +0200 Subject: [PATCH 1/2] e2e, persistent-ips: Get primary UDN ips from vmi status Due to improvements in kubevirt/kubevirt the interface on vmi.status [0] the vmi interface is now published on the vmi.status. This allows retrieving the IPs of the primary UDN interface via the vmi.status. Changing the primary UDN e2e function that retrieves the IPs to use the vmi.status. [0] https://github.com/kubevirt/kubevirt/pull/13018 Signed-off-by: Ram Lavi --- test/e2e/persistentips_test.go | 17 ++++---- test/env/getter.go | 77 ---------------------------------- 2 files changed, 7 insertions(+), 87 deletions(-) diff --git a/test/e2e/persistentips_test.go b/test/e2e/persistentips_test.go index 2d01109f..64ad5f28 100644 --- a/test/e2e/persistentips_test.go +++ b/test/e2e/persistentips_test.go @@ -36,12 +36,14 @@ import ( kubevirtv1 "kubevirt.io/api/core/v1" - testenv "github.com/kubevirt/ipam-extensions/test/env" "sigs.k8s.io/controller-runtime/pkg/client" + + testenv "github.com/kubevirt/ipam-extensions/test/env" ) const ( secondaryLogicalNetworkInterfaceName = "multus" + primaryLogicalNetworkInterfaceName = "pod" nadName = "l2-net-attach-def" ) @@ -321,7 +323,7 @@ var _ = DescribeTableSubtree("Persistent IPs", func(params testParams) { Entry("primary UDN", testParams{ role: rolePrimary, - ipsFrom: defaultNetworkStatusAnnotationIPs, + ipsFrom: primaryNetworkVMIStatusIPs, vmi: vmiWithManagedTap, }), ) @@ -346,13 +348,8 @@ func secondaryNetworkVMIStatusIPs(vmi *kubevirtv1.VirtualMachineInstance) ([]str return testenv.GetIPsFromVMIStatus(vmi, secondaryLogicalNetworkInterfaceName), nil } -func defaultNetworkStatusAnnotationIPs(vmi *kubevirtv1.VirtualMachineInstance) ([]string, error) { - defNetworkStatus, err := testenv.DefaultNetworkStatus(vmi) - if err != nil { - return nil, err - } - - return defNetworkStatus.IPs, nil +func primaryNetworkVMIStatusIPs(vmi *kubevirtv1.VirtualMachineInstance) ([]string, error) { + return testenv.GetIPsFromVMIStatus(vmi, primaryLogicalNetworkInterfaceName), nil } func vmiWithMultus(namespace string) *kubevirtv1.VirtualMachineInstance { @@ -380,7 +377,7 @@ func vmiWithMultus(namespace string) *kubevirtv1.VirtualMachineInstance { func vmiWithManagedTap(namespace string) *kubevirtv1.VirtualMachineInstance { const ( - interfaceName = "pod" + interfaceName = primaryLogicalNetworkInterfaceName cloudInitNetworkData = ` version: 2 ethernets: diff --git a/test/env/getter.go b/test/env/getter.go index 4f6f52b6..c95bde35 100644 --- a/test/env/getter.go +++ b/test/env/getter.go @@ -2,12 +2,7 @@ package env import ( "context" - "encoding/json" - "fmt" - nadv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" - - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" @@ -51,75 +46,3 @@ func GetIPsFromVMIStatus(vmi *kubevirtv1.VirtualMachineInstance, networkInterfac } return ifaceStatus.IPs } - -func virtualMachineInstancePod(vmi *kubevirtv1.VirtualMachineInstance) (*corev1.Pod, error) { - pod, err := lookupPodBySelector(vmi.Namespace, vmiLabelSelector(vmi), vmiFieldSelector(vmi)) - if err != nil { - return nil, fmt.Errorf("failed to find pod for VMI %s (%s)", vmi.Name, string(vmi.GetUID())) - } - return pod, nil -} - -func lookupPodBySelector(namespace string, labelSelector, fieldSelector map[string]string) (*corev1.Pod, error) { - pods := &corev1.PodList{} - err := Client.List( - context.Background(), - pods, - client.InNamespace(namespace), - client.MatchingLabels(labelSelector), - client.MatchingFields(fieldSelector)) - if err != nil { - return nil, err - } - - if len(pods.Items) == 0 { - return nil, fmt.Errorf("failed to lookup pod with labels %v, fields %v in namespace %s", labelSelector, fieldSelector, namespace) - } - - return &pods.Items[0], nil -} - -func vmiLabelSelector(vmi *kubevirtv1.VirtualMachineInstance) map[string]string { - return map[string]string{kubevirtv1.CreatedByLabel: string(vmi.GetUID())} -} - -func vmiFieldSelector(vmi *kubevirtv1.VirtualMachineInstance) map[string]string { - fieldSelectors := map[string]string{} - if vmi.Status.Phase == kubevirtv1.Running { - const podPhase = "status.phase" - fieldSelectors[podPhase] = string(corev1.PodRunning) - } - return fieldSelectors -} - -func parsePodNetworkStatusAnnotation(podNetStatus string) ([]nadv1.NetworkStatus, error) { - if len(podNetStatus) == 0 { - return nil, fmt.Errorf("network status annotation not found") - } - - var netStatus []nadv1.NetworkStatus - if err := json.Unmarshal([]byte(podNetStatus), &netStatus); err != nil { - return nil, err - } - - return netStatus, nil -} - -func DefaultNetworkStatus(vmi *kubevirtv1.VirtualMachineInstance) (*nadv1.NetworkStatus, error) { - virtLauncherPod, err := virtualMachineInstancePod(vmi) - if err != nil { - return nil, err - } - - netStatuses, err := parsePodNetworkStatusAnnotation(virtLauncherPod.Annotations[nadv1.NetworkStatusAnnot]) - if err != nil { - return nil, err - } - - for _, netStatus := range netStatuses { - if netStatus.Default { - return &netStatus, nil - } - } - return nil, fmt.Errorf("primary IPs not found") -} From 2bd3439965ee917e6c32e134d8e3cca149aa10b6 Mon Sep 17 00:00:00 2001 From: Ram Lavi Date: Tue, 19 Nov 2024 20:29:45 +0200 Subject: [PATCH 2/2] e2e, persistent-ips: Unify ipsFrom function for primary/secondary UDNs Now that both secondary/primary UDNs get the IPs from the vmi.status, it possible to remove the ipsFrom helper function, and instead use the interface name as a parameter of the test. Doing so unifies and simplifies the e2e tests for both secondary and primary UDNs. Signed-off-by: Ram Lavi --- test/e2e/persistentips_test.go | 48 +++++++++++++--------------------- test/env/matcher.go | 24 +++++++++-------- 2 files changed, 31 insertions(+), 41 deletions(-) diff --git a/test/e2e/persistentips_test.go b/test/e2e/persistentips_test.go index 64ad5f28..ce4ca4ef 100644 --- a/test/e2e/persistentips_test.go +++ b/test/e2e/persistentips_test.go @@ -53,9 +53,9 @@ const ( ) type testParams struct { - role string - ipsFrom func(vmi *kubevirtv1.VirtualMachineInstance) ([]string, error) - vmi func(namespace string) *kubevirtv1.VirtualMachineInstance + role string + networkInterfaceName string + vmi func(namespace string) *kubevirtv1.VirtualMachineInstance } var _ = DescribeTableSubtree("Persistent IPs", func(params testParams) { @@ -112,13 +112,12 @@ var _ = DescribeTableSubtree("Persistent IPs", func(params testParams) { WithPolling(time.Second). ShouldNot(BeEmpty()) - Expect(testenv.ThisVMI(vmi)()).Should(testenv.MatchIPs(params.ipsFrom, Not(BeEmpty()))) + Expect(testenv.ThisVMI(vmi)()).Should(testenv.MatchIPsAtInterfaceByName(params.networkInterfaceName, Not(BeEmpty()))) }) It("should keep ips after live migration", func() { Expect(testenv.Client.Get(context.Background(), client.ObjectKeyFromObject(vmi), vmi)).To(Succeed()) - vmiIPsBeforeMigration, err := params.ipsFrom(vmi) - Expect(err).NotTo(HaveOccurred()) + vmiIPsBeforeMigration := testenv.GetIPsFromVMIStatus(vmi, params.networkInterfaceName) Expect(vmiIPsBeforeMigration).NotTo(BeEmpty()) testenv.LiveMigrateVirtualMachine(td.Namespace, vm.Name) @@ -130,7 +129,7 @@ var _ = DescribeTableSubtree("Persistent IPs", func(params testParams) { WithTimeout(5 * time.Minute). Should(testenv.ContainConditionVMIReady()) - Expect(testenv.ThisVMI(vmi)()).Should(testenv.MatchIPs(params.ipsFrom, ConsistOf(vmiIPsBeforeMigration))) + Expect(testenv.ThisVMI(vmi)()).Should(testenv.MatchIPsAtInterfaceByName(params.networkInterfaceName, ConsistOf(vmiIPsBeforeMigration))) }) It("should garbage collect IPAMClaims after VM deletion", func() { @@ -189,8 +188,7 @@ var _ = DescribeTableSubtree("Persistent IPs", func(params testParams) { It("should keep ips after restart", func() { Expect(testenv.Client.Get(context.Background(), client.ObjectKeyFromObject(vmi), vmi)).To(Succeed()) - vmiIPsBeforeRestart, err := params.ipsFrom(vmi) - Expect(err).NotTo(HaveOccurred()) + vmiIPsBeforeRestart := testenv.GetIPsFromVMIStatus(vmi, params.networkInterfaceName) Expect(vmiIPsBeforeRestart).NotTo(BeEmpty()) vmiUUIDBeforeRestart := vmi.UID @@ -210,7 +208,7 @@ var _ = DescribeTableSubtree("Persistent IPs", func(params testParams) { WithTimeout(5 * time.Minute). Should(testenv.ContainConditionVMIReady()) - Expect(testenv.ThisVMI(vmi)()).Should(testenv.MatchIPs(params.ipsFrom, ConsistOf(vmiIPsBeforeRestart))) + Expect(testenv.ThisVMI(vmi)()).Should(testenv.MatchIPsAtInterfaceByName(params.networkInterfaceName, ConsistOf(vmiIPsBeforeRestart))) }) }) @@ -237,8 +235,7 @@ var _ = DescribeTableSubtree("Persistent IPs", func(params testParams) { ShouldNot(BeEmpty()) Expect(testenv.Client.Get(context.Background(), client.ObjectKeyFromObject(vmi), vmi)).To(Succeed()) - ips, err := params.ipsFrom(vmi) - Expect(err).NotTo(HaveOccurred()) + ips := testenv.GetIPsFromVMIStatus(vmi, params.networkInterfaceName) Expect(ips).NotTo(BeEmpty()) }) @@ -280,19 +277,18 @@ var _ = DescribeTableSubtree("Persistent IPs", func(params testParams) { WithPolling(time.Second). ShouldNot(BeEmpty()) - Expect(testenv.ThisVMI(vmi)()).Should(testenv.MatchIPs(params.ipsFrom, Not(BeEmpty()))) + Expect(testenv.ThisVMI(vmi)()).Should(testenv.MatchIPsAtInterfaceByName(params.networkInterfaceName, Not(BeEmpty()))) }) It("should keep ips after live migration", func() { Expect(testenv.Client.Get(context.Background(), client.ObjectKeyFromObject(vmi), vmi)).To(Succeed()) - vmiIPsBeforeMigration, err := params.ipsFrom(vmi) - Expect(err).NotTo(HaveOccurred()) + vmiIPsBeforeMigration := testenv.GetIPsFromVMIStatus(vmi, params.networkInterfaceName) Expect(vmiIPsBeforeMigration).NotTo(BeEmpty()) testenv.LiveMigrateVirtualMachine(td.Namespace, vmi.Name) testenv.CheckLiveMigrationSucceeded(td.Namespace, vmi.Name) - Expect(testenv.ThisVMI(vmi)()).Should(testenv.MatchIPs(params.ipsFrom, ConsistOf(vmiIPsBeforeMigration))) + Expect(testenv.ThisVMI(vmi)()).Should(testenv.MatchIPsAtInterfaceByName(params.networkInterfaceName, ConsistOf(vmiIPsBeforeMigration))) }) It("should garbage collect IPAMClaims after VMI deletion", func() { @@ -316,15 +312,15 @@ var _ = DescribeTableSubtree("Persistent IPs", func(params testParams) { }, Entry("secondary interfaces", testParams{ - role: roleSecondary, - ipsFrom: secondaryNetworkVMIStatusIPs, - vmi: vmiWithMultus, + role: roleSecondary, + networkInterfaceName: secondaryLogicalNetworkInterfaceName, + vmi: vmiWithMultus, }), Entry("primary UDN", testParams{ - role: rolePrimary, - ipsFrom: primaryNetworkVMIStatusIPs, - vmi: vmiWithManagedTap, + role: rolePrimary, + networkInterfaceName: primaryLogicalNetworkInterfaceName, + vmi: vmiWithManagedTap, }), ) @@ -344,14 +340,6 @@ func removeFinalizersPatch() ([]byte, error) { return json.Marshal(patch) } -func secondaryNetworkVMIStatusIPs(vmi *kubevirtv1.VirtualMachineInstance) ([]string, error) { - return testenv.GetIPsFromVMIStatus(vmi, secondaryLogicalNetworkInterfaceName), nil -} - -func primaryNetworkVMIStatusIPs(vmi *kubevirtv1.VirtualMachineInstance) ([]string, error) { - return testenv.GetIPsFromVMIStatus(vmi, primaryLogicalNetworkInterfaceName), nil -} - func vmiWithMultus(namespace string) *kubevirtv1.VirtualMachineInstance { interfaceName := secondaryLogicalNetworkInterfaceName return testenv.NewVirtualMachineInstance( diff --git a/test/env/matcher.go b/test/env/matcher.go index c7654b54..28fd0525 100644 --- a/test/env/matcher.go +++ b/test/env/matcher.go @@ -32,19 +32,21 @@ func vmiStatusConditions(vmi *kubevirtv1.VirtualMachineInstance) []kubevirtv1.Vi return vmi.Status.Conditions } -type IPResult struct { - IPs []string - Err error +func interfaceIPs(networkInterface *kubevirtv1.VirtualMachineInstanceNetworkInterface) []string { + if networkInterface == nil { + return nil + } + return networkInterface.IPs } -func MatchIPs(getIPsFunc func(vmi *kubevirtv1.VirtualMachineInstance) ([]string, error), ipsMatcher gomegatypes.GomegaMatcher) gomegatypes.GomegaMatcher { - return WithTransform(func(vmi *kubevirtv1.VirtualMachineInstance) IPResult { - ips, err := getIPsFunc(vmi) - return IPResult{IPs: ips, Err: err} - }, SatisfyAll( - WithTransform(func(result IPResult) error { return result.Err }, Succeed()), - WithTransform(func(result IPResult) []string { return result.IPs }, ipsMatcher), - )) +func MatchIPsAtInterfaceByName(interfaceName string, ipsMatcher gomegatypes.GomegaMatcher) gomegatypes.GomegaMatcher { + return WithTransform( + func(vmi *kubevirtv1.VirtualMachineInstance) *kubevirtv1.VirtualMachineInstanceNetworkInterface { + return lookupInterfaceStatusByName(vmi.Status.Interfaces, interfaceName) + }, + SatisfyAll( + Not(BeNil()), + WithTransform(interfaceIPs, ipsMatcher))) } func BeRestarted(oldUID types.UID) gomegatypes.GomegaMatcher {