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

Support externalTrafficPolicy=Local for BGP CPlane service VIP advertisement #25477

Merged

Conversation

YutaroHayakawa
Copy link
Member

@YutaroHayakawa YutaroHayakawa commented May 16, 2023

Support externalTrafficPolicy=local for BGP CPlane service VIP advertisement. Please see the commit message for more details.

You can test this PR in your environment with below lab. Thanks to the recently added Chart CI Push workflow, you don't have to build images.

lab.tar.gz

flowchart TB
  Router0["Router0 (FRR)"]
  Router0---kind-control-plane["kind-control-plane (Cilium)"]
  Router0---kind-worker["kind-worker (Cilium)"]
$ tar xvf lab.tar.gz
$ cd lab
$ VERSION=1.14.0-dev-dev.55-HEAD-f00eeee626 make deploy

# Watch what's happening
$ watch -n 1 "docker exec -it clab-bgp-cplane-router0 vtysh -c 'show bgp all wide'"

# Make endpoints on one node empty. This should withdraw the route from kind-worker.
$ kubectl drain kind-worker --ignore-daemonsets

# Restore endpoints. This should restore the route as well.
$ kubectl uncordon kind-worker
Support externalTrafficPolicy=local for BGP CPlane service VIP advertisement

@YutaroHayakawa YutaroHayakawa added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label May 16, 2023
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/bgp-cplane-etp-local/poc-1 branch 3 times, most recently from f9863b5 to ab6d6b3 Compare May 17, 2023 04:06
@YutaroHayakawa YutaroHayakawa changed the title Support externalTrafficPolicy=local for BGP CPlane service VIP advertisement Support externalTrafficPolicy=Local for BGP CPlane service VIP advertisement May 17, 2023
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/bgp-cplane-etp-local/poc-1 branch 2 times, most recently from 0743a66 to e8a5a5c Compare May 17, 2023 10:45
pkg/bgpv1/manager/reconcile.go Outdated Show resolved Hide resolved
pkg/bgpv1/manager/reconcile.go Outdated Show resolved Hide resolved
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/bgp-cplane-etp-local/poc-1 branch from e8a5a5c to 42d3452 Compare May 19, 2023 08:59
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/bgp-cplane-etp-local/poc-1 branch from 42d3452 to 294bbe4 Compare May 29, 2023 10:36
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/bgp-cplane-etp-local/poc-1 branch 4 times, most recently from f766bcd to d0b8ffe Compare June 5, 2023 08:04
@YutaroHayakawa YutaroHayakawa marked this pull request as ready for review June 5, 2023 08:36
@YutaroHayakawa YutaroHayakawa requested a review from a team as a code owner June 5, 2023 08:36
@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa
Copy link
Member Author

YutaroHayakawa commented Jun 5, 2023

Cilium Runtime: Missing the Endpoints resource on the test Hive. I'll fix it.

@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/bgp-cplane-etp-local/poc-1 branch from d0b8ffe to f00eeee Compare June 5, 2023 09:08
Copy link
Member

@dylandreimerink dylandreimerink left a comment

Choose a reason for hiding this comment

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

🚀 LGTM!

@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa
Copy link
Member Author

Cilium L4LB XDP: Hitting complexity issue. Rebasing for pulling the fix (#25892) in.

The semantics of the externalTrafficPolicy=local for service
advertisement. When at least one active endpoint present, we advertise
the LB VIP, otherwise, stop advertisement.

We can track active endpoints by tracking the Endpoints or EndpointSlice
object. They contain the list of the endpoint IPs and nodes. We leverage
a new Resource[*Endpoints] for subscribing the changes. For an efficient
processing, we wrap it with DiffStore and do reconciliation against the
service affected by the endpoint changes.

On every reconciliation, we populate the structure called localServices
from DiffStore store which represents the information that services have
at least one available local endpoint or not.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
TestLBServiceReconciler is getting longer and now it's hard to follow
what's the diff between resources used in tests. Extract resource
definition out of the test case definition and deduplicate the fields as
much as possible.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/bgp-cplane-etp-local/poc-1 branch from f00eeee to 7e6facb Compare June 5, 2023 15:47
@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa
Copy link
Member Author

Cilium Runtime: Timeout while waiting for the image. Retrying...

@YutaroHayakawa
Copy link
Member Author

All green and reviews are in. Ready to merge.

@YutaroHayakawa YutaroHayakawa added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 6, 2023
@dylandreimerink dylandreimerink merged commit 7b36892 into cilium:main Jun 6, 2023
62 checks passed
mrwulf added a commit to mrwulf/home-cluster that referenced this pull request Jun 9, 2023
@pchaigno pchaigno added release-note/major This PR introduces major new functionality to Cilium. and removed release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bgp ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/major This PR introduces major new functionality to Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants