From e6a5385e5b74708f47bf353cf7382151f1fc5926 Mon Sep 17 00:00:00 2001 From: mhmxs Date: Fri, 12 Jun 2020 12:48:21 +0200 Subject: [PATCH] Scale BGP peers --- README.md | 2 + config/rbac/role.yaml | 1 + .../routereflectorconfig_controller.go | 34 ++++----- main.go | 3 + topologies/multi.go | 70 ++++++++++--------- topologies/single.go | 10 +-- topologies/topology.go | 4 +- 7 files changed, 67 insertions(+), 57 deletions(-) diff --git a/README.md b/README.md index 03928f6..b8ca8c1 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,8 @@ During the `api/core/v1/Node` reconcile phases it calculates the right number of This is a standard Kubebuilder opertor so building and deploying process is similar as a [stock Kubebuilder project](https://book.kubebuilder.io/cronjob-tutorial/running.html). +After first reconcile phase is done don not forget to disable the [node-to-node mesh](https://docs.projectcalico.org/getting-started/kubernetes/hardway/configure-bgp-peering)! + Use latest release: ``` kustomize build config/crd | kubectl apply -f - diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index bd0ee52..ea5f096 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -21,6 +21,7 @@ rules: - bgppeers verbs: - create + - delete - get - list - update diff --git a/controllers/routereflectorconfig_controller.go b/controllers/routereflectorconfig_controller.go index c83f741..287b840 100644 --- a/controllers/routereflectorconfig_controller.go +++ b/controllers/routereflectorconfig_controller.go @@ -77,7 +77,7 @@ type reconcileImplClient interface { // +kubebuilder:rbac:groups=route-reflector.calico-route-reflector-operator.mhmxs.github.com,resources=routereflectorconfigs,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=route-reflector.calico-route-reflector-operator.mhmxs.github.com,resources=routereflectorconfigs/status,verbs=get;update;patch // +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;update;watch -// +kubebuilder:rbac:groups="crd.projectcalico.org",resources=bgppeers,verbs=get;list;create;update +// +kubebuilder:rbac:groups="crd.projectcalico.org",resources=bgppeers,verbs=get;list;create;update;delete func (r *RouteReflectorConfigReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { _ = r.Log.WithValues("routereflectorconfig", req.Name) @@ -108,13 +108,14 @@ func (r *RouteReflectorConfigReconciler) Reconcile(req ctrl.Request) (ctrl.Resul log.Errorf("Unable to list nodes because of %s", err.Error()) return nodeListError, err } + log.Debugf("Total number of nodes %d", len(nodeList.Items)) - readyNodes, actualReadyNumber, nodes := r.collectNodeInfo(nodeList.Items) + readyNodes, actualRRNumber, nodes := r.collectNodeInfo(nodeList.Items) log.Infof("Nodes are ready %d", readyNodes) - log.Infof("Actual number of healthy route reflector nodes are %d", actualReadyNumber) + log.Infof("Actual number of healthy route reflector nodes are %d", actualRRNumber) - expectedNumber := r.Topology.CalculateExpectedNumber(readyNodes) - log.Infof("Expected number of route reflector nodes are %d", expectedNumber) + expectedRRNumber := r.Topology.CalculateExpectedNumber(readyNodes) + log.Infof("Expected number of route reflector nodes are %d", expectedRRNumber) for n, isReady := range nodes { if status, ok := routeReflectorsUnderOperation[n.GetUID()]; ok { @@ -141,26 +142,27 @@ func (r *RouteReflectorConfigReconciler) Reconcile(req ctrl.Request) (ctrl.Resul return nodeReverted, nil } - if !isReady || expectedNumber == actualReadyNumber { + if !isReady || expectedRRNumber == actualRRNumber { continue } - if diff := expectedNumber - actualReadyNumber; diff != 0 { + if diff := expectedRRNumber - actualRRNumber; diff != 0 { if updated, err := r.updateRRStatus(n, diff); err != nil { log.Errorf("Unable to update node %s because of %s", n.GetName(), err.Error()) return nodeUpdateError, err } else if updated && diff > 0 { - actualReadyNumber++ + actualRRNumber++ } else if updated && diff < 0 { - actualReadyNumber-- + actualRRNumber-- } } } - if expectedNumber != actualReadyNumber { - log.Infof("Actual number %d is different than expected %d", actualReadyNumber, expectedNumber) + if expectedRRNumber != actualRRNumber { + log.Errorf("Actual number %d is different than expected %d", actualRRNumber, expectedRRNumber) } + // TODO This has several performance issue, need to fix them rrLables := client.HasLabels{r.NodeLabelKey} rrListOptions := client.ListOptions{} rrLables.ApplyToList(&rrListOptions) @@ -171,7 +173,6 @@ func (r *RouteReflectorConfigReconciler) Reconcile(req ctrl.Request) (ctrl.Resul log.Errorf("Unable to list route reflectors because of %s", err.Error()) return rrListError, err } - log.Debugf("Route reflectors are: %v", rrList.Items) existingBGPPeers, err := r.BGPPeer.ListBGPPeers() @@ -183,7 +184,6 @@ func (r *RouteReflectorConfigReconciler) Reconcile(req ctrl.Request) (ctrl.Resul log.Debugf("Existing BGPeers are: %v", existingBGPPeers.Items) currentBGPPeers := r.Topology.GenerateBGPPeers(rrList.Items, nodes, existingBGPPeers) - log.Debugf("Current BGPeers are: %v", currentBGPPeers) for _, bp := range currentBGPPeers { @@ -194,7 +194,7 @@ func (r *RouteReflectorConfigReconciler) Reconcile(req ctrl.Request) (ctrl.Resul } for _, p := range existingBGPPeers.Items { - if !findBGPPeer(p.GetName(), currentBGPPeers) { + if !findBGPPeer(currentBGPPeers, p.GetName()) { log.Debugf("Removing BGPPeer: %s", p.GetName()) if err := r.BGPPeer.RemoveBGPPeer(&p); err != nil { log.Errorf("Unable to remove BGPPeer because of %s", err.Error()) @@ -259,10 +259,10 @@ func (r *RouteReflectorConfigReconciler) updateRRStatus(node *corev1.Node, diff func (r *RouteReflectorConfigReconciler) collectNodeInfo(allNodes []corev1.Node) (readyNodes int, actualReadyNumber int, filtered map[*corev1.Node]bool) { filtered = map[*corev1.Node]bool{} - for _, n := range allNodes { + for i, n := range allNodes { isReady := isNodeReady(&n) isSchedulable := isNodeSchedulable(&n) - filtered[&n] = isReady && isSchedulable + filtered[&allNodes[i]] = isReady && isSchedulable if isReady && isSchedulable { readyNodes++ if r.Topology.IsRouteReflector(string(n.GetUID()), n.GetLabels()) { @@ -291,7 +291,7 @@ func isNodeSchedulable(node *corev1.Node) bool { return true } -func findBGPPeer(name string, peers []calicoApi.BGPPeer) bool { +func findBGPPeer(peers []calicoApi.BGPPeer, name string) bool { for _, p := range peers { if p.GetName() == name { return true diff --git a/main.go b/main.go index 1acde3b..5d87ac5 100644 --- a/main.go +++ b/main.go @@ -126,6 +126,8 @@ func main() { Max: max, Ration: ratio, } + log.Infof("Topology config: %v", topologyConfig) + var topology topologies.Topology // TODO Validation on topology if t, ok := os.LookupEnv("ROUTE_REFLECTOR_TOPOLOGY"); ok && t == "multi" { @@ -173,6 +175,7 @@ func main() { } } +// TODO more sophisticated env parse and validation or use CRD func parseEnv() (int, int, string, float64, string, string, string) { var err error clusterID := defaultClusterID diff --git a/topologies/multi.go b/topologies/multi.go index 9122ae1..d247f32 100644 --- a/topologies/multi.go +++ b/topologies/multi.go @@ -17,7 +17,7 @@ package topologies import ( "fmt" - "math/rand" + "math" "strconv" calicoApi "github.com/projectcalico/libcalico-go/lib/apis/v3" @@ -34,8 +34,8 @@ type MultiTopology struct { } func (t *MultiTopology) IsRouteReflector(nodeID string, labels map[string]string) bool { - label, ok := labels[t.NodeLabelKey] - return ok && label == t.getNodeLabel(nodeID) + _, ok := labels[t.NodeLabelKey] + return ok } func (t *MultiTopology) GetClusterID(nodeID string) string { @@ -57,42 +57,39 @@ func (t *MultiTopology) CalculateExpectedNumber(readyNodes int) int { func (t *MultiTopology) GenerateBGPPeers(routeReflectors []corev1.Node, nodes map[*corev1.Node]bool, existingPeers *calicoApi.BGPPeerList) []calicoApi.BGPPeer { bgpPeerConfigs := []calicoApi.BGPPeer{} - for n, isReady := range nodes { - if !isReady { - continue + rrConfig := findBGPPeer(existingPeers.Items, DefaultRouteReflectorMeshName) + if rrConfig == nil { + rrConfig = &calicoApi.BGPPeer{ + TypeMeta: metav1.TypeMeta{ + Kind: calicoApi.KindBGPPeer, + APIVersion: calicoApi.GroupVersionCurrent, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultRouteReflectorMeshName, + }, } + } + selector := fmt.Sprintf("has(%s)", t.NodeLabelKey) + rrConfig.Spec = calicoApi.BGPPeerSpec{ + NodeSelector: selector, + PeerSelector: selector, + } - if t.IsRouteReflector(string(n.GetUID()), n.GetLabels()) { - selector := fmt.Sprintf("has(%s)", t.NodeLabelKey) - rrConfig := findBGPPeer(DefaultRouteReflectorMeshName, existingPeers) - if rrConfig == nil { - rrConfig = &calicoApi.BGPPeer{ - TypeMeta: metav1.TypeMeta{ - Kind: calicoApi.KindBGPPeer, - APIVersion: calicoApi.GroupVersionCurrent, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: DefaultRouteReflectorMeshName, - }, - } - } - rrConfig.Spec = calicoApi.BGPPeerSpec{ - NodeSelector: "!" + selector, - PeerSelector: selector, - } - - bgpPeerConfigs = append(bgpPeerConfigs, *rrConfig) + bgpPeerConfigs = append(bgpPeerConfigs, *rrConfig) + // TODO this could cause rebalancing very ofthen so has performance issues + rrIndex := 0 + for n, isReady := range nodes { + if !isReady || t.IsRouteReflector(string(n.GetUID()), n.GetLabels()) { continue } - // TODO Do it in a more sophisticaged way - for i := 1; i <= 3; i++ { - rrID := rand.Intn(len(routeReflectors)) - name := fmt.Sprintf(DefaultRouteReflectorClientName, rrID) - rr := getRouteReflectorID(string(routeReflectors[rrID].GetUID())) + for i := 1; i <= int(math.Min(float64(len(routeReflectors)), 3)); i++ { + rr := routeReflectors[rrIndex] + rrID := getRouteReflectorID(string(rr.GetUID())) + name := fmt.Sprintf(DefaultRouteReflectorClientName+"-%s", rrID, n.GetUID()) - clientConfig := findBGPPeer(DefaultRouteReflectorMeshName, existingPeers) + clientConfig := findBGPPeer(existingPeers.Items, name) if clientConfig == nil { clientConfig = &calicoApi.BGPPeer{ TypeMeta: metav1.TypeMeta{ @@ -105,10 +102,17 @@ func (t *MultiTopology) GenerateBGPPeers(routeReflectors []corev1.Node, nodes ma } } clientConfig.Spec = calicoApi.BGPPeerSpec{ - PeerSelector: fmt.Sprintf("%s=='%d'", t.NodeLabelKey, rr), + // TODO make configurable + NodeSelector: fmt.Sprintf("kubernetes.io/hostname=='%s'", n.GetLabels()["kubernetes.io/hostname"]), + PeerSelector: fmt.Sprintf("%s=='%d'", t.NodeLabelKey, rrID), } bgpPeerConfigs = append(bgpPeerConfigs, *clientConfig) + + rrIndex++ + if rrIndex == len(routeReflectors) { + rrIndex = 0 + } } } diff --git a/topologies/single.go b/topologies/single.go index 36cfd83..3e59e78 100644 --- a/topologies/single.go +++ b/topologies/single.go @@ -76,9 +76,8 @@ func (t *SingleTopology) CalculateExpectedNumber(readyNodes int) int { func (t *SingleTopology) GenerateBGPPeers(_ []corev1.Node, _ map[*corev1.Node]bool, existingPeers *calicoApi.BGPPeerList) []calicoApi.BGPPeer { bgpPeerConfigs := []calicoApi.BGPPeer{} - selector := fmt.Sprintf("has(%s)", t.NodeLabelKey) - - rrConfig := findBGPPeer(DefaultRouteReflectorMeshName, existingPeers) + // TODO eliminate code duplication + rrConfig := findBGPPeer(existingPeers.Items, DefaultRouteReflectorMeshName) if rrConfig == nil { rrConfig = &calicoApi.BGPPeer{ TypeMeta: metav1.TypeMeta{ @@ -90,8 +89,9 @@ func (t *SingleTopology) GenerateBGPPeers(_ []corev1.Node, _ map[*corev1.Node]bo }, } } + selector := fmt.Sprintf("has(%s)", t.NodeLabelKey) rrConfig.Spec = calicoApi.BGPPeerSpec{ - NodeSelector: "!" + selector, + NodeSelector: selector, PeerSelector: selector, } @@ -99,7 +99,7 @@ func (t *SingleTopology) GenerateBGPPeers(_ []corev1.Node, _ map[*corev1.Node]bo clientConfigName := fmt.Sprintf(DefaultRouteReflectorClientName, 1) - clientConfig := findBGPPeer(clientConfigName, existingPeers) + clientConfig := findBGPPeer(existingPeers.Items, clientConfigName) if clientConfig == nil { clientConfig = &calicoApi.BGPPeer{ TypeMeta: metav1.TypeMeta{ diff --git a/topologies/topology.go b/topologies/topology.go index fd25fed..364b72f 100644 --- a/topologies/topology.go +++ b/topologies/topology.go @@ -49,8 +49,8 @@ type Config struct { Ration float64 } -func findBGPPeer(name string, peers *calicoApi.BGPPeerList) *calicoApi.BGPPeer { - for _, p := range peers.Items { +func findBGPPeer(peers []calicoApi.BGPPeer, name string) *calicoApi.BGPPeer { + for _, p := range peers { if p.GetName() == name { return &p }