Infer HyperKit HostIP as Gateway rather than hardcode to 192.168.64.1 #15720
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: p2c2e 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 |
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @p2c2e. 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/test-infra repository. |
k8s-ci-robot
added
the
size/XS
|
Can one of the admins verify this patch? |
| @@ -128,7 +128,9 @@ func HostIP(host *host.Host, clusterName string) (net.IP, error) { | |||
|
|
|||
| return net.ParseIP(ip), nil | |||
| case driver.HyperKit: | |||
| return net.ParseIP("192.168.64.1"), nil | |||
| vmIPString, _ := host.Driver.GetIP() | |||
| gatewayIPString := vmIPString[:strings.LastIndex(vmIPString, ".")+1] + "1" | |||
There was a problem hiding this comment.
Thanks for contributing to minikube, can you please put the output of minikube before/after this PR ?
and does this break normal users who currently dont have an issue ?
please comment what this code does? and how about using a library that handles incrementing the IPS instead of using string.
There was a problem hiding this comment.
- This PR specifically applies to MacOS users who have chosen Hyperkit (specified or since it is default). The code path does NOT affect any other combination of users.
- What does the code do?
90% of the users on MacOS using Hyperkit driver will have issues with mounting a filesystem (or anything that requires access to host from within VM). Unless the user is using Minikube or similar tools first time, there is a strong chance that the IP assigned to the VM will NOT be in 192.168.64.X range. On the host, we can check all existing dhcp leases on Mac in /var/db/dhcpd_leases. For regular users, the dhcp subnet assigned by Hyperkit will be a different range. In following example, I was assigned dhcpd lease with host IP of "192.168.205.5" and gateway/nameserver will then be "192.168.205.1" - Before PR* : attempting to mount a filesystem with “minikube mount” will fail via timeout since it will be trying on some non-existent IP.
$ minikube mount /tmp:/myhosttmp --v=7
📁 Mounting host path /tmp into VM as /myhosttmp ...
▪ Mount type:
▪ User ID: docker
▪ Group ID: docker
▪ Version: 9p2000.L
▪ Message Size: 262144
▪ Options: map[]
▪ Bind Address: 192.168.64.1:53740 <<<< PROBLEM: This IP is hardcoded and not right…..
🚀 Userspace file server: ufs starting
🛑 Userspace file server is shutdown
❌ Exiting due to GUEST_MOUNT_COULD_NOT_CONNECT: /bin/bash -c "sudo mount -t 9p -o dfltgid=$(grep ^docker: /etc/group | cut -d: -f3),dfltuid=$(id -u docker),msize=262144,port=53740,trans=tcp,version=9p2000.L 192.168.64.1 /myhosttmp": Process exited with status 32
stdout:
stderr:
mount: /myhosttmp: mount(2) system call failed: Connection timed out.
- After PR : attempting to mount a filesystem should just work… See below verbose log..
$ minikube mount /tmp:/myhosttmp --v=7
📁 Mounting host path /tmp into VM as /myhosttmp ...
▪ Mount type:
▪ User ID: docker
▪ Group ID: docker
▪ Version: 9p2000.L
▪ Message Size: 262144
▪ Options: map[]
▪ Bind Address: 192.168.205.1:53935 <<<< SOLVED : Uses correct ‘inferred’ IP. Works …
🚀 Userspace file server: ufs starting
✅ Successfully mounted /tmp to /myhosttmp
📌 NOTE: This process must stay alive for the mount to be accessible ...
- We cannot 'increment' IPs etc., in this case because it HAS to be a "1" for routing/gateway to the host. Any other solution will be more complicated.
Hope this clarifies. Seems like lot of folks have this issue and this PR will help them :)
p2c2e
requested review from
medyagh
and removed request for
prezha and
sharifelgamal
|
Hi @medyagh - Gentle nudge on this :) This will change will help lot of folks + help me avoid keeping a custom version of minikube! Thank You |
|
/cc @sharifelgamal Not sure if this is the right thing to do - Trying to bump this issues - since the reviewers were removed when requesting 're-review' and this is stuck in 'Changes requested'. Thanks! |
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
kvm2 driver with docker runtime Times for minikube ingress: 24.2s 28.2s 29.2s 27.6s 24.7s Times for minikube start: 54.1s 53.6s 54.3s 53.8s 54.4s docker driver with docker runtime Times for minikube ingress: 21.1s 21.1s 20.1s 20.1s 19.6s Times for minikube start: 27.5s 25.9s 26.3s 26.2s 25.6s docker driver with containerd runtime Times for minikube (PR 15720) start: 22.6s 22.8s 21.8s 21.3s 22.0s Times for minikube ingress: 19.6s 31.5s 79.5s 32.5s 19.5s |
|
These are the flake rates of all failed tests.
To see the flake rates of all tests by environment, click here. |
|
@p2c2e thank you for your patience on this, we have a long queue of PRs to review :) but the test look good |
Thanks - I completely understand. What is next though? As per the bot messages - I see need for "lgtm" before I can assign the PR to @sharifelgamal ; ( I sorely need a 'state diagram" to visualize the flow) |
fixes #11510
resolves #12729 (was closed without fix)
Rather than assume 192.168.64.1 - which might not be true if dhcpd lease is already active for some other VM etc.
We try to use the gateway IP is usually the hostip with the last octet as "1"