test: Refactor IP{Add,Del}Route helpers

- Invoke cmds for a given node instead of returning cmd itself - reduces boilerplate by quite a bit. - Changes method names form to verb + noun. Suggested-by: Chris Tarazi <[email protected]> Signed-off-by: Martynas Pumputis <[email protected]>
kh34 · Aug 18, 2020 · a7c2130 · a7c2130
1 parent 03ef3e4
commit a7c2130
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 65 deletions.
diff --git a/test/helpers/kubectl.go b/test/helpers/kubectl.go
@@ -4099,3 +4099,26 @@ func (kub *Kubectl) GetDNSProxyPort(ciliumPod string) int {
 	}
 	return port
 }
+
+// AddIPRoute adds a route to a given subnet address and a gateway on a given
+// node via the iproute2 utility suite. The function takes in a flag called
+// replace which will convert the action to replace the  route being added if
+// another route exists and matches. This allows for idempotency as the "replace"
+// action will not fail if another matching route exists, whereas "add" will fail.
+func (kub *Kubectl) AddIPRoute(nodeName, subnet, gw string, replace bool) *CmdRes {
+	action := "add"
+	if replace {
+		action = "replace"
+	}
+	cmd := fmt.Sprintf("ip route %s %s via %s", action, subnet, gw)
+
+	return kub.ExecInHostNetNS(context.TODO(), nodeName, cmd)
+}
+
+// DelIPRoute deletes a route to a given IP address and a gateway on a given
+// node via the iproute2 utility suite.
+func (kub *Kubectl) DelIPRoute(nodeName, subnet, gw string) *CmdRes {
+	cmd := fmt.Sprintf("ip route del %s via %s", subnet, gw)
+
+	return kub.ExecInHostNetNS(context.TODO(), nodeName, cmd)
+}
diff --git a/test/helpers/wrappers.go b/test/helpers/wrappers.go
@@ -16,7 +16,6 @@ package helpers
 
 import (
 	"fmt"
-	"net"
 	"strings"
 )
 
@@ -163,24 +162,3 @@ func PythonBind(addr string, port uint16, proto string) string {
 		`/usr/bin/python3 -c 'import socket; socket.socket(%s).bind((%q, %d))`,
 		strings.Join(opts, ", "), addr, port)
 }
