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

Internal traffic policy #21871

Merged
merged 11 commits into from
Dec 23, 2022
Merged

Internal traffic policy #21871

merged 11 commits into from
Dec 23, 2022

Conversation

gentoo-root
Copy link
Contributor

Internal traffic policy = Local means that internal (East-West)
connections to the service should be handled by node-local backends
only. When external and internal traffic policies are different, the
existing implementation is extended to support filtering out non-local
backends for the internal scope. When both policies are both Local, only
the external scope is used (the same already happens when both policies
are Cluster), and it contains local backends only.

Fixes: #16071

Support Kubernetes v1.21 new field internalTrafficPolicy=Local.

@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 Oct 24, 2022
@gentoo-root gentoo-root force-pushed the internal-traffic-policy branch 3 times, most recently from 2bdd0be to d16335d Compare October 27, 2022 23:05
@gentoo-root gentoo-root force-pushed the internal-traffic-policy branch 3 times, most recently from 20f300d to 057aacb Compare November 14, 2022 09:18
@maintainer-s-little-helper

This comment was marked as resolved.

Sign in to view
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Nov 14, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Nov 14, 2022
gandro
gandro reviewed Nov 14, 2022
View reviewed changes
Copy link
Member

@gandro gandro left a comment •
edited

Choose a reason for hiding this comment

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

I'm very excited to see this land! 🎉

The renaming of trafficPolicy fields in the api/ subfolder (both Hubble and OpenAPI), as well as the monitor API might break external JSON parsers trying to read the field. For example, Hubble can export those events into a JSON file which are then read by external systems.

I don't think we should rename it without warning/deprecation. If we want to rename it, I would suggest adding a new field called externalTrafficPolicy, populate both the new and old one, and deprecate the old one. Later, we can then check if it is safe to remove the old field.

@pchaigno pchaigno added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Nov 14, 2022
@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 Nov 14, 2022
@pchaigno pchaigno added release-note/major This PR introduces major new functionality to Cilium. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. and removed release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Nov 14, 2022
@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 Nov 14, 2022
@pchaigno pchaigno added the sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. label Nov 14, 2022
@gentoo-root gentoo-root force-pushed the internal-traffic-policy branch 7 times, most recently from 5404159 to ed51820 Compare November 18, 2022 21:33
@gentoo-root gentoo-root force-pushed the internal-traffic-policy branch 3 times, most recently from 6839886 to 5c82f5f Compare November 22, 2022 13:19
@gentoo-root gentoo-root marked this pull request as ready for review November 22, 2022 15:05
aditighag pushed a commit to aditighag/cilium that referenced this pull request Jan 9, 2023
@gentoo-root @aditighag
docs: Document internalTrafficPolicy-related changes in the upgrade g…
02fc238 
…uide

[ upstream commit fda3667 ]

Pull request cilium#21871 introduced a few minor behavioral changes and fixes.
Document them in the upgrade guide for Cilium 1.13.

Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
Signed-off-by: Aditi Ghag <aditi@cilium.io>
@aditighag aditighag added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 labels Jan 9, 2023
aditighag pushed a commit to aditighag/cilium that referenced this pull request Jan 10, 2023
@gentoo-root @aditighag
docs: Document internalTrafficPolicy-related changes in the upgrade g…
9648aee 
…uide

[ upstream commit fda3667 ]

Pull request cilium#21871 introduced a few minor behavioral changes and fixes.
Document them in the upgrade guide for Cilium 1.13.

Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
Signed-off-by: Aditi Ghag <aditi@cilium.io>
aditighag pushed a commit that referenced this pull request Jan 10, 2023
@gentoo-root @aditighag
docs: Document internalTrafficPolicy-related changes in the upgrade g…
4a40f2c 
…uide

[ upstream commit fda3667 ]

Pull request #21871 introduced a few minor behavioral changes and fixes.
Document them in the upgrade guide for Cilium 1.13.

Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
Signed-off-by: Aditi Ghag <aditi@cilium.io>
@aditighag aditighag added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Jan 10, 2023
@joestringer joestringer moved this from Needs backport from master to Backport done to v1.13 in 1.13.0-rc5 Jan 24, 2023
@joestringer joestringer mentioned this pull request Jan 24, 2023
Merged
@sayboras sayboras mentioned this pull request Feb 24, 2023
Merged
@sayboras sayboras mentioned this pull request Feb 27, 2023
Closed
7 tasks
sayboras added a commit to sayboras/cilium that referenced this pull request Feb 27, 2023
@sayboras
ingress: Bypass service count check
933b367 
As L7 LB service is having dummy endpoint (e.g. 192.192.192.192), the
in-cluster traffic will be dropped due to no healthy backend. This commit
is to make sure that the proper exclusion rule is done.

Relates: cilium#21871
Fixes: 38959d4

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request Feb 28, 2023
@sayboras
ingress: Bypass service count check
ff2dc24 
As L7 LB service is having dummy endpoint (e.g. 192.192.192.192), the
in-cluster traffic will be dropped due to no healthy backend. This commit
is to make sure that the proper exclusion rule is done.

