Skip to content

bgpv1: Add cilium-dbg bgp route-policies command & include it in the bugtool #28973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Nov 17, 2023
Merged

bgpv1: Add cilium-dbg bgp route-policies command & include it in the bugtool #28973

merged 5 commits into from
Nov 17, 2023

Conversation

rastislavs
Copy link
Contributor

@rastislavs rastislavs commented Nov 3, 2023 •
edited
Loading

Adds a new /bgp/route-policies API endpoint and the cilium-dbg bgp route-policies command that can be used for retrieval of BGP route policies configured in the BGP Control Plane's Router implementation. Also adds this command's output in the bugtool.

This is a debug-ability enhancement for the CiliumBGPPeeringPolicy's AdvertisedPathAttributes feature added by #27705
The command prints the route-policies generated internally by the BGP Control Plane - these are not directly configurable by the user.

Example usage:

$ # cilium bgp route-policies
VRouter   Policy Name                                   Type     Match Peers    Match Prefixes (Min..Max Len)   RIB Action   Path Actions
65001     172.0.0.1/32-CiliumLoadBalancerIPPool-32...   export   172.0.0.1/32   192.168.100.0/24 (32..32)       none         AddCommunities: [65001:100]                                       
65001     172.0.0.1/32-PodCIDR                          export   172.0.0.1/32   10.244.0.0/24 (24..24)          none         {SetLocalPreference: 150} {AddCommunities: [56564:1 65001:150]} 

$ cilium-dbg bgp route-policies -o yaml
- name: 172.0.0.1/32-CiliumLoadBalancerIPPool-325f8fc93e320b4bb1b1ea186c0f2d53e787175a3b5725c4be96cfd93fcf8f2e
  routerasn: 65001
  statements:
    - addcommunities:
        - 65001:100
      addlargecommunities: []
      matchneighbors:
        - 172.0.0.1/32
      matchprefixes:
        - cidr: 192.168.10.128/25
          prefixlenmax: 32
          prefixlenmin: 32
      routeaction: none
      setlocalpreference: -1
    - addcommunities:
        - 65001:100
      addlargecommunities: []
      matchneighbors:
        - 172.0.0.1/32
      matchprefixes:
        - cidr: 2004::/64
          prefixlenmax: 128
          prefixlenmin: 128
      routeaction: none
      setlocalpreference: -1
  type: export
- name: 172.0.0.1/32-PodCIDR
  routerasn: 65001
  statements:
    - addcommunities:
        - "56564:1"
        - 65001:150
      addlargecommunities: []
      matchneighbors:
        - 172.0.0.1/32
      matchprefixes:
        - cidr: 10.244.0.0/24
          prefixlenmax: 24
          prefixlenmin: 24
      routeaction: none
      setlocalpreference: 150
  type: export

Config used for this example:

Excerpt from CiliumBGPPeeringPolicy:

apiVersion: cilium.io/v2alpha1
kind: CiliumBGPPeeringPolicy
  name: tor
spec:
  virtualRouters:
  - exportPodCIDR: true
    localASN: 65001
    neighbors:
    - peerASN: 65000
      peerAddress: 172.0.0.1/32
      advertisedPathAttributes:
      - selectorType: CiliumLoadBalancerIPPool
        selector:
          matchLabels:
            environment: production
        communities:
          standard:
          - 65001:100
      - selectorType: PodCIDR
        communities:
          standard:
          - 65001:150
          - "56564:1"
        localPreference: 150

LB Pool:

apiVersion: cilium.io/v2alpha1
kind: CiliumLoadBalancerIPPool
metadata:
  labels:
    environment: production
  name: cilium-pool
spec:
  cidrs:
  - cidr: 192.168.10.128/25
  - cidr: 2004::0/64

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 3, 2023
@rastislavs rastislavs added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Nov 3, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 3, 2023
@rastislavs rastislavs added area/bgp Impacts the Border Gateway Protocol feature. kind/enhancement This would improve or streamline existing functionality. labels Nov 3, 2023
@rastislavs
Copy link
Contributor Author

/test

@rastislavs rastislavs marked this pull request as ready for review November 6, 2023 10:18
@rastislavs rastislavs requested review from a team as code owners November 6, 2023 10:18
qmonnet
qmonnet approved these changes Nov 6, 2023
View reviewed changes
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc changes (are minimal and) look good. I only gave a quick look at the implementation, didn't look at the details.
Thanks!

harsimran-pabla
harsimran-pabla requested changes Nov 6, 2023
View reviewed changes
Copy link
Contributor

@harsimran-pabla harsimran-pabla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change looks good, some minor comments.

@rastislavs rastislavs force-pushed the bgp-rp-cli branch 2 times, most recently from 2199fda to ff72849 Compare November 7, 2023 09:40
@rastislavs
Copy link
Contributor Author

/test

@rastislavs rastislavs mentioned this pull request Nov 7, 2023
Closed
@rastislavs
Copy link
Contributor Author

/test

harsimran-pabla
harsimran-pabla approved these changes Nov 7, 2023
View reviewed changes
Copy link
Contributor

@harsimran-pabla harsimran-pabla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, lgtm!

derailed
derailed reviewed Nov 7, 2023
View reviewed changes
Copy link
Contributor

@derailed derailed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rastislavs Nice work!

@rastislavs
Copy link
Contributor Author

/test

@rastislavs rastislavs requested a review from derailed November 9, 2023 08:08
derailed
derailed approved these changes Nov 9, 2023
View reviewed changes
Copy link
Contributor

@derailed derailed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rastislavs LG. Thanks for the updates!

@rastislavs
Copy link
Contributor Author

@nathanjsweet could you please take a look when you have some time? This PR now requires only the api review.

nathanjsweet
nathanjsweet approved these changes Nov 17, 2023
View reviewed changes
Copy link
Member

@nathanjsweet nathanjsweet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for API

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 17, 2023
@rastislavs
bgpv1: Add GetRoutePolicies Router method and implement it in GoBGP
6aed870 
Adds a new GetRoutePolicies() method for retrieving configured route policies
to the Router interface and implements it in the GoBGP Router implementation.

Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
@rastislavs
\api: Add a new API endpoint /bgp/route-policies
6910617 
Add a new API endpoint `/bgp/route-policies` to retrieve route
policies configured in the BGP Control Plane.

Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
@rastislavs
api,bgpv1: Implement /bgp/route-policies API in BGPRouterManager
5a0b930 
Implement `/bgp/route-policies` API by wiring-up API call with
BGPRouterManager and GetRoutePolicies API of the Router implementation.

Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
@rastislavs
cilium-dbg: Implement bgp route-policies command
6d88c7a 
Implement `cilium-dbg bgp route-policies` command that retrieves
route policies from the `/bgp/route-policies` API and displays the response.

Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
@rastislavs
bugtool: Include cilium-dbg bgp route-policies command output
7c65158 
Includes BGP route policies output in the bugtool to have this
BGP Control Plane information included in the sysdumps.

Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
@julianwiedmann julianwiedmann removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 17, 2023
@rastislavs
Copy link
Contributor Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 17, 2023
@julianwiedmann julianwiedmann merged commit 70ae8d0 into cilium:main Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derailed derailed derailed approved these changes

@nathanjsweet nathanjsweet nathanjsweet approved these changes

@qmonnet qmonnet qmonnet approved these changes

@harsimran-pabla harsimran-pabla harsimran-pabla approved these changes

Assignees
No one assigned
Labels
area/bgp Impacts the Border Gateway Protocol feature. kind/enhancement This would improve or streamline existing functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@rastislavs @derailed @nathanjsweet @qmonnet @harsimran-pabla @julianwiedmann