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

Egress Gateway: make CiliumEndpoint reconciliation asynchronous from k8s watcher #26741

Merged
merged 2 commits into from Jul 12, 2023

Conversation

jibi
Copy link
Member

@jibi jibi commented Jul 10, 2023

see individual commits

Fixes: #23976

@jibi jibi added release-note/misc This PR makes changes that have no direct user impact. feature/egress-gateway Impacts the egress IP gateway feature. sig/agent Cilium agent related. needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Jul 10, 2023
@jibi jibi requested review from aanm and julianwiedmann July 10, 2023 15:44
@jibi jibi requested review from a team as code owners July 10, 2023 15:44
@jibi jibi requested a review from kaworu July 10, 2023 15:44
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.0 Jul 10, 2023
@jibi jibi force-pushed the pr/jibi/egressgw-async-eps branch from 03f69b3 to 1f3beaa Compare July 10, 2023 15:55
@jibi jibi marked this pull request as draft July 10, 2023 17:32
@jibi jibi force-pushed the pr/jibi/egressgw-async-eps branch 3 times, most recently from e82b589 to 1c4eeca Compare July 10, 2023 22:05
@jibi
Copy link
Member Author

jibi commented Jul 10, 2023

/test

@jibi jibi marked this pull request as ready for review July 11, 2023 05:54
@jibi jibi requested a review from a team as a code owner July 11, 2023 05:54
@jibi jibi requested a review from qmonnet July 11, 2023 05:54
@jibi jibi added the affects/v1.13 This issue affects v1.13 branch label Jul 11, 2023
Copy link
Member

@kaworu kaworu left a comment

Choose a reason for hiding this comment

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

Helm changes LGTM.

Copy link
Member

@julianwiedmann julianwiedmann left a comment

Choose a reason for hiding this comment

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

Thank you! One CI-related comment below, would be good to understand if we have issues lurking there.

pkg/egressgateway/manager_privileged_test.go Show resolved Hide resolved
pkg/egressgateway/manager.go Show resolved Hide resolved
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.

One spelling nit, but looks good otherwise from my side

pkg/egressgateway/manager.go Outdated Show resolved Hide resolved
Documentation/spelling_wordlist.txt Outdated Show resolved Hide resolved
@jibi jibi force-pushed the pr/jibi/egressgw-async-eps branch 2 times, most recently from 1b55722 to cd2e032 Compare July 12, 2023 09:06
@qmonnet
Copy link
Member

qmonnet commented Jul 12, 2023

Doc workflow failure is my fault, should be fixed with #26780.

jibi added 2 commits July 12, 2023 12:35
instead of running the reconciliation logic for each event received by
the egress gateway manager, switch to a trigger based approach where
each event simply triggers a reconciliation, and the actual
reconciliation is be performed at most once every
--egress-gateway-reconciliation-trigger-interval time interval.

This should help reducing the CPU load in case the manager is dealing
with lots of events (for example high churn of endpoints)

Signed-off-by: Gilberto Bertin <jibi@cilium.io>
by:

* making locking more granular by introducing a new
  pendingEndpointEventsLock lock that protects accesses to the
  pendingEndpointEvents field
* using the endpoint workqueue to handle also CiliumEndpoint deletion
  events

we can completely decouple the On{Update,Delete}Endpoint methods from
the reconciliation logic, as the caller will not have to block anymore
on the main manager lock (which is held during the entire reconciliation
process)

Signed-off-by: Gilberto Bertin <jibi@cilium.io>
@jibi jibi force-pushed the pr/jibi/egressgw-async-eps branch from cd2e032 to 332cb45 Compare July 12, 2023 10:35
@jibi
Copy link
Member Author

jibi commented Jul 12, 2023

/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 Jul 12, 2023
@jibi jibi merged commit ab64a46 into main Jul 12, 2023
179 checks passed
@jibi jibi deleted the pr/jibi/egressgw-async-eps branch July 12, 2023 11:36
@jibi jibi mentioned this pull request Jul 13, 2023
13 tasks
@jibi jibi added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Jul 13, 2023
@aanm aanm added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Jul 14, 2023
@aanm aanm moved this from Needs backport from main to Backport done to v1.14 in 1.14.0 Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects/v1.13 This issue affects v1.13 branch backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. feature/egress-gateway Impacts the egress IP gateway feature. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/agent Cilium agent related.
Projects
No open projects
1.14.0
Backport done to v1.14
Development

Successfully merging this pull request may close these issues.

Pod identity propagation delay to remote IP cache when using egress gateways
5 participants