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

install: Disable kube-proxy-replacement by default #15422

Merged
merged 1 commit into from May 14, 2021
Merged

install: Disable kube-proxy-replacement by default #15422

merged 1 commit into from May 14, 2021

Conversation

tgraf
Copy link
Member

@tgraf tgraf commented Mar 22, 2021

The default value of "probe" is not ideal. It leads to a situation where
users are unaware who is performing the load-balancing. This can be
desirable for some and undesirable for others. A couple of
examples:

  • It typically leads to users continuing to run kube-proxy
  • It can lead to a situation where users believe they are running
    in kube-proxy replacement mode but the kernel requirement disables
    the feature automatically.
  • Automatic enabling of host-services can lead to incompatibility with
    Istio. Because of a the probe nature, a kernel upgrade in a cluster
    can lead to sudden issues with Istio without any additional action by
    the user.

It is generally preferred for users to take an explicit action to enable
kube-proxy and set it to strict so the cilium-agent fails if the mode is
not available.

@tgraf tgraf added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. area/helm Impacts helm charts and user deployment experience labels Mar 22, 2021
@tgraf tgraf requested review from a team as code owners March 22, 2021 19:58
@tgraf tgraf requested review from a team, joestringer and kaworu March 22, 2021 19:58
@tgraf tgraf requested a review from nathanjsweet March 22, 2021 19:58
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Mar 22, 2021
@brb
Copy link
Member

brb commented Mar 23, 2021

@pchaigno might have some concerns regarding the complexity issue which we hit once the socket-lb is disabled on newer kernels.

@tgraf
Copy link
Member Author

tgraf commented Mar 23, 2021

@pchaigno might have some concerns regarding the complexity issue which we hit once the socket-lb is disabled on newer kernels.

@pchaigno Eager to listen but I don't think we can avoid disabling it, we'll need to find a fix for the complexity issue.

@tgraf tgraf requested a review from pchaigno March 23, 2021 07:58
@pchaigno
Copy link
Member

@pchaigno Eager to listen but I don't think we can avoid disabling it, we'll need to find a fix for the complexity issue.

Ok. If we do that now, the default installation will likely fail for all users on 5.4+ until we fix the complexity issue. We don't have an obvious fix for the complexity issue.

tgraf added a commit to cilium/cilium-cli that referenced this pull request Mar 23, 2021
@tgraf
install: Disable kube-proxy-replacement by default
ecb7022 
Related: cilium/cilium#15422

Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf tgraf mentioned this pull request Mar 23, 2021
Merged
@brb brb mentioned this pull request Mar 26, 2021
Closed
brb
brb requested changes Mar 26, 2021
View reviewed changes
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

We need to block this until #14784 has been resolved.

@pchaigno
Copy link
Member

pchaigno commented Mar 29, 2021 •
edited

Let's put this into draft mode until #14726 is resolved.

@pchaigno pchaigno marked this pull request as draft March 29, 2021 08:06
@tgraf tgraf force-pushed the pr/tgraf/disable-host-services branch from 7e19a95 to b69284c Compare May 12, 2021 16:44
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.10.0-rc2 May 14, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Needs backport from master in 1.10.0-rc2 May 14, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Needs backport from master in 1.10.0-rc2 May 14, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.10.0-rc2 May 14, 2021
This was referenced May 14, 2021
Merged
Closed
Closed
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.0-rc2 May 17, 2021
@aanm aanm mentioned this pull request May 17, 2021
Merged
pchaigno pushed a commit to cilium/cilium-cli that referenced this pull request May 27, 2021
@tgraf @pchaigno
install: Disable kube-proxy-replacement by default
12d681d 
Related: cilium/cilium#15422

Signed-off-by: Thomas Graf <thomas@cilium.io>
pchaigno pushed a commit to cilium/cilium-cli that referenced this pull request Jun 1, 2021
@tgraf @pchaigno
install: Disable kube-proxy-replacement by default
7e273cb 
Related: cilium/cilium#15422

Signed-off-by: Thomas Graf <thomas@cilium.io>
pchaigno pushed a commit to cilium/cilium-cli that referenced this pull request Jun 1, 2021
@tgraf @pchaigno
install: Disable kube-proxy-replacement by default
13ca334 
Related: cilium/cilium#15422

Signed-off-by: Thomas Graf <thomas@cilium.io>
pchaigno pushed a commit to cilium/cilium-cli that referenced this pull request Jun 1, 2021
@tgraf @pchaigno
install: Disable kube-proxy-replacement by default
b649661 
We keep kube-proxy-replacement enabled in the external workload test
because that test requires host-reachable services and we want to keep
the same LB configuration for cluster pods and for the external VM.

