From f60764b07f5fc9e159678edebe55850a9f83b59f Mon Sep 17 00:00:00 2001 From: tstromberg Date: Wed, 30 Oct 2019 10:44:54 -0700 Subject: [PATCH 1/5] service: fix --url mode, add integration tests --- cmd/minikube/cmd/service.go | 32 ++++++++++------- pkg/minikube/service/service.go | 8 ++--- test/integration/functional_test.go | 55 ++++++++++++++++++++++++++--- 3 files changed, 71 insertions(+), 24 deletions(-) diff --git a/cmd/minikube/cmd/service.go b/cmd/minikube/cmd/service.go index 66e7fa8335c6..54019acb01e8 100644 --- a/cmd/minikube/cmd/service.go +++ b/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.Warning("could not parse %v (will not open)", u, err) + out.String(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(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) } } }, diff --git a/pkg/minikube/service/service.go b/pkg/minikube/service/service.go index 388ab9fc92fd..7545f0cc1d2f 100644 --- a/pkg/minikube/service/service.go +++ b/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 } diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 3837a80abe9c..4e4b8f5eb8ef 100644 --- a/test/integration/functional_test.go +++ b/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,58 @@ 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, "run", "hello-minikube", "--restart=Never", "--image=gcr.io/google_containers/echoserver:1.4", "--port=8080")) + 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-minikube", "--type=NodePort")) + if err != nil { + t.Logf("%s failed: %v (may not be an error)", rr.Args, 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-minikube") { + t.Errorf("service list got %q, wanted *hello-minikube*", rr.Stdout.String()) + } + + // Test --https --url mode + rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "service", "--namespace=default", "--https", "--url", "hello-minikube")) + if err != nil { + t.Errorf("%s failed: %v", rr.Args, err) + } + endpoint := 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-minikube", "--url", "--format={{.IP}}")) + if err != nil { + t.Errorf("%s failed: %v", rr.Args, err) + } + if rr.Stdout.String() != u.Hostname() { + t.Errorf("%s = %q, wanted %q", rr.Args, rr.Stdout.String(), u.Hostname()) + } + + // Test a regular URL + rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "service", "hello-minikube", "--url")) + if err != nil { + t.Errorf("%s failed: %v", rr.Args, err) + } + resp, err := retryablehttp.Get(rr.Stdout.String()) + if err != nil { + t.Errorf("get failed: %v\nresp: %v", err, resp) + } + if resp.StatusCode != http.StatusOK { + t.Errorf("%s = status code %d, want %d", u, resp.StatusCode, http.StatusOK) } } From 2c0f2a42dd42dc70b177a751baf9e32f54134022 Mon Sep 17 00:00:00 2001 From: tstromberg Date: Wed, 30 Oct 2019 12:35:25 -0700 Subject: [PATCH 2/5] Append newline to --url output --- cmd/minikube/cmd/service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/minikube/cmd/service.go b/cmd/minikube/cmd/service.go index 54019acb01e8..02fe19b859a6 100644 --- a/cmd/minikube/cmd/service.go +++ b/cmd/minikube/cmd/service.go @@ -84,12 +84,12 @@ var serviceCmd = &cobra.Command{ _, err := url.Parse(u) if err != nil { glog.Warning("could not parse %v (will not open)", u, err) - out.String(u) + out.String(fmt.Sprintf("%s\n", u)) continue } if serviceURLMode { - out.String(u) + 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}) From 4ce2f784f1d3c92a84b25fc967f99e2050882706 Mon Sep 17 00:00:00 2001 From: tstromberg Date: Wed, 30 Oct 2019 13:57:08 -0700 Subject: [PATCH 3/5] Fix failure issues with integration testing for service command --- cmd/minikube/cmd/service.go | 2 +- test/integration/functional_test.go | 47 ++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/cmd/minikube/cmd/service.go b/cmd/minikube/cmd/service.go index 02fe19b859a6..d2e7d993cf73 100644 --- a/cmd/minikube/cmd/service.go +++ b/cmd/minikube/cmd/service.go @@ -83,7 +83,7 @@ var serviceCmd = &cobra.Command{ for _, u := range urls { _, err := url.Parse(u) if err != nil { - glog.Warning("could not parse %v (will not open)", u, err) + glog.Warning("unable to parse %s: %v (will not open)", u, err) out.String(fmt.Sprintf("%s\n", u)) continue } diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 4e4b8f5eb8ef..00379f245b06 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -398,29 +398,37 @@ func validateProfileCmd(ctx context.Context, t *testing.T, profile string) { // validateServiceCmd asserts basic "service" command functionality func validateServiceCmd(ctx context.Context, t *testing.T, profile string) { - rr, err := Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "run", "hello-minikube", "--restart=Never", "--image=gcr.io/google_containers/echoserver:1.4", "--port=8080")) + 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-minikube", "--type=NodePort")) + 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(), "hello-minikube") { - t.Errorf("service list got %q, wanted *hello-minikube*", 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-minikube")) + rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "service", "--namespace=default", "--https", "--url", "hello-node")) if err != nil { - t.Errorf("%s failed: %v", rr.Args, err) + t.Fatalf("%s failed: %v", rr.Args, err) + } + if rr.Stderr.String() != "" { + t.Errorf("unexpected stderr output: %s", rr.Stderr) } - endpoint := rr.Stdout.String() + + endpoint := strings.TrimSpace(rr.Stdout.String()) u, err := url.Parse(endpoint) if err != nil { t.Fatalf("failed to parse %q: %v", endpoint, err) @@ -430,25 +438,36 @@ func validateServiceCmd(ctx context.Context, t *testing.T, profile string) { } // Test --format=IP - rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "service", "hello-minikube", "--url", "--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 rr.Stdout.String() != u.Hostname() { + if strings.TrimSpace(rr.Stdout.String()) != u.Hostname() { t.Errorf("%s = %q, wanted %q", rr.Args, rr.Stdout.String(), u.Hostname()) } - // Test a regular URL - rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "service", "hello-minikube", "--url")) + // 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) } - resp, err := retryablehttp.Get(rr.Stdout.String()) + + 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.Errorf("get failed: %v\nresp: %v", err, resp) + t.Fatalf("get failed: %v\nresp: %v", err, resp) } if resp.StatusCode != http.StatusOK { - t.Errorf("%s = status code %d, want %d", u, resp.StatusCode, http.StatusOK) + t.Fatalf("%s = status code %d, want %d", u, resp.StatusCode, http.StatusOK) } } From e8be75c74abf0d59e179b58d0b264d94c4c56572 Mon Sep 17 00:00:00 2001 From: tstromberg Date: Wed, 30 Oct 2019 14:06:05 -0700 Subject: [PATCH 4/5] Warning -> Warningf --- cmd/minikube/cmd/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/service.go b/cmd/minikube/cmd/service.go index d2e7d993cf73..1aed2de5932d 100644 --- a/cmd/minikube/cmd/service.go +++ b/cmd/minikube/cmd/service.go @@ -83,7 +83,7 @@ var serviceCmd = &cobra.Command{ for _, u := range urls { _, err := url.Parse(u) if err != nil { - glog.Warning("unable to parse %s: %v (will not open)", u, err) + glog.Warningf("unable to parse %s: %v (will not open)", u, err) out.String(fmt.Sprintf("%s\n", u)) continue } From c34e865ab616b752c1397f5b5b654a998aba6230 Mon Sep 17 00:00:00 2001 From: tstromberg Date: Wed, 30 Oct 2019 14:06:39 -0700 Subject: [PATCH 5/5] Improve warning message --- cmd/minikube/cmd/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/service.go b/cmd/minikube/cmd/service.go index 1aed2de5932d..8d844deb0638 100644 --- a/cmd/minikube/cmd/service.go +++ b/cmd/minikube/cmd/service.go @@ -83,7 +83,7 @@ var serviceCmd = &cobra.Command{ for _, u := range urls { _, err := url.Parse(u) if err != nil { - glog.Warningf("unable to parse %s: %v (will not open)", u, err) + glog.Warningf("failed to parse url %q: %v (will not open)", u, err) out.String(fmt.Sprintf("%s\n", u)) continue }