alert messages restructure - #13775
Draft
DaanHoogland wants to merge 7 commits into
Draft
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #13775 +/- ##
============================================
+ Coverage 19.65% 19.66% +0.01%
- Complexity 19792 19814 +22
============================================
Files 6368 6369 +1
Lines 574881 574909 +28
Branches 70351 70362 +11
============================================
+ Hits 112970 113062 +92
+ Misses 449639 449552 -87
- Partials 12272 12295 +23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Restructures several alert/log messages to be more consistent and information-rich, and introduces a shared formatter for host location descriptions (fixing CLOUDSTACK-7297).
Changes:
- Added
AlertFormatUtils.describeHostLocation(...)and reused it in HA/agent/orchestration alert call sites. - Updated many alerts/logs to include object context (e.g.,
host,domain,account) rather than only IDs/UUIDs. - Tweaked multiple failure/permission messages to improve operator readability.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java | Updates host degraded/removed debug message formatting. |
| server/src/main/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProvider.java | Simplifies HA fencing/recovery alert subject/body to include host object. |
| server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java | Improves SSH provisioning error context by logging the host object. |
| server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | Reworks several VM/host dedication alert messages; adds account/domain lookups for richer output. |
| server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java | Updates snapshot limit alert to reference account object. |
| server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java | Updates permission-denied exception messages to include domain object. |
| server/src/main/java/com/cloud/ha/KVMFencer.java | Updates fencing failure alerts to include host object. |
| server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java | Uses AlertFormatUtils for host location in HA alerts; improves VM-stopped alert host description. |
| server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java | Makes several validation exceptions include full entity objects (domain/network offering). |
| server/src/main/java/com/cloud/alert/AlertManagerImpl.java | Changes alert logging to structured logging with entity objects. |
| plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java | Updates ScaleIO host/pool alerts/logs to include host object. |
| plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java | Updates ScaleIO disconnection checks/alerts to include host/pool objects. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java | Improves Ontap attach-storage error messages by using host object. |
| plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java | Improves Datera attach-storage alert message by using fetched host object. |
| engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java | Improves default host connect failure message by including host object. |
| engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java | Updates upload/register/copy failure alerts to include object string. |
| engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java | Clarifies VM snapshot failure message to include snapshot object context. |
| engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java | Uses AlertFormatUtils to include host location in agent network-setup failure alert. |
| engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java | Uses AlertFormatUtils for consistent host location strings in disconnect/alert-state messages. |
| engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java | New shared helper for consistent host/zone/pod alert formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Suppressed comments (7)
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:2869
- The conditional uses
hostId != nullbut printshost(object). IfhostIdis set but the host lookup failed, this will produceon host [null]and loses the only reliable identifier (the host id). Consider usinghost != nullfor the host-object rendering, and fall back tohostIdwhenhostis null so the alert remains actionable.
String subject = String.format("Failed to deploy Instance [%s]", vm);
String body = String.format("Failed to deploy [%s]%s. To troubleshoot, please check the logs with [logid:%s].",
vm,
hostId != null ? String.format(" on host [%s]", host) : "",
ThreadContext.get("logcontextid"));
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:7769
- These alerts now depend on
Account.toString()/Domain.toString()for human-readable names, and will rendernullif the DAO lookup returns null—where previously the IDs were always present. To keep alerts stable and actionable, consider formatting explicitly (e.g., account name + id, domain name + id) with a fallback to the raw IDs when the DAO lookup fails, instead of relying ontoString().
Long srcAccountId = accountOfDedicatedHost(srcHost);
Long destAccountId = accountOfDedicatedHost(destHost);
if (!((srcAccountId == null) || (srcAccountId.equals(destAccountId)))) {
Account srcAccount = _accountDao.findById(srcAccountId);
Account destAccount = _accountDao.findById(destAccountId);
String msg = String.format("VM is being migrated from host %s explicitly dedicated to account %s to host %s explicitly dedicated to account %s",
srcHost, srcAccount, destHost, destAccount);
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:7779
- These alerts now depend on
Account.toString()/Domain.toString()for human-readable names, and will rendernullif the DAO lookup returns null—where previously the IDs were always present. To keep alerts stable and actionable, consider formatting explicitly (e.g., account name + id, domain name + id) with a fallback to the raw IDs when the DAO lookup fails, instead of relying ontoString().
Long srcDomainId = domainOfDedicatedHost(srcHost);
Long destDomainId = domainOfDedicatedHost(destHost);
if (!((srcDomainId == null) || (srcDomainId.equals(destDomainId)))) {
Domain srcDomain = _domainDao.findById(srcDomainId);
Domain destDomain = _domainDao.findById(destDomainId);
String msg = String.format("VM is being migrated from host %s explicitly dedicated to domain %s to host %s explicitly dedicated to domain %s",
srcHost, srcDomain, destHost, destDomain);
server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java:4597
- This test hard-codes expectations around
toString()output forAccountVOandDomainVO, which is typically not a stable API and can change without functional impact. If the production intent is to include account/domain names, it would be more robust to have production code format using explicit getters (e.g., account name / domain name) and have the test stub those getters and assert on them.
AccountVO srcAccount = Mockito.mock(AccountVO.class);
when(srcAccount.toString()).thenReturn("Account {accountName=account-a}");
AccountVO destAccount = Mockito.mock(AccountVO.class);
when(destAccount.toString()).thenReturn("Account {accountName=account-b}");
server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java:4604
- This test hard-codes expectations around
toString()output forAccountVOandDomainVO, which is typically not a stable API and can change without functional impact. If the production intent is to include account/domain names, it would be more robust to have production code format using explicit getters (e.g., account name / domain name) and have the test stub those getters and assert on them.
DomainVO srcDomain = Mockito.mock(DomainVO.class);
when(srcDomain.toString()).thenReturn("Domain {name=domain-a}");
DomainVO destDomain = Mockito.mock(DomainVO.class);
when(destDomain.toString()).thenReturn("Domain {name=domain-b}");
plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java:301
- If
_hostDao.findById(hostId)returns null, the message will become... to host null, which is less actionable than the previous host id-based message. Consider falling back tohostIdwhenhostis null (or rendering both: host + id) to preserve diagnostic value.
HostVO host = _hostDao.findById(hostId);
String msg = String.format("Unable to attach storage pool %s to host %s", storagePool, host);
engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java:26
- The PR description is still the default template (type-of-change checkboxes and testing details are not filled in). Since repository automation relies on the description for labeling/documentation, please update the PR description to clearly state the functional behavior change and how it was tested.
* Shared formatting for the host/zone/pod description that recurs, independently
* hand-rolled and inconsistently worded (and occasionally mislabelled), across the
* HA and agent-management alert call sites. See CLOUDSTACK-7297.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.
Suppressed comments (8)
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:7780
- These alert messages rely on
Account/Domainobject%sformatting (i.e.,toString()), which is not guaranteed to include a stable or user-meaningful account/domain name and can degrade intonull(e.g., deleted entities or DAO returning null). Since the goal is to include resolved names, format explicitly using stable fields (e.g., account name + id; domain path/name + id) and include a fallback to the raw ID when the entity lookup returns null.
Long srcAccountId = accountOfDedicatedHost(srcHost);
Long destAccountId = accountOfDedicatedHost(destHost);
if (!((srcAccountId == null) || (srcAccountId.equals(destAccountId)))) {
Account srcAccount = _accountDao.findById(srcAccountId);
Account destAccount = destAccountId != null ? _accountDao.findById(destAccountId) : null;
String msg = String.format("VM is being migrated from host %s explicitly dedicated to account %s to host %s %s",
srcHost, srcAccount, destHost, destAccount != null ? "explicitly dedicated to account " + destAccount : "not dedicated to a specific account");
_alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_USERVM, vm.getDataCenterId(), vm.getPodIdToDeployIn(), msg, msg);
logger.warn(msg);
}
Long srcDomainId = domainOfDedicatedHost(srcHost);
Long destDomainId = domainOfDedicatedHost(destHost);
if (!((srcDomainId == null) || (srcDomainId.equals(destDomainId)))) {
Domain srcDomain = _domainDao.findById(srcDomainId);
Domain destDomain = destDomainId != null ? _domainDao.findById(destDomainId) : null;
String msg = String.format("VM is being migrated from host %s explicitly dedicated to domain %s to host %s %s",
srcHost, srcDomain, destHost, destDomain != null ? "explicitly dedicated to domain " + destDomain : "not dedicated to a specific domain");
_alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_USERVM, vm.getDataCenterId(), vm.getPodIdToDeployIn(), msg, msg);
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:7791
- Same issue as above: the alert text depends on
accountOfVmObj.toString()and can become unclear (null) or inconsistent iftoString()changes. Use a stable representation (account name/id) and fall back toaccountOfVm(the id) when the lookup does not return an account record.
Account accountOfVmObj = _accountDao.findById(accountOfVm);
String msg = String.format("VM of account %s with implicit deployment planner being migrated to host %s", accountOfVmObj, destHost);
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:7804
- Same issue as above: the alert text depends on
accountOfVmObj.toString()and can become unclear (null) or inconsistent iftoString()changes. Use a stable representation (account name/id) and fall back toaccountOfVm(the id) when the lookup does not return an account record.
msg = String.format("Instance of Account %s with strict implicit deployment planner being migrated to host %s not having all Instances strict implicitly dedicated to Account %s", accountOfVmObj, destHost, accountOfVmObj);
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:7812
- Same issue as above: the alert text depends on
accountOfVmObj.toString()and can become unclear (null) or inconsistent iftoString()changes. Use a stable representation (account name/id) and fall back toaccountOfVm(the id) when the lookup does not return an account record.
msg = String.format("Instance of Account %s with preferred implicit deployment planner being migrated to host %s not having all Instances implicitly dedicated to Account %s", accountOfVmObj, destHost, accountOfVmObj);
engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java:38
describeHostLocationwill throw aNullPointerExceptionifhostis null, but the method signature/documentation does not state a non-null contract. Since this is a shared utility in components-api, either (a) enforce non-null with an explicit check + clear exception message and document it in Javadoc, or (b) handle null by returning an 'unknown host' description (potentially via an overload that accepts a hostId for fallback formatting).
public static String describeHostLocation(Host host, DataCenter zone, Pod pod) {
return String.format("name: %s (id: %d, uuid: %s), availability zone: %s, pod: %s",
host.getName(), host.getId(), host.getUuid(),
zone != null ? zone.getName() : "unknown",
pod != null ? pod.getName() : "unknown");
}
server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java:525
- This introduces up to 3 DAO lookups per
scheduleRestartcall (host + zone + pod). In scenarios where many VMs trigger this path (e.g., host outage), this can become an N+1 query pattern. Consider restructuring so callers that already haveHostVO/DataCenterVO/HostPodVOpass them down, or add a small per-invocation cache (e.g., map by hostId) so repeated alerts during a batch reuse the same resolved location.
HostVO stoppedHost = hostId != null ? _hostDao.findById(hostId) : null;
String hostDesc;
if (stoppedHost != null) {
DataCenterVO stoppedHostDcVO = _dcDao.findById(stoppedHost.getDataCenterId());
HostPodVO stoppedHostPodVO = _podDao.findById(stoppedHost.getPodId());
hostDesc = AlertFormatUtils.describeHostLocation(stoppedHost, stoppedHostDcVO, stoppedHostPodVO);
} else {
hostDesc = "host id: " + hostId;
}
plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java:301
- If
_hostDao.findById(hostId)returns null (e.g., host deleted between operations), this produces an alert like '... to host null', which is less actionable than the previous id-based message. Prefer a fallback such ashost != null ? host : \"id=\" + hostIdto keep the alert informative even when the host record is missing.
HostVO host = _hostDao.findById(hostId);
String msg = String.format("Unable to attach storage pool %s to host %s", storagePool, host);
engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java:27
- The PR description is still the template placeholder (missing the detailed behavior change and testing notes), but this repository’s automation/documentation relies on the description fields. Please update the PR description to clearly state what alert message formats changed, why, and how it was tested (or which tests were added).
/**
* Shared formatting for the host/zone/pod description that recurs, independently
* hand-rolled and inconsistently worded (and occasionally mislabelled), across the
* HA and agent-management alert call sites. See CLOUDSTACK-7297.
*/
Contributor
Author
|
@blueorangutan package |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.
Suppressed comments (5)
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:7779
- Same as the account case: if a domain lookup fails, the alert body will include "domain null" and omit the domain id. Include the id as a fallback so operators can still act on the alert.
Domain srcDomain = _domainDao.findById(srcDomainId);
Domain destDomain = destDomainId != null ? _domainDao.findById(destDomainId) : null;
String msg = String.format("VM is being migrated from host %s explicitly dedicated to domain %s to host %s %s",
srcHost, srcDomain, destHost, destDomain != null ? "explicitly dedicated to domain " + destDomain : "not dedicated to a specific domain");
server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java:522
- Potential NPE: HostVO.getPodId() returns a nullable Long, but this calls _podDao.findById(stoppedHost.getPodId()) unconditionally. If the host has no pod, this will pass null into the DAO and can throw, preventing the alert from being sent.
DataCenterVO stoppedHostDcVO = _dcDao.findById(stoppedHost.getDataCenterId());
HostPodVO stoppedHostPodVO = _podDao.findById(stoppedHost.getPodId());
hostDesc = AlertFormatUtils.describeHostLocation(stoppedHost, stoppedHostDcVO, stoppedHostPodVO);
plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java:303
- If _hostDao.findById(hostId) returns null, the error/alert message will read "host null" and omit the hostId. Prefer using the resolved HostVO when present, but fall back to the numeric hostId when it isn’t.
if (!answer.getResult()) {
HostVO host = _hostDao.findById(hostId);
String msg = String.format("Unable to attach storage pool %s to host %s", storagePool, host);
_alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, storagePool.getDataCenterId(), storagePool.getPodId(), msg, msg);
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:7769
- If an account lookup fails (deleted/missing), the alert will currently contain "account null" and lose the account id entirely. Since the goal is actionable context, include the id as a fallback when the Account object can’t be resolved.
This issue also appears on line 7776 of the same file.
Account srcAccount = _accountDao.findById(srcAccountId);
Account destAccount = destAccountId != null ? _accountDao.findById(destAccountId) : null;
String msg = String.format("VM is being migrated from host %s explicitly dedicated to account %s to host %s %s",
srcHost, srcAccount, destHost, destAccount != null ? "explicitly dedicated to account " + destAccount : "not dedicated to a specific account");
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:166
- If hostDao.findById(hostId) returns null, the alert text becomes "host null" and loses the only actionable identifier (hostId). Keep the richer host.toString() when available, but fall back to the numeric hostId when it isn’t.
String msg = String.format("Unable to attach storage pool %s to the host %s", pool, host);
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
Suppressed comments (5)
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:7779
- If the dedicated domain ID is set but the domain record can't be resolved, the alert will currently include
null/ say the destination is not dedicated even thoughdestDomainIdis present. Prefer falling back to domain IDs when_domainDao.findById(...)returns null.
Domain srcDomain = _domainDao.findById(srcDomainId);
Domain destDomain = destDomainId != null ? _domainDao.findById(destDomainId) : null;
String msg = String.format("VM is being migrated from host %s explicitly dedicated to domain %s to host %s %s",
srcHost, srcDomain, destHost, destDomain != null ? "explicitly dedicated to domain " + destDomain : "not dedicated to a specific domain");
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:4649
- Here
domainIdcan be null (the condition explicitly checks for it), but the error message unconditionally calls_entityMgr.findById(Domain.class, domainId). Avoid passing null into the lookup and include a fallbackidin the message so it stays actionable.
throw new InvalidParameterValueException(String.format("Unable to create disk offering by another domain-admin: %s for domain: %s", user, _entityMgr.findById(Domain.class, domainId)));
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:7769
- If the dedicated account ID is set but the account record can't be resolved, the alert currently prints
null/ falls back to “not dedicated to a specific account”, which can be misleading. Prefer falling back to the account ID when_accountDao.findById(...)returns null.
This issue also appears on line 7776 of the same file.
Account srcAccount = _accountDao.findById(srcAccountId);
Account destAccount = destAccountId != null ? _accountDao.findById(destAccountId) : null;
String msg = String.format("VM is being migrated from host %s explicitly dedicated to account %s to host %s %s",
srcHost, srcAccount, destHost, destAccount != null ? "explicitly dedicated to account " + destAccount : "not dedicated to a specific account");
plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java:301
_hostDao.findById(hostId)can return null (e.g. host removed concurrently), which would currently produce an alert like... to host null. Consider falling back to the host ID when the lookup fails.
HostVO host = _hostDao.findById(hostId);
String msg = String.format("Unable to attach storage pool %s to host %s", storagePool, host);
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:3597
- This exception message may become
... for domain: nullif the Domain lookup fails. Consider including a fallback to the domain ID to keep the error actionable.
This issue also appears on line 4649 of the same file.
throw new InvalidParameterValueException(String.format("Unable to create service offering by another domain-admin: %s for domain: %s", user, _entityMgr.findById(Domain.class, domainId)));
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18753 |
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 24.52% |
| Branch coverage | 18.69% |
Grade Scale
| Grade | Line Coverage | Meaning |
|---|---|---|
| 🟢 A | ≥ 80% | Excellent - this code sleeps well at night 😴 |
| 🟡 B | 60-79% | Good - almost there, don't stop now 😉 |
| 🟠 C | 40-59% | Acceptable - your code is wearing a seatbelt, but no airbags 😬 |
| 🔴 D | 20-39% | Marginal - boldly shipping where no test has gone before 🖖 |
| ⛔ F | < 20% | Failing - tests? what tests? 🔥 |
Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
Suppressed comments (5)
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:7779
- Alert text can end up including "domain null" (or incorrectly describing the destination as not dedicated) if the domain record is missing; use the domain ID as a fallback in the message so the alert remains actionable.
Domain srcDomain = _domainDao.findById(srcDomainId);
Domain destDomain = destDomainId != null ? _domainDao.findById(destDomainId) : null;
String msg = String.format("VM is being migrated from host %s explicitly dedicated to domain %s to host %s %s",
srcHost, srcDomain, destHost, destDomain != null ? "explicitly dedicated to domain " + destDomain : "not dedicated to a specific domain");
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:7769
- Alert text can end up including "account null" (or incorrectly describing the destination as not dedicated) if the account record is missing; use the account ID as a fallback in the message so the alert remains actionable.
This issue also appears on line 7776 of the same file.
Account srcAccount = _accountDao.findById(srcAccountId);
Account destAccount = destAccountId != null ? _accountDao.findById(destAccountId) : null;
String msg = String.format("VM is being migrated from host %s explicitly dedicated to account %s to host %s %s",
srcHost, srcAccount, destHost, destAccount != null ? "explicitly dedicated to account " + destAccount : "not dedicated to a specific account");
server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java:525
- When vm.getHostId() is null, the fallback builds "host id: null" in the alert body/subject. Use a clearer fallback (e.g. "host: unknown") so the message isn't misleading.
} else {
hostDesc = "host id: " + hostId;
}
plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java:302
- If _hostDao.findById(hostId) returns null, this alert message will include "host null". Prefer falling back to the hostId so the alert remains actionable even when the host record is missing.
HostVO host = _hostDao.findById(hostId);
String msg = String.format("Unable to attach storage pool %s to host %s", storagePool, host);
server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java:289
- These tests overwrite the static HighAvailabilityManagerImpl.VmHaEnabled ConfigKey without restoring it, which can leak state into other tests when suites are run together/in parallel. Consider saving/restoring the original in a
@After(or using try/finally within the test) to keep tests isolated.
ConfigKey<Boolean> haEnabled = Mockito.mock(ConfigKey.class);
highAvailabilityManager.VmHaEnabled = haEnabled;
Mockito.when(highAvailabilityManager.VmHaEnabled.valueIn(1L)).thenReturn(true);
|
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 24.53% |
| Branch coverage | 18.70% |
Grade Scale
| Grade | Line Coverage | Meaning |
|---|---|---|
| 🟢 A | ≥ 80% | Excellent - this code sleeps well at night 😴 |
| 🟡 B | 60-79% | Good - almost there, don't stop now 😉 |
| 🟠 C | 40-59% | Acceptable - your code is wearing a seatbelt, but no airbags 😬 |
| 🔴 D | 20-39% | Marginal - boldly shipping where no test has gone before 🖖 |
| ⛔ F | < 20% | Failing - tests? what tests? 🔥 |
Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR...
Fixes: #7297
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?