Relates: #21871
Fixes: 38959d4

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request Feb 28, 2023
@sayboras
ingress: Bypass service count check
b2d0966 
As L7 LB service is having dummy endpoint (e.g. 192.192.192.192), the
in-cluster traffic will be dropped due to no healthy backend. This commit
is to make sure that the proper exclusion rule is done.

Relates: #21871
Fixes: 38959d4

Signed-off-by: Tam Mach <tam.mach@cilium.io>
aanm pushed a commit that referenced this pull request Mar 2, 2023
@sayboras @aanm
ingress: Bypass service count check
f5fdb67 
As L7 LB service is having dummy endpoint (e.g. 192.192.192.192), the
in-cluster traffic will be dropped due to no healthy backend. This commit
is to make sure that the proper exclusion rule is done.

Relates: #21871
Fixes: 38959d4

Signed-off-by: Tam Mach <tam.mach@cilium.io>
YutaroHayakawa pushed a commit to YutaroHayakawa/cilium that referenced this pull request Mar 6, 2023
@sayboras @YutaroHayakawa
ingress: Bypass service count check
9b38767 
[ upstream commit f5fdb67 ]

As L7 LB service is having dummy endpoint (e.g. 192.192.192.192), the
in-cluster traffic will be dropped due to no healthy backend. This commit
is to make sure that the proper exclusion rule is done.

Relates: cilium#21871
Fixes: 38959d4

Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
julianwiedmann pushed a commit to julianwiedmann/cilium that referenced this pull request Mar 7, 2023
@sayboras @julianwiedmann
ingress: Bypass service count check
ec9e644 
As L7 LB service is having dummy endpoint (e.g. 192.192.192.192), the
in-cluster traffic will be dropped due to no healthy backend. This commit
is to make sure that the proper exclusion rule is done.

Relates: cilium#21871
Fixes: 38959d4

Signed-off-by: Tam Mach <tam.mach@cilium.io>
YutaroHayakawa pushed a commit that referenced this pull request Mar 8, 2023
@sayboras @YutaroHayakawa
ingress: Bypass service count check
8306178 
[ upstream commit f5fdb67 ]

As L7 LB service is having dummy endpoint (e.g. 192.192.192.192), the
in-cluster traffic will be dropped due to no healthy backend. This commit
is to make sure that the proper exclusion rule is done.

Relates: #21871
Fixes: 38959d4

Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
YutaroHayakawa pushed a commit to YutaroHayakawa/cilium that referenced this pull request Mar 17, 2023
@gentoo-root @michi-covalent
docs: Document internalTrafficPolicy-related changes in the upgrade g…
be8eb8f 
…uide

[ upstream commit fda3667 ]

Pull request cilium#21871 introduced a few minor behavioral changes and fixes.
Document them in the upgrade guide for Cilium 1.13.

Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
Signed-off-by: Aditi Ghag <aditi@cilium.io>
Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
julianwiedmann pushed a commit to julianwiedmann/cilium that referenced this pull request Apr 11, 2023
@sayboras @julianwiedmann
ingress: Bypass service count check
6dc51a7 
As L7 LB service is having dummy endpoint (e.g. 192.192.192.192), the
in-cluster traffic will be dropped due to no healthy backend. This commit
is to make sure that the proper exclusion rule is done.

Relates: cilium#21871
Fixes: 38959d4

Signed-off-by: Tam Mach <tam.mach@cilium.io>
julianwiedmann pushed a commit to julianwiedmann/cilium that referenced this pull request Apr 13, 2023
@sayboras @julianwiedmann
ingress: Bypass service count check
643060e 
As L7 LB service is having dummy endpoint (e.g. 192.192.192.192), the
in-cluster traffic will be dropped due to no healthy backend. This commit
is to make sure that the proper exclusion rule is done.

Relates: cilium#21871
Fixes: 38959d4

Signed-off-by: Tam Mach <tam.mach@cilium.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brb brb brb left review comments

@borkmann borkmann borkmann approved these changes

@pchaigno pchaigno pchaigno left review comments

@gandro gandro gandro approved these changes

@kaworu kaworu kaworu approved these changes

@joestringer joestringer joestringer approved these changes

@nebril nebril nebril approved these changes

@michi-covalent michi-covalent michi-covalent approved these changes

@nathanjsweet nathanjsweet Awaiting requested review from nathanjsweet nathanjsweet was automatically assigned from cilium/api

@glibsm glibsm Awaiting requested review from glibsm glibsm was automatically assigned from cilium/sig-hubble-api

@julianwiedmann julianwiedmann Awaiting requested review from julianwiedmann

Assignees
No one assigned
Labels
backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. release-blocker/1.13 This issue will prevent the release of the next version of Cilium. release-note/major This PR introduces major new functionality to Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers.
Projects
No open projects
1.13.0-rc5
Backport done to v1.13
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support internalTrafficPolicy for services