kubernetes / minikube

Run Kubernetes locally
https://minikube.sigs.k8s.io/
Apache License 2.0
29.45k stars 4.88k forks source link

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

Open robrich opened 4 years ago

robrich commented 4 years ago

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 commented 4 years ago

Complete guess:

Change https://github.com/kubernetes/minikube/blob/01454778ac81743d5d3adbf4d9794b0b7fccfee3/pkg/minikube/registry/drvs/hyperv/powershell.go#L33 from

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

to

func init() {
    powershell, _ = exec.LookPath("powershell") || exec.LookPath("pwsh")
}
afbjorklund commented 4 years ago

Inherited from https://github.com/docker/machine/issues/4789 and https://github.com/docker/machine/issues/4826

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

tstromberg commented 4 years ago

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 commented 4 years ago

It looks like https://github.com/docker/machine/issues/4826 identifies this issue there, and https://github.com/docker/machine/pull/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 commented 4 years ago

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 commented 4 years ago

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

afbjorklund commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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

robrich commented 3 years ago

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 commented 3 years ago

/remove-lifecycle stale

let's fix this

fejta-bot commented 3 years ago

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

robrich commented 3 years ago

Unstale please.

fejta-bot commented 3 years ago

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

robrich commented 3 years ago

Unstale please