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.

Sign up for GitHub

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

Jump to bottom

Implement velero debug #4022

Merged
merged 1 commit into from
Aug 31, 2021
Merged

Implement velero debug #4022

merged 1 commit into from
Aug 31, 2021

Conversation

reasonerjt
Copy link
Contributor

@reasonerjt reasonerjt commented Aug 9, 2021 •
edited
Loading

Please add a summary of your change

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.

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:

@github-actions github-actions bot added the Dependencies Pull requests that update a dependency file label Aug 9, 2021
@reasonerjt reasonerjt marked this pull request as draft August 9, 2021 09:21
@reasonerjt reasonerjt force-pushed the velero-debug-subcmd branch 2 times, most recently from 4c4e1af to f3eb34b Compare August 9, 2021 10:48
@reasonerjt reasonerjt marked this pull request as ready for review August 10, 2021 00:33
dsu-igeek
dsu-igeek suggested changes Aug 11, 2021
View reviewed changes
Copy link
Contributor

@dsu-igeek dsu-igeek left a 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 Show resolved Hide resolved
pkg/cmd/cli/debug/debug.go Outdated Show resolved Hide resolved
pkg/cmd/cli/debug/debug.go Outdated
return err2
}
os.Args = []string{"", "run", "--debug", f.Name(), "--args", fmt.Sprintf("%s", argString)}
return crashdCmd.Run()
Copy link
Contributor

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

Copy link
Contributor Author

@reasonerjt reasonerjt Aug 24, 2021 •
edited
Loading

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

Copy link
Contributor

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

dsu-igeek
dsu-igeek suggested changes Aug 11, 2021
View reviewed changes
Copy link
Contributor

@dsu-igeek dsu-igeek left a 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

pkg/cmd/cli/debug/cshd-scripts/velero.cshd Outdated Show resolved Hide resolved
zubron
zubron suggested changes Aug 11, 2021
View reviewed changes
Copy link
Contributor

@zubron zubron left a 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.

pkg/cmd/cli/debug/debug.go Outdated Show resolved Hide resolved
pkg/cmd/cli/debug/debug.go Outdated Show resolved Hide resolved
pkg/cmd/cli/debug/debug.go Outdated Show resolved Hide resolved
@reasonerjt reasonerjt force-pushed the velero-debug-subcmd branch 8 times, most recently from f8cabd3 to 0124894 Compare August 23, 2021 12:09
@reasonerjt reasonerjt mentioned this pull request Aug 23, 2021
Closed
zubron
zubron suggested changes Aug 24, 2021
View reviewed changes
Copy link
Contributor

@zubron zubron left a 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 👍

@reasonerjt
Implement velero debug
03b6c6d 
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>
zubron
zubron approved these changes Aug 24, 2021
View reviewed changes
Copy link
Contributor

@zubron zubron left a 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! 😄

@reasonerjt reasonerjt requested a review from zubron August 24, 2021 17:01
reasonerjt added a commit to reasonerjt/velero that referenced this pull request Aug 29, 2021
@reasonerjt
Update the design of velero debug
1f6f944 
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>
dsu-igeek
dsu-igeek approved these changes Aug 31, 2021
View reviewed changes
Copy link
Contributor

@dsu-igeek dsu-igeek left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@dsu-igeek dsu-igeek merged commit 7c75cd6 into vmware-tanzu:main Aug 31, 2021
This was referenced Aug 31, 2021
Merged
Merged
@ywk253100 ywk253100 mentioned this pull request Sep 2, 2021
Closed
reasonerjt added a commit to reasonerjt/velero that referenced this pull request Oct 25, 2021
@reasonerjt
Update the design of velero debug
b93e25f 
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>
zubron pushed a commit that referenced this pull request Oct 25, 2021
@reasonerjt
Update the design of velero debug (#4081)
430410c 
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>
danfengliu pushed a commit to danfengliu/velero that referenced this pull request Jan 25, 2022
@reasonerjt @danfengliu
Implement velero debug (vmware-tanzu#4022)
487c236 
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>
danfengliu pushed a commit to danfengliu/velero that referenced this pull request Jan 25, 2022
@reasonerjt @danfengliu
Update the design of velero debug (vmware-tanzu#4081)
29d1e30 
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>
gyaozhou pushed a commit to gyaozhou/velero-read that referenced this pull request May 14, 2022
@reasonerjt @gyaozhou
Implement velero debug (vmware-tanzu#4022)
120f3b8 
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>
gyaozhou pushed a commit to gyaozhou/velero-read that referenced this pull request May 14, 2022
@reasonerjt @gyaozhou
Update the design of velero debug (vmware-tanzu#4081)
d2e1b3c 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zubron zubron zubron approved these changes

@dsu-igeek dsu-igeek dsu-igeek approved these changes

Assignees

@reasonerjt reasonerjt

Labels
Dependencies Pull requests that update a dependency file has-changelog has-unit-tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FE: Create a velero debug command for gathering troubleshooting information
3 participants
@reasonerjt @dsu-igeek @zubron