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

bgpv1: Avoid binding to ephemeral ports in tests #26209

Open
rastislavs opened this issue Jun 14, 2023 · 1 comment
Open

bgpv1: Avoid binding to ephemeral ports in tests #26209

rastislavs opened this issue Jun 14, 2023 · 1 comment
Labels
area/bgp area/CI Continuous Integration testing issue or flake kind/cleanup This includes no functional changes. pinned These issues are not marked stale by our issue bot.

Comments

@rastislavs
Copy link
Contributor

rastislavs commented Jun 14, 2023

The tests in:

  • pkg/bgpv1/test ("BGP component tests")
  • pkg/bgpv1/manager/reconcile_test.go ("Reconciler unit tests")

bind to ports above 1023 (in particular: 1790-1794). This can cause issues on some systems where these ports fall under the ephemeral port number range. Although it should be fine in most Linux distributions which usually default the ephemeral port range to 32768-60999, we should avoid this.

For the "BGP component tests":

  • We can use the IANA-assigned port for BGP (179) for all BGP instances in the test, given that they do not bind to ::179 / 0.0.0.0:179, but use distinct bind address (Global.ListenAddresses in GoBGP). That may require adding a new way of exposing this config for the Cilium agent's BGP instance, for example with a new annotation similar to the existing local-port one.

For the "Reconciler unit tests":

  • These tests can mock GoBGP so that binding is not really needed anymore,
  • or we can make the necessary tests privileged and used IANA-assigned port numbers - however, apart from the BGP one (179), we would need one more as the test exercises the change of the port.
@rastislavs rastislavs added area/CI Continuous Integration testing issue or flake kind/cleanup This includes no functional changes. area/bgp labels Jun 14, 2023
@github-actions
Copy link

This issue has been automatically marked as stale because it has not
had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Aug 14, 2023
@YutaroHayakawa YutaroHayakawa added pinned These issues are not marked stale by our issue bot. and removed stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. labels Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bgp area/CI Continuous Integration testing issue or flake kind/cleanup This includes no functional changes. pinned These issues are not marked stale by our issue bot.
Projects
None yet
Development

No branches or pull requests

2 participants