-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
framework/cluster: fix NPE for ms-host status when mgr stops #10500
base: main
Are you sure you want to change the base?
Conversation
This handles an NPE case for when management server host status is not found in the DB, when stopping the cluster manager. Signed-off-by: Rohit Yadav <[email protected]>
@blueorangutan package |
@rohityadavcloud 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. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10500 +/- ##
============================================
+ Coverage 16.15% 16.17% +0.01%
- Complexity 13274 13292 +18
============================================
Files 5666 5668 +2
Lines 498078 498187 +109
Branches 60267 60291 +24
============================================
+ Hits 80481 80592 +111
+ Misses 408584 408572 -12
- Partials 9013 9023 +10
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:
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 12659 |
@blueorangutan package |
@rohityadavcloud 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. |
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.
code lgtm
it looks this issue exists for some years |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12661 |
framework/cluster/src/main/java/com/cloud/cluster/ClusterManagerImpl.java
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.
clgtm
@rohityadavcloud, I think we should also address this fix on 4.20: cloudstack/framework/cluster/src/main/java/com/cloud/cluster/ClusterManagerImpl.java Lines 1103 to 1110 in a841ed9
What do you think? |
@bernardodemarco yes, I didn't send it on 4.20 branch as I reckon this affects devs (on non-Linux env) more than production users on Linux. But feel free to send a PR. |
Ok, got it. So, after this one gets merged, I'll backport it to the 4.20 branch in a new PR |
…erImpl.java Co-authored-by: dahn <[email protected]>
framework/cluster/src/main/java/com/cloud/cluster/ClusterManagerImpl.java
Outdated
Show resolved
Hide resolved
@blueorangutan package |
@rohityadavcloud 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 12693 |
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 handles an NPE case for when management server host status is not found in the DB, when stopping the cluster manager.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
On Mac OS, the stat collector doesn't run well as the code assumes to be running on Linux. On non-Linux env, this is easily reproducible.