Support multi mount-string #21056
Support multi mount-string #21056
Conversation
k8s-ci-robot
added
the
cncf-cla: no
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bugwz 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 |
|
Welcome @bugwz! |
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @bugwz. 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 Once the patch is verified, the new status will be reflected by the 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-sigs/prow repository. |
|
Can one of the admins verify this patch? |
k8s-ci-robot
added
size/L
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
@bugwz can you plz share Before/After this PR output of minikube? |
This comment has been minimized.
This comment has been minimized.
|
@medyagh Below is the comparison information, please take a look～ Before: After: [bugwz@iZj6cibr8kn3thfefwcyg3Z minikube]$ ./out/minikube version
minikube version: v1.36.0
commit: 14c679030368174bde9cbd24753f8c141c6e6da0
[bugwz@iZj6cibr8kn3thfefwcyg3Z minikube]# ./out/minikube start --help
...
--mount-string='/home/bugwz:/minikube-host':
The argument to pass the minikube mount command on start, in a semicolon-separated format.
...
[bugwz@iZj6cibr8kn3thfefwcyg3Z minikube]$ ./out/minikube start --driver="podman" --mount --mount-string="/data01:/data01;/data02:/data02:Z,rshared"
😄 minikube v1.36.0 on Centos 8.5.2111 (amd64)
❗ The minimum required version for podman is "4.9.0". your version is "3.3.1". minikube might not work. use at your own risk. To install latest version please see https://podman.io/getting-started/installation.html
✨ Using the podman driver based on user configuration
🧯 The requested memory allocation of 3072MiB does not leave room for system overhead (total system memory: 3625MiB). You may face stability issues.
💡 Suggestion: Start minikube with less memory allocated: 'minikube start --memory=3072mb'
📌 Using Podman driver with root privileges
👍 Starting "minikube" primary control-plane node in "minikube" cluster
🚜 Pulling base image v0.0.47 ...
E0712 12:30:33.731925 33550 cache.go:225] Error downloading kic artifacts: not yet implemented, see issue #8426
🔥 Creating podman container (CPUs=2, Memory=3072MB) ...
🐳 Preparing Kubernetes v1.33.2 on Docker 28.1.1 ...
▪ Generating certificates and keys ...
▪ Booting up control plane ...
▪ Configuring RBAC rules ...
🔗 Configuring bridge CNI (Container Networking Interface) ...
🔎 Verifying Kubernetes components...
▪ Using image gcr.io/k8s-minikube/storage-provisioner:v5
🌟 Enabled addons: storage-provisioner, default-storageclass
🏄 Done! kubectl is now configured to use "minikube" cluster and "default" namespace by default
[bugwz@iZj6cibr8kn3thfefwcyg3Z minikube]$ sudo podman inspect minikube
[
{
"Id": "7cea00a807474ac6a111bc91ab33c9a857ca17831c1ba4df0521221bdca384b9",
"Created": "2025-07-12T12:30:43.641329142+08:00",
"Path": "/usr/local/bin/entrypoint",
"Args": [
"/sbin/init"
],
...
"Mounts": [
{
"Type": "volume",
"Name": "minikube",
"Source": "/var/lib/containers/storage/volumes/minikube/_data",
"Destination": "/var",
"Driver": "local",
"Mode": "",
"Options": [
"exec",
"nosuid",
"nodev",
"rbind"
],
"RW": true,
"Propagation": "rprivate"
},
{
"Type": "bind",
"Source": "/lib/modules",
"Destination": "/lib/modules",
"Driver": "",
"Mode": "",
"Options": [
"rbind"
],
"RW": false,
"Propagation": "rprivate"
},
{
"Type": "bind",
"Source": "/data01",
"Destination": "/data01",
"Driver": "",
"Mode": "",
"Options": [
"rbind"
],
"RW": true,
"Propagation": "rprivate"
},
{
"Type": "bind",
"Source": "/data02",
"Destination": "/data02",
"Driver": "",
"Mode": "",
"Options": [
"rbind"
],
"RW": true,
"Propagation": "rshared"
}
],
... |
|
thank you @bugwz can you plz fix the lint issue ? |
| Name: "relative container path", | ||
| MountString: "/foo/bar:baz/bat:private", | ||
| ExpectErr: true, | ||
| Name: "relative container path", |
There was a problem hiding this comment.
please add more unit test cases and include some test cases with semi colomon
cmd/minikube/cmd/start_flags.go
Outdated
| @@ -173,7 +173,7 @@ func initMinikubeFlags() { | |||
| startCmd.Flags().Bool(embedCerts, false, "if true, will embed the certs in kubeconfig.") | |||
| startCmd.Flags().StringP(containerRuntime, "c", constants.DefaultContainerRuntime, fmt.Sprintf("The container runtime to be used. Valid options: %s (default: auto)", strings.Join(cruntime.ValidRuntimes(), ", "))) | |||
| startCmd.Flags().Bool(createMount, false, "This will start the mount daemon and automatically mount files into minikube.") | |||
| startCmd.Flags().String(mountString, constants.DefaultMountDir+":/minikube-host", "The argument to pass the minikube mount command on start.") | |||
| startCmd.Flags().String(mountString, constants.DefaultMountDir+":/minikube-host", "The argument to pass the minikube mount command on start, in a semicolon-separated format.") | |||
There was a problem hiding this comment.
can add an example here for clarity
|
@medyagh Thank you for the suggestion, it has been updated. |
This comment has been minimized.
This comment has been minimized.
|
/retest-this-please |
There was a problem hiding this comment.
@bugwz does this mean multi-mount is only available for when users "starts" minikube?
how about if the user uses "minikube mount" command ?
I feel like this is a super useful feature and we should add some documents about it in our website maybe in tutorials
https://minikube.sigs.k8s.io/docs/handbook/mount/
https://minikube.sigs.k8s.io/docs/commands/mount/
There was a problem hiding this comment.
I tried manually, the "mount command" also needs to be updated so it can support multiple mount folders, would you plz add that feature there as well ?
$ minikube mount ~/.asdf/:/tmp1/;~/.vscode/:/tmp2
📁 Mounting host path /Users/medya/.asdf/ into VM as /tmp1/ ...
▪ Mount type: 9p
▪ User ID: docker
▪ Group ID: docker
▪ Version: 9p2000.L
▪ Message Size: 262144
▪ Options: map[]
▪ Bind Address: 192.168.106.1:59806
🚀 Userspace file server: ufs starting
✅ Successfully mounted /Users/medya/.asdf/ to /tmp1/
Manually checked only /tmp1 was inside minikube
This comment has been minimized.
This comment has been minimized.
|
@bugwz are you interested to add the same functionality to "minikube mount" command as well so it has feature parity? |
|
@medyagh Thank you for your suggestion. I will continue to improve as per your advice. Stay tuned!😜 |
k8s-ci-robot
added
the
needs-rebase
|
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-sigs/prow repository. |
|
@bugwz: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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-sigs/prow repository. I understand the commands that are listed here. |
|
Hello, I have submitted new changes, please conduct a code review. The relevant changes are as follows:
Design approach for minikube mount:
Although the 9p server processes messages in parallel, our execution of the |
|
kvm2 driver with docker runtime Times for minikube ingress: 14.9s 15.4s 14.9s 14.5s 14.9s Times for minikube start: 48.2s 47.1s 51.0s 49.2s 49.6s docker driver with docker runtime Times for minikube start: 25.4s 22.8s 21.8s 21.7s 25.9s Times for minikube ingress: 13.3s 12.8s 13.8s 13.3s 12.8s docker driver with containerd runtime Times for minikube ingress: 22.8s 22.8s 22.8s 22.8s 23.3s Times for minikube start: 25.1s 23.9s 22.1s 23.3s 22.5s |
|
@nirs please review this |
|
Is anyone following up? |
--mount is not use now. For --mount-string we should not use non standard format like this. A better way would be to use multiple --mount-string flags using cmd.Flags().StringArray(): This will be parsed as: The advantage is not having to use non-standard separator for users, and having to write special code for parsing the flags. We ware adding virtiofs support (currently in vfkit and krunkit), so we need to pass options that can work with virtiofs. This should work also with 9p (legacy mount for other drivers), and for podamn/docker we need the special Z option. We have issues with how --mount-options and other flags like --mount-uid and --mount=guid will be handled. We want to depracate these flags in 1.37 and remove them in 1.38, so options will be specified per mount. See #21149 |
We should not support such thing, we can accept multiple arguments: But do can we mount multiple mounts with single mount command? It makes sense if we can run single mount server serving multiple commands. If we actually start multiple servers it is better to support one mount argument. |
When using
minikube start, we can pass in multiple paths that we want to mount inside the container through themount-stringparameter, for example:minikube start --mount --mount-string="/data01:/data01;/data02:/data02:Z,rshared"