Add kubectl desc nodes to minikube logs #7105

prasadkatti · 2020-03-18T23:29:28Z

k8s-ci-robot · 2020-03-18T23:29:36Z

Hi @prasadkatti. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2020-03-18T23:30:02Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: prasadkatti
To complete the pull request process, please assign priyawadhwa
You can assign the PR to them by writing /assign @priyawadhwa in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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

minikube-bot · 2020-03-18T23:30:03Z

Can one of the admins verify this patch?

prasadkatti · 2020-03-18T23:32:50Z

/assign @priyawadhwa

Hi @priyawadhwa. I am a newbie at go and minikube. If there are any error handling or source code formatting issues, please let me know.

codecov-io · 2020-03-18T23:36:21Z

Codecov Report

Merging #7105 into master will not change coverage by %.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master    #7105   +/-   ##
=======================================
  Coverage   37.12%   37.12%           
=======================================
  Files         146      146           
  Lines        9124     9124           
=======================================
  Hits         3387     3387           
  Misses       5321     5321           
  Partials      416      416

Impacted Files	Coverage Δ
cmd/minikube/cmd/logs.go	`10.86% <0.00%> (ø)`
pkg/minikube/bootstrapper/bootstrapper.go	`0.00% <ø> (ø)`
pkg/minikube/logs/logs.go	`2.35% <0.00%> (ø)`

priyawadhwa · 2020-03-19T21:26:10Z

/ok-to-test

minikube-pr-bot · 2020-03-19T21:31:03Z

Error: running mkcmp: exit status 1

priyawadhwa · 2020-03-19T21:37:01Z

pkg/minikube/logs/logs.go

@@ -186,5 +190,15 @@ func logCommands(r cruntime.Manager, bs bootstrapper.Bootstrapper, length int, f
 	}
 	cmds[r.Name()] = r.SystemLogCmd(length)
 	cmds["container status"] = cruntime.ContainerStatusCommand()
+
+	cfg, err := config.Load(viper.GetString(config.ProfileName))
+	if err != nil && !config.IsNotExist(err) {


You can just say

if err != nil

here

okay, thanks for the review! Are the failing checks unrelated to my changes? I couldn't really tell.

minikube-pr-bot · 2020-03-19T22:08:48Z

All Times minikube: [ 70.045328 73.533552 72.477204]
All Times Minikube (PR 7105): [ 72.702375 73.201969 73.453014]

Average minikube: 72.018694
Average Minikube (PR 7105): 73.119119

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 7105) |
+----------------------+-----------+--------------------+
| minikube v           |  0.261973 |           0.248090 |
| Creating kvm2        | 45.920534 |          46.018552 |
| Preparing Kubernetes |  0.904961 |           0.906296 |
| Pulling images       |           |                    |
| Launching Kubernetes | 22.601833 |          23.674541 |
| Waiting for cluster  |  0.439471 |           0.109773 |
+----------------------+-----------+--------------------+

tstromberg · 2020-03-20T13:49:39Z

pkg/minikube/logs/logs.go

@@ -186,5 +190,15 @@ func logCommands(r cruntime.Manager, bs bootstrapper.Bootstrapper, length int, f
 	}
 	cmds[r.Name()] = r.SystemLogCmd(length)
 	cmds["container status"] = cruntime.ContainerStatusCommand()
+
+	cfg, err := config.Load(viper.GetString(config.ProfileName))


Nicely done! Thank you!

So, the frustrating part is that the configuration for this profile is already loaded, but the logs package does not have access to it. If it isn't too much trouble on your part, can you help us get that addressed? It'll be cleaner and faster that way.

I know I said differently in the issue, but lets start by moving this addition to the kubeadm bootstrapper, since it's the only one that guarantees where the kubelet is installed. Start my moving your code here:

minikube/pkg/minikube/bootstrapper/kubeadm/kubeadm.go

Line 113 in 729642e

