minikube service opens in browser when it should not

[retracile]

The exact command to reproduce the issue:

minikube service --namespace mynamespace --url mynodeportservice

The full output of the command that failed:

In versions of minikube through 1.3.1, this output the service URL to stdout, and did not open the URL in the default browser. In 1.5.1, it outputs the URL and the "🎉 Opening kubernetes service ..." message to stdout, and it opens in the default browser.

With --url=false, it does not open in the browser, but now it outputs an ascii-art table instead of the plain URL.
With --url=true, it behaves the same as with --url.

Based on finding #5699, I tried using --format="{{ .IP }}". That leads to even greater insanity... it emits the message about opening the service in the default browser, but then fails to do so because it tries to open a file in the current directory matching the IP address, and errors out.

Please restore the older behavior so scripts can get the URL for a service, and fix --format to not try to open the resulting string in the default browser.

The output of the minikube logs command:

The operating system version:
Mojave
minikube 1.5.1 installed via homebrew

[tstromberg]

This sounds like a regression, possibly caused by #5718. Perhaps the urlMode logic got inverted?

/cc @nanikjava

[retracile]

Looks like the logic moved from pkg/minikube/servcice/service.go to cmd/minikube/cmd/service.go between 1.3.1 and 1.5.1.
The logic in the command code is only looking at the length of urlString list to determine if it should open the browser, and is not considering the value of serviceURLMode.

[retracile]

This might improve matters; haven't tried it yet

--- a/cmd/minikube/cmd/service.go
+++ b/cmd/minikube/cmd/service.go
@@ -82,7 +82,7 @@ var serviceCmd = &cobra.Command{
                        exit.WithError("Error opening service", err)
                }

-               if len(urlString) != 0 {
+               if !serviceURLMode && len(urlString) != 0 {
                        out.T(out.Celebrate, "Opening kubernetes service  {{.namespace_name}}/{{.service_name}} in default browser...", out.V{"namespace_name": namespace, "service_name": svc})

                        for _, url := range urlString {

[tstromberg]

@retracile - Thanks, I'm working on a PR now with an integration test to avoid future similar breakage.

This definitely revealed a short-coming in our testing.

[retracile]

Cool, thanks. :)
Also, the out.T message has two spaces between "service" and "{{"; should probably only have one.

[retracile]

A quick local test shows that change fixes it for me, and --url --format "{{.IP}}" also works. :)

[tstromberg]

@retracile - Do you mind checking if #5790 looks good to you?

You can test it on macOS by using:

curl -LO https://storage.googleapis.com/minikube-builds/5790/minikube-darwin-amd64 
chmod u+x minikube-darwin-amd64
./minikube-darwin-amd64 service <flags>

[retracile]

There's no newline in the --url output, but there was in v1.5.0

Tangent: Under some situations I'm also seeing a * prefix on the output... this is very close to when the service is created, and I've seen that in older versions as well. I have not found a way to consistently reproduce that, so hadn't reported it. I haven't seen anything in the docs to indicate what that might be.

[tstromberg]

The newline seemed incorrect to me, so I removed it out. I could preserve if it it's useful.

What command are you you seeing the * prefix?

[retracile]

*NIX commands generally include the newline at the end; otherwise you wind up with your shell prompt tacked on the end of the output (when using it interactively). When using it within a script, it works either way.

[tstromberg]

Good point. Will add newline back.

…

On Wed, Oct 30, 2019, 12:13 PM retracile ***@***.***> wrote: *NIX commands generally include the newline at the end; otherwise you wind up with your shell prompt tacked on the end of the output (when using it interactively). When using it within a script, it works either way. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#5789?email_source=notifications&email_token=AAAYYMASKYH6YWOH4KBGLQLQRHMF3A5CNFSM4JG4WQQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECVOGFI#issuecomment-548070165>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAYYMB7PGDFOIHQCZCHQ5DQRHMF3ANCNFSM4JG4WQQQ> .

[retracile]

For the * prefix, that's a separate issue, and dates way back. I see it when running the same command discussed here, but it appears to only happen for a brief period after the service is created... brief enough that I don't think I've ever managed to reproduce it outside of automated scripting. Maybe it happens in the time between the service being created and it being ready? Or something? ... I'm guessing there.

Regardless, it's a separate issue from this one.

[tstromberg]

@retracile - Regardless, it sounds wrong. If you know of a way to reproduce it, let me know.

The asterisk is likely caused by a call to out.T() rather than the raw equivalent out.String(). I removed the calls I saw, but there may be others.

[retracile]

out.T() vs out.String() is a useful clue; maybe I can track it down at some point. Thanks.

[tstromberg]

Reopening until we release v1.5.2 containing this fix (tomorrow).

[tstromberg]

Fixed in v1.5.2. Thank you for reporting this issue!