Skip to content
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.

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

BGP: Introduce new cli to get bgp peering state #1500

Merged
merged 1 commit into from
Apr 13, 2023

Conversation

harsimran-pabla
Copy link
Contributor

@harsimran-pabla harsimran-pabla commented Apr 11, 2023

This change introduces new BGP status CLI : cilium bgp peers. Status of BGP peers is returned from all or specific node in json or summary formats.

./cilium bgp peers --help
Gets BGP peering status from all nodes in the cluster

Usage:
  cilium bgp peers [flags]

Flags:
      --agent-pod-selector string   Label on cilium-agent pods to select with (default "k8s-app=cilium")
  -h, --help                        help for peers
      --node string                 Node from which BGP status will be fetched, omit to select all nodes
  -o, --output string               Output format. One of: json, summary (default "summary")
      --wait-duration duration      Maximum time to wait for result, default 1 minute (default 1m0s)

Signed-off-by: harsimran pabla [email protected]

@harsimran-pabla harsimran-pabla temporarily deployed to ci April 11, 2023 14:46 — with GitHub Actions Inactive
@harsimran-pabla
Copy link
Contributor Author

harsimran-pabla commented Apr 11, 2023

Example 1

Per node output :

> ./cilium bgp peers --node clab-bgp-cplane-demo-worker3 -o json
{
 "clab-bgp-cplane-demo-worker3": [
  {
   "families": [
    {
     "accepted": 11,
     "advertised": 1,
     "afi": "ipv4",
     "received": 11,
     "safi": "unicast"
    },
    {
     "afi": "ipv6",
     "safi": "unicast"
    }
   ],
   "local-asn": 65011,
   "peer-address": "10.0.0.2",
   "peer-asn": 65011,
   "session-state": "established",
   "uptime-nanoseconds": 422509145342
  }
 ]
}

> ./cilium bgp peers --node clab-bgp-cplane-demo-worker3
Node                           Local AS   Peer AS   Peer Address   Session State   Uptime   Family         Received   Advertised
clab-bgp-cplane-demo-worker3   65011      65011     10.0.0.2       established     7m5s     ipv4/unicast   11         1
                                                                                            ipv6/unicast   0          0

@harsimran-pabla
Copy link
Contributor Author

Example 2

Output from 4 BGP nodes

 > ./cilium bgp peers
Node                                 Local AS   Peer AS   Peer Address   Session State   Uptime   Family         Received   Advertised
clab-bgp-cplane-demo-control-plane   65010      65010     10.0.0.1       established     6s       ipv4/unicast   11         1
                                                                                                  ipv6/unicast   0          0
clab-bgp-cplane-demo-worker          65010      65010     10.0.0.1       established     6s       ipv4/unicast   11         1
                                                                                                  ipv6/unicast   0          0
clab-bgp-cplane-demo-worker2         65011      65011     10.0.0.2       established     5s       ipv4/unicast   11         1
                                                                                                  ipv6/unicast   0          0
clab-bgp-cplane-demo-worker3         65011      65011     10.0.0.2       established     8s       ipv4/unicast   11         1
                                                                                                  ipv6/unicast   0          0

