Consolidate Node Peer Annotations

[catherinetcai]

Adds support for peer configuration that's in YAML format according to: #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

[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

[aauren]

We should probably put a comment above these saying that they're deprecated or some such so that we don't have to carry them indefinitely.

We should also update the markdown docs with the same message

[catherinetcai]

Updated the markdown docs with the new annotation + examples.

[aauren]

We could probably consolidate this to make it just accept the peerCfg instead of piece-mealing it out like this.

[catherinetcai]

Hows this?

[aauren]

I've been slowly trying to pull functionality out of the monolith controllers. I would do that here. As a matter of fact, there is a relatively recent bgp package at the top pkg level now.

Probably the best thing to do would be to externalize this, struct and its related functions (although I think most of them already are) and then put them in that package to continue grouping BGP functionality together.

[catherinetcai]

Moved this into the bgp package. I also noticed that there's a lot of duplication with the validation logic for the BGP peer configs, so I decided to lift that out from the routing package as well and put it into the bgp package.

[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?