Skip to content

Commit

Permalink
fix clusteripprefixset import policy (#771)
Browse files Browse the repository at this point in the history
  • Loading branch information
tamihiro authored and murali-reddy committed Sep 9, 2019
1 parent 803bd90 commit 3aacd48
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 2 deletions.
18 changes: 16 additions & 2 deletions pkg/controllers/routing/bgp_policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ func (nrc *NetworkRoutingController) AddPolicies() error {
nrc.bgpServer.AddDefinedSet(clusterIPPrefixSet)
}

iBGPPeers := make([]string, 0)
if nrc.bgpEnableInternal {
// Get the current list of the nodes from the local cache
nodes := nrc.nodeLister.List()
iBGPPeers := make([]string, 0)
for _, node := range nodes {
nodeObj := node.(*v1core.Node)
nodeIP, err := utils.GetNodeIP(nodeObj)
Expand Down Expand Up @@ -97,6 +97,17 @@ func (nrc *NetworkRoutingController) AddPolicies() error {
}
}

// a slice of all peers is used as a match condition for reject statement of clusteripprefixset import polcy
allBgpPeers := append(externalBgpPeers, iBGPPeers...)
ns, _ := table.NewNeighborSet(config.NeighborSet{
NeighborSetName: "allpeerset",
NeighborInfoList: allBgpPeers,
})
err = nrc.bgpServer.ReplaceDefinedSet(ns)
if err != nil {
nrc.bgpServer.AddDefinedSet(ns)
}

err = nrc.addExportPolicies()
if err != nil {
return err
Expand Down Expand Up @@ -258,7 +269,7 @@ func (nrc *NetworkRoutingController) addExportPolicies() error {
}

// BGP import policies are added so that the following conditions are met:
// - do not import Service VIPs at all, instead traffic to service VIPs should be sent to the gateway and ECMPed from there
// - do not import Service VIPs advertised from any peers, instead each kube-router originates and injects Service VIPs into local rib.
func (nrc *NetworkRoutingController) addImportPolicies() error {
statements := make([]config.Statement, 0)

Expand All @@ -267,6 +278,9 @@ func (nrc *NetworkRoutingController) addImportPolicies() error {
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "clusteripprefixset",
},
MatchNeighborSet: config.MatchNeighborSet{
NeighborSet: "allpeerset",
},
},
Actions: config.Actions{
RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
Expand Down
87 changes: 87 additions & 0 deletions pkg/controllers/routing/network_routes_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1490,6 +1490,7 @@ type PolicyTestCase struct {
podDefinedSet *config.DefinedSets
clusterIPDefinedSet *config.DefinedSets
externalPeerDefinedSet *config.DefinedSets
allPeerDefinedSet *config.DefinedSets
exportPolicyStatements []*config.Statement
importPolicyStatements []*config.Statement
err error
Expand Down Expand Up @@ -1578,6 +1579,17 @@ func Test_AddPolicies(t *testing.T) {
BgpDefinedSets: config.BgpDefinedSets{},
},
&config.DefinedSets{},
&config.DefinedSets{
PrefixSets: []config.PrefixSet{},
NeighborSets: []config.NeighborSet{
{
NeighborSetName: "allpeerset",
NeighborInfoList: []string{},
},
},
TagSets: []config.TagSet{},
BgpDefinedSets: config.BgpDefinedSets{},
},
[]*config.Statement{
{
Name: "kube_router_export_stmt0",
Expand All @@ -1604,6 +1616,10 @@ func Test_AddPolicies(t *testing.T) {
PrefixSet: "clusteripprefixset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
MatchNeighborSet: config.MatchNeighborSet{
NeighborSet: "allpeerset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
},
Actions: config.Actions{
RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
Expand Down Expand Up @@ -1711,6 +1727,17 @@ func Test_AddPolicies(t *testing.T) {
TagSets: []config.TagSet{},
BgpDefinedSets: config.BgpDefinedSets{},
},
&config.DefinedSets{
PrefixSets: []config.PrefixSet{},
NeighborSets: []config.NeighborSet{
{
NeighborSetName: "allpeerset",
NeighborInfoList: []string{"10.10.0.1/32", "10.10.0.2/32"},
},
},
TagSets: []config.TagSet{},
BgpDefinedSets: config.BgpDefinedSets{},
},
[]*config.Statement{
{
Name: "kube_router_export_stmt0",
Expand Down Expand Up @@ -1753,6 +1780,10 @@ func Test_AddPolicies(t *testing.T) {
PrefixSet: "clusteripprefixset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
MatchNeighborSet: config.MatchNeighborSet{
NeighborSet: "allpeerset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
},
Actions: config.Actions{
RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
Expand Down Expand Up @@ -1860,6 +1891,17 @@ func Test_AddPolicies(t *testing.T) {
TagSets: []config.TagSet{},
BgpDefinedSets: config.BgpDefinedSets{},
},
&config.DefinedSets{
PrefixSets: []config.PrefixSet{},
NeighborSets: []config.NeighborSet{
{
NeighborSetName: "allpeerset",
NeighborInfoList: []string{"10.10.0.1/32", "10.10.0.2/32"},
},
},
TagSets: []config.TagSet{},
BgpDefinedSets: config.BgpDefinedSets{},
},
[]*config.Statement{
{
Name: "kube_router_export_stmt0",
Expand All @@ -1886,6 +1928,10 @@ func Test_AddPolicies(t *testing.T) {
PrefixSet: "clusteripprefixset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
MatchNeighborSet: config.MatchNeighborSet{
NeighborSet: "allpeerset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
},
Actions: config.Actions{
RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
Expand Down Expand Up @@ -1996,6 +2042,17 @@ func Test_AddPolicies(t *testing.T) {
TagSets: []config.TagSet{},
BgpDefinedSets: config.BgpDefinedSets{},
},
&config.DefinedSets{
PrefixSets: []config.PrefixSet{},
NeighborSets: []config.NeighborSet{
{
NeighborSetName: "allpeerset",
NeighborInfoList: []string{"10.10.0.1/32", "10.10.0.2/32"},
},
},
TagSets: []config.TagSet{},
BgpDefinedSets: config.BgpDefinedSets{},
},
[]*config.Statement{
{
Name: "kube_router_export_stmt0",
Expand Down Expand Up @@ -2044,6 +2101,10 @@ func Test_AddPolicies(t *testing.T) {
PrefixSet: "clusteripprefixset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
MatchNeighborSet: config.MatchNeighborSet{
NeighborSet: "allpeerset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
},
Actions: config.Actions{
RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
Expand Down Expand Up @@ -2153,6 +2214,17 @@ func Test_AddPolicies(t *testing.T) {
TagSets: []config.TagSet{},
BgpDefinedSets: config.BgpDefinedSets{},
},
&config.DefinedSets{
PrefixSets: []config.PrefixSet{},
NeighborSets: []config.NeighborSet{
{
NeighborSetName: "allpeerset",
NeighborInfoList: []string{"10.10.0.1/32", "10.10.0.2/32"},
},
},
TagSets: []config.TagSet{},
BgpDefinedSets: config.BgpDefinedSets{},
},
[]*config.Statement{
{
Name: "kube_router_export_stmt0",
Expand Down Expand Up @@ -2195,6 +2267,10 @@ func Test_AddPolicies(t *testing.T) {
PrefixSet: "clusteripprefixset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
MatchNeighborSet: config.MatchNeighborSet{
NeighborSet: "allpeerset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
},
Actions: config.Actions{
RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
Expand Down Expand Up @@ -2280,6 +2356,17 @@ func Test_AddPolicies(t *testing.T) {
t.Error("unexpected external peer defined set")
}

allPeerDefinedSet, err := testcase.nrc.bgpServer.GetDefinedSet(table.DEFINED_TYPE_NEIGHBOR, "allpeerset")
if err != nil {
t.Fatalf("error validating defined sets: %v", err)
}

if !allPeerDefinedSet.Equal(testcase.allPeerDefinedSet) {
t.Logf("expected all peer defined set: %+v", testcase.allPeerDefinedSet.NeighborSets)
t.Logf("actual all peer defined set: %+v", allPeerDefinedSet.NeighborSets)
t.Error("unexpected all peer defined set")
}

checkPolicies(t, testcase, table.POLICY_DIRECTION_EXPORT, table.ROUTE_TYPE_REJECT, testcase.exportPolicyStatements)
checkPolicies(t, testcase, table.POLICY_DIRECTION_IMPORT, table.ROUTE_TYPE_ACCEPT, testcase.importPolicyStatements)
})
Expand Down

0 comments on commit 3aacd48

Please sign in to comment.