diff --git a/engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java b/engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java new file mode 100644 index 000000000000..41c90a0f73f9 --- /dev/null +++ b/engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java @@ -0,0 +1,45 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.alert; + +import com.cloud.dc.DataCenter; +import com.cloud.dc.Pod; +import com.cloud.host.Host; + +/** + * 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. + */ +public final class AlertFormatUtils { + + private AlertFormatUtils() { + } + + public static String describeHostLocation(Host host, DataCenter zone, Pod pod) { + if (host == null) { + // we should never get here, but if we do, at least we won't get an NPE + return String.format("No host to describe for availability zone: %s, pod: %s", + zone != null ? zone.getName() : "unknown", + pod != null ? pod.getName() : "unknown"); + } + 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"); + } +} diff --git a/engine/components-api/src/test/java/com/cloud/alert/AlertFormatUtilsTest.java b/engine/components-api/src/test/java/com/cloud/alert/AlertFormatUtilsTest.java new file mode 100644 index 000000000000..3e29fe62ca5f --- /dev/null +++ b/engine/components-api/src/test/java/com/cloud/alert/AlertFormatUtilsTest.java @@ -0,0 +1,94 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.alert; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.when; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.dc.DataCenter; +import com.cloud.dc.Pod; +import com.cloud.host.Host; + +@RunWith(MockitoJUnitRunner.class) +public class AlertFormatUtilsTest { + + @Mock + Host host; + @Mock + DataCenter zone; + @Mock + Pod pod; + + @Test + public void describeHostLocationIncludesNameIdUuidZoneAndPod() { + setUpHost(); + setUpZone(); + setUpPod(); + + String result = AlertFormatUtils.describeHostLocation(host, zone, pod); + + assertEquals("name: cs-kvm06 (id: 37, uuid: host-uuid), availability zone: Milton1, pod: Milton1-Pod1", result); + } + + @Test + public void describeHostLocationFallsBackToUnknownForNullZone() { + setUpHost(); + setUpPod(); + + String result = AlertFormatUtils.describeHostLocation(host, null, pod); + + assertEquals("name: cs-kvm06 (id: 37, uuid: host-uuid), availability zone: unknown, pod: Milton1-Pod1", result); + } + + @Test + public void describeHostLocationFallsBackToUnknownForNullPod() { + setUpHost(); + setUpZone(); + + String result = AlertFormatUtils.describeHostLocation(host, zone, null); + + assertEquals("name: cs-kvm06 (id: 37, uuid: host-uuid), availability zone: Milton1, pod: unknown", result); + } + + @Test + public void describeHostLocationFallsBackToUnknownForNullZoneAndPod() { + setUpHost(); + + String result = AlertFormatUtils.describeHostLocation(host, null, null); + + assertEquals("name: cs-kvm06 (id: 37, uuid: host-uuid), availability zone: unknown, pod: unknown", result); + } + + private void setUpHost() { + when(host.getName()).thenReturn("cs-kvm06"); + when(host.getId()).thenReturn(37L); + when(host.getUuid()).thenReturn("host-uuid"); + } + + private void setUpZone() { + when(zone.getName()).thenReturn("Milton1"); + } + + private void setUpPod() { + when(pod.getName()).thenReturn("Milton1-Pod1"); + } +} diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java index 1215829d92f8..1ea6439f3f32 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java @@ -91,6 +91,7 @@ import com.cloud.agent.api.UnsupportedAnswer; import com.cloud.agent.transport.Request; import com.cloud.agent.transport.Response; +import com.cloud.alert.AlertFormatUtils; import com.cloud.alert.AlertManager; import com.cloud.cluster.ManagementServerHostVO; import com.cloud.cluster.dao.ManagementServerHostDao; @@ -1151,7 +1152,7 @@ protected boolean handleDisconnectWithInvestigation(final AgentAttache attache, logger.debug(String.format("Skipping sending alert for %s as it already in %s state", host, host.getStatus())); } else if (!HOST_DOWN_ALERT_UNSUPPORTED_HOST_TYPES.contains(host.getType())) { - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Host down, " + host.getId(), message); + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Host down, " + host, message); } event = Status.Event.HostDown; } else if (determinedState == Status.Up) { @@ -1173,7 +1174,7 @@ protected boolean handleDisconnectWithInvestigation(final AgentAttache attache, } else if (currentStatus == Status.Up) { final DataCenterVO dcVO = _dcDao.findById(host.getDataCenterId()); final HostPodVO podVO = _podDao.findById(host.getPodId()); - final String hostDesc = "name: " + host.getName() + " (id:" + host.getUuid() + "), availability zone: " + dcVO.getName() + ", pod: " + podVO.getName(); + final String hostDesc = AlertFormatUtils.describeHostLocation(host, dcVO, podVO); if (host.getType() != Host.Type.SecondaryStorage && host.getType() != Host.Type.ConsoleProxy) { _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Host disconnected, " + hostDesc, "If the agent for host [" + hostDesc + "] is not restarted within " + AlertWait + " seconds, host will go to Alert state"); @@ -1184,12 +1185,11 @@ protected boolean handleDisconnectWithInvestigation(final AgentAttache attache, // if we end up here we are in alert state, send an alert final DataCenterVO dcVO = _dcDao.findById(host.getDataCenterId()); final HostPodVO podVO = _podDao.findById(host.getPodId()); - final String podName = podVO != null ? podVO.getName() : "NO POD"; - final String hostDesc = String.format("%s, availability zone: %s, pod: %s", host, dcVO, podName); + final String hostDesc = AlertFormatUtils.describeHostLocation(host, dcVO, podVO); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), String.format("Host in ALERT state, %s", hostDesc), - String.format("In availability zone %s, host is in alert state: %s", dcVO, host)); + String.format("Host is in alert state: %s", hostDesc)); } } else { logger.debug("The next status of agent {} is not Alert, no need to investigate what happened", host); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 84a397349cec..dc7d90edecba 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -82,6 +82,7 @@ import com.cloud.agent.api.routing.NetworkElementCommand; import com.cloud.agent.api.to.NicTO; import com.cloud.agent.api.to.deployasis.OVFNetworkTO; +import com.cloud.alert.AlertFormatUtils; import com.cloud.alert.AlertManager; import com.cloud.api.query.dao.DomainRouterJoinDao; import com.cloud.api.query.vo.DomainRouterJoinVO; @@ -4497,7 +4498,8 @@ public void processConnect(final Host host, final StartupCommand cmd, final bool if (!answer.getResult()) { logger.warn("Unable to setup agent {} due to {}", host, answer.getDetails()); - final String msg = "Incorrect Network setup on agent, Reinitialize agent after network names are setup, details : " + answer.getDetails(); + final String msg = "Incorrect Network setup on agent " + AlertFormatUtils.describeHostLocation(host, dc, null) + + ", Reinitialize agent after network names are setup, details : " + answer.getDetails(); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, dcId, host.getPodId(), msg, msg); throw new ConnectionException(true, msg); } else { diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java index aced750bd320..98cb5fc616c1 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java @@ -257,7 +257,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed); String subject = "Take snapshot failed for Instance: " + userVm.getDisplayName(); - String message = "Snapshot operation failed for Instance: " + userVm.getDisplayName() + ", Please check and delete if any stale volumes created with Instance Snapshot id: " + vmSnapshot.getVmId(); + String message = "Snapshot operation failed for Instance: " + userVm.getDisplayName() + ", Please check and delete if any stale volumes created with " + vmSnapshot; alertManager.sendAlert(AlertManager.AlertType.ALERT_TYPE_VM_SNAPSHOT, userVm.getDataCenterId(), userVm.getPodIdToDeployIn(), subject, message); } catch (NoTransitionException e1) { logger.error("Cannot set Instance Snapshot state due to: " + e1.getMessage()); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java index 26b39e30776f..c7cd30b4dde7 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java @@ -248,7 +248,7 @@ protected Void createTemplateAsyncCallback(AsyncCallbackDispatcher } for (Long domainId : filteredDomainIds) { if (domainId == null || !_domainDao.isChildDomain(account.getDomainId(), domainId)) { - throw new InvalidParameterValueException(String.format("Unable to create disk offering by another domain-admin: %s for domain: %s", user, _entityMgr.findById(Domain.class, domainId).getUuid())); + throw new InvalidParameterValueException(String.format("Unable to create disk offering by another domain-admin: %s for domain: %s", user, _entityMgr.findById(Domain.class, domainId))); } } } else if (account.getType() != Account.Type.ADMIN) { @@ -7740,7 +7740,7 @@ public NetworkOfferingVO createNetworkOffering(final String name, final String d // only one network offering in the system can be Required final List offerings = _networkOfferingDao.listByAvailability(Availability.Required, false); if (!offerings.isEmpty()) { - throw new InvalidParameterValueException("System already has network offering id=" + offerings.get(0).getId() + " with availability " + Availability.Required); + throw new InvalidParameterValueException("System already has network offering " + offerings.get(0) + " with availability " + Availability.Required); } } @@ -8056,7 +8056,7 @@ public Pair, Integer> searchForNetworkOfferings( throw new InvalidParameterValueException("Unable to find the domain by id=" + domainId); } if (!_domainDao.isChildDomain(caller.getDomainId(), domainId)) { - throw new InvalidParameterValueException(String.format("Unable to list network offerings for domain: %s as caller does not have access for it", domain.getUuid())); + throw new InvalidParameterValueException(String.format("Unable to list network offerings for domain: %s as caller does not have access for it", domain)); } } @@ -8920,7 +8920,7 @@ public NetworkOffering updateNetworkOffering(final UpdateNetworkOfferingCmd cmd) // only one network offering in the system can be Required final List offerings = _networkOfferingDao.listByAvailability(Availability.Required, false); if (!offerings.isEmpty() && offerings.get(0).getId() != offeringToUpdate.getId()) { - throw new InvalidParameterValueException("System already has network offering id=" + offerings.get(0).getId() + " with availability " + throw new InvalidParameterValueException("System already has network offering " + offerings.get(0) + " with availability " + Availability.Required); } } diff --git a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java index 755de00dec26..dadecdfe92f1 100644 --- a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java +++ b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java @@ -52,6 +52,7 @@ import org.apache.commons.collections.CollectionUtils; import com.cloud.agent.AgentManager; +import com.cloud.alert.AlertFormatUtils; import com.cloud.alert.AlertManager; import com.cloud.cluster.ClusterManagerListener; import com.cloud.consoleproxy.ConsoleProxyManager; @@ -374,7 +375,7 @@ public void scheduleRestartForVmsOnHost(final HostVO host, boolean investigate, } // send an email alert that the host is down, include VMs HostPodVO podVO = _podDao.findById(host.getPodId()); - String hostDesc = "name: " + host.getName() + " (id:" + host.getId() + "), availability zone: " + dcVO.getName() + ", pod: " + podVO.getName(); + String hostDesc = AlertFormatUtils.describeHostLocation(host, dcVO, podVO); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Host is down, " + hostDesc, "Host [" + hostDesc + "] is down." + ((sb != null) ? sb.toString() : "")); @@ -513,9 +514,17 @@ public void scheduleRestart(VMInstanceVO vm, boolean investigate, ReasonType rea } if (!(ForceHA.value() || vm.isHaEnabled())) { - String hostDesc = "id:" + vm.getHostId() + ", availability zone id:" + vm.getDataCenterId() + ", pod id:" + vm.getPodIdToDeployIn(); + 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; + } _alertMgr.sendAlert(alertType, vm.getDataCenterId(), vm.getPodIdToDeployIn(), "VM (name: " + vm.getHostName() + ", id: " + vm.getId() + - ") stopped unexpectedly on host " + hostDesc, "Virtual Machine " + vm.getHostName() + " (id: " + vm.getId() + ") running on host [" + vm.getHostId() + + ") stopped unexpectedly on host " + hostDesc, "Virtual Machine " + vm.getHostName() + " (id: " + vm.getId() + ") running on host [" + hostDesc + "] stopped unexpectedly."); if (logger.isDebugEnabled()) { diff --git a/server/src/main/java/com/cloud/ha/KVMFencer.java b/server/src/main/java/com/cloud/ha/KVMFencer.java index 4a6606b09cc3..11d39bccedf1 100644 --- a/server/src/main/java/com/cloud/ha/KVMFencer.java +++ b/server/src/main/java/com/cloud/ha/KVMFencer.java @@ -108,8 +108,8 @@ public Boolean fenceOff(VirtualMachine vm, Host host) { } _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), - "Unable to fence off host: " + host.getId(), - "Fencing off host " + host.getId() + " did not succeed after asking " + i + " hosts. " + + "Unable to fence off host: " + host, + "Fencing off host " + host + " did not succeed after asking " + i + " hosts. " + "Check Agent logs for more information."); logger.error("Unable to fence off {} on {}", vm.toString(), host.toString()); diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index bc3abd30d880..e6bc802a848c 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -1008,12 +1008,12 @@ public ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege if (Domain.ROOT_DOMAIN == domainId) { // no one can add limits on ROOT domain, disallow... - throw new PermissionDeniedException("Cannot update resource limit for ROOT domain " + domainId + ", permission denied"); + throw new PermissionDeniedException("Cannot update resource limit for ROOT domain " + (domain != null ? domain : "id " + domainId) + ", permission denied"); } if ((caller.getDomainId() == domainId) && caller.getType() == Account.Type.DOMAIN_ADMIN || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN) { // if the admin is trying to update their own domain, disallow... - throw new PermissionDeniedException("Unable to update resource limit for domain " + domainId + ", permission denied"); + throw new PermissionDeniedException("Unable to update resource limit for domain " + (domain != null ? domain : "id " + domainId) + ", permission denied"); } if (StringUtils.isNotEmpty(tag)) { long untaggedLimit = findCorrectResourceLimitForDomain(domain, resourceType, null); diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java index dc33a4442a33..96d6b4173fac 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -2082,7 +2082,7 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, return snapshot; } catch (ResourceAllocationException e) { if (snapshotType != Type.MANUAL) { - String msg = String.format("Snapshot resource limit exceeded for account id : %s. Failed to create recurring snapshots", owner.getId()); + String msg = String.format("Snapshot resource limit exceeded for account: %s. Failed to create recurring snapshots", owner); logger.warn(msg); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_UPDATE_RESOURCE_COUNT, 0L, 0L, msg, msg + ". Please, use updateResourceLimit to increase the limit"); } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index b3bc69835ff5..1c880a8c0a57 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2862,10 +2862,10 @@ private void updateVmStateForFailedVmCreation(Long vmId, Long hostId) { volumeMgr.destroyVolume(volume); } } - String subject = String.format("Failed to deploy Instance [ID: %s]", vm.getUuid()); + 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]", hostId) : "", + host != null ? String.format(" on host [%s]", host) : (hostId != null ? String.format(" on host [id: %s]", hostId) : ""), ThreadContext.get("logcontextid")); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_USERVM, vm.getDataCenterId(), vm.getPodIdToDeployIn(), subject, body); @@ -7760,15 +7760,23 @@ public void checkHostsDedication(VMInstanceVO vm, long srcHostId, long destHostI //if hosts are dedicated to different account/domains, raise an alert if (srcExplDedicated && destExplDedicated) { - if (!((accountOfDedicatedHost(srcHost) == null) || (accountOfDedicatedHost(srcHost).equals(accountOfDedicatedHost(destHost))))) { - String msg = String.format("VM is being migrated from host %s explicitly dedicated to account %d to host %s explicitly dedicated to account %d", - srcHost, accountOfDedicatedHost(srcHost), destHost, accountOfDedicatedHost(destHost)); + 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); } - if (!((domainOfDedicatedHost(srcHost) == null) || (domainOfDedicatedHost(srcHost).equals(domainOfDedicatedHost(destHost))))) { - String msg = String.format("VM is being migrated from host %s explicitly dedicated to domain %d to host %s explicitly dedicated to domain %d", - srcHost, domainOfDedicatedHost(srcHost), destHost, domainOfDedicatedHost(destHost)); + 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); logger.warn(msg); } @@ -7779,7 +7787,8 @@ public void checkHostsDedication(VMInstanceVO vm, long srcHostId, long destHostI if (deployPlanner.getDeploymentPlanner() != null && deployPlanner.getDeploymentPlanner().equals("ImplicitDedicationPlanner")) { //VM is deployed using implicit planner long accountOfVm = vm.getAccountId(); - String msg = String.format("VM of account %d with implicit deployment planner being migrated to host %s", accountOfVm, destHost); + Account accountOfVmObj = _accountDao.findById(accountOfVm); + String msg = String.format("VM of account %s with implicit deployment planner being migrated to host %s", accountOfVmObj, destHost); //Get all vms on destination host boolean emptyDestination = false; List vmsOnDest = getVmsOnHost(destHostId); @@ -7792,7 +7801,7 @@ public void checkHostsDedication(VMInstanceVO vm, long srcHostId, long destHostI if (!isServiceOfferingUsingPlannerInPreferredMode(vm.getServiceOfferingId())) { //Check if all vms on destination host are created using strict implicit mode if (!checkIfAllVmsCreatedInStrictMode(accountOfVm, vmsOnDest)) { - msg = String.format("Instance of Account %d with strict implicit deployment planner being migrated to host %s not having all Instances strict implicitly dedicated to Account %d", accountOfVm, destHost, accountOfVm); + 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); } } else { //If vm is deployed using preferred implicit planner, check if all vms on destination host must be @@ -7800,7 +7809,7 @@ public void checkHostsDedication(VMInstanceVO vm, long srcHostId, long destHostI for (VMInstanceVO vmsDest : vmsOnDest) { ServiceOfferingVO destPlanner = serviceOfferingDao.findById(vm.getId(), vmsDest.getServiceOfferingId()); if (!((destPlanner.getDeploymentPlanner() != null && destPlanner.getDeploymentPlanner().equals("ImplicitDedicationPlanner")) && vmsDest.getAccountId() == accountOfVm)) { - msg = String.format("Instance of Account %d with preferred implicit deployment planner being migrated to host %s not having all Instances implicitly dedicated to Account %d", accountOfVm, destHost, accountOfVm); + 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); } } } diff --git a/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java b/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java index 73ff79301fb7..a1e3a3cf6cab 100644 --- a/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java @@ -330,7 +330,7 @@ private boolean provisionKvmHostViaSsh(Host host, String caProvider) { return true; } catch (Exception e) { - logger.error("Error during forced SSH provisioning for KVM host " + host.getUuid(), e); + logger.error("Error during forced SSH provisioning for KVM host " + host, e); return false; } finally { if (sshConnection != null) { diff --git a/server/src/main/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProvider.java b/server/src/main/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProvider.java index 2d77e6f9d20c..c8dbf17d9a81 100644 --- a/server/src/main/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProvider.java +++ b/server/src/main/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProvider.java @@ -95,11 +95,11 @@ public void sendAlert(final Host host, final HAConfig.HAState nextState) { String subject = "HA operation performed for host"; String body = subject; if (HAConfig.HAState.Fencing.equals(nextState)) { - subject = String.format("HA Fencing of host id=%d, in dc id=%d performed", host.getId(), host.getDataCenterId()); - body = String.format("HA Fencing has been performed for host id=%d, uuid=%s in datacenter id=%d", host.getId(), host.getUuid(), host.getDataCenterId()); + subject = String.format("HA Fencing of host %s performed", host); + body = String.format("HA Fencing has been performed for host %s", host); } else if (HAConfig.HAState.Recovering.equals(nextState)) { - subject = String.format("HA Recovery of host id=%d, in dc id=%d performed", host.getId(), host.getDataCenterId()); - body = String.format("HA Recovery has been performed for host id=%d, uuid=%s in datacenter id=%d", host.getId(), host.getUuid(), host.getDataCenterId()); + subject = String.format("HA Recovery of host %s performed", host); + body = String.format("HA Recovery has been performed for host %s", host); } alertManager.sendAlert(AlertService.AlertType.ALERT_TYPE_HA_ACTION, host.getDataCenterId(), host.getPodId(), subject, body); } diff --git a/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java b/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java index d5013f71cb5a..c015229ad1c1 100644 --- a/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java @@ -260,7 +260,7 @@ private boolean isOutOfBandManagementEnabledForHost(Long hostId) { Host host = hostDao.findById(hostId); if (host == null || host.getResourceState() == ResourceState.Degraded) { String state = host != null ? String.valueOf(host.getResourceState()) : null; - logger.debug("Host [id={}, uuid={}, state={}] was removed or placed in Degraded state by the Admin.", hostId, host != null ? host.getUuid() : "", state); + logger.debug("Host [{}] was removed or placed in Degraded state (state={}) by the Admin.", host != null ? host : "id=" + hostId, state); return false; } diff --git a/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java b/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java index 626f2cda172f..9cdcfd9d6767 100644 --- a/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java +++ b/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java @@ -43,6 +43,7 @@ import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; @@ -270,6 +271,83 @@ public void scheduleRestartHostNotSupported() { highAvailabilityManager.scheduleRestart(vm, true); } + @Test + public void scheduleRestartVmStoppedUnexpectedlyResolvesHostLocation() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm.getDataCenterId()).thenReturn(1L); + Mockito.when(vm.getHostId()).thenReturn(5L); + Mockito.when(vm.getPodIdToDeployIn()).thenReturn(2L); + Mockito.when(vm.getHypervisorType()).thenReturn(HypervisorType.KVM); + Mockito.when(vm.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(vm.isHaEnabled()).thenReturn(false); + Mockito.when(vm.getId()).thenReturn(3L); + Mockito.when(vm.getHostName()).thenReturn("i-2-3-VM"); + Mockito.when(vm.getUuid()).thenReturn("vm-uuid"); + + ConfigKey haEnabled = Mockito.mock(ConfigKey.class); + highAvailabilityManager.VmHaEnabled = haEnabled; + Mockito.when(highAvailabilityManager.VmHaEnabled.valueIn(1L)).thenReturn(true); + + Mockito.when(hostVO.getId()).thenReturn(5L); + Mockito.when(hostVO.getName()).thenReturn("cs-kvm06"); + Mockito.when(hostVO.getUuid()).thenReturn("host-uuid"); + Mockito.when(hostVO.getDataCenterId()).thenReturn(1L); + Mockito.when(hostVO.getPodId()).thenReturn(2L); + Mockito.when(_hostDao.findById(5L)).thenReturn(hostVO); + + DataCenterVO dcVO = Mockito.mock(DataCenterVO.class); + Mockito.when(dcVO.getName()).thenReturn("Milton1"); + Mockito.when(_dcDao.findById(1L)).thenReturn(dcVO); + + HostPodVO podVO = Mockito.mock(HostPodVO.class); + Mockito.when(podVO.getName()).thenReturn("Milton1-Pod1"); + Mockito.when(_podDao.findById(2L)).thenReturn(podVO); + + Mockito.when(_instanceDao.findByUuid("vm-uuid")).thenReturn(vm); + Mockito.when(_haDao.findPreviousHA(3L)).thenReturn(new ArrayList<>()); + + highAvailabilityManager.scheduleRestart(vm, false); + + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + Mockito.verify(_alertMgr).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), Mockito.eq(1L), Mockito.eq(2L), + Mockito.anyString(), bodyCaptor.capture()); + assertTrue(bodyCaptor.getValue().contains("name: cs-kvm06")); + assertTrue(bodyCaptor.getValue().contains("id: 5")); + assertTrue(bodyCaptor.getValue().contains("uuid: host-uuid")); + assertTrue(bodyCaptor.getValue().contains("availability zone: Milton1")); + assertTrue(bodyCaptor.getValue().contains("pod: Milton1-Pod1")); + } + + @Test + public void scheduleRestartVmStoppedUnexpectedlyFallsBackWhenHostGone() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm.getDataCenterId()).thenReturn(1L); + Mockito.when(vm.getHostId()).thenReturn(5L); + Mockito.when(vm.getPodIdToDeployIn()).thenReturn(2L); + Mockito.when(vm.getHypervisorType()).thenReturn(HypervisorType.KVM); + Mockito.when(vm.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(vm.isHaEnabled()).thenReturn(false); + Mockito.when(vm.getId()).thenReturn(3L); + Mockito.when(vm.getHostName()).thenReturn("i-2-3-VM"); + Mockito.when(vm.getUuid()).thenReturn("vm-uuid"); + + ConfigKey haEnabled = Mockito.mock(ConfigKey.class); + highAvailabilityManager.VmHaEnabled = haEnabled; + Mockito.when(highAvailabilityManager.VmHaEnabled.valueIn(1L)).thenReturn(true); + + Mockito.when(_hostDao.findById(5L)).thenReturn(null); + + Mockito.when(_instanceDao.findByUuid("vm-uuid")).thenReturn(vm); + Mockito.when(_haDao.findPreviousHA(3L)).thenReturn(new ArrayList<>()); + + highAvailabilityManager.scheduleRestart(vm, false); + + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + Mockito.verify(_alertMgr).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), Mockito.eq(1L), Mockito.eq(2L), + Mockito.anyString(), bodyCaptor.capture()); + assertTrue(bodyCaptor.getValue().contains("host id: 5")); + } + @Test public void scheduleStop() { VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); diff --git a/server/src/test/java/com/cloud/ha/KVMFencerTest.java b/server/src/test/java/com/cloud/ha/KVMFencerTest.java index c4b5666c0206..74ba1cca488d 100644 --- a/server/src/test/java/com/cloud/ha/KVMFencerTest.java +++ b/server/src/test/java/com/cloud/ha/KVMFencerTest.java @@ -88,7 +88,6 @@ public void testWithSingleHostDown() { Mockito.when(host.getDataCenterId()).thenReturn(1l); Mockito.when(host.getPodId()).thenReturn(1l); Mockito.when(host.getStatus()).thenReturn(Status.Down); - Mockito.when(host.getId()).thenReturn(1l); VirtualMachine virtualMachine = Mockito.mock(VirtualMachine.class); Mockito.when(resourceManager.listAllHostsInCluster(1l)).thenReturn(Collections.singletonList(host)); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index f70a3abd5871..205e03ec80c7 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -102,6 +102,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockedConstruction; @@ -114,10 +115,14 @@ import com.cloud.api.query.dao.ServiceOfferingJoinDao; import com.cloud.api.query.vo.ServiceOfferingJoinVO; import com.cloud.configuration.Resource; +import com.cloud.alert.AlertManager; import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; +import com.cloud.dc.DedicatedResourceVO; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.DedicatedResourceDao; import com.cloud.deploy.DataCenterDeployment; +import com.cloud.deploy.dao.PlannerHostReservationDao; import com.cloud.deploy.DeployDestination; import com.cloud.deploy.DeploymentPlanner; import com.cloud.deploy.DeploymentPlanningManager; @@ -396,6 +401,15 @@ public class UserVmManagerImplTest { @Mock DomainDao domainDaoMock; + @Mock + DedicatedResourceDao dedicatedResourceDao; + + @Mock + AlertManager alertManager; + + @Mock + PlannerHostReservationDao plannerHostReservationDao; + @Mock DomainVO domainVoMock; @@ -4547,4 +4561,64 @@ public void verifyVmLimits_constrainedOffering_throwsException() { userVmManagerImpl.verifyVmLimits(userVmVoMock, customParameters)); Assert.assertTrue(ex.getMessage().startsWith("The CPU speed of this offering")); } + + @Test + public void checkHostsDedicationAlertsIncludeResolvedAccountAndDomainNames() { + long srcHostId = 10L; + long destHostId = 20L; + long vmId = 1L; + long serviceOfferingId = 2L; + + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + when(vm.getId()).thenReturn(vmId); + when(vm.getDataCenterId()).thenReturn(1L); + when(vm.getPodIdToDeployIn()).thenReturn(2L); + when(vm.getServiceOfferingId()).thenReturn(serviceOfferingId); + + HostVO srcHost = Mockito.mock(HostVO.class); + when(srcHost.getId()).thenReturn(srcHostId); + HostVO destHost = Mockito.mock(HostVO.class); + when(destHost.getId()).thenReturn(destHostId); + when(hostDao.findById(srcHostId)).thenReturn(srcHost); + when(hostDao.findById(destHostId)).thenReturn(destHost); + + DedicatedResourceVO srcDedication = Mockito.mock(DedicatedResourceVO.class); + when(srcDedication.getAccountId()).thenReturn(100L); + when(srcDedication.getDomainId()).thenReturn(200L); + DedicatedResourceVO destDedication = Mockito.mock(DedicatedResourceVO.class); + when(destDedication.getAccountId()).thenReturn(300L); + when(destDedication.getDomainId()).thenReturn(400L); + when(dedicatedResourceDao.findByHostId(srcHostId)).thenReturn(srcDedication); + when(dedicatedResourceDao.findByHostId(destHostId)).thenReturn(destDedication); + + 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}"); + when(accountDao.findById(100L)).thenReturn(srcAccount); + when(accountDao.findById(300L)).thenReturn(destAccount); + + 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}"); + when(domainDaoMock.findById(200L)).thenReturn(srcDomain); + when(domainDaoMock.findById(400L)).thenReturn(destDomain); + + ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class); + when(serviceOffering.getDeploymentPlanner()).thenReturn(null); + when(_serviceOfferingDao.findById(vmId, serviceOfferingId)).thenReturn(serviceOffering); + + when(plannerHostReservationDao.listAllDedicatedHosts()).thenReturn(new ArrayList<>()); + + userVmManagerImpl.checkHostsDedication(vm, srcHostId, destHostId); + + ArgumentCaptor subjectCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + Mockito.verify(alertManager, Mockito.times(2)).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), + Mockito.eq(1L), Mockito.eq(2L), subjectCaptor.capture(), bodyCaptor.capture()); + List messages = bodyCaptor.getAllValues(); + assertTrue(messages.stream().anyMatch(m -> m.contains("account-a") && m.contains("account-b"))); + assertTrue(messages.stream().anyMatch(m -> m.contains("domain-a") && m.contains("domain-b"))); + } }