Implement kubeadm reset #34807

luxas · 2016-10-14T09:27:27Z

@kubernetes/sig-cluster-lifecycle

This change is

dgoodwin

This worked great on CentOS, just a few questions and one very minor request for changes.

dgoodwin · 2016-10-14T13:38:22Z

cmd/kubeadm/app/cmd/init.go

@@ -150,7 +150,7 @@ func NewInit(cfgPath string, cfg *kubeadmapi.MasterConfiguration, skipPreFlight
 	}

 	if !skipPreFlight {
-		fmt.Println("<cmd/init> Running pre-flight checks")
+		fmt.Println("Running pre-flight checks")


Intentional?

Yep, I think we decided to drop it
At least it was dropped in join

dgoodwin · 2016-10-14T13:43:18Z

cmd/kubeadm/app/preflight/checks.go

 		PortOpenCheck{port: 10250},
-		PortOpenCheck{port: 10251},
-		PortOpenCheck{port: 10252},


Did these turn out to not be necessary?

This is for the node, 10251 and 10252 for cm and scheduler and 8080 for unsecured apiserver

dgoodwin · 2016-10-14T13:50:09Z

cmd/kubeadm/app/cmd/reset.go

+	exec.Command("sh", "-c", "cat /proc/mounts | awk '{print $2}' | grep '/var/lib/kubelet' | xargs umount").Run()
+
+	dirsToRemove := []string{"/var/lib/kubelet", "/var/lib/etcd", "/etc/kubernetes"}
+	fmt.Printf("Deleting the stateful directories: [%v]\n", dirsToRemove)


This shows up as: Deleting the stateful directories: [[/var/lib/kubelet /var/lib/etcd /etc/kubernetes]]

Can probably drop the [] in code.

dgoodwin · 2016-10-14T13:50:53Z

cmd/kubeadm/app/cmd/reset.go

+
+	fmt.Printf("Stopping all running docker containers...")
+	if err := exec.Command("sh", "-c", "docker ps -q | xargs docker kill").Run(); err != nil {
+		fmt.Printf("failed to stop the running containers")


Should we actually clean these up or just kill? My reset script I'd been clearing them out entirely.

Also should we only hit containers with a k8s prefix?

Thinking we may want to filter to k8s containers only, I could envision a developer deciding to init master on their workstation.

I thought about that, leads to more work, but is probably worth it

Not sure if we should remove them though... the user might want to read logs

I think we should use docker rm --force --volumes here.

the user might want to read logs

Maybe, but more user will complain about disk space used up by those containers.

errordeveloper · 2016-10-14T19:14:26Z

cmd/kubeadm/app/cmd/reset.go

+		fmt.Printf("Stopping the %s service...\n", serviceToStop)
+		initSystem.ServiceStop(serviceToStop)
+	} else {
+		fmt.Printf("no supported init system detected, skipping service checks for %s. kubeadm reset may not have full effect now.\n", serviceToStop)


Why doesn't initsystem.GetInitSystem() return that string as an error?

errordeveloper · 2016-10-14T19:16:07Z

cmd/kubeadm/app/cmd/reset.go

+
+	fmt.Printf("Unmounting directories in /var/lib/kubelet...\n")
+	// Don't check for errors here, since umount will return a non-zero exit code if there is no directories to umount
+	exec.Command("sh", "-c", "cat /proc/mounts | awk '{print $2}' | grep '/var/lib/kubelet' | xargs umount").Run()


I think find /var/lib/kubelet | xargs -n 1 findmnt -n -t tmpfs -o TARGET -T | uniq | xargs -r umount is cleaner.

If you depend on awk, you don't have to use grep, i.e. awk '$2 ~ /\/var\/lib\/kubelet { print $2 }' | xargs umount, or even awk '$2 ~ /\/var\/lib\/kubelet { system("umount "$2) }, on the other hand you don't even have to shell out...

I think we should improve that later on...
I don't think find /var/lib/kubelet | xargs -n 1 findmnt -n -t tmpfs -o TARGET -T | uniq | xargs -r umount is better because it will do a find in the whole /var/lib/kubelet dir which may be large.

/proc/mounts is a very small file with the information we need.

I couldn't get awk working standalone, so someone can take that as a fun thing improve later :)

errordeveloper · 2016-10-14T19:24:17Z

cmd/kubeadm/app/cmd/reset.go

+
+	fmt.Printf("Stopping all running docker containers...")
+	if err := exec.Command("sh", "-c", "docker ps -q | xargs docker kill").Run(); err != nil {
+		fmt.Printf("failed to stop the running containers")


I think we should use docker rm --force --volumes here.

the user might want to read logs

Maybe, but more user will complain about disk space used up by those containers.

luxas · 2016-10-14T21:44:23Z

PTAL @pires @mikedanese @dgoodwin @errordeveloper

luxas · 2016-10-14T21:45:14Z

I'd like to get this lgtm'd ASAP, if @mikedanese could do it, it would be awesome! (Ilya is offline now)

k8s-ci-robot · 2016-10-14T22:22:40Z

Jenkins verification failed for commit e4268db1a84db9667d3a8745905f95965b9547fe. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

dgoodwin

LGTM

luxas · 2016-10-15T07:57:13Z

Applying @dgoodwin's LGTM

pires · 2016-10-15T08:34:35Z

cmd/kubeadm/app/cmd/reset.go

+}
+
+func RunReset(out io.Writer, cmd *cobra.Command, skipPreFlight bool) error {
+


Why the empty line?

pires · 2016-10-15T08:36:33Z

cmd/kubeadm/app/cmd/reset.go

+		fmt.Println("Running pre-flight checks")
+		err := preflight.RunResetCheck()
+		if err != nil {
+			return err


This will break pre-flight checks. It should be return &preflight.PreFlightError{Msg: err.Error()}} instead.

Ah, I copied as it was for init and join after the original preflight PR, which seems to have had this bug then.

Will fix

luxas · 2016-10-15T09:04:13Z

Fixed @pires' comments, waiting for the build to be green before applying lgtm again

pires

LGTM

k8s-github-robot · 2016-10-15T12:30:49Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2016-10-15T13:10:35Z

Automatic merge from submit-queue

@luxas

Automatic merge from submit-queue kubeadm join: Added support for config file. As more behavior (#34719, #34807, fix for #33641) is added to `kubeadm join`, this will be eventually very much needed. Makes sense to go in sooner rather than later. Also references #34501 and #34884. /cc @luxas @mikedanese

googlebot added the cla: yes label Oct 14, 2016

k8s-github-robot assigned errordeveloper Oct 14, 2016

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Oct 14, 2016

luxas added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Oct 14, 2016

dgoodwin reviewed Oct 14, 2016

View reviewed changes

errordeveloper suggested changes Oct 14, 2016

View reviewed changes

pires mentioned this pull request Oct 14, 2016

there should be a way to leave cluster in kubeadm #34404

Closed

luxas force-pushed the kubeadm_reset branch from 7460ab2 to e4268db Compare October 14, 2016 21:40

dgoodwin approved these changes Oct 14, 2016

View reviewed changes

luxas assigned dgoodwin Oct 15, 2016

luxas force-pushed the kubeadm_reset branch from e4268db to df14acb Compare October 15, 2016 07:15

luxas added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 15, 2016

pires suggested changes Oct 15, 2016

View reviewed changes

luxas removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 15, 2016

Implement kubeadm reset

ecdaa71

luxas force-pushed the kubeadm_reset branch from df14acb to ecdaa71 Compare October 15, 2016 08:58

pires approved these changes Oct 15, 2016

View reviewed changes

luxas added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 15, 2016

luxas mentioned this pull request Oct 15, 2016

release a new version of kubeadm #34884

Closed

pires mentioned this pull request Oct 15, 2016

kubeadm join: Added support for config file. #34885

Merged

luxas added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Oct 15, 2016

k8s-github-robot merged commit 88d6d7a into kubernetes:master Oct 15, 2016

luxas mentioned this pull request Oct 16, 2016

Add a kubeadm reset command #34044

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement kubeadm reset #34807

Implement kubeadm reset #34807

luxas commented Oct 14, 2016 •

edited by k8s-oncall

dgoodwin left a comment

dgoodwin Oct 14, 2016

luxas Oct 14, 2016

pires Oct 14, 2016

dgoodwin Oct 14, 2016

luxas Oct 14, 2016

dgoodwin Oct 14, 2016

dgoodwin Oct 14, 2016

dgoodwin Oct 14, 2016

luxas Oct 14, 2016

errordeveloper Oct 14, 2016

errordeveloper Oct 14, 2016

errordeveloper Oct 14, 2016

errordeveloper Oct 14, 2016

luxas Oct 14, 2016

errordeveloper Oct 14, 2016

luxas commented Oct 14, 2016

luxas commented Oct 14, 2016

k8s-ci-robot commented Oct 14, 2016

dgoodwin left a comment

luxas commented Oct 15, 2016

pires Oct 15, 2016

pires Oct 15, 2016

luxas Oct 15, 2016

luxas commented Oct 15, 2016

pires left a comment

k8s-github-robot commented Oct 15, 2016

k8s-github-robot commented Oct 15, 2016

		}

		func RunReset(out io.Writer, cmd *cobra.Command, skipPreFlight bool) error {

Implement kubeadm reset #34807

Implement kubeadm reset #34807

Conversation

luxas commented Oct 14, 2016 • edited by k8s-oncall

dgoodwin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luxas commented Oct 14, 2016

luxas commented Oct 14, 2016

k8s-ci-robot commented Oct 14, 2016

dgoodwin left a comment

Choose a reason for hiding this comment

luxas commented Oct 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luxas commented Oct 15, 2016

pires left a comment

Choose a reason for hiding this comment

k8s-github-robot commented Oct 15, 2016

k8s-github-robot commented Oct 15, 2016

luxas commented Oct 14, 2016 •

edited by k8s-oncall