mirrors/kube-router
1
0
Fork 0
mirror of https://github.com/cloudnativelabs/kube-router.git synced 2025-11-18 11:31:12 +01:00
Code Issues Packages Projects Releases Wiki Activity

fix(bgp_policies_test.go): use startBgpServer()

Browse Source 
Use startBgpServer() rather than doing things individually, so that we
can follow the logic path of how kube-router actually works better. This
allows us to use annotations rather than set stuff manually and allows
us to test more of the code-path of the NRC.

Additionally, this change allows us to actually test some errors better
such as, make sure that startBgpServer() actually throws the error we
expect when only one part of the prepend ASN annotation is present.
Previously, we were not actually testing this code path.
This commit is contained in:
Aaron U'Ren 2021-05-13 10:56:00 -05:00
parent a5d6560751
commit fa7bcdeb06
1 changed files with 70 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

108
pkg/controllers/routing/bgp_policies_test.go
View File

@ -2,6 +2,7 @@ package routing
import (
"context"
"fmt"
"reflect"
"testing"
"time"
@ -26,7 +27,8 @@ type PolicyTestCase struct {
allPeerDefinedSet *gobgpapi.DefinedSet
exportPolicyStatements []*gobgpapi.Statement
importPolicyStatements []*gobgpapi.Statement
err error
addPolicyErr error
startBGPServerErr error
}
func Test_AddPolicies(t *testing.T) {
@ -36,6 +38,8 @@ func Test_AddPolicies(t *testing.T) {
&NetworkRoutingController{
clientset: fake.NewSimpleClientset(),
hostnameOverride: "node-1",
routerID: "10.0.0.0",
bgpPort: 10000,
bgpFullMeshMode: false,
bgpEnableInternal: true,
bgpServer: gobgp.NewBgpServer(),
@ -121,6 +125,7 @@ func Test_AddPolicies(t *testing.T) {
MatchType: gobgpapi.MatchType_ANY,
Name: "iBGPpeerset",
},
RpkiResult: -1,
},
Actions: &gobgpapi.Actions{
RouteAction: gobgpapi.RouteAction_ACCEPT,
@ -139,6 +144,7 @@ func Test_AddPolicies(t *testing.T) {
MatchType: gobgpapi.MatchType_ANY,
Name: "allpeerset",
},
RpkiResult: -1,
},
Actions: &gobgpapi.Actions{
RouteAction: gobgpapi.RouteAction_REJECT,
@ -146,12 +152,15 @@ func Test_AddPolicies(t *testing.T) {
},
},
nil,
nil,
},
{
"has nodes, services with external peers",
&NetworkRoutingController{
clientset: fake.NewSimpleClientset(),
hostnameOverride: "node-1",
routerID: "10.0.0.0",
bgpPort: 10000,
bgpFullMeshMode: false,
bgpEnableInternal: true,
bgpServer: gobgp.NewBgpServer(),
@ -253,6 +262,7 @@ func Test_AddPolicies(t *testing.T) {
MatchType: gobgpapi.MatchType_ANY,
Name: "iBGPpeerset",
},
RpkiResult: -1,
},
Actions: &gobgpapi.Actions{
RouteAction: gobgpapi.RouteAction_ACCEPT,
@ -269,6 +279,7 @@ func Test_AddPolicies(t *testing.T) {
MatchType: gobgpapi.MatchType_ANY,
Name: "externalpeerset",
},
RpkiResult: -1,
},
Actions: &gobgpapi.Actions{
RouteAction: gobgpapi.RouteAction_ACCEPT,
@ -287,6 +298,7 @@ func Test_AddPolicies(t *testing.T) {
MatchType: gobgpapi.MatchType_ANY,
Name: "allpeerset",
},
RpkiResult: -1,
},
Actions: &gobgpapi.Actions{
RouteAction: gobgpapi.RouteAction_REJECT,
@ -294,12 +306,15 @@ func Test_AddPolicies(t *testing.T) {
},
},
nil,
nil,
},
{
"has nodes, services with external peers and iBGP disabled",
&NetworkRoutingController{
clientset: fake.NewSimpleClientset(),
hostnameOverride: "node-1",
routerID: "10.0.0.0",
bgpPort: 10000,
bgpFullMeshMode: false,
bgpEnableInternal: false,
bgpServer: gobgp.NewBgpServer(),
@ -401,6 +416,7 @@ func Test_AddPolicies(t *testing.T) {
MatchType: gobgpapi.MatchType_ANY,
Name: "externalpeerset",
},
RpkiResult: -1,
},
Actions: &gobgpapi.Actions{
RouteAction: gobgpapi.RouteAction_ACCEPT,
@ -419,6 +435,7 @@ func Test_AddPolicies(t *testing.T) {
MatchType: gobgpapi.MatchType_ANY,
Name: "allpeerset",
},
RpkiResult: -1,
},
Actions: &gobgpapi.Actions{
RouteAction: gobgpapi.RouteAction_REJECT,
@ -426,17 +443,17 @@ func Test_AddPolicies(t *testing.T) {
},
},
nil,
nil,
},
{
"prepends AS with external peers",
&NetworkRoutingController{
clientset: fake.NewSimpleClientset(),
hostnameOverride: "node-1",
routerID: "10.0.0.0",
bgpPort: 10000,
bgpEnableInternal: true,
bgpFullMeshMode: false,
pathPrepend: true,
pathPrependCount: 5,
pathPrependAS: "65100",
bgpServer: gobgp.NewBgpServer(),
activeNodes: make(map[string]bool),
podCidr: "172.20.0.0/24",
@ -459,7 +476,9 @@ func Test_AddPolicies(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "node-1",
Annotations: map[string]string{
"kube-router.io/node.asn": "100",
"kube-router.io/node.asn": "100",
"kube-router.io/path-prepend.as": "65100",
"kube-router.io/path-prepend.repeat-n": "5",
},
},
Status: v1core.NodeStatus{
@ -536,6 +555,7 @@ func Test_AddPolicies(t *testing.T) {
MatchType: gobgpapi.MatchType_ANY,
Name: "iBGPpeerset",
},
RpkiResult: -1,
},
Actions: &gobgpapi.Actions{
RouteAction: gobgpapi.RouteAction_ACCEPT,
@ -552,6 +572,7 @@ func Test_AddPolicies(t *testing.T) {
MatchType: gobgpapi.MatchType_ANY,
Name: "externalpeerset",
},
RpkiResult: -1,
},
Actions: &gobgpapi.Actions{
RouteAction: gobgpapi.RouteAction_ACCEPT,
@ -574,6 +595,7 @@ func Test_AddPolicies(t *testing.T) {
MatchType: gobgpapi.MatchType_ANY,
Name: "allpeerset",
},
RpkiResult: -1,
},
Actions: &gobgpapi.Actions{
RouteAction: gobgpapi.RouteAction_REJECT,
@ -581,16 +603,17 @@ func Test_AddPolicies(t *testing.T) {
},
},
nil,
nil,
},
{
"only prepends AS when both node annotations are present",
&NetworkRoutingController{
clientset: fake.NewSimpleClientset(),
hostnameOverride: "node-1",
routerID: "10.0.0.0",
bgpPort: 10000,
bgpEnableInternal: true,
bgpFullMeshMode: false,
pathPrepend: false,
pathPrependAS: "65100",
bgpServer: gobgp.NewBgpServer(),
activeNodes: make(map[string]bool),
podCidr: "172.20.0.0/24",
@ -613,7 +636,8 @@ func Test_AddPolicies(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "node-1",
Annotations: map[string]string{
"kube-router.io/node.asn": "100",
"kube-router.io/node.asn": "100",
"kube-router.io/path-prepend.as": "65100",
},
},
Status: v1core.NodeStatus{
@ -690,6 +714,7 @@ func Test_AddPolicies(t *testing.T) {
MatchType: gobgpapi.MatchType_ANY,
Name: "iBGPpeerset",
},
RpkiResult: -1,
},
Actions: &gobgpapi.Actions{
RouteAction: gobgpapi.RouteAction_ACCEPT,
@ -706,6 +731,7 @@ func Test_AddPolicies(t *testing.T) {
MatchType: gobgpapi.MatchType_ANY,
Name: "externalpeerset",
},
RpkiResult: -1,
},
Actions: &gobgpapi.Actions{
RouteAction: gobgpapi.RouteAction_ACCEPT,
@ -728,6 +754,7 @@ func Test_AddPolicies(t *testing.T) {
MatchType: gobgpapi.MatchType_ANY,
Name: "allpeerset",
},
RpkiResult: -1,
},
Actions: &gobgpapi.Actions{
RouteAction: gobgpapi.RouteAction_REJECT,
@ -735,20 +762,31 @@ func Test_AddPolicies(t *testing.T) {
},
},
nil,
fmt.Errorf("both %s and %s must be set", pathPrependASNAnnotation, pathPrependRepeatNAnnotation),
},
}
for _, testcase := range testcases {
t.Run(testcase.name, func(t *testing.T) {
go testcase.nrc.bgpServer.Serve()
global := &gobgpapi.Global{
As: 1,
RouterId: "10.0.0.0",
ListenPort: 10000,
startInformersForRoutes(testcase.nrc, testcase.nrc.clientset)
if err := createNodes(testcase.nrc.clientset, testcase.existingNodes); err != nil {
t.Errorf("failed to create existing nodes: %v", err)
}
err := testcase.nrc.bgpServer.StartBgp(context.Background(), &gobgpapi.StartBgpRequest{Global: global})
if err != nil {
t.Fatalf("failed to start BGP server: %v", err)
if err := createServices(testcase.nrc.clientset, testcase.existingServices); err != nil {
t.Errorf("failed to create existing nodes: %v", err)
}
err := testcase.nrc.startBgpServer(false)
if !reflect.DeepEqual(err, testcase.startBGPServerErr) {
t.Logf("expected err when invoking startBGPServer(): %v", testcase.startBGPServerErr)
t.Logf("actual err from startBGPServer() received: %v", err)
t.Error("unexpected error")
}
// If the server was not expected to start we should stop here as the rest of the tests are unimportant
if testcase.startBGPServerErr != nil {
return
}
defer func() {
if err := testcase.nrc.bgpServer.StopBgp(context.Background(), &gobgpapi.StopBgpRequest{}); err != nil {
@ -756,16 +794,6 @@ func Test_AddPolicies(t *testing.T) {
}
}()
startInformersForRoutes(testcase.nrc, testcase.nrc.clientset)
if err = createNodes(testcase.nrc.clientset, testcase.existingNodes); err != nil {
t.Errorf("failed to create existing nodes: %v", err)
}
if err = createServices(testcase.nrc.clientset, testcase.existingServices); err != nil {
t.Errorf("failed to create existing nodes: %v", err)
}
// ClusterIPs and ExternalIPs
waitForListerWithTimeout(testcase.nrc.svcLister, time.Second*10, t)
@ -777,21 +805,25 @@ func Test_AddPolicies(t *testing.T) {
nodeInformer := informerFactory.Core().V1().Nodes().Informer()
testcase.nrc.nodeLister = nodeInformer.GetIndexer()
err = testcase.nrc.AddPolicies()
if !reflect.DeepEqual(err, testcase.err) {
t.Logf("expected err %v", testcase.err)
t.Logf("actual err %v", err)
if !reflect.DeepEqual(err, testcase.addPolicyErr) {
t.Logf("expected err when invoking AddPolicies(): %v", testcase.addPolicyErr)
t.Logf("actual err from AddPolicies() received: %v", err)
t.Error("unexpected error")
}
// If we expect AddPolicies() to fail we should stop here as there is no point in further evaluating policies
if testcase.addPolicyErr != nil {
return
}
err = testcase.nrc.bgpServer.ListDefinedSet(context.Background(), &gobgpapi.ListDefinedSetRequest{
DefinedType: gobgpapi.DefinedType_PREFIX,
Name: "podcidrdefinedset"}, func(podDefinedSet *gobgpapi.DefinedSet) {
if !reflect.DeepEqual(podDefinedSet, testcase.podDefinedSet) {
t.Logf("expected pod defined set: %+v", testcase.podDefinedSet)
t.Logf("actual pod defined set: %+v", podDefinedSet)
t.Error("unexpected pod defined set")
}
})
err = testcase.nrc.bgpServer.ListDefinedSet(context.Background(),
&gobgpapi.ListDefinedSetRequest{DefinedType: gobgpapi.DefinedType_PREFIX, Name: "podcidrdefinedset"},
func(podDefinedSet *gobgpapi.DefinedSet) {
if !reflect.DeepEqual(podDefinedSet, testcase.podDefinedSet) {
t.Logf("expected pod defined set: %+v", testcase.podDefinedSet)
t.Logf("actual pod defined set: %+v", podDefinedSet)
t.Error("unexpected pod defined set")
}
})
if err != nil {
t.Fatalf("error validating defined sets: %v", err)
}