-
-// IPAddRoute returns a string representing the command to add a route to a
-// given IP address and a gateway via the iproute2 utility suite. The function
-// takes in a flag called replace which will convert the action to replace the
-// route being added if another route exists and matches. This allows for
-// idempotency as the "replace" action will not fail if another matching route
-// exists, whereas "add" will fail.
-func IPAddRoute(ip, gw net.IP, replace bool) string {
-	action := "add"
-	if replace {
-		action = "replace"
-	}
-
-	return fmt.Sprintf("ip route %s %s via %s", action, ip.String(), gw.String())
-}
-
-// IPDelRoute returns a string representing the command to delete a route to a
-// given IP address and a gateway via the iproute2 utility suite.
-func IPDelRoute(ip, gw net.IP) string {
-	return fmt.Sprintf("ip route del %s via %s", ip.String(), gw.String())
-}
diff --git a/test/k8sT/DatapathConfiguration.go b/test/k8sT/DatapathConfiguration.go
@@ -751,20 +751,16 @@ func testPodHTTPToOutside(kubectl *helpers.Kubectl, outsideURL string, expectNod
 	for _, pod := range pods {
 		By("Making ten curl requests from %q to %q", pod, outsideURL)
 
-		hostIP := net.ParseIP(hostIPs[pod])
-		podIP := net.ParseIP(podIPs[pod])
+		hostIP := hostIPs[pod]
+		podIP := podIPs[pod]
 
 		if expectPodIP {
 			// Make pods reachable from the host which doesn't run Cilium
-			_, err := kubectl.ExecInHostNetNSByLabel(context.TODO(),
-				helpers.GetNodeWithoutCilium(),
-				helpers.IPAddRoute(podIP, hostIP, false))
-			ExpectWithOffset(1, err).Should(BeNil(), "Failed to add ip route")
+			kubectl.AddIPRoute(helpers.GetNodeWithoutCilium(), podIP, hostIP, false).
+				ExpectSuccess("Failed to add ip route")
 			defer func() {
-				_, err := kubectl.ExecInHostNetNSByLabel(context.TODO(),
-					helpers.GetNodeWithoutCilium(),
-					helpers.IPDelRoute(podIP, hostIP))
-				ExpectWithOffset(1, err).Should(BeNil(), "Failed to del ip route")
+				kubectl.DelIPRoute(helpers.GetNodeWithoutCilium(), podIP, hostIP).
+					ExpectSuccess("Failed to del ip route")
 			}()
 		}
 
@@ -775,7 +771,8 @@ func testPodHTTPToOutside(kubectl *helpers.Kubectl, outsideURL string, expectNod
 
 			if expectNodeIP || expectPodIP {
 				// Parse the IPs to avoid issues with 4-in-6 formats
-				sourceIP := net.ParseIP(strings.TrimSpace(strings.Split(res.Stdout(), "=")[1]))
+				sourceIP := net.ParseIP(strings.TrimSpace(
+					strings.Split(res.Stdout(), "=")[1])).String()
 				if expectNodeIP {
 					Expect(sourceIP).To(Equal(hostIP), "Expected node IP")
 				}

diff --git a/test/k8sT/Policies.go b/test/k8sT/Policies.go
@@ -17,7 +17,6 @@ package k8sTest
 import (
 	"context"
 	"fmt"
-	"net"
 	"regexp"
 	"time"
 
@@ -1123,10 +1122,10 @@ var _ = Describe("K8sPolicyTest", func() {
 				outsideNodeName, outsideIP string // k8s3 node (doesn't have agent running)
 
 				backendPod   v1.Pod // The pod that k8s3 node is hitting
-				backendPodIP net.IP
+				backendPodIP string
 
 				hostNodeName       string // Node that backendPod ends up on
-				hostIPOfBackendPod net.IP
+				hostIPOfBackendPod string
 
 				policyVerdictAllowRegex, policyVerdictDenyRegex *regexp.Regexp
 			)
@@ -1155,8 +1154,8 @@ var _ = Describe("K8sPolicyTest", func() {
 				Expect(demoPods.Items).To(HaveLen(2))
 
 				backendPod = demoPods.Items[0] // We'll take the first one; doesn't matter
-				backendPodIP = net.ParseIP(backendPod.Status.PodIP)
-				hostIPOfBackendPod = net.ParseIP(backendPod.Status.HostIP)
+				backendPodIP = backendPod.Status.PodIP
+				hostIPOfBackendPod = backendPod.Status.HostIP
 				hostNodeName = backendPod.Spec.NodeName // Save the name of node backend pod is on
 
 				By("Adding a static route to %s on the %s node (outside)",
@@ -1166,26 +1165,20 @@ var _ = Describe("K8sPolicyTest", func() {
 				// K8s Services, for the sake of simplicity. Making the backend
 				// pod IP directly routable on the "outside" node is sufficient
 				// to validate the policy under test.
-				res, err := kubectl.ExecInHostNetNS(context.TODO(), outsideNodeName,
-					helpers.IPAddRoute(backendPodIP, hostIPOfBackendPod, true))
+				res := kubectl.AddIPRoute(outsideNodeName, backendPodIP, hostIPOfBackendPod, true)
 				Expect(res).To(getMatcher(true))
-				Expect(err).ToNot(HaveOccurred(),
-					"Cannot exec in outside node %s", outsideNodeName)
 
 				policyVerdictAllowRegex = regexp.MustCompile(
 					fmt.Sprintf("Policy verdict log: .+action allow.+%s:[0-9]+ -> %s:80 tcp SYN",
-						outsideIP,
-						backendPodIP.String()))
+						outsideIP, backendPodIP))
 				policyVerdictDenyRegex = regexp.MustCompile(
 					fmt.Sprintf("Policy verdict log: .+action deny.+%s:[0-9]+ -> %s:80 tcp SYN",
-						outsideIP,
-						backendPodIP.String()))
+						outsideIP, backendPodIP))
 			})
 
 			AfterAll(func() {
 				// Remove the route on the outside node.
-				kubectl.ExecInHostNetNS(context.TODO(), outsideNodeName,
-					helpers.IPDelRoute(backendPodIP, hostIPOfBackendPod))
+				kubectl.DelIPRoute(outsideNodeName, backendPodIP, hostIPOfBackendPod)
 
 				// Revert Cilium installation back to before this Context.
 				By("Redeploying Cilium with default configuration")

diff --git a/test/k8sT/Services.go b/test/k8sT/Services.go
@@ -789,15 +789,11 @@ var _ = Describe("K8sServicesTest", func() {
 			tftpURL := getTFTPLink(svcExternalIP, data.Spec.Ports[1].Port)
 
 			// Add the route on the outside node to the external IP addr
-			cmd := helpers.IPAddRoute(net.ParseIP(svcExternalIP), net.ParseIP(k8s1IP), false)
-			res, err := kubectl.ExecInHostNetNS(context.TODO(), outsideNodeName, cmd)
-			ExpectWithOffset(1, err).Should(BeNil(), "Cannot exec in outside node %s", outsideNodeName)
-			ExpectWithOffset(1, res).Should(helpers.CMDSuccess(),
-				"Can not exec %q on outside node %s", cmd, outsideNodeName)
+			kubectl.AddIPRoute(outsideNodeName, svcExternalIP, k8s1IP, false).
+				ExpectSuccess("Cannot add ip route")
 			defer func() {
-				kubectl.ExecInHostNetNS(context.TODO(),
-					outsideNodeName,
-					helpers.IPDelRoute(net.ParseIP(svcExternalIP), net.ParseIP(k8s1IP)))
+				kubectl.DelIPRoute(outsideNodeName, svcExternalIP, k8s1IP).
+					ExpectSuccess("Cannot del ip route")
 			}()
 
 			// Should work from outside via the external IP
@@ -1489,15 +1485,9 @@ var _ = Describe("K8sServicesTest", func() {
 								helpers.DefaultNamespace, "test-lb", 30*time.Second)
 							Expect(err).Should(BeNil(), "Cannot retrieve loadbalancer IP for test-lb")
 							// Add route to the LB IP addr via k8s1 node
-							cmd := helpers.IPAddRoute(net.ParseIP(lbIP), net.ParseIP(k8s1IP), false)
-							res, err := kubectl.ExecInHostNetNS(context.TODO(), outsideNodeName, cmd)
-							ExpectWithOffset(1, err).Should(BeNil(), "Cannot exec in outside node %s", outsideNodeName)
-							ExpectWithOffset(1, res).Should(helpers.CMDSuccess(),
-								"Can not exec %q on outside node %s", cmd, outsideNodeName)
-							defer func() {
-								cmd := helpers.IPDelRoute(net.ParseIP(lbIP), net.ParseIP(k8s1IP))
-								kubectl.ExecInHostNetNS(context.TODO(), outsideNodeName, cmd)
-							}()
+							kubectl.AddIPRoute(outsideNodeName, lbIP, k8s1IP, false).
+								ExpectSuccess("Cannot add ip route")
+							defer func() { kubectl.DelIPRoute(outsideNodeName, lbIP, k8s1IP) }()
 							// Check connectivity from outside
 							testCurlFromOutside("http://"+lbIP, 10, false)
 						})