Add support for nf_conntrack_tcp_be_liberal sysctl to kube-proxy

[aroradaman]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #94861

Special notes for your reviewer:

Does this PR introduce a user-facing change?

kube-proxy: Added an option/flag for configuring the `nf_conntrack_tcp_be_liberal` sysctl (in the kernel's netfilter conntrack subsystem).  When enabled, kube-proxy will not install the DROP rule for invalid conntrack states, which currently breaks users of asymmetric routing.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[aroradaman]

/test all

[aojea]

The open issue talks about parity with the existing TCP options #120214

We expressed several times that kube-proxy should not be too invasive with the node parameters, why do we add nf_conntrack_tcp_be_liberal here?
Bear in mind we can be stepping into some other component setting this values already

/assign @danwinship @khenidak

As you commented on the referenced issue

[danwinship]

I said we shouldn't set it by default. I forgot we already had infra for letting the admin configure conntrack-related sysctls. That's fine.

[danwinship]

@danwinship should I handle this case (almost hypothetical ) - user sets tcp-be-liberal true in config and false in command line flag.

It should behave the same way other flags behave... I think the current expectation is that the command line flags are completely overridden by the config file if you pass --config. (But I'm not even sure about that and it's kind of a mess.)

[aroradaman]

It should behave the same way other flags behave... I think the current expectation is that the command line flags are completely overridden by the config file if you pass --config. (But I'm not even sure about that and it's kind of a mess.)

Yes, if --config is passed command line flags are completely overridden (except hostname).

#120354 (comment), here you suggested to manually sync command line flag with config, does this still hold ?

I added the logic to give precedence to command line flag over config for this option, the same way we do for hostname. LMK, will remove this logic.

kubernetes/cmd/kube-proxy/app/server.go

Lines 347 to 359 in f3c13d8

	// processHostnameOverrideFlag processes hostname-override flag
	func (o *Options) processHostnameOverrideFlag() error {
	// Check if hostname-override flag is set and use value since configFile always overrides
	if len(o.hostnameOverride) > 0 {
	hostName := strings.TrimSpace(o.hostnameOverride)
	if len(hostName) == 0 {
	return fmt.Errorf("empty hostname-override is invalid")
	}
	o.config.HostnameOverride = strings.ToLower(hostName)
	}
	return nil
	}

[danwinship]

oh, sorry, I meant "sync" as in "make sure that the right one overrides the other one". If the command-line flag reads directly into a field in o.config then the overriding happens automatically (because reading the config file overwrites the values set by the FlagSet) but if the command-line flag reads into a special field in the Options then you have do a bit of extra work. Which, I guess is just checking whether o.ConfigFile is set or not, and only copying the flag value into o.config if it wasn't.

But anyway, we only needed the cliflags.TriState if we want to be able to specify both "clear the flag" and "don't change the value of the flag". But since we only care about the latter, we can use a bool where true means set it and false means don't change it, and then you can just read directly into the o.config and let the normal overriding mechanism handle it.

[danwinship]

/retitle Add support for `nf_conntrack_tcp_be_liberal_ sysctl to kube-proxy
/lgtm
/approve
/assign @thockin
for API approval

The release note should be updated to reference #94861 and mention that this you can now work around that problem by specifying this option, which will cause kube-proxy to not create the DROP rule that breaks asymmetric routing.

Oh, also, the "Fixes" part of the initial comment is wrong now since the UDP stuff got split out, but it should say this fixes 94861.

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 3e6390648bd8b2fa8c94e33b3a35c77b142658fd

[aojea]

/lgtm

[aroradaman]

ping @thockin

[thockin]

Thanks!

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aroradaman, danwinship, thockin

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:

• cmd/kube-proxy/OWNERS [danwinship,thockin]
• pkg/generated/openapi/OWNERS [thockin]
• pkg/proxy/apis/config/OWNERS [danwinship,thockin]
• staging/src/k8s.io/kube-proxy/config/OWNERS [thockin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[leilajal]

/triage accepted