Consolidate Node Peer Annotations

[catherinetcai]

Adds support for peer configuration that's in YAML format according to: https://github.com/cloudnativelabs/kube-router/issues/1393

I consolidated the peer configurations from both the consolidated annotation and single annotations into populating a single config struct.

Tested this by spinning up a cluster using https://github.com/aauren/kube-router-automation and adding the following annotations onto the aws-controller and aws-worker nodes and then running kubectl exec into the pods to validate that the BGP configurations were picked up.

apiVersion: v1
kind: Node
metadata:
  name: aws-controller
  annotations:
    kube-router.io/peer.ips: "10.95.0.254"
    kube-router.io/peer.asns: "4200000001"
    kube-router.io/peer.localips: "192.168.1.0"

apiVersion: v1
kind: Node
metadata:
  name: aws-worker
  annotations:
    kube-router.io/peers: |
      - remoteip: 10.0.0.1
        remoteasn: 64640
        password: cGFzc3dvcmQ=
        localip: 192.168.0.1

I'm going to keep this MR in the draft state until I'm able to do a more thorough testing cycle with kubetest2.

@aauren

[catherinetcai]

@catherinetcai Thanks for starting work on this!

Most of my comments are nits, there are no major problems that I can see with this work. I like the fact that you pulled out all of the BGP parsing stuff into functions on a struct. I think that's already going a long ways towards making the code more readable.

The other main thing I was looking for was keeping backwards compatibility, but it looks like you've done that pretty well.

The configuration structure itself looks pretty good, although I'm still mulling that over a bit to see if there would be some way to make some of the items less duplicated. For instance, people are likely to have similar Password, RemoteASN, and RemoteIP for many of the peers. Maybe it would be possible to define a global default that people could specify once, and then allow individual entries to override that default when necessary?

Or maybe we could introduce a concept of peer groups? https://github.com/osrg/gobgp/blob/master/docs/sources/configuration.md?plain=1#L175-L187 which would follow more of the gobgp standard?

FRR has this standard as well: https://github.com/aauren/kube-router-automation/blob/main/ansible/playbooks/roles/bgp_router/templates/frr.conf.j2#L36-L42

I really like the idea of following more of the GoBGP standard. Do you have any thoughts for how those should be passed in?

[aauren]

@catherinetcai Thanks for starting work on this! Most of my comments are nits, there are no major problems that I can see with this work. I like the fact that you pulled out all of the BGP parsing stuff into functions on a struct. I think that's already going a long ways towards making the code more readable. The other main thing I was looking for was keeping backwards compatibility, but it looks like you've done that pretty well. The configuration structure itself looks pretty good, although I'm still mulling that over a bit to see if there would be some way to make some of the items less duplicated. For instance, people are likely to have similar Password, RemoteASN, and RemoteIP for many of the peers. Maybe it would be possible to define a global default that people could specify once, and then allow individual entries to override that default when necessary? Or maybe we could introduce a concept of peer groups? https://github.com/osrg/gobgp/blob/master/docs/sources/configuration.md?plain=1#L175-L187 which would follow more of the gobgp standard? FRR has this standard as well: https://github.com/aauren/kube-router-automation/blob/main/ansible/playbooks/roles/bgp_router/templates/frr.conf.j2#L36-L42

I really like the idea of following more of the GoBGP standard. Do you have any thoughts for how those should be passed in?

Heh... I think that it felt more apparent to me when I hadn't thought it through all the way. I was thinking that you could just add a peerGroup to the annotation yaml and then reference it. But this is per-node not cluster wide. I suppose that we could do something like add a config file or allow users to add a group via a parameter to kube-router, but those all feel a bit like a hack.

I guess for now we leave it as it is. But maybe in the future, we add a CRD or something? This is similar to what Cilium does: https://docs.cilium.io/en/stable/network/bgp-control-plane/bgp-control-plane-v2 Although theirs is a bit different because they have nodeSelectors in their BGP config so that users can control node applications that way. Which I suppose is more k8s idiomatic.

[catherinetcai]

@aauren Rebased my changes against master so that there's no more merge conflicts between recent changes to network_routes_controller.go and go.mod/go.sum.

[catherinetcai]

@aauren Cleaned up the last nits and rebased against master for the go.mod/go.sum. Thank you!

[catherinetcai]

Closing in favor of https://github.com/cloudnativelabs/kube-router/pull/1972, which is the squashed version of this