-
Notifications
You must be signed in to change notification settings - Fork 373
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
Add goroutine stack dump in support bundle #5538
Add goroutine stack dump in support bundle #5538
Conversation
Signed-off-by: Aniket Raj <info.aniketraj@gmail.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.
This LGTM. I will wait for @tnqn to review since he opened the corresponding Github issue. In particular, Quan was originally asking for this to be optional (with an antctl command-line flag). But at the time, I don't think he was aware that we already collected the heap profile unconditionally. So let's see what he says.
@tnqn Did you get a chance to look at this? LMK if a CLI flag needs to be added. |
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.
LGTM, the patch is really timely, even yesterday I was debugging a deadlock issue: #5565. With this enhancement, it could have been much easier to identify the root cause.
Now I don't think we need an extra flag since memprofile is already there, and even the other information included in the bundle could have something users don't want to be publicly visible, like address and log. We should perhaps always suggest users to share supportbundle in a private way.
/test-e2e |
Fix the TestVMAgent/testExternalNodeSupportBundleCollection test. The test case was not updated correctly after antrea-io#5538 was merged. Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Fix the TestVMAgent/testExternalNodeSupportBundleCollection test. The test case was not updated correctly after antrea-io#5538 was merged. Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Fix the TestVMAgent/testExternalNodeSupportBundleCollection test. The test case was not updated correctly after #5538 was merged. Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Fix the TestVMAgent/testExternalNodeSupportBundleCollection test. The test case was not updated correctly after antrea-io#5538 was merged. Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Fix the TestVMAgent/testExternalNodeSupportBundleCollection test. The test case was not updated correctly after #5538 was merged. Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Fix the TestVMAgent/testExternalNodeSupportBundleCollection test. The test case was not updated correctly after antrea-io#5538 was merged. Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Fix the TestVMAgent/testExternalNodeSupportBundleCollection test. The test case was not updated correctly after #5538 was merged. Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Fix the TestVMAgent/testExternalNodeSupportBundleCollection test. The test case was not updated correctly after antrea-io#5538 was merged. Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Fixes #5243
This PR adds goroutine stack dump in support bundle for both agent and controller. Sample shown below for controller.
The original issue also talks about addition of
/metrics
API response to support bundle but I hope it'd be fine if it's done in a follow-up PR.