Skip to content
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

minikube docker-env doesn't understand powershell core #9183

Open
robrich opened this issue Sep 3, 2020 · 15 comments
Open

minikube docker-env doesn't understand powershell core #9183

robrich opened this issue Sep 3, 2020 · 15 comments
Labels
cause/libmachine-bug Bugs caused by libmachine help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. os/windows priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@robrich
Copy link
Contributor

robrich commented Sep 3, 2020

Steps to reproduce the issue:

  1. Install PowerShell core (https://github.com/PowerShell/PowerShell/releases)
  2. Run powershell core (pwsh.exe)
  3. minikube start ...
  4. minikube docker-env

Full output of failed command:

❯ minikube docker-env
You can further specify your shell with either 'cmd' or 'powershell' with the --shell flag.

SET DOCKER_TLS_VERIFY=1
SET DOCKER_HOST=tcp://192.168.184.153:2376
SET DOCKER_CERT_PATH=C:\Users\Me\.minikube\certs
SET MINIKUBE_ACTIVE_DOCKERD=minikube
REM To point your shell to minikube's docker-daemon, run:
REM @FOR /f "tokens=*" %i IN ('minikube -p minikube docker-env') DO @%i

Expected output

❯ minikube docker-env --shell powershell
$Env:DOCKER_TLS_VERIFY = "1"
$Env:DOCKER_HOST = "tcp://192.168.184.153:2376"
$Env:DOCKER_CERT_PATH = "C:\Users\Me\.minikube\certs"
$Env:MINIKUBE_ACTIVE_DOCKERD = "minikube"
# To point your shell to minikube's docker-daemon, run:
# & minikube -p minikube docker-env | Invoke-Expression

Note how passing in --shell powershell yielded the correct results, but using the default auto-detect did not detect powershell.

Note: running on regular powershell works just fine. This issue only affects powershell core.

@robrich
Copy link
Contributor Author

robrich commented Sep 3, 2020

Complete guess:

Change

powershell, _ = exec.LookPath("powershell")
from

func init() {
	powershell, _ = exec.LookPath("powershell")
}

to

func init() {
	powershell, _ = exec.LookPath("powershell") || exec.LookPath("pwsh")
}

@afbjorklund
Copy link
Collaborator

afbjorklund commented Sep 3, 2020

Inherited from docker/machine#4789 and docker/machine#4826

I think you mean || rather than && ? (it is pseudo-code, but)

@afbjorklund afbjorklund added cause/libmachine-bug Bugs caused by libmachine os/windows priority/backlog Higher priority than priority/awaiting-more-evidence. kind/bug Categorizes issue or PR as related to a bug. labels Sep 3, 2020
@tstromberg
Copy link
Contributor

At the moment, we defer to github.com/docker/machine/libmachine/shell for it's shell detection. I wonder if there is a better library for us to use.

@robrich
Copy link
Contributor Author

robrich commented Sep 4, 2020

It looks like docker/machine#4826 identifies this issue there, and docker/machine#4827 is set to fix it ... but currently yields a broken build.

@afbjorklund you are completely correct. Comment updated. And I see the rest of my comment is redundant to your post too. :D

@afbjorklund
Copy link
Collaborator

We have already forked the code, so will have to fix it (as you outlined, just look for the new name and blame the lack of backwards compatibility on Windows)

@medyagh
Copy link
Member

medyagh commented Oct 7, 2020

@afbjorklund do we have a plan to pull that code ?

@medyagh medyagh added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed priority/backlog Higher priority than priority/awaiting-more-evidence. labels Oct 7, 2020
@afbjorklund
Copy link
Collaborator

I didn't see any PR, but it should be straightforward to fix ? Apparently there is another place, in "hyperv.go":

pkg/minikube/registry/drvs/hyperv/hyperv.go:	path, err := exec.LookPath("powershell")
pkg/minikube/registry/drvs/hyperv/powershell.go:	powershell, _ = exec.LookPath("powershell")

@NotWearingPants
Copy link

I've closed my issue & PR in favor of docker/machine#4788 who solved it first with the exact same fix, and lucked out to not have a broken build.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 25, 2021
@robrich
Copy link
Contributor Author

robrich commented Feb 25, 2021

Unstale please. In theory this is a 2-line fix. I definitely don't have enough perspective to criticize, but a few months of inactivity seems a mistake. What's the fastest path to resolution?

@prezha
Copy link
Contributor

prezha commented Mar 3, 2021

/remove-lifecycle stale

let's fix this

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 3, 2021
@prezha prezha added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Mar 3, 2021
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 1, 2021
@robrich
Copy link
Contributor Author

robrich commented Jun 2, 2021

Unstale please.

@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 2, 2021
@robrich
Copy link
Contributor Author

robrich commented Jul 2, 2021

Unstale please

@spowelljr spowelljr added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Jul 14, 2021
@spowelljr spowelljr removed the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Dec 1, 2021
@spowelljr spowelljr added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cause/libmachine-bug Bugs caused by libmachine help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. os/windows priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Development

No branches or pull requests

9 participants