Merge pull request #265 from cybozu-go/avoid-adding-fou-devices-for-p…

…ods-not-using-egress

Fix to avoid adding FoU devices for pods that don't use its egress

v2/Makefile

@@ -21,7 +21,7 @@ PROTOC_OUTPUTS = pkg/cnirpc/cni.pb.go pkg/cnirpc/cni_grpc.pb.go ../docs/cni-grpc
 GOOS := $(shell go env GOOS)
 GOARCH := $(shell go env GOARCH)
 PROTOC := PATH=$(PWD)/bin:'$(PATH)' $(PWD)/bin/protoc -I=$(PWD)/include:.
-PODNSLIST = pod1 pod2 pod3
+PODNSLIST = pod1 pod2 pod3 pod4 pod5 pod6
 NATNSLIST = nat-client nat-router nat-egress nat-target
 OTHERNSLIST = test-egress-dual test-egress-v4 test-egress-v6 \
 	test-client-dual test-client-v4 test-client-v6 test-client-custom \

v2/e2e/coil_test.go

@@ -368,6 +368,10 @@ var _ = Describe("Coil", func() {
 			Expect(resp).To(HaveLen(1 << 20))
 		}
 
+		By("creating a dummy pod don't use egress")
+		// dummy pod must be created after creating a net-client pod
+		kubectlSafe(nil, "apply", "-f", "manifests/dummy_pod.yaml")
+
 		By("updating Egress in the internet namespace")
 		kubectlSafe(nil, "apply", "-f", "manifests/egress-updated.yaml")
 
@@ -446,6 +450,20 @@ var _ = Describe("Coil", func() {
 			return nil
 		}).Should(Succeed())
 
+		By("confirming that the fou device must be one in dummy_pod")
+		out, err := kubectl(nil, "exec", "dummy", "--", "ip", "-j", "link", "show")
+		Expect(err).NotTo(HaveOccurred())
+		var dummyPodLinks []link
+		err = json.Unmarshal(out, &dummyPodLinks)
+		Expect(err).NotTo(HaveOccurred())
+		fouCount := 0
+		for _, l := range dummyPodLinks {
+			if strings.HasPrefix(l.Ifname, "fou") && l.Ifname != "fou-dummy" {
+				fouCount += 1
+			}
+		}
+		Expect(fouCount).To(Equal(1))
+
 		By("sending and receiving HTTP request from nat-client")
 		data = make([]byte, 1<<20) // 1 MiB
 		resp = kubectlSafe(data, "exec", "-i", "nat-client", "--", "curl", "-sf", "-T", "-", fakeURL)

v2/e2e/manifests/dummy_pod.yaml

@@ -0,0 +1,18 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: dummy 
+  namespace: default
+  annotations:
+    egress.coil.cybozu.com/internet: egress-sport-auto
+spec:
+  tolerations:
+  - key: test
+    operator: Exists
+  nodeSelector:
+    test: coil
+    kubernetes.io/hostname: coil-worker
+  containers:
+  - name: ubuntu
+    image: quay.io/cybozu/ubuntu-debug:22.04
+    command: ["pause"]

v2/e2e/manifests/nat-client-sport-auto.yaml

@@ -11,6 +11,7 @@ spec:
     operator: Exists
   nodeSelector:
     test: coil
+    kubernetes.io/hostname: coil-worker2
   containers:
   - name: ubuntu
     image: ghcr.io/cybozu/ubuntu-debug:22.04

v2/e2e/manifests/nat-client.yaml

@@ -11,6 +11,7 @@ spec:
     operator: Exists
   nodeSelector:
     test: coil
+    kubernetes.io/hostname: coil-worker
   containers:
   - name: ubuntu
     image: ghcr.io/cybozu/ubuntu-debug:22.04

v2/pkg/nodenet/pod.go

@@ -422,7 +422,9 @@ func (pn *podNetwork) Update(podIPv4, podIPv6 net.IP, hook SetupHook) error {
 
 	var netNsPath string
 	for _, c := range podConfigs {
-		if c.IPv4.Equal(podIPv4) || c.IPv6.Equal(podIPv6) {
+		// When both c.IPvX and podIPvX are nil, net.IP.Equal() returns always true.
+		// To avoid comparing nil to nil, confirm c.IPvX is not nil.
+		if (c.IPv4 != nil && c.IPv4.Equal(podIPv4)) || (c.IPv6 != nil && c.IPv6.Equal(podIPv6)) {
 			netNsPath, err = getNetNsPath(c.HostVethName)
 			if err != nil {
 				return err