Merge pull request #5229 from tssurya/revert-localnet-changes

Revert Localnet changes

Author: tssurya

go-controller/pkg/node/gateway_shared_intf.go

@@ -1952,10 +1952,9 @@ func commonFlows(hostSubnets []*net.IPNet, bridge *bridgeConfiguration) ([]strin
 							defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, config.Default.ConntrackZone,
 							netConfig.masqCTMark, ofPortPhys))
 
-					// Allow (a) OVN->host traffic on the same node
-					// (b) host->host traffic on the same node
+					// Allow OVN->Host traffic on the same node
 					if config.Gateway.Mode == config.GatewayModeShared || config.Gateway.Mode == config.GatewayModeLocal {
-						dftFlows = append(dftFlows, hostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, false)...)
+						dftFlows = append(dftFlows, ovnToHostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, false)...)
 					}
 				} else {
 					//  for UDN we additionally SNAT the packet from masquerade IP -> node IP
@@ -2049,10 +2048,9 @@ func commonFlows(hostSubnets []*net.IPNet, bridge *bridgeConfiguration) ([]strin
 							"actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:%s",
 							defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, config.Default.ConntrackZone, netConfig.masqCTMark, ofPortPhys))
 
-					// Allow (a) OVN->host traffic on the same node
-					// (b) host->host traffic on the same node
+					// Allow OVN->Host traffic on the same node
 					if config.Gateway.Mode == config.GatewayModeShared || config.Gateway.Mode == config.GatewayModeLocal {
-						dftFlows = append(dftFlows, hostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, true)...)
+						dftFlows = append(dftFlows, ovnToHostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, true)...)
 					}
 				} else {
 					//  for UDN we additionally SNAT the packet from masquerade IP -> node IP
@@ -2247,15 +2245,23 @@ func pmtudDropFlows(bridge *bridgeConfiguration, ipAddrs []string) []string {
 	return flows
 }
 
-// hostNetworkNormalActionFlows returns the flows that allow IP{v4,v6} traffic:
-// a. from pods in the OVN network to pods in a localnet network, on the same node
-// b. from pods on the host to pods in a localnet network, on the same node
-// when the localnet is mapped to breth0.
-// The expected srcMAC is the MAC address of breth0 and the expected hostSubnets is the host subnets found on the node
-// primary interface.
-func hostNetworkNormalActionFlows(netConfig *bridgeUDNConfiguration, srcMAC string, hostSubnets []*net.IPNet, isV6 bool) []string {
+// ovnToHostNetworkNormalActionFlows returns the flows that allow IP{v4,v6} traffic from the OVN network to the host network
+// when the destination is on the same node as the sender. This is necessary for pods in the default network to reach
+// localnet pods on the same node, when the localnet is mapped to breth0. The expected srcMAC is the MAC address of breth0
+// and the expected hostSubnets is the host subnets found on the node primary interface.
+func ovnToHostNetworkNormalActionFlows(netConfig *bridgeUDNConfiguration, srcMAC string, hostSubnets []*net.IPNet, isV6 bool) []string {
+	var inPort, ctMark, ipFamily, ipFamilyDest string
 	var flows []string
-	var ipFamily, ipFamilyDest string
+
+	if config.Gateway.Mode == config.GatewayModeShared {
+		inPort = netConfig.ofPortPatch
+		ctMark = netConfig.masqCTMark
+	} else if config.Gateway.Mode == config.GatewayModeLocal {
+		inPort = "LOCAL"
+		ctMark = ctMarkHost
+	} else {
+		return nil
+	}
 
 	if isV6 {
 		ipFamily = "ipv6"
@@ -2265,69 +2271,38 @@ func hostNetworkNormalActionFlows(netConfig *bridgeUDNConfiguration, srcMAC stri
 		ipFamilyDest = "nw_dst"
 	}
 
-	formatFlow := func(inPort, destIP, ctMark string) string {
-		// Matching IP traffic will be handled by the bridge instead of being output directly
-		// to the NIC by the existing flow at prio=100.
-		flowTemplate := "cookie=%s, priority=102, in_port=%s, dl_src=%s, %s, %s=%s, " +
-			"actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:NORMAL"
-		return fmt.Sprintf(flowTemplate,
-			defaultOpenFlowCookie,
-			inPort,
-			srcMAC,
-			ipFamily,
-			ipFamilyDest,
-			destIP,
-			config.Default.ConntrackZone,
-			ctMark)
-	}
-
-	// Traffic path (a): OVN->localnet for shared gw mode
-	if config.Gateway.Mode == config.GatewayModeShared {
-		for _, hostSubnet := range hostSubnets {
-			if utilnet.IsIPv6(hostSubnet.IP) != isV6 {
-				continue
-			}
-			flows = append(flows, formatFlow(netConfig.ofPortPatch, hostSubnet.String(), netConfig.masqCTMark))
-		}
-	}
-
-	// Traffic path (a): OVN->localnet for local gw mode
-	// Traffic path (b): host->localnet for both gw modes
 	for _, hostSubnet := range hostSubnets {
-		if utilnet.IsIPv6(hostSubnet.IP) != isV6 {
+		if (hostSubnet.IP.To4() == nil) != isV6 {
 			continue
 		}
-		flows = append(flows, formatFlow("LOCAL", hostSubnet.String(), ctMarkHost))
-	}
-
-	if isV6 {
-		// IPv6 neighbor discovery uses ICMPv6 messages sent to a special destination (ff02::1:ff00:0/104)
-		// that is unrelated to the host subnets matched in the prio=102 flow above.
-		// Allow neighbor discovery by matching against ICMP type and ingress port.
-		formatICMPFlow := func(inPort, ctMark string, icmpType int) string {
-			icmpFlowTemplate := "cookie=%s, priority=102, in_port=%s, dl_src=%s, icmp6, icmpv6_type=%d, " +
-				"actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:NORMAL"
-			return fmt.Sprintf(icmpFlowTemplate,
+		// IP traffic from the OVN network to the host network should be handled normally by the bridge instead of
+		// being output directly to the NIC by the existing flow at prio=100.
+		flows = append(flows,
+			fmt.Sprintf("cookie=%s, priority=102, in_port=%s, dl_src=%s, %s, %s=%s, "+
+				"actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:NORMAL",
 				defaultOpenFlowCookie,
 				inPort,
 				srcMAC,
-				icmpType,
+				ipFamily,
+				ipFamilyDest,
+				hostSubnet.String(),
 				config.Default.ConntrackZone,
-				ctMark)
-		}
+				ctMark))
+	}
 
+	if isV6 {
+		// Neighbor discovery in IPv6 happens through ICMPv6 messages to a special destination (ff02::1:ff00:0/104),
+		// which has nothing to do with the host subnets we're matching against in the flow above at prio=102.
+		// Let's allow neighbor discovery by matching against icmp type and in_port.
 		for _, icmpType := range []int{types.NeighborSolicitationICMPType, types.NeighborAdvertisementICMPType} {
-			// Traffic path (a) for ICMP: OVN-> localnet for shared gw mode
-			if config.Gateway.Mode == config.GatewayModeShared {
-				flows = append(flows,
-					formatICMPFlow(netConfig.ofPortPatch, netConfig.masqCTMark, icmpType))
-			}
-
-			// Traffic path (a) for ICMP: OVN->localnet for local gw mode
-			// Traffic path (b) for ICMP: host->localnet for both gw modes
-			flows = append(flows, formatICMPFlow("LOCAL", ctMarkHost, icmpType))
+			flows = append(flows,
+				fmt.Sprintf("cookie=%s, priority=102, in_port=%s, dl_src=%s, icmp6, icmpv6_type=%d, "+
+					"actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:NORMAL",
+					defaultOpenFlowCookie, inPort, srcMAC, icmpType,
+					config.Default.ConntrackZone, ctMark))
 		}
 	}
+
 	return flows
 }
 

test/e2e/multihoming.go

@@ -331,31 +331,17 @@ var _ = Describe("Multi Homing", func() {
 				kickstartPod(cs, clientPodConfig)
 
 				// Check that the client pod can reach the server pod on the server localnet interface
-				var serverIPs []string
-				if serverPodConfig.hostNetwork {
-					serverIPs, err = podIPsFromStatus(cs, serverPodConfig.namespace, serverPodConfig.name)
-				} else {
-					serverIPs, err = podIPsForAttachment(cs, serverPod.Namespace, serverPod.Name, netConfig.name)
-
-				}
+				serverIPs, err := podIPsForAttachment(cs, f.Namespace.Name, serverPod.GetName(), netConfig.name)
 				Expect(err).NotTo(HaveOccurred())
-
 				for _, serverIP := range serverIPs {
 					By(fmt.Sprintf("asserting the *client* can contact the server pod exposed endpoint: %q on port %q", serverIP, port))
-					curlArgs := []string{}
-					pingArgs := []string{}
-					if clientPodConfig.attachments != nil {
-						// When the client is attached to a localnet, send probes from the localnet interface
-						curlArgs = []string{"--interface", "net1"}
-						pingArgs = []string{"-I", "net1"}
-					}
 					Eventually(func() error {
-						return reachServerPodFromClient(cs, serverPodConfig, clientPodConfig, serverIP, port, curlArgs...)
+						return reachServerPodFromClient(cs, serverPodConfig, clientPodConfig, serverIP, port)
 					}, 2*time.Minute, 6*time.Second).Should(Succeed())
 
 					By(fmt.Sprintf("asserting the *client* can ping the server pod exposed endpoint: %q", serverIP))
 					Eventually(func() error {
-						return pingServerPodFromClient(cs, serverPodConfig, clientPodConfig, serverIP, pingArgs...)
+						return pingServerPodFromClient(cs, serverPodConfig, clientPodConfig, serverIP)
 					}, 2*time.Minute, 6*time.Second).Should(Succeed())
 				}
 			},
@@ -403,52 +389,6 @@ var _ = Describe("Multi Homing", func() {
 				},
 				Label("BUG", "OCPBUGS-43004"),
 			),
-			ginkgo.Entry(
-				"can reach a host-networked pod on a different node",
-				networkAttachmentConfigParams{
-					name:     secondaryNetworkName,
-					topology: "localnet",
-				},
-				podConfiguration{ // client on localnet
-					attachments: []nadapi.NetworkSelectionElement{{
-						Name: secondaryNetworkName,
-					}},
-					name:                         clientPodName,
-					nodeSelector:                 map[string]string{nodeHostnameKey: workerOneNodeName},
-					isPrivileged:                 true,
-					needsIPRequestFromHostSubnet: true,
-				},
-				podConfiguration{ // server on default network, pod is host-networked
-					name:         podName,
-					containerCmd: httpServerContainerCmd(port),
-					nodeSelector: map[string]string{nodeHostnameKey: workerTwoNodeName},
-					hostNetwork:  true,
-				},
-				Label("STORY", "SDN-5345"),
-			),
-			ginkgo.Entry(
-				"can reach a host-networked pod on the same node",
-				networkAttachmentConfigParams{
-					name:     secondaryNetworkName,
-					topology: "localnet",
-				},
-				podConfiguration{ // client on localnet
-					attachments: []nadapi.NetworkSelectionElement{{
-						Name: secondaryNetworkName,
-					}},
-					name:                         clientPodName,
-					nodeSelector:                 map[string]string{nodeHostnameKey: workerTwoNodeName},
-					isPrivileged:                 true,
-					needsIPRequestFromHostSubnet: true,
-				},
-				podConfiguration{ // server on default network, pod is host-networked
-					name:         podName,
-					containerCmd: httpServerContainerCmd(port),
-					nodeSelector: map[string]string{nodeHostnameKey: workerTwoNodeName},
-					hostNetwork:  true,
-				},
-				Label("STORY", "SDN-5345"),
-			),
 		)
 	})
 
@@ -908,6 +848,7 @@ var _ = Describe("Multi Homing", func() {
 		Context("localnet OVN-K secondary network", func() {
 			const (
 				clientPodName          = "client-pod"
+				nodeHostnameKey        = "kubernetes.io/hostname"
 				servicePort            = 9000
 				dockerNetworkName      = "underlay"
 				underlayServiceIP      = "60.128.0.1"

test/e2e/multihoming_utils.go

@@ -161,7 +161,6 @@ type podConfiguration struct {
 	isPrivileged                 bool
 	labels                       map[string]string
 	requiresExtraNamespace       bool
-	hostNetwork                  bool
 	needsIPRequestFromHostSubnet bool
 }
 
@@ -172,7 +171,6 @@ func generatePodSpec(config podConfiguration) *v1.Pod {
 	}
 	podSpec.Spec.NodeSelector = config.nodeSelector
 	podSpec.Labels = config.labels
-	podSpec.Spec.HostNetwork = config.hostNetwork
 	if config.isPrivileged {
 		podSpec.Spec.Containers[0].SecurityContext.Privileged = ptr.To(true)
 	} else {
@@ -255,19 +253,17 @@ func inRange(cidr string, ip string) error {
 	return fmt.Errorf("ip [%s] is NOT in range %s", ip, cidr)
 }
 
-func connectToServer(clientPodConfig podConfiguration, serverIP string, port int, args ...string) error {
-	target := net.JoinHostPort(serverIP, fmt.Sprintf("%d", port))
-	baseArgs := []string{
+func connectToServer(clientPodConfig podConfiguration, serverIP string, port int) error {
+	_, err := e2ekubectl.RunKubectl(
+		clientPodConfig.namespace,
 		"exec",
 		clientPodConfig.name,
 		"--",
 		"curl",
 		"--connect-timeout",
 		"2",
-	}
-	baseArgs = append(baseArgs, args...)
-
-	_, err := e2ekubectl.RunKubectl(clientPodConfig.namespace, append(baseArgs, target)...)
+		net.JoinHostPort(serverIP, fmt.Sprintf("%d", port)),
+	)
 	return err
 }
 
@@ -312,19 +308,16 @@ func getSecondaryInterfaceMTU(clientPodConfig podConfiguration) (int, error) {
 	return mtu, nil
 }
 
-func pingServer(clientPodConfig podConfiguration, serverIP string, args ...string) error {
-	baseArgs := []string{
+func pingServer(clientPodConfig podConfiguration, serverIP string) error {
+	_, err := e2ekubectl.RunKubectl(
+		clientPodConfig.namespace,
 		"exec",
 		clientPodConfig.name,
 		"--",
 		"ping",
 		"-c", "1", // send one ICMP echo request
 		"-W", "2", // timeout after 2 seconds if no response
-	}
-	baseArgs = append(baseArgs, args...)
-
-	_, err := e2ekubectl.RunKubectl(clientPodConfig.namespace, append(baseArgs, serverIP)...)
-
+		serverIP)
 	return err
 }
 
@@ -388,18 +381,6 @@ func podIPForAttachment(k8sClient clientset.Interface, podNamespace string, podN
 	return ips[ipIndex], nil
 }
 
-func podIPsFromStatus(k8sClient clientset.Interface, podNamespace string, podName string) ([]string, error) {
-	pod, err := k8sClient.CoreV1().Pods(podNamespace).Get(context.Background(), podName, metav1.GetOptions{})
-	if err != nil {
-		return nil, err
-	}
-	podIPs := make([]string, 0, len(pod.Status.PodIPs))
-	for _, podIP := range pod.Status.PodIPs {
-		podIPs = append(podIPs, podIP.IP)
-	}
-	return podIPs, nil
-}
-
 func allowedClient(podName string) string {
 	return "allowed-" + podName
 }
@@ -629,27 +610,27 @@ func allowedTCPPortsForPolicy(allowPorts ...int) []mnpapi.MultiNetworkPolicyPort
 	return portAllowlist
 }
 
-func reachServerPodFromClient(cs clientset.Interface, serverConfig podConfiguration, clientConfig podConfiguration, serverIP string, serverPort int, args ...string) error {
+func reachServerPodFromClient(cs clientset.Interface, serverConfig podConfiguration, clientConfig podConfiguration, serverIP string, serverPort int) error {
 	updatedPod, err := cs.CoreV1().Pods(serverConfig.namespace).Get(context.Background(), serverConfig.name, metav1.GetOptions{})
 	if err != nil {
 		return err
 	}
 
 	if updatedPod.Status.Phase == v1.PodRunning {
-		return connectToServer(clientConfig, serverIP, serverPort, args...)
+		return connectToServer(clientConfig, serverIP, serverPort)
 	}
 
 	return fmt.Errorf("pod not running. /me is sad")
 }
 
-func pingServerPodFromClient(cs clientset.Interface, serverConfig podConfiguration, clientConfig podConfiguration, serverIP string, args ...string) error {
+func pingServerPodFromClient(cs clientset.Interface, serverConfig podConfiguration, clientConfig podConfiguration, serverIP string) error {
 	updatedPod, err := cs.CoreV1().Pods(serverConfig.namespace).Get(context.Background(), serverConfig.name, metav1.GetOptions{})
 	if err != nil {
 		return err
 	}
 
 	if updatedPod.Status.Phase == v1.PodRunning {
-		return pingServer(clientConfig, serverIP, args...)
+		return pingServer(clientConfig, serverIP)
 	}
 
 	return fmt.Errorf("pod not running. /me is sad")