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
Get Diagnostics: Download logs and diagnostics data from SSVM, CPVM, Router #3350
Get Diagnostics: Download logs and diagnostics data from SSVM, CPVM, Router #3350
Conversation
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.
one structural comment: the use of polymorphism in the DiagnosticsFiles fileprocessor package may not be the most efficient, but seems functional. havy testing needed to but bith unit and marvin tests are provided.
server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
Outdated
Show resolved
Hide resolved
Design review:
|
@blueorangutan package |
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2788 |
@blueorangutan test |
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-3591)
|
@rhtyd the current implementation is using cmd-answer pattern to indicate which files needs to be copied, but using SCP to copy the files using this library |
d2f3096
to
a3067f5
Compare
@nvazquez let me revisit this PR soon, I need to check between which points we're scp-ing. The VR logs can grow as much as 0.5GBs in size, if we're passing the payload via cmd-answer pattern then it can potentially cause out-of-memory issues in the JVM process/agent that does this. |
Rebased against master, will test and review the design and implementation. |
Several issues with the implementation fixed, there are two global settings essentially to configure the list of CPVM/SSVM files and router files which are hosted in the NFS based secondary storage at Global settings added by this PR:
|
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.
Works OK, however it adds dependency on SSVM/secondary storage and copying of files assume that secondary storage is NFS based and is accessible. On non-NFS storages, this may not explicitly tell the admin that it does not work for non-NFS secondary storage (in most cases unlikely).
Pending testing on XenServer and VMware. KVM LGTM.
@blueorangutan package |
Packaging result: ✖centos6 ✖centos7 ✖debian. JID-555 |
@blueorangutan package |
@anuragaw a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-559 |
@blueorangutan test |
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-736)
|
@blueorangutan package |
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-568 |
@blueorangutan test |
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-747)
|
@blueorangutan package |
@anuragaw a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-592 |
4e622d8
to
b3d46d8
Compare
@blueorangutan package |
@anuragaw a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-598 |
@blueorangutan test |
@anuragaw a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
LGTM. Tested manually by @borisstoyanov and reviewed by me |
Trillian test result (tid-772)
|
Merging based on LGTMs/Approvals and extensive manual testing (in-house) |
…Router (apache#3350) * * Complete API implementation * Complete UI integration * Complete marvin test * Complete Secondary storage GC background task * improve UI labels * slight reword and add another missing description * improve download message clarity * Address comments * multiple fixes and cleanups Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com> * fix more bugs, let it return ip rule list in another log file Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com> * fix missing iprule bug Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com> * add support for ARCHIVE type of object to be linked/setup on secstorage Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com> * Fix retrieving files for Xenserver * Update get_diagnostics_files.py * Fix bug where executable scripts weren't handled * Fixed error on script cmd generation * Do not filter name for log files as it would override similar prefix script names * Addressed code review comments * log error instead of printstacktrace * Treat script as executable and shell script * Check missing script name case and write to output instead of catching exception * Use shell = true instead of shlex to support any executable * fix xenserver bug * don't set dir permission for vmware * Code review comments - refactoring * Add check for possible NPE * Remove unused imoprt after rebase * Add better description for configs Co-authored-by: Nicolas Vazquez <nicovazquez90@gmail.com> Co-authored-by: Rohit Yadav <rohit@apache.org> Co-authored-by: Anurag Awasthi <anurag.awasthi@shapeblue.com>
Description
This implements a new feature to get logs and diagnostics data from systemvms (CPVM, SSVM) and virtual routers as a downloadable zip file from the secondary storage. The diagnostics zip files live in the
diagnostics
directory of the secondary storage. The feature is only supported for NFS based secondary storage and root admins.FS: https://cwiki.apache.org/confluence/display/CLOUDSTACK/CloudStack+Get+Diagnostics+Data+API
The feature adds the following global settings:
In the UI, the root admin can now see a download button to get the diagnostics data from CPVM, SSVM and VRs.
Fixes: #3593
Types of changes
Screenshots (if appropriate):
Download icon in menu:
Prompt to override default files:
Download link:
Files in the tarball:
How Has This Been Tested?
Manual testing of downloading diagnostics for VRs, SSVMs and CPVMs