-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Improve message "Network is unavailable. Please contact administrator" #11981
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
Conversation
DaanHoogland
left a comment
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
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11981 +/- ##
============================================
- Coverage 17.46% 17.46% -0.01%
+ Complexity 15516 15514 -2
============================================
Files 5913 5913
Lines 529385 529389 +4
Branches 64679 64680 +1
============================================
Hits 92450 92450
- Misses 426517 426521 +4
Partials 10418 10418
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:
|
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
Outdated
Show resolved
Hide resolved
|
not sure if this makes sense, but type breaks as it expects a throwable. |
DaanHoogland
left a comment
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
nvazquez
left a comment
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.
Thanks @daviftorres - just a comment from my review
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
Outdated
Show resolved
Hide resolved
|
@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 15819 |
|
sorry if I have been misleading you @daviftorres , but @nvazquez , any idea how to implement your requirement from #11981 (comment) in this asynchronous context? |
|
Hi @daviftorres I have pushed a fix to your branch/PR to address the comment. I have also reverted the 'Guest Network' text for just 'Network' as the VM starting may also be a system VM or a VR. cc. @DaanHoogland @blueorangutan package |
|
@nvazquez 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 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15820 |
|
@blueorangutan test |
|
@nvazquez a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-14861)
|
This trivial change provide more details for root administrators during troubleshooting. See discussion apache#11980
…ManagerImpl.java Co-authored-by: dahn <daan.hoogland@gmail.com>
nvazquez
left a comment
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
|
@blueorangutan package |
|
@RosiKyu a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16560 |
|
Sorry, verification failed: With the following:
// Before
throw new CloudRuntimeException("Network is unavailable. Please contact administrator", e).add(VirtualMachine.class, vmUuid);
// After
Account callingAccount = CallContext.current().getCallingAccount();
String errorSuffix = (callingAccount != null && callingAccount.getType() == Account.Type.ADMIN) ?
String.format("Failure: %s", e.getMessage()) :
"Please contact administrator.";
throw new CloudRuntimeException(String.format("The Network for VM %s is unavailable. %s", vmUuid, errorSuffix), e).add(VirtualMachine.class, vmUuid);
Scenario 1: Stop VR and Deploy VM
Result: Could not reproduce Scenario 2: Destroy VR at Hypervisor Level
Result: Code path not triggered Log analysis: The exception scope was Scenario 3: Simulate VR Version Mismatch via DB
Result: Could not reproduce Note: Global setting Observations
ConclusionStatus: Unable to reproduce the exact error path The specific code path modified by PR #11981 requires a
I could not trigger the exact condition where this error message would be displayed. |
RosiKyu
left a comment
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
Description
This trivial change provide more details for root administrators during troubleshooting.
See discussion #11980
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?