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

bgpv2: container labs for various types of advertisements #32522

Merged
merged 5 commits into from
May 28, 2024

Conversation

harsimran-pabla
Copy link
Contributor

Adding container labs for local development and testing of BGPv2 APIs.

PR is split by BGP advertisement type

  • PodCIDR
  • Multi-homing
  • CiliumPodIPPool
  • Service types : Loadbalancer, cluster IP, external IP

@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 May 13, 2024
@harsimran-pabla harsimran-pabla added the release-note/misc This PR makes changes that have no direct user impact. label May 13, 2024
@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 May 13, 2024
@harsimran-pabla harsimran-pabla added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. area/bgp labels May 13, 2024
@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 May 13, 2024
@harsimran-pabla harsimran-pabla marked this pull request as ready for review May 15, 2024 18:18
@harsimran-pabla harsimran-pabla requested a review from a team as a code owner May 15, 2024 18:18
@harsimran-pabla harsimran-pabla requested review from bimmlerd, a team and danehans and removed request for a team May 15, 2024 18:18
Copy link
Member

@YutaroHayakawa YutaroHayakawa left a comment

Choose a reason for hiding this comment

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

Nice work! I think we can merge PodCIDR and Service labs into the single lab as their ContainerLab and Cilium configurations are the same.

contrib/containerlab/bgpv2/pod-cidr/Makefile Outdated Show resolved Hide resolved
Copy link
Member

@bimmlerd bimmlerd left a comment

Choose a reason for hiding this comment

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

I'm a bit lost as to what these are - could we add a bit of documentation somewhere?

contrib/containerlab/bgpv2/multi-homing/bgp.yaml Outdated Show resolved Hide resolved
contrib/containerlab/bgpv2/multi-homing/Makefile Outdated Show resolved Hide resolved
@harsimran-pabla harsimran-pabla removed the request for review from danehans May 16, 2024 12:36
@harsimran-pabla harsimran-pabla force-pushed the hpabla/bgp/v2/labs branch 2 times, most recently from 7b44ab4 to 6e1285f Compare May 16, 2024 19:00
@harsimran-pabla
Copy link
Contributor Author

harsimran-pabla commented May 16, 2024

Hi @bimmlerd

I'm a bit lost as to what these are - could we add a bit of documentation somewhere?

Sorry, I should have added readme section from the start. I added a new commit which contains description about each lab and prerequisite to get this working. c4ec47f

@harsimran-pabla
Copy link
Contributor Author

@YutaroHayakawa @bimmlerd Please have another look at this PR when you get a chance 🙏

Adding lab to demonstrate two BGP neighbors from same node to which pod
CIDR prefixes are advertised.

Signed-off-by: harsimran pabla <hpabla@isovalent.com>
Adding lab to demonstrate advertisement of CiliumPodIPPool using BGPv2
APIs.

Signed-off-by: harsimran pabla <hpabla@isovalent.com>
Adding labs to show service advertisements for load-balancer, external
IP and cluster IP using BGPv2 APIs. Also adding example of PodCIDR
advertisement, each using different NLRI attributes.

Signed-off-by: harsimran pabla <hpabla@isovalent.com>
Container lab directory contains only BGP specific labs. Assigning
directory code ownership to sig-bgp for time being.

Signed-off-by: harsimran pabla <hpabla@isovalent.com>
Adding readme files to provide pre-requisites required to run container
lab kind kubernetes cluster. As well as documentation about individual
lab.

Signed-off-by: harsimran pabla <hpabla@isovalent.com>
@harsimran-pabla
Copy link
Contributor Author

Not sure if a full test run is required for this change? There is no modification to the core Cilium code base, just new labs in contrib directory. @YutaroHayakawa @joestringer any concerns about adding ready-to-merge label manually, since basic pull request checks have passed?

@harsimran-pabla
Copy link
Contributor Author

/test

@joestringer
Copy link
Member

Making an exception since this is just going into contrib/.

@joestringer joestringer merged commit 67d32f3 into cilium:main May 28, 2024
62 of 64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bgp release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants