Skip to content

Commit

Permalink
Merge pull request #482 from heschlie/fix-480
Browse files Browse the repository at this point in the history
Segfault fix in Calico Node to K8s Node conversion
  • Loading branch information
heschlie authored Jul 25, 2017
2 parents 521dd1c + ea7ed21 commit 9171efc
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 79 deletions.
72 changes: 0 additions & 72 deletions lib/backend/k8s/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ import (
"k8s.io/apimachinery/pkg/util/intstr"
k8sapi "k8s.io/client-go/pkg/api/v1"
extensions "k8s.io/client-go/pkg/apis/extensions/v1beta1"

"github.com/projectcalico/libcalico-go/lib/backend/k8s/resources"
"github.com/projectcalico/libcalico-go/lib/net"
)

var _ = Describe("Test parsing strings", func() {
Expand Down Expand Up @@ -688,72 +685,3 @@ var _ = Describe("Test Namespace conversion", func() {
})
})
})

var _ = Describe("Test Node conversion", func() {

It("should parse a k8s Node to a Calico Node", func() {
l := map[string]string{"net.beta.kubernetes.io/role": "master"}
node := k8sapi.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "TestNode",
Labels: l,
ResourceVersion: "1234",
Annotations: map[string]string{
"projectcalico.org/IPv4Address": "172.17.17.10/24",
"projectcalico.org/ASNumber": "2546",
},
},
Status: k8sapi.NodeStatus{
Addresses: []k8sapi.NodeAddress{
k8sapi.NodeAddress{
Type: k8sapi.NodeInternalIP,
Address: "172.17.17.10",
},
k8sapi.NodeAddress{
Type: k8sapi.NodeExternalIP,
Address: "192.168.1.100",
},
k8sapi.NodeAddress{
Type: k8sapi.NodeHostName,
Address: "172-17-17-10",
},
},
},
Spec: k8sapi.NodeSpec{},
}

n, err := resources.K8sNodeToCalico(&node)
Expect(err).NotTo(HaveOccurred())

// Ensure we got the correct values.
felixAddress := *n.Value.(*model.Node).FelixIPv4
bgpAddress := *n.Value.(*model.Node).BGPIPv4Addr
bgpNet := *n.Value.(*model.Node).BGPIPv4Net
labels := n.Value.(*model.Node).Labels
asn := n.Value.(*model.Node).BGPASNumber

ip, ipNet, _ := net.ParseCIDR("172.17.17.10/24")

Expect(felixAddress).To(Equal(*ip))
Expect(bgpAddress).To(Equal(*ip))
Expect(bgpNet).To(Equal(*ipNet))
Expect(labels).To(Equal(l))
Expect(asn.String()).To(Equal("2546"))
})

It("should error on an invalid IP", func() {
l := map[string]string{"net.beta.kubernetes.io/role": "master"}
node := k8sapi.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "TestNode",
Labels: l,
ResourceVersion: "1234",
Annotations: map[string]string{"projectcalico.org/IPv4Address": "172.984.12.5/24"},
},
Spec: k8sapi.NodeSpec{},
}

_, err := resources.K8sNodeToCalico(&node)
Expect(err).To(HaveOccurred())
})
})
5 changes: 5 additions & 0 deletions lib/backend/k8s/resources/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ import (
"k8s.io/client-go/rest"
)

const (
nodeBgpIpv4CidrAnnotation = "projectcalico.org/IPv4Address"
nodeBgpAsnAnnotation = "projectcalico.org/ASNumber"
)

func NewNodeClient(c *kubernetes.Clientset, r *rest.RESTClient) K8sResourceClient {
return &retryWrapper{
client: &nodeClient{
Expand Down
18 changes: 11 additions & 7 deletions lib/backend/k8s/resources/node_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func K8sNodeToCalico(node *kapiv1.Node) (*model.KVPair, error) {
calicoNode := model.Node{}
calicoNode.Labels = node.Labels
annotations := node.ObjectMeta.Annotations
cidrString, ok := annotations["projectcalico.org/IPv4Address"]
cidrString, ok := annotations[nodeBgpIpv4CidrAnnotation]
if ok {
ip, cidr, err := net.ParseCIDR(cidrString)
if err != nil {
Expand All @@ -47,7 +47,7 @@ func K8sNodeToCalico(node *kapiv1.Node) (*model.KVPair, error) {
calicoNode.BGPIPv4Net = cidr
}

asnString, ok := annotations["projectcalico.org/ASNumber"]
asnString, ok := annotations[nodeBgpAsnAnnotation]
if ok {
asn, err := numorstring.ASNumberFromString(asnString)
if err != nil {
Expand All @@ -67,15 +67,19 @@ func K8sNodeToCalico(node *kapiv1.Node) (*model.KVPair, error) {
func mergeCalicoK8sNode(calicoNode *model.Node, k8sNode *kapiv1.Node) (*kapiv1.Node, error) {
// In order to make sure we always end up with a CIDR that has the IP and not just network
// we assemble the CIDR from BGPIPv4Addr and BGPIPv4Net.
subnet, _ := calicoNode.BGPIPv4Net.Mask.Size()
ipCidr := fmt.Sprintf("%s/%d", calicoNode.BGPIPv4Addr.String(), subnet)
k8sNode.Annotations["projectcalico.org/IPv4Address"] = ipCidr
if calicoNode.BGPIPv4Net != nil {
subnet, _ := calicoNode.BGPIPv4Net.Mask.Size()
ipCidr := fmt.Sprintf("%s/%d", calicoNode.BGPIPv4Addr.String(), subnet)
k8sNode.Annotations[nodeBgpIpv4CidrAnnotation] = ipCidr
} else {
delete(k8sNode.Annotations, nodeBgpIpv4CidrAnnotation)
}

// Don't set the ASNumber if it is nil, and ensure it does not exist in k8s.
if calicoNode.BGPASNumber != nil {
k8sNode.Annotations["projectcalico.org/ASNumber"] = calicoNode.BGPASNumber.String()
k8sNode.Annotations[nodeBgpAsnAnnotation] = calicoNode.BGPASNumber.String()
} else {
delete(k8sNode.Annotations, "projectcalico.org/ASNumber")
delete(k8sNode.Annotations, nodeBgpAsnAnnotation)
}

return k8sNode, nil
Expand Down
135 changes: 135 additions & 0 deletions lib/backend/k8s/resources/node_conversion_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
package resources

import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"github.com/projectcalico/libcalico-go/lib/backend/model"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8sapi "k8s.io/client-go/pkg/api/v1"

"github.com/projectcalico/libcalico-go/lib/net"
"github.com/projectcalico/libcalico-go/lib/numorstring"
)

var _ = Describe("Test Node conversion", func() {

It("should parse a k8s Node to a Calico Node", func() {
l := map[string]string{"net.beta.kubernetes.io/role": "master"}
node := k8sapi.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "TestNode",
Labels: l,
ResourceVersion: "1234",
Annotations: map[string]string{
nodeBgpIpv4CidrAnnotation: "172.17.17.10/24",
nodeBgpAsnAnnotation: "2546",
},
},
Status: k8sapi.NodeStatus{
Addresses: []k8sapi.NodeAddress{
k8sapi.NodeAddress{
Type: k8sapi.NodeInternalIP,
Address: "172.17.17.10",
},
k8sapi.NodeAddress{
Type: k8sapi.NodeExternalIP,
Address: "192.168.1.100",
},
k8sapi.NodeAddress{
Type: k8sapi.NodeHostName,
Address: "172-17-17-10",
},
},
},
Spec: k8sapi.NodeSpec{},
}

n, err := K8sNodeToCalico(&node)
Expect(err).NotTo(HaveOccurred())

// Ensure we got the correct values.
felixAddress := *n.Value.(*model.Node).FelixIPv4
bgpAddress := *n.Value.(*model.Node).BGPIPv4Addr
bgpNet := *n.Value.(*model.Node).BGPIPv4Net
labels := n.Value.(*model.Node).Labels
asn := n.Value.(*model.Node).BGPASNumber

ip, ipNet, _ := net.ParseCIDR("172.17.17.10/24")

Expect(felixAddress).To(Equal(*ip))
Expect(bgpAddress).To(Equal(*ip))
Expect(bgpNet).To(Equal(*ipNet))
Expect(labels).To(Equal(l))
Expect(asn.String()).To(Equal("2546"))
})

It("should error on an invalid IP", func() {
l := map[string]string{"net.beta.kubernetes.io/role": "master"}
node := k8sapi.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "TestNode",
Labels: l,
ResourceVersion: "1234",
Annotations: map[string]string{nodeBgpIpv4CidrAnnotation: "172.984.12.5/24"},
},
Spec: k8sapi.NodeSpec{},
}

_, err := K8sNodeToCalico(&node)
Expect(err).To(HaveOccurred())
})

It("Should parse and remove BGP info when given Calico Node with empty BGP spec", func() {
l := map[string]string{"net.beta.kubernetes.io/role": "master"}
k8sNode := &k8sapi.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "TestNode",
Labels: l,
ResourceVersion: "1234",
Annotations: map[string]string{
nodeBgpIpv4CidrAnnotation: "172.17.17.10/24",
nodeBgpAsnAnnotation: "2546",
},
},
Spec: k8sapi.NodeSpec{},
}

calicoNode := &model.Node{}

newK8sNode, err := mergeCalicoK8sNode(calicoNode, k8sNode)
Expect(err).NotTo(HaveOccurred())
Expect(newK8sNode.Annotations).NotTo(HaveKey(nodeBgpIpv4CidrAnnotation))
Expect(newK8sNode.Annotations).NotTo(HaveKey(nodeBgpAsnAnnotation))
})

It("Should merger Calico Nodes into K8s Nodes", func() {
l := map[string]string{"net.beta.kubernetes.io/role": "master"}
k8sNode := &k8sapi.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "TestNode",
Labels: l,
ResourceVersion: "1234",
Annotations: make(map[string]string),
},
Spec: k8sapi.NodeSpec{},
}

ip, cidr, _ := net.ParseCIDR("172.17.17.10/24")
asn, _ := numorstring.ASNumberFromString("2456")

calicoNode := &model.Node{
BGPIPv4Net: cidr,
FelixIPv4: ip,
BGPIPv4Addr: ip,
BGPASNumber: &asn,
}

newK8sNode, err := mergeCalicoK8sNode(calicoNode, k8sNode)
Expect(err).NotTo(HaveOccurred())
Expect(newK8sNode.Annotations).To(HaveKeyWithValue(nodeBgpIpv4CidrAnnotation, "172.17.17.10/24"))
Expect(newK8sNode.Annotations).To(HaveKeyWithValue(nodeBgpAsnAnnotation, "2456"))
})
})

0 comments on commit 9171efc

Please sign in to comment.