Add spinner at preparing Kubernetes... #9855

alonyb · 2020-12-04T19:22:48Z

This PR is to #9091

Spinner was added in step:

🐳 Preparing Kubernetes v1.19.4 on Docker 19.03.13 ...

This spinner works with this repo(module) forked in alonyb/spinner

PR of original repo is here

With this user feels more comfortable UI

In Windows was testing too and it works without problem

Next step:
Add spinner in another processes as (Starting Control plane ... and pulling docker image... and others.)

minikube start --output=json
output:

{"data":{"currentstep":"0","message":"minikube v1.15.1 on Darwin 10.15.7","name":"Initial Minikube Setup","totalsteps":"12"},"datacontenttype":"application/json","id":"7e20202a-babe-48e1-910d-6b525e8e7ef2","source":"https://minikube.sigs.k8s.io/","specversion":"1.0","type":"io.k8s.sigs.minikube.step"} {"data":{"currentstep":"1","message":"Automatically selected the docker driver","name":"Selecting Driver","totalsteps":"12"},"datacontenttype":"application/json","id":"ba60dba1-7394-4a9b-88f4-9929f3f07580","source":"https://minikube.sigs.k8s.io/","specversion":"1.0","type":"io.k8s.sigs.minikube.step"} {"data":{"currentstep":"3","message":"Starting control plane node minikube in cluster minikube","name":"Starting Node","totalsteps":"12"},"datacontenttype":"application/json","id":"bfb2efda-bf26-4e47-b745-1b0a3366e13e","source":"https://minikube.sigs.k8s.io/","specversion":"1.0","type":"io.k8s.sigs.minikube.step"} {"data":{"currentstep":"6","message":"Creating docker container (CPUs=2, Memory=1990MB) ...","name":"Creating Container","totalsteps":"12"},"datacontenttype":"application/json","id":"b3017c52-e565-49d5-850a-f038bef8d3f5","source":"https://minikube.sigs.k8s.io/","specversion":"1.0","type":"io.k8s.sigs.minikube.step"} {"data":{"currentstep":"8","message":"Preparing Kubernetes v1.19.4 on Docker 19.03.13 ...","name":"Preparing Kubernetes","totalsteps":"12"},"datacontenttype":"application/json","id":"108d626e-6a0a-4ce3-8521-f52d9cacd6af","source":"https://minikube.sigs.k8s.io/","specversion":"1.0","type":"io.k8s.sigs.minikube.step"} {"data":{"currentstep":"10","message":"Verifying Kubernetes components...","name":"Verifying Kubernetes","totalsteps":"12"},"datacontenttype":"application/json","id":"63cded8a-fe6b-44d8-87b2-ba9aa1d82d4d","source":"https://minikube.sigs.k8s.io/","specversion":"1.0","type":"io.k8s.sigs.minikube.step"} {"data":{"currentstep":"11","message":"Enabled addons: default-storageclass, storage-provisioner","name":"Enabling Addons","totalsteps":"12"},"datacontenttype":"application/json","id":"65a9f4d7-9866-417a-a7c4-d5e9cdecfdeb","source":"https://minikube.sigs.k8s.io/","specversion":"1.0","type":"io.k8s.sigs.minikube.step"} {"data":{"message":"/usr/local/bin/kubectl is version 1.16.6-beta.0, which may have incompatibilites with Kubernetes 1.19.4."},"datacontenttype":"application/json","id":"11e64450-c0d2-4884-b210-12dcd2bc3e50","source":"https://minikube.sigs.k8s.io/","specversion":"1.0","type":"io.k8s.sigs.minikube.warning"} {"data":{"message":"Want kubectl v1.19.4? Try 'minikube kubectl -- get pods -A'"},"datacontenttype":"application/json","id":"df0d59f2-983e-4392-8529-16343e895300","source":"https://minikube.sigs.k8s.io/","specversion":"1.0","type":"io.k8s.sigs.minikube.info"} {"data":{"currentstep":"12","message":"Done! kubectl is now configured to use \"minikube\" cluster and \"default\" namespace by default","name":"Done","totalsteps":"12"},"datacontenttype":"application/json","id":"4cc4a768-206b-48fd-8ccb-d355059508f3","source":"https://minikube.sigs.k8s.io/","specversion":"1.0","type":"io.k8s.sigs.minikube.step"}

pkg/minikube/out/out.go

cmd/minikube/cmd/config/addons_list.go

…d-spinner

medyagh

