Defer dashboard deployment until "minikube dashboard" is executed #3485
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tstromberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
| @@ -65,19 +65,30 @@ var dashboardCmd = &cobra.Command{ | |||
| } | |||
| cluster.EnsureMinikubeRunningOrExit(api, 1) | |||
|
|
|||
| fmt.Fprintln(os.Stderr, "Enabling dashboard ...") | |||
| // Enable the dashboard add-on | |||
| err = configcmd.Set("dashboard", "true") | |||
There was a problem hiding this comment.
is this idempontent? If yes, then we don't need "Enabling dashboard ..." the second time (I think addons enable is idempotent) - would be nice to check for enablement.
| ns := "kube-system" | ||
| svc := "kubernetes-dashboard" | ||
| if err = util.RetryAfter(30, func() error { return service.CheckService(ns, svc) }, 1*time.Second); err != nil { | ||
| fmt.Fprintln(os.Stderr, "Verifying dashboard health ...") | ||
| if err = util.RetryAfter(180, func() error { return service.CheckService(ns, svc) }, 1*time.Second); err != nil { |
There was a problem hiding this comment.
It would be interesting to keep a "watch" on the dashboard pod + on a separate line the result of the service status? But this idea might be better off in a separate PR. It's just 3 minutes of potential waiting is definitely long. Out of those 1 minute is almost guaranteed to be just the addon-manager kicking in...
There was a problem hiding this comment.
A good idea, but due to the complexity l will leave this for another PR. This step generally only takes a second or two, but to avoid flakes in places with poor connectivity, I wanted to make sure to wait an extended period for the pod to come up.
| fmt.Fprintf(os.Stderr, "%s:%s is not running: %v\n", ns, svc, err) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| fmt.Fprintln(os.Stderr, "Launching proxy ...") |
There was a problem hiding this comment.
| fmt.Fprintln(os.Stderr, "Launching proxy ...") | |
| fmt.Fprintln(os.Stderr, "Launching dashboard proxy ...") |
| p, hostPort, err := kubectlProxy() | ||
| if err != nil { | ||
| glog.Fatalf("kubectl proxy: %v", err) | ||
| } | ||
| url := dashboardURL(hostPort, ns, svc) | ||
|
|
||
| fmt.Fprintln(os.Stderr, "Verifying proxy health ...") |
There was a problem hiding this comment.
| fmt.Fprintln(os.Stderr, "Verifying proxy health ...") | |
| fmt.Fprintln(os.Stderr, "Verifying dashboard proxy health ...") |
|
A lot of people loved having features like the cache and the dashboard available by default, but I guess if it clearly documented how to turn them on again they can still do it on the command-line (or auto-enable). Guess it was features like heapster that made it too heavy , don't recall the dashboard itself being too bad ? Especially not if comparing to the boot time and resource consumption of the VM... |
tstromberg
changed the title
|
@afbjorklund - Thanks for raising the point that the PR title/description was not well crafted. This doesn't effectively change the behavior to users: the dashboard command will still work just as it has. It simply delays the dashboard pod startup until the user first wants to use it. |
|
@minikube-bot OK to test |
This simplifies cluster startup, and saves about 20MB resident memory (1% of our default VM size). More importantly, it avoids starting unused services, downloading unnecessary files, and gumming the logs up with possibly unimportant error messages relating to them.
This PR doesn't change the behavior substantially, as currently minikube starts the dashboard pod, but only blocks until it's healthy when the "dashboard" command is executed. This PR simply defers the startup as well as the health check.