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..c3b8e7527445 --- /dev/null +++ b/engine/components-api/src/main/java/com/cloud/alert/AlertFormatUtils.java @@ -0,0 +1,47 @@ +// 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 static final String UNKNOWN = "unknown"; + + private AlertFormatUtils() { + } + + public static String describeHostLocation(Host host, DataCenter zone, Pod pod) { + if (host == null) { + // we should never get here, but if we do, at least we won't get an NPE + return String.format("No host to describe for availability zone: %s, pod: %s", + zone != null ? zone.getName() : UNKNOWN, + pod != null ? pod.getName() : UNKNOWN); + } + return String.format("name: %s (id: %d, uuid: %s), availability zone: %s, pod: %s", + host.getName(), host.getId(), host.getUuid(), + zone != null ? zone.getName() : UNKNOWN, + pod != null ? pod.getName() : UNKNOWN); + } +} diff --git a/engine/components-api/src/test/java/com/cloud/alert/AlertFormatUtilsTest.java b/engine/components-api/src/test/java/com/cloud/alert/AlertFormatUtilsTest.java new file mode 100644 index 000000000000..89dcf84797d0 --- /dev/null +++ b/engine/components-api/src/test/java/com/cloud/alert/AlertFormatUtilsTest.java @@ -0,0 +1,111 @@ +// 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); + } + + @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); + when(host.getUuid()).thenReturn("host-uuid"); + } + + private void setUpZone() { + when(zone.getName()).thenReturn("Milton1"); + } + + private void setUpPod() { + when(pod.getName()).thenReturn("Milton1-Pod1"); + } +} diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java index 1215829d92f8..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 @@ -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; @@ -91,6 +92,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 +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.getId(), 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) { @@ -1173,9 +1175,9 @@ 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, + _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; @@ -1184,12 +1186,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); - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, + final String hostDesc = AlertFormatUtils.describeHostLocation(host, dcVO, podVO); + _alertMgr.sendAlert(AlertService.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); @@ -1704,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); @@ -2006,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"); } } diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 84a397349cec..dc7d90edecba 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -82,6 +82,7 @@ import com.cloud.agent.api.routing.NetworkElementCommand; import com.cloud.agent.api.to.NicTO; import com.cloud.agent.api.to.deployasis.OVFNetworkTO; +import com.cloud.alert.AlertFormatUtils; import com.cloud.alert.AlertManager; import com.cloud.api.query.dao.DomainRouterJoinDao; import com.cloud.api.query.vo.DomainRouterJoinVO; @@ -4497,7 +4498,8 @@ public void processConnect(final Host host, final StartupCommand cmd, final bool if (!answer.getResult()) { logger.warn("Unable to setup agent {} due to {}", host, answer.getDetails()); - final String msg = "Incorrect Network setup on agent, Reinitialize agent after network names are setup, details : " + answer.getDetails(); + final String msg = "Incorrect Network setup on agent " + AlertFormatUtils.describeHostLocation(host, dc, null) + + ", Reinitialize agent after network names are setup, details : " + answer.getDetails(); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, dcId, host.getPodId(), msg, msg); throw new ConnectionException(true, msg); } else { diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java index aced750bd320..98cb5fc616c1 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java @@ -257,7 +257,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed); String subject = "Take snapshot failed for Instance: " + userVm.getDisplayName(); - String message = "Snapshot operation failed for Instance: " + userVm.getDisplayName() + ", Please check and delete if any stale volumes created with Instance Snapshot id: " + vmSnapshot.getVmId(); + String message = "Snapshot operation failed for Instance: " + userVm.getDisplayName() + ", Please check and delete if any stale volumes created with " + vmSnapshot; alertManager.sendAlert(AlertManager.AlertType.ALERT_TYPE_VM_SNAPSHOT, userVm.getDataCenterId(), userVm.getPodIdToDeployIn(), subject, message); } catch (NoTransitionException e1) { logger.error("Cannot set Instance Snapshot state due to: " + e1.getMessage()); diff --git a/engine/storage/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/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 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/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java index 7644d4688f7e..4047eca58d32 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java @@ -163,7 +163,7 @@ public boolean hostConnect(long hostId, long poolId) throws StorageConflictExcep } if (!answer.getResult()) { - String msg = String.format("Unable to attach storage pool %s to the host %d", pool, hostId); + String msg = String.format("Unable to attach storage pool %s to the host %s", pool, host); alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, pool.getDataCenterId(), pool.getPodId(), msg, msg); throw new CloudRuntimeException(String.format("Unable to establish connection from storage head to storage pool %s due to %s %s", pool, answer.getDetails(), pool.getUuid())); 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/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/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/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/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/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..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; @@ -97,7 +98,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); @@ -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(AlertService.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(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: " + 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(AlertService.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: {} and host: {}", storagePool, 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: {} and host: {}", storagePool, host); return true; } 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/HighAvailabilityManagerImpl.java b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java index 755de00dec26..dadecdfe92f1 100644 --- a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java +++ b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java @@ -52,6 +52,7 @@ import org.apache.commons.collections.CollectionUtils; import com.cloud.agent.AgentManager; +import com.cloud.alert.AlertFormatUtils; import com.cloud.alert.AlertManager; import com.cloud.cluster.ClusterManagerListener; import com.cloud.consoleproxy.ConsoleProxyManager; @@ -374,7 +375,7 @@ public void scheduleRestartForVmsOnHost(final HostVO host, boolean investigate, } // send an email alert that the host is down, include VMs HostPodVO podVO = _podDao.findById(host.getPodId()); - String hostDesc = "name: " + host.getName() + " (id:" + host.getId() + "), availability zone: " + dcVO.getName() + ", pod: " + podVO.getName(); + String hostDesc = AlertFormatUtils.describeHostLocation(host, dcVO, podVO); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Host is down, " + hostDesc, "Host [" + hostDesc + "] is down." + ((sb != null) ? sb.toString() : "")); @@ -513,9 +514,17 @@ public void scheduleRestart(VMInstanceVO vm, boolean investigate, ReasonType rea } if (!(ForceHA.value() || vm.isHaEnabled())) { - String hostDesc = "id:" + vm.getHostId() + ", availability zone id:" + vm.getDataCenterId() + ", pod id:" + vm.getPodIdToDeployIn(); + HostVO stoppedHost = hostId != null ? _hostDao.findById(hostId) : null; + String hostDesc; + if (stoppedHost != null) { + DataCenterVO stoppedHostDcVO = _dcDao.findById(stoppedHost.getDataCenterId()); + HostPodVO stoppedHostPodVO = _podDao.findById(stoppedHost.getPodId()); + hostDesc = AlertFormatUtils.describeHostLocation(stoppedHost, stoppedHostDcVO, stoppedHostPodVO); + } else { + hostDesc = "host id: " + hostId; + } _alertMgr.sendAlert(alertType, vm.getDataCenterId(), vm.getPodIdToDeployIn(), "VM (name: " + vm.getHostName() + ", id: " + vm.getId() + - ") stopped unexpectedly on host " + hostDesc, "Virtual Machine " + vm.getHostName() + " (id: " + vm.getId() + ") running on host [" + vm.getHostId() + + ") stopped unexpectedly on host " + hostDesc, "Virtual Machine " + vm.getHostName() + " (id: " + vm.getId() + ") running on host [" + hostDesc + "] stopped unexpectedly."); if (logger.isDebugEnabled()) { diff --git a/server/src/main/java/com/cloud/ha/KVMFencer.java b/server/src/main/java/com/cloud/ha/KVMFencer.java index 4a6606b09cc3..11d39bccedf1 100644 --- a/server/src/main/java/com/cloud/ha/KVMFencer.java +++ b/server/src/main/java/com/cloud/ha/KVMFencer.java @@ -108,8 +108,8 @@ public Boolean fenceOff(VirtualMachine vm, Host host) { } _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), - "Unable to fence off host: " + host.getId(), - "Fencing off host " + host.getId() + " did not succeed after asking " + i + " hosts. " + + "Unable to fence off host: " + host, + "Fencing off host " + host + " did not succeed after asking " + i + " hosts. " + "Check Agent logs for more information."); logger.error("Unable to fence off {} on {}", vm.toString(), host.toString()); diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index bc3abd30d880..e6bc802a848c 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -1008,12 +1008,12 @@ public ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege if (Domain.ROOT_DOMAIN == domainId) { // no one can add limits on ROOT domain, disallow... - throw new PermissionDeniedException("Cannot update resource limit for ROOT domain " + domainId + ", permission denied"); + throw new PermissionDeniedException("Cannot update resource limit for ROOT domain " + (domain != null ? domain : "id " + domainId) + ", permission denied"); } if ((caller.getDomainId() == domainId) && caller.getType() == Account.Type.DOMAIN_ADMIN || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN) { // if the admin is trying to update their own domain, disallow... - throw new PermissionDeniedException("Unable to update resource limit for domain " + domainId + ", permission denied"); + throw new PermissionDeniedException("Unable to update resource limit for domain " + (domain != null ? domain : "id " + domainId) + ", permission denied"); } if (StringUtils.isNotEmpty(tag)) { long untaggedLimit = findCorrectResourceLimitForDomain(domain, resourceType, null); diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java index dc33a4442a33..96d6b4173fac 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -2082,7 +2082,7 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, return snapshot; } catch (ResourceAllocationException e) { if (snapshotType != Type.MANUAL) { - String msg = String.format("Snapshot resource limit exceeded for account id : %s. Failed to create recurring snapshots", owner.getId()); + String msg = String.format("Snapshot resource limit exceeded for account: %s. Failed to create recurring snapshots", owner); logger.warn(msg); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_UPDATE_RESOURCE_COUNT, 0L, 0L, msg, msg + ". Please, use updateResourceLimit to increase the limit"); } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index b3bc69835ff5..575b8c2d74b2 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2862,10 +2862,18 @@ 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 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, - hostId != null ? String.format(" on host [%s]", hostId) : "", + hostDesc, ThreadContext.get("logcontextid")); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_USERVM, vm.getDataCenterId(), vm.getPodIdToDeployIn(), subject, body); @@ -7760,15 +7768,23 @@ public void checkHostsDedication(VMInstanceVO vm, long srcHostId, long destHostI //if hosts are dedicated to different account/domains, raise an alert if (srcExplDedicated && destExplDedicated) { - if (!((accountOfDedicatedHost(srcHost) == null) || (accountOfDedicatedHost(srcHost).equals(accountOfDedicatedHost(destHost))))) { - String msg = String.format("VM is being migrated from host %s explicitly dedicated to account %d to host %s explicitly dedicated to account %d", - srcHost, accountOfDedicatedHost(srcHost), destHost, accountOfDedicatedHost(destHost)); + Long srcAccountId = accountOfDedicatedHost(srcHost); + Long destAccountId = accountOfDedicatedHost(destHost); + if (!((srcAccountId == null) || (srcAccountId.equals(destAccountId)))) { + Account srcAccount = _accountDao.findById(srcAccountId); + Account destAccount = destAccountId != null ? _accountDao.findById(destAccountId) : null; + String msg = String.format("VM is being migrated from host %s explicitly dedicated to account %s to host %s %s", + srcHost, srcAccount, destHost, destAccount != null ? "explicitly dedicated to account " + destAccount : "not dedicated to a specific account"); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_USERVM, vm.getDataCenterId(), vm.getPodIdToDeployIn(), msg, msg); logger.warn(msg); } - if (!((domainOfDedicatedHost(srcHost) == null) || (domainOfDedicatedHost(srcHost).equals(domainOfDedicatedHost(destHost))))) { - String msg = String.format("VM is being migrated from host %s explicitly dedicated to domain %d to host %s explicitly dedicated to domain %d", - srcHost, domainOfDedicatedHost(srcHost), destHost, domainOfDedicatedHost(destHost)); + Long srcDomainId = domainOfDedicatedHost(srcHost); + Long destDomainId = domainOfDedicatedHost(destHost); + if (!((srcDomainId == null) || (srcDomainId.equals(destDomainId)))) { + Domain srcDomain = _domainDao.findById(srcDomainId); + Domain destDomain = destDomainId != null ? _domainDao.findById(destDomainId) : null; + String msg = String.format("VM is being migrated from host %s explicitly dedicated to domain %s to host %s %s", + srcHost, srcDomain, destHost, destDomain != null ? "explicitly dedicated to domain " + destDomain : "not dedicated to a specific domain"); _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_USERVM, vm.getDataCenterId(), vm.getPodIdToDeployIn(), msg, msg); logger.warn(msg); } @@ -7779,7 +7795,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 +7809,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 +7817,7 @@ public void checkHostsDedication(VMInstanceVO vm, long srcHostId, long destHostI for (VMInstanceVO vmsDest : vmsOnDest) { ServiceOfferingVO destPlanner = serviceOfferingDao.findById(vm.getId(), vmsDest.getServiceOfferingId()); if (!((destPlanner.getDeploymentPlanner() != null && destPlanner.getDeploymentPlanner().equals("ImplicitDedicationPlanner")) && vmsDest.getAccountId() == accountOfVm)) { - msg = String.format("Instance of Account %d with preferred implicit deployment planner being migrated to host %s not having all Instances implicitly dedicated to Account %d", accountOfVm, destHost, accountOfVm); + msg = String.format("Instance of Account %s with preferred implicit deployment planner being migrated to host %s not having all Instances implicitly dedicated to Account %s", accountOfVmObj, destHost, accountOfVmObj); } } } diff --git a/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java b/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java index 73ff79301fb7..a1e3a3cf6cab 100644 --- a/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java @@ -330,7 +330,7 @@ private boolean provisionKvmHostViaSsh(Host host, String caProvider) { return true; } catch (Exception e) { - logger.error("Error during forced SSH provisioning for KVM host " + host.getUuid(), e); + logger.error("Error during forced SSH provisioning for KVM host " + host, e); return false; } finally { if (sshConnection != null) { diff --git a/server/src/main/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProvider.java b/server/src/main/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProvider.java index 2d77e6f9d20c..c8dbf17d9a81 100644 --- a/server/src/main/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProvider.java +++ b/server/src/main/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProvider.java @@ -95,11 +95,11 @@ public void sendAlert(final Host host, final HAConfig.HAState nextState) { String subject = "HA operation performed for host"; String body = subject; if (HAConfig.HAState.Fencing.equals(nextState)) { - subject = String.format("HA Fencing of host id=%d, in dc id=%d performed", host.getId(), host.getDataCenterId()); - body = String.format("HA Fencing has been performed for host id=%d, uuid=%s in datacenter id=%d", host.getId(), host.getUuid(), host.getDataCenterId()); + subject = String.format("HA Fencing of host %s performed", host); + body = String.format("HA Fencing has been performed for host %s", host); } else if (HAConfig.HAState.Recovering.equals(nextState)) { - subject = String.format("HA Recovery of host id=%d, in dc id=%d performed", host.getId(), host.getDataCenterId()); - body = String.format("HA Recovery has been performed for host id=%d, uuid=%s in datacenter id=%d", host.getId(), host.getUuid(), host.getDataCenterId()); + subject = String.format("HA Recovery of host %s performed", host); + body = String.format("HA Recovery has been performed for host %s", host); } alertManager.sendAlert(AlertService.AlertType.ALERT_TYPE_HA_ACTION, host.getDataCenterId(), host.getPodId(), subject, body); } diff --git a/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java b/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java index d5013f71cb5a..c015229ad1c1 100644 --- a/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java @@ -260,7 +260,7 @@ private boolean isOutOfBandManagementEnabledForHost(Long hostId) { Host host = hostDao.findById(hostId); if (host == null || host.getResourceState() == ResourceState.Degraded) { String state = host != null ? String.valueOf(host.getResourceState()) : null; - logger.debug("Host [id={}, uuid={}, state={}] was removed or placed in Degraded state by the Admin.", hostId, host != null ? host.getUuid() : "", state); + logger.debug("Host [{}] was removed or placed in Degraded state (state={}) by the Admin.", host != null ? host : "id=" + hostId, state); return false; } diff --git a/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java b/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java index 626f2cda172f..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; @@ -43,6 +46,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 +274,83 @@ public void scheduleRestartHostNotSupported() { highAvailabilityManager.scheduleRestart(vm, true); } + @Test + public void scheduleRestartVmStoppedUnexpectedlyResolvesHostLocation() { + 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; + when(highAvailabilityManager.VmHaEnabled.valueIn(1L)).thenReturn(true); + + 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 = mock(DataCenterVO.class); + when(dcVO.getName()).thenReturn("Milton1"); + when(_dcDao.findById(1L)).thenReturn(dcVO); + + HostPodVO podVO = mock(HostPodVO.class); + when(podVO.getName()).thenReturn("Milton1-Pod1"); + when(_podDao.findById(2L)).thenReturn(podVO); + + when(_instanceDao.findByUuid("vm-uuid")).thenReturn(vm); + when(_haDao.findPreviousHA(3L)).thenReturn(new ArrayList<>()); + + highAvailabilityManager.scheduleRestart(vm, false); + + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + 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 = 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; + when(highAvailabilityManager.VmHaEnabled.valueIn(1L)).thenReturn(true); + + when(_hostDao.findById(5L)).thenReturn(null); + + when(_instanceDao.findByUuid("vm-uuid")).thenReturn(vm); + when(_haDao.findPreviousHA(3L)).thenReturn(new ArrayList<>()); + + highAvailabilityManager.scheduleRestart(vm, false); + + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + verify(_alertMgr).sendAlert(Mockito.eq(AlertManager.AlertType.ALERT_TYPE_USERVM), Mockito.eq(1L), Mockito.eq(2L), + Mockito.anyString(), bodyCaptor.capture()); + assertTrue(bodyCaptor.getValue().contains("host id: 5")); + } + @Test public void scheduleStop() { VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); diff --git a/server/src/test/java/com/cloud/ha/KVMFencerTest.java b/server/src/test/java/com/cloud/ha/KVMFencerTest.java index c4b5666c0206..74ba1cca488d 100644 --- a/server/src/test/java/com/cloud/ha/KVMFencerTest.java +++ b/server/src/test/java/com/cloud/ha/KVMFencerTest.java @@ -88,7 +88,6 @@ public void testWithSingleHostDown() { Mockito.when(host.getDataCenterId()).thenReturn(1l); Mockito.when(host.getPodId()).thenReturn(1l); Mockito.when(host.getStatus()).thenReturn(Status.Down); - Mockito.when(host.getId()).thenReturn(1l); VirtualMachine virtualMachine = Mockito.mock(VirtualMachine.class); Mockito.when(resourceManager.listAllHostsInCluster(1l)).thenReturn(Collections.singletonList(host)); diff --git a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java index a4e0c953c0ee..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; @@ -66,6 +68,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 +1334,82 @@ 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}")); + verify(resourceLimitDao, 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(); + } + verify(resourceLimitDao, 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)); + verify(resourceLimitDao, 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(); + } + verify(resourceLimitDao, 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 f70a3abd5871..1420773ade57 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,221 @@ 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 testServiceOfferingId = 2L; + + 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 = mock(HostVO.class); + when(srcHost.getId()).thenReturn(srcHostId); + HostVO destHost = mock(HostVO.class); + when(destHost.getId()).thenReturn(destHostId); + when(hostDao.findById(srcHostId)).thenReturn(srcHost); + when(hostDao.findById(destHostId)).thenReturn(destHost); + + DedicatedResourceVO srcDedication = mock(DedicatedResourceVO.class); + when(srcDedication.getAccountId()).thenReturn(100L); + when(srcDedication.getDomainId()).thenReturn(200L); + 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 = mock(AccountVO.class); + when(srcAccount.toString()).thenReturn("Account {accountName=account-a}"); + 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 = mock(DomainVO.class); + when(srcDomain.toString()).thenReturn("Domain {name=domain-a}"); + 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); + + when(serviceOffering.getDeploymentPlanner()).thenReturn(null); + when(_serviceOfferingDao.findById(vmId, testServiceOfferingId)).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); + 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"))); + assertTrue(messages.stream().anyMatch(m -> m.contains("domain-a") && m.contains("domain-b"))); + } + + @Test + public void checkHostsDedicationAlertNotesDestinationNotDedicatedToSpecificAccount() { + long srcHostId = 10L; + long destHostId = 20L; + long testServiceOfferingId = 2L; + + 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 = mock(HostVO.class); + when(srcHost.getId()).thenReturn(srcHostId); + 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 = mock(DedicatedResourceVO.class); + when(srcDedication.getAccountId()).thenReturn(100L); + when(srcDedication.getDomainId()).thenReturn((Long) null); + 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 = mock(AccountVO.class); + when(srcAccount.toString()).thenReturn("Account {accountName=account-a}"); + when(accountDao.findById(100L)).thenReturn(srcAccount); + + when(serviceOffering.getDeploymentPlanner()).thenReturn(null); + when(_serviceOfferingDao.findById(vmId, testServiceOfferingId)).thenReturn(serviceOffering); + + when(plannerHostReservationDao.listAllDedicatedHosts()).thenReturn(new ArrayList<>()); + + userVmManagerImpl.checkHostsDedication(vm, srcHostId, destHostId); + + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + 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")); + } + + @Test + public void checkHostsDedicationAlertNotesDestinationNotDedicatedToSpecificDomain() { + long srcHostId = 10L; + long destHostId = 20L; + long testServiceOfferingId = 2L; + + 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 = mock(HostVO.class); + when(srcHost.getId()).thenReturn(srcHostId); + 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 = mock(DedicatedResourceVO.class); + when(srcDedication.getAccountId()).thenReturn((Long) null); + when(srcDedication.getDomainId()).thenReturn(200L); + 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 = mock(DomainVO.class); + when(srcDomain.toString()).thenReturn("Domain {name=domain-a}"); + when(domainDaoMock.findById(200L)).thenReturn(srcDomain); + + when(serviceOffering.getDeploymentPlanner()).thenReturn(null); + when(_serviceOfferingDao.findById(vmId, testServiceOfferingId)).thenReturn(serviceOffering); + + when(plannerHostReservationDao.listAllDedicatedHosts()).thenReturn(new ArrayList<>()); + + userVmManagerImpl.checkHostsDedication(vm, srcHostId, destHostId); + + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + 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 = 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 testVmId = 3L; + Long hostId = 5L; + mockStoppedVmForFailedCreation(testVmId); + + 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); + 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 testVmId = 3L; + Long hostId = 5L; + mockStoppedVmForFailedCreation(testVmId); + + when(hostDao.findById(hostId)).thenReturn(null); + + ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", testVmId, hostId); + + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + 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 testVmId = 3L; + mockStoppedVmForFailedCreation(testVmId); + + ReflectionTestUtils.invokeMethod(userVmManagerImpl, "updateVmStateForFailedVmCreation", testVmId, (Long) null); + + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + 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 new file mode 100644 index 000000000000..779dd7584b36 --- /dev/null +++ b/server/src/test/java/org/apache/cloudstack/ha/provider/host/HAAbstractHostProviderTest.java @@ -0,0 +1,135 @@ +// 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.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; +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); + + when(host.getDataCenterId()).thenReturn(1L); + when(host.getPodId()).thenReturn(2L); + 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); + 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); + 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); + 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()); + } +} 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); + } +}