-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add logs for storage pools reordering #10419
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
base: main
Are you sure you want to change the base?
Conversation
Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10419 +/- ##
============================================
- Coverage 16.58% 16.57% -0.01%
- Complexity 13870 13987 +117
============================================
Files 5719 5745 +26
Lines 507194 510845 +3651
Branches 61573 62136 +563
============================================
+ Hits 84093 84697 +604
- Misses 413681 416676 +2995
- Partials 9420 9472 +52
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
...rage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
Outdated
Show resolved
Hide resolved
...rage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
Outdated
Show resolved
Hide resolved
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 general remark on these changes; I see only trace and debug logs, but if axtra logs are really needed I would expect at least hight level logging to be added (info and higher) were considerations made to do this? (DEBUG should not be on in production environments in principle, unless trouble shooting is going on)
Alright, I've just changed the level of logs on reordering methods, @DaanHoogland |
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.
makes mostly sense, just one info i'd like to question.
...rage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
Outdated
Show resolved
Hide resolved
@blueorangutan package |
@julien-vaz a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12691 |
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.
clgtm
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.
clgtm
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@julien-vaz could you fix the conflicts? |
Sure! |
Substitui por nova sintaxe do Log4j
…ocator/AbstractStoragePoolAllocator.java Co-authored-by: dahn <[email protected]>
53c59bf
to
04e69aa
Compare
@blueorangutan package |
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✖️ debian ✔️ suse15. SL-JID 13617 |
@blueorangutan test |
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-13481)
|
@sureshanaparti , do you think we need anymore testing on this? (/me not convinced) |
I've just built the packages locally, so I'm triggering the Blue Orangutan again. |
@blueorangutan package |
@julien-vaz a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
why do you think this is needed @julien-vaz ? |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13881 |
Because the debian build has failed previously |
ok, nothing related to your changes but now you asked for it ;) |
@DaanHoogland a [SL] Trillian-Jenkins test job (ubuntu22 mgmt + kvm-ubuntu22) has been kicked to run smoke tests |
[SF] Trillian test result (tid-13665)
|
...rage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
Outdated
Show resolved
Hide resolved
sorry @julien-vaz , I did some greps on the logs and it all looks fine. |
…ocator/AbstractStoragePoolAllocator.java Co-authored-by: dahn <[email protected]>
@blueorangutan package |
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✖️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 14158 |
Description
Since the storage pool reordering process was lacking logs for troubleshooting, some debug level log messages were added and trace level log messages were changed to the debug level. Also, the log messages were rewritten according to the new Log4j2 syntax.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
How Has This Been Tested?