Ability to enable/disable node advertising its pod CIDR to external BGP peers (#408)

* Node should advertise its pod CIDR to external BGP peers only if
--advertise-node-pod-cidr is set to true (defaults to true).
This is to enable a case where pod's remain non-routable from out of the
cluster but service VIP's can be routable from out side the cluster.

* fix unit test

* address review comments

Author: murali-reddy

docs/README.md

@@ -103,6 +103,7 @@ Usage of kube-router:
       --advertise-cluster-ip             Add Cluster IP of the service to the RIB so that it gets advertises to the BGP peers.
       --advertise-external-ip            Add External IP of service to the RIB so that it gets advertised to the BGP peers.
       --advertise-loadbalancer-ip        Add LoadbBalancer IP of service status as set by the LB provider to the RIB so that it gets advertised to the BGP peers.
+      --advertise-pod-cidr               Add Node's POD cidr to the RIB so that it gets advertised to the BGP peers. (default true)
       --bgp-graceful-restart             Enables the BGP Graceful Restart capability so that routes are preserved on unexpected restarts
       --cleanup-config                   Cleanup iptables rules, ipvs, ipset configuration and exit.
       --cluster-asn uint                 ASN number under which cluster nodes will run iBGP.

pkg/controllers/routing/export_policies.go

@@ -2,20 +2,27 @@ package routing
 
 import (
 	"errors"
+	"fmt"
 
 	"github.com/cloudnativelabs/kube-router/pkg/utils"
 	"github.com/osrg/gobgp/config"
 	"github.com/osrg/gobgp/table"
+	v1core "k8s.io/api/core/v1"
 )
 
-// Each node advertises its pod CIDR to the nodes with same ASN (iBGP peers) and to the global BGP peer
-// or per node BGP peer. Each node ends up advertising not only pod CIDR assigned to the self but other
-// learned routes to the node pod CIDR's as well to global BGP peer or per node BGP peers. external BGP
-// peer will randomly (since all path have equal selection attributes) select the routes from multiple
-// routes to a pod CIDR which will result in extra hop. To prevent this behaviour this methods add
-// defult export policy to reject everything and an explicit policy is added so that each node only
-// advertised the pod CIDR assigned to it. Additionally export policy is added so that each node
-// advertises cluster IP's ONLY to the external BGP peers (and not to iBGP peers).
+// BGP export policies are added so that following conditions are met
+//
+// - by default export of all routes from the RIB to the neighbour's is denied, and explicity statements are added i
+//   to permit the desired routes to be exported
+// - each node is allowed to advertise its assigned pod CIDR's to all of its iBGP peer neighbours with same ASN
+// - each node is allowed to advertise its assigned pod CIDR's to all of its external BGP peer neighbours
+//   only if --advertise-pod-cidr flag is set to true
+// - each node is NOT allowed to advertise its assigned pod CIDR's to all of its external BGP peer neighbours
+//   only if --advertise-pod-cidr flag is set to false
+// - each node is allowed to advertise service VIP's (cluster ip, load balancer ip, external IP) ONLY to external
+//   BGP peers
+// - each node is NOT allowed to advertise service VIP's (cluster ip, load balancer ip, external IP) to
+//   iBGP peers
 func (nrc *NetworkRoutingController) addExportPolicies() error {
 
 	// we are rr server do not add export policies
@@ -59,13 +66,35 @@ func (nrc *NetworkRoutingController) addExportPolicies() error {
 
 	statements := make([]config.Statement, 0)
 
+	// 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)
+		if err != nil {
+			return fmt.Errorf("Failed to find a node IP: %s", err)
+		}
+		iBGPPeers = append(iBGPPeers, nodeIP.String())
+	}
+	iBGPPeerNS, _ := table.NewNeighborSet(config.NeighborSet{
+		NeighborSetName:  "iBGPpeerset",
+		NeighborInfoList: iBGPPeers,
+	})
+	err = nrc.bgpServer.ReplaceDefinedSet(iBGPPeerNS)
+	if err != nil {
+		nrc.bgpServer.AddDefinedSet(iBGPPeerNS)
+	}
 	// statement to represent the export policy to permit advertising node's pod CIDR
 	statements = append(statements,
 		config.Statement{
 			Conditions: config.Conditions{
 				MatchPrefixSet: config.MatchPrefixSet{
 					PrefixSet: "podcidrprefixset",
 				},
+				MatchNeighborSet: config.MatchNeighborSet{
+					NeighborSet: "iBGPpeerset",
+				},
 			},
 			Actions: config.Actions{
 				RouteDisposition: config.ROUTE_DISPOSITION_ACCEPT_ROUTE,
@@ -107,6 +136,21 @@ func (nrc *NetworkRoutingController) addExportPolicies() error {
 				RouteDisposition: config.ROUTE_DISPOSITION_ACCEPT_ROUTE,
 			},
 		})
+		if nrc.advertisePodCidr {
+			statements = append(statements, config.Statement{
+				Conditions: config.Conditions{
+					MatchPrefixSet: config.MatchPrefixSet{
+						PrefixSet: "podcidrprefixset",
+					},
+					MatchNeighborSet: config.MatchNeighborSet{
+						NeighborSet: "externalpeerset",
+					},
+				},
+				Actions: config.Actions{
+					RouteDisposition: config.ROUTE_DISPOSITION_ACCEPT_ROUTE,
+				},
+			})
+		}
 	}
 
 	definition := config.PolicyDefinition{

pkg/controllers/routing/network_routes_controller.go

@@ -66,6 +66,7 @@ type NetworkRoutingController struct {
 	advertiseClusterIP      bool
 	advertiseExternalIP     bool
 	advertiseLoadBalancerIP bool
+	advertisePodCidr        bool
 	defaultNodeAsnNumber    uint32
 	nodeAsnNumber           uint32
 	globalPeerRouters       []*config.NeighborConfig
@@ -753,6 +754,7 @@ func NewNetworkRoutingController(clientset kubernetes.Interface,
 	nrc.advertiseClusterIP = kubeRouterConfig.AdvertiseClusterIp
 	nrc.advertiseExternalIP = kubeRouterConfig.AdvertiseExternalIp
 	nrc.advertiseLoadBalancerIP = kubeRouterConfig.AdvertiseLoadBalancerIp
+	nrc.advertisePodCidr = kubeRouterConfig.AdvertiseNodePodCidr
 
 	nrc.enableOverlays = kubeRouterConfig.EnableOverlay
 

pkg/controllers/routing/network_routes_controller_test.go

@@ -1236,6 +1236,10 @@ func Test_addExportPolicies(t *testing.T) {
 							PrefixSet:       "podcidrprefixset",
 							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
 						},
+						MatchNeighborSet: config.MatchNeighborSet{
+							NeighborSet:     "iBGPpeerset",
+							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
+						},
 					},
 					Actions: config.Actions{
 						RouteDisposition: config.ROUTE_DISPOSITION_ACCEPT_ROUTE,
@@ -1350,6 +1354,10 @@ func Test_addExportPolicies(t *testing.T) {
 							PrefixSet:       "podcidrprefixset",
 							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
 						},
+						MatchNeighborSet: config.MatchNeighborSet{
+							NeighborSet:     "iBGPpeerset",
+							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
+						},
 					},
 					Actions: config.Actions{
 						RouteDisposition: config.ROUTE_DISPOSITION_ACCEPT_ROUTE,
@@ -1408,6 +1416,9 @@ func Test_addExportPolicies(t *testing.T) {
 			testcase.nrc.advertiseExternalIP = true
 			testcase.nrc.advertiseLoadBalancerIP = false
 
+			informerFactory := informers.NewSharedInformerFactory(testcase.nrc.clientset, 0)
+			nodeInformer := informerFactory.Core().V1().Nodes().Informer()
+			testcase.nrc.nodeLister = nodeInformer.GetIndexer()
 			err = testcase.nrc.addExportPolicies()
 			if !reflect.DeepEqual(err, testcase.err) {
 				t.Logf("expected err %v", testcase.err)

pkg/options/options.go

@@ -10,6 +10,7 @@ import (
 type KubeRouterConfig struct {
 	AdvertiseClusterIp      bool
 	AdvertiseExternalIp     bool
+	AdvertiseNodePodCidr    bool
 	AdvertiseLoadBalancerIp bool
 	BGPGracefulRestart      bool
 	CleanupConfig           bool
@@ -90,6 +91,8 @@ func (s *KubeRouterConfig) AddFlags(fs *pflag.FlagSet) {
 		"Add External IP of service to the RIB so that it gets advertised to the BGP peers.")
 	fs.BoolVar(&s.AdvertiseLoadBalancerIp, "advertise-loadbalancer-ip", false,
 		"Add LoadbBalancer IP of service status as set by the LB provider to the RIB so that it gets advertised to the BGP peers.")
+	fs.BoolVar(&s.AdvertiseNodePodCidr, "advertise-pod-cidr", true,
+		"Add Node's POD cidr to the RIB so that it gets advertised to the BGP peers.")
 	fs.IPSliceVar(&s.PeerRouters, "peer-router-ips", s.PeerRouters,
 		"The ip address of the external router to which all nodes will peer and advertise the cluster ip and pod cidr's.")
 	fs.UintVar(&s.ClusterAsn, "cluster-asn", s.ClusterAsn,