Disable default hypervisors mounts + fix 9P path on Darwin #6806

starbassma · 2020-02-26T11:01:30Z

Hello,

this PR :

Fix default 9P mount path on Darwin ($HOME instead of /Users, same path on all OS)
Add the default 9P mount path for Xhyve Hypervisor (doc)
Disable default mounts provided by the hypervisors (lax behavior, can lead to data loss on the host machine)

check
fixes #6788

Regards,

…or Xhyve

k8s-ci-robot · 2020-02-26T11:01:38Z

Welcome @starbassma!

It looks like this is your first PR to kubernetes/minikube 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/minikube has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2020-02-26T11:01:38Z

Hi @starbassma. 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 · 2020-02-26T11:01:54Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: starbassma
To complete the pull request process, please assign medyagh
You can assign the PR to them by writing /assign @medyagh 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:

OWNERS

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

minikube-bot · 2020-02-26T11:03:04Z

Can one of the admins verify this patch?

medyagh · 2020-02-26T23:31:51Z

/ok-to-test

minikube-pr-bot · 2020-02-26T23:37:01Z

Error: running mkcmp: exit status 1

starbassma · 2020-02-27T22:43:41Z

mkcmp

seems unrelated to the modifications

priyawadhwa · 2020-02-28T00:01:47Z

@starbassma yah you can ignore mkcmp errors here

medyagh · 2020-02-26T16:58:50Z

cmd/minikube/cmd/start.go

