fix(utils): make install-kubectl.sh work on macOS#976
Conversation
install-kubectl.sh always downloaded the linux/amd64 kubectl, so on macOS
it installed an unexecutable Linux ELF (later kubectl calls fail with an
exec format error). Detect OS (uname -s) and arch (uname -m) and download
the matching bin/{linux,darwin}/{amd64,arm64}/kubectl, mirroring the OS/arch
detection added to install-minikube-cluster.sh in vllm-project#970.
Also: treat an existing kubectl as installed only if it actually runs
(`kubectl version --client`), so a stale Linux binary left on PATH on macOS
is replaced rather than skipped; and use `curl -f` so HTTP errors abort
instead of saving an error page as the binary.
Follow-up to the review on vllm-project#970; relates to vllm-project#931.
Signed-off-by: HumphreySun98 <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the utils/install-kubectl.sh script to dynamically detect the host OS and architecture (supporting Linux/Darwin and amd64/arm64) to download the appropriate kubectl binary, and improves the kubectl_exists check to ensure the binary is executable. The reviewer suggested extracting the nested curl command used for fetching the stable version into a separate step with explicit error handling to improve robustness and debuggability.
| curl -LO "https://dl.k8s.io/release/$(curl -L -s https://dl.k8s.io/release/stable.txt)/bin/linux/amd64/kubectl" | ||
| # Install kubectl for the detected platform. -f makes curl fail on HTTP errors | ||
| # instead of saving an error page as the binary. | ||
| curl -fLO "https://dl.k8s.io/release/$(curl -fL -s https://dl.k8s.io/release/stable.txt)/bin/${HOST_OS}/${HOST_ARCH}/kubectl" |
There was a problem hiding this comment.
Nesting the curl command to fetch the stable version inside the main download URL makes error handling and debugging difficult if the network request fails.
If the stable version lookup fails (e.g., due to a temporary network issue or DNS failure), the inner curl will fail silently (due to -s), resulting in an empty version string and a cryptic 404 error from the outer curl.
Consider extracting the version lookup into its own step with explicit error handling. This improves robustness and provides a clear error message to the user.
| curl -fLO "https://dl.k8s.io/release/$(curl -fL -s https://dl.k8s.io/release/stable.txt)/bin/${HOST_OS}/${HOST_ARCH}/kubectl" | |
| if ! KUBECTL_VERSION="$(curl -fL -s https://dl.k8s.io/release/stable.txt)" || [ -z "$KUBECTL_VERSION" ]; then | |
| echo "ERROR: Failed to fetch the latest stable kubectl version" >&2 | |
| exit 1 | |
| fi | |
| curl -fLO "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/${HOST_OS}/${HOST_ARCH}/kubectl" |
2 participants
Summary
Follow-up to the review on #970 (and same macOS root cause as #931):
utils/install-kubectl.shalways downloaded thelinux/amd64kubectl, so on macOS it installs an unexecutable Linux ELF and laterkubectlcalls fail with an exec-format error. #970 fixedinstall-minikube-cluster.sh, which calls this script — so without this change the end-to-end flow still breaks on macOS at the kubectl step.Changes
uname -s→ linux/darwin) and arch (uname -m→ amd64/arm64) and download the matchingbin/{linux,darwin}/{amd64,arm64}/kubectl, mirroring the detection added toinstall-minikube-cluster.shin fix(utils): make install-minikube-cluster.sh work on macOS #970.kubectl version --client), so a stale Linux binary left onPATHon macOS is replaced instead of skipped (this was the exact#931failure mode).curl -fso an HTTP error aborts the script (viaset -e) instead of saving an error page as the binary.Linux behavior is unchanged:
HOST_OS/HOST_ARCHresolve tolinux/amd64on a normal x86_64 Linux host, giving the same URL as before.Test plan
bash -n utils/install-kubectl.sh— passes (shellcheck runs in CI).bin/linux/amd64/kubectl(identical to the previous hardcoded URL).bin/darwin/arm64/kubectl; a previously-installed Linux binary onPATHno longer short-circuits the install.AI assistance was used; I have reviewed, run, and can defend every change.