-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
Restore kube-proxy's support for 0 values for conntrack settings #55261
Restore kube-proxy's support for 0 values for conntrack settings #55261
Conversation
cc @kubernetes/sig-network-pr-reviews |
ad6fe78
to
6e38f1f
Compare
When kube-proxy was refactored to use a configuration file, the ability to use 0 for conntrack min, max, max per core, and tcp timeouts was inadvertently broken; if you specified 0, it would instead apply the default value from defaults.go. This change restores the ability to use 0 to mean 0. Signed-off-by: Andy Goldstein <andy.goldstein@gmail.com>
Signed-off-by: Andy Goldstein <andy.goldstein@gmail.com>
6e38f1f
to
9a53ee9
Compare
In addition to that, I would really like someone familiar with conntrack settings, especially our desired defaulting vs 0 values, to review it. |
/test pull-kubernetes-unit |
/retest |
@@ -146,12 +146,15 @@ func AddFlags(options *Options, fs *pflag.FlagSet) { | |||
fs.Float32Var(&options.config.ClientConnection.QPS, "kube-api-qps", options.config.ClientConnection.QPS, "QPS to use while talking with kubernetes apiserver") | |||
fs.IntVar(&options.config.ClientConnection.Burst, "kube-api-burst", options.config.ClientConnection.Burst, "Burst to use while talking with kubernetes apiserver") | |||
fs.DurationVar(&options.config.UDPIdleTimeout.Duration, "udp-timeout", options.config.UDPIdleTimeout.Duration, "How long an idle UDP connection will be kept open (e.g. '250ms', '2s'). Must be greater than 0. Only applicable for proxy-mode=userspace") | |||
fs.Int32Var(&options.config.Conntrack.Max, "conntrack-max", options.config.Conntrack.Max, | |||
if options.config.Conntrack.Max == nil { |
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.
Is there a doc or slides or something somewhere that explains the lifecycle of config these days? It looks crazy complicated, and I am trying to wrap my head around it.
NewOptions()
ApplyDefaults()
AddFlags()
At some point we read a config file, too. Where can I learn more about current state and eventual final state? Sorry for losing track of this.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ncdc, thockin Associated issue: 50787 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
Automatic merge from submit-queue (batch tested with PRs 55247, 55324, 55261, 55147, 54052). If you want to cherry-pick this change to another branch, please follow the instructions here. |
@thockin I'm not aware of any docs (I know, I know). Once the deprecated command line flags are removed, ApplyDefaults() and the giant AddFlags() go away. When we load the config file, it applies defaults as part of the decoding, so it will get simpler in the end. |
What this PR does / why we need it: re-allow 0 values for kube-proxy conntrack min, max, max per core, tcp close wait timeout, tcp established timeout.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #50787
Special notes for your reviewer:
Release note: