service: fix --url mode, add integration tests

cmd/minikube/cmd/service.go

@@ -18,17 +18,18 @@ package cmd
 
 import (
 	"fmt"
+	"net/url"
 	"os"
 	"text/template"
 
+	"github.com/golang/glog"
 	"github.com/pkg/browser"
-
-	"k8s.io/minikube/pkg/minikube/out"
-
 	"github.com/spf13/cobra"
+
 	"k8s.io/minikube/pkg/minikube/cluster"
 	"k8s.io/minikube/pkg/minikube/exit"
 	"k8s.io/minikube/pkg/minikube/machine"
+	"k8s.io/minikube/pkg/minikube/out"
 	"k8s.io/minikube/pkg/minikube/service"
 )
 
@@ -74,21 +75,26 @@ var serviceCmd = &cobra.Command{
 			os.Exit(1)
 		}
 
-		var urlString []string
-
-		urlString, err = service.WaitForService(api, namespace, svc,
-			serviceURLTemplate, serviceURLMode, https, wait, interval)
+		urls, err := service.WaitForService(api, namespace, svc, serviceURLTemplate, serviceURLMode, https, wait, interval)
 		if err != nil {
 			exit.WithError("Error opening service", err)
 		}
 
-		if 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 _, u := range urls {
+			_, err := url.Parse(u)
+			if err != nil {
+				glog.Warningf("failed to parse url %q: %v (will not open)", u, err)
+				out.String(fmt.Sprintf("%s\n", u))
+				continue
+			}
 
-			for _, url := range urlString {
-				if err := browser.OpenURL(url); err != nil {
-					exit.WithError(fmt.Sprintf("browser failed to open url %s", url), err)
-				}
+			if serviceURLMode {
+				out.String(fmt.Sprintf("%s\n", u))
+				continue
+			}
+			out.T(out.Celebrate, "Opening service {{.namespace_name}}/{{.service_name}} in default browser...", out.V{"namespace_name": namespace, "service_name": svc})
+			if err := browser.OpenURL(u); err != nil {
+				exit.WithError(fmt.Sprintf("open url failed: %s", u), err)
 			}
 		}
 	},

pkg/minikube/service/service.go

@@ -300,12 +300,8 @@ func WaitForService(api libmachine.API, namespace string, service string, urlTem
 	}
 
 	for _, bareURLString := range serviceURL.URLs {
-		url, isHTTPSchemedURL := OptionallyHTTPSFormattedURLString(bareURLString, https)
-
-		if urlMode || !isHTTPSchemedURL {
-			out.T(out.Empty, url)
-			urlList = append(urlList, url)
-		}
+		url, _ := OptionallyHTTPSFormattedURLString(bareURLString, https)
+		urlList = append(urlList, url)
 	}
 	return urlList, nil
 }

test/integration/functional_test.go

@@ -89,7 +89,7 @@ func TestFunctional(t *testing.T) {
 			{"LogsCmd", validateLogsCmd},
 			{"MountCmd", validateMountCmd},
 			{"ProfileCmd", validateProfileCmd},
-			{"ServicesCmd", validateServicesCmd},
+			{"ServiceCmd", validateServiceCmd},
 			{"AddonsCmd", validateAddonsCmd},
 			{"PersistentVolumeClaim", validatePersistentVolumeClaim},
 			{"TunnelCmd", validateTunnelCmd},
@@ -397,13 +397,77 @@ func validateProfileCmd(ctx context.Context, t *testing.T, profile string) {
 }
 
 // validateServiceCmd asserts basic "service" command functionality
-func validateServicesCmd(ctx context.Context, t *testing.T, profile string) {
-	rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "service", "list"))
+func validateServiceCmd(ctx context.Context, t *testing.T, profile string) {
+	rr, err := Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "create", "deployment", "hello-node", "--image=gcr.io/hello-minikube-zero-install/hello-node"))
+	if err != nil {
+		t.Logf("%s failed: %v (may not be an error)", rr.Args, err)
+	}
+	rr, err = Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "expose", "deployment", "hello-node", "--type=NodePort", "--port=8080"))
+	if err != nil {
+		t.Logf("%s failed: %v (may not be an error)", rr.Args, err)
+	}
+
+	if _, err := PodWait(ctx, t, profile, "default", "app=hello-node", 4*time.Minute); err != nil {
+		t.Fatalf("wait: %v", err)
+	}
+
+	rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "service", "list"))
 	if err != nil {
 		t.Errorf("%s failed: %v", rr.Args, err)
 	}
-	if !strings.Contains(rr.Stdout.String(), "kubernetes") {
-		t.Errorf("service list got %q, wanted *kubernetes*", rr.Stdout.String())
+	if !strings.Contains(rr.Stdout.String(), "hello-node") {
+		t.Errorf("service list got %q, wanted *hello-node*", rr.Stdout.String())
+	}
+
+	// Test --https --url mode
+	rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "service", "--namespace=default", "--https", "--url", "hello-node"))
+	if err != nil {
+		t.Fatalf("%s failed: %v", rr.Args, err)
+	}
+	if rr.Stderr.String() != "" {
+		t.Errorf("unexpected stderr output: %s", rr.Stderr)
+	}
+
+	endpoint := strings.TrimSpace(rr.Stdout.String())
+	u, err := url.Parse(endpoint)
+	if err != nil {
+		t.Fatalf("failed to parse %q: %v", endpoint, err)
+	}
+	if u.Scheme != "https" {
+		t.Errorf("got scheme: %q, expected: %q", u.Scheme, "https")
+	}
+
+	// Test --format=IP
+	rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "service", "hello-node", "--url", "--format={{.IP}}"))
+	if err != nil {
+		t.Errorf("%s failed: %v", rr.Args, err)
+	}
+	if strings.TrimSpace(rr.Stdout.String()) != u.Hostname() {
+		t.Errorf("%s = %q, wanted %q", rr.Args, rr.Stdout.String(), u.Hostname())
+	}
+
+	// Test a regular URLminikube
+	rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "service", "hello-node", "--url"))
+	if err != nil {
+		t.Errorf("%s failed: %v", rr.Args, err)
+	}
+
+	endpoint = strings.TrimSpace(rr.Stdout.String())
+	u, err = url.Parse(endpoint)
+	if err != nil {
+		t.Fatalf("failed to parse %q: %v", endpoint, err)
+	}
+	if u.Scheme != "http" {
+		t.Fatalf("got scheme: %q, expected: %q", u.Scheme, "http")
+	}
+
+	t.Logf("url: %s", endpoint)
+	resp, err := retryablehttp.Get(endpoint)
+	if err != nil {
+		t.Fatalf("get failed: %v\nresp: %v", err, resp)
+	}
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("%s = status code %d, want %d", u, resp.StatusCode, http.StatusOK)
 	}
 }