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
remove kube-proxy mode fallback #111806
remove kube-proxy mode fallback #111806
Conversation
Please note that we're already in Test Freeze for the Fast forwards are scheduled to happen every 6 hours, whereas the most recent run was: Thu Aug 11 13:31:21 UTC 2022. |
@danwinship: This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1a02709
to
a3923f8
Compare
cmd/kube-proxy/app/server_others.go
Outdated
@@ -555,28 +547,15 @@ func cidrTuple(cidrList string) [2]string { | |||
return cidrs | |||
} | |||
|
|||
func getProxyMode(proxyMode string, canUseIPVS bool) string { | |||
func getProxyMode(proxyMode string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we can get rid of this since this should be handled by validation a3923f8#r946585562
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, I had gotten rid of it originally but the unit tests fail. Looks like that's a unit test problem though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validation allows ""
which means "kube-proxy should pick the best available proxy" (so it doesn't actually get defaulted). So we need something.
/lgtm |
This was implemented partly in server.go and partly in server_others.go even though even the parts in server.go were totally linux-specific. Simplify things by putting it all in server_others.go and get rid of some unnecessary abstraction.
Back when iptables was first made the default, there were theoretically some users who wouldn't have been able to support it due to having an old /sbin/iptables. But kube-proxy no longer does the things that didn't work with old iptables, and we removed that check a long time ago. There is also a check for a new-enough kernel version, but it's checking for a feature which was added in kernel 3.6, and no one could possibly be running Kubernetes with a kernel that old. So the fallback code now never actually falls back, so it should just be removed.
If the user passes "--proxy-mode ipvs", and it is not possible to use IPVS, then error out rather than falling back to iptables. There was never any good reason to be doing fallback; this was presumably erroneously added to parallel the iptables-to-userspace fallback (which only existed because we had wanted iptables to be the default but not all systems could support it). In particular, if the user passed configuration options for ipvs, then they presumably *didn't* pass configuration options for iptables, and so even if the iptables proxy is able to run, it is likely to be misconfigured.
a3923f8
to
946ce55
Compare
/retest 👁️
|
// that it exists. If this Proxier is chosen, we'll initialize it as we | ||
// need. | ||
func (lkct LinuxKernelCompatTester) IsCompatible() error { | ||
_, err := utilsysctl.New().GetSysctl(sysctlRouteLocalnet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm |
What type of PR is this?
/kind bug
/kind cleanup
What this PR does / why we need it:
kube-proxy --proxy-mode ipvs
will fall back to iptables mode if IPVS mode is not usable, but this is not actually a reasonable behavior, at all, and was presumably only added by mistaken analogy to the iptables-to-userspace fallback, which was there because we wanted to make iptables the default but there used to be some systems that couldn't support it.On that topic, there are no longer systems that can run kubernetes but can't run the iptables proxy (but can run the userspace proxy), so remove the iptables-to-userspace fallback too.
Which issue(s) this PR fixes:
Fixes #111709
Does this PR introduce a user-facing change?
/sig network
/area ipvs
/priority important-soon