Merge pull request #157 from JacobTanenbaum/release-4.4-BZ1829442

[release 4.4] bug 1829442: Remove errors based on passing nil elements to MarshalPodAnnotations()
openshift · May 14, 2020 · 16af8f8 · 16af8f8
2 parents d043621 + 146a3f7
commit 16af8f8
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 8 deletions.
diff --git a/go-controller/pkg/ovn/pods.go b/go-controller/pkg/ovn/pods.go
@@ -331,18 +331,24 @@ func (oc *Controller) addLogicalPort(pod *kapi.Pod) error {
 		if err != nil {
 			return err
 		}
+
+		var gwIPs []net.IP
+		if gwIP != nil {
+			gwIPs = []net.IP{gwIP}
+		}
+
 		marshalledAnnotation, err := util.MarshalPodAnnotation(&util.PodAnnotation{
 			IPs:      []*net.IPNet{podCIDR},
 			MAC:      podMac,
-			Gateways: []net.IP{gwIP},
+			Gateways: gwIPs,
 			Routes:   routes,
 		})
 		if err != nil {
 			return fmt.Errorf("error creating pod network annotation: %v", err)
 		}
 
 		klog.V(5).Infof("Annotation values: ip=%s ; mac=%s ; gw=%s\nAnnotation=%s",
-			podCIDR, podMac, gwIP, marshalledAnnotation)
+			podCIDR, podMac, gwIPs, marshalledAnnotation)
 		if err = oc.kube.SetAnnotationsOnPod(pod, marshalledAnnotation); err != nil {
 			return fmt.Errorf("failed to set annotation on pod %s: %v", pod.Name, err)
 		}

diff --git a/go-controller/pkg/util/pod_annotation.go b/go-controller/pkg/util/pod_annotation.go
@@ -182,7 +182,7 @@ func UnmarshalPodAnnotation(annotations map[string]string) (*PodAnnotation, erro
 	}
 	for _, gwstr := range a.Gateways {
 		gw := net.ParseIP(gwstr)
-		if err != nil {
+		if gw == nil {
 			return nil, fmt.Errorf("failed to parse pod gateway %q", gwstr)
 		}
 		podAnnotation.Gateways = append(podAnnotation.Gateways, gw)

diff --git a/go-controller/pkg/util/pod_annotation_test.go b/go-controller/pkg/util/pod_annotation_test.go
@@ -1,6 +1,7 @@
 package util
 
 import (
+	"fmt"
 	"net"
 
 	ovntest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing"
@@ -12,9 +13,10 @@ import (
 var _ = Describe("Pod annotation tests", func() {
 	It("marshals network info to pod annotations", func() {
 		type testcase struct {
-			name string
-			in   *PodAnnotation
-			out  map[string]string
+			name         string
+			in           *PodAnnotation
+			out          map[string]string
+			unmarshalErr error
 		}
 
 		testcases := []testcase{
@@ -39,6 +41,18 @@ var _ = Describe("Pod annotation tests", func() {
 					"k8s.ovn.org/pod-networks": `{"default":{"ip_addresses":["192.168.0.5/24"],"mac_address":"0a:58:fd:98:00:01","ip_address":"192.168.0.5/24"}}`,
 				},
 			},
+			{
+				name: "Nil entry in GW",
+				in: &PodAnnotation{
+					IPs:      []*net.IPNet{ovntest.MustParseIPNet("192.168.0.5/24")},
+					MAC:      ovntest.MustParseMAC("0A:58:FD:98:00:01"),
+					Gateways: []net.IP{nil},
+				},
+				out: map[string]string{
+					"k8s.ovn.org/pod-networks": `{"default":{"ip_addresses":["192.168.0.5/24"],"mac_address":"0a:58:fd:98:00:01","gateway_ips":["\u003cnil\u003e"],"ip_address":"192.168.0.5/24","gateway_ip":"\u003cnil\u003e"}}`,
+				},
+				unmarshalErr: fmt.Errorf(`failed to parse pod gateway "<nil>"`),
+			},
 			{
 				name: "Routes",
 				in: &PodAnnotation{
@@ -91,8 +105,12 @@ var _ = Describe("Pod annotation tests", func() {
 			Expect(err).NotTo(HaveOccurred(), "test case %q got unexpected marshalling error", tc.name)
 			Expect(marshalled).To(Equal(tc.out), "test case %q marshalled to wrong value", tc.name)
 			unmarshalled, err := UnmarshalPodAnnotation(marshalled)
-			Expect(err).NotTo(HaveOccurred(), "test case %q got unexpected unmarshalling error", tc.name)
-			Expect(unmarshalled).To(Equal(tc.in), "test case %q unmarshalled to wrong value", tc.name)
+			if tc.unmarshalErr == nil {
+				Expect(err).NotTo(HaveOccurred(), "test case %q got unexpected unmarshalling error", tc.name)
+				Expect(unmarshalled).To(Equal(tc.in), "test case %q unmarshalled to wrong value", tc.name)
+			} else {
+				Expect(err).To(Equal(tc.unmarshalErr))
+			}
 		}
 	})
 })