Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move errors getting logs into log output itself #18007

Merged
merged 2 commits into from
Jan 29, 2024

Conversation

spowelljr
Copy link
Member

@spowelljr spowelljr commented Jan 20, 2024

Rationale:
Currently if we fail to retrieve logs from a pod or something else, we output the error to the user themselves, which can often confuse the user and make them thing the minikube logs command failed. Also, because this information isn't included in the logs, those triaging issues have no idea about the failure to retrieve logs.

For the before and after below I hard-coded a failure.

Before - the user:

$ minikube logs --file=logs.txt
E0119 16:08:44.907221   93053 logs.go:196] command /bin/bash -c "this is a fake command that will error" failed with error: /bin/bash -c "this is a fake command that will error": Process exited with status 127
stdout:

stderr:
/bin/bash: line 1: this: command not found
 output: "\n** stderr ** \n/bin/bash: line 1: this: command not found\n\n** /stderr **"

❗  unable to fetch logs for: forced-error

Note how to above could easily make the user think they failed to generate a logs file

Before - the generated file (debugging)

{"level":"info","ts":"2024-01-20T00:05:14.574Z","caller":"mvcc/hash.go:137","msg":"storing new hash","hash":325874407,"revision":3943,"compact-revision":3704}


==> forced-error <==                             

==> kernel <==
 00:08:44 up  3:12,  0 users,  load average: 0.49, 0.35, 0.31 
Linux minikube 6.5.11-linuxkit #1 SMP PREEMPT Wed Dec  6 17:08:31 UTC 2023 aarch64 aarch64 aarch64 GNU/Linux 
PRETTY_NAME="Ubuntu 22.04.3 LTS"

Note the empty output for forced error which would be confusing for someone trying to debug the users issue

After - the user

$ minikube logs --file=logs.txt
✅  Logs file created (logs.txt), remember to include it when reporting issues!

No error output and new message included to inform the user that the file was created

After - the generated file (debugging)

{"level":"info","ts":"2024-01-20T00:10:14.590Z","caller":"mvcc/hash.go:137","msg":"storing new hash","hash":2871990605,"revision":4182,"compact-revision":3943}


==> forced-error <==                             
command /bin/bash -c "this is a fake command that will error" failed with error: /bin/bash -c "this is a fake command that will error": Process exited with status 127
stdout:   

stderr:   
/bin/bash: line 1: this: command not found


==> kernel <==
 00:12:17 up  3:16,  0 users,  load average: 0.14, 0.25, 0.27 
Linux minikube 6.5.11-linuxkit #1 SMP PREEMPT Wed Dec  6 17:08:31 UTC 2023 aarch64 aarch64 aarch64 GNU/Linux 
PRETTY_NAME="Ubuntu 22.04.3 LTS"  

The debugger can now see what caused the logs to not be reported

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 20, 2024
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 20, 2024
@spowelljr
Copy link
Member Author

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jan 20, 2024
Copy link
Member

@medyagh medyagh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would hugely improve the triaging experience

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@spowelljr
Copy link
Member Author

/retest-this-please

@spowelljr spowelljr merged commit 36f2879 into kubernetes:master Jan 29, 2024
22 of 41 checks passed
@spowelljr spowelljr deleted the modifyLogErrors branch January 29, 2024 19:04
@minikube-pr-bot
Copy link

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 18007) |
+----------------+----------+---------------------+
| minikube start | 50.7s    | 52.5s               |
| enable ingress | 26.4s    | 27.2s               |
+----------------+----------+---------------------+

Times for minikube start: 51.7s 52.2s 49.5s 50.5s 49.5s
Times for minikube (PR 18007) start: 52.9s 53.0s 52.0s 53.4s 50.9s

Times for minikube ingress: 25.7s 27.2s 27.2s 27.2s 24.6s
Times for minikube (PR 18007) ingress: 29.2s 27.2s 24.1s 27.7s 27.6s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 18007) |
+----------------+----------+---------------------+
| minikube start | 23.8s    | 23.2s               |
| enable ingress | 20.0s    | 19.8s               |
+----------------+----------+---------------------+

Times for minikube start: 25.7s 24.5s 22.6s 24.3s 22.0s
Times for minikube (PR 18007) start: 25.3s 21.9s 24.9s 21.6s 22.2s

Times for minikube ingress: 20.8s 19.8s 19.8s 18.8s 20.8s
Times for minikube (PR 18007) ingress: 20.8s 19.8s 20.8s 18.8s 18.8s

docker driver with containerd runtime

+-------------------+----------+---------------------+
|      COMMAND      | MINIKUBE | MINIKUBE (PR 18007) |
+-------------------+----------+---------------------+
| minikube start    | 21.7s    | 21.8s               |
| ⚠️  enable ingress | 28.5s    | 34.5s ⚠️             |
+-------------------+----------+---------------------+