I dont know think we need to pass true or false to out.String
I think we only need it in out.Step (is there a reason we pass it to out.String ?

also consider using a Constant in the out package for a better code readability

for example

out.Step(style.Tip, false, "kubectl not found. If you need it, try: 'minikube kubectl -- get pods -A'")

can be chaged to

out.Step(style.Tip, out.Spinning, "kubectl not found. If you need it, try: 'minikube kubectl -- get pods -A'")

and for false it would

out.Step(style.Tip, out.NoSpinner, "kubectl not found. If you need it, try: 'minikube kubectl -- get pods -A'")

medyagh · 2020-12-10T00:07:51Z

cmd/minikube/cmd/delete.go

@@ -142,7 +142,7 @@ func runDelete(cmd *cobra.Command, args []string) {
 	if purge && len(profilesToDelete) > 1 && !deleteAll {
 		out.ErrT(style.Notice, "Multiple minikube profiles were found - ")
 		for _, p := range profilesToDelete {
-			out.Step(style.Notice, "    - {{.profile}}", out.V{"profile": p.Name})
+			out.Step(style.Notice, out.NoSpinner, "    - {{.profile}}", out.V{"profile": p.Name})


lets move out.NoSpinner to style.NoSpinner

Rather than adding an extra argument: the styles are designed to carry all information necessary for display. I would suggest adding a Spin bool to style.Options:

minikube/pkg/minikube/style/style.go

Line 37 in ead11c6

type Options struct {

Great idea! thanks @tstromberg from 42 files we reduce it to 6

medyagh

thank you for this PR ! looks good (literally Looks good)

k8s-ci-robot · 2020-12-11T00:02:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alonyb, medyagh

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:

~~OWNERS~~ [medyagh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

medyagh · 2020-12-11T00:02:13Z

/ok-to-test

minikube-pr-bot · 2020-12-11T00:36:24Z

kvm2 Driver
error collecting results for kvm2 driver: timing run 1 with Minikube (PR 9855): timing cmd: [/home/performance-monitor/.minikube/minikube-binaries/9855/minikube start --driver=kvm2]: waiting for minikube: exit status 85
docker Driver
Times for minikube: 65.7s 27.3s 27.1s
Average time for minikube: 40.0s

Times for Minikube (PR 9855): 29.7s 29.2s 28.2s
Average time for Minikube (PR 9855): 29.0s

Averages Time Per Log

+--------------------------------+----------+--------------------+
|              LOG               | MINIKUBE | MINIKUBE (PR 9855) |
+--------------------------------+----------+--------------------+
| * minikube v1.15.1 on Debian   | 0.2s     | 0.2s               |
|                           9.11 |          |                    |
| * Using the docker driver      | 0.1s     | 0.1s               |
| based on user configuration    |          |                    |
| * Starting control plane node  | 0.1s     | 0.1s               |
| minikube in cluster minikube   |          |                    |
| * Creating docker container    | 10.5s    | 27.3s              |
| (CPUs=2, Memory=3700MB) ...    |          |                    |
| * Preparing Kubernetes v1.20.0 | 21.6s    | 0.0s               |
| on Docker 19.03.14 ...         |          |                    |
| * Verifying Kubernetes         | 7.5s     | 1.2s               |
| components...                  |          |                    |
| * Enabled addons:              | 0.1s     | 0.2s               |
| default-storageclass,          |          |                    |
| storage-provisioner            |          |                    |
| * Done! kubectl is now         | 0.0s     | 0.0s               |
| configured to use "minikube"   |          |                    |
| cluster and "default"          |          |                    |
| namespace by default           |          |                    |
+--------------------------------+----------+--------------------+

minikube-pr-bot · 2020-12-11T00:43:29Z

kvm2 Driver
error collecting results for kvm2 driver: timing run 0 with minikube: timing cmd: [./minikube start --driver=kvm2]: waiting for minikube: exit status 105
docker Driver
Times for minikube: 29.5s 27.9s 28.7s
Average time for minikube: 28.7s

Times for Minikube (PR 9855): 29.7s 28.8s 29.4s
Average time for Minikube (PR 9855): 29.3s

Averages Time Per Log

+--------------------------------+----------+--------------------+
|              LOG               | MINIKUBE | MINIKUBE (PR 9855) |
+--------------------------------+----------+--------------------+
| * minikube v1.15.1 on Debian   | 0.2s     | 0.2s               |
|                           9.11 |          |                    |
| * Using the docker driver      | 0.1s     | 0.1s               |
| based on user configuration    |          |                    |
| * Starting control plane node  | 0.1s     | 0.1s               |
| minikube in cluster minikube   |          |                    |
| * Creating docker container    | 8.9s     | 27.6s              |
| (CPUs=2, Memory=3700MB) ...    |          |                    |
| * Preparing Kubernetes v1.20.0 | 18.1s    | 0.0s               |
| on Docker 19.03.14 ...         |          |                    |
| * Verifying Kubernetes         | 1.3s     | 1.3s               |
| components...                  |          |                    |
| * Enabled addons:              | 0.1s     | 0.1s               |
| storage-provisioner,           |          |                    |
| default-storageclass           |          |                    |
| * Done! kubectl is now         | 0.0s     | 0.0s               |
| configured to use "minikube"   |          |                    |
| cluster and "default"          |          |                    |
| namespace by default           |          |                    |
+--------------------------------+----------+--------------------+

alonyb added 10 commits December 2, 2020 01:17

change config func call

df37c78

add new method to print spinner

2e3e0ab

remove break line in spinner line

7a0a2b0

add spinner library to mod

65dc5b8

add break line in windows case

88947d6

remove unnecesary file

ead02fd

remove windows break line

f37a4aa

change spinner module version

6e37ffb

restore break line

994ff36

remove old versions of the spinner

b6ee8f2

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 4, 2020

k8s-ci-robot requested review from priyawadhwa and tstromberg December 4, 2020 19:23

medyagh requested changes Dec 4, 2020

View reviewed changes

pkg/minikube/out/out.go Outdated Show resolved Hide resolved

pkg/minikube/out/out.go Outdated Show resolved Hide resolved

medyagh requested changes Dec 4, 2020

View reviewed changes

pkg/minikube/out/out.go Outdated Show resolved Hide resolved

add spinner param in step func

33b6e9a

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 7, 2020

alonyb added 5 commits December 6, 2020 21:55

remove comment line

f7abca6

fix delete unittest

23dc03c

fix lint if statement

64f4895

fix lint goimports

32bb174

restore go sum

7a9979b

medyagh requested changes Dec 8, 2020

View reviewed changes

cmd/minikube/cmd/config/addons_list.go Outdated Show resolved Hide resolved

add older lines in go sum

6b89930

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 8, 2020

Merge branch 'master' into w/add-spinner

b330cdb

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 8, 2020

change bool to second parameter

a763e05

alonyb added 4 commits December 9, 2020 09:12

Merge branch 'master' of https://github.com/alonyb/minikube into w/ad…

8d11d8b

…d-spinner

fix repo with upstream

291ea78

add comment and fix bool param

73ea4f2

fix lint

032b43c

alonyb requested a review from medyagh December 9, 2020 14:39

fix unittest

e45b42f

medyagh requested changes Dec 9, 2020

View reviewed changes

alonyb added 4 commits December 9, 2020 18:17

change false to constant

c24eda6

remove bool var from String func

42aac91

change more false by constant

e7ca292

fix lint

f729145

medyagh requested changes Dec 10, 2020

View reviewed changes

remove param && add spin bool to style enum

277e096

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 10, 2020

alonyb added 3 commits December 10, 2020 08:46

restore unittest

d474058

move spinner character const to style

8ee8989

fix lint

2d80cfc

medyagh approved these changes Dec 11, 2020

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 11, 2020

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Dec 11, 2020

medyagh merged commit 71b0237 into kubernetes:master Dec 11, 2020

This was referenced Dec 11, 2020

Remove line cleaning with \r briandowns/spinner#108

Closed

add spinner to Preparing Kubernetes that takes 20 seconds ... #9091

Closed

spowelljr mentioned this pull request Dec 11, 2020

Fixed output on back to back spinner steps #9935

Merged

alonyb mentioned this pull request Dec 22, 2020

Add spinner in creating container step #10024

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add spinner at preparing Kubernetes... #9855

Add spinner at preparing Kubernetes... #9855

alonyb commented Dec 4, 2020 •

edited

medyagh left a comment

medyagh Dec 10, 2020

tstromberg Dec 10, 2020

alonyb Dec 10, 2020

medyagh left a comment •

edited

k8s-ci-robot commented Dec 11, 2020

medyagh commented Dec 11, 2020

minikube-pr-bot commented Dec 11, 2020

minikube-pr-bot commented Dec 11, 2020

Add spinner at preparing Kubernetes... #9855

Add spinner at preparing Kubernetes... #9855

Conversation

alonyb commented Dec 4, 2020 • edited

medyagh left a comment

Choose a reason for hiding this comment

medyagh Dec 10, 2020

Choose a reason for hiding this comment

tstromberg Dec 10, 2020

Choose a reason for hiding this comment

alonyb Dec 10, 2020

Choose a reason for hiding this comment

medyagh left a comment • edited

Choose a reason for hiding this comment

k8s-ci-robot commented Dec 11, 2020

medyagh commented Dec 11, 2020

minikube-pr-bot commented Dec 11, 2020

minikube-pr-bot commented Dec 11, 2020

alonyb commented Dec 4, 2020 •

edited

medyagh left a comment •

edited