From 55f81d389a0c86155700266aa9a6678923733ad3 Mon Sep 17 00:00:00 2001 From: dougbtv Date: Fri, 11 Oct 2024 12:49:35 -0400 Subject: [PATCH] Assigns default=true on a multiple interface return for first interface with a gateway This is necessary especially under crio, as you may not want to present the default gateway interfaces IP address as the first CNI response, as crio utilizes that IP address as the PodIP, so you may need flexibility in expression of which interface should be presented first, vs. another interface which should be the default gateway (which we mark as default=true in the network-status) --- pkg/utils/net-attach-def.go | 35 ++++++++++++++++++-- pkg/utils/net-attach-def_test.go | 56 ++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/pkg/utils/net-attach-def.go b/pkg/utils/net-attach-def.go index a22f5579..97e6be1c 100644 --- a/pkg/utils/net-attach-def.go +++ b/pkg/utils/net-attach-def.go @@ -122,6 +122,16 @@ func GetNetworkStatus(pod *corev1.Pod) ([]v1.NetworkStatus, error) { return netStatuses, err } +// gatewayInterfaceIndex determines the index of the first interface that has a gateway +func gatewayInterfaceIndex(ips []*cni100.IPConfig) int { + for _, ipConfig := range ips { + if ipConfig.Gateway != nil && ipConfig.Interface != nil { + return *ipConfig.Interface + } + } + return -1 +} + // CreateNetworkStatuses creates an array of NetworkStatus from CNI result // Not to be confused with CreateNetworkStatus (singular) // This is the preferred method and picks up when CNI ADD results contain multiple container interfaces @@ -152,14 +162,36 @@ func CreateNetworkStatuses(r cnitypes.Result, networkName string, defaultNetwork // Same for DNS v1dns := convertDNS(result.DNS) + // Check for a gateway-associated interface, we'll use this later if we did to mark as the default. + gwInterfaceIdx := -1 + if defaultNetwork { + gwInterfaceIdx = gatewayInterfaceIndex(result.IPs) + } + // Initialize NetworkStatus for each container interface (e.g. with sandbox present) indexOfFoundPodInterface := 0 foundFirstSandboxIface := false + didSetDefault := false for i, iface := range result.Interfaces { if iface.Sandbox != "" { + isDefault := false + + // If there's a gateway listed for this interface index found in the ips, we mark that interface as default + // notably, we use the first one we find. + if defaultNetwork && i == gwInterfaceIdx && !didSetDefault { + isDefault = true + didSetDefault = true + } + + // Otherwise, if we didn't find it, we use the first sandbox interface. + if defaultNetwork && gwInterfaceIdx == -1 && !foundFirstSandboxIface { + isDefault = true + foundFirstSandboxIface = true + } + ns := &v1.NetworkStatus{ Name: networkName, - Default: defaultNetwork && !foundFirstSandboxIface, + Default: isDefault, Interface: iface.Name, Mac: iface.Mac, Mtu: iface.Mtu, @@ -172,7 +204,6 @@ func CreateNetworkStatuses(r cnitypes.Result, networkName string, defaultNetwork // Map original index to the new slice index indexMap[i] = indexOfFoundPodInterface indexOfFoundPodInterface++ - foundFirstSandboxIface = true } } diff --git a/pkg/utils/net-attach-def_test.go b/pkg/utils/net-attach-def_test.go index 0d5fa956..f4dc221e 100644 --- a/pkg/utils/net-attach-def_test.go +++ b/pkg/utils/net-attach-def_test.go @@ -101,6 +101,62 @@ var _ = Describe("Netwok Attachment Definition manipulations", func() { Expect(fakeStatus).To(Equal(getStatuses)) }) + It("matches default=true when gateway is present for an interface", func() { + // CNI result using cni100.Result format + ifone := 1 + iftwo := 3 + cniResult := &cni100.Result{ + CNIVersion: "1.0.0", + Interfaces: []*cni100.Interface{ + { + Name: "0xsomething", + Mac: "be:ee:ee:ee:ee:ef", + }, + { + Name: "eth0", + Mac: "b1:aa:aa:aa:a5:ee", + Sandbox: "/var/run/netns/cni-309e7579-1b15-f905-5150-2cd232b0dad9", + }, + { + Name: "0xotherthing", + Mac: "b0:00:00:00:00:0f", + }, + { + Name: "other-primary", + Mac: "c0:00:00:00:00:01", + Sandbox: "/var/run/netns/cni-309e7579-1b15-f905-5150-2cd232b0dad9", + }, + }, + IPs: []*cni100.IPConfig{ + { + Interface: &ifone, + Address: *EnsureCIDR("10.244.1.6/24"), + }, + { + Interface: &iftwo, + Address: *EnsureCIDR("10.20.1.3/24"), + Gateway: net.ParseIP("10.20.1.1"), + }, + }, + } + + // Call CreateNetworkStatuses with this CNI result + networkName := "test-network" + defaultNetwork := true + deviceInfo := &v1.DeviceInfo{} // mock device info if needed + + networkStatuses, err := CreateNetworkStatuses(cniResult, networkName, defaultNetwork, deviceInfo) + Expect(err).NotTo(HaveOccurred()) + Expect(networkStatuses).To(HaveLen(2)) // expecting 2 statuses for sandboxed interfaces + + // Check that the interface with the gateway is marked as default + Expect(networkStatuses[0].Interface).To(Equal("eth0")) + Expect(networkStatuses[0].Default).To(BeFalse()) + + Expect(networkStatuses[1].Interface).To(Equal("other-primary")) + Expect(networkStatuses[1].Default).To(BeTrue()) // other-primary should be default because it has a gateway + }) + Context("create network status from cni result", func() { var cniResult *cni100.Result var networkStatus *v1.NetworkStatus