Repository navigation
vmnet: Fix brew install path on Intel Macs and support brew bin path - #23039
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nirs The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/ok-to-test |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
This PR updates vmnet-helper discovery on macOS to support Homebrew installs on both Intel and Apple Silicon, and to handle vmnet-helper v1.0.0 moving from libexec to bin. It also limits Homebrew-path probing to macOS 26+ where Homebrew is the recommended install method.
Changes:
- Add
detect.BrewPrefix()to choose the default Homebrew prefix based on architecture. - Update vmnet-helper lookup to (on macOS 26+) prefer
<brewPrefix>/bin/vmnet-helper, then fall back to<brewPrefix>/opt/vmnet-helper/libexec/vmnet-helper, and finally the legacy/opt/vmnet-helper/bin/vmnet-helper. - Parse macOS version once and pass a
semver.Versionthrough helper selection and sudo-requirement logic; update unit tests accordingly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/minikube/detect/detect.go | Adds BrewPrefix() for Homebrew prefix selection. |
| pkg/drivers/common/vmnet/vmnet.go | Updates vmnet-helper path discovery to support Intel prefix + bin/libexec locations and gates Homebrew probing on macOS 26+. |
| pkg/drivers/common/vmnet/vmnet_test.go | Adjusts tests to match the new semver.Version macOS version plumbing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
I noticed we used homebrew in other parts of code like qemu
for example qemuFirmwarePath
is that possible or should we use it there too ?
We can use this for qemu firmware path and socket_vment. I can do a followup after we finish with 1.39. |
|
@medyagh Copilot comments addressed |
The brew install path was hardcoded to /opt/homebrew which is only correct on Apple Silicon. On Intel Macs Homebrew uses /usr/local. Add detect.BrewPrefix() returning the default Homebrew prefix for the current OS and architecture. Returns an empty string on unsupported platforms (e.g. Windows). Use it to build the vmnet-helper brew path.
Homebrew is the recommended install method only on macOS 26 and later. On older macOS versions only the legacy install path should be searched. Parse the macOS version once in getHelperInfo() and pass the parsed semver.Version to both findHelper() and helperNeedsSudo(), avoiding redundant parsing.
vmnet-helper >= 1.0 will install to the brew bin directory instead of libexec. Search the new bin path first, falling back to the current libexec path for older versions.
Split vmnet-helper path construction from findHelper() into helperSearchPaths(). findHelper() now only searches the returned paths in order; helperSearchPaths() builds the list based on macOS version and Homebrew prefix. This makes the expected search order explicit and testable without touching the filesystem or requiring vmnet-helper to be installed. Add TestHelperSearchPaths with static expected paths for: - macOS 15: legacy install path only - macOS 26: brew bin and libexec paths, then legacy fallback Intel and Apple Silicon brew prefixes are covered via runtime.GOARCH in the test expectations.
|
kvm2 driver with docker runtime DetailsTimes for minikube start: 41.9s 40.0s 40.1s 39.1s 39.1s Times for minikube ingress: 19.3s 19.3s 19.3s 18.8s 20.4s docker driver with docker runtime DetailsTimes for minikube start: 22.4s 19.1s 21.5s 18.5s 21.4s Times for minikube ingress: 10.7s 12.7s 12.7s 12.6s 12.6s docker driver with containerd runtime DetailsTimes for minikube start: 19.2s 16.6s 19.9s 16.2s 16.8s Times for minikube ingress: 23.7s 23.6s 23.7s 23.7s 23.6s |
|
/retest-required |
|
/test pull-minikube-vfkit-docker-macos-arm |
|
@nirs: The following tests 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. DetailsInstructions 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 8BD0 commands that are listed here. |
Fix brew path on Intel Macs and support brew bin path for vmnet-helper v1.0.0.
Fix brew install path on Intel Macs: the brew path was hardcoded to
/opt/homebrew which is only correct on Apple Silicon. Add
detect.BrewPrefix() returning the correct Homebrew prefix for the
current architecture.
Search brew install path only on macOS 26+: Homebrew is the
recommended install method only on macOS 26 and later. On older macOS
versions only the legacy install path is searched.
Support vmnet-helper brew install in bin: vmnet-helper >= 1.0 will
install to the brew bin directory instead of libexec. Search the new
bin path first, falling back to the current libexec path for older
versions.
Fixes #23038
Fixes #22915