@@ -190,7 +190,7 @@ func initKubernetesFlags() {
 // initDriverFlags inits the commandline flags for vm drivers
 func initDriverFlags() {
 	startCmd.Flags().String("vm-driver", "", fmt.Sprintf("Driver is one of: %v (defaults to auto-detect)", driver.DisplaySupportedDrivers()))
-	startCmd.Flags().Bool(disableDriverMounts, false, "Disables the filesystem mounts provided by the hypervisors")
+	startCmd.Flags().Bool(disableDriverMounts, true, "Disables the filesystem mounts provided by the hypervisors")


is this gonna break the Corp Cert Tutorial? https://minikube.sigs.k8s.io/docs/tutorials/untrusted_root_certificate/

No, it won't, certificates are copied through the api

medyagh · 2020-02-26T17:00:02Z

pkg/minikube/constants/constants_darwin.go

+	"k8s.io/client-go/util/homedir"
+)
+
+// DefaultMountDir is the default mount directory for Darwin


remove comment "For Darwin "

medyagh · 2020-02-28T17:43:09Z

cmd/minikube/cmd/start.go

@@ -190,7 +190,7 @@ func initKubernetesFlags() {
 // initDriverFlags inits the commandline flags for vm drivers
 func initDriverFlags() {
 	startCmd.Flags().String("vm-driver", "", fmt.Sprintf("Driver is one of: %v (defaults to auto-detect)", driver.DisplaySupportedDrivers()))
-	startCmd.Flags().Bool(disableDriverMounts, false, "Disables the filesystem mounts provided by the hypervisors")
+	startCmd.Flags().Bool(disableDriverMounts, true, "Disables the filesystem mounts provided by the hypervisors")


double negative sin the flags are tricky,

I agree, should we consider renaming this flag ?

yes I would say lets rename this flag !

codecov-io · 2020-02-28T19:27:53Z

Codecov Report

Merging #6806 into master will decrease coverage by 0.19%.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           master   #6806     +/-   ##
========================================
- Coverage    38.5%   38.3%   -0.2%     
========================================
  Files         142     142             
  Lines        8695    8740     +45     
========================================
  Hits         3348    3348             
- Misses       4927    4971     +44     
- Partials      420     421      +1

Impacted Files	Coverage Δ
cmd/minikube/cmd/start.go	`32.74% <100%> (ø)`	⬆️
pkg/minikube/machine/delete.go	`29.72% <0%> (-16.11%)`	⬇️
cmd/minikube/cmd/service.go	`9.85% <0%> (-5.7%)`	⬇️
pkg/minikube/machine/start.go	`53.84% <0%> (-1.16%)`	⬇️
pkg/addons/addons.go	`45.12% <0%> (-0.56%)`	⬇️
cmd/minikube/cmd/delete.go	`24.88% <0%> (ø)`	⬆️
cmd/minikube/cmd/config/profile.go	`0% <0%> (ø)`	⬆️

minikube-pr-bot · 2020-02-28T19:34:25Z

All Times minikube: [ 92.772734 92.923790 92.800023]
All Times Minikube (PR 6806): [ 95.314334 93.190668 92.015382]

Average minikube: 92.832182
Average Minikube (PR 6806): 93.506794

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 6806) |
+----------------------+-----------+--------------------+
| minikube v           |  0.259006 |           0.216577 |
| Creating kvm2        | 20.600817 |          20.147557 |
| Preparing Kubernetes | 49.354341 |          49.406815 |
| Pulling images       |           |                    |
| Launching Kubernetes | 21.284509 |          21.952370 |
| Waiting for cluster  |  0.069547 |           0.397761 |
+----------------------+-----------+--------------------+

afbjorklund · 2020-03-01T22:15:23Z

site/content/en/docs/Tasks/mount.md

@@ -71,10 +71,17 @@ Some hypervisors, have built-in host folder sharing. Driver mounts are reliable
 | VirtualBox | macOS | /Users | /Users |
 | VirtualBox | Windows | C://Users | /c/Users |
 | VMware Fusion | macOS | /Users | /Users |
+| Xhyve | macOS | /Users | /Users |


The xhyve driver is not supported anymore, see 914713f

k8s-ci-robot · 2020-03-12T18:59:14Z

@starbassma: PR needs rebase.

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.

tstromberg · 2020-04-09T11:13:17Z

I agree with this PR - but will close until it can be rebased.

Please re-open it when it should be reviewed again.

starbassma added 2 commits February 26, 2020 11:29

fix default 9P mount path on Darwin + doc update

7549bb1

disable default mounts provided by the hypervisors + doc mount path f…

944cada

…or Xhyve

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 26, 2020

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 Feb 26, 2020

k8s-ci-robot requested review from blueelvis and RA489 February 26, 2020 11:02

starbassma changed the title ~~Disbale default hypervisors mounts + fix 9P path on Darwin~~ Disable default hypervisors mounts + fix 9P path on Darwin Feb 26, 2020

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 26, 2020

medyagh requested changes Feb 28, 2020

View reviewed changes

delete comm

4a45500

starbassma requested a review from medyagh February 28, 2020 20:18

afbjorklund reviewed Mar 1, 2020

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 12, 2020

tstromberg closed this Apr 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable default hypervisors mounts + fix 9P path on Darwin #6806

Disable default hypervisors mounts + fix 9P path on Darwin #6806

starbassma commented Feb 26, 2020

k8s-ci-robot commented Feb 26, 2020

k8s-ci-robot commented Feb 26, 2020

k8s-ci-robot commented Feb 26, 2020

minikube-bot commented Feb 26, 2020

medyagh commented Feb 26, 2020

minikube-pr-bot commented Feb 26, 2020

starbassma commented Feb 27, 2020

priyawadhwa commented Feb 28, 2020

medyagh Feb 26, 2020

starbassma Feb 28, 2020

medyagh Feb 26, 2020

starbassma Feb 28, 2020

medyagh Feb 28, 2020

starbassma Feb 28, 2020

medyagh Mar 2, 2020

codecov-io commented Feb 28, 2020

minikube-pr-bot commented Feb 28, 2020

afbjorklund Mar 1, 2020 •

edited

k8s-ci-robot commented Mar 12, 2020

tstromberg commented Apr 9, 2020 •

edited

Disable default hypervisors mounts + fix 9P path on Darwin #6806

Disable default hypervisors mounts + fix 9P path on Darwin #6806

Conversation

starbassma commented Feb 26, 2020

k8s-ci-robot commented Feb 26, 2020

k8s-ci-robot commented Feb 26, 2020

k8s-ci-robot commented Feb 26, 2020

minikube-bot commented Feb 26, 2020

medyagh commented Feb 26, 2020

minikube-pr-bot commented Feb 26, 2020

starbassma commented Feb 27, 2020

priyawadhwa commented Feb 28, 2020

medyagh Feb 26, 2020

Choose a reason for hiding this comment

starbassma Feb 28, 2020

Choose a reason for hiding this comment

medyagh Feb 26, 2020

Choose a reason for hiding this comment

starbassma Feb 28, 2020

Choose a reason for hiding this comment

medyagh Feb 28, 2020

Choose a reason for hiding this comment

starbassma Feb 28, 2020

Choose a reason for hiding this comment

medyagh Mar 2, 2020

Choose a reason for hiding this comment

codecov-io commented Feb 28, 2020

Codecov Report

minikube-pr-bot commented Feb 28, 2020

afbjorklund Mar 1, 2020 • edited

Choose a reason for hiding this comment

k8s-ci-robot commented Mar 12, 2020

tstromberg commented Apr 9, 2020 • edited

afbjorklund Mar 1, 2020 •

edited

tstromberg commented Apr 9, 2020 •

edited