-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Implement velero debug
#4022
Implement velero debug
#4022
Conversation
0687a6a
to
9b42999
Compare
9b42999
to
655ed6d
Compare
4c4e1af
to
f3eb34b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, I ran it and it works, that's pretty cool! Some changes that would be nice. Would you add an issue for an E2E test for this? We should be able to verify that debug is getting the expected logs. Also, let's add an issue for documentation, we should probably update site/content/docs/main/troubleshooting.md
pkg/cmd/cli/debug/debug.go
Outdated
return err2 | ||
} | ||
os.Args = []string{"", "run", "--debug", f.Name(), "--args", fmt.Sprintf("%s", argString)} | ||
return crashdCmd.Run() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we want customers to run this and they're probably just going to run "velero debug", let's output the file that was written and some instructions on what to do with it. Let's discuss where we should tell them to put it for support requests, can't think of the right answer at the moment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about I submit another PR to update this section?
https://velero.io/docs/v1.6/troubleshooting/#general-troubleshooting-information
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that sounds good. I also think we'll need another follow up post-release to update our issue template: https://github.com/vmware-tanzu/velero/blob/main/.github/ISSUE_TEMPLATE/bug_report.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Listed some more resources we should capture
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @reasonerjt! This is looking great so far and I am really excited for this to be available 😄 I just have a few comments and suggestions on top of what @dsu-igeek has already suggested.
f8cabd3
to
0124894
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the suggested changes, @reasonerjt! I've just spotted a few issues with the output path for the tarball, and I have some questions about the k8s library upgrade and the verbose setting but it's looking great otherwise 👍
This PR added a subcommand `velero debug`, which leverages `crashd` to collect logs and specs of velero server components and bundle them in a tarball. Signed-off-by: Daniel Jiang <jiangd@vmware.com>
0124894
to
03b6c6d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this, @reasonerjt! 😄
After the PR to implement `velero debug` - vmware-tanzu#4022 is reviewed, there are some suggestion to let the command collect more resources, this commit make the change to the design doc to reflect those changes. It also remove some sections that are no longer relevant after `crashd` has made enhancement in the v0.3.4 release. Signed-off-by: Daniel Jiang <jiangd@vmware.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
After the PR to implement `velero debug` - vmware-tanzu#4022 is reviewed, there are some suggestion to let the command collect more resources, this commit make the change to the design doc to reflect those changes. It also remove some sections that are no longer relevant after `crashd` has made enhancement in the v0.3.4 release. Signed-off-by: Daniel Jiang <jiangd@vmware.com>
After the PR to implement `velero debug` - #4022 is reviewed, there are some suggestion to let the command collect more resources, this commit make the change to the design doc to reflect those changes. It also remove some sections that are no longer relevant after `crashd` has made enhancement in the v0.3.4 release. Signed-off-by: Daniel Jiang <jiangd@vmware.com>
This PR added a subcommand `velero debug`, which leverages `crashd` to collect logs and specs of velero server components and bundle them in a tarball. Signed-off-by: Daniel Jiang <jiangd@vmware.com>
After the PR to implement `velero debug` - vmware-tanzu#4022 is reviewed, there are some suggestion to let the command collect more resources, this commit make the change to the design doc to reflect those changes. It also remove some sections that are no longer relevant after `crashd` has made enhancement in the v0.3.4 release. Signed-off-by: Daniel Jiang <jiangd@vmware.com>
This PR added a subcommand `velero debug`, which leverages `crashd` to collect logs and specs of velero server components and bundle them in a tarball. Signed-off-by: Daniel Jiang <jiangd@vmware.com>
After the PR to implement `velero debug` - vmware-tanzu#4022 is reviewed, there are some suggestion to let the command collect more resources, this commit make the change to the design doc to reflect those changes. It also remove some sections that are no longer relevant after `crashd` has made enhancement in the v0.3.4 release. Signed-off-by: Daniel Jiang <jiangd@vmware.com>
Please add a summary of your change
This PR added a subcommand
velero debug
, which leveragescrashd
tocollect logs and specs of velero server components and bundle them in a
tarball.
After this repo is pinned to v0.3.4 of
crashd
the k8s libraries were updated and caused failures in test cases.This PR fixed the failures and I opened #4059 to do further code cleanup.
Does your change fix a particular issue?
Fixes #675
Please indicate you've done the following:
/kind changelog-not-required
.site/content/docs/main
.