> ./cilium bgp peers -o json
{
 "clab-bgp-cplane-demo-control-plane": [
  {
   "families": [
    {
     "accepted": 11,
     "advertised": 1,
     "afi": "ipv4",
     "received": 11,
     "safi": "unicast"
    },
    {
     "afi": "ipv6",
     "safi": "unicast"
    }
   ],
   "local-asn": 65010,
   "peer-address": "10.0.0.1",
   "peer-asn": 65010,
   "session-state": "established",
   "uptime-nanoseconds": 15416378912
  }
 ],
 "clab-bgp-cplane-demo-worker": [
  {
   "families": [
    {
     "accepted": 11,
     "advertised": 1,
     "afi": "ipv4",
     "received": 11,
     "safi": "unicast"
    },
    {
     "afi": "ipv6",
     "safi": "unicast"
    }
   ],
   "local-asn": 65010,
   "peer-address": "10.0.0.1",
   "peer-asn": 65010,
   "session-state": "established",
   "uptime-nanoseconds": 15412779062
  }
 ],
 "clab-bgp-cplane-demo-worker2": [
  {
   "families": [
    {
     "accepted": 11,
     "advertised": 1,
     "afi": "ipv4",
     "received": 11,
     "safi": "unicast"
    },
    {
     "afi": "ipv6",
     "safi": "unicast"
    }
   ],
   "local-asn": 65011,
   "peer-address": "10.0.0.2",
   "peer-asn": 65011,
   "session-state": "established",
   "uptime-nanoseconds": 14422631014
  }
 ],
 "clab-bgp-cplane-demo-worker3": [
  {
   "families": [
    {
     "accepted": 11,
     "advertised": 1,
     "afi": "ipv4",
     "received": 11,
     "safi": "unicast"
    },
    {
     "afi": "ipv6",
     "safi": "unicast"
    }
   ],
   "local-asn": 65011,
   "peer-address": "10.0.0.2",
   "peer-asn": 65011,
   "session-state": "established",
   "uptime-nanoseconds": 17406343458
  }
 ]
}


@harsimran-pabla harsimran-pabla temporarily deployed to ci April 11, 2023 14:51 — with GitHub Actions Inactive
@harsimran-pabla harsimran-pabla marked this pull request as ready for review April 11, 2023 14:52
@harsimran-pabla harsimran-pabla requested a review from a team as a code owner April 11, 2023 14:52
@harsimran-pabla harsimran-pabla requested a review from asauber April 11, 2023 14:52
@harsimran-pabla harsimran-pabla requested review from a team and dylandreimerink and removed request for a team April 11, 2023 15:16
Copy link
Member

@asauber asauber left a comment

Choose a reason for hiding this comment

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

Very clean implementation, LGTM. Perhaps we want a test that would flag changes to the summary format.

@harsimran-pabla harsimran-pabla requested a review from a team as a code owner April 11, 2023 21:10
@harsimran-pabla harsimran-pabla temporarily deployed to ci April 11, 2023 21:10 — with GitHub Actions Inactive
@harsimran-pabla
Copy link
Contributor Author

@asauber Added test case to verify the format of summary output. Had to pull testify/require in vendor directory - so some more files are changed. Please take a look at latest commit for changes.

@harsimran-pabla harsimran-pabla temporarily deployed to ci April 11, 2023 21:15 — with GitHub Actions Inactive
@harsimran-pabla harsimran-pabla temporarily deployed to ci April 11, 2023 22:16 — with GitHub Actions Inactive
Copy link
Member

@asauber asauber left a comment

Choose a reason for hiding this comment

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

Thanks for the test. Looks great

Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Code changes LGTM.

Could you please also update CODEOWNERS to assign bgp/ to @cilium/sig-bgp as follows?

diff --git a/CODEOWNERS b/CODEOWNERS
index d9bb8f5f2155..1ad7dcb11d5f 100644
--- a/CODEOWNERS
+++ b/CODEOWNERS
@@ -22,6 +22,7 @@
 /.github/kind-config*.yaml @cilium/ci-structure
 /.github/tools/ @cilium/ci-structure
 /.github/workflows/ @cilium/github-sec @cilium/ci-structure
+/bpf/ @cilium/sig-bgp
 /cmd/ @cilium/cli
 /clustermesh/ @cilium/sig-clustermesh
 /connectivity/ @cilium/cli

This change introduces new BGP status CLI : 'cilium bgp peers'.
Status of BGP peers is returned from all or specific node in json or summary formats.

Signed-off-by: harsimran pabla <[email protected]>
@harsimran-pabla harsimran-pabla requested a review from a team as a code owner April 13, 2023 17:01
@harsimran-pabla harsimran-pabla temporarily deployed to ci April 13, 2023 17:01 — with GitHub Actions Inactive
Copy link
Contributor

@ldelossa ldelossa left a comment

Choose a reason for hiding this comment

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

LGTM

@harsimran-pabla harsimran-pabla added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Apr 13, 2023
@tklauser tklauser merged commit ac9bcfa into cilium:master Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants