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

Fix crash when deleting the cluster but it doesn't exist #4980

Merged

Conversation

blueelvis
Copy link
Contributor

Fixes #4459

I think that we should have a force deletion switch as well where it deletes the .minikube folder as well. What do you think?

-Pranav

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 4, 2019
@k8s-ci-robot
Copy link
Contributor

Hi @blueelvis. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 4, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: blueelvis
To complete the pull request process, please assign afbjorklund
You can assign the PR to them by writing /assign @afbjorklund in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

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

@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 4, 2019
@afbjorklund
Copy link
Collaborator

Can you please fix your line endings to not insert the ^M (\r) on every line ?

https://help.github.com/en/articles/configuring-git-to-handle-line-endings

It is making the changes to the build_guide.md look much bigger than what it is...

Thanks!

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 5, 2019
@blueelvis
Copy link
Contributor Author

Sorry about that. Fixed the issue. For some reason, the other files were in LF. Let me know if this requires any more edits :)

Thanks!

@RA489
Copy link

RA489 commented Aug 5, 2019

@minikube-bot OK to test

@tstromberg
Copy link
Contributor

Fixes #4459

I think that we should have a force deletion switch as well where it deletes the .minikube folder as well. What do you think?

-Pranav

I'm not sure what you mean here. delete, if successful, should always remove the profile from ~/.minikube/profiles. I don't think it should delete ~/.minikube, as it may cause inadvertent data loss.

@blueelvis
Copy link
Contributor Author

Agreed that it could cause the deletion of the entire .minikube folder. We should provide a flag to delete that as well. It could cause the data loss but asking the users to go and manually delete seems a bit lengthy when we can add a flag to do the same. Although it could always ask for confirmation and then also display a big warning.

If you agree, I can implement this in this same PR as well.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 13, 2019
@sharifelgamal
Copy link
Collaborator

@blueelvis This looks to be ready to merge once it's rebased.

@blueelvis blueelvis force-pushed the 4459-delete-even-if-vm-doesnt-exist branch from d151c33 to e0d3e0c Compare September 15, 2019 14:10
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 15, 2019
@blueelvis
Copy link
Contributor Author

@sharifelgamal @tstromberg - I am creating a new issue to delete the .minikube folder as well. This PR 's conflicts have been fixed. Please merge.

-Pranav

@TravisBuddy
Copy link

Travis tests have failed

Hey @blueelvis,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

1st Build

View build log

make test
which go-bindata || GO111MODULE=off GOBIN=/home/travis/gopath/bin go get github.com/jteeuwen/go-bindata/...
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.12.9.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.0/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/assets/assets.go -pkg assets deploy/addons/...
gofmt -s -w pkg/minikube/assets/assets.go
which go-bindata || GO111MODULE=off GOBIN=/home/travis/gopath/bin go get github.com/jteeuwen/go-bindata/...
/home/travis/gopath/bin/go-bindata
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.12.9.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.0/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/translate/translations.go -pkg translate translations/...
gofmt -s -w pkg/minikube/translate/translations.go
./test.sh
= go mod ================================================================
ok
= make lint =============================================================
golangci/golangci-lint info checking GitHub for tag 'v1.17.1'
golangci/golangci-lint info found version: 1.17.1 for v1.17.1/linux/amd64
golangci/golangci-lint info installed out/linters/golangci-lint
pkg/minikube/cluster/cluster.go:287: File is not `goimports`-ed (goimports)
		exit.WithCodeT(exit.Failure,"Unable to get the status of the cluster.")
cmd/minikube/cmd/delete.go:82:107: err.Name undefined (type error has no field or method Name) (typecheck)
			out.T(out.Meh, `"{{.name}}" cluster does not exist. Proceeding ahead with cleanup.`, out.V{"name": err.Name})
			                                                                                                       ^
Makefile:334: recipe for target 'lint-ci' failed
make[1]: *** [lint-ci] Error 1
= boilerplate ===========================================================
ok
= schema_check ==========================================================
ok
= go test ===============================================================
# k8s.io/minikube/cmd/minikube/cmd [k8s.io/minikube/cmd/minikube/cmd.test]
cmd/minikube/cmd/delete.go:82: err.Name undefined (type error has no field or method Name)
FAIL	k8s.io/minikube/cmd/minikube/cmd [build failed]
--- FAIL: TestUnsetConfig (0.00s)
    unset_test.go:30: Failed to set the property "cpus"
    unset_test.go:35: Failed to read config "specified key could not be found in config"
    unset_test.go:39: Expected cpus to be 1 but got 
    unset_test.go:44: Failed to unset property "create /home/travis/.minikube/config/config.json: open /home/travis/.minikube/config/config.json: no such file or directory"
FAIL
coverage: 18.7% of statements
FAIL	k8s.io/minikube/cmd/minikube/cmd/config	0.059s
ok  	k8s.io/minikube/pkg/drivers	0.027s	coverage: 18.4% of statements
ok  	k8s.io/minikube/pkg/drivers/kvm	0.056s	coverage: 2.3% of statements
ok  	k8s.io/minikube/pkg/minikube/assets	0.036s	coverage: 61.8% of statements
ok  	k8s.io/minikube/pkg/minikube/bootstrapper	4.028s	coverage: 73.3% of statements
ok  	k8s.io/minikube/pkg/minikube/bootstrapper/kubeadm	0.073s	coverage: 30.6% of statements
ok  	k8s.io/minikube/pkg/minikube/cluster	0.661s	coverage: 53.1% of statements
ok  	k8s.io/minikube/pkg/minikube/config	0.027s	coverage: 76.0% of statements
ok  	k8s.io/minikube/pkg/minikube/cruntime	0.010s	coverage: 62.4% of statements
ok  	k8s.io/minikube/pkg/minikube/extract	0.023s	coverage: 56.7% of statements
ok  	k8s.io/minikube/pkg/minikube/kubeconfig	0.057s	coverage: 75.9% of statements
ok  	k8s.io/minikube/pkg/minikube/logs	0.025s	coverage: 1.4% of statements
ok  	k8s.io/minikube/pkg/minikube/machine	0.035s	coverage: 11.3% of statements
ok  	k8s.io/minikube/pkg/minikube/notify	0.039s	coverage: 82.1% of statements
ok  	k8s.io/minikube/pkg/minikube/out	0.020s	coverage: 70.3% of statements
ok  	k8s.io/minikube/pkg/minikube/problem	0.008s	coverage: 100.0% of statements
ok  	k8s.io/minikube/pkg/minikube/proxy	0.010s	coverage: 100.0% of statements
ok  	k8s.io/minikube/pkg/minikube/registry	0.015s	coverage: 100.0% of statements
ok  	k8s.io/minikube/pkg/minikube/service	0.074s	coverage: 35.9% of statements
ok  	k8s.io/minikube/pkg/minikube/sshutil	0.715s	coverage: 75.0% of statements
ok  	k8s.io/minikube/pkg/minikube/translate	0.005s	coverage: 8.4% of statements
ok  	k8s.io/minikube/pkg/util/retry	0.002s	coverage: 0.0% of statements
Makefile:232: recipe for target 'test' failed
make: *** [test] Error 20
TravisBuddy Request Identifier: ba55a3c0-d7c3-11e9-b57f-f7c3bbd28520

@tstromberg
Copy link
Contributor

pkg/minikube/cluster/cluster.go:287: File is not `goimports`-ed (goimports)
		exit.WithCodeT(exit.Failure,"Unable to get the status of the cluster.")
cmd/minikube/cmd/delete.go:82:107: err.Name undefined (type error has no field or method Name) (typecheck)
			out.T(out.Meh, `"{{.name}}" cluster does not exist. Proceeding ahead with cleanup.`, out.V{"name": err.Name})

@blueelvis
Copy link
Contributor Author

@tstromberg - I fixed the linting and the above error mentioned. Any idea why Jenkins is failing? Doesn't seem to be related to this PR.

@tstromberg
Copy link
Contributor

hyperkit: #4206
virtualbox: Fixed in #5438
none: #5332

@tstromberg
Copy link
Contributor

Thank you!

@tstromberg tstromberg merged commit 213f93d into kubernetes:master Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hyperv: If there is no VM , minikube delete should be able to proceed
8 participants