func (k *Bootstrapper) LogCommands(o bootstrapper.LogOptions) map[string]string {

Now, to plumb in our missing loaded configuration (config.ClusterConfig). First pass the cfg variable to logs.Output:

minikube/cmd/minikube/cmd/logs.go

Line 109 in 729642e

err = logs.Output(cr, bs, runner, numberOfLines)

Then logs.logCommands, where we are now:

minikube/pkg/minikube/logs/logs.go

Line 158 in 729642e

cmds := logCommands(r, bs, lines, false)

Then to bs.LogCommands (effectively kubeadm.LogCommands), which handles bootstrapper specific logs:

minikube/pkg/minikube/logs/logs.go

Line 196 in 729642e

cmds := bs.LogCommands(bootstrapper.LogOptions{Lines: length, Follow: follow})

One tip: Some packages name the config.ClusterConfig variable as cfg, some name it cc. Use whichever nomenclature you find in the package, which I believe will usually be cfg in this part of the code base.

If this is all too much, please let me know. Thank you again! The code looks great otherwise.

I can definitely try. If I need help along the way, I will ask on the slack channel.

minikube-pr-bot · 2020-03-20T22:10:30Z

All Times minikube: [ 68.167756 69.435963 64.223079]
All Times Minikube (PR 7105): [ 63.560245 63.536464 68.230130]

Average minikube: 67.275599
Average Minikube (PR 7105): 65.108946

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 7105) |
+----------------------+-----------+--------------------+
| minikube v           |  0.217418 |           0.214019 |
| Creating kvm2        | 43.281157 |          42.415408 |
| Preparing Kubernetes |  0.743551 |           0.745798 |
| Pulling images       |           |                    |
| Launching Kubernetes | 20.878489 |          19.896172 |
| Waiting for cluster  |  0.398720 |           0.233759 |
+----------------------+-----------+--------------------+

minikube-pr-bot · 2020-03-21T01:26:23Z

All Times minikube: [ 68.277951 66.968466 65.261791]
All Times Minikube (PR 7105): [ 61.692418 65.943892 64.247131]

Average minikube: 66.836069
Average Minikube (PR 7105): 63.961147

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 7105) |
+----------------------+-----------+--------------------+
| minikube v           |  0.221126 |           0.206042 |
| Creating kvm2        | 42.923513 |          41.312698 |
| Preparing Kubernetes |  0.749826 |           0.735914 |
| Pulling images       |           |                    |
| Launching Kubernetes | 20.841427 |          19.516512 |
| Waiting for cluster  |  0.402799 |           0.561161 |
+----------------------+-----------+--------------------+

minikube-pr-bot · 2020-03-21T06:20:18Z

All Times minikube: [ 67.952134 63.561334 64.747868]
All Times Minikube (PR 7105): [ 64.043165 64.311696 67.609506]

Average minikube: 65.420445
Average Minikube (PR 7105): 65.321456

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 7105) |
+----------------------+-----------+--------------------+
| minikube v           |  0.243852 |           0.211874 |
| Creating kvm2        | 42.443723 |          41.885994 |
| Preparing Kubernetes | 20.849254 |           0.744920 |
| Pulling images       |           |                    |
| Launching Kubernetes |           |          20.275756 |
| Waiting for cluster  |  0.238035 |           0.565157 |
+----------------------+-----------+--------------------+

tstromberg · 2020-03-21T17:36:09Z

make test shows:

pkg/minikube/node/start.go:115:72: not enough arguments in call to logs.FindProblems
	have (cruntime.Manager, bootstrapper.Bootstrapper, command.Runner)
	want (cruntime.Manager, bootstrapper.Bootstrapper, "k8s.io/minikube/pkg/minikube/config".ClusterConfig, logs.logRunner)

Need any help with this PR?

prasadkatti · 2020-03-21T19:04:31Z

No, I think I am good for now. The multi-node PR was merged which has created some merge conflicts. I am trying to resolve those conflicts. I will reach out for help if I get stuck. Thanks! :)

minikube-pr-bot · 2020-03-21T19:29:48Z

All Times Minikube (PR 7105): [ 69.496254 70.614402 71.053381]
All Times minikube: [ 70.516323 68.219146 70.147858]

Average minikube: 69.627776
Average Minikube (PR 7105): 70.388013

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 7105) |
+----------------------+-----------+--------------------+
| minikube v           |  0.252745 |           0.224114 |
| Creating kvm2        | 43.444403 |          44.790834 |
| Preparing Kubernetes | 23.705339 |           0.858089 |
| Pulling images       |           |                    |
| Launching Kubernetes |           |          22.211336 |
| Waiting for cluster  |           |           0.426548 |
+----------------------+-----------+--------------------+

