Merge pull request #449 from fasaxc/fix-nil-node-ip-2

Avoid setting HostIP to a nil pointer (when we expect a nil interface).
projectcalico · Jun 12, 2017 · 3cfe5e8 · 3cfe5e8
2 parents 6dbd98f + ea48663
commit 3cfe5e8
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 2 deletions.
diff --git a/lib/backend/k8s/syncer.go b/lib/backend/k8s/syncer.go
@@ -907,14 +907,19 @@ func (syn *kubeSyncer) parseNodeEvent(e watch.Event) (*model.KVPair, *model.KVPa
 
 	kvp, err := resources.K8sNodeToCalico(node)
 	if err != nil {
-		log.Panicf("%s", err)
+		log.WithError(err).Panic("Failed to convert k8s node to Calico node.")
 	}
 
 	kvpHostIp := &model.KVPair{
 		Key:      model.HostIPKey{Hostname: node.Name},
-		Value:    kvp.Value.(*model.Node).BGPIPv4Addr,
 		Revision: kvp.Revision,
 	}
+	caliNode := kvp.Value.(*model.Node)
+	if caliNode.BGPIPv4Addr != nil {
+		// Only set the value if it's non nil.  We want to avoid setting Value to
+		// an interface containing a nil value instead of a nil interface.
+		kvpHostIp.Value = caliNode.BGPIPv4Addr
+	}
 
 	kvpIPIPAddr, err := getTunIp(node)
 	if err != nil || kvpIPIPAddr == nil {

diff --git a/lib/backend/k8s/syncer_test.go b/lib/backend/k8s/syncer_test.go
@@ -23,6 +23,7 @@ import (
 	"github.com/projectcalico/libcalico-go/lib/backend/api"
 	"github.com/projectcalico/libcalico-go/lib/backend/k8s/thirdparty"
 	"github.com/projectcalico/libcalico-go/lib/backend/model"
+	"github.com/projectcalico/libcalico-go/lib/net"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/watch"
 	k8sapi "k8s.io/client-go/pkg/api/v1"
@@ -225,6 +226,64 @@ var _ = Describe("Test Syncer", func() {
 		Expect(syn).NotTo(BeNil())
 	})
 
+	Describe("without starting the syncer", func() {
+		// These tests reach in and test individual methods.
+		It("should parse a node event with a nil IPs", func() {
+			kv1, kv2 := syn.parseNodeEvent(watch.Event{
+				Object: &k8sapi.Node{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "some-host",
+						ResourceVersion: "1234",
+					},
+				},
+			})
+			Expect(kv1).To(Equal(&model.KVPair{
+				Key:      model.HostIPKey{Hostname: "some-host"},
+				Revision: "1234",
+			}))
+			Expect(kv2).To(Equal(&model.KVPair{
+				Key: model.HostConfigKey{
+					Hostname: "some-host",
+					Name:     "IpInIpTunnelAddr",
+				},
+				Revision: "1234",
+			}))
+		})
+		It("should parse a node event with IPs set", func() {
+			kv1, kv2 := syn.parseNodeEvent(watch.Event{
+				Object: &k8sapi.Node{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "some-host",
+						ResourceVersion: "1234",
+						Annotations: map[string]string{
+							"projectcalico.org/IPv4Address": "11.0.0.1/24",
+						},
+					},
+					Spec: k8sapi.NodeSpec{
+						PodCIDR: "10.0.10.0/24",
+					},
+				},
+			})
+			// Using ParseCIDR here so that we get the same format of IP address as
+			// K8sNodeToCalico.
+			ip, _, err := net.ParseCIDR("11.0.0.1/24")
+			Expect(err).NotTo(HaveOccurred())
+			Expect(kv1).To(Equal(&model.KVPair{
+				Key:      model.HostIPKey{Hostname: "some-host"},
+				Value:    ip,
+				Revision: "1234",
+			}))
+			Expect(kv2).To(Equal(&model.KVPair{
+				Key: model.HostConfigKey{
+					Hostname: "some-host",
+					Name:     "IpInIpTunnelAddr",
+				},
+				Value:    "10.0.10.1",
+				Revision: "1234",
+			}))
+		})
+	})
+
 	Context("with a running syncer", func() {
 
 		BeforeEach(func() {