From af4364ba6f5d6d25fb24585ead6c28cdd4b38623 Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Thu, 18 Feb 2021 13:34:13 -0700 Subject: [PATCH 1/6] Updated dashboard URL detection logic for DashboardCmd test --- test/integration/functional_test.go | 25 ++++++++++++++++++++----- test/integration/util_test.go | 25 ------------------------- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index d29a8f791635..cc7f19cd895b 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -19,6 +19,7 @@ limitations under the License. package integration import ( + "bufio" "bytes" "context" "encoding/json" @@ -546,8 +547,11 @@ func validateStatusCmd(ctx context.Context, t *testing.T, profile string) { func validateDashboardCmd(ctx context.Context, t *testing.T, profile string) { defer PostMortemLogs(t, profile) + mctx, cancel := context.WithTimeout(ctx, Seconds(300)) + defer cancel() + args := []string{"dashboard", "--url", "-p", profile, "--alsologtostderr", "-v=1"} - ss, err := Start(t, exec.CommandContext(ctx, Target(), args...)) + ss, err := Start(t, exec.CommandContext(mctx, Target(), args...)) if err != nil { t.Errorf("failed to run minikube dashboard. args %q : %v", args, err) } @@ -555,13 +559,12 @@ func validateDashboardCmd(ctx context.Context, t *testing.T, profile string) { ss.Stop(t) }() - start := time.Now() - s, err := ReadLineWithTimeout(ss.Stdout, Seconds(300)) + s, err := dashboardURL(ss.Stdout) if err != nil { if runtime.GOOS == "windows" { - t.Skipf("failed to read url within %s: %v\noutput: %q\n", time.Since(start), err, s) + t.Skip(err) } - t.Fatalf("failed to read url within %s: %v\noutput: %q\n", time.Since(start), err, s) + t.Fatal(err) } u, err := url.Parse(strings.TrimSpace(s)) @@ -583,6 +586,18 @@ func validateDashboardCmd(ctx context.Context, t *testing.T, profile string) { } } +// dashboardURL gets the dashboard URL from the command stdout. +func dashboardURL(b *bufio.Reader) (string, error) { + s := bufio.NewScanner(b) + for s.Scan() { + l := s.Text() + if strings.HasPrefix(l, "http") { + return l, nil + } + } + return "", fmt.Errorf("output didn't produce a URL") +} + // validateDryRun asserts that the dry-run mode quickly exits with the right code func validateDryRun(ctx context.Context, t *testing.T, profile string) { // dry-run mode should always be able to finish quickly (<5s) diff --git a/test/integration/util_test.go b/test/integration/util_test.go index c924b78c2e18..c5d3094a3681 100644 --- a/test/integration/util_test.go +++ b/test/integration/util_test.go @@ -26,31 +26,6 @@ import ( "k8s.io/minikube/pkg/minikube/localpath" ) -// ReadLineWithTimeout reads a line of text from a buffer with a timeout -func ReadLineWithTimeout(b *bufio.Reader, timeout time.Duration) (string, error) { - s := make(chan string) - e := make(chan error) - go func() { - read, err := b.ReadString('\n') - if err != nil { - e <- err - } else { - s <- read - } - close(s) - close(e) - }() - - select { - case line := <-s: - return line, nil - case err := <-e: - return "", err - case <-time.After(timeout): - return "", fmt.Errorf("timeout after %s", timeout) - } -} - // UniqueProfileName returns a reasonably unique profile name func UniqueProfileName(prefix string) string { if *forceProfile != "" { From d9aca3d0c9fafe1b640f8251fc247d9217ff6f6a Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Thu, 18 Feb 2021 17:34:22 -0700 Subject: [PATCH 2/6] make dashboard URL check more strict --- test/integration/functional_test.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index cc7f19cd895b..9cf735c8402b 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -559,7 +559,7 @@ func validateDashboardCmd(ctx context.Context, t *testing.T, profile string) { ss.Stop(t) }() - s, err := dashboardURL(ss.Stdout) + s, err := dashboardURL(t, ss.Stdout) if err != nil { if runtime.GOOS == "windows" { t.Skip(err) @@ -587,11 +587,18 @@ func validateDashboardCmd(ctx context.Context, t *testing.T, profile string) { } // dashboardURL gets the dashboard URL from the command stdout. -func dashboardURL(b *bufio.Reader) (string, error) { +func dashboardURL(t *testing.T, b *bufio.Reader) (string, error) { s := bufio.NewScanner(b) + + // match http://127.0.0.1:XXXXX/api/v1/namespaces/kubernetes-dashboard/services/http:kubernetes-dashboard:/proxy/ + dashURLRegexp, err := regexp.Compile(`^http:\/\/127\.0\.0\.1:[0-9]{5}\/api\/v1\/namespaces\/kubernetes-dashboard\/services\/http:kubernetes-dashboard:\/proxy\/$`) + if err != nil { + t.Fatalf("dashboard URL regex is invalid: %v", err) + } + for s.Scan() { l := s.Text() - if strings.HasPrefix(l, "http") { + if dashURLRegexp.MatchString(l) { return l, nil } } From 635f89b079d8807ccf2f71c32ff4663ad21d3479 Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Thu, 18 Feb 2021 17:39:44 -0700 Subject: [PATCH 3/6] variable rename --- test/integration/functional_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 9cf735c8402b..4fbd0304c58e 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -588,7 +588,7 @@ func validateDashboardCmd(ctx context.Context, t *testing.T, profile string) { // dashboardURL gets the dashboard URL from the command stdout. func dashboardURL(t *testing.T, b *bufio.Reader) (string, error) { - s := bufio.NewScanner(b) + scanner := bufio.NewScanner(b) // match http://127.0.0.1:XXXXX/api/v1/namespaces/kubernetes-dashboard/services/http:kubernetes-dashboard:/proxy/ dashURLRegexp, err := regexp.Compile(`^http:\/\/127\.0\.0\.1:[0-9]{5}\/api\/v1\/namespaces\/kubernetes-dashboard\/services\/http:kubernetes-dashboard:\/proxy\/$`) @@ -596,10 +596,10 @@ func dashboardURL(t *testing.T, b *bufio.Reader) (string, error) { t.Fatalf("dashboard URL regex is invalid: %v", err) } - for s.Scan() { - l := s.Text() - if dashURLRegexp.MatchString(l) { - return l, nil + for scanner.Scan() { + s := scanner.Text() + if dashURLRegexp.MatchString(s) { + return s, nil } } return "", fmt.Errorf("output didn't produce a URL") From 2982b7db4ca82349eb28a49288112c709325d0bc Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Thu, 18 Feb 2021 17:42:06 -0700 Subject: [PATCH 4/6] use MustCompile instead of Compile as per Go lint --- test/integration/functional_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 4fbd0304c58e..53f0a632b954 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -559,7 +559,7 @@ func validateDashboardCmd(ctx context.Context, t *testing.T, profile string) { ss.Stop(t) }() - s, err := dashboardURL(t, ss.Stdout) + s, err := dashboardURL(ss.Stdout) if err != nil { if runtime.GOOS == "windows" { t.Skip(err) @@ -587,14 +587,11 @@ func validateDashboardCmd(ctx context.Context, t *testing.T, profile string) { } // dashboardURL gets the dashboard URL from the command stdout. -func dashboardURL(t *testing.T, b *bufio.Reader) (string, error) { +func dashboardURL(b *bufio.Reader) (string, error) { scanner := bufio.NewScanner(b) // match http://127.0.0.1:XXXXX/api/v1/namespaces/kubernetes-dashboard/services/http:kubernetes-dashboard:/proxy/ - dashURLRegexp, err := regexp.Compile(`^http:\/\/127\.0\.0\.1:[0-9]{5}\/api\/v1\/namespaces\/kubernetes-dashboard\/services\/http:kubernetes-dashboard:\/proxy\/$`) - if err != nil { - t.Fatalf("dashboard URL regex is invalid: %v", err) - } + dashURLRegexp := regexp.MustCompile(`^http:\/\/127\.0\.0\.1:[0-9]{5}\/api\/v1\/namespaces\/kubernetes-dashboard\/services\/http:kubernetes-dashboard:\/proxy\/$`) for scanner.Scan() { s := scanner.Text() From 1a5c1f8219a021d3ebe77349d589e1e49ad7e76e Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Thu, 18 Feb 2021 17:44:23 -0700 Subject: [PATCH 5/6] refactored to make a bit cleaner --- test/integration/functional_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 53f0a632b954..4f78a2281efb 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -588,15 +588,14 @@ func validateDashboardCmd(ctx context.Context, t *testing.T, profile string) { // dashboardURL gets the dashboard URL from the command stdout. func dashboardURL(b *bufio.Reader) (string, error) { - scanner := bufio.NewScanner(b) - // match http://127.0.0.1:XXXXX/api/v1/namespaces/kubernetes-dashboard/services/http:kubernetes-dashboard:/proxy/ dashURLRegexp := regexp.MustCompile(`^http:\/\/127\.0\.0\.1:[0-9]{5}\/api\/v1\/namespaces\/kubernetes-dashboard\/services\/http:kubernetes-dashboard:\/proxy\/$`) - for scanner.Scan() { - s := scanner.Text() - if dashURLRegexp.MatchString(s) { - return s, nil + s := bufio.NewScanner(b) + for s.Scan() { + t := s.Text() + if dashURLRegexp.MatchString(t) { + return t, nil } } return "", fmt.Errorf("output didn't produce a URL") From 3a0e3bd3008f06b4a1219095fde3313399140d7e Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Thu, 18 Feb 2021 18:27:55 -0700 Subject: [PATCH 6/6] added missing error check to scanner --- test/integration/functional_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 4f78a2281efb..791dffb898f8 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -598,6 +598,9 @@ func dashboardURL(b *bufio.Reader) (string, error) { return t, nil } } + if err := s.Err(); err != nil { + return "", fmt.Errorf("failed reading input: %v", err) + } return "", fmt.Errorf("output didn't produce a URL") }