tstromberg · 2020-03-21T22:13:11Z

Looks great. Thank you!

prasadkatti · 2020-03-21T22:15:14Z

Thank you for your help!

Add describe nodes to minikube logs

68f973c

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 18, 2020

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 18, 2020

k8s-ci-robot requested review from priyawadhwa and RA489 March 18, 2020 23:30

k8s-ci-robot assigned priyawadhwa Mar 18, 2020

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 19, 2020

priyawadhwa approved these changes Mar 19, 2020

View reviewed changes

priyawadhwa reviewed Mar 19, 2020

View reviewed changes

minor update based on review feedback

8d48c68

tstromberg reviewed Mar 20, 2020

View reviewed changes

Add logic to get desc node output in bootstrapper LogCommands func

4faa31e

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 20, 2020

prasadkatti added 2 commits March 20, 2020 16:46

Merge branch 'master' into add_desc_node_to_minikube_logs

df80a60

some more plumbing work to pass cfg in kverify.go

c3f1f5e

Merge branch 'master' into add_desc_node_to_minikube_logs

a880bb7

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

prasadkatti added 2 commits March 21, 2020 12:18

Update the call to logProblems in node/start.go

ba6dbc5

Merge branch 'master' into add_desc_node_to_minikube_logs

376271a

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

prasadkatti requested a review from tstromberg March 21, 2020 20:01

tstromberg merged commit cf887e7 into kubernetes:master Mar 21, 2020

This was referenced Mar 23, 2020

status: show guest VM health (cpu/ram/disk) #3574

Open

dashboard: show kubectl describe output when deployment fails #4749

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add kubectl desc nodes to minikube logs #7105

Add kubectl desc nodes to minikube logs #7105

prasadkatti commented Mar 18, 2020

k8s-ci-robot commented Mar 18, 2020

k8s-ci-robot commented Mar 18, 2020

minikube-bot commented Mar 18, 2020

prasadkatti commented Mar 18, 2020

codecov-io commented Mar 18, 2020 •

edited

Loading

priyawadhwa commented Mar 19, 2020

minikube-pr-bot commented Mar 19, 2020

priyawadhwa Mar 19, 2020

prasadkatti Mar 19, 2020

minikube-pr-bot commented Mar 19, 2020

tstromberg Mar 20, 2020 •

edited

Loading

prasadkatti Mar 20, 2020

minikube-pr-bot commented Mar 20, 2020

minikube-pr-bot commented Mar 21, 2020

minikube-pr-bot commented Mar 21, 2020

tstromberg commented Mar 21, 2020

prasadkatti commented Mar 21, 2020

minikube-pr-bot commented Mar 21, 2020

tstromberg commented Mar 21, 2020

prasadkatti commented Mar 21, 2020

Add kubectl desc nodes to minikube logs #7105

Add kubectl desc nodes to minikube logs #7105

Conversation

prasadkatti commented Mar 18, 2020

k8s-ci-robot commented Mar 18, 2020

k8s-ci-robot commented Mar 18, 2020

minikube-bot commented Mar 18, 2020

prasadkatti commented Mar 18, 2020

codecov-io commented Mar 18, 2020 • edited Loading

Codecov Report

priyawadhwa commented Mar 19, 2020

minikube-pr-bot commented Mar 19, 2020

priyawadhwa Mar 19, 2020

Choose a reason for hiding this comment

prasadkatti Mar 19, 2020

Choose a reason for hiding this comment

minikube-pr-bot commented Mar 19, 2020

tstromberg Mar 20, 2020 • edited Loading

Choose a reason for hiding this comment

prasadkatti Mar 20, 2020

Choose a reason for hiding this comment

minikube-pr-bot commented Mar 20, 2020

minikube-pr-bot commented Mar 21, 2020

minikube-pr-bot commented Mar 21, 2020

tstromberg commented Mar 21, 2020

prasadkatti commented Mar 21, 2020

minikube-pr-bot commented Mar 21, 2020

tstromberg commented Mar 21, 2020

prasadkatti commented Mar 21, 2020

codecov-io commented Mar 18, 2020 •

edited

Loading

tstromberg Mar 20, 2020 •

edited

Loading