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 06091735d70..f7128876d7d 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,6 +38,7 @@ 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.dao.ConfigurationDao; import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; @@ -111,6 +112,13 @@ import com.google.common.base.Preconditions; 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 @@ -155,7 +163,9 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver { private boolean setVolumeLimitsOnSDC(VolumeVO volume, Host host, DataStore dataStore, Long iopsLimit, Long bandwidthLimitInKbps) throws Exception { sdcManager = ComponentContext.inject(sdcManager); - final String sdcId = sdcManager.prepareSDC(host, dataStore); + // 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); 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()); @@ -192,7 +202,9 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver { public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore) { try { sdcManager = ComponentContext.inject(sdcManager); - final String sdcId = sdcManager.prepareSDC(host, dataStore); + // 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); 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())); @@ -233,7 +245,8 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver { } try { - final String sdcId = getConnectedSdc(dataStore.getId(), host.getId()); + sdcManager = ComponentContext.inject(sdcManager); + final String sdcId = sdcManager.getConnectedSdc(host, dataStore); 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; @@ -252,8 +265,9 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver { LOGGER.debug("Revoking access for PowerFlex volume snapshot: " + snapshot.getPath()); client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(snapshot.getPath()), sdcId); } - if (client.listVolumesMappedToSdc(sdcId).isEmpty()) { - sdcManager = ComponentContext.inject(sdcManager); + + // don't stop SDC if connect on demand disabled + if (Boolean.TRUE.equals(ConnectOnDemand.value()) && client.listVolumesMappedToSdc(sdcId).isEmpty()) { sdcManager.stopSDC(host, dataStore); } } catch (Exception e) { @@ -269,8 +283,8 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver { try { LOGGER.debug("Revoking access for PowerFlex volume: " + volumePath); - - final String sdcId = getConnectedSdc(dataStore.getId(), host.getId()); + sdcManager = ComponentContext.inject(sdcManager); + final String sdcId = sdcManager.getConnectedSdc(host, dataStore); 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; @@ -278,8 +292,8 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver { final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId()); client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(volumePath), sdcId); - if (client.listVolumesMappedToSdc(sdcId).isEmpty()) { - sdcManager = ComponentContext.inject(sdcManager); + // don't stop SDC if connect on demand disabled + if (Boolean.TRUE.equals(ConnectOnDemand.value()) && client.listVolumesMappedToSdc(sdcId).isEmpty()) { sdcManager.stopSDC(host, dataStore); } } catch (Exception e) { @@ -292,24 +306,6 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver { 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; diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManager.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManager.java index 696643cb17a..ede893786b5 100644 --- a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManager.java +++ b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManager.java @@ -29,6 +29,14 @@ public interface ScaleIOSDCManager { */ boolean areSDCConnectionsWithinLimit(Long storagePoolId); + /** + * Returns connected SDC Id. + * @param host the host + * @param dataStore the datastore + * @return SDC Id of the host + */ + String getConnectedSdc(Host host, DataStore dataStore); + /** * Prepares/starts the SDC on the host. * @param host the host 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 b121a1da66f..69ec098db02 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 @@ -115,7 +115,7 @@ public class ScaleIOSDCManagerImpl implements ScaleIOSDCManager { long poolId = dataStore.getId(); long hostId = host.getId(); - String sdcId = getConnectedSdc(poolId, hostId); + String sdcId = getConnectedSdc(host, dataStore); if (StringUtils.isNotBlank(sdcId)) { LOGGER.debug(String.format("SDC %s already connected for the pool: %d on host: %d, no need to prepare/start it", sdcId, poolId, hostId)); return sdcId; @@ -245,9 +245,7 @@ public class ScaleIOSDCManagerImpl implements ScaleIOSDCManager { throw new CloudRuntimeException("Unable to unprepare SDC, couldn't lock on " + hostIdStorageSystemIdLockString); } - long poolId = dataStore.getId(); - long hostId = host.getId(); - String sdcId = getConnectedSdc(poolId, hostId); + String sdcId = getConnectedSdc(host, dataStore); if (StringUtils.isBlank(sdcId)) { LOGGER.debug("SDC not connected, no need to unprepare it"); return true; @@ -296,7 +294,11 @@ public class ScaleIOSDCManagerImpl implements ScaleIOSDCManager { } } - private String getConnectedSdc(long poolId, long hostId) { + @Override + public String getConnectedSdc(Host host, DataStore dataStore) { + long poolId = dataStore.getId(); + long hostId = host.getId(); + try { StoragePoolHostVO poolHostVO = storagePoolHostDao.findByPoolHost(poolId, hostId); if (poolHostVO == null) { 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 f9cd2cda4f1..0a5743ac0fc 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,16 +20,23 @@ 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; @@ -38,6 +45,8 @@ 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; @@ -74,12 +83,17 @@ import com.cloud.vm.VirtualMachine; import com.cloud.vm.dao.VMInstanceDao; @RunWith(PowerMockRunner.class) -@PrepareForTest(RemoteHostEndPoint.class) +@PrepareForTest({RemoteHostEndPoint.class, ComponentContext.class}) public class ScaleIOPrimaryDataStoreDriverTest { @Spy @InjectMocks ScaleIOPrimaryDataStoreDriver scaleIOPrimaryDataStoreDriver = new ScaleIOPrimaryDataStoreDriver(); + + @Spy + @InjectMocks + ScaleIOSDCManager sdcManager = new ScaleIOSDCManagerImpl(); + @Mock StoragePoolDetailsDao storagePoolDetailsDao; @Mock @@ -449,7 +463,7 @@ public class ScaleIOPrimaryDataStoreDriverTest { } @Test - public void deleteSourceVolumeFailureScenarioAfterSuccessfulBlockCopy() throws Exception { + public void deleteSourceVolumeFailureScenarioAfterSuccessfulBlockCopyOnDemandDisabled() throws Exception { // Either Volume deletion success or failure method should complete VolumeInfo srcData = Mockito.mock(VolumeInfo.class); @@ -463,18 +477,63 @@ 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(scaleIOPrimaryDataStoreDriver).getConnectedSdc(1L, 1L); + doReturn(sdcId).when(sdcManager).getConnectedSdc(host, srcStore); ScaleIOGatewayClient client = Mockito.mock(ScaleIOGatewayClient.class); - doReturn(client).when(scaleIOPrimaryDataStoreDriver) - .getScaleIOClient(any()); - doReturn(true).when(client).unmapVolumeFromSdc(any(), any()); - when(client.deleteVolume(any())).thenReturn(false); + + 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(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; + + scaleIOPrimaryDataStoreDriver.deleteSourceVolumeAfterSuccessfulBlockCopy(srcData, host); + + verify(sdcManager, times(1)).stopSDC(isA(Host.class), isA(DataStore.class)); + } + @Test public void deleteSourceVolumeFailureScenarioWhenNoSDCisFound() { // Either Volume deletion success or failure method should complete @@ -490,8 +549,11 @@ public class ScaleIOPrimaryDataStoreDriverTest { when(srcData.getDataStore()).thenReturn(srcStore); when(srcData.getTO()).thenReturn(volumeTO); when(volumeTO.getPath()).thenReturn(srcVolumePath); - String sdcId = "7332760565f6340f"; - doReturn(null).when(scaleIOPrimaryDataStoreDriver).getConnectedSdc(1L, 1L); + + PowerMockito.mockStatic(ComponentContext.class); + PowerMockito.when(ComponentContext.inject(eq(sdcManager))).thenReturn(sdcManager); + + doReturn(null).when(sdcManager).getConnectedSdc(host, srcStore); scaleIOPrimaryDataStoreDriver.deleteSourceVolumeAfterSuccessfulBlockCopy(srcData, host); }