Times for minikube start: 20.5s 21.0s 23.3s 20.8s 23.1s
Times for minikube (PR 18007) start: 20.3s 19.7s 20.8s 24.2s 24.1s

Times for minikube ingress: 31.3s 31.3s 18.4s 30.3s 31.4s
Times for minikube (PR 18007) ingress: 31.3s 31.3s 47.3s 31.3s 31.3s

@minikube-pr-bot

This comment has been minimized.

@minikube-pr-bot
Copy link

These are the flake rates of all failed tests.

Environment Failed Tests Flake Rate (%)
Docker_Linux_containerd TestFunctional/parallel/ImageCommands/ImageLoadDaemon (gopogh) 0.00 (chart)
Docker_Linux_containerd TestFunctional/parallel/ImageCommands/ImageLoadFromFile (gopogh) 0.00 (chart)
Docker_Linux_containerd TestFunctional/parallel/ImageCommands/ImageReloadDaemon (gopogh) 0.00 (chart)
Docker_Linux_containerd TestFunctional/parallel/ImageCommands/ImageSaveDaemon (gopogh) 0.00 (chart)
Docker_Linux_containerd TestFunctional/parallel/ImageCommands/ImageSaveToFile (gopogh) 0.00 (chart)
Docker_Linux_containerd TestFunctional/parallel/ImageCommands/ImageTagAndLoadDaemon (gopogh) 0.00 (chart)
Docker_Linux_containerd TestFunctional/serial/CacheCmd/cache/add_local (gopogh) 0.00 (chart)
Docker_Linux_crio TestFunctional/parallel/ImageCommands/ImageReloadDaemon (gopogh) 0.00 (chart)
Docker_Linux_crio TestFunctional/parallel/ImageCommands/ImageSaveDaemon (gopogh) 0.00 (chart)
Docker_Linux_crio TestFunctional/serial/CacheCmd/cache/add_local (gopogh) 0.00 (chart)
Docker_Linux TestFunctional/parallel/ImageCommands/ImageLoadDaemon (gopogh) 0.00 (chart)
Docker_Linux TestFunctional/parallel/ImageCommands/ImageLoadFromFile (gopogh) 0.00 (chart)
Docker_Linux TestFunctional/parallel/ImageCommands/ImageReloadDaemon (gopogh) 0.00 (chart)
Docker_Linux TestFunctional/parallel/ImageCommands/ImageSaveDaemon (gopogh) 0.00 (chart)
Docker_Linux TestFunctional/parallel/ImageCommands/ImageSaveToFile (gopogh) 0.00 (chart)
Docker_Linux TestFunctional/parallel/ImageCommands/ImageTagAndLoadDaemon (gopogh) 0.00 (chart)
Docker_Linux TestFunctional/serial/CacheCmd/cache/add_local (gopogh) 0.00 (chart)
KVM_Linux_containerd TestFunctional/parallel/ImageCommands/ImageLoadDaemon (gopogh) 0.00 (chart)
KVM_Linux_containerd TestFunctional/parallel/ImageCommands/ImageLoadFromFile (gopogh) 0.00 (chart)
KVM_Linux_containerd TestFunctional/parallel/ImageCommands/ImageReloadDaemon (gopogh) 0.00 (chart)
KVM_Linux_containerd TestFunctional/parallel/ImageCommands/ImageSaveDaemon (gopogh) 0.00 (chart)
KVM_Linux_containerd TestFunctional/parallel/ImageCommands/ImageSaveToFile (gopogh) 0.00 (chart)
KVM_Linux_containerd TestFunctional/parallel/ImageCommands/ImageTagAndLoadDaemon (gopogh) 0.00 (chart)
KVM_Linux_containerd TestFunctional/serial/CacheCmd/cache/add_local (gopogh) 0.00 (chart)
KVM_Linux_crio TestFunctional/parallel/ImageCommands/ImageReloadDaemon (gopogh) 0.00 (chart)
KVM_Linux_crio TestFunctional/parallel/ImageCommands/ImageSaveDaemon (gopogh) 0.00 (chart)
KVM_Linux_crio TestFunctional/parallel/ImageCommands/ImageSaveToFile (gopogh) 0.00 (chart)
KVM_Linux_crio TestFunctional/serial/CacheCmd/cache/add_local (gopogh) 0.00 (chart)
KVM_Linux TestFunctional/parallel/ImageCommands/ImageLoadDaemon (gopogh) 0.00 (chart)
KVM_Linux TestFunctional/parallel/ImageCommands/ImageLoadFromFile (gopogh) 0.00 (chart)
More tests... Continued...

Too many tests failed - See test logs for more details.

To see the flake rates of all tests by environment, click here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants