Skip to content

Commit 8a88e86

Browse files
nanikjavananikjava
committed
Add boolean to disable opening browser during test
The WaitAndMaybeOpenService(..) method allow user to open the service url in a browser when found, this create issue during testing as it's opening browser and consuming resources. The fix is to introduce a boolean flag allowing the caller to specify whether to just print out the url or open a browser window
1 parent 920f20c commit 8a88e86

File tree

4 files changed

+77
-23
lines changed

4 files changed

+77
-23
lines changed

‎cmd/minikube/cmd/config/open.go

+15-1
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,12 @@ limitations under the License.
1717
package config
1818

1919
import (
20+
"fmt"
2021
"os"
2122
"text/template"
2223

24+
"github.com/pkg/browser"
25+
2326
"github.com/spf13/cobra"
2427
"k8s.io/minikube/pkg/minikube/assets"
2528
"k8s.io/minikube/pkg/minikube/cluster"
@@ -95,9 +98,20 @@ You can add one by annotating a service with the label {{.labelName}}:{{.addonNa
9598
}
9699
for i := range serviceList.Items {
97100
svc := serviceList.Items[i].ObjectMeta.Name
98-
if err := service.WaitAndMaybeOpenService(api, namespace, svc, addonsURLTemplate, addonsURLMode, https, wait, interval); err != nil {
101+
var urlString []string
102+
103+
if urlString, err = service.WaitForService(api, namespace, svc, addonsURLTemplate, addonsURLMode, https, wait, interval); err != nil {
99104
exit.WithCodeT(exit.Unavailable, "Wait failed: {{.error}}", out.V{"error": err})
100105
}
106+
107+
if len(urlString) != 0 {
108+
out.T(out.Celebrate, "Opening kubernetes service {{.namespace_name}}/{{.service_name}} in default browser...", out.V{"namespace_name": namespace, "service_name": svc})
109+
for _, url := range urlString {
110+
if err := browser.OpenURL(url); err != nil {
111+
exit.WithError(fmt.Sprintf("browser failed to open url %s", url), err)
112+
}
113+
}
114+
}
101115
}
102116
},
103117
}

‎cmd/minikube/cmd/service.go

+19-1
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,14 @@ limitations under the License.
1717
package cmd
1818

1919
import (
20+
"fmt"
2021
"os"
2122
"text/template"
2223

24+
"github.com/pkg/browser"
25+
26+
"k8s.io/minikube/pkg/minikube/out"
27+
2328
"github.com/spf13/cobra"
2429
"k8s.io/minikube/pkg/minikube/cluster"
2530
"k8s.io/minikube/pkg/minikube/exit"
@@ -68,11 +73,24 @@ var serviceCmd = &cobra.Command{
6873
if !cluster.IsMinikubeRunning(api) {
6974
os.Exit(1)
7075
}
71-
err = service.WaitAndMaybeOpenService(api, namespace, svc,
76+
77+
var urlString []string
78+
79+
urlString, err = service.WaitForService(api, namespace, svc,
7280
serviceURLTemplate, serviceURLMode, https, wait, interval)
7381
if err != nil {
7482
exit.WithError("Error opening service", err)
7583
}
84+
85+
if len(urlString) != 0 {
86+
out.T(out.Celebrate, "Opening kubernetes service {{.namespace_name}}/{{.service_name}} in default browser...", out.V{"namespace_name": namespace, "service_name": svc})
87+
88+
for _, url := range urlString {
89+
if err := browser.OpenURL(url); err != nil {
90+
exit.WithError(fmt.Sprintf("browser failed to open url %s", url), err)
91+
}
92+
}
93+
}
7694
},
7795
}
7896

‎pkg/minikube/service/service.go

+12-15
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
"github.com/docker/machine/libmachine"
3030
"github.com/golang/glog"
3131
"github.com/olekukonko/tablewriter"
32-
"github.com/pkg/browser"
3332
"github.com/pkg/errors"
3433
"github.com/spf13/viper"
3534
core "k8s.io/api/core/v1"
@@ -265,22 +264,24 @@ func PrintServiceList(writer io.Writer, data [][]string) {
265264
table.Render()
266265
}
267266

268-
// WaitAndMaybeOpenService waits for a service, and opens it when running
269-
func WaitAndMaybeOpenService(api libmachine.API, namespace string, service string, urlTemplate *template.Template, urlMode bool, https bool,
270-
wait int, interval int) error {
267+
// WaitForService waits for a service, and opens it when running
268+
func WaitForService(api libmachine.API, namespace string, service string, urlTemplate *template.Template, urlMode bool, https bool,
269+
wait int, interval int) ([]string, error) {
270+
271+
var urlList []string
271272
// Convert "Amount of time to wait" and "interval of each check" to attempts
272273
if interval == 0 {
273274
interval = 1
274275
}
275276
chkSVC := func() error { return CheckService(namespace, service) }
276277

277278
if err := retry.Expo(chkSVC, time.Duration(interval)*time.Second, time.Duration(wait)*time.Second); err != nil {
278-
return errors.Wrapf(err, "Service %s was not found in %q namespace. You may select another namespace by using 'minikube service %s -n <namespace>", service, namespace, service)
279+
return urlList, errors.Wrapf(err, "Service %s was not found in %q namespace. You may select another namespace by using 'minikube service %s -n <namespace>", service, namespace, service)
279280
}
280281

281282
serviceURL, err := GetServiceURLsForService(api, namespace, service, urlTemplate)
282283
if err != nil {
283-
return errors.Wrap(err, "Check that minikube is running and that you have specified the correct namespace")
284+
return urlList, errors.Wrap(err, "Check that minikube is running and that you have specified the correct namespace")
284285
}
285286

286287
if !urlMode {
@@ -295,22 +296,18 @@ func WaitAndMaybeOpenService(api libmachine.API, namespace string, service strin
295296

296297
if len(serviceURL.URLs) == 0 {
297298
out.T(out.Sad, "service {{.namespace_name}}/{{.service_name}} has no node port", out.V{"namespace_name": namespace, "service_name": service})
298-
return nil
299+
return urlList, nil
299300
}
300301

301302
for _, bareURLString := range serviceURL.URLs {
302-
urlString, isHTTPSchemedURL := OptionallyHTTPSFormattedURLString(bareURLString, https)
303+
url, isHTTPSchemedURL := OptionallyHTTPSFormattedURLString(bareURLString, https)
303304

304305
if urlMode || !isHTTPSchemedURL {
305-
out.T(out.Empty, urlString)
306-
} else {
307-
out.T(out.Celebrate, "Opening kubernetes service {{.namespace_name}}/{{.service_name}} in default browser...", out.V{"namespace_name": namespace, "service_name": service})
308-
if err := browser.OpenURL(urlString); err != nil {
309-
out.ErrT(out.Empty, "browser failed to open url: {{.error}}", out.V{"error": err})
310-
}
306+
out.T(out.Empty, url)
307+
urlList = append(urlList, url)
311308
}
312309
}
313-
return nil
310+
return urlList, nil
314311
}
315312

316313
// GetServiceListByLabel returns a ServiceList by label

‎pkg/minikube/service/service_test.go

+31-6
Original file line numberDiff line numberDiff line change
@@ -885,6 +885,15 @@ func TestWaitAndMaybeOpenService(t *testing.T) {
885885
api: defaultAPI,
886886
urlMode: true,
887887
https: true,
888+
expected: []string{"https://127.0.0.1:1111", "https://127.0.0.1:2222"},
889+
},
890+
{
891+
description: "correctly return serviceURLs, http, url mode",
892+
namespace: "default",
893+
service: "mock-dashboard",
894+
api: defaultAPI,
895+
urlMode: true,
896+
https: false,
888897
expected: []string{"http://127.0.0.1:1111", "http://127.0.0.1:2222"},
889898
},
890899
{
@@ -903,12 +912,28 @@ func TestWaitAndMaybeOpenService(t *testing.T) {
903912
servicesMap: serviceNamespaces,
904913
endpointsMap: endpointNamespaces,
905914
}
906-
err := WaitAndMaybeOpenService(test.api, test.namespace, test.service, defaultTemplate, test.urlMode, test.https, 1, 0)
915+
916+
var urlList []string
917+
urlList, err := WaitForService(test.api, test.namespace, test.service, defaultTemplate, test.urlMode, test.https, 1, 0)
907918
if test.err && err == nil {
908-
t.Fatalf("WaitAndMaybeOpenService expected to fail for test: %v", test)
919+
t.Fatalf("WaitForService expected to fail for test: %v", test)
909920
}
910921
if !test.err && err != nil {
911-
t.Fatalf("WaitAndMaybeOpenService not expected to fail but got err: %v", err)
922+
t.Fatalf("WaitForService not expected to fail but got err: %v", err)
923+
}
924+
925+
if test.urlMode {
926+
// check the size of the url slices
927+
if len(urlList) != len(test.expected) {
928+
t.Fatalf("WaitForService returned [%d] urls while expected is [%d] url", len(urlList), len(test.expected))
929+
}
930+
931+
// check content of the expected url
932+
for i, v := range test.expected {
933+
if v != urlList[i] {
934+
t.Fatalf("WaitForService returned [%s] urls while expected is [%s] url", urlList[i], v)
935+
}
936+
}
912937
}
913938

914939
})
@@ -954,12 +979,12 @@ func TestWaitAndMaybeOpenServiceForNotDefaultNamspace(t *testing.T) {
954979
servicesMap: serviceNamespaceOther,
955980
endpointsMap: endpointNamespaces,
956981
}
957-
err := WaitAndMaybeOpenService(test.api, test.namespace, test.service, defaultTemplate, test.urlMode, test.https, 1, 0)
982+
_, err := WaitForService(test.api, test.namespace, test.service, defaultTemplate, test.urlMode, test.https, 1, 0)
958983
if test.err && err == nil {
959-
t.Fatalf("WaitAndMaybeOpenService expected to fail for test: %v", test)
984+
t.Fatalf("WaitForService expected to fail for test: %v", test)
960985
}
961986
if !test.err && err != nil {
962-
t.Fatalf("WaitAndMaybeOpenService not expected to fail but got err: %v", err)
987+
t.Fatalf("WaitForService not expected to fail but got err: %v", err)
963988
}
964989

965990
})

0 commit comments

Comments
 (0)