From df54c6a5b46aa509dbeea62b77353f4feb20aabb Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Tue, 2 Oct 2018 22:25:45 -0700 Subject: [PATCH 01/15] Use "kubectl proxy" instead of a NodePort to expose the dashboard. This provides an additional level of security, by enforcing host checking, applying port randomization, and requiring explicit user intent to expose the service to the host. --- cmd/minikube/cmd/dashboard.go | 67 +++++++++++++++++----- deploy/addons/dashboard/dashboard-svc.yaml | 8 +-- pkg/minikube/service/service.go | 15 ++++- test/integration/addons_test.go | 7 ++- 4 files changed, 73 insertions(+), 24 deletions(-) diff --git a/cmd/minikube/cmd/dashboard.go b/cmd/minikube/cmd/dashboard.go index 362e88869477..3963fd367ea1 100644 --- a/cmd/minikube/cmd/dashboard.go +++ b/cmd/minikube/cmd/dashboard.go @@ -17,13 +17,16 @@ limitations under the License. package cmd import ( + "bufio" "fmt" "os" - "text/template" + "os/exec" + "regexp" "time" "github.com/golang/glog" "github.com/pkg/browser" + "github.com/pkg/errors" "github.com/spf13/cobra" "k8s.io/minikube/pkg/minikube/cluster" "k8s.io/minikube/pkg/minikube/machine" @@ -34,6 +37,8 @@ import ( var ( dashboardURLMode bool + // Matches: 127.0.0.1:8001 + hostPortRe = regexp.MustCompile(`127.0.0.1:\d{4,}`) ) // dashboardCmd represents the dashboard command @@ -42,6 +47,7 @@ var dashboardCmd = &cobra.Command{ Short: "Opens/displays the kubernetes dashboard URL for your local cluster", Long: `Opens/displays the kubernetes dashboard URL for your local cluster`, Run: func(cmd *cobra.Command, args []string) { + glog.Infof("Setting up dashboard ...") api, err := machine.NewAPIClient() if err != nil { fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err) @@ -58,26 +64,59 @@ var dashboardCmd = &cobra.Command{ os.Exit(1) } - urls, err := service.GetServiceURLsForService(api, namespace, svc, template.Must(template.New("dashboardServiceFormat").Parse(defaultServiceFormatTemplate))) + p, hostPort, err := kubectlProxy() if err != nil { - fmt.Fprintln(os.Stderr, err) - fmt.Fprintln(os.Stderr, "Check that minikube is running.") - os.Exit(1) - } - if len(urls) == 0 { - errMsg := "There appears to be no url associated with dashboard, this is not expected, exiting" - glog.Infoln(errMsg) - os.Exit(1) + glog.Fatalf("kubectl proxy: %v", err) } + url := dashboardURL(hostPort, namespace, svc) if dashboardURLMode { - fmt.Fprintln(os.Stdout, urls[0]) - } else { - fmt.Fprintln(os.Stdout, "Opening kubernetes dashboard in default browser...") - browser.OpenURL(urls[0]) + fmt.Fprintln(os.Stdout, url) + return } + fmt.Fprintln(os.Stdout, fmt.Sprintf("Opening %s in your default browser...", url)) + browser.OpenURL(url) + p.Wait() }, } +// kubectlProxy runs "kubectl proxy", returning host:port +func kubectlProxy() (*exec.Cmd, string, error) { + glog.Infof("Searching for kubectl ...") + path, err := exec.LookPath("kubectl") + if err != nil { + return nil, "", errors.Wrap(err, "Unable to find kubectl in PATH") + } + cmd := exec.Command(path, "proxy", "--port=0") + stdoutPipe, err := cmd.StdoutPipe() + if err != nil { + return nil, "", errors.Wrap(err, "stdout") + } + + glog.Infof("Executing: %s %s", cmd.Path, cmd.Args) + if err := cmd.Start(); err != nil { + return nil, "", errors.Wrap(err, "start") + } + glog.Infof("proxy should be running ...") + reader := bufio.NewReader(stdoutPipe) + glog.Infof("Reading stdout pipe ...") + out, err := reader.ReadString('\n') + if err != nil { + return nil, "", errors.Wrap(err, "read") + } + return cmd, parseHostPort(out), nil +} + +func dashboardURL(proxy string, ns string, svc string) string { + // Reference: https://github.com/kubernetes/dashboard/wiki/Accessing-Dashboard---1.7.X-and-above + return fmt.Sprintf("http://%s/api/v1/namespaces/%s/services/http:%s:/proxy/", proxy, ns, svc) +} + +func parseHostPort(out string) string { + // Starting to serve on 127.0.0.1:8001 + glog.Infof("Parsing: %s ...", out) + return hostPortRe.FindString(out) +} + func init() { dashboardCmd.Flags().BoolVar(&dashboardURLMode, "url", false, "Display the kubernetes dashboard in the CLI instead of opening it in the default browser") RootCmd.AddCommand(dashboardCmd) diff --git a/deploy/addons/dashboard/dashboard-svc.yaml b/deploy/addons/dashboard/dashboard-svc.yaml index b39a8001cb68..1bc317deff62 100644 --- a/deploy/addons/dashboard/dashboard-svc.yaml +++ b/deploy/addons/dashboard/dashboard-svc.yaml @@ -24,10 +24,8 @@ metadata: kubernetes.io/minikube-addons: dashboard kubernetes.io/minikube-addons-endpoint: dashboard spec: - type: NodePort ports: - - port: 80 - targetPort: 9090 - nodePort: 30000 + - port: 80 + targetPort: 9090 selector: - app: kubernetes-dashboard + k8s-app: kubernetes-dashboard diff --git a/pkg/minikube/service/service.go b/pkg/minikube/service/service.go index 2fb7c82a2d61..39745ec69359 100644 --- a/pkg/minikube/service/service.go +++ b/pkg/minikube/service/service.go @@ -25,6 +25,7 @@ import ( "time" "github.com/docker/machine/libmachine" + "github.com/pkg/browser" "github.com/pkg/errors" "k8s.io/api/core/v1" @@ -35,6 +36,7 @@ import ( "text/template" + "github.com/golang/glog" "github.com/spf13/viper" "k8s.io/apimachinery/pkg/labels" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -197,27 +199,34 @@ func CheckService(namespace string, service string) error { return errors.Wrap(err, "Error getting kubernetes client") } services := client.Services(namespace) + glog.Infof("services: %+v", services) err = validateService(services, service) if err != nil { return errors.Wrap(err, "Error validating service") } + // Add logic here to switch between needing external endpoints or not. endpoints := client.Endpoints(namespace) - return checkEndpointReady(endpoints, service) + glog.Infof("%s:%s endpoints: %+v", namespace, service, endpoints) + return nil + // return checkEndpointReady(endpoints, service) } func validateService(s corev1.ServiceInterface, service string) error { - if _, err := s.Get(service, metav1.GetOptions{}); err != nil { + svc, err := s.Get(service, metav1.GetOptions{}) + if err != nil { return errors.Wrapf(err, "Error getting service %s", service) } + glog.Infof("%s: %+v", service, svc) return nil } func checkEndpointReady(endpoints corev1.EndpointsInterface, service string) error { endpoint, err := endpoints.Get(service, metav1.GetOptions{}) + glog.Infof("%s endpoint: %+v", service, endpoint) if err != nil { return &util.RetriableError{Err: errors.Errorf("Error getting endpoints for service %s", service)} } - const notReadyMsg = "Waiting, endpoint for service is not ready yet...\n" + notReadyMsg := fmt.Sprintf("Waiting, endpoint for %s is not ready yet...\n", service) if len(endpoint.Subsets) == 0 { fmt.Fprintf(os.Stderr, notReadyMsg) return &util.RetriableError{Err: errors.New("Endpoint for service is not ready yet")} diff --git a/test/integration/addons_test.go b/test/integration/addons_test.go index 6c7ac759e80a..ca30f4fa9e60 100644 --- a/test/integration/addons_test.go +++ b/test/integration/addons_test.go @@ -71,12 +71,15 @@ func testDashboard(t *testing.T) { if u.Scheme != "http" { t.Fatalf("wrong scheme in dashboard URL, expected http, actual %s", u.Scheme) } - _, port, err := net.SplitHostPort(u.Host) + host, port, err := net.SplitHostPort(u.Host) if err != nil { t.Fatalf("failed to split dashboard host %s: %v", u.Host, err) } if port != "30000" { - t.Fatalf("Dashboard is exposed on wrong port, expected 30000, actual %s", port) + t.Errorf("Dashboard is exposed on wrong port, expected 30000, actual %s", port) + } + if host != "127.0.0.1" { + t.Errorf("host is %s, expected 127.0.0.1", host) } } From 21776a09b5fa9a86a47f200a7bc0616e795f2498 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Tue, 2 Oct 2018 22:31:26 -0700 Subject: [PATCH 02/15] Fix service port indentation. --- deploy/addons/dashboard/dashboard-svc.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deploy/addons/dashboard/dashboard-svc.yaml b/deploy/addons/dashboard/dashboard-svc.yaml index 1bc317deff62..967ca8efdb4c 100644 --- a/deploy/addons/dashboard/dashboard-svc.yaml +++ b/deploy/addons/dashboard/dashboard-svc.yaml @@ -25,7 +25,7 @@ metadata: kubernetes.io/minikube-addons-endpoint: dashboard spec: ports: - - port: 80 - targetPort: 9090 + - port: 80 + targetPort: 9090 selector: k8s-app: kubernetes-dashboard From bc2dbe3b088124f59fd54bd24e0ba536ff48e8fe Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 3 Oct 2018 12:56:54 -0700 Subject: [PATCH 03/15] Improve readability of ISO download errors + include URL. This is a separate issue I bumped into while developing the dashboard PR. --- pkg/minikube/cluster/cluster.go | 2 +- pkg/util/downloader.go | 4 ++-- pkg/util/utils.go | 3 +++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/minikube/cluster/cluster.go b/pkg/minikube/cluster/cluster.go index f37ccd2f91da..50c58c364e04 100644 --- a/pkg/minikube/cluster/cluster.go +++ b/pkg/minikube/cluster/cluster.go @@ -217,7 +217,7 @@ func createHost(api libmachine.API, config cfg.MachineConfig) (*host.Host, error if config.VMDriver != "none" { if err := config.Downloader.CacheMinikubeISOFromURL(config.MinikubeISO); err != nil { - return nil, errors.Wrap(err, "Error attempting to cache minikube ISO from URL") + return nil, errors.Wrap(err, "unable to cache ISO") } } diff --git a/pkg/util/downloader.go b/pkg/util/downloader.go index 8a72c6eb525b..11e490d869e4 100644 --- a/pkg/util/downloader.go +++ b/pkg/util/downloader.go @@ -72,9 +72,9 @@ func (f DefaultDownloader) CacheMinikubeISOFromURL(isoURL string) error { options.ChecksumHash = crypto.SHA256 } - fmt.Println("Downloading Minikube ISO") + fmt.Printf("Downloading Minikube ISO: %s\n", isoURL) if err := download.ToFile(isoURL, f.GetISOCacheFilepath(isoURL), options); err != nil { - return errors.Wrap(err, "Error downloading Minikube ISO") + return errors.Wrap(err, "ToFile") } return nil diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 40cc5491e319..783ba1332491 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -97,14 +97,17 @@ func Retry(attempts int, callback func() error) (err error) { func RetryAfter(attempts int, callback func() error, d time.Duration) (err error) { m := MultiError{} for i := 0; i < attempts; i++ { + glog.V(1).Infof("retry loop %d", i) err = callback() if err == nil { return nil } m.Collect(err) if _, ok := err.(*RetriableError); !ok { + glog.Infof("non-retriable error: %v", err) return m.ToError() } + glog.V(2).Infof("sleeping %s", d) time.Sleep(d) } return m.ToError() From ca9ca6f29b479d20adf88595667584ac74f40af3 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 3 Oct 2018 12:57:55 -0700 Subject: [PATCH 04/15] Switch the label back to k8s-app instead of app for minikube compatibility. --- deploy/addons/dashboard/dashboard-svc.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deploy/addons/dashboard/dashboard-svc.yaml b/deploy/addons/dashboard/dashboard-svc.yaml index 967ca8efdb4c..04ccc0b932bc 100644 --- a/deploy/addons/dashboard/dashboard-svc.yaml +++ b/deploy/addons/dashboard/dashboard-svc.yaml @@ -28,4 +28,4 @@ spec: - port: 80 targetPort: 9090 selector: - k8s-app: kubernetes-dashboard + app: kubernetes-dashboard From 8fd45bc7516259165fc0d1e176f430ac0be1cdc2 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 3 Oct 2018 13:00:13 -0700 Subject: [PATCH 05/15] Block until the proxy URL is healthy. This is necessary now that there is no proper service endpoint to block on. --- cmd/minikube/cmd/dashboard.go | 39 ++++++++++++----- pkg/minikube/service/service.go | 47 ++++----------------- pkg/minikube/service/service_test.go | 62 ++++++---------------------- 3 files changed, 50 insertions(+), 98 deletions(-) diff --git a/cmd/minikube/cmd/dashboard.go b/cmd/minikube/cmd/dashboard.go index 3963fd367ea1..be9c93cd0a7c 100644 --- a/cmd/minikube/cmd/dashboard.go +++ b/cmd/minikube/cmd/dashboard.go @@ -19,6 +19,7 @@ package cmd import ( "bufio" "fmt" + "net/http" "os" "os/exec" "regexp" @@ -47,20 +48,20 @@ var dashboardCmd = &cobra.Command{ Short: "Opens/displays the kubernetes dashboard URL for your local cluster", Long: `Opens/displays the kubernetes dashboard URL for your local cluster`, Run: func(cmd *cobra.Command, args []string) { - glog.Infof("Setting up dashboard ...") api, err := machine.NewAPIClient() + defer api.Close() if err != nil { - fmt.Fprintf(os.Stderr, "Error getting client: %s\n", err) + fmt.Fprintf(os.Stderr, "Error creating client: %v\n", err) os.Exit(1) } - defer api.Close() - cluster.EnsureMinikubeRunningOrExit(api, 1) + namespace := "kube-system" svc := "kubernetes-dashboard" - if err = commonutil.RetryAfter(20, func() error { return service.CheckService(namespace, svc) }, 6*time.Second); err != nil { - fmt.Fprintf(os.Stderr, "Could not find finalized endpoint being pointed to by %s: %s\n", svc, err) + glog.Infof("Looking for dashboard service ...") + if err = commonutil.RetryAfter(30, func() error { return service.CheckService(namespace, svc) }, 1*time.Second); err != nil { + fmt.Fprintf(os.Stderr, "%s:%s is not running: %s\n", namespace, svc, err) os.Exit(1) } @@ -69,6 +70,12 @@ var dashboardCmd = &cobra.Command{ glog.Fatalf("kubectl proxy: %v", err) } url := dashboardURL(hostPort, namespace, svc) + + if err = commonutil.RetryAfter(30, func() error { return checkURL(url) }, 1*time.Second); err != nil { + fmt.Fprintf(os.Stderr, "%s is not responding properly: %s\n", url, err) + os.Exit(1) + } + if dashboardURLMode { fmt.Fprintln(os.Stdout, url) return @@ -81,7 +88,6 @@ var dashboardCmd = &cobra.Command{ // kubectlProxy runs "kubectl proxy", returning host:port func kubectlProxy() (*exec.Cmd, string, error) { - glog.Infof("Searching for kubectl ...") path, err := exec.LookPath("kubectl") if err != nil { return nil, "", errors.Wrap(err, "Unable to find kubectl in PATH") @@ -96,9 +102,8 @@ func kubectlProxy() (*exec.Cmd, string, error) { if err := cmd.Start(); err != nil { return nil, "", errors.Wrap(err, "start") } - glog.Infof("proxy should be running ...") reader := bufio.NewReader(stdoutPipe) - glog.Infof("Reading stdout pipe ...") + glog.Infof("Started proxy, now reading stdout pipe ...") out, err := reader.ReadString('\n') if err != nil { return nil, "", errors.Wrap(err, "read") @@ -112,11 +117,25 @@ func dashboardURL(proxy string, ns string, svc string) string { } func parseHostPort(out string) string { + glog.Infof("Parsing output: %s ...", out) // Starting to serve on 127.0.0.1:8001 - glog.Infof("Parsing: %s ...", out) return hostPortRe.FindString(out) } +func checkURL(url string) error { + resp, err := http.Get(url) + glog.Infof("%s response: %s %s", url, err, resp) + if err != nil { + return err + } + if resp.StatusCode != http.StatusOK { + return &commonutil.RetriableError{ + Err: fmt.Errorf("unexpected response code: %d", resp.StatusCode), + } + } + return nil +} + func init() { dashboardCmd.Flags().BoolVar(&dashboardURLMode, "url", false, "Display the kubernetes dashboard in the CLI instead of opening it in the default browser") RootCmd.AddCommand(dashboardCmd) diff --git a/pkg/minikube/service/service.go b/pkg/minikube/service/service.go index 39745ec69359..6b4cfeeb5fac 100644 --- a/pkg/minikube/service/service.go +++ b/pkg/minikube/service/service.go @@ -25,6 +25,7 @@ import ( "time" "github.com/docker/machine/libmachine" + "github.com/golang/glog" "github.com/pkg/browser" "github.com/pkg/errors" @@ -36,7 +37,6 @@ import ( "text/template" - "github.com/golang/glog" "github.com/spf13/viper" "k8s.io/apimachinery/pkg/labels" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -191,52 +191,23 @@ func printURLsForService(c corev1.CoreV1Interface, ip, service, namespace string return urls, nil } -// CheckService waits for the specified service to be ready by returning an error until the service is up -// The check is done by polling the endpoint associated with the service and when the endpoint exists, returning no error->service-online +// CheckService checks if a service is listening on a port. func CheckService(namespace string, service string) error { client, err := K8s.GetCoreClient() if err != nil { return errors.Wrap(err, "Error getting kubernetes client") } - services := client.Services(namespace) - glog.Infof("services: %+v", services) - err = validateService(services, service) - if err != nil { - return errors.Wrap(err, "Error validating service") - } - // Add logic here to switch between needing external endpoints or not. - endpoints := client.Endpoints(namespace) - glog.Infof("%s:%s endpoints: %+v", namespace, service, endpoints) - return nil - // return checkEndpointReady(endpoints, service) -} - -func validateService(s corev1.ServiceInterface, service string) error { - svc, err := s.Get(service, metav1.GetOptions{}) - if err != nil { - return errors.Wrapf(err, "Error getting service %s", service) - } - glog.Infof("%s: %+v", service, svc) - return nil -} -func checkEndpointReady(endpoints corev1.EndpointsInterface, service string) error { - endpoint, err := endpoints.Get(service, metav1.GetOptions{}) - glog.Infof("%s endpoint: %+v", service, endpoint) + svc, err := client.Services(namespace).Get(service, metav1.GetOptions{}) if err != nil { - return &util.RetriableError{Err: errors.Errorf("Error getting endpoints for service %s", service)} - } - notReadyMsg := fmt.Sprintf("Waiting, endpoint for %s is not ready yet...\n", service) - if len(endpoint.Subsets) == 0 { - fmt.Fprintf(os.Stderr, notReadyMsg) - return &util.RetriableError{Err: errors.New("Endpoint for service is not ready yet")} - } - for _, subset := range endpoint.Subsets { - if len(subset.Addresses) == 0 { - fmt.Fprintf(os.Stderr, notReadyMsg) - return &util.RetriableError{Err: errors.New("No endpoints for service are ready yet")} + return &util.RetriableError{ + Err: errors.Wrapf(err, "Error getting service %s", service), } } + if len(svc.Spec.Ports) == 0 { + return fmt.Errorf("%s:%s has no ports", namespace, service) + } + glog.Infof("Found service: %+v", svc) return nil } diff --git a/pkg/minikube/service/service_test.go b/pkg/minikube/service/service_test.go index f28bce3f9767..473856c12fb0 100644 --- a/pkg/minikube/service/service_test.go +++ b/pkg/minikube/service/service_test.go @@ -133,44 +133,6 @@ func (e MockEndpointsInterface) Get(name string, _ metav1.GetOptions) (*v1.Endpo return endpoint, nil } -func TestCheckEndpointReady(t *testing.T) { - var tests = []struct { - description string - service string - err bool - }{ - { - description: "Endpoint with no subsets should return an error", - service: "no-subsets", - err: true, - }, - { - description: "Endpoint with no ready endpoints should return an error", - service: "not-ready", - err: true, - }, - { - description: "Endpoint with at least one ready endpoint should not return an error", - service: "one-ready", - err: false, - }, - } - - for _, test := range tests { - test := test - t.Run(test.description, func(t *testing.T) { - t.Parallel() - err := checkEndpointReady(&MockEndpointsInterface{}, test.service) - if err != nil && !test.err { - t.Errorf("Check endpoints returned an error: %+v", err) - } - if err == nil && test.err { - t.Errorf("Check endpoints should have returned an error but returned nil") - } - }) - } -} - type MockServiceInterface struct { fake.FakeServices ServiceList *v1.ServiceList @@ -296,30 +258,30 @@ func TestOptionallyHttpsFormattedUrlString(t *testing.T) { expectedIsHttpSchemedURL bool }{ { - description: "no https for http schemed with no https option", - bareUrlString: "http://192.168.99.100:30563", - https: false, + description: "no https for http schemed with no https option", + bareUrlString: "http://192.168.99.100:30563", + https: false, expectedHttpsFormattedUrlString: "http://192.168.99.100:30563", expectedIsHttpSchemedURL: true, }, { - description: "no https for non-http schemed with no https option", - bareUrlString: "xyz.http.myservice:30563", - https: false, + description: "no https for non-http schemed with no https option", + bareUrlString: "xyz.http.myservice:30563", + https: false, expectedHttpsFormattedUrlString: "xyz.http.myservice:30563", expectedIsHttpSchemedURL: false, }, { - description: "https for http schemed with https option", - bareUrlString: "http://192.168.99.100:30563", - https: true, + description: "https for http schemed with https option", + bareUrlString: "http://192.168.99.100:30563", + https: true, expectedHttpsFormattedUrlString: "https://192.168.99.100:30563", expectedIsHttpSchemedURL: true, }, { - description: "no https for non-http schemed with https option and http substring", - bareUrlString: "xyz.http.myservice:30563", - https: true, + description: "no https for non-http schemed with https option and http substring", + bareUrlString: "xyz.http.myservice:30563", + https: true, expectedHttpsFormattedUrlString: "xyz.http.myservice:30563", expectedIsHttpSchemedURL: false, }, From ac6dacff4a59020b537e0edc3a975833448b18a3 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 3 Oct 2018 13:14:47 -0700 Subject: [PATCH 06/15] Address lint issues resulting from the dashboard proxy refactor. --- cmd/minikube/cmd/dashboard.go | 52 +++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/cmd/minikube/cmd/dashboard.go b/cmd/minikube/cmd/dashboard.go index be9c93cd0a7c..a9b21b3face0 100644 --- a/cmd/minikube/cmd/dashboard.go +++ b/cmd/minikube/cmd/dashboard.go @@ -33,7 +33,7 @@ import ( "k8s.io/minikube/pkg/minikube/machine" "k8s.io/minikube/pkg/minikube/service" - commonutil "k8s.io/minikube/pkg/util" + "k8s.io/minikube/pkg/util" ) var ( @@ -45,23 +45,27 @@ var ( // dashboardCmd represents the dashboard command var dashboardCmd = &cobra.Command{ Use: "dashboard", - Short: "Opens/displays the kubernetes dashboard URL for your local cluster", - Long: `Opens/displays the kubernetes dashboard URL for your local cluster`, + Short: "Access the kubernetes dashboard running within the minikube cluster", + Long: `Access the kubernetes dashboard running within the minikube cluster`, Run: func(cmd *cobra.Command, args []string) { api, err := machine.NewAPIClient() - defer api.Close() + defer func() { + err := api.Close() + if err != nil { + glog.Warningf("Failed to close API: %v", err) + } + }() + if err != nil { fmt.Fprintf(os.Stderr, "Error creating client: %v\n", err) os.Exit(1) } cluster.EnsureMinikubeRunningOrExit(api, 1) - namespace := "kube-system" + ns := "kube-system" svc := "kubernetes-dashboard" - - glog.Infof("Looking for dashboard service ...") - if err = commonutil.RetryAfter(30, func() error { return service.CheckService(namespace, svc) }, 1*time.Second); err != nil { - fmt.Fprintf(os.Stderr, "%s:%s is not running: %s\n", namespace, svc, err) + if err = util.RetryAfter(30, func() error { return service.CheckService(ns, svc) }, 1*time.Second); err != nil { + fmt.Fprintf(os.Stderr, "%s:%s is not running: %s\n", ns, svc, err) os.Exit(1) } @@ -69,9 +73,9 @@ var dashboardCmd = &cobra.Command{ if err != nil { glog.Fatalf("kubectl proxy: %v", err) } - url := dashboardURL(hostPort, namespace, svc) + url := dashboardURL(hostPort, ns, svc) - if err = commonutil.RetryAfter(30, func() error { return checkURL(url) }, 1*time.Second); err != nil { + if err = util.RetryAfter(30, func() error { return checkURL(url) }, 1*time.Second); err != nil { fmt.Fprintf(os.Stderr, "%s is not responding properly: %s\n", url, err) os.Exit(1) } @@ -81,8 +85,13 @@ var dashboardCmd = &cobra.Command{ return } fmt.Fprintln(os.Stdout, fmt.Sprintf("Opening %s in your default browser...", url)) - browser.OpenURL(url) - p.Wait() + if err = browser.OpenURL(url); err != nil { + fmt.Fprintf(os.Stderr, fmt.Sprintf("failed to open browser: %v", err)) + } + glog.Infof("Waiting forever for kubectl proxy to exit ...") + if err = p.Wait(); err != nil { + glog.Errorf("Wait: %v", err) + } }, } @@ -108,28 +117,23 @@ func kubectlProxy() (*exec.Cmd, string, error) { if err != nil { return nil, "", errors.Wrap(err, "read") } - return cmd, parseHostPort(out), nil + glog.Infof("Output: %s ...", out) + return cmd, hostPortRe.FindString(out), nil } func dashboardURL(proxy string, ns string, svc string) string { // Reference: https://github.com/kubernetes/dashboard/wiki/Accessing-Dashboard---1.7.X-and-above - return fmt.Sprintf("http://%s/api/v1/namespaces/%s/services/http:%s:/proxy/", proxy, ns, svc) -} - -func parseHostPort(out string) string { - glog.Infof("Parsing output: %s ...", out) - // Starting to serve on 127.0.0.1:8001 - return hostPortRe.FindString(out) + return fmt.Sprintf("http://%s/api/v1/nss/%s/services/http:%s:/proxy/", proxy, ns, svc) } func checkURL(url string) error { resp, err := http.Get(url) - glog.Infof("%s response: %s %s", url, err, resp) + glog.Infof("%s response: %s %+v", url, err, resp) if err != nil { return err } if resp.StatusCode != http.StatusOK { - return &commonutil.RetriableError{ + return &util.RetriableError{ Err: fmt.Errorf("unexpected response code: %d", resp.StatusCode), } } @@ -137,6 +141,6 @@ func checkURL(url string) error { } func init() { - dashboardCmd.Flags().BoolVar(&dashboardURLMode, "url", false, "Display the kubernetes dashboard in the CLI instead of opening it in the default browser") + dashboardCmd.Flags().BoolVar(&dashboardURLMode, "url", false, "Display dashboard URL instead of opening a browser") RootCmd.AddCommand(dashboardCmd) } From 4c35c505f6983f72458ad10b0b52bdf4a7f2f7d4 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 3 Oct 2018 13:22:36 -0700 Subject: [PATCH 07/15] Double HTTP wait time and make error messages more readable. --- cmd/minikube/cmd/dashboard.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/cmd/minikube/cmd/dashboard.go b/cmd/minikube/cmd/dashboard.go index a9b21b3face0..2ab38450cd56 100644 --- a/cmd/minikube/cmd/dashboard.go +++ b/cmd/minikube/cmd/dashboard.go @@ -75,7 +75,7 @@ var dashboardCmd = &cobra.Command{ } url := dashboardURL(hostPort, ns, svc) - if err = util.RetryAfter(30, func() error { return checkURL(url) }, 1*time.Second); err != nil { + if err = util.RetryAfter(60, func() error { return checkURL(url) }, 1*time.Second); err != nil { fmt.Fprintf(os.Stderr, "%s is not responding properly: %s\n", url, err) os.Exit(1) } @@ -99,23 +99,25 @@ var dashboardCmd = &cobra.Command{ func kubectlProxy() (*exec.Cmd, string, error) { path, err := exec.LookPath("kubectl") if err != nil { - return nil, "", errors.Wrap(err, "Unable to find kubectl in PATH") + return nil, "", errors.Wrap(err, "kubectl not found in PATH") } + + // port=0 picks a random system port. cmd := exec.Command(path, "proxy", "--port=0") stdoutPipe, err := cmd.StdoutPipe() if err != nil { - return nil, "", errors.Wrap(err, "stdout") + return nil, "", errors.Wrap(err, "cmd stdout") } glog.Infof("Executing: %s %s", cmd.Path, cmd.Args) if err := cmd.Start(); err != nil { - return nil, "", errors.Wrap(err, "start") + return nil, "", errors.Wrap(err, "proxy start") } reader := bufio.NewReader(stdoutPipe) - glog.Infof("Started proxy, now reading stdout pipe ...") + glog.Infof("proxy started, reading stdout pipe ...") out, err := reader.ReadString('\n') if err != nil { - return nil, "", errors.Wrap(err, "read") + return nil, "", errors.Wrap(err, "reading stdout pipe") } glog.Infof("Output: %s ...", out) return cmd, hostPortRe.FindString(out), nil @@ -130,7 +132,7 @@ func checkURL(url string) error { resp, err := http.Get(url) glog.Infof("%s response: %s %+v", url, err, resp) if err != nil { - return err + return errors.Wrap(err, "checkURL") } if resp.StatusCode != http.StatusOK { return &util.RetriableError{ From 2956621c20c048201c8884f3a26d6e294ac89240 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 3 Oct 2018 13:30:09 -0700 Subject: [PATCH 08/15] Add function comments, improve proxy stdout log message --- cmd/minikube/cmd/dashboard.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/dashboard.go b/cmd/minikube/cmd/dashboard.go index 2ab38450cd56..7f1f9e198a4d 100644 --- a/cmd/minikube/cmd/dashboard.go +++ b/cmd/minikube/cmd/dashboard.go @@ -39,6 +39,7 @@ import ( var ( dashboardURLMode bool // Matches: 127.0.0.1:8001 + // TODO(tstromberg): Get kubectl to implement a stable supported output format. hostPortRe = regexp.MustCompile(`127.0.0.1:\d{4,}`) ) @@ -119,15 +120,17 @@ func kubectlProxy() (*exec.Cmd, string, error) { if err != nil { return nil, "", errors.Wrap(err, "reading stdout pipe") } - glog.Infof("Output: %s ...", out) + glog.Infof("proxy stdout: %s", out) return cmd, hostPortRe.FindString(out), nil } +// dashboardURL generates a URL for accessing the dashboard service func dashboardURL(proxy string, ns string, svc string) string { // Reference: https://github.com/kubernetes/dashboard/wiki/Accessing-Dashboard---1.7.X-and-above return fmt.Sprintf("http://%s/api/v1/nss/%s/services/http:%s:/proxy/", proxy, ns, svc) } +// checkURL checks if a URL returns 200 HTTP OK func checkURL(url string) error { resp, err := http.Get(url) glog.Infof("%s response: %s %+v", url, err, resp) From d38705a42b41d38fb2e0e23cdeaa3a66f337158a Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 3 Oct 2018 13:58:25 -0700 Subject: [PATCH 09/15] Fix overzealous replacement of namespace -> ns that broke URL generation --- cmd/minikube/cmd/dashboard.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/dashboard.go b/cmd/minikube/cmd/dashboard.go index 7f1f9e198a4d..d8df81bf80f4 100644 --- a/cmd/minikube/cmd/dashboard.go +++ b/cmd/minikube/cmd/dashboard.go @@ -127,7 +127,7 @@ func kubectlProxy() (*exec.Cmd, string, error) { // dashboardURL generates a URL for accessing the dashboard service func dashboardURL(proxy string, ns string, svc string) string { // Reference: https://github.com/kubernetes/dashboard/wiki/Accessing-Dashboard---1.7.X-and-above - return fmt.Sprintf("http://%s/api/v1/nss/%s/services/http:%s:/proxy/", proxy, ns, svc) + return fmt.Sprintf("http://%s/api/v1/namespaces/%s/services/http:%s:/proxy/", proxy, ns, svc) } // checkURL checks if a URL returns 200 HTTP OK From c079bb193458a7e385ba2c39b014bd74be5baccf Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 3 Oct 2018 15:06:30 -0700 Subject: [PATCH 10/15] Update integration tests to handle newly persistant dashboard. --- cmd/minikube/cmd/dashboard.go | 11 +++---- test/integration/addons_test.go | 51 +++++++++++++++++++-------------- test/integration/mount_test.go | 2 +- test/integration/util/util.go | 13 +++++++-- 4 files changed, 47 insertions(+), 30 deletions(-) diff --git a/cmd/minikube/cmd/dashboard.go b/cmd/minikube/cmd/dashboard.go index d8df81bf80f4..88093b12e66a 100644 --- a/cmd/minikube/cmd/dashboard.go +++ b/cmd/minikube/cmd/dashboard.go @@ -83,12 +83,13 @@ var dashboardCmd = &cobra.Command{ if dashboardURLMode { fmt.Fprintln(os.Stdout, url) - return - } - fmt.Fprintln(os.Stdout, fmt.Sprintf("Opening %s in your default browser...", url)) - if err = browser.OpenURL(url); err != nil { - fmt.Fprintf(os.Stderr, fmt.Sprintf("failed to open browser: %v", err)) + } else { + fmt.Fprintln(os.Stdout, fmt.Sprintf("Opening %s in your default browser...", url)) + if err = browser.OpenURL(url); err != nil { + fmt.Fprintf(os.Stderr, fmt.Sprintf("failed to open browser: %v", err)) + } } + glog.Infof("Waiting forever for kubectl proxy to exit ...") if err = p.Wait(); err != nil { glog.Errorf("Wait: %v", err) diff --git a/test/integration/addons_test.go b/test/integration/addons_test.go index ca30f4fa9e60..ea88fb41f3c4 100644 --- a/test/integration/addons_test.go +++ b/test/integration/addons_test.go @@ -19,9 +19,10 @@ limitations under the License. package integration import ( - "errors" "fmt" + "io/ioutil" "net" + "net/http" "net/url" "path/filepath" "strings" @@ -49,37 +50,45 @@ func testDashboard(t *testing.T) { t.Parallel() minikubeRunner := NewMinikubeRunner(t) - var u *url.URL - - checkDashboard := func() error { - var err error - dashboardURL := minikubeRunner.RunCommand("dashboard --url", false) - if dashboardURL == "" { - return errors.New("error getting dashboard URL") - } - u, err = url.Parse(strings.TrimSpace(dashboardURL)) + _, out := minikubeRunner.RunDaemon("dashboard --url") + defer func() { + err := cmd.Process.Kill() if err != nil { - return err + t.Logf("Failed to kill mount command: %v", err) } - return nil + }() + + s, err := out.ReadString('\n') + if err != nil { + t.Fatalf("failed to read url: %v", err) } - if err := util.Retry(t, checkDashboard, 2*time.Second, 60); err != nil { - t.Fatalf("error checking dashboard URL: %v", err) + u, err := url.Parse(strings.TrimSpace(s)) + if err != nil { + t.Fatalf("failed to parse %q: %v", s, err) } if u.Scheme != "http" { - t.Fatalf("wrong scheme in dashboard URL, expected http, actual %s", u.Scheme) + t.Errorf("got Scheme %s, expected http", u.Scheme) } - host, port, err := net.SplitHostPort(u.Host) + host, _, err := net.SplitHostPort(u.Host) if err != nil { - t.Fatalf("failed to split dashboard host %s: %v", u.Host, err) - } - if port != "30000" { - t.Errorf("Dashboard is exposed on wrong port, expected 30000, actual %s", port) + t.Fatalf("failed SplitHostPort: %v", err) } if host != "127.0.0.1" { - t.Errorf("host is %s, expected 127.0.0.1", host) + t.Errorf("got host %s, expected 127.0.0.1", host) + } + + resp, err := http.Get(u.String()) + if err != nil { + t.Fatalf("failed get: %v", err) + } + if resp.StatusCode != http.StatusOK { + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatalf("Unable to read http response body: %v", err) + } + t.Errorf("%s returned status code %d, expected %d.\nbody:\n%s", u, resp.StatusCode, http.StatusOK, body) } } diff --git a/test/integration/mount_test.go b/test/integration/mount_test.go index 9975d205ae12..7627d8494454 100644 --- a/test/integration/mount_test.go +++ b/test/integration/mount_test.go @@ -46,7 +46,7 @@ func testMounting(t *testing.T) { defer os.RemoveAll(tempDir) mountCmd := fmt.Sprintf("mount %s:/mount-9p", tempDir) - cmd := minikubeRunner.RunDaemon(mountCmd) + cmd, _ := minikubeRunner.RunDaemon(mountCmd) defer func() { err := cmd.Process.Kill() if err != nil { diff --git a/test/integration/util/util.go b/test/integration/util/util.go index 7945ac4e184b..bef89d63d1f0 100644 --- a/test/integration/util/util.go +++ b/test/integration/util/util.go @@ -17,6 +17,7 @@ limitations under the License. package util import ( + "bufio" "bytes" "encoding/json" "fmt" @@ -81,15 +82,21 @@ func (m *MinikubeRunner) RunCommand(command string, checkError bool) string { return string(stdout) } -func (m *MinikubeRunner) RunDaemon(command string) *exec.Cmd { +func (m *MinikubeRunner) RunDaemon(command string) (*exec.Cmd, *bufio.Reader) { commandArr := strings.Split(command, " ") path, _ := filepath.Abs(m.BinaryPath) cmd := exec.Command(path, commandArr...) - err := cmd.Start() + stdoutPipe, err := cmd.StdoutPipe() + if err != nil { + m.T.Fatalf("stdout pipe failed: %v", err) + } + + err = cmd.Start() if err != nil { m.T.Fatalf("Error running command: %s %s", command, err) } - return cmd + return cmd, bufio.NewReader(stdoutPipe) + } func (m *MinikubeRunner) SSH(command string) (string, error) { From 57054acfb60a03272153d1da02d815faeef24c6e Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 3 Oct 2018 15:12:40 -0700 Subject: [PATCH 11/15] Add missing reference to cmd. --- test/integration/addons_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/addons_test.go b/test/integration/addons_test.go index ea88fb41f3c4..208bad7c2241 100644 --- a/test/integration/addons_test.go +++ b/test/integration/addons_test.go @@ -50,7 +50,7 @@ func testDashboard(t *testing.T) { t.Parallel() minikubeRunner := NewMinikubeRunner(t) - _, out := minikubeRunner.RunDaemon("dashboard --url") + cmd, out := minikubeRunner.RunDaemon("dashboard --url") defer func() { err := cmd.Process.Kill() if err != nil { From 7feb46edcbd35ffe8e8b097c38223050179b9542 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 3 Oct 2018 15:16:37 -0700 Subject: [PATCH 12/15] Removed unneccesary changes to download errors --- pkg/minikube/cluster/cluster.go | 2 +- pkg/util/downloader.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/minikube/cluster/cluster.go b/pkg/minikube/cluster/cluster.go index 50c58c364e04..f37ccd2f91da 100644 --- a/pkg/minikube/cluster/cluster.go +++ b/pkg/minikube/cluster/cluster.go @@ -217,7 +217,7 @@ func createHost(api libmachine.API, config cfg.MachineConfig) (*host.Host, error if config.VMDriver != "none" { if err := config.Downloader.CacheMinikubeISOFromURL(config.MinikubeISO); err != nil { - return nil, errors.Wrap(err, "unable to cache ISO") + return nil, errors.Wrap(err, "Error attempting to cache minikube ISO from URL") } } diff --git a/pkg/util/downloader.go b/pkg/util/downloader.go index 11e490d869e4..8a72c6eb525b 100644 --- a/pkg/util/downloader.go +++ b/pkg/util/downloader.go @@ -72,9 +72,9 @@ func (f DefaultDownloader) CacheMinikubeISOFromURL(isoURL string) error { options.ChecksumHash = crypto.SHA256 } - fmt.Printf("Downloading Minikube ISO: %s\n", isoURL) + fmt.Println("Downloading Minikube ISO") if err := download.ToFile(isoURL, f.GetISOCacheFilepath(isoURL), options); err != nil { - return errors.Wrap(err, "ToFile") + return errors.Wrap(err, "Error downloading Minikube ISO") } return nil From 534325e88ffa51e5ad070fa5b3b31bb127638575 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 3 Oct 2018 21:57:17 -0700 Subject: [PATCH 13/15] Use %v for errors instead of %s --- cmd/minikube/cmd/dashboard.go | 6 +++--- test/integration/util/util.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/minikube/cmd/dashboard.go b/cmd/minikube/cmd/dashboard.go index 88093b12e66a..1bde5f801d39 100644 --- a/cmd/minikube/cmd/dashboard.go +++ b/cmd/minikube/cmd/dashboard.go @@ -66,7 +66,7 @@ var dashboardCmd = &cobra.Command{ ns := "kube-system" svc := "kubernetes-dashboard" if err = util.RetryAfter(30, func() error { return service.CheckService(ns, svc) }, 1*time.Second); err != nil { - fmt.Fprintf(os.Stderr, "%s:%s is not running: %s\n", ns, svc, err) + fmt.Fprintf(os.Stderr, "%s:%s is not running: %v\n", ns, svc, err) os.Exit(1) } @@ -77,7 +77,7 @@ var dashboardCmd = &cobra.Command{ url := dashboardURL(hostPort, ns, svc) if err = util.RetryAfter(60, func() error { return checkURL(url) }, 1*time.Second); err != nil { - fmt.Fprintf(os.Stderr, "%s is not responding properly: %s\n", url, err) + fmt.Fprintf(os.Stderr, "%s is not responding properly: %v\n", url, err) os.Exit(1) } @@ -134,7 +134,7 @@ func dashboardURL(proxy string, ns string, svc string) string { // checkURL checks if a URL returns 200 HTTP OK func checkURL(url string) error { resp, err := http.Get(url) - glog.Infof("%s response: %s %+v", url, err, resp) + glog.Infof("%s response: %v %+v", url, err, resp) if err != nil { return errors.Wrap(err, "checkURL") } diff --git a/test/integration/util/util.go b/test/integration/util/util.go index bef89d63d1f0..881a16592de8 100644 --- a/test/integration/util/util.go +++ b/test/integration/util/util.go @@ -93,7 +93,7 @@ func (m *MinikubeRunner) RunDaemon(command string) (*exec.Cmd, *bufio.Reader) { err = cmd.Start() if err != nil { - m.T.Fatalf("Error running command: %s %s", command, err) + m.T.Fatalf("Error running command: %s %v", command, err) } return cmd, bufio.NewReader(stdoutPipe) From fd0d46680e2e639ec759079fecb62f464b9372ed Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Thu, 4 Oct 2018 11:35:24 -0700 Subject: [PATCH 14/15] Respect the -p flag to set the appropriate profile. --- cmd/minikube/cmd/dashboard.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/minikube/cmd/dashboard.go b/cmd/minikube/cmd/dashboard.go index 1bde5f801d39..3372a82cb4f5 100644 --- a/cmd/minikube/cmd/dashboard.go +++ b/cmd/minikube/cmd/dashboard.go @@ -30,6 +30,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" "k8s.io/minikube/pkg/minikube/cluster" + "k8s.io/minikube/pkg/minikube/config" "k8s.io/minikube/pkg/minikube/machine" "k8s.io/minikube/pkg/minikube/service" @@ -104,8 +105,9 @@ func kubectlProxy() (*exec.Cmd, string, error) { return nil, "", errors.Wrap(err, "kubectl not found in PATH") } - // port=0 picks a random system port. - cmd := exec.Command(path, "proxy", "--port=0") + // port=0 picks a random system port + // config.GetMachineName() respects the -p (profile) flag + cmd := exec.Command(path, "--context", config.GetMachineName(), "proxy", "--port=0") stdoutPipe, err := cmd.StdoutPipe() if err != nil { return nil, "", errors.Wrap(err, "cmd stdout") From 687b62cfe7cae82e3a1cd47fe75e23f782fc6a56 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Thu, 4 Oct 2018 13:40:08 -0700 Subject: [PATCH 15/15] Let gofmt 1.10.4 rewrite service_test.go. NOTE: This differs against what 1.11 would do, but matches our current Travis CI configuration. --- pkg/minikube/service/service_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/minikube/service/service_test.go b/pkg/minikube/service/service_test.go index 473856c12fb0..7268e49eaaed 100644 --- a/pkg/minikube/service/service_test.go +++ b/pkg/minikube/service/service_test.go @@ -258,30 +258,30 @@ func TestOptionallyHttpsFormattedUrlString(t *testing.T) { expectedIsHttpSchemedURL bool }{ { - description: "no https for http schemed with no https option", - bareUrlString: "http://192.168.99.100:30563", - https: false, + description: "no https for http schemed with no https option", + bareUrlString: "http://192.168.99.100:30563", + https: false, expectedHttpsFormattedUrlString: "http://192.168.99.100:30563", expectedIsHttpSchemedURL: true, }, { - description: "no https for non-http schemed with no https option", - bareUrlString: "xyz.http.myservice:30563", - https: false, + description: "no https for non-http schemed with no https option", + bareUrlString: "xyz.http.myservice:30563", + https: false, expectedHttpsFormattedUrlString: "xyz.http.myservice:30563", expectedIsHttpSchemedURL: false, }, { - description: "https for http schemed with https option", - bareUrlString: "http://192.168.99.100:30563", - https: true, + description: "https for http schemed with https option", + bareUrlString: "http://192.168.99.100:30563", + https: true, expectedHttpsFormattedUrlString: "https://192.168.99.100:30563", expectedIsHttpSchemedURL: true, }, { - description: "no https for non-http schemed with https option and http substring", - bareUrlString: "xyz.http.myservice:30563", - https: true, + description: "no https for non-http schemed with https option and http substring", + bareUrlString: "xyz.http.myservice:30563", + https: true, expectedHttpsFormattedUrlString: "xyz.http.myservice:30563", expectedIsHttpSchemedURL: false, },