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.

Sign up for GitHub

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

Jump to bottom

garp: Announce Pods with Gratuitous ARP #25482

Merged
merged 2 commits into from Jun 15, 2023
Merged

garp: Announce Pods with Gratuitous ARP #25482

merged 2 commits into from Jun 15, 2023

Conversation

markpash
Copy link
Contributor

@markpash markpash commented May 16, 2023 •
edited

garp: Announce Pods with Gratuitous ARP

This introduces a new feature that advertises Pod IPs on the L2 domain
using Gratuitous ARP packets.

When enabled, k8s Pod upsert and delete events are processed and GARP
packets are sent on the chosen interface, when required.

The EventProcessor cell introduced here is what processes the k8s Pod
events and maintains an internal state to make sure to only send GARP
packets when the Pod is created, or the IP is changed for some reason.
Pod deletion events simply erase the entry from the state.

There are new agent flags and helm values introduced to enable the
feature and to chose which interface to send GARP packets on.

Signed-off-by: Mark Pashmfouroush mark@isovalent.com

Added Gratuitous ARP Pod Announcements

@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 16, 2023
@markpash markpash force-pushed the pr/markpash/garp-events-cell branch 7 times, most recently from 225b0cd to 2339f2f Compare May 30, 2023 01:12
@markpash markpash changed the title wip: garp add events processor garp: Announce Pods with Gratuitous ARP May 30, 2023
@markpash markpash force-pushed the pr/markpash/garp-events-cell branch 2 times, most recently from 719d782 to 5f81b9f Compare May 30, 2023 01:28
@markpash markpash added kind/feature This introduces new functionality. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels May 30, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels May 30, 2023
@markpash markpash marked this pull request as ready for review May 30, 2023 08:50
@markpash markpash requested review from a team as code owners May 30, 2023 08:50
@markpash
Copy link
Contributor Author

/test

joamaki
joamaki requested changes Jun 5, 2023
View reviewed changes
pkg/datapath/garp/events.go Outdated Show resolved Hide resolved
pkg/datapath/garp/events.go Outdated Show resolved Hide resolved
pkg/datapath/garp/events.go Outdated Show resolved Hide resolved
daemon/cmd/cells.go Outdated Show resolved Hide resolved
daemon/cmd/daemon_main.go Outdated Show resolved Hide resolved
pkg/datapath/garp/events.go Outdated Show resolved Hide resolved
pkg/datapath/garp/events.go Outdated Show resolved Hide resolved
@sayboras sayboras added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jun 6, 2023
julianwiedmann
julianwiedmann approved these changes Jun 7, 2023
View reviewed changes
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.

Just a small comments, I trust Jussi's review on the hive/cell aspects.

Do you also have doc updates for this feature lined up? 🙏

install/kubernetes/cilium/templates/cilium-configmap.yaml Outdated Show resolved Hide resolved
pkg/datapath/garp/events_test.go Outdated Show resolved Hide resolved
@markpash markpash force-pushed the pr/markpash/garp-events-cell branch 3 times, most recently from 500a37f to 5a7d9b5 Compare June 13, 2023 02:24
@markpash markpash requested a review from joamaki June 13, 2023 09:10
@sayboras sayboras removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jun 13, 2023
sayboras
sayboras approved these changes Jun 13, 2023
View reviewed changes
Copy link
Member

@sayboras sayboras 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 ✔️

joamaki
joamaki requested changes Jun 13, 2023
View reviewed changes
pkg/datapath/garp/cells.go Outdated Show resolved Hide resolved
@markpash markpash force-pushed the pr/markpash/garp-events-cell branch 6 times, most recently from fe2e5df to 79a964e Compare June 14, 2023 12:25
@markpash
Copy link
Contributor Author

/test

@markpash markpash requested a review from joamaki June 14, 2023 13:26
joamaki
joamaki approved these changes Jun 15, 2023
View reviewed changes
Copy link
Contributor

@joamaki joamaki left a comment

Choose a reason for hiding this comment

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

LGTM!

@markpash markpash added the release-blocker/1.14 This issue will prevent the release of the next version of Cilium. label Jun 15, 2023
@markpash
garp: Announce Pods with Gratuitous ARP
ded16c6 
This introduces a new feature that advertises Pod IPs on the L2 domain
using Gratuitous ARP packets.

When enabled, k8s Pod upsert and delete events are processed and GARP
packets are sent on the chosen interface, when required.

The GARP cell introduced here is what processes the k8s Pod
events and maintains an internal state to make sure to only send GARP
packets when the Pod is created, or the IP is changed for some reason.
Pod deletion events simply erase the entry from the state.

There are new agent flags and helm values introduced to enable the
feature and to chose which interface to send GARP packets on.

Signed-off-by: Mark Pashmfouroush <mark@isovalent.com>
@markpash
garp: Switch processor to use cilium EndpointManager
fe99cb1 
Signed-off-by: Mark Pashmfouroush <mark@isovalent.com>
@markpash markpash force-pushed the pr/markpash/garp-events-cell branch from 79a964e to fe99cb1 Compare June 15, 2023 13:03
@markpash
Copy link
Contributor Author

/test

joestringer
joestringer approved these changes Jun 15, 2023
View reviewed changes
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Approving on behalf of @cilium/docs-structure . However, I would expect some additional user-facing documentation and perhaps e2e tests(?) to accompany the feature. Please do follow up on those.

@joestringer joestringer merged commit 16a5ba1 into cilium:main Jun 15, 2023
62 checks passed
@maxpain
Copy link

maxpain commented Jun 30, 2023 •
edited

Hello. I want to use the new L2 pod announcements feature in my homelab single-node Talos cluster.
I want my pods to use the flat local network, but I don't understand how to configure pod CIDRs correctly.
My network is 10.0.0.0/22 (10.0.0.1 - 10.0.3.255).
Is it correct to use 10.0.1.0/24 as a pod subnet CIDR?

In general, I want my local network devices to have direct access to pods in my Talos cluster via L2 10gbe switch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joamaki joamaki joamaki approved these changes

@joestringer joestringer joestringer approved these changes

@nbusseneau nbusseneau nbusseneau approved these changes

@sayboras sayboras sayboras approved these changes

@julianwiedmann julianwiedmann julianwiedmann approved these changes

@zacharysarah zacharysarah Awaiting requested review from zacharysarah zacharysarah is a code owner automatically assigned from cilium/docs-structure

Assignees
No one assigned
Labels
kind/feature This introduces new functionality. release-blocker/1.14 This issue will prevent the release of the next version of Cilium. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@markpash @maxpain @joamaki @joestringer @nbusseneau @sayboras @julianwiedmann