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

improve "kubeadm reset" kube-proxy cleanup #3133

Open
danwinship opened this issue Dec 15, 2024 · 4 comments
Open

improve "kubeadm reset" kube-proxy cleanup #3133

danwinship opened this issue Dec 15, 2024 · 4 comments
Labels
area/kube-proxy area/reset kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.

Comments

@danwinship
Copy link

@pacoxu pointed out the kubeadm reset kube-proxy cleanup instructions in kubernetes/kubernetes#128886. I started writing a better version:

diff --git a/cmd/kubeadm/app/cmd/reset.go b/cmd/kubeadm/app/cmd/reset.go
index 5fdc7ec193f..dbfede5c695 100644
--- a/cmd/kubeadm/app/cmd/reset.go
+++ b/cmd/kubeadm/app/cmd/reset.go
@@ -47,13 +47,23 @@ import (
 )
 
 var (
-	iptablesCleanupInstructions = dedent.Dedent(`
-		The reset process does not reset or clean up iptables rules or IPVS tables.
-		If you wish to reset iptables, you must do so manually by using the "iptables" command.
+	kubeProxyCleanupInstructions = dedent.Dedent(`
+		The reset process does not reset or clean up rules created by kube-proxy.
 
-		If your cluster was setup to utilize IPVS, run ipvsadm --clear (or similar)
-		to reset your system's IPVS tables.
+		If you wish to reset iptables rules created by kube-proxy in "iptables"
+		or "ipvs" mode, you must do so manually by using the "iptables" command.
 
+		If you were running kube-proxy in "ipvs" mode, run
+		    ipvsadm --clear
+		(or similar) to reset your system's IPVS tables.
+
+		If you were running kube-proxy in "nftables" mode, run
+		    nft delete table ip kube-proxy
+		    nft delete table ip6 kube-proxy
+		to remove kube-proxy's nftables rules.
+	`)
+
+	kubeconfigCleanupInstructions = dedent.Dedent(`
 		The reset process does not clean your kubeconfig files and you must remove them manually.
 		Please, check the contents of the $HOME/.kube/config file.
 	`)
@@ -236,8 +246,10 @@ func newCmdReset(in io.Reader, out io.Writer, resetOptions *resetOptions) *cobra
 
 			// output help text instructing user how to remove cni folders
 			fmt.Print(cniCleanupInstructions)
-			// Output help text instructing user how to remove iptables rules
-			fmt.Print(iptablesCleanupInstructions)
+			// Output help text instructing user how to remove kube-proxy rules
+			fmt.Print(kubeProxyCleanupInstructions)
+			// Output help text instructing user how to remove kubeconfigs
+			fmt.Print(kubeconfigCleanupInstructions)
 			return nil
 		},
 	}

But it seems weird and awkward that we explain how to clean up nftables and the ipvs half of ipvs mode, but we don't explain how to clean up iptables (including the iptables half of ipvs mode). Plus, kubeadm is no longer requiring the user to have the iptables binaries installed any more anyway...

kube-proxy already has a --cleanup flag that will clean up all iptables, ipvs, and nftables rules. Maybe it would be nicer to tell the user to run something like:

docker run -it --network=host ${KUBE_PROXY_IMAGE} kube-proxy --cleanup

(not tested). Or maybe something with crictl (except obviously the user isn't going to have that installed). Or maybe kubeadm reset could (optionally?) run the pod itself via CRI? Or by running kubelet in standalone mode just long enough to run one pod? I'm not sure what would be easiest for kubeadm...

Anyway, feel free to just take the patch above if you want the simple fix.

@pacoxu
Copy link
Member

pacoxu commented Dec 16, 2024

kubernetes/kubernetes#126596 we don't depends on crictl as well after v1.31.

  • For suggestions, crictl, docker or nerdctl are similar. I prefer docker as it can work. crictl will not work for run -it.

@neolit123
Copy link
Member

neolit123 commented Dec 16, 2024

@danwinship
i don't think we should print any step-by-step docs in the reset output. we could show a link to a page hosted at k/website that instructs users how to do the cleanup with the --cleanup flag.

i think kube-proxy needs to:

  • document its dependencies
  • document how to use the --cleanup flag or how to do manual cleanup for various modes

kubernetes/kubernetes#126596 we don't depends on crictl as well after v1.31.

  • For suggestions, crictl, docker or nerdctl are similar. I prefer docker as it can work. crictl will not work for run -it.

yes, kubeadm now interacts directly with the CRI. 'reset' can create a container and call kube-proxy with the --cleanup flag, but some users of kubeadm don't deploy kube-proxy, so that would mean also checking if the kube-proxy config map exists. i think this is doable, but i don't have bandwidth to play with that. also, i think it may step over the line of the 'best-effort' principle of 'reset' i.e. we consider 'reset' to be a best effort cleanup of the host - some things are skipped, some things may fail.

cc @SataQiu @carlory
for additional comments.

@neolit123 neolit123 added kind/feature Categorizes issue or PR as related to a new feature. kind/documentation Categorizes issue or PR as related to documentation. area/kube-proxy priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. area/reset labels Dec 16, 2024
@carlory
Copy link
Member

carlory commented Dec 17, 2024

i don't think we should print any step-by-step docs in the reset output. we could show a link to a page hosted at k/website that instructs users how to do the cleanup with the --cleanup flag.

+1. A document link is preferred.

@SataQiu
Copy link
Member

SataQiu commented Dec 17, 2024

also +1 to link to a website page
we just need to keep the web page up to date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kube-proxy area/reset kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.
Projects
None yet
Development

No branches or pull requests

5 participants