Added support for custom BGP ports with 179 still being default (#492) (#493)

* Introduced new cmdline flag --bgp-port, which controls BGP Server listening port and remote port of in-cluster node peers

    * Introduced new cmdline flag --peer-router-ports, which controls remote BGP port for external peers

    * Introduced new node annotation kube-router.io/peer.ports with same effect as --peer-router-ports

Author: johanot

docs/user-guide.md

@@ -31,6 +31,7 @@ Usage of kube-router:
       --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
+      --bgp-port uint16                  The port open for incoming BGP connections and to use for connecting with other BGP peers. (default 179)
       --cache-sync-timeout duration      The timeout for cache synchronization (e.g. '5s', '1m'). Must be greater than 0. (default 1m0s)
       --cleanup-config                   Cleanup iptables rules, ipvs, ipset configuration and exit.
       --cluster-asn uint                 ASN number under which cluster nodes will run iBGP.
@@ -58,6 +59,7 @@ Usage of kube-router:
       --peer-router-ips ipSlice          The ip address of the external router to which all nodes will peer and advertise the cluster ip and pod cidr's. (default [])
       --peer-router-multihop-ttl uint8   Enable eBGP multihop supports -- sets multihop-ttl. (Relevant only if ttl >= 2)
       --peer-router-passwords strings    Password for authenticating against the BGP peer defined with "--peer-router-ips".
+      --peer-router-ports uints          The remote port of the external BGP to which all nodes will peer. If not set, default BGP port (179) will be used. (default [])
       --routes-sync-period duration      The delay between route updates and advertisements (e.g. '5s', '1m', '2h22m'). Must be greater than 0. (default 5m0s)
       --run-firewall                     Enables Network Policy -- sets up iptables to provide ingress firewall for pods. (default true)
       --run-router                       Enables Pod Networking -- Advertises and learns the routes to Pods via iBGP. (default true)

pkg/controllers/routing/bgp_peers.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/cloudnativelabs/kube-router/pkg/metrics"
+	"github.com/cloudnativelabs/kube-router/pkg/options"
 	"github.com/cloudnativelabs/kube-router/pkg/utils"
 	"github.com/golang/glog"
 	"github.com/osrg/gobgp/config"
@@ -90,6 +91,11 @@ func (nrc *NetworkRoutingController) syncInternalPeers() {
 				NeighborAddress: nodeIP.String(),
 				PeerAs:          nrc.nodeAsnNumber,
 			},
+			Transport: config.Transport{
+				Config: config.TransportConfig{
+					RemotePort: nrc.bgpPort,
+				},
+			},
 		}
 
 		if nrc.bgpGracefulRestart {
@@ -173,11 +179,8 @@ func (nrc *NetworkRoutingController) syncInternalPeers() {
 }
 
 // connectToExternalBGPPeers adds all the configured eBGP peers (global or node specific) as neighbours
-func connectToExternalBGPPeers(server *gobgp.BgpServer, peerConfigs []*config.NeighborConfig, bgpGracefulRestart bool, peerMultihopTtl uint8) error {
-	for _, peerConfig := range peerConfigs {
-		n := &config.Neighbor{
-			Config: *peerConfig,
-		}
+func connectToExternalBGPPeers(server *gobgp.BgpServer, peerNeighbors []*config.Neighbor, bgpGracefulRestart bool, peerMultihopTtl uint8) error {
+	for _, n := range peerNeighbors {
 
 		if bgpGracefulRestart {
 			n.GracefulRestart = config.GracefulRestart{
@@ -216,6 +219,7 @@ func connectToExternalBGPPeers(server *gobgp.BgpServer, peerConfigs []*config.Ne
 			}
 		}
 		err := server.AddNeighbor(n)
+		peerConfig := n.Config
 		if err != nil {
 			return fmt.Errorf("Error peering with peer router "+
 				"%q due to: %s", peerConfig.NeighborAddress, err)
@@ -227,9 +231,9 @@ func connectToExternalBGPPeers(server *gobgp.BgpServer, peerConfigs []*config.Ne
 }
 
 // Does validation and returns neighbor configs
-func newGlobalPeers(ips []net.IP, asns []uint32, passwords []string) (
-	[]*config.NeighborConfig, error) {
-	peers := make([]*config.NeighborConfig, 0)
+func newGlobalPeers(ips []net.IP, ports []uint16, asns []uint32, passwords []string) (
+	[]*config.Neighbor, error) {
+	peers := make([]*config.Neighbor, 0)
 
 	// Validations
 	if len(ips) != len(asns) {
@@ -244,20 +248,39 @@ func newGlobalPeers(ips []net.IP, asns []uint32, passwords []string) (
 			"Example: \"pass,,pass\" OR [\"pass\",\"\",\"pass\"].")
 	}
 
+	if len(ips) != len(ports) && len(ports) != 0 {
+		return nil, errors.New("Invalid peer router config. " +
+			"The number of ports should either be zero, or one per peer router." +
+			" If blank items are used, it will default to standard BGP port, " +
+			strconv.Itoa(options.DEFAULT_BGP_PORT) + "\n" +
+			"Example: \"port,,port\" OR [\"port\",\"\",\"port\"].")
+	}
+
 	for i := 0; i < len(ips); i++ {
 		if !((asns[i] >= 64512 && asns[i] <= 65535) ||
 			(asns[i] >= 4200000000 && asns[i] <= 4294967294)) {
 			return nil, fmt.Errorf("Invalid ASN number \"%d\" for global BGP peer",
 				asns[i])
 		}
 
-		peer := &config.NeighborConfig{
-			NeighborAddress: ips[i].String(),
-			PeerAs:          asns[i],
+		peer := &config.Neighbor{
+			Config: config.NeighborConfig{
+				NeighborAddress: ips[i].String(),
+				PeerAs:          asns[i],
+			},
+			Transport: config.Transport{
+				Config: config.TransportConfig{
+					RemotePort: options.DEFAULT_BGP_PORT,
+				},
+			},
+		}
+
+		if len(ports) != 0 {
+			peer.Transport.Config.RemotePort = ports[i]
 		}
 
 		if len(passwords) != 0 {
-			peer.AuthPassword = passwords[i]
+			peer.Config.AuthPassword = passwords[i]
 		}
 
 		peers = append(peers, peer)

pkg/controllers/routing/export_policies.go

@@ -121,7 +121,7 @@ func (nrc *NetworkRoutingController) addExportPolicies() error {
 	externalBgpPeers := make([]string, 0)
 	if len(nrc.globalPeerRouters) != 0 {
 		for _, peer := range nrc.globalPeerRouters {
-			externalBgpPeers = append(externalBgpPeers, peer.NeighborAddress)
+			externalBgpPeers = append(externalBgpPeers, peer.Config.NeighborAddress)
 		}
 	}
 	if len(nrc.nodePeerRouters) != 0 {

pkg/controllers/routing/network_routes_controller.go

@@ -46,6 +46,7 @@ const (
 	peerASNAnnotation                 = "kube-router.io/peer.asns"
 	peerIPAnnotation                  = "kube-router.io/peer.ips"
 	peerPasswordAnnotation            = "kube-router.io/peer.passwords"
+	peerPortAnnotation                = "kube-router.io/peer.ports"
 	rrClientAnnotation                = "kube-router.io/rr.client"
 	rrServerAnnotation                = "kube-router.io/rr.server"
 	svcLocalAnnotation                = "kube-router.io/service.local"
@@ -73,7 +74,7 @@ type NetworkRoutingController struct {
 	advertisePodCidr        bool
 	defaultNodeAsnNumber    uint32
 	nodeAsnNumber           uint32
-	globalPeerRouters       []*config.NeighborConfig
+	globalPeerRouters       []*config.Neighbor
 	nodePeerRouters         []string
 	enableCNI               bool
 	bgpFullMeshMode         bool
@@ -84,6 +85,7 @@ type NetworkRoutingController struct {
 	peerMultihopTTL         uint8
 	MetricsEnabled          bool
 	bgpServerStarted        bool
+	bgpPort                 uint16
 	bgpRRClient             bool
 	bgpRRServer             bool
 	bgpClusterID            uint32
@@ -661,6 +663,7 @@ func (nrc *NetworkRoutingController) startBgpServer() error {
 			As:               nodeAsnNumber,
 			RouterId:         nrc.nodeIP.String(),
 			LocalAddressList: localAddressList,
+			Port:             int32(nrc.bgpPort),
 		},
 	}
 
@@ -700,6 +703,19 @@ func (nrc *NetworkRoutingController) startBgpServer() error {
 			return fmt.Errorf("Failed to parse node's Peer Addresses Annotation: %s", err)
 		}
 
+		// Get Global Peer Router ASN configs
+		nodeBgpPeerPortsAnnotation, ok := node.ObjectMeta.Annotations[peerPortAnnotation]
+		// Default to default BGP port if port annotation is not found
+		var peerPorts = make([]uint16, 0)
+		if ok {
+			portStrings := stringToSlice(nodeBgpPeerPortsAnnotation, ",")
+			peerPorts, err = stringSliceToUInt16(portStrings)
+			if err != nil {
+				nrc.bgpServer.Stop()
+				return fmt.Errorf("Failed to parse node's Peer Port Numbers Annotation: %s", err)
+			}
+		}
+
 		// Get Global Peer Router Password configs
 		var peerPasswords []string
 		nodeBGPPasswordsAnnotation, ok := node.ObjectMeta.Annotations[peerPasswordAnnotation]
@@ -715,7 +731,7 @@ func (nrc *NetworkRoutingController) startBgpServer() error {
 		}
 
 		// Create and set Global Peer Router complete configs
-		nrc.globalPeerRouters, err = newGlobalPeers(peerIPs, peerASNs, peerPasswords)
+		nrc.globalPeerRouters, err = newGlobalPeers(peerIPs, peerPorts, peerASNs, peerPasswords)
 		if err != nil {
 			nrc.bgpServer.Stop()
 			return fmt.Errorf("Failed to process Global Peer Router configs: %s", err)
@@ -820,12 +836,20 @@ func NewNetworkRoutingController(clientset kubernetes.Interface,
 	nrc.advertisePodCidr = kubeRouterConfig.AdvertiseNodePodCidr
 	nrc.enableOverlays = kubeRouterConfig.EnableOverlay
 
+	nrc.bgpPort = kubeRouterConfig.BGPPort
+
 	// Convert ints to uint32s
 	peerASNs := make([]uint32, 0)
 	for _, i := range kubeRouterConfig.PeerASNs {
 		peerASNs = append(peerASNs, uint32(i))
 	}
 
+	// Convert uints to uint16s
+	peerPorts := make([]uint16, 0)
+	for _, i := range kubeRouterConfig.PeerPorts {
+		peerPorts = append(peerPorts, uint16(i))
+	}
+
 	// Decode base64 passwords
 	peerPasswords := make([]string, 0)
 	if len(kubeRouterConfig.PeerPasswords) != 0 {
@@ -835,7 +859,7 @@ func NewNetworkRoutingController(clientset kubernetes.Interface,
 		}
 	}
 
-	nrc.globalPeerRouters, err = newGlobalPeers(kubeRouterConfig.PeerRouters,
+	nrc.globalPeerRouters, err = newGlobalPeers(kubeRouterConfig.PeerRouters, peerPorts,
 		peerASNs, peerPasswords)
 	if err != nil {
 		return nil, fmt.Errorf("Error processing Global Peer Router configs: %s", err)

pkg/controllers/routing/network_routes_controller_test.go

@@ -1258,12 +1258,12 @@ func Test_addExportPolicies(t *testing.T) {
 				bgpEnableInternal: true,
 				bgpServer:         gobgp.NewBgpServer(),
 				activeNodes:       make(map[string]bool),
-				globalPeerRouters: []*config.NeighborConfig{
+				globalPeerRouters: []*config.Neighbor{
 					{
-						NeighborAddress: "10.10.0.1",
+						Config: config.NeighborConfig{NeighborAddress: "10.10.0.1"},
 					},
 					{
-						NeighborAddress: "10.10.0.2",
+						Config: config.NeighborConfig{NeighborAddress: "10.10.0.2"},
 					},
 				},
 				nodeAsnNumber: 100,
@@ -1393,12 +1393,12 @@ func Test_addExportPolicies(t *testing.T) {
 				bgpEnableInternal: false,
 				bgpServer:         gobgp.NewBgpServer(),
 				activeNodes:       make(map[string]bool),
-				globalPeerRouters: []*config.NeighborConfig{
+				globalPeerRouters: []*config.Neighbor{
 					{
-						NeighborAddress: "10.10.0.1",
+						Config: config.NeighborConfig{NeighborAddress: "10.10.0.1"},
 					},
 					{
-						NeighborAddress: "10.10.0.2",
+						Config: config.NeighborConfig{NeighborAddress: "10.10.0.2"},
 					},
 				},
 				nodeAsnNumber: 100,
@@ -1515,12 +1515,12 @@ func Test_addExportPolicies(t *testing.T) {
 				pathPrependAS:     "65100",
 				bgpServer:         gobgp.NewBgpServer(),
 				activeNodes:       make(map[string]bool),
-				globalPeerRouters: []*config.NeighborConfig{
+				globalPeerRouters: []*config.Neighbor{
 					{
-						NeighborAddress: "10.10.0.1",
+						Config: config.NeighborConfig{NeighborAddress: "10.10.0.1"},
 					},
 					{
-						NeighborAddress: "10.10.0.2",
+						Config: config.NeighborConfig{NeighborAddress: "10.10.0.2"},
 					},
 				},
 				nodeAsnNumber: 100,
@@ -1658,12 +1658,12 @@ func Test_addExportPolicies(t *testing.T) {
 				pathPrependAS:     "65100",
 				bgpServer:         gobgp.NewBgpServer(),
 				activeNodes:       make(map[string]bool),
-				globalPeerRouters: []*config.NeighborConfig{
+				globalPeerRouters: []*config.Neighbor{
 					{
-						NeighborAddress: "10.10.0.1",
+						Config: config.NeighborConfig{NeighborAddress: "10.10.0.1"},
 					},
 					{
-						NeighborAddress: "10.10.0.2",
+						Config: config.NeighborConfig{NeighborAddress: "10.10.0.2"},
 					},
 				},
 				nodeAsnNumber: 100,

pkg/controllers/routing/utils.go

@@ -35,6 +35,18 @@ func stringSliceToIPs(s []string) ([]net.IP, error) {
 	return ips, nil
 }
 
+func stringSliceToUInt16(s []string) ([]uint16, error) {
+	ints := make([]uint16, 0)
+	for _, intString := range s {
+		newInt, err := strconv.ParseUint(intString, 0, 16)
+		if err != nil {
+			return nil, fmt.Errorf("Could not parse \"%s\" as an integer", intString)
+		}
+		ints = append(ints, uint16(newInt))
+	}
+	return ints, nil
+}
+
 func stringSliceToUInt32(s []string) ([]uint32, error) {
 	ints := make([]uint32, 0)
 	for _, intString := range s {

pkg/options/options.go

@@ -5,14 +5,18 @@ import (
 	"time"
 
 	"github.com/spf13/pflag"
+	"strconv"
 )
 
+const DEFAULT_BGP_PORT = 179
+
 type KubeRouterConfig struct {
 	AdvertiseClusterIp      bool
 	AdvertiseExternalIp     bool
 	AdvertiseNodePodCidr    bool
 	AdvertiseLoadBalancerIp bool
 	BGPGracefulRestart      bool
+	BGPPort                 uint16
 	CacheSyncTimeout        time.Duration
 	CleanupConfig           bool
 	ClusterAsn              uint
@@ -40,6 +44,7 @@ type KubeRouterConfig struct {
 	PeerASNs                []uint
 	PeerMultihopTtl         uint8
 	PeerPasswords           []string
+	PeerPorts               []uint
 	PeerRouters             []net.IP
 	RoutesSyncPeriod        time.Duration
 	RunFirewall             bool
@@ -101,6 +106,8 @@ func (s *KubeRouterConfig) AddFlags(fs *pflag.FlagSet) {
 		"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.UintSliceVar(&s.PeerPorts, "peer-router-ports", s.PeerPorts,
+		"The remote port of the external BGP to which all nodes will peer. If not set, default BGP port ("+strconv.Itoa(DEFAULT_BGP_PORT)+") will be used.")
 	fs.UintVar(&s.ClusterAsn, "cluster-asn", s.ClusterAsn,
 		"ASN number under which cluster nodes will run iBGP.")
 	fs.UintSliceVar(&s.PeerASNs, "peer-router-asns", s.PeerASNs,
@@ -111,6 +118,8 @@ func (s *KubeRouterConfig) AddFlags(fs *pflag.FlagSet) {
 		"Each node in the cluster will setup BGP peering with rest of the nodes.")
 	fs.BoolVar(&s.BGPGracefulRestart, "bgp-graceful-restart", false,
 		"Enables the BGP Graceful Restart capability so that routes are preserved on unexpected restarts")
+	fs.Uint16Var(&s.BGPPort, "bgp-port", DEFAULT_BGP_PORT,
+		"The port open for incoming BGP connections and to use for connecting with other BGP peers.")
 	fs.BoolVar(&s.EnableCNI, "enable-cni", true,
 		"Enable CNI plugin. Disable if you want to use kube-router features alongside another CNI plugin.")
 	fs.BoolVar(&s.EnableiBGP, "enable-ibgp", true,