From 7f132d5026f815e984ca61728c3e1e3cd908e1f2 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 3 Aug 2026 11:03:29 +0200 Subject: [PATCH 01/18] generic formatting facility for alert messages --- .../com/cloud/alert/AlertFormatUtils.java | 39 +++++++++++++++++++ .../cloud/agent/manager/AgentManagerImpl.java | 10 ++--- .../image/BaseImageStoreDriverImpl.java | 6 +-- .../provider/DefaultHostListener.java | 2 +- .../cloud/ha/HighAvailabilityManagerImpl.java | 15 +++++-- .../provider/host/HAAbstractHostProvider.java | 8 ++-- 6 files changed, 64 insertions(+), 16 deletions(-) create mode 100644 engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java 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..390e1a77097e --- /dev/null +++ b/engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java @@ -0,0 +1,39 @@ +// 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) { + 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/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/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 Date: Mon, 3 Aug 2026 11:05:38 +0200 Subject: [PATCH 02/18] adjust calls to format alert strings --- .../orchestration/NetworkOrchestrator.java | 4 ++- .../vmsnapshot/ScaleIOVMSnapshotStrategy.java | 2 +- .../provider/DateraHostListener.java | 3 +- .../storage/listener/OntapHostListener.java | 6 ++-- .../driver/ScaleIOPrimaryDataStoreDriver.java | 4 +-- .../provider/ScaleIOHostListener.java | 14 ++++----- .../com/cloud/alert/AlertManagerImpl.java | 2 +- .../ConfigurationManagerImpl.java | 10 +++--- .../src/main/java/com/cloud/ha/KVMFencer.java | 4 +-- .../ResourceLimitManagerImpl.java | 4 +-- .../storage/snapshot/SnapshotManagerImpl.java | 2 +- .../java/com/cloud/vm/UserVmManagerImpl.java | 31 ++++++++++++------- .../apache/cloudstack/ca/CAManagerImpl.java | 2 +- .../OutOfBandManagementServiceImpl.java | 2 +- 14 files changed, 51 insertions(+), 39 deletions(-) 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/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java b/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java index 08bc89737f26..5d8b4308918a 100644 --- a/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java +++ b/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java @@ -297,7 +297,8 @@ private void sendModifyStoragePoolCommand(ModifyStoragePoolCommand cmd, StorageP } if (!answer.getResult()) { - String msg = String.format("Unable to attach storage pool %s to host %d", storagePool, hostId); + 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); diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java index ecdd3efd2c5c..35d90a58f8be 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java @@ -95,7 +95,7 @@ public boolean hostConnect(long hostId, long poolId) { } if (!answer.getResult()) { - String msg = String.format("Unable to attach storage pool %s to host %d", pool, hostId); + String msg = String.format("Unable to attach storage pool %s to host %s", pool, host); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, pool.getDataCenterId(), pool.getPodId(), msg, msg); @@ -107,8 +107,8 @@ public boolean hostConnect(long hostId, long poolId) { if (!(answer instanceof ModifyStoragePoolAnswer)) { throw new CloudRuntimeException(String.format( - "Unexpected answer type %s returned for modify storage pool command for pool %s on host %d", - answer.getClass().getName(), pool, hostId)); + "Unexpected answer type %s returned for modify storage pool command for pool %s on host %s", + answer.getClass().getName(), pool, host)); } ModifyStoragePoolAnswer mspAnswer = (ModifyStoragePoolAnswer) answer; diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java index 14cb82a4c2b3..e419d0ddd7eb 100644 --- a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java +++ b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java @@ -1552,7 +1552,7 @@ public boolean canDisconnectHostFromStoragePool(Host host, StoragePool pool) { final ScaleIOGatewayClient client = getScaleIOClient(pool); return client.listVolumesMappedToSdc(sdcId).isEmpty(); } catch (Exception e) { - logger.warn("Unable to check whether the host: " + host.getId() + " can be disconnected from storage pool: " + pool.getId() + ", due to " + e.getMessage(), e); + logger.warn("Unable to check whether the host: " + host + " can be disconnected from storage pool: " + pool + ", due to " + e.getMessage(), e); return false; } } @@ -1564,7 +1564,7 @@ private void alertHostSdcDisconnection(Host host) { logger.warn("SDC not connected on the host: {}", host); String msg = String.format("SDC not connected on the host: %s, reconnect the SDC to MDM", host); - alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC disconnected on host: " + host.getUuid(), msg); + alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC disconnected on host: " + host, msg); } @Override diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java index f169b581b2ca..379b8c22f248 100644 --- a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java +++ b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java @@ -123,7 +123,7 @@ private String getSdcIdOfHost(HostVO host, DataStore dataStore) { if (MapUtils.isEmpty(poolDetails)) { String msg = String.format("PowerFlex storage SDC details not found on the host: %s, (re)install SDC and restart agent", host); logger.warn(msg); - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC details not found on host: " + host.getUuid(), msg); + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC details not found on host: " + host, msg); return null; } @@ -138,16 +138,16 @@ private String getSdcIdOfHost(HostVO host, DataStore dataStore) { if (StringUtils.isBlank(sdcId)) { String msg = String.format("Couldn't retrieve PowerFlex storage SDC details from the host: %s, add MDMs if On-demand connect disabled or try (re)install SDC & restart agent", host); logger.warn(msg); - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC details not found on host: " + host.getUuid(), msg); + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC details not found on host: " + host, msg); return null; } if (details.containsKey(ScaleIOSDCManager.ConnectOnDemand.key())) { String connectOnDemand = details.get(ScaleIOSDCManager.ConnectOnDemand.key()); if (connectOnDemand != null && !Boolean.parseBoolean(connectOnDemand) && !_sdcManager.isHostSdcConnected(sdcId, dataStore, 15)) { - logger.warn("SDC not connected on the host: " + hostId); - String msg = "SDC not connected on the host: " + hostId + ", reconnect the SDC to MDM and restart agent"; - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC not connected on host: " + host.getUuid(), msg); + logger.warn("SDC not connected on the host: " + host); + String msg = "SDC not connected on host: " + host + ", reconnect the SDC to MDM and restart agent"; + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC not connected on host: " + host, msg); return null; } } @@ -213,7 +213,7 @@ public boolean hostDisconnected(long hostId, long poolId) { ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(false, storagePool, storagePool.getPath(), details); ModifyStoragePoolAnswer answer = sendModifyStoragePoolCommand(cmd, storagePool, host); if (!answer.getResult()) { - logger.error("Failed to disconnect storage pool: " + storagePool + " and host: " + hostId); + logger.error("Failed to disconnect storage pool: " + storagePool + " and host: " + host); return false; } @@ -221,7 +221,7 @@ public boolean hostDisconnected(long hostId, long poolId) { if (storagePoolHost != null) { _storagePoolHostDao.deleteStoragePoolHostDetails(hostId, poolId); } - logger.info("Connection removed between storage pool: " + storagePool + " and host: " + hostId); + logger.info("Connection removed between storage pool: " + storagePool + " and host: " + host); return true; } diff --git a/server/src/main/java/com/cloud/alert/AlertManagerImpl.java b/server/src/main/java/com/cloud/alert/AlertManagerImpl.java index 27b445ba3769..6b67dbbddde8 100644 --- a/server/src/main/java/com/cloud/alert/AlertManagerImpl.java +++ b/server/src/main/java/com/cloud/alert/AlertManagerImpl.java @@ -815,7 +815,7 @@ public void sendAlert(AlertType alertType, DataCenter dataCenter, Pod pod, Clust Long clusterId = cluster == null ? null : cluster.getId(); Long podId = pod == null ? null : pod.getId(); long dcId = dataCenter == null ? 0L : dataCenter.getId(); - logger.warn(String.format("alertType=[%s] dataCenterId=[%s] podId=[%s] clusterId=[%s] message=[%s].", alertType, dcId, podId, clusterId, subject)); + logger.warn("alertType=[{}] dataCenter=[{}] pod=[{}] cluster=[{}] message=[{}].", alertType, dataCenter, pod, cluster, subject); AlertVO alert = getAlertForTrivialAlertType(alertType, dcId, podId, clusterId); if (alert == null) { diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 837254ed8b36..1644e1a0cbdc 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -3594,7 +3594,7 @@ protected ServiceOfferingVO createServiceOffering(final long userId, final boole } for (Long domainId : filteredDomainIds) { if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) { - throw new InvalidParameterValueException(String.format("Unable to create service offering by another domain-admin: %s for domain: %s", user, _entityMgr.findById(Domain.class, domainId).getUuid())); + throw new InvalidParameterValueException(String.format("Unable to create service offering by another domain-admin: %s for domain: %s", user, _entityMgr.findById(Domain.class, domainId))); } } } else if (account.getType() != Account.Type.ADMIN) { @@ -4646,7 +4646,7 @@ protected DiskOfferingVO createDiskOffering(final Long userId, final List } 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/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..23dbe2717cec 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 + ", 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 + ", 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..12e8a9faf75f 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) : "", + hostId != null ? String.format(" on host [%s]", host) : "", 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 = _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); _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 = _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); _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/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; } From 72d12d4dcaa1393c2863b9b43ec7200ddcf75731 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 3 Aug 2026 17:37:21 +0200 Subject: [PATCH 03/18] tests --- .../com/cloud/alert/AlertFormatUtilsTest.java | 94 +++++++++++++++++++ .../ha/HighAvailabilityManagerImplTest.java | 78 +++++++++++++++ .../com/cloud/vm/UserVmManagerImplTest.java | 74 +++++++++++++++ 3 files changed, 246 insertions(+) create mode 100644 engine/components-api/src/test/java/com/cloud/alert/AlertFormatUtilsTest.java 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/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/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"))); + } } From 665a1139c50fbc373d268b4270b0507f0a2260ba Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 3 Aug 2026 18:04:56 +0200 Subject: [PATCH 04/18] address copilots comments --- .../java/com/cloud/alert/AlertManagerImpl.java | 2 +- .../resourcelimit/ResourceLimitManagerImpl.java | 4 ++-- .../main/java/com/cloud/vm/UserVmManagerImpl.java | 14 +++++++------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/com/cloud/alert/AlertManagerImpl.java b/server/src/main/java/com/cloud/alert/AlertManagerImpl.java index 6b67dbbddde8..27b445ba3769 100644 --- a/server/src/main/java/com/cloud/alert/AlertManagerImpl.java +++ b/server/src/main/java/com/cloud/alert/AlertManagerImpl.java @@ -815,7 +815,7 @@ public void sendAlert(AlertType alertType, DataCenter dataCenter, Pod pod, Clust Long clusterId = cluster == null ? null : cluster.getId(); Long podId = pod == null ? null : pod.getId(); long dcId = dataCenter == null ? 0L : dataCenter.getId(); - logger.warn("alertType=[{}] dataCenter=[{}] pod=[{}] cluster=[{}] message=[{}].", alertType, dataCenter, pod, cluster, subject); + logger.warn(String.format("alertType=[%s] dataCenterId=[%s] podId=[%s] clusterId=[%s] message=[%s].", alertType, dcId, podId, clusterId, subject)); AlertVO alert = getAlertForTrivialAlertType(alertType, dcId, podId, clusterId); if (alert == null) { diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 23dbe2717cec..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 " + domain + ", 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 " + domain + ", 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/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 12e8a9faf75f..1c880a8c0a57 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2865,7 +2865,7 @@ private void updateVmStateForFailedVmCreation(Long vmId, Long hostId) { 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) : "", + 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); @@ -7764,9 +7764,9 @@ public void checkHostsDedication(VMInstanceVO vm, long srcHostId, long destHostI 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); + 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); } @@ -7774,9 +7774,9 @@ public void checkHostsDedication(VMInstanceVO vm, long srcHostId, long destHostI 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); + 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); } From 1b5305101a8da3b269ae9456252f0cf79634756f Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Tue, 4 Aug 2026 06:23:32 +0200 Subject: [PATCH 05/18] NPE guard --- .../src/main/java/com/cloud/alert/AlertFormatUtils.java | 6 ++++++ 1 file changed, 6 insertions(+) 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 index 390e1a77097e..41c90a0f73f9 100644 --- a/engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java +++ b/engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java @@ -31,6 +31,12 @@ 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", From 6dddc3f7b1fe604e20b673af363128a5861f8495 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Tue, 4 Aug 2026 07:50:21 +0200 Subject: [PATCH 06/18] removed unnecessary stub --- server/src/test/java/com/cloud/ha/KVMFencerTest.java | 1 - 1 file changed, 1 deletion(-) 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)); From 9de1303a0c8b9224bdb48c4717d621608b84395d Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Tue, 4 Aug 2026 11:31:09 +0200 Subject: [PATCH 07/18] relevant tests --- .../com/cloud/alert/AlertFormatUtilsTest.java | 17 +++++ .../ResourceLimitManagerImplTest.java | 41 ++++++++++++ .../com/cloud/vm/UserVmManagerImplTest.java | 63 +++++++++++++++++++ 3 files changed, 121 insertions(+) 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 index 3e29fe62ca5f..89dcf84797d0 100644 --- a/engine/components-api/src/test/java/com/cloud/alert/AlertFormatUtilsTest.java +++ b/engine/components-api/src/test/java/com/cloud/alert/AlertFormatUtilsTest.java @@ -78,6 +78,23 @@ public void describeHostLocationFallsBackToUnknownForNullZoneAndPod() { assertEquals("name: cs-kvm06 (id: 37, uuid: host-uuid), availability zone: unknown, pod: unknown", result); } + @Test + public void describeHostLocationDescribesZoneAndPodForNullHost() { + setUpZone(); + setUpPod(); + + String result = AlertFormatUtils.describeHostLocation(null, zone, pod); + + assertEquals("No host to describe for availability zone: Milton1, pod: Milton1-Pod1", result); + } + + @Test + public void describeHostLocationFallsBackToUnknownForNullHostAndNullZoneAndPod() { + String result = AlertFormatUtils.describeHostLocation(null, null, null); + + assertEquals("No host to describe for availability zone: unknown, pod: unknown", result); + } + private void setUpHost() { when(host.getName()).thenReturn("cs-kvm06"); when(host.getId()).thenReturn(37L); diff --git a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java index a4e0c953c0ee..e74011dd5168 100644 --- a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java +++ b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java @@ -66,6 +66,7 @@ import com.cloud.domain.dao.DomainDao; import com.cloud.event.ActionEventUtils; import com.cloud.event.EventTypes; +import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceAllocationException; import com.cloud.offering.DiskOffering; import com.cloud.offering.ServiceOffering; @@ -1331,6 +1332,46 @@ public void testUpdateResourceLimitForDomain() { } } + @Test + public void testUpdateResourceLimitForRootDomainThrowsPermissionDenied() { + Long domainId = Domain.ROOT_DOMAIN; + + Domain domain = mock(Domain.class); + when(domain.toString()).thenReturn("Domain {name=ROOT}"); + when(entityManager.findById(Domain.class, domainId)).thenReturn(domain); + + PermissionDeniedException ex = Assert.assertThrows(PermissionDeniedException.class, + () -> resourceLimitManager.updateResourceLimit(null, domainId, 8, 20L, null)); + + Assert.assertTrue(ex.getMessage().contains("Domain {name=ROOT}")); + Mockito.verify(resourceLimitDao, Mockito.never()).update(Mockito.anyLong(), Mockito.anyLong()); + } + + @Test + public void testUpdateResourceLimitForOwnDomainByDomainAdminThrowsPermissionDenied() { + Long domainId = 2L; + + Domain domain = mock(Domain.class); + when(domain.toString()).thenReturn("Domain {name=domain-a}"); + when(entityManager.findById(Domain.class, domainId)).thenReturn(domain); + + Account domainAdminAccount = mock(Account.class); + when(domainAdminAccount.getType()).thenReturn(Account.Type.DOMAIN_ADMIN); + when(domainAdminAccount.getDomainId()).thenReturn(domainId); + User user = mock(User.class); + CallContext.unregister(); + CallContext.register(user, domainAdminAccount); + + try { + PermissionDeniedException ex = Assert.assertThrows(PermissionDeniedException.class, + () -> resourceLimitManager.updateResourceLimit(null, domainId, 8, 20L, null)); + Assert.assertTrue(ex.getMessage().contains("Domain {name=domain-a}")); + } finally { + CallContext.unregister(); + } + Mockito.verify(resourceLimitDao, Mockito.never()).update(Mockito.anyLong(), Mockito.anyLong()); + } + @Test public void consolidatedResourceLimitsForAllResourceTypesWithAccountId() { Long accountId = 1L; diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 205e03ec80c7..49c61e1baf8f 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -4621,4 +4621,67 @@ public void checkHostsDedicationAlertsIncludeResolvedAccountAndDomainNames() { 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"))); } + + private UserVmVO mockStoppedVmForFailedCreation(Long vmId) { + UserVmVO vm = Mockito.mock(UserVmVO.class); + when(vm.getState()).thenReturn(VirtualMachine.State.Stopped); + when(vm.getId()).thenReturn(vmId); + when(vm.getDataCenterId()).thenReturn(1L); + when(vm.getPodIdToDeployIn()).thenReturn(2L); + when(vm.getAccountId()).thenReturn(10L); + when(vm.isDisplayVm()).thenReturn(true); + when(vm.getServiceOfferingId()).thenReturn(20L); + when(vm.getTemplateId()).thenReturn(30L); + when(vm.toString()).thenReturn("VM {id=" + vmId + ", name=i-2-3-VM}"); + when(userVmDao.findById(vmId)).thenReturn(vm); + when(volumeDaoMock.findUsableVolumesForInstance(vmId)).thenReturn(new ArrayList<>()); + return vm; + } + + @Test + public void updateVmStateForFailedVmCreationIncludesResolvedHostInAlert() { + Long vmId = 3L; + Long hostId = 5L; + mockStoppedVmForFailedCreation(vmId); + + HostVO host = Mockito.mock(HostVO.class); + when(host.toString()).thenReturn("Host {id=5, name=cs-kvm06}"); + when(hostDao.findById(hostId)).thenReturn(host); + + ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", vmId, hostId); + + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), + Mockito.eq(1L), Mockito.eq(2L), Mockito.anyString(), bodyCaptor.capture()); + assertTrue(bodyCaptor.getValue().contains("on host [Host {id=5, name=cs-kvm06}]")); + } + + @Test + public void updateVmStateForFailedVmCreationFallsBackToHostIdWhenHostNotFound() { + Long vmId = 3L; + Long hostId = 5L; + mockStoppedVmForFailedCreation(vmId); + + when(hostDao.findById(hostId)).thenReturn(null); + + ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", vmId, hostId); + + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), + Mockito.eq(1L), Mockito.eq(2L), Mockito.anyString(), bodyCaptor.capture()); + assertTrue(bodyCaptor.getValue().contains("on host [id: 5]")); + } + + @Test + public void updateVmStateForFailedVmCreationOmitsHostSegmentWhenHostIdIsNull() { + Long vmId = 3L; + mockStoppedVmForFailedCreation(vmId); + + ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", vmId, (Long) null); + + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), + Mockito.eq(1L), Mockito.eq(2L), Mockito.anyString(), bodyCaptor.capture()); + assertFalse(bodyCaptor.getValue().contains("on host")); + } } From 42ab4f79c805aac9691f00fe9eb82c02153fed25 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Wed, 5 Aug 2026 10:48:18 +0200 Subject: [PATCH 08/18] tests --- .../ResourceLimitManagerImplTest.java | 36 +++++ .../com/cloud/vm/UserVmManagerImplTest.java | 100 +++++++++++++ .../host/HAAbstractHostProviderTest.java | 132 ++++++++++++++++++ 3 files changed, 268 insertions(+) create mode 100644 server/src/test/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProviderTest.java diff --git a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java index e74011dd5168..2b3418dafc0c 100644 --- a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java +++ b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java @@ -1372,6 +1372,42 @@ public void testUpdateResourceLimitForOwnDomainByDomainAdminThrowsPermissionDeni Mockito.verify(resourceLimitDao, Mockito.never()).update(Mockito.anyLong(), Mockito.anyLong()); } + @Test + public void testUpdateResourceLimitForRootDomainFallsBackToIdWhenDomainNotFound() { + Long domainId = Domain.ROOT_DOMAIN; + + when(entityManager.findById(Domain.class, domainId)).thenReturn(null); + + PermissionDeniedException ex = Assert.assertThrows(PermissionDeniedException.class, + () -> resourceLimitManager.updateResourceLimit(null, domainId, 8, 20L, null)); + + Assert.assertTrue(ex.getMessage().contains("id " + domainId)); + Mockito.verify(resourceLimitDao, Mockito.never()).update(Mockito.anyLong(), Mockito.anyLong()); + } + + @Test + public void testUpdateResourceLimitForOwnDomainByDomainAdminFallsBackToIdWhenDomainNotFound() { + Long domainId = 2L; + + when(entityManager.findById(Domain.class, domainId)).thenReturn(null); + + Account domainAdminAccount = mock(Account.class); + when(domainAdminAccount.getType()).thenReturn(Account.Type.DOMAIN_ADMIN); + when(domainAdminAccount.getDomainId()).thenReturn(domainId); + User user = mock(User.class); + CallContext.unregister(); + CallContext.register(user, domainAdminAccount); + + try { + PermissionDeniedException ex = Assert.assertThrows(PermissionDeniedException.class, + () -> resourceLimitManager.updateResourceLimit(null, domainId, 8, 20L, null)); + Assert.assertTrue(ex.getMessage().contains("id " + domainId)); + } finally { + CallContext.unregister(); + } + Mockito.verify(resourceLimitDao, Mockito.never()).update(Mockito.anyLong(), Mockito.anyLong()); + } + @Test public void consolidatedResourceLimitsForAllResourceTypesWithAccountId() { Long accountId = 1L; diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 49c61e1baf8f..b7279742270c 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -4622,6 +4622,106 @@ public void checkHostsDedicationAlertsIncludeResolvedAccountAndDomainNames() { assertTrue(messages.stream().anyMatch(m -> m.contains("domain-a") && m.contains("domain-b"))); } + @Test + public void checkHostsDedicationAlertNotesDestinationNotDedicatedToSpecificAccount() { + 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); + + // src host is dedicated to an account; dest host is dedicated to a whole domain (no account), so + // destAccountId resolves to null even though destHost is explicitly dedicated. + DedicatedResourceVO srcDedication = Mockito.mock(DedicatedResourceVO.class); + when(srcDedication.getAccountId()).thenReturn(100L); + when(srcDedication.getDomainId()).thenReturn((Long) null); + DedicatedResourceVO destDedication = Mockito.mock(DedicatedResourceVO.class); + when(destDedication.getAccountId()).thenReturn((Long) null); + 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}"); + when(accountDao.findById(100L)).thenReturn(srcAccount); + + 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 bodyCaptor = ArgumentCaptor.forClass(String.class); + Mockito.verify(alertManager, Mockito.times(1)).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), + Mockito.eq(1L), Mockito.eq(2L), Mockito.anyString(), bodyCaptor.capture()); + assertTrue(bodyCaptor.getValue().contains("account-a")); + assertTrue(bodyCaptor.getValue().contains("not dedicated to a specific account")); + } + + @Test + public void checkHostsDedicationAlertNotesDestinationNotDedicatedToSpecificDomain() { + 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); + + // src host is dedicated to a whole domain (no account); dest host is dedicated to an account, so + // destDomainId resolves to null even though destHost is explicitly dedicated. + DedicatedResourceVO srcDedication = Mockito.mock(DedicatedResourceVO.class); + when(srcDedication.getAccountId()).thenReturn((Long) null); + when(srcDedication.getDomainId()).thenReturn(200L); + DedicatedResourceVO destDedication = Mockito.mock(DedicatedResourceVO.class); + when(destDedication.getAccountId()).thenReturn(300L); + when(destDedication.getDomainId()).thenReturn((Long) null); + when(dedicatedResourceDao.findByHostId(srcHostId)).thenReturn(srcDedication); + when(dedicatedResourceDao.findByHostId(destHostId)).thenReturn(destDedication); + + DomainVO srcDomain = Mockito.mock(DomainVO.class); + when(srcDomain.toString()).thenReturn("Domain {name=domain-a}"); + when(domainDaoMock.findById(200L)).thenReturn(srcDomain); + + 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 bodyCaptor = ArgumentCaptor.forClass(String.class); + Mockito.verify(alertManager, Mockito.times(1)).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), + Mockito.eq(1L), Mockito.eq(2L), Mockito.anyString(), bodyCaptor.capture()); + assertTrue(bodyCaptor.getValue().contains("domain-a")); + assertTrue(bodyCaptor.getValue().contains("not dedicated to a specific domain")); + } + private UserVmVO mockStoppedVmForFailedCreation(Long vmId) { UserVmVO vm = Mockito.mock(UserVmVO.class); when(vm.getState()).thenReturn(VirtualMachine.State.Stopped); diff --git a/server/src/test/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProviderTest.java b/server/src/test/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProviderTest.java new file mode 100644 index 000000000000..4b4979a52435 --- /dev/null +++ b/server/src/test/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProviderTest.java @@ -0,0 +1,132 @@ +// 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 org.apache.cloudstack.ha.provider.host; + +import static org.junit.Assert.assertTrue; + +import org.apache.cloudstack.alert.AlertService; +import org.apache.cloudstack.ha.HAConfig; +import org.apache.cloudstack.ha.provider.HACheckerException; +import org.apache.cloudstack.ha.provider.HAFenceException; +import org.apache.cloudstack.ha.provider.HARecoveryException; +import org.joda.time.DateTime; +import org.junit.Before; +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; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.alert.AlertManager; +import com.cloud.host.Host; +import com.cloud.host.HostVO; + +@RunWith(MockitoJUnitRunner.class) +public class HAAbstractHostProviderTest { + + @Mock + AlertManager alertManager; + + @Mock + HostVO host; + + private HAAbstractHostProvider provider; + + private static final class TestHAHostProvider extends HAAbstractHostProvider { + @Override + public boolean isEligible(Host r) { + return true; + } + + @Override + public boolean isHealthy(Host r) throws HACheckerException { + return true; + } + + @Override + public boolean hasActivity(Host r, DateTime afterThis) throws HACheckerException { + return true; + } + + @Override + public boolean recover(Host r) throws HARecoveryException { + return true; + } + + @Override + public boolean fence(Host r) throws HAFenceException { + return true; + } + + @Override + public Object getConfigValue(HAProviderConfig name, Host r) { + return null; + } + } + + @Before + public void setup() { + provider = new TestHAHostProvider(); + ReflectionTestUtils.setField(provider, "alertManager", alertManager); + + Mockito.when(host.getDataCenterId()).thenReturn(1L); + Mockito.when(host.getPodId()).thenReturn(2L); + Mockito.when(host.toString()).thenReturn("Host {id=5, name=cs-kvm06}"); + } + + @Test + public void sendAlertForFencingStateDescribesHostAndOperation() { + provider.sendAlert(host, HAConfig.HAState.Fencing); + + ArgumentCaptor subjectCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertService.AlertType.ALERT_TYPE_HA_ACTION), Mockito.eq(1L), Mockito.eq(2L), + subjectCaptor.capture(), bodyCaptor.capture()); + assertTrue(subjectCaptor.getValue().contains("HA Fencing")); + assertTrue(subjectCaptor.getValue().contains("Host {id=5, name=cs-kvm06}")); + assertTrue(bodyCaptor.getValue().contains("HA Fencing has been performed")); + assertTrue(bodyCaptor.getValue().contains("Host {id=5, name=cs-kvm06}")); + } + + @Test + public void sendAlertForRecoveringStateDescribesHostAndOperation() { + provider.sendAlert(host, HAConfig.HAState.Recovering); + + ArgumentCaptor subjectCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertService.AlertType.ALERT_TYPE_HA_ACTION), Mockito.eq(1L), Mockito.eq(2L), + subjectCaptor.capture(), bodyCaptor.capture()); + assertTrue(subjectCaptor.getValue().contains("HA Recovery")); + assertTrue(subjectCaptor.getValue().contains("Host {id=5, name=cs-kvm06}")); + assertTrue(bodyCaptor.getValue().contains("HA Recovery has been performed")); + assertTrue(bodyCaptor.getValue().contains("Host {id=5, name=cs-kvm06}")); + } + + @Test + public void sendAlertForOtherStatesUsesGenericSubjectAndBody() { + provider.sendAlert(host, HAConfig.HAState.Available); + + ArgumentCaptor subjectCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertService.AlertType.ALERT_TYPE_HA_ACTION), Mockito.eq(1L), Mockito.eq(2L), + subjectCaptor.capture(), bodyCaptor.capture()); + assertTrue(subjectCaptor.getValue().equals("HA operation performed for host")); + assertTrue(bodyCaptor.getValue().equals("HA operation performed for host")); + } +} From 1c696145033df31f21ae6df380c3c4a5b23bcc9d Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 14 Aug 2026 11:11:31 +0200 Subject: [PATCH 09/18] deal with dedup warning --- .../main/java/com/cloud/alert/AlertFormatUtils.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) 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 index 41c90a0f73f9..c3b8e7527445 100644 --- a/engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java +++ b/engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java @@ -27,6 +27,8 @@ */ public final class AlertFormatUtils { + private static final String UNKNOWN = "unknown"; + private AlertFormatUtils() { } @@ -34,12 +36,12 @@ 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"); + 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"); + zone != null ? zone.getName() : UNKNOWN, + pod != null ? pod.getName() : UNKNOWN); } } From f2805b5496393567022de9c027276f7eea6d1a82 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 14 Aug 2026 12:10:54 +0200 Subject: [PATCH 10/18] deal with shadowing vars warning --- .../com/cloud/vm/UserVmManagerImplTest.java | 42 ++++++++----------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index b7279742270c..7922a58dd2b6 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -4566,14 +4566,13 @@ public void verifyVmLimits_constrainedOffering_throwsException() { public void checkHostsDedicationAlertsIncludeResolvedAccountAndDomainNames() { long srcHostId = 10L; long destHostId = 20L; - long vmId = 1L; - long serviceOfferingId = 2L; + long testServiceOfferingId = 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); + when(vm.getServiceOfferingId()).thenReturn(testServiceOfferingId); HostVO srcHost = Mockito.mock(HostVO.class); when(srcHost.getId()).thenReturn(srcHostId); @@ -4605,9 +4604,8 @@ public void checkHostsDedicationAlertsIncludeResolvedAccountAndDomainNames() { 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(_serviceOfferingDao.findById(vmId, testServiceOfferingId)).thenReturn(serviceOffering); when(plannerHostReservationDao.listAllDedicatedHosts()).thenReturn(new ArrayList<>()); @@ -4626,14 +4624,13 @@ public void checkHostsDedicationAlertsIncludeResolvedAccountAndDomainNames() { public void checkHostsDedicationAlertNotesDestinationNotDedicatedToSpecificAccount() { long srcHostId = 10L; long destHostId = 20L; - long vmId = 1L; - long serviceOfferingId = 2L; + long testServiceOfferingId = 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); + when(vm.getServiceOfferingId()).thenReturn(testServiceOfferingId); HostVO srcHost = Mockito.mock(HostVO.class); when(srcHost.getId()).thenReturn(srcHostId); @@ -4657,9 +4654,8 @@ public void checkHostsDedicationAlertNotesDestinationNotDedicatedToSpecificAccou when(srcAccount.toString()).thenReturn("Account {accountName=account-a}"); when(accountDao.findById(100L)).thenReturn(srcAccount); - ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class); when(serviceOffering.getDeploymentPlanner()).thenReturn(null); - when(_serviceOfferingDao.findById(vmId, serviceOfferingId)).thenReturn(serviceOffering); + when(_serviceOfferingDao.findById(vmId, testServiceOfferingId)).thenReturn(serviceOffering); when(plannerHostReservationDao.listAllDedicatedHosts()).thenReturn(new ArrayList<>()); @@ -4676,14 +4672,13 @@ public void checkHostsDedicationAlertNotesDestinationNotDedicatedToSpecificAccou public void checkHostsDedicationAlertNotesDestinationNotDedicatedToSpecificDomain() { long srcHostId = 10L; long destHostId = 20L; - long vmId = 1L; - long serviceOfferingId = 2L; + long testServiceOfferingId = 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); + when(vm.getServiceOfferingId()).thenReturn(testServiceOfferingId); HostVO srcHost = Mockito.mock(HostVO.class); when(srcHost.getId()).thenReturn(srcHostId); @@ -4707,9 +4702,8 @@ public void checkHostsDedicationAlertNotesDestinationNotDedicatedToSpecificDomai when(srcDomain.toString()).thenReturn("Domain {name=domain-a}"); when(domainDaoMock.findById(200L)).thenReturn(srcDomain); - ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class); when(serviceOffering.getDeploymentPlanner()).thenReturn(null); - when(_serviceOfferingDao.findById(vmId, serviceOfferingId)).thenReturn(serviceOffering); + when(_serviceOfferingDao.findById(vmId, testServiceOfferingId)).thenReturn(serviceOffering); when(plannerHostReservationDao.listAllDedicatedHosts()).thenReturn(new ArrayList<>()); @@ -4740,15 +4734,15 @@ private UserVmVO mockStoppedVmForFailedCreation(Long vmId) { @Test public void updateVmStateForFailedVmCreationIncludesResolvedHostInAlert() { - Long vmId = 3L; + Long testVmId = 3L; Long hostId = 5L; - mockStoppedVmForFailedCreation(vmId); + mockStoppedVmForFailedCreation(testVmId); HostVO host = Mockito.mock(HostVO.class); when(host.toString()).thenReturn("Host {id=5, name=cs-kvm06}"); when(hostDao.findById(hostId)).thenReturn(host); - ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", vmId, hostId); + ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", testVmId, hostId); ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), @@ -4758,13 +4752,13 @@ public void updateVmStateForFailedVmCreationIncludesResolvedHostInAlert() { @Test public void updateVmStateForFailedVmCreationFallsBackToHostIdWhenHostNotFound() { - Long vmId = 3L; + Long testVmId = 3L; Long hostId = 5L; - mockStoppedVmForFailedCreation(vmId); + mockStoppedVmForFailedCreation(testVmId); when(hostDao.findById(hostId)).thenReturn(null); - ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", vmId, hostId); + ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", testVmId, hostId); ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), @@ -4774,10 +4768,10 @@ public void updateVmStateForFailedVmCreationFallsBackToHostIdWhenHostNotFound() @Test public void updateVmStateForFailedVmCreationOmitsHostSegmentWhenHostIdIsNull() { - Long vmId = 3L; - mockStoppedVmForFailedCreation(vmId); + Long testVmId = 3L; + mockStoppedVmForFailedCreation(testVmId); - ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", vmId, (Long) null); + ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", testVmId, (Long) null); ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), From 58bfd737c59d204116a71eee545492c18fa7aa3c Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 14 Aug 2026 12:47:40 +0200 Subject: [PATCH 11/18] deal with static use warning --- .../main/java/com/cloud/agent/manager/AgentManagerImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 1ea6439f3f32..41d3d0df455a 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 @@ -44,6 +44,7 @@ import com.cloud.utils.StringUtils; import org.apache.cloudstack.agent.lb.IndirectAgentLB; +import org.apache.cloudstack.alert.AlertService; import org.apache.cloudstack.ca.CAManager; import org.apache.cloudstack.command.ReconcileCommandService; import org.apache.cloudstack.command.ReconcileCommandUtils; @@ -1152,7 +1153,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, message); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Host down, " + host, message); } event = Status.Event.HostDown; } else if (determinedState == Status.Up) { From a7d8aa1afb8f8534b6e01ede0b68c956de853815 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 14 Aug 2026 12:57:41 +0200 Subject: [PATCH 12/18] call static type static --- .../java/com/cloud/agent/manager/AgentManagerImpl.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 41d3d0df455a..8eca848ac17d 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 @@ -1177,7 +1177,7 @@ protected boolean handleDisconnectWithInvestigation(final AgentAttache attache, final HostPodVO podVO = _podDao.findById(host.getPodId()); 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, + _alertMgr.sendAlert(AlertService.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"); } event = Status.Event.AgentDisconnected; @@ -1187,7 +1187,7 @@ protected boolean handleDisconnectWithInvestigation(final AgentAttache attache, final DataCenterVO dcVO = _dcDao.findById(host.getDataCenterId()); final HostPodVO podVO = _podDao.findById(host.getPodId()); final String hostDesc = AlertFormatUtils.describeHostLocation(host, dcVO, podVO); - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), String.format("Host in ALERT state, %s", hostDesc), String.format("Host is in alert state: %s", hostDesc)); @@ -1705,10 +1705,10 @@ protected void processRequest(final Link link, final Request request) { final HostPodVO podVO = _podDao.findById(host.getPodId()); final String hostDesc = String.format("%s, availability zone: %s, pod: %s", host, dcVO, podVO); - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_ROUTING, host.getDataCenterId(), host.getPodId(), "Host lost connection to gateway, " + hostDesc, + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_ROUTING, host.getDataCenterId(), host.getPodId(), "Host lost connection to gateway, " + hostDesc, "Host [" + hostDesc + "] lost connection to gateway (default route) and is possibly having network connection issues."); } else { - _alertMgr.clearAlert(AlertManager.AlertType.ALERT_TYPE_ROUTING, host.getDataCenterId(), host.getPodId()); + _alertMgr.clearAlert(AlertService.AlertType.ALERT_TYPE_ROUTING, host.getDataCenterId(), host.getPodId()); } } else { logger.debug("Not processing {} for agent id={}; can't find the host in the DB", PingRoutingCommand.class.getSimpleName(), cmdHostId); @@ -2007,7 +2007,7 @@ protected void runInContext() { final DataCenterVO dcVO = _dcDao.findById(host.getDataCenterId()); final HostPodVO podVO = _podDao.findById(host.getPodId()); final String hostDesc = "name: " + host.getName() + " (id:" + host.getId() + "), availability zone: " + dcVO.getName() + ", pod: " + podVO.getName(); - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Migration Complete for host " + hostDesc, + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Migration Complete for host " + hostDesc, "Host [" + hostDesc + "] is ready for maintenance"); } } From 383b8c6428c015377a2abf30ed0a2ef5a6266a96 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 14 Aug 2026 14:36:46 +0200 Subject: [PATCH 13/18] sonarqube warning nested ternary operator --- .../src/main/java/com/cloud/vm/UserVmManagerImpl.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 1c880a8c0a57..575b8c2d74b2 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2863,9 +2863,17 @@ private void updateVmStateForFailedVmCreation(Long vmId, Long hostId) { } } String subject = String.format("Failed to deploy Instance [%s]", vm); + String hostDesc; + if (host != null) { + hostDesc = String.format(" on host [%s]", host); + } else if (hostId != null) { + hostDesc = String.format(" on host [id: %s]", hostId); + } else { + hostDesc = ""; + } String body = String.format("Failed to deploy [%s]%s. To troubleshoot, please check the logs with [logid:%s].", vm, - host != null ? String.format(" on host [%s]", host) : (hostId != null ? String.format(" on host [id: %s]", hostId) : ""), + hostDesc, ThreadContext.get("logcontextid")); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_USERVM, vm.getDataCenterId(), vm.getPodIdToDeployIn(), subject, body); From 9a787ba78aaced5c647e4edbb91e4944c4372f64 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 14 Aug 2026 14:37:24 +0200 Subject: [PATCH 14/18] sonarqube warning uniused var --- .../storage/datastore/provider/ScaleIOHostListener.java | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java index 379b8c22f248..7f3b70d271e4 100644 --- a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java +++ b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java @@ -97,7 +97,6 @@ public boolean hostConnect(long hostId, long poolId) { private String getSdcIdOfHost(HostVO host, DataStore dataStore) { StoragePool storagePool = (StoragePool) dataStore; - long hostId = host.getId(); long poolId = storagePool.getId(); String systemId = null; StoragePoolDetailVO systemIdDetail = _storagePoolDetailsDao.findDetail(poolId, ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID); From d93a6ecc954d3cb7c3d45c552d03cd4b5897c9db Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 14 Aug 2026 14:38:02 +0200 Subject: [PATCH 15/18] sonarqube warning wrong assertions used --- .../ha/provider/host/HAAbstractHostProviderTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProviderTest.java b/server/src/test/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProviderTest.java index 4b4979a52435..ab282709a741 100644 --- a/server/src/test/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProviderTest.java +++ b/server/src/test/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProviderTest.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.ha.provider.host; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import org.apache.cloudstack.alert.AlertService; @@ -126,7 +127,7 @@ public void sendAlertForOtherStatesUsesGenericSubjectAndBody() { ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertService.AlertType.ALERT_TYPE_HA_ACTION), Mockito.eq(1L), Mockito.eq(2L), subjectCaptor.capture(), bodyCaptor.capture()); - assertTrue(subjectCaptor.getValue().equals("HA operation performed for host")); - assertTrue(bodyCaptor.getValue().equals("HA operation performed for host")); + assertEquals("HA operation performed for host", subjectCaptor.getValue()); + assertEquals("HA operation performed for host", bodyCaptor.getValue()); } } From a93c48dc71ad909a4de824661ee250101ca6d693 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 14 Aug 2026 15:55:19 +0200 Subject: [PATCH 16/18] sonarqube warning use static imports for mock and verification methods --- .../ha/HighAvailabilityManagerImplTest.java | 93 ++++++++++--------- .../ResourceLimitManagerImplTest.java | 10 +- .../com/cloud/vm/UserVmManagerImplTest.java | 58 ++++++------ .../host/HAAbstractHostProviderTest.java | 14 +-- 4 files changed, 91 insertions(+), 84 deletions(-) diff --git a/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java b/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java index 9cdcfd9d6767..4f1c5de08bac 100644 --- a/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java +++ b/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java @@ -20,6 +20,9 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import java.lang.reflect.Array; import java.lang.reflect.Field; @@ -273,43 +276,43 @@ public void scheduleRestartHostNotSupported() { @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); + VMInstanceVO vm = mock(VMInstanceVO.class); + when(vm.getDataCenterId()).thenReturn(1L); + when(vm.getHostId()).thenReturn(5L); + when(vm.getPodIdToDeployIn()).thenReturn(2L); + when(vm.getHypervisorType()).thenReturn(HypervisorType.KVM); + when(vm.getType()).thenReturn(VirtualMachine.Type.User); + when(vm.isHaEnabled()).thenReturn(false); + when(vm.getId()).thenReturn(3L); + when(vm.getHostName()).thenReturn("i-2-3-VM"); + when(vm.getUuid()).thenReturn("vm-uuid"); + + ConfigKey haEnabled = mock(ConfigKey.class); highAvailabilityManager.VmHaEnabled = haEnabled; - Mockito.when(highAvailabilityManager.VmHaEnabled.valueIn(1L)).thenReturn(true); + 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); + when(hostVO.getId()).thenReturn(5L); + when(hostVO.getName()).thenReturn("cs-kvm06"); + when(hostVO.getUuid()).thenReturn("host-uuid"); + when(hostVO.getDataCenterId()).thenReturn(1L); + when(hostVO.getPodId()).thenReturn(2L); + 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); + DataCenterVO dcVO = mock(DataCenterVO.class); + when(dcVO.getName()).thenReturn("Milton1"); + 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); + HostPodVO podVO = mock(HostPodVO.class); + when(podVO.getName()).thenReturn("Milton1-Pod1"); + when(_podDao.findById(2L)).thenReturn(podVO); - Mockito.when(_instanceDao.findByUuid("vm-uuid")).thenReturn(vm); - Mockito.when(_haDao.findPreviousHA(3L)).thenReturn(new ArrayList<>()); + when(_instanceDao.findByUuid("vm-uuid")).thenReturn(vm); + 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), + 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")); @@ -320,30 +323,30 @@ public void scheduleRestartVmStoppedUnexpectedlyResolvesHostLocation() { @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); + VMInstanceVO vm = mock(VMInstanceVO.class); + when(vm.getDataCenterId()).thenReturn(1L); + when(vm.getHostId()).thenReturn(5L); + when(vm.getPodIdToDeployIn()).thenReturn(2L); + when(vm.getHypervisorType()).thenReturn(HypervisorType.KVM); + when(vm.getType()).thenReturn(VirtualMachine.Type.User); + when(vm.isHaEnabled()).thenReturn(false); + when(vm.getId()).thenReturn(3L); + when(vm.getHostName()).thenReturn("i-2-3-VM"); + when(vm.getUuid()).thenReturn("vm-uuid"); + + ConfigKey haEnabled = mock(ConfigKey.class); highAvailabilityManager.VmHaEnabled = haEnabled; - Mockito.when(highAvailabilityManager.VmHaEnabled.valueIn(1L)).thenReturn(true); + when(highAvailabilityManager.VmHaEnabled.valueIn(1L)).thenReturn(true); - Mockito.when(_hostDao.findById(5L)).thenReturn(null); + when(_hostDao.findById(5L)).thenReturn(null); - Mockito.when(_instanceDao.findByUuid("vm-uuid")).thenReturn(vm); - Mockito.when(_haDao.findPreviousHA(3L)).thenReturn(new ArrayList<>()); + when(_instanceDao.findByUuid("vm-uuid")).thenReturn(vm); + 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), + 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")); } diff --git a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java index 2b3418dafc0c..166756f0c0c9 100644 --- a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java +++ b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java @@ -17,6 +17,8 @@ package com.cloud.resourcelimit; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.lang.reflect.Field; @@ -1344,7 +1346,7 @@ public void testUpdateResourceLimitForRootDomainThrowsPermissionDenied() { () -> resourceLimitManager.updateResourceLimit(null, domainId, 8, 20L, null)); Assert.assertTrue(ex.getMessage().contains("Domain {name=ROOT}")); - Mockito.verify(resourceLimitDao, Mockito.never()).update(Mockito.anyLong(), Mockito.anyLong()); + verify(resourceLimitDao, never()).update(Mockito.anyLong(), Mockito.anyLong()); } @Test @@ -1369,7 +1371,7 @@ public void testUpdateResourceLimitForOwnDomainByDomainAdminThrowsPermissionDeni } finally { CallContext.unregister(); } - Mockito.verify(resourceLimitDao, Mockito.never()).update(Mockito.anyLong(), Mockito.anyLong()); + verify(resourceLimitDao, never()).update(Mockito.anyLong(), Mockito.anyLong()); } @Test @@ -1382,7 +1384,7 @@ public void testUpdateResourceLimitForRootDomainFallsBackToIdWhenDomainNotFound( () -> resourceLimitManager.updateResourceLimit(null, domainId, 8, 20L, null)); Assert.assertTrue(ex.getMessage().contains("id " + domainId)); - Mockito.verify(resourceLimitDao, Mockito.never()).update(Mockito.anyLong(), Mockito.anyLong()); + verify(resourceLimitDao, never()).update(Mockito.anyLong(), Mockito.anyLong()); } @Test @@ -1405,7 +1407,7 @@ public void testUpdateResourceLimitForOwnDomainByDomainAdminFallsBackToIdWhenDom } finally { CallContext.unregister(); } - Mockito.verify(resourceLimitDao, Mockito.never()).update(Mockito.anyLong(), Mockito.anyLong()); + verify(resourceLimitDao, never()).update(Mockito.anyLong(), Mockito.anyLong()); } @Test diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 7922a58dd2b6..1420773ade57 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -4568,38 +4568,38 @@ public void checkHostsDedicationAlertsIncludeResolvedAccountAndDomainNames() { long destHostId = 20L; long testServiceOfferingId = 2L; - VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + VMInstanceVO vm = mock(VMInstanceVO.class); when(vm.getId()).thenReturn(vmId); when(vm.getDataCenterId()).thenReturn(1L); when(vm.getPodIdToDeployIn()).thenReturn(2L); when(vm.getServiceOfferingId()).thenReturn(testServiceOfferingId); - HostVO srcHost = Mockito.mock(HostVO.class); + HostVO srcHost = mock(HostVO.class); when(srcHost.getId()).thenReturn(srcHostId); - HostVO destHost = Mockito.mock(HostVO.class); + HostVO destHost = 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); + DedicatedResourceVO srcDedication = mock(DedicatedResourceVO.class); when(srcDedication.getAccountId()).thenReturn(100L); when(srcDedication.getDomainId()).thenReturn(200L); - DedicatedResourceVO destDedication = Mockito.mock(DedicatedResourceVO.class); + DedicatedResourceVO destDedication = 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); + AccountVO srcAccount = mock(AccountVO.class); when(srcAccount.toString()).thenReturn("Account {accountName=account-a}"); - AccountVO destAccount = Mockito.mock(AccountVO.class); + AccountVO destAccount = 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); + DomainVO srcDomain = mock(DomainVO.class); when(srcDomain.toString()).thenReturn("Domain {name=domain-a}"); - DomainVO destDomain = Mockito.mock(DomainVO.class); + DomainVO destDomain = mock(DomainVO.class); when(destDomain.toString()).thenReturn("Domain {name=domain-b}"); when(domainDaoMock.findById(200L)).thenReturn(srcDomain); when(domainDaoMock.findById(400L)).thenReturn(destDomain); @@ -4613,7 +4613,7 @@ public void checkHostsDedicationAlertsIncludeResolvedAccountAndDomainNames() { 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), + verify(alertManager, 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"))); @@ -4626,31 +4626,31 @@ public void checkHostsDedicationAlertNotesDestinationNotDedicatedToSpecificAccou long destHostId = 20L; long testServiceOfferingId = 2L; - VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + VMInstanceVO vm = mock(VMInstanceVO.class); when(vm.getId()).thenReturn(vmId); when(vm.getDataCenterId()).thenReturn(1L); when(vm.getPodIdToDeployIn()).thenReturn(2L); when(vm.getServiceOfferingId()).thenReturn(testServiceOfferingId); - HostVO srcHost = Mockito.mock(HostVO.class); + HostVO srcHost = mock(HostVO.class); when(srcHost.getId()).thenReturn(srcHostId); - HostVO destHost = Mockito.mock(HostVO.class); + HostVO destHost = mock(HostVO.class); when(destHost.getId()).thenReturn(destHostId); when(hostDao.findById(srcHostId)).thenReturn(srcHost); when(hostDao.findById(destHostId)).thenReturn(destHost); // src host is dedicated to an account; dest host is dedicated to a whole domain (no account), so // destAccountId resolves to null even though destHost is explicitly dedicated. - DedicatedResourceVO srcDedication = Mockito.mock(DedicatedResourceVO.class); + DedicatedResourceVO srcDedication = mock(DedicatedResourceVO.class); when(srcDedication.getAccountId()).thenReturn(100L); when(srcDedication.getDomainId()).thenReturn((Long) null); - DedicatedResourceVO destDedication = Mockito.mock(DedicatedResourceVO.class); + DedicatedResourceVO destDedication = mock(DedicatedResourceVO.class); when(destDedication.getAccountId()).thenReturn((Long) null); when(destDedication.getDomainId()).thenReturn(400L); when(dedicatedResourceDao.findByHostId(srcHostId)).thenReturn(srcDedication); when(dedicatedResourceDao.findByHostId(destHostId)).thenReturn(destDedication); - AccountVO srcAccount = Mockito.mock(AccountVO.class); + AccountVO srcAccount = mock(AccountVO.class); when(srcAccount.toString()).thenReturn("Account {accountName=account-a}"); when(accountDao.findById(100L)).thenReturn(srcAccount); @@ -4662,7 +4662,7 @@ public void checkHostsDedicationAlertNotesDestinationNotDedicatedToSpecificAccou userVmManagerImpl.checkHostsDedication(vm, srcHostId, destHostId); ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); - Mockito.verify(alertManager, Mockito.times(1)).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), + verify(alertManager, times(1)).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), Mockito.eq(1L), Mockito.eq(2L), Mockito.anyString(), bodyCaptor.capture()); assertTrue(bodyCaptor.getValue().contains("account-a")); assertTrue(bodyCaptor.getValue().contains("not dedicated to a specific account")); @@ -4674,31 +4674,31 @@ public void checkHostsDedicationAlertNotesDestinationNotDedicatedToSpecificDomai long destHostId = 20L; long testServiceOfferingId = 2L; - VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + VMInstanceVO vm = mock(VMInstanceVO.class); when(vm.getId()).thenReturn(vmId); when(vm.getDataCenterId()).thenReturn(1L); when(vm.getPodIdToDeployIn()).thenReturn(2L); when(vm.getServiceOfferingId()).thenReturn(testServiceOfferingId); - HostVO srcHost = Mockito.mock(HostVO.class); + HostVO srcHost = mock(HostVO.class); when(srcHost.getId()).thenReturn(srcHostId); - HostVO destHost = Mockito.mock(HostVO.class); + HostVO destHost = mock(HostVO.class); when(destHost.getId()).thenReturn(destHostId); when(hostDao.findById(srcHostId)).thenReturn(srcHost); when(hostDao.findById(destHostId)).thenReturn(destHost); // src host is dedicated to a whole domain (no account); dest host is dedicated to an account, so // destDomainId resolves to null even though destHost is explicitly dedicated. - DedicatedResourceVO srcDedication = Mockito.mock(DedicatedResourceVO.class); + DedicatedResourceVO srcDedication = mock(DedicatedResourceVO.class); when(srcDedication.getAccountId()).thenReturn((Long) null); when(srcDedication.getDomainId()).thenReturn(200L); - DedicatedResourceVO destDedication = Mockito.mock(DedicatedResourceVO.class); + DedicatedResourceVO destDedication = mock(DedicatedResourceVO.class); when(destDedication.getAccountId()).thenReturn(300L); when(destDedication.getDomainId()).thenReturn((Long) null); when(dedicatedResourceDao.findByHostId(srcHostId)).thenReturn(srcDedication); when(dedicatedResourceDao.findByHostId(destHostId)).thenReturn(destDedication); - DomainVO srcDomain = Mockito.mock(DomainVO.class); + DomainVO srcDomain = mock(DomainVO.class); when(srcDomain.toString()).thenReturn("Domain {name=domain-a}"); when(domainDaoMock.findById(200L)).thenReturn(srcDomain); @@ -4710,14 +4710,14 @@ public void checkHostsDedicationAlertNotesDestinationNotDedicatedToSpecificDomai userVmManagerImpl.checkHostsDedication(vm, srcHostId, destHostId); ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); - Mockito.verify(alertManager, Mockito.times(1)).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), + verify(alertManager, times(1)).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), Mockito.eq(1L), Mockito.eq(2L), Mockito.anyString(), bodyCaptor.capture()); assertTrue(bodyCaptor.getValue().contains("domain-a")); assertTrue(bodyCaptor.getValue().contains("not dedicated to a specific domain")); } private UserVmVO mockStoppedVmForFailedCreation(Long vmId) { - UserVmVO vm = Mockito.mock(UserVmVO.class); + UserVmVO vm = mock(UserVmVO.class); when(vm.getState()).thenReturn(VirtualMachine.State.Stopped); when(vm.getId()).thenReturn(vmId); when(vm.getDataCenterId()).thenReturn(1L); @@ -4738,14 +4738,14 @@ public void updateVmStateForFailedVmCreationIncludesResolvedHostInAlert() { Long hostId = 5L; mockStoppedVmForFailedCreation(testVmId); - HostVO host = Mockito.mock(HostVO.class); + HostVO host = mock(HostVO.class); when(host.toString()).thenReturn("Host {id=5, name=cs-kvm06}"); when(hostDao.findById(hostId)).thenReturn(host); ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", testVmId, hostId); ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); - Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), + verify(alertManager).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), Mockito.eq(1L), Mockito.eq(2L), Mockito.anyString(), bodyCaptor.capture()); assertTrue(bodyCaptor.getValue().contains("on host [Host {id=5, name=cs-kvm06}]")); } @@ -4761,7 +4761,7 @@ public void updateVmStateForFailedVmCreationFallsBackToHostIdWhenHostNotFound() ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", testVmId, hostId); ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); - Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), + verify(alertManager).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), Mockito.eq(1L), Mockito.eq(2L), Mockito.anyString(), bodyCaptor.capture()); assertTrue(bodyCaptor.getValue().contains("on host [id: 5]")); } @@ -4774,7 +4774,7 @@ public void updateVmStateForFailedVmCreationOmitsHostSegmentWhenHostIdIsNull() { ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", testVmId, (Long) null); ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); - Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), + verify(alertManager).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), Mockito.eq(1L), Mockito.eq(2L), Mockito.anyString(), bodyCaptor.capture()); assertFalse(bodyCaptor.getValue().contains("on host")); } diff --git a/server/src/test/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProviderTest.java b/server/src/test/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProviderTest.java index ab282709a741..779dd7584b36 100644 --- a/server/src/test/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProviderTest.java +++ b/server/src/test/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProviderTest.java @@ -18,6 +18,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import org.apache.cloudstack.alert.AlertService; import org.apache.cloudstack.ha.HAConfig; @@ -86,9 +88,9 @@ public void setup() { provider = new TestHAHostProvider(); ReflectionTestUtils.setField(provider, "alertManager", alertManager); - Mockito.when(host.getDataCenterId()).thenReturn(1L); - Mockito.when(host.getPodId()).thenReturn(2L); - Mockito.when(host.toString()).thenReturn("Host {id=5, name=cs-kvm06}"); + when(host.getDataCenterId()).thenReturn(1L); + when(host.getPodId()).thenReturn(2L); + when(host.toString()).thenReturn("Host {id=5, name=cs-kvm06}"); } @Test @@ -97,7 +99,7 @@ public void sendAlertForFencingStateDescribesHostAndOperation() { ArgumentCaptor subjectCaptor = ArgumentCaptor.forClass(String.class); ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); - Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertService.AlertType.ALERT_TYPE_HA_ACTION), Mockito.eq(1L), Mockito.eq(2L), + verify(alertManager).sendAlert(Mockito.eq(AlertService.AlertType.ALERT_TYPE_HA_ACTION), Mockito.eq(1L), Mockito.eq(2L), subjectCaptor.capture(), bodyCaptor.capture()); assertTrue(subjectCaptor.getValue().contains("HA Fencing")); assertTrue(subjectCaptor.getValue().contains("Host {id=5, name=cs-kvm06}")); @@ -111,7 +113,7 @@ public void sendAlertForRecoveringStateDescribesHostAndOperation() { ArgumentCaptor subjectCaptor = ArgumentCaptor.forClass(String.class); ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); - Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertService.AlertType.ALERT_TYPE_HA_ACTION), Mockito.eq(1L), Mockito.eq(2L), + verify(alertManager).sendAlert(Mockito.eq(AlertService.AlertType.ALERT_TYPE_HA_ACTION), Mockito.eq(1L), Mockito.eq(2L), subjectCaptor.capture(), bodyCaptor.capture()); assertTrue(subjectCaptor.getValue().contains("HA Recovery")); assertTrue(subjectCaptor.getValue().contains("Host {id=5, name=cs-kvm06}")); @@ -125,7 +127,7 @@ public void sendAlertForOtherStatesUsesGenericSubjectAndBody() { ArgumentCaptor subjectCaptor = ArgumentCaptor.forClass(String.class); ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); - Mockito.verify(alertManager).sendAlert(Mockito.eq(AlertService.AlertType.ALERT_TYPE_HA_ACTION), Mockito.eq(1L), Mockito.eq(2L), + verify(alertManager).sendAlert(Mockito.eq(AlertService.AlertType.ALERT_TYPE_HA_ACTION), Mockito.eq(1L), Mockito.eq(2L), subjectCaptor.capture(), bodyCaptor.capture()); assertEquals("HA operation performed for host", subjectCaptor.getValue()); assertEquals("HA operation performed for host", bodyCaptor.getValue()); From 4f7e0e7a51da895311b2a01cd2c146bacb8709ee Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Sat, 15 Aug 2026 13:33:19 +0200 Subject: [PATCH 17/18] deal with sonarqube warnings --- .../datastore/provider/ScaleIOHostListener.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java index 7f3b70d271e4..1ff44fe2486f 100644 --- a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java +++ b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java @@ -23,6 +23,7 @@ import javax.inject.Inject; +import org.apache.cloudstack.alert.AlertService; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener; @@ -122,7 +123,7 @@ private String getSdcIdOfHost(HostVO host, DataStore dataStore) { if (MapUtils.isEmpty(poolDetails)) { String msg = String.format("PowerFlex storage SDC details not found on the host: %s, (re)install SDC and restart agent", host); logger.warn(msg); - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC details not found on host: " + host, msg); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC details not found on host: " + host, msg); return null; } @@ -137,16 +138,16 @@ private String getSdcIdOfHost(HostVO host, DataStore dataStore) { if (StringUtils.isBlank(sdcId)) { String msg = String.format("Couldn't retrieve PowerFlex storage SDC details from the host: %s, add MDMs if On-demand connect disabled or try (re)install SDC & restart agent", host); logger.warn(msg); - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC details not found on host: " + host, msg); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC details not found on host: " + host, msg); return null; } if (details.containsKey(ScaleIOSDCManager.ConnectOnDemand.key())) { String connectOnDemand = details.get(ScaleIOSDCManager.ConnectOnDemand.key()); if (connectOnDemand != null && !Boolean.parseBoolean(connectOnDemand) && !_sdcManager.isHostSdcConnected(sdcId, dataStore, 15)) { - logger.warn("SDC not connected on the host: " + host); + logger.warn("SDC not connected on the host: {}", host); String msg = "SDC not connected on host: " + host + ", reconnect the SDC to MDM and restart agent"; - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC not connected on host: " + host, msg); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC not connected on host: " + host, msg); return null; } } @@ -212,7 +213,7 @@ public boolean hostDisconnected(long hostId, long poolId) { ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(false, storagePool, storagePool.getPath(), details); ModifyStoragePoolAnswer answer = sendModifyStoragePoolCommand(cmd, storagePool, host); if (!answer.getResult()) { - logger.error("Failed to disconnect storage pool: " + storagePool + " and host: " + host); + logger.error("Failed to disconnect storage pool: {} and host: {}", storagePool, host); return false; } @@ -220,7 +221,7 @@ public boolean hostDisconnected(long hostId, long poolId) { if (storagePoolHost != null) { _storagePoolHostDao.deleteStoragePoolHostDetails(hostId, poolId); } - logger.info("Connection removed between storage pool: " + storagePool + " and host: " + host); + logger.info("Connection removed between storage pool: {} and host: {}", storagePool, host); return true; } From bcd5b3b24614691aabacef74a8bdebaf4cf00f5e Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Sat, 15 Aug 2026 17:25:22 +0200 Subject: [PATCH 18/18] tests --- .../ScaleIOVMSnapshotStrategyTest.java | 476 +++++++++++++++ .../image/BaseImageStoreDriverImplTest.java | 490 +++++++++++++++ .../provider/DefaultHostListenerTest.java | 405 +++++++++++++ .../provider/DateraHostListenerTest.java | 567 ++++++++++++++++++ .../listener/OntapHostListenerTest.java | 344 +++++++++++ .../OutOfBandManagementServiceImplTest.java | 470 +++++++++++++++ 6 files changed, 2752 insertions(+) create mode 100644 engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategyTest.java create mode 100644 engine/storage/src/test/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImplTest.java create mode 100644 engine/storage/volume/src/test/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListenerTest.java create mode 100644 plugins/storage/volume/datera/src/test/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListenerTest.java create mode 100644 plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/listener/OntapHostListenerTest.java create mode 100644 server/src/test/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImplTest.java diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategyTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategyTest.java new file mode 100644 index 000000000000..e139a50e3580 --- /dev/null +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategyTest.java @@ -0,0 +1,476 @@ +/* + * 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 org.apache.cloudstack.storage.vmsnapshot; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyLong; +import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Collections; + +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.alert.AlertManager; +import com.cloud.event.EventTypes; +import com.cloud.event.UsageEventUtils; +import com.cloud.storage.Storage; +import com.cloud.storage.Storage.ImageFormat; +import com.cloud.storage.dao.DiskOfferingDao; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.fsm.NoTransitionException; +import com.cloud.vm.UserVmVO; +import com.cloud.vm.dao.UserVmDao; +import com.cloud.vm.snapshot.VMSnapshot; +import com.cloud.vm.snapshot.VMSnapshotDetailsVO; +import com.cloud.vm.snapshot.VMSnapshotVO; +import com.cloud.vm.snapshot.dao.VMSnapshotDao; +import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao; + +@RunWith(MockitoJUnitRunner.class) +public class ScaleIOVMSnapshotStrategyTest { + + @Mock + VMSnapshotHelper vmSnapshotHelper; + @Mock + UserVmDao userVmDao; + @Mock + VMSnapshotDao vmSnapshotDao; + @Mock + VMSnapshotDetailsDao vmSnapshotDetailsDao; + @Mock + ConfigurationDao configurationDao; + @Mock + VolumeDao volumeDao; + @Mock + DiskOfferingDao diskOfferingDao; + @Mock + PrimaryDataStoreDao storagePoolDao; + @Mock + StoragePoolDetailsDao storagePoolDetailsDao; + @Mock + AlertManager alertManager; + + private ScaleIOVMSnapshotStrategy strategy; + + @Before + public void setup() { + strategy = new ScaleIOVMSnapshotStrategy(); + ReflectionTestUtils.setField(strategy, "vmSnapshotHelper", vmSnapshotHelper); + ReflectionTestUtils.setField(strategy, "userVmDao", userVmDao); + ReflectionTestUtils.setField(strategy, "vmSnapshotDao", vmSnapshotDao); + ReflectionTestUtils.setField(strategy, "vmSnapshotDetailsDao", vmSnapshotDetailsDao); + ReflectionTestUtils.setField(strategy, "configurationDao", configurationDao); + ReflectionTestUtils.setField(strategy, "volumeDao", volumeDao); + ReflectionTestUtils.setField(strategy, "diskOfferingDao", diskOfferingDao); + ReflectionTestUtils.setField(strategy, "storagePoolDao", storagePoolDao); + ReflectionTestUtils.setField(strategy, "storagePoolDetailsDao", storagePoolDetailsDao); + ReflectionTestUtils.setField(strategy, "alertManager", alertManager); + } + + // ------------------------------------------------------------------ + // configure(String, Map) + // ------------------------------------------------------------------ + + @Test + public void configureReadsWaitValueFromConfigurationDao() throws Exception { + when(configurationDao.getValue("vmsnapshot.create.wait")).thenReturn("120"); + + boolean result = strategy.configure("ScaleIOVMSnapshotStrategy", null); + + assertTrue(result); + assertEquals(120, (int) ReflectionTestUtils.getField(strategy, "_wait")); + } + + @Test + public void configureDefaultsWaitTo1800WhenConfigValueUnset() throws Exception { + when(configurationDao.getValue("vmsnapshot.create.wait")).thenReturn(null); + + boolean result = strategy.configure("ScaleIOVMSnapshotStrategy", null); + + assertTrue(result); + assertEquals(1800, (int) ReflectionTestUtils.getField(strategy, "_wait")); + } + + // ------------------------------------------------------------------ + // canHandle(VMSnapshot) + // ------------------------------------------------------------------ + + @Test + public void canHandleThrowsWhenNoVolumesFoundForVm() { + VMSnapshot vmSnapshot = mock(VMSnapshot.class); + when(vmSnapshot.getVmId()).thenReturn(1L); + when(vmSnapshot.getUuid()).thenReturn("vmsnapshot-uuid"); + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(null); + + try { + strategy.canHandle(vmSnapshot); + fail("Expected CloudRuntimeException"); + } catch (CloudRuntimeException expected) { + assertTrue(expected.getMessage().contains("vmsnapshot-uuid")); + } + } + + @Test + public void canHandleReturnsCantHandleWhenNonAllocatedAndNoSnapshotGroupDetail() { + VMSnapshot vmSnapshot = mock(VMSnapshot.class); + when(vmSnapshot.getVmId()).thenReturn(1L); + when(vmSnapshot.getId()).thenReturn(10L); + when(vmSnapshot.getState()).thenReturn(VMSnapshot.State.Ready); + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(Collections.emptyList()); + when(vmSnapshotDetailsDao.findDetails(10L, "SnapshotGroupId")).thenReturn(Collections.emptyList()); + + StrategyPriority result = strategy.canHandle(vmSnapshot); + + assertEquals(StrategyPriority.CANT_HANDLE, result); + } + + @Test + public void canHandleReturnsHighestWhenNonAllocatedWithSnapshotGroupDetailAndPowerFlexVolumes() { + VMSnapshot vmSnapshot = mock(VMSnapshot.class); + when(vmSnapshot.getVmId()).thenReturn(1L); + when(vmSnapshot.getId()).thenReturn(10L); + when(vmSnapshot.getState()).thenReturn(VMSnapshot.State.Ready); + + VolumeObjectTO volumeTO = mock(VolumeObjectTO.class); + when(volumeTO.getPoolId()).thenReturn(5L); + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(Collections.singletonList(volumeTO)); + when(vmSnapshotHelper.getStoragePoolType(5L)).thenReturn(Storage.StoragePoolType.PowerFlex); + when(vmSnapshotDetailsDao.findDetails(10L, "SnapshotGroupId")) + .thenReturn(Collections.singletonList(mock(VMSnapshotDetailsVO.class))); + + StrategyPriority result = strategy.canHandle(vmSnapshot); + + assertEquals(StrategyPriority.HIGHEST, result); + } + + @Test + public void canHandleReturnsHighestForAllocatedSnapshotWithNoVolumes() { + VMSnapshot vmSnapshot = mock(VMSnapshot.class); + when(vmSnapshot.getVmId()).thenReturn(1L); + when(vmSnapshot.getState()).thenReturn(VMSnapshot.State.Allocated); + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(Collections.emptyList()); + + StrategyPriority result = strategy.canHandle(vmSnapshot); + + assertEquals(StrategyPriority.HIGHEST, result); + // Allocated state skips the SnapshotGroupId detail lookup entirely. + verify(vmSnapshotDetailsDao, never()).findDetails(anyLong(), anyString()); + } + + @Test + public void canHandleReturnsCantHandleWhenVolumeIsNotOnPowerFlexPool() { + VMSnapshot vmSnapshot = mock(VMSnapshot.class); + when(vmSnapshot.getVmId()).thenReturn(1L); + when(vmSnapshot.getState()).thenReturn(VMSnapshot.State.Allocated); + + VolumeObjectTO volumeTO = mock(VolumeObjectTO.class); + when(volumeTO.getPoolId()).thenReturn(5L); + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(Collections.singletonList(volumeTO)); + when(vmSnapshotHelper.getStoragePoolType(5L)).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + + StrategyPriority result = strategy.canHandle(vmSnapshot); + + assertEquals(StrategyPriority.CANT_HANDLE, result); + } + + // ------------------------------------------------------------------ + // canHandle(Long vmId, Long rootPoolId, boolean snapshotMemory) + // ------------------------------------------------------------------ + + @Test + public void canHandleWithMemorySnapshotAlwaysReturnsCantHandle() { + StrategyPriority result = strategy.canHandle(1L, 5L, true); + + assertEquals(StrategyPriority.CANT_HANDLE, result); + verify(vmSnapshotHelper, never()).getVolumeTOList(any()); + } + + @Test + public void canHandleByIdsReturnsCantHandleWhenVolumeListIsNull() { + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(null); + + StrategyPriority result = strategy.canHandle(1L, 5L, false); + + assertEquals(StrategyPriority.CANT_HANDLE, result); + } + + @Test + public void canHandleByIdsReturnsCantHandleWhenVolumeListIsEmpty() { + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(Collections.emptyList()); + + StrategyPriority result = strategy.canHandle(1L, 5L, false); + + assertEquals(StrategyPriority.CANT_HANDLE, result); + } + + @Test + public void canHandleByIdsReturnsCantHandleWhenPoolTypeIsNotPowerFlex() { + Long poolId = 5L; + VolumeObjectTO volumeTO = mock(VolumeObjectTO.class); + when(volumeTO.getPoolId()).thenReturn(poolId); + // getFormat() is never reached: poolType != PowerFlex short-circuits the "||" chain first. + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(Collections.singletonList(volumeTO)); + when(vmSnapshotHelper.getStoragePoolType(poolId)).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + + StrategyPriority result = strategy.canHandle(1L, poolId, false); + + assertEquals(StrategyPriority.CANT_HANDLE, result); + } + + @Test + public void canHandleByIdsReturnsCantHandleWhenVolumeFormatIsNotRaw() { + Long poolId = 5L; + VolumeObjectTO volumeTO = mock(VolumeObjectTO.class); + when(volumeTO.getPoolId()).thenReturn(poolId); + when(volumeTO.getFormat()).thenReturn(ImageFormat.QCOW2); + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(Collections.singletonList(volumeTO)); + when(vmSnapshotHelper.getStoragePoolType(poolId)).thenReturn(Storage.StoragePoolType.PowerFlex); + + StrategyPriority result = strategy.canHandle(1L, poolId, false); + + assertEquals(StrategyPriority.CANT_HANDLE, result); + } + + @Test + public void canHandleByIdsReturnsCantHandleWhenVolumePoolIdDoesNotMatchRootPoolId() { + VolumeObjectTO volumeTO = mock(VolumeObjectTO.class); + when(volumeTO.getPoolId()).thenReturn(5L); + when(volumeTO.getFormat()).thenReturn(ImageFormat.RAW); + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(Collections.singletonList(volumeTO)); + when(vmSnapshotHelper.getStoragePoolType(5L)).thenReturn(Storage.StoragePoolType.PowerFlex); + + StrategyPriority result = strategy.canHandle(1L, 6L, false); + + assertEquals(StrategyPriority.CANT_HANDLE, result); + } + + @Test + public void canHandleByIdsReturnsHighestWhenPoolTypeFormatAndRootPoolIdAllMatch() { + // Use the very same boxed Long instance for both the volume's pool id and the + // rootPoolId argument: the production code compares them with != (reference + // equality on the boxed Long), so identity must match for the happy path. + Long poolId = 5L; + VolumeObjectTO volumeTO = mock(VolumeObjectTO.class); + when(volumeTO.getPoolId()).thenReturn(poolId); + when(volumeTO.getFormat()).thenReturn(ImageFormat.RAW); + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(Collections.singletonList(volumeTO)); + when(vmSnapshotHelper.getStoragePoolType(poolId)).thenReturn(Storage.StoragePoolType.PowerFlex); + + StrategyPriority result = strategy.canHandle(1L, poolId, false); + + assertEquals(StrategyPriority.HIGHEST, result); + } + + // ------------------------------------------------------------------ + // updateOperationFailed(VMSnapshot) + // ------------------------------------------------------------------ + + @Test + public void updateOperationFailedDelegatesToVmSnapshotHelper() throws NoTransitionException { + VMSnapshot vmSnapshot = mock(VMSnapshot.class); + + strategy.updateOperationFailed(vmSnapshot); + + verify(vmSnapshotHelper).vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed); + } + + @Test + public void updateOperationFailedRethrowsNoTransitionException() throws NoTransitionException { + VMSnapshot vmSnapshot = mock(VMSnapshot.class); + NoTransitionException noTransitionException = new NoTransitionException("cannot transition"); + when(vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed)) + .thenThrow(noTransitionException); + + try { + strategy.updateOperationFailed(vmSnapshot); + fail("Expected NoTransitionException"); + } catch (NoTransitionException expected) { + assertEquals(noTransitionException, expected); + } + } + + // ------------------------------------------------------------------ + // deleteVMSnapshotFromDB(VMSnapshot, boolean unmanage) + // ------------------------------------------------------------------ + + @Test + public void deleteVMSnapshotFromDBRemovesSnapshotAndSkipsUsageEventWhenNotUnmanaged() { + VMSnapshot vmSnapshot = mock(VMSnapshot.class); + when(vmSnapshot.getVmId()).thenReturn(1L); + when(vmSnapshot.getId()).thenReturn(10L); + + UserVmVO userVm = mock(UserVmVO.class); + when(userVm.getId()).thenReturn(1L); + when(userVmDao.findById(1L)).thenReturn(userVm); + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(Collections.emptyList()); + when(vmSnapshotDao.remove(10L)).thenReturn(true); + + try (MockedStatic usageEventUtils = mockStatic(UsageEventUtils.class)) { + boolean result = strategy.deleteVMSnapshotFromDB(vmSnapshot, false); + + assertTrue(result); + verify(vmSnapshotDao).remove(10L); + usageEventUtils.verify(() -> UsageEventUtils.publishUsageEvent( + eq(EventTypes.EVENT_VM_SNAPSHOT_OFF_PRIMARY), anyLong(), anyLong(), anyLong(), anyString(), + any(), any(), any(), any(), anyString(), anyString(), any()), never()); + } + } + + @Test + public void deleteVMSnapshotFromDBPublishesOffPrimaryUsageEventWhenUnmanaged() { + VMSnapshot vmSnapshot = mock(VMSnapshot.class); + when(vmSnapshot.getVmId()).thenReturn(1L); + when(vmSnapshot.getId()).thenReturn(10L); + when(vmSnapshot.getAccountId()).thenReturn(2L); + when(vmSnapshot.getName()).thenReturn("vm-snapshot-name"); + when(vmSnapshot.getUuid()).thenReturn("vmsnapshot-uuid"); + + UserVmVO userVm = mock(UserVmVO.class); + when(userVm.getId()).thenReturn(1L); + when(userVm.getDataCenterId()).thenReturn(3L); + when(userVmDao.findById(1L)).thenReturn(userVm); + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(Collections.emptyList()); + when(vmSnapshotDao.remove(10L)).thenReturn(true); + + try (MockedStatic usageEventUtils = mockStatic(UsageEventUtils.class)) { + boolean result = strategy.deleteVMSnapshotFromDB(vmSnapshot, true); + + assertTrue(result); + verify(vmSnapshotDao).remove(10L); + usageEventUtils.verify(() -> UsageEventUtils.publishUsageEvent( + eq(EventTypes.EVENT_VM_SNAPSHOT_OFF_PRIMARY), anyLong(), anyLong(), anyLong(), anyString(), + any(), any(), any(), any(), anyString(), anyString(), any()), times(1)); + } + } + + @Test + public void deleteVMSnapshotFromDBThrowsWhenExpungeRequestedTransitionFails() throws NoTransitionException { + VMSnapshot vmSnapshot = mock(VMSnapshot.class); + when(vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.ExpungeRequested)) + .thenThrow(new NoTransitionException("cannot transition")); + + try { + strategy.deleteVMSnapshotFromDB(vmSnapshot, false); + fail("Expected CloudRuntimeException"); + } catch (CloudRuntimeException expected) { + // expected: state transition failure is wrapped in a CloudRuntimeException + } + verify(vmSnapshotDao, never()).remove(anyLong()); + } + + // ------------------------------------------------------------------ + // takeVMSnapshot(VMSnapshot) + // + // The happy path of takeVMSnapshot() is not exercised here: past the state + // transition and volume bookkeeping, it calls getScaleIOClient(storagePool), + // which reaches the real ScaleIOGatewayClientConnectionPool.getInstance() + // singleton. That singleton cannot be swapped out in a plain Mockito unit + // test, and constructing a real gateway client requires a live PowerFlex + // gateway. Instead, this test drives the method far enough to reach that + // call (using an empty volume list so no volume/disk-offering mocking is + // needed) and asserts on the failure path: the singleton predictably throws + // (its Preconditions check rejects the mock storage pool's default id of 0), + // which is caught by takeVMSnapshot's own catch block, converted into a + // CloudRuntimeException, and re-thrown after the finally block sends the + // alert whose subject/body this PR changed to include vmSnapshot.toString(). + // ------------------------------------------------------------------ + + @Test + public void takeVMSnapshotSendsAlertWithSnapshotDetailsWhenGatewayClientCannotBeCreated() throws Exception { + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + when(vmSnapshot.getVmId()).thenReturn(1L); + when(vmSnapshot.toString()).thenReturn("VMSnapshot {id=10, name=snap1}"); + + UserVmVO userVm = mock(UserVmVO.class); + when(userVm.getId()).thenReturn(1L); + when(userVm.getDisplayName()).thenReturn("test-vm"); + when(userVm.getDataCenterId()).thenReturn(3L); + when(userVm.getPodIdToDeployIn()).thenReturn(4L); + when(userVmDao.findById(1L)).thenReturn(userVm); + + when(vmSnapshotHelper.getVolumeTOList(1L)).thenReturn(Collections.emptyList()); + + StoragePoolVO storagePool = mock(StoragePoolVO.class); + when(vmSnapshotHelper.getStoragePoolForVM(userVm)).thenReturn(storagePool); + when(vmSnapshotDao.findCurrentSnapshotByVmId(1L)).thenReturn(null); + + try { + strategy.takeVMSnapshot(vmSnapshot); + fail("Expected CloudRuntimeException from the (unreachable in unit tests) ScaleIO gateway client"); + } catch (CloudRuntimeException expected) { + // expected: getScaleIOClient() cannot succeed without a real PowerFlex gateway + } + + verify(vmSnapshotHelper).vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed); + + ArgumentCaptor subjectCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + verify(alertManager).sendAlert(eq(AlertManager.AlertType.ALERT_TYPE_VM_SNAPSHOT), eq(3L), eq(4L), + subjectCaptor.capture(), bodyCaptor.capture()); + + assertTrue(subjectCaptor.getValue().contains("Take snapshot failed")); + assertTrue(subjectCaptor.getValue().contains("test-vm")); + assertTrue(bodyCaptor.getValue().contains("test-vm")); + assertTrue(bodyCaptor.getValue().contains("VMSnapshot {id=10, name=snap1}")); + } + + @Test + public void takeVMSnapshotThrowsCloudRuntimeExceptionWhenCreateRequestedTransitionFails() throws Exception { + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + when(vmSnapshot.getVmId()).thenReturn(1L); + UserVmVO userVm = mock(UserVmVO.class); + when(userVmDao.findById(1L)).thenReturn(userVm); + when(vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.CreateRequested)) + .thenThrow(new NoTransitionException("cannot transition")); + + try { + strategy.takeVMSnapshot(vmSnapshot); + fail("Expected CloudRuntimeException"); + } catch (CloudRuntimeException expected) { + // expected: NoTransitionException on CreateRequested is wrapped + } + // The CreateRequested failure is thrown before the try/finally block that + // sends the failure alert, so no alert should have been raised here. + verify(alertManager, never()).sendAlert(any(), anyLong(), any(), anyString(), anyString()); + } +} diff --git a/engine/storage/src/test/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImplTest.java b/engine/storage/src/test/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImplTest.java new file mode 100644 index 000000000000..75e0f7b3d695 --- /dev/null +++ b/engine/storage/src/test/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImplTest.java @@ -0,0 +1,490 @@ +/* + * 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 org.apache.cloudstack.storage.image; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyLong; +import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Collections; +import java.util.List; + +import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; +import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; +import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; +import org.apache.cloudstack.framework.async.AsyncCallbackDispatcher; +import org.apache.cloudstack.framework.async.AsyncCompletionCallback; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.storage.command.CommandResult; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO; +import org.apache.cloudstack.storage.endpoint.DefaultEndPointSelector; +import org.apache.cloudstack.storage.image.deployasis.DeployAsIsHelper; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.storage.DownloadAnswer; +import com.cloud.agent.api.storage.GetDatadisksAnswer; +import com.cloud.agent.api.to.DataObjectType; +import com.cloud.agent.api.to.DataStoreTO; +import com.cloud.agent.api.to.DataTO; +import com.cloud.agent.api.to.DatadiskTO; +import com.cloud.agent.api.to.NfsTO; +import com.cloud.alert.AlertManager; +import com.cloud.host.dao.HostDao; +import com.cloud.secstorage.CommandExecLogDao; +import com.cloud.storage.DataStoreRole; +import com.cloud.storage.VMTemplateStorageResourceAssoc; +import com.cloud.storage.VMTemplateZoneVO; +import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VMTemplateZoneDao; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.storage.download.DownloadMonitor; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.dao.SecondaryStorageVmDao; + +@RunWith(MockitoJUnitRunner.class) +public class BaseImageStoreDriverImplTest { + + @Mock + VMTemplateDao _templateDao; + @Mock + DownloadMonitor _downloadMonitor; + @Mock + VolumeDao volumeDao; + @Mock + VolumeDataStoreDao _volumeStoreDao; + @Mock + TemplateDataStoreDao _templateStoreDao; + @Mock + SnapshotDataStoreDao snapshotDataStoreDao; + @Mock + EndPointSelector _epSelector; + @Mock + ConfigurationDao configDao; + @Mock + VMTemplateZoneDao _vmTemplateZoneDao; + @Mock + AlertManager _alertMgr; + @Mock + DefaultEndPointSelector _defaultEpSelector; + @Mock + DeployAsIsHelper deployAsIsHelper; + @Mock + HostDao hostDao; + @Mock + CommandExecLogDao _cmdExecLogDao; + @Mock + SecondaryStorageVmDao _secStorageVmDao; + @Mock + AgentManager agentMgr; + @Mock + DataStoreManager dataStoreManager; + + @Mock + DataObject dataObject; + @Mock + DataStore dataStore; + + private TestDriver driver; + + private static class TestDriver extends BaseImageStoreDriverImpl { + @Override + public String createEntityExtractUrl(DataStore store, String installPath, com.cloud.storage.Storage.ImageFormat format, DataObject dataObject) { + return null; + } + + @Override + public com.cloud.agent.api.to.DataStoreTO getStoreTO(DataStore store) { + return null; + } + } + + @Before + public void setup() { + driver = new TestDriver(); + ReflectionTestUtils.setField(driver, "_templateDao", _templateDao); + ReflectionTestUtils.setField(driver, "_downloadMonitor", _downloadMonitor); + ReflectionTestUtils.setField(driver, "volumeDao", volumeDao); + ReflectionTestUtils.setField(driver, "_volumeStoreDao", _volumeStoreDao); + ReflectionTestUtils.setField(driver, "_templateStoreDao", _templateStoreDao); + ReflectionTestUtils.setField(driver, "snapshotDataStoreDao", snapshotDataStoreDao); + ReflectionTestUtils.setField(driver, "_epSelector", _epSelector); + ReflectionTestUtils.setField(driver, "configDao", configDao); + ReflectionTestUtils.setField(driver, "_vmTemplateZoneDao", _vmTemplateZoneDao); + ReflectionTestUtils.setField(driver, "_alertMgr", _alertMgr); + ReflectionTestUtils.setField(driver, "_defaultEpSelector", _defaultEpSelector); + ReflectionTestUtils.setField(driver, "deployAsIsHelper", deployAsIsHelper); + ReflectionTestUtils.setField(driver, "hostDao", hostDao); + ReflectionTestUtils.setField(driver, "_cmdExecLogDao", _cmdExecLogDao); + ReflectionTestUtils.setField(driver, "_secStorageVmDao", _secStorageVmDao); + ReflectionTestUtils.setField(driver, "agentMgr", agentMgr); + ReflectionTestUtils.setField(driver, "dataStoreManager", dataStoreManager); + } + + // ---------- createTemplateAsyncCallback ---------- + + @SuppressWarnings("unchecked") + private AsyncCallbackDispatcher dispatcherReturning(DownloadAnswer answer) { + AsyncCallbackDispatcher dispatcher = mock(AsyncCallbackDispatcher.class); + when(dispatcher.getResult()).thenReturn(answer); + return dispatcher; + } + + @SuppressWarnings("unchecked") + private AsyncCompletionCallback mockParentCallback() { + return mock(AsyncCompletionCallback.class); + } + + @Test + public void createTemplateAsyncCallbackSendsAlertOnErrorDownloadState() { + when(dataObject.getId()).thenReturn(10L); + when(dataObject.getDataStore()).thenReturn(dataStore); + when(dataObject.toString()).thenReturn("Template[id=10]"); + when(dataStore.getId()).thenReturn(20L); + when(_templateStoreDao.findByStoreTemplate(20L, 10L)).thenReturn(null); + + VMTemplateZoneVO zoneVO = mock(VMTemplateZoneVO.class); + when(zoneVO.getZoneId()).thenReturn(5L); + when(_vmTemplateZoneDao.listByTemplateId(10L)).thenReturn(Collections.singletonList(zoneVO)); + + DownloadAnswer answer = new DownloadAnswer("job-1", 0, "download failed", VMTemplateStorageResourceAssoc.Status.DOWNLOAD_ERROR, + null, null, 0L, 0L, null); + AsyncCallbackDispatcher dispatcher = dispatcherReturning(answer); + AsyncCompletionCallback parentCallback = mockParentCallback(); + BaseImageStoreDriverImpl.CreateContext context = driver.new CreateContext<>(parentCallback, dataObject); + + driver.createTemplateAsyncCallback(dispatcher, context); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(parentCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertEquals("download failed", resultCaptor.getValue().getResult()); + + ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); + verify(_alertMgr).sendAlert(eq(AlertManager.AlertType.ALERT_TYPE_UPLOAD_FAILED), eq(5L), eq((Long) null), msgCaptor.capture(), msgCaptor.capture()); + assertTrue(msgCaptor.getValue().contains("Template[id=10]")); + assertTrue(msgCaptor.getValue().contains("Failed to register template")); + } + + @Test + public void createTemplateAsyncCallbackUpdatesChecksumOnDownloaded() { + when(dataObject.getId()).thenReturn(11L); + when(dataObject.getDataStore()).thenReturn(dataStore); + when(dataStore.getId()).thenReturn(21L); + when(_templateStoreDao.findByStoreTemplate(21L, 11L)).thenReturn(null); + when(_templateDao.createForUpdate()).thenReturn(new com.cloud.storage.VMTemplateVO()); + + DownloadAnswer answer = new DownloadAnswer("job-2", 100, null, VMTemplateStorageResourceAssoc.Status.DOWNLOADED, + "/path", "/install", 1024L, 1024L, "abcd1234"); + AsyncCallbackDispatcher dispatcher = dispatcherReturning(answer); + AsyncCompletionCallback parentCallback = mockParentCallback(); + BaseImageStoreDriverImpl.CreateContext context = driver.new CreateContext<>(parentCallback, dataObject); + + driver.createTemplateAsyncCallback(dispatcher, context); + + verify(_templateDao).update(eq(11L), any(com.cloud.storage.VMTemplateVO.class)); + verify(parentCallback).complete(any(CreateCmdResult.class)); + verify(_alertMgr, never()).sendAlert(any(AlertManager.AlertType.class), anyLong(), any(), anyString(), anyString()); + } + + // ---------- createVolumeAsyncCallback ---------- + + @Test + public void createVolumeAsyncCallbackSendsAlertWithVolStoreZoneIdOnError() { + when(dataObject.getId()).thenReturn(30L); + when(dataObject.getDataStore()).thenReturn(dataStore); + when(dataObject.toString()).thenReturn("Volume[id=30]"); + when(dataStore.getId()).thenReturn(40L); + + VolumeDataStoreVO volStoreVO = mock(VolumeDataStoreVO.class); + when(volStoreVO.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.DOWNLOAD_IN_PROGRESS); + when(volStoreVO.getZoneId()).thenReturn(99L); + when(volStoreVO.getId()).thenReturn(1L); + when(_volumeStoreDao.findByStoreVolume(40L, 30L)).thenReturn(volStoreVO); + when(_volumeStoreDao.createForUpdate()).thenReturn(new VolumeDataStoreVO()); + + DownloadAnswer answer = new DownloadAnswer("job-3", 0, "upload failed", VMTemplateStorageResourceAssoc.Status.DOWNLOAD_ERROR, + null, null, 0L, 0L, null); + AsyncCallbackDispatcher dispatcher = dispatcherReturning(answer); + AsyncCompletionCallback parentCallback = mockParentCallback(); + BaseImageStoreDriverImpl.CreateContext context = driver.new CreateContext<>(parentCallback, dataObject); + + driver.createVolumeAsyncCallback(dispatcher, context); + + ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); + verify(_alertMgr).sendAlert(eq(AlertManager.AlertType.ALERT_TYPE_UPLOAD_FAILED), eq(99L), eq((Long) null), msgCaptor.capture(), msgCaptor.capture()); + assertTrue(msgCaptor.getValue().contains("Volume[id=30]")); + assertTrue(msgCaptor.getValue().contains("Failed to upload volume")); + } + + @Test + public void createVolumeAsyncCallbackSendsAlertWithNegativeOneZoneIdWhenVolStoreNull() { + when(dataObject.getId()).thenReturn(31L); + when(dataObject.getDataStore()).thenReturn(dataStore); + when(dataObject.toString()).thenReturn("Volume[id=31]"); + when(dataStore.getId()).thenReturn(41L); + when(_volumeStoreDao.findByStoreVolume(41L, 31L)).thenReturn(null); + + DownloadAnswer answer = new DownloadAnswer("job-4", 0, "upload failed again", VMTemplateStorageResourceAssoc.Status.DOWNLOAD_ERROR, + null, null, 0L, 0L, null); + AsyncCallbackDispatcher dispatcher = dispatcherReturning(answer); + AsyncCompletionCallback parentCallback = mockParentCallback(); + BaseImageStoreDriverImpl.CreateContext context = driver.new CreateContext<>(parentCallback, dataObject); + + driver.createVolumeAsyncCallback(dispatcher, context); + + verify(_alertMgr).sendAlert(eq(AlertManager.AlertType.ALERT_TYPE_UPLOAD_FAILED), eq(-1L), eq((Long) null), any(), any()); + } + + // ---------- createSnapshotAsyncCallback ---------- + + @Test + public void createSnapshotAsyncCallbackSendsAlertUsingDataStoreManagerZoneId() { + when(dataObject.getId()).thenReturn(50L); + when(dataObject.getDataStore()).thenReturn(dataStore); + when(dataObject.toString()).thenReturn("Snapshot[id=50]"); + when(dataStore.getId()).thenReturn(60L); + when(dataStore.getRole()).thenReturn(DataStoreRole.Image); + when(snapshotDataStoreDao.findByStoreSnapshot(DataStoreRole.Image, 60L, 50L)).thenReturn(null); + when(dataStoreManager.getStoreZoneId(60L, DataStoreRole.Image)).thenReturn(7L); + + DownloadAnswer answer = new DownloadAnswer("job-5", 0, "copy failed", VMTemplateStorageResourceAssoc.Status.DOWNLOAD_ERROR, + null, null, 0L, 0L, null); + AsyncCallbackDispatcher dispatcher = dispatcherReturning(answer); + AsyncCompletionCallback parentCallback = mockParentCallback(); + BaseImageStoreDriverImpl.CreateContext context = driver.new CreateContext<>(parentCallback, dataObject); + + driver.createSnapshotAsyncCallback(dispatcher, context); + + ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); + verify(_alertMgr).sendAlert(eq(AlertManager.AlertType.ALERT_TYPE_UPLOAD_FAILED), eq(7L), eq((Long) null), msgCaptor.capture(), msgCaptor.capture()); + assertTrue(msgCaptor.getValue().contains("Snapshot[id=50]")); + assertTrue(msgCaptor.getValue().contains("Failed to copy snapshot")); + } + + @Test + public void createSnapshotAsyncCallbackCompletesOnDownloaded() { + when(dataObject.getId()).thenReturn(51L); + when(dataObject.getDataStore()).thenReturn(dataStore); + when(dataStore.getId()).thenReturn(61L); + when(snapshotDataStoreDao.findByStoreSnapshot(DataStoreRole.Image, 61L, 51L)).thenReturn(null); + + DownloadAnswer answer = new DownloadAnswer("job-6", 100, null, VMTemplateStorageResourceAssoc.Status.DOWNLOADED, + "/path", "/install", 100L, 100L, null); + AsyncCallbackDispatcher dispatcher = dispatcherReturning(answer); + AsyncCompletionCallback parentCallback = mockParentCallback(); + BaseImageStoreDriverImpl.CreateContext context = driver.new CreateContext<>(parentCallback, dataObject); + + driver.createSnapshotAsyncCallback(dispatcher, context); + + verify(parentCallback).complete(any(CreateCmdResult.class)); + verify(_alertMgr, never()).sendAlert(any(AlertManager.AlertType.class), anyLong(), any(), anyString(), anyString()); + } + + // ---------- canCopy ---------- + + private DataObject nfsImageDataObject(DataObjectType type) { + DataObject obj = mock(DataObject.class); + DataStore store = mock(DataStore.class); + NfsTO nfsTO = mock(NfsTO.class); + when(store.getTO()).thenReturn(nfsTO); + when(store.getRole()).thenReturn(DataStoreRole.Image); + when(obj.getDataStore()).thenReturn(store); + when(obj.getType()).thenReturn(type); + return obj; + } + + @Test + public void canCopyReturnsTrueForMatchingNfsImageTemplates() { + DataObject src = nfsImageDataObject(DataObjectType.TEMPLATE); + DataObject dest = nfsImageDataObject(DataObjectType.TEMPLATE); + + assertTrue(driver.canCopy(src, dest)); + } + + @Test + public void canCopyReturnsFalseForNonNfsDataStoreTO() { + DataObject src = mock(DataObject.class); + DataObject dest = nfsImageDataObject(DataObjectType.TEMPLATE); + + DataStore srcStore = mock(DataStore.class); + DataStoreTO nonNfsTO = mock(DataStoreTO.class); + when(srcStore.getTO()).thenReturn(nonNfsTO); + when(src.getDataStore()).thenReturn(srcStore); + + assertFalse(driver.canCopy(src, dest)); + } + + @Test + public void canCopyReturnsFalseForMismatchedRole() { + DataObject src = mock(DataObject.class); + DataObject dest = nfsImageDataObject(DataObjectType.TEMPLATE); + + DataStore srcStore = mock(DataStore.class); + NfsTO nfsTO = mock(NfsTO.class); + when(srcStore.getTO()).thenReturn(nfsTO); + when(srcStore.getRole()).thenReturn(DataStoreRole.Primary); + when(src.getDataStore()).thenReturn(srcStore); + + assertFalse(driver.canCopy(src, dest)); + } + + @Test + public void canCopyReturnsFalseForMismatchedType() { + DataObject src = nfsImageDataObject(DataObjectType.TEMPLATE); + DataObject dest = nfsImageDataObject(DataObjectType.VOLUME); + + assertFalse(driver.canCopy(src, dest)); + } + + // ---------- deleteAsync ---------- + + @SuppressWarnings("unchecked") + @Test + public void deleteAsyncReturnsErrorWhenNoEndpoint() { + DataTO dataTO = mock(DataTO.class); + when(dataObject.getTO()).thenReturn(dataTO); + when(_epSelector.select(dataObject)).thenReturn(null); + + AsyncCompletionCallback callback = mock(AsyncCompletionCallback.class); + + driver.deleteAsync(dataStore, dataObject, callback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); + verify(callback).complete(resultCaptor.capture()); + assertNotNull(resultCaptor.getValue().getResult()); + assertTrue(resultCaptor.getValue().getResult().contains("No remote endpoint")); + } + + @SuppressWarnings("unchecked") + @Test + public void deleteAsyncReturnsFailureDetailsFromAnswer() { + DataTO dataTO = mock(DataTO.class); + when(dataObject.getTO()).thenReturn(dataTO); + EndPoint ep = mock(EndPoint.class); + when(_epSelector.select(dataObject)).thenReturn(ep); + Answer answer = new Answer(null, false, "delete failed on host"); + when(ep.sendMessage(any())).thenReturn(answer); + + AsyncCompletionCallback callback = mock(AsyncCompletionCallback.class); + + driver.deleteAsync(dataStore, dataObject, callback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); + verify(callback).complete(resultCaptor.capture()); + assertEquals("delete failed on host", resultCaptor.getValue().getResult()); + } + + // ---------- getDataDiskTemplates ---------- + + @Test + public void getDataDiskTemplatesReturnsListOnSuccess() { + DataTO dataTO = mock(DataTO.class); + when(dataObject.getTO()).thenReturn(dataTO); + when(dataObject.getDataStore()).thenReturn(dataStore); + EndPoint ep = mock(EndPoint.class); + when(_defaultEpSelector.select(dataStore)).thenReturn(ep); + + List disks = Collections.singletonList(mock(DatadiskTO.class)); + GetDatadisksAnswer answer = new GetDatadisksAnswer(disks); + when(ep.sendMessage(any())).thenReturn(answer); + + List result = driver.getDataDiskTemplates(dataObject, "cfg-1"); + + assertEquals(disks, result); + } + + @Test + public void getDataDiskTemplatesThrowsWhenNoEndpoint() { + DataTO dataTO = mock(DataTO.class); + when(dataObject.getTO()).thenReturn(dataTO); + when(dataObject.getDataStore()).thenReturn(dataStore); + when(_defaultEpSelector.select(dataStore)).thenReturn(null); + + try { + driver.getDataDiskTemplates(dataObject, "cfg-2"); + fail("Expected CloudRuntimeException"); + } catch (CloudRuntimeException expected) { + assertTrue(expected.getMessage().contains("Get Data disk command failed")); + } + } + + @Test + public void getDataDiskTemplatesThrowsWhenAnswerResultFalse() { + DataTO dataTO = mock(DataTO.class); + when(dataObject.getTO()).thenReturn(dataTO); + when(dataObject.getDataStore()).thenReturn(dataStore); + EndPoint ep = mock(EndPoint.class); + when(_defaultEpSelector.select(dataStore)).thenReturn(ep); + Answer answer = new Answer(null, false, "disk listing failed"); + when(ep.sendMessage(any())).thenReturn(answer); + + try { + driver.getDataDiskTemplates(dataObject, "cfg-3"); + fail("Expected CloudRuntimeException"); + } catch (CloudRuntimeException expected) { + assertTrue(expected.getMessage().contains("disk listing failed")); + } + } + + // ---------- getHttpProxy ---------- + + @Test + public void getHttpProxyReturnsNullWhenProxyNotSet() { + Object proxy = ReflectionTestUtils.invokeMethod(driver, "getHttpProxy"); + assertEquals(null, proxy); + } + + @Test + public void getHttpProxyReturnsProxyWhenValidUriSet() { + ReflectionTestUtils.setField(driver, "_proxy", "http://proxyhost:3128"); + Object proxy = ReflectionTestUtils.invokeMethod(driver, "getHttpProxy"); + assertNotNull(proxy); + } + + @Test + public void getHttpProxyReturnsNullWhenUriInvalid() { + ReflectionTestUtils.setField(driver, "_proxy", "http://invalid uri with spaces"); + Object proxy = ReflectionTestUtils.invokeMethod(driver, "getHttpProxy"); + assertEquals(null, proxy); + } +} diff --git a/engine/storage/volume/src/test/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListenerTest.java b/engine/storage/volume/src/test/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListenerTest.java new file mode 100644 index 000000000000..3b444497d15d --- /dev/null +++ b/engine/storage/volume/src/test/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListenerTest.java @@ -0,0 +1,405 @@ +// 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 org.apache.cloudstack.storage.datastore.provider; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Collections; +import java.util.List; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CleanupPersistentNetworkResourceCommand; +import com.cloud.agent.api.Command; +import com.cloud.agent.api.ModifyStoragePoolAnswer; +import com.cloud.agent.api.SetupPersistentNetworkCommand; +import com.cloud.agent.api.StoragePoolInfo; +import com.cloud.alert.AlertManager; +import com.cloud.configuration.ConfigurationManager; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.exception.StorageConflictException; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; +import com.cloud.network.NetworkModel; +import com.cloud.network.dao.NetworkDao; +import com.cloud.network.dao.NetworkVO; +import com.cloud.offerings.NetworkOfferingVO; +import com.cloud.offerings.dao.NetworkOfferingDao; +import com.cloud.storage.DataStoreRole; +import com.cloud.storage.Storage; +import com.cloud.storage.StorageManager; +import com.cloud.storage.StoragePoolHostVO; +import com.cloud.storage.StorageService; +import com.cloud.storage.dao.StoragePoolHostDao; +import com.cloud.utils.exception.CloudRuntimeException; + +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; + +@RunWith(MockitoJUnitRunner.class) +public class DefaultHostListenerTest { + + @Mock + AgentManager agentMgr; + @Mock + DataStoreManager dataStoreMgr; + @Mock + AlertManager alertMgr; + @Mock + StoragePoolHostDao storagePoolHostDao; + @Mock + PrimaryDataStoreDao primaryStoreDao; + @Mock + StoragePoolDetailsDao storagePoolDetailsDao; + @Mock + StorageManager storageManager; + @Mock + StorageService storageService; + @Mock + DataCenterDao zoneDao; + @Mock + NetworkOfferingDao networkOfferingDao; + @Mock + HostDao hostDao; + @Mock + NetworkModel networkModel; + @Mock + ConfigurationManager configManager; + @Mock + NetworkDao networkDao; + + @Mock + PrimaryDataStore pool; + @Mock + HostVO host; + @Mock + StoragePoolVO poolVO; + @Mock + ModifyStoragePoolAnswer mspAnswer; + + private DefaultHostListener listener; + + private static final long HOST_ID = 5L; + private static final long POOL_ID = 10L; + + @Before + public void setup() { + listener = new DefaultHostListener(); + ReflectionTestUtils.setField(listener, "agentMgr", agentMgr); + ReflectionTestUtils.setField(listener, "dataStoreMgr", dataStoreMgr); + ReflectionTestUtils.setField(listener, "alertMgr", alertMgr); + ReflectionTestUtils.setField(listener, "storagePoolHostDao", storagePoolHostDao); + ReflectionTestUtils.setField(listener, "primaryStoreDao", primaryStoreDao); + ReflectionTestUtils.setField(listener, "storagePoolDetailsDao", storagePoolDetailsDao); + ReflectionTestUtils.setField(listener, "storageManager", storageManager); + ReflectionTestUtils.setField(listener, "storageService", storageService); + ReflectionTestUtils.setField(listener, "zoneDao", zoneDao); + ReflectionTestUtils.setField(listener, "networkOfferingDao", networkOfferingDao); + ReflectionTestUtils.setField(listener, "hostDao", hostDao); + ReflectionTestUtils.setField(listener, "networkModel", networkModel); + ReflectionTestUtils.setField(listener, "configManager", configManager); + ReflectionTestUtils.setField(listener, "networkDao", networkDao); + } + + private void setUpPoolForConnect(Storage.StoragePoolType poolType) { + when(dataStoreMgr.getDataStore(POOL_ID, DataStoreRole.Primary)).thenReturn(pool); + when(pool.getId()).thenReturn(POOL_ID); + when(pool.getPoolType()).thenReturn(poolType); + when(pool.getDataCenterId()).thenReturn(1L); + when(pool.getPodId()).thenReturn(2L); + when(storageManager.getStoragePoolNFSMountOpts(eq(pool), any())).thenReturn(new com.cloud.utils.Pair<>(null, false)); + when(hostDao.findById(HOST_ID)).thenReturn(host); + when(primaryStoreDao.findById(POOL_ID)).thenReturn(poolVO); + when(poolVO.getId()).thenReturn(POOL_ID); + } + + private StoragePoolInfo newPoolInfo() { + return new StoragePoolInfo("uuid", "hostAddr", "/host/path", "/local/path", Storage.StoragePoolType.NetworkFilesystem, 1000L, 500L); + } + + // ---- hostAdded ---- + + @Test + public void hostAddedAlwaysReturnsTrue() { + assertTrue(listener.hostAdded(123L)); + } + + // ---- hostConnect ---- + + @Test + public void hostConnectHappyPathPersistsStoragePoolHostAndSetsUpPersistentNetwork() throws StorageConflictException { + setUpPoolForConnect(Storage.StoragePoolType.NetworkFilesystem); + when(agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(mspAnswer); + when(mspAnswer.getResult()).thenReturn(true); + when(mspAnswer.getPoolInfo()).thenReturn(newPoolInfo()); + when(storagePoolHostDao.findByPoolHost(POOL_ID, HOST_ID)).thenReturn(null); + when(networkDao.getAllPersistentNetworksFromZone(anyLong())).thenReturn(Collections.emptyList()); + + boolean result = listener.hostConnect(HOST_ID, POOL_ID); + + assertTrue(result); + verify(storagePoolHostDao).persist(any(StoragePoolHostVO.class)); + verify(primaryStoreDao).update(eq(POOL_ID), eq(poolVO)); + verify(storageService).updateStorageCapabilities(POOL_ID, false); + } + + @Test + public void hostConnectUpdatesExistingStoragePoolHostWhenAlreadyPresent() throws StorageConflictException { + setUpPoolForConnect(Storage.StoragePoolType.NetworkFilesystem); + when(agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(mspAnswer); + when(mspAnswer.getResult()).thenReturn(true); + when(mspAnswer.getPoolInfo()).thenReturn(newPoolInfo()); + StoragePoolHostVO existing = new StoragePoolHostVO(POOL_ID, HOST_ID, "/old/path"); + when(storagePoolHostDao.findByPoolHost(POOL_ID, HOST_ID)).thenReturn(existing); + when(networkDao.getAllPersistentNetworksFromZone(anyLong())).thenReturn(Collections.emptyList()); + + boolean result = listener.hostConnect(HOST_ID, POOL_ID); + + assertTrue(result); + verify(storagePoolHostDao, never()).persist(any(StoragePoolHostVO.class)); + verify(primaryStoreDao).update(eq(POOL_ID), eq(poolVO)); + assertEquals("/local/path", existing.getLocalPath()); + } + + @Test + public void hostConnectThrowsWhenAnswerIsNull() { + setUpPoolForConnect(Storage.StoragePoolType.NetworkFilesystem); + when(agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(null); + + assertThrows(CloudRuntimeException.class, () -> listener.hostConnect(HOST_ID, POOL_ID)); + } + + @Test + public void hostConnectSendsAlertContainingHostAndThrowsWhenAnswerFails() { + setUpPoolForConnect(Storage.StoragePoolType.NetworkFilesystem); + when(host.toString()).thenReturn("Host {id=5, name=cs-kvm06}"); + when(agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(mspAnswer); + when(mspAnswer.getResult()).thenReturn(false); + + assertThrows(CloudRuntimeException.class, () -> listener.hostConnect(HOST_ID, POOL_ID)); + + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(String.class); + verify(alertMgr).sendAlert(eq(AlertManager.AlertType.ALERT_TYPE_HOST), eq(1L), eq(2L), messageCaptor.capture(), messageCaptor.capture()); + assertTrue(messageCaptor.getValue().contains("Host {id=5, name=cs-kvm06}")); + assertTrue(messageCaptor.getValue().contains("Unable to attach storage pool")); + } + + @Test + public void hostConnectThrowsStorageConflictExceptionWhenLocalStorageAlreadyAdded() { + setUpPoolForConnect(Storage.StoragePoolType.NetworkFilesystem); + when(pool.isShared()).thenReturn(true); + when(agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(mspAnswer); + when(mspAnswer.getResult()).thenReturn(true); + when(mspAnswer.getLocalDatastoreName()).thenReturn("datastore1"); + StoragePoolVO conflictingPool = mock(StoragePoolVO.class); + when(conflictingPool.getPath()).thenReturn("datastore1"); + when(primaryStoreDao.listLocalStoragePoolByPath(1L, "datastore1")).thenReturn(Collections.singletonList(conflictingPool)); + + assertThrows(StorageConflictException.class, () -> listener.hostConnect(HOST_ID, POOL_ID)); + } + + @Test + public void hostConnectValidatesAndSyncsDatastoreClusterChildren() throws StorageConflictException { + setUpPoolForConnect(Storage.StoragePoolType.DatastoreCluster); + when(agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(mspAnswer); + when(mspAnswer.getResult()).thenReturn(true); + when(mspAnswer.getPoolInfo()).thenReturn(newPoolInfo()); + List children = Collections.singletonList(mock(ModifyStoragePoolAnswer.class)); + when(mspAnswer.getDatastoreClusterChildren()).thenReturn(children); + when(networkDao.getAllPersistentNetworksFromZone(anyLong())).thenReturn(Collections.emptyList()); + + boolean result = listener.hostConnect(HOST_ID, POOL_ID); + + assertTrue(result); + verify(storageManager).validateChildDatastoresToBeAddedInUpState(poolVO, children); + verify(storageManager).syncDatastoreClusterStoragePool(POOL_ID, children, HOST_ID); + } + + // Note: the CLVM secure-zero-fill detail-setting branch (ClvmPoolManager.isClvmPoolType(...) together + // with ClvmPoolManager.CLVMSecureZeroFill.valueIn(poolId)) is intentionally not covered here. + // CLVMSecureZeroFill is a static ConfigKey whose value() falls back to a process-wide static + // ConfigDepot (ConfigKey.s_depot) that may already have been initialised as a side effect of other + // tests running earlier in the same JVM/module test run, making the outcome of valueIn(...) + // order-dependent and awkward to control from a plain Mockito unit test without also mocking + // static state shared across the whole test module. + + // ---- hostDisconnected ---- + + @Test + public void hostDisconnectedReturnsFalseWhenHostNotFound() { + when(hostDao.findById(HOST_ID)).thenReturn(null); + + assertFalse(listener.hostDisconnected(HOST_ID, POOL_ID)); + } + + @Test + public void hostDisconnectedThrowsWhenDeleteCommandAnswerIsNull() { + when(hostDao.findById(HOST_ID)).thenReturn(host); + when(dataStoreMgr.getDataStore(POOL_ID, DataStoreRole.Primary)).thenReturn(pool); + when(host.getId()).thenReturn(HOST_ID); + when(agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(null); + + assertThrows(CloudRuntimeException.class, () -> listener.hostDisconnected(HOST_ID, POOL_ID)); + } + + @Test + public void hostDisconnectedLogsMessageContainingHostAndPoolAndReturnsFalseWhenAnswerFails() { + when(hostDao.findById(HOST_ID)).thenReturn(host); + when(host.getId()).thenReturn(HOST_ID); + when(host.toString()).thenReturn("Host {id=5, name=cs-kvm06}"); + when(dataStoreMgr.getDataStore(POOL_ID, DataStoreRole.Primary)).thenReturn(pool); + when(pool.toString()).thenReturn("Pool {id=10, name=primary1}"); + when(pool.getDataCenterId()).thenReturn(1L); + when(pool.getPodId()).thenReturn(2L); + Answer answer = mock(Answer.class); + when(answer.getResult()).thenReturn(false); + when(agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(answer); + + boolean result = listener.hostDisconnected(HOST_ID, POOL_ID); + + assertFalse(result); + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(String.class); + verify(alertMgr).sendAlert(eq(AlertManager.AlertType.ALERT_TYPE_HOST), eq(1L), eq(2L), messageCaptor.capture(), messageCaptor.capture()); + assertTrue(messageCaptor.getValue().contains("Host {id=5, name=cs-kvm06}")); + assertTrue(messageCaptor.getValue().contains("Pool {id=10, name=primary1}")); + } + + @Test + public void hostDisconnectedRemovesStoragePoolHostDetailsAndReturnsTrueOnSuccess() { + when(hostDao.findById(HOST_ID)).thenReturn(host); + when(host.getId()).thenReturn(HOST_ID); + when(dataStoreMgr.getDataStore(POOL_ID, DataStoreRole.Primary)).thenReturn(pool); + Answer answer = mock(Answer.class); + when(answer.getResult()).thenReturn(true); + when(agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(answer); + StoragePoolHostVO storagePoolHost = new StoragePoolHostVO(POOL_ID, HOST_ID, "/local/path"); + when(storagePoolHostDao.findByPoolHost(POOL_ID, HOST_ID)).thenReturn(storagePoolHost); + + boolean result = listener.hostDisconnected(HOST_ID, POOL_ID); + + assertTrue(result); + verify(storagePoolHostDao).deleteStoragePoolHostDetails(HOST_ID, POOL_ID); + } + + // ---- hostAboutToBeRemoved ---- + + @Test + public void hostAboutToBeRemovedReturnsFalseWhenHostNotFound() { + when(hostDao.findById(HOST_ID)).thenReturn(null); + + assertFalse(listener.hostAboutToBeRemoved(HOST_ID)); + } + + @Test + public void hostAboutToBeRemovedSkipsNetworkWhenAnswerIsNullAndStillReturnsTrue() { + when(hostDao.findById(HOST_ID)).thenReturn(host); + when(host.getDataCenterId()).thenReturn(1L); + NetworkVO network = mock(NetworkVO.class); + NetworkOfferingVO offering = mock(NetworkOfferingVO.class); + when(networkDao.getAllPersistentNetworksFromZone(1L)).thenReturn(Collections.singletonList(network)); + when(networkOfferingDao.findById(anyLong())).thenReturn(offering); + when(agentMgr.easySend(eq(HOST_ID), any(CleanupPersistentNetworkResourceCommand.class))).thenReturn(null); + + boolean result = listener.hostAboutToBeRemoved(HOST_ID); + + assertTrue(result); + verify(agentMgr).easySend(eq(HOST_ID), any(CleanupPersistentNetworkResourceCommand.class)); + } + + @Test + public void hostAboutToBeRemovedLogsErrorWhenAnswerFailsAndStillReturnsTrue() { + when(hostDao.findById(HOST_ID)).thenReturn(host); + when(host.getDataCenterId()).thenReturn(1L); + NetworkVO network = mock(NetworkVO.class); + NetworkOfferingVO offering = mock(NetworkOfferingVO.class); + when(networkDao.getAllPersistentNetworksFromZone(1L)).thenReturn(Collections.singletonList(network)); + when(networkOfferingDao.findById(anyLong())).thenReturn(offering); + Answer answer = mock(Answer.class); + when(answer.getResult()).thenReturn(false); + when(agentMgr.easySend(eq(HOST_ID), any(CleanupPersistentNetworkResourceCommand.class))).thenReturn(answer); + + boolean result = listener.hostAboutToBeRemoved(HOST_ID); + + assertTrue(result); + verify(agentMgr).easySend(eq(HOST_ID), any(CleanupPersistentNetworkResourceCommand.class)); + } + + // ---- hostEnabled ---- + + @Test + public void hostEnabledReturnsFalseWhenHostNotFound() { + when(hostDao.findById(HOST_ID)).thenReturn(null); + + assertFalse(listener.hostEnabled(HOST_ID)); + } + + @Test + public void hostEnabledSetsUpPersistentNetworkForEachPersistentNetwork() { + when(hostDao.findById(HOST_ID)).thenReturn(host); + when(host.getId()).thenReturn(HOST_ID); + when(host.getDataCenterId()).thenReturn(1L); + NetworkVO network1 = mock(NetworkVO.class); + NetworkVO network2 = mock(NetworkVO.class); + NetworkOfferingVO offering = mock(NetworkOfferingVO.class); + when(networkDao.getAllPersistentNetworksFromZone(1L)).thenReturn(java.util.Arrays.asList(network1, network2)); + when(networkOfferingDao.findById(anyLong())).thenReturn(offering); + Answer answer = mock(Answer.class); + when(answer.getResult()).thenReturn(true); + when(agentMgr.easySend(eq(HOST_ID), any(SetupPersistentNetworkCommand.class))).thenReturn(answer); + + boolean result = listener.hostEnabled(HOST_ID); + + assertTrue(result); + verify(agentMgr, times(2)).easySend(eq(HOST_ID), any(SetupPersistentNetworkCommand.class)); + } + + @Test + public void hostEnabledThrowsWhenSetupPersistentNetworkAnswerIsNull() { + when(hostDao.findById(HOST_ID)).thenReturn(host); + when(host.getId()).thenReturn(HOST_ID); + when(host.getDataCenterId()).thenReturn(1L); + NetworkVO network = mock(NetworkVO.class); + NetworkOfferingVO offering = mock(NetworkOfferingVO.class); + when(networkDao.getAllPersistentNetworksFromZone(1L)).thenReturn(Collections.singletonList(network)); + when(networkOfferingDao.findById(anyLong())).thenReturn(offering); + when(agentMgr.easySend(eq(HOST_ID), any(SetupPersistentNetworkCommand.class))).thenReturn(null); + + assertThrows(CloudRuntimeException.class, () -> listener.hostEnabled(HOST_ID)); + } +} diff --git a/plugins/storage/volume/datera/src/test/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListenerTest.java b/plugins/storage/volume/datera/src/test/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListenerTest.java new file mode 100644 index 000000000000..b9fa51cec5b8 --- /dev/null +++ b/plugins/storage/volume/datera/src/test/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListenerTest.java @@ -0,0 +1,567 @@ +/* + * 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 org.apache.cloudstack.storage.datastore.provider; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyLong; +import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.isNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; + +import java.util.Arrays; +import java.util.Collections; + +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.datastore.util.DateraObject; +import org.apache.cloudstack.storage.datastore.util.DateraUtil; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.ModifyStoragePoolAnswer; +import com.cloud.agent.api.ModifyStoragePoolCommand; +import com.cloud.agent.api.ModifyTargetsCommand; +import com.cloud.alert.AlertManager; +import com.cloud.dc.ClusterDetailsDao; +import com.cloud.dc.ClusterDetailsVO; +import com.cloud.dc.ClusterVO; +import com.cloud.dc.dao.ClusterDao; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; +import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.storage.DataStoreRole; +import com.cloud.storage.StoragePool; +import com.cloud.storage.StoragePoolHostVO; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.StoragePoolHostDao; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.utils.db.GlobalLock; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.dao.VMInstanceDao; + +@RunWith(MockitoJUnitRunner.class) +public class DateraHostListenerTest { + + @Mock + private AgentManager _agentMgr; + @Mock + private AlertManager _alertMgr; + @Mock + private ClusterDao _clusterDao; + @Mock + private ClusterDetailsDao _clusterDetailsDao; + @Mock + private DataStoreManager _dataStoreMgr; + @Mock + private HostDao _hostDao; + @Mock + private PrimaryDataStoreDao _storagePoolDao; + @Mock + private StoragePoolDetailsDao _storagePoolDetailsDao; + @Mock + private StoragePoolHostDao storagePoolHostDao; + @Mock + private VMInstanceDao _vmDao; + @Mock + private VolumeDao _volumeDao; + + private DateraHostListener listener; + + @Before + public void setup() { + listener = new DateraHostListener(); + + ReflectionTestUtils.setField(listener, "_agentMgr", _agentMgr); + ReflectionTestUtils.setField(listener, "_alertMgr", _alertMgr); + ReflectionTestUtils.setField(listener, "_clusterDao", _clusterDao); + ReflectionTestUtils.setField(listener, "_clusterDetailsDao", _clusterDetailsDao); + ReflectionTestUtils.setField(listener, "_dataStoreMgr", _dataStoreMgr); + ReflectionTestUtils.setField(listener, "_hostDao", _hostDao); + ReflectionTestUtils.setField(listener, "_storagePoolDao", _storagePoolDao); + ReflectionTestUtils.setField(listener, "_storagePoolDetailsDao", _storagePoolDetailsDao); + ReflectionTestUtils.setField(listener, "storagePoolHostDao", storagePoolHostDao); + ReflectionTestUtils.setField(listener, "_vmDao", _vmDao); + ReflectionTestUtils.setField(listener, "_volumeDao", _volumeDao); + } + + private StoragePool mockStoragePool() { + return mock(StoragePool.class, withSettings().extraInterfaces(DataStore.class)); + } + + // ---------- hostAdded ---------- + + @Test + public void hostAddedAlwaysReturnsTrue() { + assertTrue(listener.hostAdded(1L)); + } + + // ---------- hostConnect ---------- + + @Test + public void hostConnectReturnsFalseWhenHostNotFound() { + long hostId = 10L; + long storagePoolId = 100L; + + when(_hostDao.findById(hostId)).thenReturn(null); + + assertFalse(listener.hostConnect(hostId, storagePoolId)); + + verify(storagePoolHostDao, never()).persist(any(StoragePoolHostVO.class)); + } + + @Test + public void hostConnectPersistsNewStoragePoolHostWhenNoneExists() { + long hostId = 10L; + long storagePoolId = 100L; + + HostVO host = mock(HostVO.class); + when(host.getHypervisorType()).thenReturn(HypervisorType.Hyperv); + when(_hostDao.findById(hostId)).thenReturn(host); + when(storagePoolHostDao.findByPoolHost(storagePoolId, hostId)).thenReturn(null); + + assertTrue(listener.hostConnect(hostId, storagePoolId)); + + verify(storagePoolHostDao, times(1)).persist(any(StoragePoolHostVO.class)); + } + + @Test + public void hostConnectDoesNotPersistWhenStoragePoolHostAlreadyExists() { + long hostId = 10L; + long storagePoolId = 100L; + + HostVO host = mock(HostVO.class); + when(host.getHypervisorType()).thenReturn(HypervisorType.Hyperv); + when(_hostDao.findById(hostId)).thenReturn(host); + when(storagePoolHostDao.findByPoolHost(storagePoolId, hostId)).thenReturn(mock(StoragePoolHostVO.class)); + + assertTrue(listener.hostConnect(hostId, storagePoolId)); + + verify(storagePoolHostDao, never()).persist(any(StoragePoolHostVO.class)); + } + + @Test + public void hostConnectForXenServerSendsModifyStoragePoolCommandPerStoragePath() { + long hostId = 10L; + long storagePoolId = 100L; + long clusterId = 5L; + long vmHostId = 20L; + + HostVO host = mock(HostVO.class); + when(host.getId()).thenReturn(hostId); + when(host.getClusterId()).thenReturn(clusterId); + when(host.getHypervisorType()).thenReturn(HypervisorType.XenServer); + when(_hostDao.findById(hostId)).thenReturn(host); + when(storagePoolHostDao.findByPoolHost(storagePoolId, hostId)).thenReturn(null); + + StoragePool storagePool = mockStoragePool(); + when(_dataStoreMgr.getDataStore(storagePoolId, DataStoreRole.Primary)).thenReturn((DataStore) storagePool); + + VolumeVO volume1 = mock(VolumeVO.class); + when(volume1.getInstanceId()).thenReturn(1001L); + when(volume1.get_iScsiName()).thenReturn("iqn-1"); + + VolumeVO volume2 = mock(VolumeVO.class); + when(volume2.getInstanceId()).thenReturn(1002L); + when(volume2.get_iScsiName()).thenReturn("iqn-2"); + + when(_volumeDao.findNonDestroyedVolumesByPoolId(eq(storagePoolId), isNull())).thenReturn(Arrays.asList(volume1, volume2)); + + VMInstanceVO vm1 = mock(VMInstanceVO.class); + when(vm1.getHostId()).thenReturn(vmHostId); + when(_vmDao.findById(1001L)).thenReturn(vm1); + + VMInstanceVO vm2 = mock(VMInstanceVO.class); + when(vm2.getHostId()).thenReturn(vmHostId); + when(_vmDao.findById(1002L)).thenReturn(vm2); + + HostVO vmHost = mock(HostVO.class); + when(vmHost.getClusterId()).thenReturn(clusterId); + when(_hostDao.findById(vmHostId)).thenReturn(vmHost); + + when(_agentMgr.easySend(eq(hostId), any(ModifyStoragePoolCommand.class))) + .thenReturn(new ModifyStoragePoolAnswer(null, true, "ok")); + + assertTrue(listener.hostConnect(hostId, storagePoolId)); + + verify(_agentMgr, times(2)).easySend(eq(hostId), any(ModifyStoragePoolCommand.class)); + verify(_alertMgr, never()).sendAlert(any(), anyLong(), any(), anyString(), anyString()); + } + + @Test + public void hostConnectForXenServerSendsAlertUsingHostToStringWhenAnswerFails() { + long hostId = 10L; + long storagePoolId = 100L; + long clusterId = 5L; + long vmHostId = 20L; + + HostVO host = mock(HostVO.class); + when(host.getId()).thenReturn(hostId); + when(host.getClusterId()).thenReturn(clusterId); + when(host.getHypervisorType()).thenReturn(HypervisorType.XenServer); + when(host.toString()).thenReturn("Host {id=10, name=xen-01}"); + when(_hostDao.findById(hostId)).thenReturn(host); + when(storagePoolHostDao.findByPoolHost(storagePoolId, hostId)).thenReturn(null); + + StoragePool storagePool = mockStoragePool(); + when(_dataStoreMgr.getDataStore(storagePoolId, DataStoreRole.Primary)).thenReturn((DataStore) storagePool); + + VolumeVO volume = mock(VolumeVO.class); + when(volume.getInstanceId()).thenReturn(1001L); + when(volume.get_iScsiName()).thenReturn("iqn-1"); + when(_volumeDao.findNonDestroyedVolumesByPoolId(eq(storagePoolId), isNull())).thenReturn(Collections.singletonList(volume)); + + VMInstanceVO vm = mock(VMInstanceVO.class); + when(vm.getHostId()).thenReturn(vmHostId); + when(_vmDao.findById(1001L)).thenReturn(vm); + + HostVO vmHost = mock(HostVO.class); + when(vmHost.getClusterId()).thenReturn(clusterId); + when(_hostDao.findById(vmHostId)).thenReturn(vmHost); + + when(_agentMgr.easySend(eq(hostId), any(ModifyStoragePoolCommand.class))) + .thenReturn(new Answer(null, false, "failure")); + + try { + listener.hostConnect(hostId, storagePoolId); + fail("Expected a CloudRuntimeException to be thrown"); + } catch (CloudRuntimeException e) { + // expected + } + + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(String.class); + verify(_alertMgr).sendAlert(eq(AlertManager.AlertType.ALERT_TYPE_HOST), anyLong(), any(), messageCaptor.capture(), anyString()); + assertTrue(messageCaptor.getValue().contains("Host {id=10, name=xen-01}")); + } + + @Test + public void hostConnectForKvmSendsModifyStoragePoolCommand() { + long hostId = 10L; + long storagePoolId = 100L; + + HostVO host = mock(HostVO.class); + when(host.getHypervisorType()).thenReturn(HypervisorType.KVM); + when(_hostDao.findById(hostId)).thenReturn(host); + when(storagePoolHostDao.findByPoolHost(storagePoolId, hostId)).thenReturn(null); + + StoragePool storagePool = mockStoragePool(); + when(_dataStoreMgr.getDataStore(storagePoolId, DataStoreRole.Primary)).thenReturn((DataStore) storagePool); + + when(_agentMgr.easySend(eq(hostId), any(ModifyStoragePoolCommand.class))) + .thenReturn(new ModifyStoragePoolAnswer(null, true, "ok")); + + assertTrue(listener.hostConnect(hostId, storagePoolId)); + + verify(_agentMgr, times(1)).easySend(eq(hostId), any(ModifyStoragePoolCommand.class)); + // the 2-arg handleKVM overload does not consult volumes/VMs at all + verify(_volumeDao, never()).findNonDestroyedVolumesByPoolId(anyLong(), any()); + } + + // ---------- hostDisconnected ---------- + + @Test + public void hostDisconnectedDeletesDetailsWhenStoragePoolHostExists() { + long hostId = 10L; + long storagePoolId = 100L; + + when(storagePoolHostDao.findByPoolHost(storagePoolId, hostId)).thenReturn(mock(StoragePoolHostVO.class)); + + assertTrue(listener.hostDisconnected(hostId, storagePoolId)); + + verify(storagePoolHostDao, times(1)).deleteStoragePoolHostDetails(hostId, storagePoolId); + } + + @Test + public void hostDisconnectedDoesNothingWhenStoragePoolHostDoesNotExist() { + long hostId = 10L; + long storagePoolId = 100L; + + when(storagePoolHostDao.findByPoolHost(storagePoolId, hostId)).thenReturn(null); + + assertTrue(listener.hostDisconnected(hostId, storagePoolId)); + + verify(storagePoolHostDao, never()).deleteStoragePoolHostDetails(anyLong(), anyLong()); + } + + // ---------- hostAboutToBeRemoved ---------- + + @Test + public void hostAboutToBeRemovedForVmwareSendsModifyTargetsCommandWithAddFalse() { + long hostId = 10L; + long clusterId = 5L; + + HostVO host = mock(HostVO.class); + when(host.getId()).thenReturn(hostId); + when(host.getClusterId()).thenReturn(clusterId); + when(host.getHypervisorType()).thenReturn(HypervisorType.VMware); + when(_hostDao.findById(hostId)).thenReturn(host); + + StoragePoolVO storagePool = mock(StoragePoolVO.class); + when(storagePool.getId()).thenReturn(100L); + when(_storagePoolDao.findPoolsByProvider(DateraUtil.PROVIDER_NAME)).thenReturn(Collections.singletonList(storagePool)); + when(_storagePoolDao.findById(100L)).thenReturn(storagePool); + + when(_volumeDao.findNonDestroyedVolumesByPoolId(eq(100L), isNull())).thenReturn(Collections.emptyList()); + + when(_agentMgr.easySend(eq(hostId), any(ModifyTargetsCommand.class))) + .thenReturn(new Answer(null, true, "ok")); + + assertTrue(listener.hostAboutToBeRemoved(hostId)); + + ArgumentCaptor cmdCaptor = ArgumentCaptor.forClass(ModifyTargetsCommand.class); + verify(_agentMgr).easySend(eq(hostId), cmdCaptor.capture()); + assertFalse(cmdCaptor.getValue().getAdd()); + } + + @Test + public void hostAboutToBeRemovedForVmwareSendsAlertUsingHostToStringWhenAnswerFails() { + long hostId = 10L; + long clusterId = 5L; + + HostVO host = mock(HostVO.class); + when(host.getId()).thenReturn(hostId); + when(host.getClusterId()).thenReturn(clusterId); + when(host.getHypervisorType()).thenReturn(HypervisorType.VMware); + when(host.toString()).thenReturn("Host {id=10, name=vmware-01}"); + when(_hostDao.findById(hostId)).thenReturn(host); + + StoragePoolVO storagePool = mock(StoragePoolVO.class); + when(storagePool.getId()).thenReturn(100L); + when(_storagePoolDao.findPoolsByProvider(DateraUtil.PROVIDER_NAME)).thenReturn(Collections.singletonList(storagePool)); + when(_storagePoolDao.findById(100L)).thenReturn(storagePool); + + when(_volumeDao.findNonDestroyedVolumesByPoolId(eq(100L), isNull())).thenReturn(Collections.emptyList()); + + when(_agentMgr.easySend(eq(hostId), any(ModifyTargetsCommand.class))) + .thenReturn(new Answer(null, false, "failure")); + + try { + listener.hostAboutToBeRemoved(hostId); + fail("Expected a CloudRuntimeException to be thrown"); + } catch (CloudRuntimeException e) { + // expected + } + + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(String.class); + verify(_alertMgr).sendAlert(eq(AlertManager.AlertType.ALERT_TYPE_HOST), anyLong(), any(), messageCaptor.capture(), anyString()); + assertTrue(messageCaptor.getValue().contains("Host {id=10, name=vmware-01}")); + } + + @Test + public void hostAboutToBeRemovedForNonVmwareHostIsANoOpAndReturnsTrue() { + long hostId = 10L; + + HostVO host = mock(HostVO.class); + when(host.getHypervisorType()).thenReturn(HypervisorType.KVM); + when(_hostDao.findById(hostId)).thenReturn(host); + + assertTrue(listener.hostAboutToBeRemoved(hostId)); + + verify(_agentMgr, never()).easySend(anyLong(), any()); + verify(_storagePoolDao, never()).findPoolsByProvider(anyString()); + } + + // ---------- hostRemoved ---------- + // + // hostRemoved() acquires a GlobalLock.getInternLock(...) before doing any work. GlobalLock.lock() + // delegates to DbUtil.getGlobalLock(), which opens a real JDBC connection (TransactionLegacy.getStandaloneConnection()) + // - not viable/safe in a plain unit test. GlobalLock is a plain (non-final) class though, and its static + // factory method is just a lookup in an in-process map, so we mock the static factory itself + // (MockedStatic) to hand back a fully-mocked GlobalLock instance. This avoids ever touching the + // real lock()/unlock() implementation (and therefore the DB), while still exercising all of hostRemoved()'s + // own logic. + + @Test + public void hostRemovedReturnsTrueWhenNoStoragePoolsUseTheProvider() { + long hostId = 10L; + long clusterId = 5L; + + ClusterVO clusterVO = mock(ClusterVO.class); + when(clusterVO.getUuid()).thenReturn("cluster-uuid"); + when(_clusterDao.findById(clusterId)).thenReturn(clusterVO); + + HostVO hostVO = mock(HostVO.class); + when(hostVO.getUuid()).thenReturn("host-uuid"); + when(_hostDao.findByIdIncludingRemoved(hostId)).thenReturn(hostVO); + + when(_storagePoolDao.findPoolsByProvider(DateraUtil.PROVIDER_NAME)).thenReturn(Collections.emptyList()); + + GlobalLock lock = mock(GlobalLock.class); + when(lock.lock(5)).thenReturn(true); + + try (MockedStatic globalLockMock = mockStatic(GlobalLock.class)) { + globalLockMock.when(() -> GlobalLock.getInternLock("cluster-uuid")).thenReturn(lock); + + assertTrue(listener.hostRemoved(hostId, clusterId)); + } + + verify(lock).unlock(); + verify(lock).releaseRef(); + } + + @Test + public void hostRemovedReturnsTrueWhenNoInitiatorGroupIsConfiguredForCluster() { + long hostId = 10L; + long clusterId = 5L; + + ClusterVO clusterVO = mock(ClusterVO.class); + when(clusterVO.getUuid()).thenReturn("cluster-uuid"); + when(_clusterDao.findById(clusterId)).thenReturn(clusterVO); + + HostVO hostVO = mock(HostVO.class); + when(hostVO.getUuid()).thenReturn("host-uuid"); + when(_hostDao.findByIdIncludingRemoved(hostId)).thenReturn(hostVO); + + StoragePoolVO storagePool = mock(StoragePoolVO.class); + when(storagePool.getId()).thenReturn(100L); + when(_storagePoolDao.findPoolsByProvider(DateraUtil.PROVIDER_NAME)).thenReturn(Collections.singletonList(storagePool)); + + // no ClusterDetailsVO configured for the initiator group key -> clusterDetail is null + when(_clusterDetailsDao.findDetail(eq(clusterId), anyString())).thenReturn(null); + + GlobalLock lock = mock(GlobalLock.class); + when(lock.lock(5)).thenReturn(true); + + try (MockedStatic globalLockMock = mockStatic(GlobalLock.class)) { + globalLockMock.when(() -> GlobalLock.getInternLock("cluster-uuid")).thenReturn(lock); + + assertTrue(listener.hostRemoved(hostId, clusterId)); + } + + verify(lock).unlock(); + verify(lock).releaseRef(); + } + + @Test + public void hostRemovedRemovesInitiatorFromMatchingInitiatorGroup() throws Exception { + long hostId = 10L; + long clusterId = 5L; + long storagePoolId = 100L; + String initiatorGroupName = "CS-InitiatorGroup-1"; + + ClusterVO clusterVO = mock(ClusterVO.class); + when(clusterVO.getUuid()).thenReturn("cluster-uuid"); + when(_clusterDao.findById(clusterId)).thenReturn(clusterVO); + + HostVO hostVO = mock(HostVO.class); + when(hostVO.getUuid()).thenReturn("host-uuid"); + when(hostVO.getStorageUrl()).thenReturn("iqn.host"); + when(_hostDao.findByIdIncludingRemoved(hostId)).thenReturn(hostVO); + + StoragePoolVO storagePool = mock(StoragePoolVO.class); + when(storagePool.getId()).thenReturn(storagePoolId); + when(_storagePoolDao.findPoolsByProvider(DateraUtil.PROVIDER_NAME)).thenReturn(Collections.singletonList(storagePool)); + + // computed with the real DateraUtil implementation, before DateraUtil gets static-mocked below + String initiatorGroupKey = DateraUtil.getInitiatorGroupKey(storagePoolId); + + ClusterDetailsVO clusterDetail = mock(ClusterDetailsVO.class); + when(clusterDetail.getValue()).thenReturn(initiatorGroupName); + when(_clusterDetailsDao.findDetail(clusterId, initiatorGroupKey)).thenReturn(clusterDetail); + + DateraObject.DateraConnection connection = mock(DateraObject.DateraConnection.class); + DateraObject.Initiator initiator = mock(DateraObject.Initiator.class); + when(initiator.getPath()).thenReturn("/initiator-path"); + DateraObject.InitiatorGroup initiatorGroup = mock(DateraObject.InitiatorGroup.class); + + GlobalLock lock = mock(GlobalLock.class); + when(lock.lock(5)).thenReturn(true); + + // NOTE: no CALLS_REAL_METHODS default here - the real DateraUtil.removeInitiatorFromGroup() would go on + // to make a genuine HTTP call, so every static method reachable from hostRemoved() is stubbed explicitly. + try (MockedStatic globalLockMock = mockStatic(GlobalLock.class); + MockedStatic dateraUtilMock = mockStatic(DateraUtil.class)) { + + globalLockMock.when(() -> GlobalLock.getInternLock("cluster-uuid")).thenReturn(lock); + + dateraUtilMock.when(() -> DateraUtil.getInitiatorGroupKey(storagePoolId)).thenReturn(initiatorGroupKey); + dateraUtilMock.when(() -> DateraUtil.hostSupport_iScsi(hostVO)).thenReturn(true); + dateraUtilMock.when(() -> DateraUtil.getDateraConnection(storagePoolId, _storagePoolDetailsDao)).thenReturn(connection); + dateraUtilMock.when(() -> DateraUtil.getInitiator(connection, "iqn.host")).thenReturn(initiator); + dateraUtilMock.when(() -> DateraUtil.getInitiatorGroup(connection, initiatorGroupName)).thenReturn(initiatorGroup); + dateraUtilMock.when(() -> DateraUtil.isInitiatorPresentInGroup(initiator, initiatorGroup)).thenReturn(true); + + assertTrue(listener.hostRemoved(hostId, clusterId)); + + dateraUtilMock.verify(() -> DateraUtil.removeInitiatorFromGroup(connection, "/initiator-path", initiatorGroupName)); + } + + verify(lock).unlock(); + verify(lock).releaseRef(); + } + + @Test + public void hostRemovedThrowsWhenLockCannotBeAcquired() { + long hostId = 10L; + long clusterId = 5L; + + ClusterVO clusterVO = mock(ClusterVO.class); + when(clusterVO.getUuid()).thenReturn("cluster-uuid"); + when(_clusterDao.findById(clusterId)).thenReturn(clusterVO); + + HostVO hostVO = mock(HostVO.class); + when(hostVO.getUuid()).thenReturn("host-uuid"); + when(_hostDao.findByIdIncludingRemoved(hostId)).thenReturn(hostVO); + + GlobalLock lock = mock(GlobalLock.class); + when(lock.lock(5)).thenReturn(false); + + try (MockedStatic globalLockMock = mockStatic(GlobalLock.class)) { + globalLockMock.when(() -> GlobalLock.getInternLock("cluster-uuid")).thenReturn(lock); + + try { + listener.hostRemoved(hostId, clusterId); + fail("Expected a CloudRuntimeException to be thrown"); + } catch (CloudRuntimeException e) { + // expected + } + } + + verify(lock, never()).unlock(); + verify(lock, never()).releaseRef(); + } + + // ---------- hostEnabled ---------- + + @Test + public void hostEnabledAlwaysReturnsTrue() { + assertTrue(listener.hostEnabled(1L)); + } +} diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/listener/OntapHostListenerTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/listener/OntapHostListenerTest.java new file mode 100644 index 000000000000..3de3a71bd4b2 --- /dev/null +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/listener/OntapHostListenerTest.java @@ -0,0 +1,344 @@ +/* + * 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 org.apache.cloudstack.storage.listener; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyLong; +import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.HashMap; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.Command; +import com.cloud.agent.api.ModifyStoragePoolAnswer; +import com.cloud.agent.api.StoragePoolInfo; +import com.cloud.alert.AlertManager; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; +import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.storage.StoragePoolHostVO; +import com.cloud.storage.dao.StoragePoolHostDao; +import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; + +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +public class OntapHostListenerTest { + + private static final long HOST_ID = 1L; + private static final long POOL_ID = 2L; + private static final String LOCAL_PATH = "/mnt/ontap/vol1"; + + @Mock + private AgentManager _agentMgr; + @Mock + private AlertManager _alertMgr; + @Mock + private PrimaryDataStoreDao _storagePoolDao; + @Mock + private HostDao _hostDao; + @Mock + private StoragePoolHostDao storagePoolHostDao; + @Mock + private StoragePoolDetailsDao _storagePoolDetailsDao; + + @Mock + private HostVO host; + @Mock + private StoragePoolVO pool; + + private OntapHostListener listener; + + @BeforeEach + void setUp() { + listener = new OntapHostListener(); + ReflectionTestUtils.setField(listener, "_agentMgr", _agentMgr); + ReflectionTestUtils.setField(listener, "_alertMgr", _alertMgr); + ReflectionTestUtils.setField(listener, "_storagePoolDao", _storagePoolDao); + ReflectionTestUtils.setField(listener, "_hostDao", _hostDao); + ReflectionTestUtils.setField(listener, "storagePoolHostDao", storagePoolHostDao); + ReflectionTestUtils.setField(listener, "_storagePoolDetailsDao", _storagePoolDetailsDao); + } + + private void setupValidHostAndPool() { + when(_hostDao.findById(HOST_ID)).thenReturn(host); + when(host.getHypervisorType()).thenReturn(HypervisorType.KVM); + when(_storagePoolDao.findById(POOL_ID)).thenReturn(pool); + when(_storagePoolDetailsDao.listDetailsKeyPairs(POOL_ID)).thenReturn(new HashMap<>()); + } + + private ModifyStoragePoolAnswer mockSuccessfulAnswer(long capacityBytes, long availableBytes) { + StoragePoolInfo poolInfo = mock(StoragePoolInfo.class); + when(poolInfo.getLocalPath()).thenReturn(LOCAL_PATH); + when(poolInfo.getCapacityBytes()).thenReturn(capacityBytes); + when(poolInfo.getAvailableBytes()).thenReturn(availableBytes); + + ModifyStoragePoolAnswer answer = mock(ModifyStoragePoolAnswer.class); + when(answer.getResult()).thenReturn(true); + when(answer.getPoolInfo()).thenReturn(poolInfo); + return answer; + } + + // --------------------------------------------------------------- + // hostConnect + // --------------------------------------------------------------- + + @Test + public void hostConnectReturnsFalseWhenHostNotFound() { + when(_hostDao.findById(HOST_ID)).thenReturn(null); + + assertFalse(listener.hostConnect(HOST_ID, POOL_ID)); + + verify(_storagePoolDao, never()).findById(anyLong()); + } + + @Test + public void hostConnectReturnsFalseWhenHypervisorIsNotKvm() { + when(_hostDao.findById(HOST_ID)).thenReturn(host); + when(host.getHypervisorType()).thenReturn(HypervisorType.XenServer); + + assertFalse(listener.hostConnect(HOST_ID, POOL_ID)); + + verify(_storagePoolDao, never()).findById(anyLong()); + } + + @Test + public void hostConnectReturnsFalseWhenPoolNotFound() { + when(_hostDao.findById(HOST_ID)).thenReturn(host); + when(host.getHypervisorType()).thenReturn(HypervisorType.KVM); + when(_storagePoolDao.findById(POOL_ID)).thenReturn(null); + + assertFalse(listener.hostConnect(HOST_ID, POOL_ID)); + + verify(_agentMgr, never()).easySend(anyLong(), any(Command.class)); + } + + @Test + public void hostConnectPersistsNewStoragePoolHostAndUpdatesCapacityWhenNoExistingRef() { + setupValidHostAndPool(); + when(storagePoolHostDao.findByPoolHost(POOL_ID, HOST_ID)).thenReturn(null); + ModifyStoragePoolAnswer answer = mockSuccessfulAnswer(1000L, 400L); + when(_agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(answer); + + assertTrue(listener.hostConnect(HOST_ID, POOL_ID)); + + verify(storagePoolHostDao).persist(any(StoragePoolHostVO.class)); + verify(storagePoolHostDao, never()).update(anyLong(), any(StoragePoolHostVO.class)); + verify(pool).setCapacityBytes(1000L); + verify(pool).setUsedBytes(600L); + verify(_storagePoolDao).update(anyLong(), eq(pool)); + } + + @Test + public void hostConnectUpdatesExistingStoragePoolHostRef() { + setupValidHostAndPool(); + StoragePoolHostVO existing = mock(StoragePoolHostVO.class); + when(existing.getId()).thenReturn(5L); + when(storagePoolHostDao.findByPoolHost(POOL_ID, HOST_ID)).thenReturn(existing); + ModifyStoragePoolAnswer answer = mockSuccessfulAnswer(1000L, 400L); + when(_agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(answer); + + assertTrue(listener.hostConnect(HOST_ID, POOL_ID)); + + verify(existing).setLocalPath(LOCAL_PATH); + verify(storagePoolHostDao).update(eq(5L), eq(existing)); + verify(storagePoolHostDao, never()).persist(any(StoragePoolHostVO.class)); + } + + @Test + public void hostConnectDoesNotUpdatePoolCapacityWhenCapacityBytesIsZero() { + setupValidHostAndPool(); + when(storagePoolHostDao.findByPoolHost(POOL_ID, HOST_ID)).thenReturn(null); + ModifyStoragePoolAnswer answer = mockSuccessfulAnswer(0L, 0L); + when(_agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(answer); + + assertTrue(listener.hostConnect(HOST_ID, POOL_ID)); + + verify(pool, never()).setCapacityBytes(anyLong()); + verify(_storagePoolDao, never()).update(anyLong(), any(StoragePoolVO.class)); + } + + @Test + public void hostConnectReturnsFalseWhenAnswerIsNull() { + setupValidHostAndPool(); + when(_agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(null); + + assertFalse(listener.hostConnect(HOST_ID, POOL_ID)); + + verify(_alertMgr, never()).sendAlert(any(), anyLong(), any(), anyString(), anyString()); + } + + @Test + public void hostConnectSendsAlertContainingHostWhenAnswerResultIsFalse() { + setupValidHostAndPool(); + when(host.toString()).thenReturn("Host {id=1, name=kvm-host-1}"); + ModifyStoragePoolAnswer answer = mock(ModifyStoragePoolAnswer.class); + when(answer.getResult()).thenReturn(false); + when(answer.getDetails()).thenReturn("agent could not mount volume"); + when(_agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(answer); + + assertFalse(listener.hostConnect(HOST_ID, POOL_ID)); + + ArgumentCaptor subjectCaptor = ArgumentCaptor.forClass(String.class); + verify(_alertMgr).sendAlert(eq(AlertManager.AlertType.ALERT_TYPE_HOST), anyLong(), any(), + subjectCaptor.capture(), anyString()); + assertTrue(subjectCaptor.getValue().contains("Host {id=1, name=kvm-host-1}")); + } + + @Test + public void hostConnectReturnsFalseWhenAnswerIsNotModifyStoragePoolAnswer() { + setupValidHostAndPool(); + Answer answer = mock(Answer.class); + when(answer.getResult()).thenReturn(true); + when(_agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(answer); + + assertFalse(listener.hostConnect(HOST_ID, POOL_ID)); + + verify(_alertMgr, never()).sendAlert(any(), anyLong(), any(), anyString(), anyString()); + } + + @Test + public void hostConnectReturnsFalseWhenPoolInfoIsNull() { + setupValidHostAndPool(); + ModifyStoragePoolAnswer answer = mock(ModifyStoragePoolAnswer.class); + when(answer.getResult()).thenReturn(true); + when(answer.getPoolInfo()).thenReturn(null); + when(_agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(answer); + + assertFalse(listener.hostConnect(HOST_ID, POOL_ID)); + + verify(storagePoolHostDao, never()).persist(any(StoragePoolHostVO.class)); + } + + @Test + public void hostConnectReturnsFalseWhenAgentThrowsException() { + setupValidHostAndPool(); + when(_agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenThrow(new CloudRuntimeException("agent unreachable")); + + assertFalse(listener.hostConnect(HOST_ID, POOL_ID)); + } + + // --------------------------------------------------------------- + // hostDisconnected + // --------------------------------------------------------------- + + @Test + public void hostDisconnectedReturnsFalseWhenHostNotFound() { + when(_hostDao.findById(HOST_ID)).thenReturn(null); + + assertFalse(listener.hostDisconnected(HOST_ID, POOL_ID)); + + verify(_storagePoolDao, never()).findById(anyLong()); + } + + @Test + public void hostDisconnectedReturnsFalseWhenPoolNotFound() { + when(_hostDao.findById(HOST_ID)).thenReturn(host); + when(_storagePoolDao.findById(POOL_ID)).thenReturn(null); + + assertFalse(listener.hostDisconnected(HOST_ID, POOL_ID)); + + verify(_agentMgr, never()).easySend(anyLong(), any(Command.class)); + } + + @Test + public void hostDisconnectedReturnsTrueOnSuccessfulAnswer() { + when(_hostDao.findById(HOST_ID)).thenReturn(host); + when(_storagePoolDao.findById(POOL_ID)).thenReturn(pool); + Answer answer = mock(Answer.class); + when(answer.getResult()).thenReturn(true); + when(_agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(answer); + + assertTrue(listener.hostDisconnected(HOST_ID, POOL_ID)); + } + + @Test + public void hostDisconnectedReturnsFalseWhenAnswerIsNull() { + when(_hostDao.findById(HOST_ID)).thenReturn(host); + when(_storagePoolDao.findById(POOL_ID)).thenReturn(pool); + when(_agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(null); + + assertFalse(listener.hostDisconnected(HOST_ID, POOL_ID)); + } + + @Test + public void hostDisconnectedReturnsFalseWhenAnswerResultIsFalse() { + when(_hostDao.findById(HOST_ID)).thenReturn(host); + when(_storagePoolDao.findById(POOL_ID)).thenReturn(pool); + Answer answer = mock(Answer.class); + when(answer.getResult()).thenReturn(false); + when(answer.getDetails()).thenReturn("failed to unmount"); + when(_agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenReturn(answer); + + assertFalse(listener.hostDisconnected(HOST_ID, POOL_ID)); + } + + @Test + public void hostDisconnectedReturnsFalseWhenAgentThrowsException() { + when(_hostDao.findById(HOST_ID)).thenReturn(host); + when(_storagePoolDao.findById(POOL_ID)).thenReturn(pool); + when(_agentMgr.easySend(eq(HOST_ID), any(Command.class))).thenThrow(new CloudRuntimeException("agent unreachable")); + + assertFalse(listener.hostDisconnected(HOST_ID, POOL_ID)); + } + + // --------------------------------------------------------------- + // Trivial no-op overrides + // --------------------------------------------------------------- + + @Test + public void hostAboutToBeRemovedAlwaysReturnsFalse() { + assertFalse(listener.hostAboutToBeRemoved(HOST_ID)); + } + + @Test + public void hostRemovedAlwaysReturnsFalse() { + assertFalse(listener.hostRemoved(HOST_ID, 99L)); + } + + @Test + public void hostEnabledAlwaysReturnsFalse() { + assertFalse(listener.hostEnabled(HOST_ID)); + } + + @Test + public void hostAddedAlwaysReturnsFalse() { + assertFalse(listener.hostAdded(HOST_ID)); + } +} diff --git a/server/src/test/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImplTest.java b/server/src/test/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImplTest.java new file mode 100644 index 000000000000..5934ec07c03f --- /dev/null +++ b/server/src/test/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImplTest.java @@ -0,0 +1,470 @@ +// 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 org.apache.cloudstack.outofbandmanagement; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Collections; +import java.util.concurrent.Executors; + +import org.apache.cloudstack.api.response.OutOfBandManagementResponse; +import org.apache.cloudstack.outofbandmanagement.dao.OutOfBandManagementDao; +import org.apache.cloudstack.poll.BackgroundPollManager; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.alert.AlertManager; +import com.cloud.dc.ClusterDetailsDao; +import com.cloud.dc.ClusterDetailsVO; +import com.cloud.dc.DataCenter; +import com.cloud.dc.DataCenterDetailVO; +import com.cloud.dc.dao.ClusterDao; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.DataCenterDetailsDao; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; +import com.cloud.org.Cluster; +import com.cloud.resource.ResourceState; +import com.cloud.utils.exception.CloudRuntimeException; +import com.google.common.cache.CacheBuilder; +import com.google.common.collect.ImmutableMap; + +@RunWith(MockitoJUnitRunner.class) +public class OutOfBandManagementServiceImplTest { + + private static final String OOBM_ENABLED_DETAIL = "outOfBandManagementEnabled"; + + @Mock + private ClusterDao clusterDao; + @Mock + private ClusterDetailsDao clusterDetailsDao; + @Mock + private DataCenterDao dataCenterDao; + @Mock + private DataCenterDetailsDao dataCenterDetailsDao; + @Mock + private OutOfBandManagementDao outOfBandManagementDao; + @Mock + private HostDao hostDao; + @Mock + private AlertManager alertMgr; + @Mock + private BackgroundPollManager backgroundPollManager; + + @Mock + private HostVO host; + @Mock + private DataCenter zone; + @Mock + private Cluster cluster; + + private OutOfBandManagementServiceImpl service; + + @BeforeClass + public static void setUpStaticFields() throws Exception { + Field cacheField = OutOfBandManagementServiceImpl.class.getDeclaredField("hostAlertCache"); + cacheField.setAccessible(true); + cacheField.set(null, CacheBuilder.newBuilder().build()); + + Field executorField = OutOfBandManagementServiceImpl.class.getDeclaredField("backgroundSyncBlockingExecutor"); + executorField.setAccessible(true); + executorField.set(null, Executors.newSingleThreadExecutor()); + } + + @Before + public void setUp() { + service = new OutOfBandManagementServiceImpl(); + ReflectionTestUtils.setField(service, "clusterDao", clusterDao); + ReflectionTestUtils.setField(service, "clusterDetailsDao", clusterDetailsDao); + ReflectionTestUtils.setField(service, "dataCenterDao", dataCenterDao); + ReflectionTestUtils.setField(service, "dataCenterDetailsDao", dataCenterDetailsDao); + ReflectionTestUtils.setField(service, "outOfBandManagementDao", outOfBandManagementDao); + ReflectionTestUtils.setField(service, "hostDao", hostDao); + ReflectionTestUtils.setField(service, "alertMgr", alertMgr); + ReflectionTestUtils.setField(service, "backgroundPollManager", backgroundPollManager); + } + + private boolean invokeIsOutOfBandManagementEnabledForHost(Long hostId) throws Exception { + Method m = OutOfBandManagementServiceImpl.class.getDeclaredMethod("isOutOfBandManagementEnabledForHost", Long.class); + m.setAccessible(true); + return (boolean) m.invoke(service, hostId); + } + + // ---------- isOutOfBandManagementEnabled(Host) and private helpers ---------- + + @Test + public void isOutOfBandManagementEnabledReturnsFalseForNullHost() { + assertFalse(service.isOutOfBandManagementEnabled(null)); + } + + @Test + public void isOutOfBandManagementEnabledReturnsFalseWhenZoneDisabled() { + when(host.getDataCenterId()).thenReturn(1L); + when(dataCenterDetailsDao.findDetail(1L, OOBM_ENABLED_DETAIL)).thenReturn(new DataCenterDetailVO(1L, OOBM_ENABLED_DETAIL, "false", true)); + + assertFalse(service.isOutOfBandManagementEnabled(host)); + verify(clusterDetailsDao, never()).findDetail(anyLong(), any()); + } + + @Test + public void isOutOfBandManagementEnabledReturnsFalseWhenClusterDisabled() { + when(host.getDataCenterId()).thenReturn(1L); + when(host.getClusterId()).thenReturn(2L); + when(dataCenterDetailsDao.findDetail(1L, OOBM_ENABLED_DETAIL)).thenReturn(null); + when(clusterDetailsDao.findDetail(2L, OOBM_ENABLED_DETAIL)).thenReturn(new ClusterDetailsVO(2L, OOBM_ENABLED_DETAIL, "false")); + + assertFalse(service.isOutOfBandManagementEnabled(host)); + verify(hostDao, never()).findById(anyLong()); + } + + @Test + public void isOutOfBandManagementEnabledReturnsFalseWhenHostIsDegraded() { + when(host.getDataCenterId()).thenReturn(1L); + when(host.getClusterId()).thenReturn(2L); + when(host.getId()).thenReturn(3L); + when(dataCenterDetailsDao.findDetail(1L, OOBM_ENABLED_DETAIL)).thenReturn(null); + when(clusterDetailsDao.findDetail(2L, OOBM_ENABLED_DETAIL)).thenReturn(null); + when(hostDao.findById(3L)).thenReturn(host); + when(host.getResourceState()).thenReturn(ResourceState.Degraded); + + assertFalse(service.isOutOfBandManagementEnabled(host)); + } + + @Test + public void isOutOfBandManagementEnabledReturnsFalseWhenHostWasRemoved() { + when(host.getDataCenterId()).thenReturn(1L); + when(host.getClusterId()).thenReturn(2L); + when(host.getId()).thenReturn(3L); + when(dataCenterDetailsDao.findDetail(1L, OOBM_ENABLED_DETAIL)).thenReturn(null); + when(clusterDetailsDao.findDetail(2L, OOBM_ENABLED_DETAIL)).thenReturn(null); + when(hostDao.findById(3L)).thenReturn(null); + + assertFalse(service.isOutOfBandManagementEnabled(host)); + } + + @Test + public void isOutOfBandManagementEnabledReturnsFalseWhenNoConfig() { + when(host.getDataCenterId()).thenReturn(1L); + when(host.getClusterId()).thenReturn(2L); + when(host.getId()).thenReturn(3L); + when(dataCenterDetailsDao.findDetail(1L, OOBM_ENABLED_DETAIL)).thenReturn(null); + when(clusterDetailsDao.findDetail(2L, OOBM_ENABLED_DETAIL)).thenReturn(null); + when(hostDao.findById(3L)).thenReturn(host); + when(host.getResourceState()).thenReturn(ResourceState.Enabled); + when(outOfBandManagementDao.findByHost(3L)).thenReturn(null); + + assertFalse(service.isOutOfBandManagementEnabled(host)); + } + + @Test + public void isOutOfBandManagementEnabledReturnsFalseWhenConfigDisabled() { + when(host.getDataCenterId()).thenReturn(1L); + when(host.getClusterId()).thenReturn(2L); + when(host.getId()).thenReturn(3L); + when(dataCenterDetailsDao.findDetail(1L, OOBM_ENABLED_DETAIL)).thenReturn(null); + when(clusterDetailsDao.findDetail(2L, OOBM_ENABLED_DETAIL)).thenReturn(null); + when(hostDao.findById(3L)).thenReturn(host); + when(host.getResourceState()).thenReturn(ResourceState.Enabled); + OutOfBandManagementVO config = new OutOfBandManagementVO(3L); + config.setEnabled(false); + when(outOfBandManagementDao.findByHost(3L)).thenReturn(config); + + assertFalse(service.isOutOfBandManagementEnabled(host)); + } + + @Test + public void isOutOfBandManagementEnabledReturnsTrueWhenFullyEnabled() { + when(host.getDataCenterId()).thenReturn(1L); + when(host.getClusterId()).thenReturn(2L); + when(host.getId()).thenReturn(3L); + when(dataCenterDetailsDao.findDetail(1L, OOBM_ENABLED_DETAIL)).thenReturn(new DataCenterDetailVO(1L, OOBM_ENABLED_DETAIL, "true", true)); + when(clusterDetailsDao.findDetail(2L, OOBM_ENABLED_DETAIL)).thenReturn(new ClusterDetailsVO(2L, OOBM_ENABLED_DETAIL, "true")); + when(hostDao.findById(3L)).thenReturn(host); + when(host.getResourceState()).thenReturn(ResourceState.Enabled); + OutOfBandManagementVO config = new OutOfBandManagementVO(3L); + config.setEnabled(true); + when(outOfBandManagementDao.findByHost(3L)).thenReturn(config); + + assertTrue(service.isOutOfBandManagementEnabled(host)); + } + + @Test + public void isOutOfBandManagementEnabledForHostReturnsFalseForNullHostId() throws Exception { + assertFalse(invokeIsOutOfBandManagementEnabledForHost(null)); + } + + // ---------- enable/disable on a zone ---------- + + @Test + public void enableOutOfBandManagementZonePersistsEnabledDetail() { + when(zone.getId()).thenReturn(10L); + + OutOfBandManagementResponse response = service.enableOutOfBandManagement(zone); + + verify(dataCenterDetailsDao).persist(10L, OOBM_ENABLED_DETAIL, String.valueOf(true)); + assertTrue(response.getEnabled()); + assertTrue(response.getSuccess()); + } + + @Test + public void disableOutOfBandManagementZonePersistsDisabledDetailAndTransitionsHosts() { + when(zone.getId()).thenReturn(10L); + when(hostDao.listIdsByDataCenterId(10L)).thenReturn(Arrays.asList(1L, 2L)); + // use a state from which a real "Disabled" transition exists (On/Off/Unknown) so that + // transitionPowerStateToDisabled()'s short-circuiting "result = result && transitionPowerState(...)" + // does not skip the second host once the first one is processed. + OutOfBandManagementVO config1 = mock(OutOfBandManagementVO.class); + when(config1.getState()).thenReturn(OutOfBandManagement.PowerState.On); + when(config1.getHostId()).thenReturn(1L); + OutOfBandManagementVO config2 = mock(OutOfBandManagementVO.class); + when(config2.getState()).thenReturn(OutOfBandManagement.PowerState.On); + when(config2.getHostId()).thenReturn(2L); + when(outOfBandManagementDao.findByHost(1L)).thenReturn(config1); + when(outOfBandManagementDao.findByHost(2L)).thenReturn(config2); + when(outOfBandManagementDao.updateState(any(), any(), any(), any(), any())).thenReturn(true); + + OutOfBandManagementResponse response = service.disableOutOfBandManagement(zone); + + verify(dataCenterDetailsDao).persist(10L, OOBM_ENABLED_DETAIL, String.valueOf(false)); + verify(outOfBandManagementDao).findByHost(1L); + verify(outOfBandManagementDao).findByHost(2L); + assertFalse(response.getEnabled()); + assertTrue(response.getSuccess()); + } + + // ---------- enable/disable on a cluster ---------- + + @Test + public void enableOutOfBandManagementClusterPersistsEnabledDetail() { + when(cluster.getId()).thenReturn(20L); + + OutOfBandManagementResponse response = service.enableOutOfBandManagement(cluster); + + verify(clusterDetailsDao).persist(20L, OOBM_ENABLED_DETAIL, String.valueOf(true)); + assertTrue(response.getEnabled()); + assertTrue(response.getSuccess()); + } + + @Test + public void disableOutOfBandManagementClusterPersistsDisabledDetailAndTransitionsHosts() { + when(cluster.getId()).thenReturn(20L); + when(hostDao.listIdsByClusterId(20L)).thenReturn(Collections.singletonList(5L)); + OutOfBandManagementVO config = mock(OutOfBandManagementVO.class); + when(config.getState()).thenReturn(OutOfBandManagement.PowerState.On); + when(config.getHostId()).thenReturn(5L); + when(outOfBandManagementDao.findByHost(5L)).thenReturn(config); + when(outOfBandManagementDao.updateState(any(), any(), any(), any(), any())).thenReturn(true); + + OutOfBandManagementResponse response = service.disableOutOfBandManagement(cluster); + + verify(clusterDetailsDao).persist(20L, OOBM_ENABLED_DETAIL, String.valueOf(false)); + verify(outOfBandManagementDao).findByHost(5L); + assertFalse(response.getEnabled()); + assertTrue(response.getSuccess()); + } + + // ---------- enable/disable on a host (exercises static hostAlertCache) ---------- + + @Test + public void enableOutOfBandManagementHostThrowsWhenNoConfig() { + when(host.getId()).thenReturn(5L); + when(outOfBandManagementDao.findByHost(5L)).thenReturn(null); + + assertThrows(CloudRuntimeException.class, () -> service.enableOutOfBandManagement(host)); + } + + @Test + public void disableOutOfBandManagementHostThrowsWhenNoConfig() { + when(host.getId()).thenReturn(5L); + when(outOfBandManagementDao.findByHost(5L)).thenReturn(null); + + assertThrows(CloudRuntimeException.class, () -> service.disableOutOfBandManagement(host)); + } + + @Test + public void enableOutOfBandManagementHostInvalidatesCacheAndPersists() { + when(host.getId()).thenReturn(5L); + OutOfBandManagementVO config = new OutOfBandManagementVO(5L); + config.setEnabled(false); + when(outOfBandManagementDao.findByHost(5L)).thenReturn(config); + when(outOfBandManagementDao.update(anyLong(), any(OutOfBandManagementVO.class))).thenReturn(true); + + OutOfBandManagementResponse response = service.enableOutOfBandManagement(host); + + assertTrue(config.isEnabled()); + verify(outOfBandManagementDao).update(anyLong(), eq(config)); + // called once from getConfigForHost() and again from transitionPowerStateToDisabled() + verify(outOfBandManagementDao, times(2)).findByHost(5L); + assertTrue(response.getEnabled()); + assertTrue(response.getSuccess()); + } + + @Test + public void disableOutOfBandManagementHostInvalidatesCacheAndPersists() { + when(host.getId()).thenReturn(5L); + OutOfBandManagementVO config = new OutOfBandManagementVO(5L); + config.setEnabled(true); + when(outOfBandManagementDao.findByHost(5L)).thenReturn(config); + when(outOfBandManagementDao.update(anyLong(), any(OutOfBandManagementVO.class))).thenReturn(true); + + OutOfBandManagementResponse response = service.disableOutOfBandManagement(host); + + assertFalse(config.isEnabled()); + verify(outOfBandManagementDao).update(anyLong(), eq(config)); + verify(outOfBandManagementDao, times(2)).findByHost(5L); + assertFalse(response.getEnabled()); + assertTrue(response.getSuccess()); + } + + @Test + public void enableOutOfBandManagementHostSkipsTransitionWhenUpdateFails() { + when(host.getId()).thenReturn(5L); + OutOfBandManagementVO config = new OutOfBandManagementVO(5L); + when(outOfBandManagementDao.findByHost(5L)).thenReturn(config); + when(outOfBandManagementDao.update(anyLong(), any(OutOfBandManagementVO.class))).thenReturn(false); + + service.enableOutOfBandManagement(host); + + // only the initial getConfigForHost() lookup, no transitionPowerStateToDisabled() lookup + verify(outOfBandManagementDao, times(1)).findByHost(5L); + } + + // ---------- configure(Host, options) ---------- + + @Test + public void configureHostCreatesNewConfigWhenNoneExists() { + OutOfBandManagementDriver driver = mock(OutOfBandManagementDriver.class); + when(driver.getName()).thenReturn("ipmitool"); + service.setOutOfBandManagementDrivers(Collections.singletonList(driver)); + service.start(); + + when(host.getId()).thenReturn(5L); + OutOfBandManagementVO persisted = new OutOfBandManagementVO(5L); + when(outOfBandManagementDao.findByHost(5L)).thenReturn(null, persisted); + when(outOfBandManagementDao.persist(any(OutOfBandManagementVO.class))).thenReturn(persisted); + when(outOfBandManagementDao.update(anyLong(), any(OutOfBandManagementVO.class))).thenReturn(true); + + ImmutableMap options = ImmutableMap.of( + OutOfBandManagement.Option.DRIVER, "ipmitool", + OutOfBandManagement.Option.ADDRESS, "1.2.3.4"); + + OutOfBandManagementResponse response = service.configure(host, options); + + verify(outOfBandManagementDao).persist(any(OutOfBandManagementVO.class)); + assertEquals("ipmitool", persisted.getDriver()); + assertEquals("1.2.3.4", persisted.getAddress()); + assertTrue(response.getSuccess()); + } + + @Test + public void configureHostUpdatesExistingConfig() { + OutOfBandManagementDriver driver = mock(OutOfBandManagementDriver.class); + when(driver.getName()).thenReturn("ipmitool"); + service.setOutOfBandManagementDrivers(Collections.singletonList(driver)); + service.start(); + + when(host.getId()).thenReturn(5L); + OutOfBandManagementVO existing = new OutOfBandManagementVO(5L); + existing.setDriver("ipmitool"); + when(outOfBandManagementDao.findByHost(5L)).thenReturn(existing); + when(outOfBandManagementDao.update(anyLong(), any(OutOfBandManagementVO.class))).thenReturn(true); + + ImmutableMap options = ImmutableMap.of(OutOfBandManagement.Option.ADDRESS, "5.6.7.8"); + + OutOfBandManagementResponse response = service.configure(host, options); + + verify(outOfBandManagementDao, never()).persist(any(OutOfBandManagementVO.class)); + verify(outOfBandManagementDao).update(anyLong(), eq(existing)); + assertEquals("5.6.7.8", existing.getAddress()); + assertTrue(response.getSuccess()); + } + + @Test + public void configureHostThrowsWhenDriverMissingOrInvalid() { + when(host.getId()).thenReturn(5L); + OutOfBandManagementVO existing = new OutOfBandManagementVO(5L); + when(outOfBandManagementDao.findByHost(5L)).thenReturn(existing); + + ImmutableMap options = ImmutableMap.of(OutOfBandManagement.Option.DRIVER, "no-such-driver"); + + assertThrows(CloudRuntimeException.class, () -> service.configure(host, options)); + verify(outOfBandManagementDao, never()).update(anyLong(), any(OutOfBandManagementVO.class)); + } + + @Test + public void configureHostThrowsWhenUpdateFails() { + OutOfBandManagementDriver driver = mock(OutOfBandManagementDriver.class); + when(driver.getName()).thenReturn("ipmitool"); + service.setOutOfBandManagementDrivers(Collections.singletonList(driver)); + service.start(); + + when(host.getId()).thenReturn(5L); + OutOfBandManagementVO existing = new OutOfBandManagementVO(5L); + existing.setDriver("ipmitool"); + when(outOfBandManagementDao.findByHost(5L)).thenReturn(existing); + when(outOfBandManagementDao.update(anyLong(), any(OutOfBandManagementVO.class))).thenReturn(false); + + ImmutableMap options = ImmutableMap.of(); + + assertThrows(CloudRuntimeException.class, () -> service.configure(host, options)); + } + + // ---------- trivial getters/setters ---------- + + @Test + public void getNameReturnsConfiguredName() { + ReflectionTestUtils.setField(service, "name", "OutOfBandManagementService"); + assertEquals("OutOfBandManagementService", service.getName()); + } + + @Test + public void getIdReturnsConfiguredServiceId() { + ReflectionTestUtils.setField(service, "serviceId", 99L); + assertEquals(99L, service.getId()); + } + + @Test + public void getConfigComponentNameReturnsSimpleClassName() { + assertEquals("OutOfBandManagementServiceImpl", service.getConfigComponentName()); + } + + @Test + public void getConfigKeysReturnsAllConfigKeys() { + assertNotNull(service.getConfigKeys()); + assertEquals(3, service.getConfigKeys().length); + } +}