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 60bad6683a4..3a0654eb8e7 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 @@ -38,8 +38,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; -import org.apache.cloudstack.framework.config.ConfigKey; -import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; @@ -110,16 +108,9 @@ import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.dao.VMInstanceDao; import com.google.common.base.Preconditions; -public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver, Configurable { +public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver { private static final Logger LOGGER = Logger.getLogger(ScaleIOPrimaryDataStoreDriver.class); - static ConfigKey ConnectOnDemand = new ConfigKey<>("Storage", - Boolean.class, - "powerflex.connect.on.demand", - Boolean.FALSE.toString(), - "Connect PowerFlex client on Host when first Volume created and disconnect when last Volume deleted (or always stay connected otherwise).", - Boolean.TRUE); - @Inject EndPointSelector selector; @Inject @@ -164,9 +155,7 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver, Co private boolean setVolumeLimitsOnSDC(VolumeVO volume, Host host, DataStore dataStore, Long iopsLimit, Long bandwidthLimitInKbps) throws Exception { sdcManager = ComponentContext.inject(sdcManager); - // don't connect SDC if connect on demand disabled - final String sdcId = Boolean.TRUE.equals(ConnectOnDemand.value()) ? sdcManager.prepareSDC(host, dataStore) : - sdcManager.getConnectedSdc(host, dataStore); + final String sdcId = sdcManager.prepareSDC(host, dataStore); if (StringUtils.isBlank(sdcId)) { alertHostSdcDisconnection(host); throw new CloudRuntimeException("Unable to grant access to volume: " + volume.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress()); @@ -203,9 +192,7 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver, Co public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore) { try { sdcManager = ComponentContext.inject(sdcManager); - // don't connect SDC if connect on demand disabled - final String sdcId = Boolean.TRUE.equals(ConnectOnDemand.value()) ? sdcManager.prepareSDC(host, dataStore) : - sdcManager.getConnectedSdc(host, dataStore); + final String sdcId = sdcManager.prepareSDC(host, dataStore); if (StringUtils.isBlank(sdcId)) { alertHostSdcDisconnection(host); throw new CloudRuntimeException(String.format("Unable to grant access to %s: %s, no Sdc connected with host ip: %s", dataObject.getType(), dataObject.getId(), host.getPrivateIpAddress())); @@ -246,8 +233,7 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver, Co } try { - sdcManager = ComponentContext.inject(sdcManager); - final String sdcId = sdcManager.getConnectedSdc(host, dataStore); + final String sdcId = getConnectedSdc(dataStore.getId(), host.getId()); if (StringUtils.isBlank(sdcId)) { LOGGER.warn(String.format("Unable to revoke access for %s: %s, no Sdc connected with host ip: %s", dataObject.getType(), dataObject.getId(), host.getPrivateIpAddress())); return; @@ -266,9 +252,8 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver, Co LOGGER.debug("Revoking access for PowerFlex volume snapshot: " + snapshot.getPath()); client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(snapshot.getPath()), sdcId); } - - // don't stop SDC if connect on demand disabled - if (Boolean.TRUE.equals(ConnectOnDemand.value()) && client.listVolumesMappedToSdc(sdcId).isEmpty()) { + if (client.listVolumesMappedToSdc(sdcId).isEmpty()) { + sdcManager = ComponentContext.inject(sdcManager); sdcManager.stopSDC(host, dataStore); } } catch (Exception e) { @@ -284,8 +269,8 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver, Co try { LOGGER.debug("Revoking access for PowerFlex volume: " + volumePath); - sdcManager = ComponentContext.inject(sdcManager); - final String sdcId = sdcManager.getConnectedSdc(host, dataStore); + + final String sdcId = getConnectedSdc(dataStore.getId(), host.getId()); if (StringUtils.isBlank(sdcId)) { LOGGER.warn(String.format("Unable to revoke access for volume: %s, no Sdc connected with host ip: %s", volumePath, host.getPrivateIpAddress())); return; @@ -293,8 +278,8 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver, Co final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId()); client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(volumePath), sdcId); - // don't stop SDC if connect on demand disabled - if (Boolean.TRUE.equals(ConnectOnDemand.value()) && client.listVolumesMappedToSdc(sdcId).isEmpty()) { + if (client.listVolumesMappedToSdc(sdcId).isEmpty()) { + sdcManager = ComponentContext.inject(sdcManager); sdcManager.stopSDC(host, dataStore); } } catch (Exception e) { @@ -307,6 +292,24 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver, Co revokeAccess(dataObject, host, dataStore); } + public String getConnectedSdc(long poolId, long hostId) { + try { + StoragePoolHostVO poolHostVO = storagePoolHostDao.findByPoolHost(poolId, hostId); + if (poolHostVO == null) { + return null; + } + + final ScaleIOGatewayClient client = getScaleIOClient(poolId); + if (client.isSdcConnected(poolHostVO.getLocalPath())) { + return poolHostVO.getLocalPath(); + } + } catch (Exception e) { + LOGGER.warn("Couldn't check SDC connection for the host: " + hostId + " and storage pool: " + poolId + " due to " + e.getMessage(), e); + } + + return null; + } + @Override public boolean requiresAccessForMigration(DataObject dataObject) { return true; @@ -553,7 +556,7 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver, Co } } } else { - LOGGER.debug(String.format("No encryption configured for data volume %s", volumeInfo)); + LOGGER.debug(String.format("No encryption configured for data volume %s", volumeInfo)); } return answer; @@ -776,7 +779,7 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver, Co long newSize = destData.getSize() + (1<<30); LOGGER.debug(String.format("Destination volume %s(%s) is configured for encryption. Resizing to fit headers, new size %s will be rounded up to nearest 8Gi", destInfo.getId(), destData.getSize(), newSize)); ResizeVolumePayload p = new ResizeVolumePayload(newSize, destInfo.getMinIops(), destInfo.getMaxIops(), - destInfo.getHypervisorSnapshotReserve(), false, destInfo.getAttachedVmName(), null, true); + destInfo.getHypervisorSnapshotReserve(), false, destInfo.getAttachedVmName(), null, true); destInfo.addPayload(p); resizeVolume(destInfo); } else { @@ -1520,14 +1523,4 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver, Co public boolean zoneWideVolumesAvailableWithoutClusterMotion() { return true; } - - @Override - public String getConfigComponentName() { - return ScaleIOPrimaryDataStoreDriver.class.getSimpleName(); - } - - @Override - public ConfigKey[] getConfigKeys() { - return new ConfigKey[]{ConnectOnDemand}; - } } diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManagerImpl.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManagerImpl.java index 69ec098db02..38f10ebdc85 100644 --- a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManagerImpl.java +++ b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManagerImpl.java @@ -24,10 +24,13 @@ import javax.inject.Inject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.storage.datastore.client.ScaleIOGatewayClient; import org.apache.cloudstack.storage.datastore.client.ScaleIOGatewayClientConnectionPool; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; +import org.apache.cloudstack.storage.datastore.driver.ScaleIOPrimaryDataStoreDriver; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; @@ -50,9 +53,17 @@ import com.cloud.utils.db.GlobalLock; import com.cloud.utils.exception.CloudRuntimeException; @Component -public class ScaleIOSDCManagerImpl implements ScaleIOSDCManager { +public class ScaleIOSDCManagerImpl implements ScaleIOSDCManager, Configurable { private static final Logger LOGGER = Logger.getLogger(ScaleIOSDCManagerImpl.class); + static ConfigKey ConnectOnDemand = new ConfigKey<>("Storage", + Boolean.class, + "powerflex.connect.on.demand", + Boolean.FALSE.toString(), + "Connect PowerFlex client on Host when first Volume created and disconnect when last Volume deleted (or always stay connected otherwise).", + Boolean.TRUE, + ConfigKey.Scope.Zone); + @Inject AgentManager agentManager; @Inject @@ -93,6 +104,10 @@ public class ScaleIOSDCManagerImpl implements ScaleIOSDCManager { @Override public String prepareSDC(Host host, DataStore dataStore) { + if (Boolean.FALSE.equals(ConnectOnDemand.valueIn(host.getDataCenterId()))) { + return getConnectedSdc(host, dataStore); + } + String systemId = storagePoolDetailsDao.findDetail(dataStore.getId(), ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID).getValue(); if (systemId == null) { throw new CloudRuntimeException("Unable to prepare SDC, failed to get the system id for PowerFlex storage pool: " + dataStore.getName()); @@ -226,6 +241,9 @@ public class ScaleIOSDCManagerImpl implements ScaleIOSDCManager { @Override public boolean stopSDC(Host host, DataStore dataStore) { + if(Boolean.FALSE.equals(ConnectOnDemand.valueIn(host.getDataCenterId()))) { + return true; + } String systemId = storagePoolDetailsDao.findDetail(dataStore.getId(), ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID).getValue(); if (systemId == null) { throw new CloudRuntimeException("Unable to unprepare SDC, failed to get the system id for PowerFlex storage pool: " + dataStore.getName()); @@ -345,4 +363,15 @@ public class ScaleIOSDCManagerImpl implements ScaleIOSDCManager { private ScaleIOGatewayClient getScaleIOClient(final Long storagePoolId) throws Exception { return ScaleIOGatewayClientConnectionPool.getInstance().getClient(storagePoolId, storagePoolDetailsDao); } + + + @Override + public String getConfigComponentName() { + return ScaleIOPrimaryDataStoreDriver.class.getSimpleName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[]{ConnectOnDemand}; + } } diff --git a/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriverTest.java b/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriverTest.java index 0a5743ac0fc..f9cd2cda4f1 100644 --- a/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriverTest.java +++ b/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriverTest.java @@ -20,23 +20,16 @@ package org.apache.cloudstack.storage.datastore.driver; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.Optional; -import com.cloud.utils.component.ComponentContext; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService; -import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.storage.RemoteHostEndPoint; import org.apache.cloudstack.storage.command.CreateObjectAnswer; @@ -45,8 +38,6 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.datastore.manager.ScaleIOSDCManager; -import org.apache.cloudstack.storage.datastore.manager.ScaleIOSDCManagerImpl; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.junit.Assert; import org.junit.Before; @@ -83,17 +74,12 @@ import com.cloud.vm.VirtualMachine; import com.cloud.vm.dao.VMInstanceDao; @RunWith(PowerMockRunner.class) -@PrepareForTest({RemoteHostEndPoint.class, ComponentContext.class}) +@PrepareForTest(RemoteHostEndPoint.class) public class ScaleIOPrimaryDataStoreDriverTest { @Spy @InjectMocks ScaleIOPrimaryDataStoreDriver scaleIOPrimaryDataStoreDriver = new ScaleIOPrimaryDataStoreDriver(); - - @Spy - @InjectMocks - ScaleIOSDCManager sdcManager = new ScaleIOSDCManagerImpl(); - @Mock StoragePoolDetailsDao storagePoolDetailsDao; @Mock @@ -463,7 +449,7 @@ public class ScaleIOPrimaryDataStoreDriverTest { } @Test - public void deleteSourceVolumeFailureScenarioAfterSuccessfulBlockCopyOnDemandDisabled() throws Exception { + public void deleteSourceVolumeFailureScenarioAfterSuccessfulBlockCopy() throws Exception { // Either Volume deletion success or failure method should complete VolumeInfo srcData = Mockito.mock(VolumeInfo.class); @@ -477,61 +463,16 @@ public class ScaleIOPrimaryDataStoreDriverTest { when(srcData.getDataStore()).thenReturn(srcStore); when(srcData.getTO()).thenReturn(volumeTO); when(volumeTO.getPath()).thenReturn(srcVolumePath); - - PowerMockito.mockStatic(ComponentContext.class); - PowerMockito.when(ComponentContext.inject(eq(sdcManager))).thenReturn(sdcManager); - String sdcId = "7332760565f6340f"; - doReturn(sdcId).when(sdcManager).getConnectedSdc(host, srcStore); + doReturn(sdcId).when(scaleIOPrimaryDataStoreDriver).getConnectedSdc(1L, 1L); ScaleIOGatewayClient client = Mockito.mock(ScaleIOGatewayClient.class); - - doReturn(client).when(scaleIOPrimaryDataStoreDriver).getScaleIOClient(any()); - - doReturn(true).when(client).unmapVolumeFromSdc(anyString(), anyString()); - doReturn(false).when(client).deleteVolume(anyString()); - - scaleIOPrimaryDataStoreDriver.deleteSourceVolumeAfterSuccessfulBlockCopy(srcData, host); - } - - @Test - public void deleteSourceVolumeFailureScenarioAfterSuccessfulBlockCopyOnDemandEnabled() throws Exception { - // Either Volume deletion success or failure method should complete - - VolumeInfo srcData = Mockito.mock(VolumeInfo.class); - Host host = Mockito.mock(Host.class); - when(host.getId()).thenReturn(1L); - String srcVolumePath = "bec0ba7700000007:vol-11-6aef-10ee"; - - DataStore srcStore = Mockito.mock(DataStore.class); - when(srcStore.getId()).thenReturn(1L); - DataTO volumeTO = Mockito.mock(DataTO.class); - when(srcData.getDataStore()).thenReturn(srcStore); - when(srcData.getTO()).thenReturn(volumeTO); - when(volumeTO.getPath()).thenReturn(srcVolumePath); - - PowerMockito.mockStatic(ComponentContext.class); - PowerMockito.when(ComponentContext.inject(eq(sdcManager))).thenReturn(sdcManager); - - String sdcId = "7332760565f6340f"; - doReturn(sdcId).when(sdcManager).getConnectedSdc(host, srcStore); - - ScaleIOGatewayClient client = Mockito.mock(ScaleIOGatewayClient.class); - doReturn(client).when(scaleIOPrimaryDataStoreDriver).getScaleIOClient(any()); - + doReturn(client).when(scaleIOPrimaryDataStoreDriver) + .getScaleIOClient(any()); doReturn(true).when(client).unmapVolumeFromSdc(any(), any()); - doReturn(false).when(client).deleteVolume(anyString()); - - ConfigKey connectOnDemand = Mockito.mock(ConfigKey.class); - doReturn(Boolean.TRUE).when(connectOnDemand).value(); - - doReturn(true).when(sdcManager).stopSDC(isA(Host.class), isA(DataStore.class)); - - scaleIOPrimaryDataStoreDriver.ConnectOnDemand = connectOnDemand; + when(client.deleteVolume(any())).thenReturn(false); scaleIOPrimaryDataStoreDriver.deleteSourceVolumeAfterSuccessfulBlockCopy(srcData, host); - - verify(sdcManager, times(1)).stopSDC(isA(Host.class), isA(DataStore.class)); } @Test @@ -549,11 +490,8 @@ public class ScaleIOPrimaryDataStoreDriverTest { when(srcData.getDataStore()).thenReturn(srcStore); when(srcData.getTO()).thenReturn(volumeTO); when(volumeTO.getPath()).thenReturn(srcVolumePath); - - PowerMockito.mockStatic(ComponentContext.class); - PowerMockito.when(ComponentContext.inject(eq(sdcManager))).thenReturn(sdcManager); - - doReturn(null).when(sdcManager).getConnectedSdc(host, srcStore); + String sdcId = "7332760565f6340f"; + doReturn(null).when(scaleIOPrimaryDataStoreDriver).getConnectedSdc(1L, 1L); scaleIOPrimaryDataStoreDriver.deleteSourceVolumeAfterSuccessfulBlockCopy(srcData, host); }