-
Notifications
You must be signed in to change notification settings - Fork 182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set correct powershell path for Windows #47
Conversation
@kvaps could you have a look again? |
kindly ping @kvaps |
I'm not sure if they ever got back to you but it doesn't work for me on my 2019 EKS v1.27 optimised ami nodes
|
@elitistphoenix could you check what errors are happening in a different shell while running the node-shell command? |
same (obviously) for the -- echo 123 one too |
@feiskyer sorry for late reply, and thank you for your contribution. The main problem is that I wish to see something like: os="$(kubectl get node -o jsonpath="{.metadata.labels.kubernetes\.io/os}-{.metadata.labels.kubernetes\.io/arch}" pve1 || exit 1)"
case $os in
linux-*)
image="${KUBECTL_NODE_SHELL_IMAGE:-docker.io/library/alpine}"
name="nsenter"
;;
windows-amd64)
image="${KUBECTL_NODE_SHELL_IMAGE_WINDOWS_AMD64:-mcr.microsoft.com/powershell}"
name="pwsh"
;;
windows-arm64)
image="${KUBECTL_NODE_SHELL_IMAGE_WINDOWS_ARM64:-mcr.microsoft.com/powershell}"
name="pwsh"
;;
esac
|
Since a hostprocess container is spawned, what about https://github.com/microsoft/windows-host-process-containers-base-image ? Then, the powershell from host can be used. |
Switching Windows container image to mcr.microsoft.com/oss/kubernetes/windows-host-process-containers-base-image:v1.0.0 so that all Windows container versions are supported.
Thanks for the suggestions. Tried this new image and indeed working well on both 2022 and 2019. PR updated with this base image. PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
Fix #46