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.

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

Allow switching kube-proxy mode for k8s cluster > 1.16 #2238

Merged

Conversation

DockToFuture
Copy link
Member

@DockToFuture DockToFuture commented Apr 27, 2020

What this PR does / why we need it:
This PR lifts the restrictions on kube-proxy to allow switching of kube-proxy mode (IPTables, IPVS) for k8s cluster > 1.16. Further the iptables and ipvs rules of the previous mode are cleaned up in an init container before running kube-proxy in the new mode.

PR kubernetes/kubernetes#78775 fixed the cleanup error in v1.16 which led in the past to the restrictions.

Which issue(s) this PR fixes:
Fixes #2225

Special notes for your reviewer:

Release note:

Restrictions on kube-proxy are lifted to allow switching of kube-proxy mode (IPTables, IPVS) for k8s cluster > 1.16. 

rfranzke
rfranzke previously approved these changes Apr 27, 2020
@ialidzhikov
Copy link
Member

@DockToFuture, can you update the PR description with the corresponding k/k issue and provide details about it?

@DockToFuture
Copy link
Member Author

Will update it, please wait with merging.

@rfranzke
Copy link
Member

/kind/enhancement

@ghost ghost added the kind/enhancement Enhancement, improvement, extension label Apr 29, 2020
@rfranzke
Copy link
Member

@DockToFuture what's the status of this PR?

@DockToFuture
Copy link
Member Author

Before we switch the proxy-mode we have to run kube-proxy with the --cleanup flag and wait until it successfully terminates otherwise the ipvs rules or iptables rules won't be properly purged. It will not work after the mode is switched. Therefore I'm looking for a proper way how to implement it.

@vpnachev
Copy link
Member

vpnachev commented May 4, 2020

An init container with kube-proxy --cleanup should do the job, what do you think?

@DockToFuture
Copy link
Member Author

This cleans up the current mode not the one from which kube-proxy has switched.

@zanetworker
Copy link
Contributor

zanetworker commented May 4, 2020

would a prestop hook to do the cleanup help here?

@rfranzke
Copy link
Member

rfranzke commented May 5, 2020

A prestop hook would be executed whenever the kube-proxy pod stops, so I don't think that's what we want. Instead, would it be viable to go with @vpnachev's idea? Could we add an init container that reads the configured mode from the kube-proxy config and then decides whether to cleanup?

Basically: When the mode is iptables then check if there are IPVS artifacts on the node, and if yes then run the --cleanup job, if not simply exit 0. If the mode is IPVS then check if there are iptable artifacts on the node, and if yes then run the --cleanup job, if not simply exit 0.

If that's not possible then we would have to "remember" the old mode and then decide in code when to spawn the --cleanup job. However, this would be more complex. So can we achieve above suggestion?

@vpnachev
Copy link
Member

vpnachev commented May 5, 2020

When the mode is iptables then check if there are IPVS artifacts on the node, and if yes then run the --cleanup job, if not simply exit 0. If the mode is IPVS then check if there are iptable artifacts on the node, and if yes then run the --cleanup job, if not simply exit 0.

This was exactly what I was thinking last night. I am just not sure how to properly detect existing artifacts, so maybe the init container can dump the current active mode in a file on the host, e.g. /kube-proxy-mode.

OLD_KUBE_PROXY_MODE=$(cat /kube-proxy-mode)
[[ ${OLD_KUBE_PROXY_MODE} == ${KUBE_PROXY_MODE} ]] && exit 0
kube-proxy --clean-up || exit $?
echo ${KUBE_PROXY_MODE} >/kube-proxy-mode

The KUBE_PROXY_MODE variable can be set by gardener in the daemonset, or can be read from /var/lib/kube-proxy-config/config.yaml file.

@rfranzke
Copy link
Member

rfranzke commented May 5, 2020

+💯 @vpnachev

@zanetworker
Copy link
Contributor

@rfranzke the --cleanup flag cleans up both IPVS and IPtables according to the documentation, do we need the check here? Or is that not the case @DockToFuture

--cleanup
  If true cleanup iptables and ipvs rules and exit.

@rfranzke
Copy link
Member

rfranzke commented May 5, 2020

You need the check because you only want to run the cleanup if you switch the mode. Otherwise, we end up with the same problem like the pre-stop hook - it would be executed everytime when kube-proxy starts. 😁

Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

Tested it out, works great, just a few left-over cosmetics, otherwise lgtm.

Copy link
Member

@vpnachev vpnachev left a comment

Choose a reason for hiding this comment

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

Thank you!

It looks very well now, I have just a few minor change requests.

rfranzke
rfranzke previously approved these changes May 13, 2020
Copy link
Member

@vpnachev vpnachev left a comment

Choose a reason for hiding this comment

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

/lgtm

@vpnachev vpnachev merged commit 2427a00 into gardener:master May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Enhancement, improvement, extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lift restriction that prevents switching kube proxy mode
8 participants