Related: cilium/cilium#15422
Signed-off-by: Thomas Graf <thomas@cilium.io>
pchaigno pushed a commit to cilium/cilium-cli that referenced this pull request Jun 2, 2021
@tgraf @pchaigno
install: Disable kube-proxy-replacement by default
013efa4 
We keep kube-proxy-replacement enabled in the external workload test
because that test requires host-reachable services and we want to keep
the same LB configuration for cluster pods and for the external VM.

Related: cilium/cilium#15422
Signed-off-by: Thomas Graf <thomas@cilium.io>
pchaigno pushed a commit to cilium/cilium-cli that referenced this pull request Jun 7, 2021
@tgraf @pchaigno
install: Disable kube-proxy-replacement by default
2536edd 
We keep kube-proxy-replacement enabled in the external workload test
because that test requires host-reachable services and we want to keep
the same LB configuration for cluster pods and for the external VM.

Related: cilium/cilium#15422
Signed-off-by: Thomas Graf <thomas@cilium.io>
pchaigno pushed a commit to cilium/cilium-cli that referenced this pull request Jun 7, 2021
@tgraf @pchaigno
install: Disable kube-proxy-replacement by default
48e94fb 
We keep kube-proxy-replacement enabled in the external workload test
because that test requires host-reachable services and we want to keep
the same LB configuration for cluster pods and for the external VM.

Related: cilium/cilium#15422
Signed-off-by: Thomas Graf <thomas@cilium.io>
pchaigno pushed a commit to cilium/cilium-cli that referenced this pull request Jun 9, 2021
@tgraf @pchaigno
install: Disable kube-proxy-replacement by default
f1b455c 
We keep kube-proxy-replacement enabled in the external workload test
because that test requires host-reachable services and we want to keep
the same LB configuration for cluster pods and for the external VM.

Related: cilium/cilium#15422
Signed-off-by: Thomas Graf <thomas@cilium.io>
pchaigno pushed a commit to cilium/cilium-cli that referenced this pull request Jun 9, 2021
@tgraf @pchaigno
install: Disable kube-proxy-replacement by default
7cac92c 
We keep kube-proxy-replacement enabled in the external workload test
because that test requires host-reachable services and we want to keep
the same LB configuration for cluster pods and for the external VM.

Related: cilium/cilium#15422
Signed-off-by: Thomas Graf <thomas@cilium.io>
aditighag pushed a commit to cilium/cilium-cli that referenced this pull request Jun 10, 2021
@tgraf @aditighag
install: Disable kube-proxy-replacement by default
bf2c1f9 
We keep kube-proxy-replacement enabled in the external workload test
because that test requires host-reachable services and we want to keep
the same LB configuration for cluster pods and for the external VM.

Related: cilium/cilium#15422
Signed-off-by: Thomas Graf <thomas@cilium.io>
@pchaigno pchaigno mentioned this pull request Nov 8, 2021
Merged
aditighag pushed a commit to aditighag/cilium-cli that referenced this pull request Apr 21, 2023
@tgraf @aditighag
install: Disable kube-proxy-replacement by default
4706869 
We keep kube-proxy-replacement enabled in the external workload test
because that test requires host-reachable services and we want to keep
the same LB configuration for cluster pods and for the external VM.

Related: cilium/cilium#15422
Signed-off-by: Thomas Graf <thomas@cilium.io>
michi-covalent pushed a commit to michi-covalent/cilium that referenced this pull request May 30, 2023
@tgraf @michi-covalent
install: Disable kube-proxy-replacement by default
8722a19 
We keep kube-proxy-replacement enabled in the external workload test
because that test requires host-reachable services and we want to keep
the same LB configuration for cluster pods and for the external VM.

Related: cilium#15422
Signed-off-by: Thomas Graf <thomas@cilium.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seanmwinn seanmwinn seanmwinn left review comments

@brb brb brb requested changes

@kaworu kaworu kaworu approved these changes

@nathanjsweet nathanjsweet nathanjsweet approved these changes

@pchaigno pchaigno Awaiting requested review from pchaigno

@joestringer joestringer Awaiting requested review from joestringer

Assignees

@kaworu kaworu

@nathanjsweet nathanjsweet

@joestringer joestringer

@pchaigno pchaigno

Labels
area/helm Impacts helm charts and user deployment experience release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
No open projects
1.10.0
Done
1.10.0-rc2
Backport done to v1.10
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@tgraf @brb @pchaigno @joestringer @aanm @kaworu @nathanjsweet @seanmwinn