You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/05/09 09:56:08 UTC
[GitHub] [cloudstack] sureshanaparti opened a new pull request, #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
sureshanaparti opened a new pull request, #6367:
URL: https://github.com/apache/cloudstack/pull/6367
### Description
This PR enhances the existing PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection, mainly the SDC (ScaleIo Data Client) connection.
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
<!--- ********************************************************************************* -->
<!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ********************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [x] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
Manually tested VM/Volume operations on PowerFlex/ScaleIO storage.
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1120961135
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1121002470
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1121652035
<b>Trillian test result (tid-4152)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37395 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6367-t4152-kvm-centos7.zip
Smoke tests completed. 96 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1161261507
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6367 (SL-JID-1786)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1160229176
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1160058276
@sureshanaparti a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a diff in pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on code in PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#discussion_r879163607
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java:
##########
@@ -53,6 +54,16 @@ public class ScaleIOUtil {
// VOL-ID 6c33633100000009 MDM-ID 218ce1797566a00f
// VOL-ID 6c3362a30000000a MDM-ID 218ce1797566a00f
+ private static final String QUERY_GUID_CMD = "drv_cfg --query_guid";
+ // Sample output for cmd: drv_cfg --query_guid:
+ // B0E3BFB8-C20B-43BF-93C8-13339E85AA50
Review Comment:
ok @DaanHoogland
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a diff in pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on code in PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#discussion_r901321812
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java:
##########
@@ -69,7 +73,41 @@ public boolean hostConnect(long hostId, long poolId) {
return false;
}
- if (!isHostSdcConnected(host.getPrivateIpAddress(), poolId)) {
+ StoragePool storagePool = (StoragePool)_dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary);
+
+ String systemId = _storagePoolDetailsDao.findDetail(poolId, ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID).getValue();
+ if (systemId == null) {
+ throw new CloudRuntimeException("Failed to get the system id for PowerFlex storage pool " + storagePool.getName());
+ }
+ Map<String,String> details = new HashMap<>();
+ details.put(ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID, systemId);
+
+ ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, storagePool, storagePool.getPath(), details);
+ ModifyStoragePoolAnswer answer = sendModifyStoragePoolCommand(cmd, storagePool, hostId);
+ Map<String,String> poolDetails = answer.getPoolInfo().getDetails();
+ if (MapUtils.isEmpty(poolDetails)) {
+ String msg = "SDC details not found on the host: " + hostId + ", (re)install SDC and restart agent";
+ s_logger.warn(msg);
+ _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC not found on host: " + host.getUuid(), msg);
+ return false;
+ }
+
+ String sdcId = null;
+ if (poolDetails.containsKey(ScaleIOGatewayClient.SDC_ID)) {
+ sdcId = poolDetails.get(ScaleIOGatewayClient.SDC_ID);
+ } else if (poolDetails.containsKey(ScaleIOGatewayClient.SDC_GUID)) {
+ String sdcGuid = poolDetails.get(ScaleIOGatewayClient.SDC_GUID);
+ sdcId = getHostSdcId(sdcGuid, poolId);
+ }
+
+ if (StringUtils.isBlank(sdcId)) {
+ String msg = "Couldn't retrieve SDC details from the host: " + hostId + ", (re)install SDC and restart agent";
+ s_logger.warn(msg);
+ _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC details not found on host: " + host.getUuid(), msg);
+ return false;
+ }
+
+ if (!isHostSdcConnected(sdcId, poolId)) {
Review Comment:
will do this refactoring in another PR if needed. thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a diff in pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on code in PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#discussion_r870214822
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java:
##########
@@ -341,20 +343,25 @@ public KVMPhysicalDisk getPhysicalDisk(StoragePoolType type, String poolUuid, St
public KVMStoragePool createStoragePool(String name, String host, int port, String path, String userInfo, StoragePoolType type) {
// primary storage registers itself through here
- return createStoragePool(name, host, port, path, userInfo, type, true);
+ return createStoragePool(name, host, port, path, userInfo, type, null, true);
+ }
+
+ public KVMStoragePool createStoragePool(String name, String host, int port, String path, String userInfo, StoragePoolType type, Map<String, String> details) {
+ // primary storage registers itself through here
Review Comment:
We can turn this comment into a Javadoc.
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java:
##########
@@ -1013,6 +1013,22 @@ public Sdc getSdc(String sdcId) {
return get("/instances/Sdc::" + sdcId, Sdc.class);
}
+ @Override
+ public String getSdcIdByGuid(String sdcGuid) {
+ Preconditions.checkArgument(StringUtils.isNotEmpty(sdcGuid), "SDC Guid cannot be null");
+
+ List<Sdc> sdcs = listSdcs();
+ if(sdcs != null) {
+ for (Sdc sdc : sdcs) {
+ if (sdcGuid.equalsIgnoreCase(sdc.getSdcGuid())) {
+ return sdc.getId();
+ }
+ }
+ }
Review Comment:
Suggestion: we can invert the first `if` and add a return to reduce indentation:
```suggestion
if (sdcs == null) {
return null;
}
for (Sdc sdc : sdcs) {
if (sdcGuid.equalsIgnoreCase(sdc.getSdcGuid())) {
return sdc.getId();
}
}
```
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java:
##########
@@ -97,6 +107,52 @@ public static final String getSystemIdForVolume(String volumeId) {
return result;
}
+ public static String getSdcGuid() {
+ String queryGuidCmd = ScaleIOUtil.SDC_HOME_PATH + "/bin/" + ScaleIOUtil.QUERY_GUID_CMD;
+ String result = Script.runSimpleBashScript(queryGuidCmd);
+ if (result == null) {
+ LOGGER.warn("Failed to get SDC guid");
+ return null;
+ }
+
+ if (result.isEmpty()) {
+ LOGGER.warn("No SDC guid retrieved");
+ return null;
+ }
+
+ String guidRegEx = "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$";
+ if (!result.matches(guidRegEx)) {
Review Comment:
We can use `UuidUtils#validateUUID` here, as it does the same validation.
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java:
##########
@@ -78,27 +116,38 @@ public boolean hostConnect(long hostId, long poolId) {
StoragePoolHostVO storagePoolHost = _storagePoolHostDao.findByPoolHost(poolId, hostId);
if (storagePoolHost == null) {
- storagePoolHost = new StoragePoolHostVO(poolId, hostId, "");
+ storagePoolHost = new StoragePoolHostVO(poolId, hostId, sdcId);
_storagePoolHostDao.persist(storagePoolHost);
+ } else {
+ storagePoolHost.setLocalPath(sdcId);
+ _storagePoolHostDao.update(storagePoolHost.getId(), storagePoolHost);
}
- StoragePool storagePool = (StoragePool)_dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary);
- ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, storagePool);
- sendModifyStoragePoolCommand(cmd, storagePool, hostId);
+ s_logger.info("Connection established between storage pool: " + storagePool + " and host: " + hostId);
return true;
}
- private boolean isHostSdcConnected(String hostIpAddress, long poolId) {
+ private String getHostSdcId(String sdcGuid, long poolId) {
+ try {
+ ScaleIOGatewayClient client = ScaleIOGatewayClientConnectionPool.getInstance().getClient(poolId, _storagePoolDetailsDao);
+ return client.getSdcIdByGuid(sdcGuid);
+ } catch (NoSuchAlgorithmException | KeyManagementException | URISyntaxException e) {
+ s_logger.error("Failed to get host SDC id", e);
Review Comment:
We can add more context to the log.
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/ScaleIOPrimaryDataStoreLifeCycle.java:
##########
@@ -352,6 +325,22 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper
return true;
}
+ private void checkConnectedSdcs(Long dataStoreId) {
+ boolean haveConnectedSdcs = false;
+ try {
+ ScaleIOGatewayClient client = ScaleIOGatewayClientConnectionPool.getInstance().getClient(dataStoreId, storagePoolDetailsDao);
+ haveConnectedSdcs = client.haveConnectedSdcs();
+ } catch (NoSuchAlgorithmException | KeyManagementException | URISyntaxException e) {
+ LOGGER.error("Failed to create storage pool", e);
Review Comment:
We can add more context to the log, like the `dataStoreId`.
##########
core/src/main/java/com/cloud/agent/api/ModifyStoragePoolCommand.java:
##########
@@ -32,13 +33,21 @@ public class ModifyStoragePoolCommand extends Command {
private StorageFilerTO pool;
private String localPath;
private String storagePath;
+ private Map<String, String> details;
public ModifyStoragePoolCommand(boolean add, StoragePool pool, String localPath) {
this.add = add;
this.pool = new StorageFilerTO(pool);
this.localPath = localPath;
}
+ public ModifyStoragePoolCommand(boolean add, StoragePool pool, String localPath, Map<String, String> details) {
+ this.add = add;
+ this.pool = new StorageFilerTO(pool);
+ this.localPath = localPath;
Review Comment:
```suggestion
this(add, pool, localPath);
```
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java:
##########
@@ -1035,28 +1051,39 @@ public Sdc getConnectedSdcByIp(String ipAddress) {
}
@Override
- public List<String> listConnectedSdcIps() {
- List<String> sdcIps = new ArrayList<>();
+ public boolean haveConnectedSdcs() {
List<Sdc> sdcs = listSdcs();
if(sdcs != null) {
for (Sdc sdc : sdcs) {
if (MDM_CONNECTED_STATE.equalsIgnoreCase(sdc.getMdmConnectionState())) {
- sdcIps.add(sdc.getSdcIp());
+ return true;
}
}
}
- return sdcIps;
+ return false;
+ }
+
+ @Override
+ public boolean isSdcConnected(String sdcId) {
+ Preconditions.checkArgument(StringUtils.isNotEmpty(sdcId), "SDC Id cannot be null");
+
+ Sdc sdc = getSdc(sdcId);
+ if (sdc != null && MDM_CONNECTED_STATE.equalsIgnoreCase(sdc.getMdmConnectionState())) {
+ return true;
+ }
+
+ return false;
Review Comment:
```suggestion
return sdc != null && MDM_CONNECTED_STATE.equalsIgnoreCase(sdc.getMdmConnectionState());
```
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/ScaleIOPrimaryDataStoreLifeCycle.java:
##########
@@ -352,6 +325,22 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper
return true;
}
+ private void checkConnectedSdcs(Long dataStoreId) {
+ boolean haveConnectedSdcs = false;
+ try {
+ ScaleIOGatewayClient client = ScaleIOGatewayClientConnectionPool.getInstance().getClient(dataStoreId, storagePoolDetailsDao);
+ haveConnectedSdcs = client.haveConnectedSdcs();
+ } catch (NoSuchAlgorithmException | KeyManagementException | URISyntaxException e) {
+ LOGGER.error("Failed to create storage pool", e);
+ throw new CloudRuntimeException("Failed to establish connection with PowerFlex Gateway to create storage pool");
+ }
+
+ if (!haveConnectedSdcs) {
+ LOGGER.debug("No connected SDCs found for the PowerFlex storage pool");
Review Comment:
We can add more context to the log, like the `dataStoreId`.
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java:
##########
@@ -191,41 +194,59 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore)
final VolumeVO volume = volumeDao.findById(dataObject.getId());
LOGGER.debug("Revoking access for PowerFlex volume: " + volume.getPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for volume: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
- client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(volume.getPath()), sdc.getId());
+ final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
+ client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(volume.getPath()), sdcId);
} else if (DataObjectType.TEMPLATE.equals(dataObject.getType())) {
final VMTemplateStoragePoolVO templatePoolRef = vmTemplatePoolDao.findByPoolTemplate(dataStore.getId(), dataObject.getId(), null);
LOGGER.debug("Revoking access for PowerFlex template volume: " + templatePoolRef.getInstallPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for template: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
- client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(templatePoolRef.getInstallPath()), sdc.getId());
+ final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
+ client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(templatePoolRef.getInstallPath()), sdcId);
} else if (DataObjectType.SNAPSHOT.equals(dataObject.getType())) {
SnapshotInfo snapshot = (SnapshotInfo) dataObject;
LOGGER.debug("Revoking access for PowerFlex volume snapshot: " + snapshot.getPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for snapshot: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
- client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(snapshot.getPath()), sdc.getId());
+ final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
+ client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(snapshot.getPath()), sdcId);
}
} catch (Exception e) {
LOGGER.warn("Failed to revoke access due to: " + e.getMessage(), e);
}
}
+ private 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) {
Review Comment:
Is necessary a catch Pokémon here?
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java:
##########
@@ -69,7 +73,41 @@ public boolean hostConnect(long hostId, long poolId) {
return false;
}
- if (!isHostSdcConnected(host.getPrivateIpAddress(), poolId)) {
+ StoragePool storagePool = (StoragePool)_dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary);
+
+ String systemId = _storagePoolDetailsDao.findDetail(poolId, ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID).getValue();
+ if (systemId == null) {
+ throw new CloudRuntimeException("Failed to get the system id for PowerFlex storage pool " + storagePool.getName());
+ }
+ Map<String,String> details = new HashMap<>();
+ details.put(ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID, systemId);
+
+ ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, storagePool, storagePool.getPath(), details);
+ ModifyStoragePoolAnswer answer = sendModifyStoragePoolCommand(cmd, storagePool, hostId);
+ Map<String,String> poolDetails = answer.getPoolInfo().getDetails();
+ if (MapUtils.isEmpty(poolDetails)) {
+ s_logger.warn("SDC details not found on the host: " + hostId);
+ String msg = "SDC details not found on the host: " + hostId + ", (re)install SDC and restart agent";
Review Comment:
```suggestion
String msg = "SDC details not found on the host: " + hostId + ", (re)install SDC and restart agent";
s_logger.warn(msg);
```
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java:
##########
@@ -69,7 +73,41 @@ public boolean hostConnect(long hostId, long poolId) {
return false;
}
- if (!isHostSdcConnected(host.getPrivateIpAddress(), poolId)) {
+ StoragePool storagePool = (StoragePool)_dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary);
+
+ String systemId = _storagePoolDetailsDao.findDetail(poolId, ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID).getValue();
+ if (systemId == null) {
+ throw new CloudRuntimeException("Failed to get the system id for PowerFlex storage pool " + storagePool.getName());
+ }
+ Map<String,String> details = new HashMap<>();
+ details.put(ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID, systemId);
+
+ ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, storagePool, storagePool.getPath(), details);
+ ModifyStoragePoolAnswer answer = sendModifyStoragePoolCommand(cmd, storagePool, hostId);
+ Map<String,String> poolDetails = answer.getPoolInfo().getDetails();
+ if (MapUtils.isEmpty(poolDetails)) {
+ s_logger.warn("SDC details not found on the host: " + hostId);
+ String msg = "SDC details not found on the host: " + hostId + ", (re)install SDC and restart agent";
+ _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC not found on host: " + host.getUuid(), msg);
+ return false;
+ }
+
+ String sdcId = null;
+ if (poolDetails.containsKey(ScaleIOGatewayClient.SDC_ID)) {
+ sdcId = poolDetails.get(ScaleIOGatewayClient.SDC_ID);
+ } else if (poolDetails.containsKey(ScaleIOGatewayClient.SDC_GUID)) {
+ String sdcGuid = poolDetails.get(ScaleIOGatewayClient.SDC_GUID);
+ sdcId = getHostSdcId(sdcGuid, poolId);
+ }
+
+ if (StringUtils.isBlank(sdcId)) {
+ s_logger.warn("Couldn't retrieve SDC details from the host: " + hostId);
+ String msg = "Couldn't retrieve SDC details from the host: " + hostId + ", (re)install SDC and restart agent";
Review Comment:
```suggestion
String msg = "Couldn't retrieve SDC details from the host: " + hostId + ", (re)install SDC and restart agent";
s_logger.warn(msg);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1128424347
@GutoVeronezi thanks for the review. addressed the review comments, please check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1129128407
<b>Trillian test result (tid-4197)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39668 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6367-t4197-kvm-centos7.zip
Smoke tests completed. 96 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1129615496
> @sureshanaparti do we know anybody who has tested it?
no @rohityadavcloud, @borisstoyanov might test this, but may not be with separate host network (for storage connectivity). Testing without separate host network should be Ok, as this PR removes the dependency of the host's private address for ScaleIO operations, and uses SDC (ScaleIO Data Client) Id instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1161298874
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3606
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1128455994
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6367)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_coverage&view=list) [15.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1129607858
@sureshanaparti do we know anybody who has tested it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1161574578
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1160227888
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1121002718
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1120997728
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3374
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1122237705
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1123227653
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1160197733
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_multiplication_x: suse15. SL-JID 3593
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1162382242
<b>Trillian test result (tid-4346)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36917 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6367-t4346-kvm-centos7.zip
Smoke tests completed. 98 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1161258245
@sureshanaparti a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1160103239
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6367)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_coverage&view=list) [15.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1161504199
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#discussion_r879160463
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java:
##########
@@ -53,6 +54,16 @@ public class ScaleIOUtil {
// VOL-ID 6c33633100000009 MDM-ID 218ce1797566a00f
// VOL-ID 6c3362a30000000a MDM-ID 218ce1797566a00f
+ private static final String QUERY_GUID_CMD = "drv_cfg --query_guid";
+ // Sample output for cmd: drv_cfg --query_guid:
+ // B0E3BFB8-C20B-43BF-93C8-13339E85AA50
Review Comment:
Can we move this to some javadoc? it is a bit out of the blue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1120908938
@sureshanaparti a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1128446951
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1121007323
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6367)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL) [13 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_coverage&view=list) [14.8% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#discussion_r875661080
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java:
##########
@@ -191,41 +194,59 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore)
final VolumeVO volume = volumeDao.findById(dataObject.getId());
LOGGER.debug("Revoking access for PowerFlex volume: " + volume.getPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for volume: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
Review Comment:
this bit seems to be repeated quite a few times. can we extract?
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java:
##########
@@ -69,7 +73,41 @@ public boolean hostConnect(long hostId, long poolId) {
return false;
}
- if (!isHostSdcConnected(host.getPrivateIpAddress(), poolId)) {
+ StoragePool storagePool = (StoragePool)_dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary);
+
+ String systemId = _storagePoolDetailsDao.findDetail(poolId, ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID).getValue();
+ if (systemId == null) {
+ throw new CloudRuntimeException("Failed to get the system id for PowerFlex storage pool " + storagePool.getName());
+ }
+ Map<String,String> details = new HashMap<>();
+ details.put(ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID, systemId);
+
+ ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, storagePool, storagePool.getPath(), details);
+ ModifyStoragePoolAnswer answer = sendModifyStoragePoolCommand(cmd, storagePool, hostId);
+ Map<String,String> poolDetails = answer.getPoolInfo().getDetails();
+ if (MapUtils.isEmpty(poolDetails)) {
+ String msg = "SDC details not found on the host: " + hostId + ", (re)install SDC and restart agent";
+ s_logger.warn(msg);
+ _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC not found on host: " + host.getUuid(), msg);
+ return false;
+ }
+
+ String sdcId = null;
+ if (poolDetails.containsKey(ScaleIOGatewayClient.SDC_ID)) {
+ sdcId = poolDetails.get(ScaleIOGatewayClient.SDC_ID);
+ } else if (poolDetails.containsKey(ScaleIOGatewayClient.SDC_GUID)) {
+ String sdcGuid = poolDetails.get(ScaleIOGatewayClient.SDC_GUID);
+ sdcId = getHostSdcId(sdcGuid, poolId);
+ }
+
+ if (StringUtils.isBlank(sdcId)) {
+ String msg = "Couldn't retrieve SDC details from the host: " + hostId + ", (re)install SDC and restart agent";
+ s_logger.warn(msg);
+ _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC details not found on host: " + host.getUuid(), msg);
+ return false;
+ }
+
+ if (!isHostSdcConnected(sdcId, poolId)) {
Review Comment:
ca this bit be extracted in a few bits/modularised a bit more to improve readability?
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java:
##########
@@ -191,41 +194,59 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore)
final VolumeVO volume = volumeDao.findById(dataObject.getId());
LOGGER.debug("Revoking access for PowerFlex volume: " + volume.getPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for volume: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
Review Comment:
maybe wit an extra `boolean doAlert`
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java:
##########
@@ -53,6 +54,16 @@ public class ScaleIOUtil {
// VOL-ID 6c33633100000009 MDM-ID 218ce1797566a00f
// VOL-ID 6c3362a30000000a MDM-ID 218ce1797566a00f
+ private static final String QUERY_GUID_CMD = "drv_cfg --query_guid";
+ // Sample output for cmd: drv_cfg --query_guid:
+ // B0E3BFB8-C20B-43BF-93C8-13339E85AA50
+
+ private static final String QUERY_MDMS_CMD = "drv_cfg --query_mdms";
+ // Sample output for cmd: drv_cfg --query_mdms:
+ // Retrieved 2 mdm(s)
+ // MDM-ID 3ef46cbf2aaf5d0f SDC ID 6b18479c00000003 INSTALLATION ID 68ab55462cbb3ae4 IPs [0]-x.x.x.x [1]-x.x.x.x
+ // MDM-ID 2e706b2740ec200f SDC ID 301b852c00000003 INSTALLATION ID 33f8662e7a5c1e6c IPs [0]-x.x.x.x [1]-x.x.x.x
Review Comment:
javadoc?
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java:
##########
@@ -191,41 +194,59 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore)
final VolumeVO volume = volumeDao.findById(dataObject.getId());
LOGGER.debug("Revoking access for PowerFlex volume: " + volume.getPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for volume: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
- client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(volume.getPath()), sdc.getId());
+ final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
+ client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(volume.getPath()), sdcId);
} else if (DataObjectType.TEMPLATE.equals(dataObject.getType())) {
final VMTemplateStoragePoolVO templatePoolRef = vmTemplatePoolDao.findByPoolTemplate(dataStore.getId(), dataObject.getId(), null);
LOGGER.debug("Revoking access for PowerFlex template volume: " + templatePoolRef.getInstallPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for template: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
- client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(templatePoolRef.getInstallPath()), sdc.getId());
+ final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
+ client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(templatePoolRef.getInstallPath()), sdcId);
} else if (DataObjectType.SNAPSHOT.equals(dataObject.getType())) {
SnapshotInfo snapshot = (SnapshotInfo) dataObject;
LOGGER.debug("Revoking access for PowerFlex volume snapshot: " + snapshot.getPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for snapshot: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
- client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(snapshot.getPath()), sdc.getId());
+ final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
+ client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(snapshot.getPath()), sdcId);
}
} catch (Exception e) {
LOGGER.warn("Failed to revoke access due to: " + e.getMessage(), e);
}
}
+ private 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) {
Review Comment:
storage maybe you want to inform in the message that the problem is with the client and not with the MS-code?
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java:
##########
@@ -53,6 +54,16 @@ public class ScaleIOUtil {
// VOL-ID 6c33633100000009 MDM-ID 218ce1797566a00f
// VOL-ID 6c3362a30000000a MDM-ID 218ce1797566a00f
+ private static final String QUERY_GUID_CMD = "drv_cfg --query_guid";
+ // Sample output for cmd: drv_cfg --query_guid:
+ // B0E3BFB8-C20B-43BF-93C8-13339E85AA50
Review Comment:
is this javadoc for the above constant?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1161255851
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1120908661
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1122238839
@sureshanaparti a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1128445673
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3418
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1160823537
<b>Trillian test result (tid-4338)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36727 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6367-t4338-kvm-centos7.zip
Smoke tests completed. 98 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1161516200
<b>Trillian Build Failed (tid-4345)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a diff in pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on code in PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#discussion_r879159562
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java:
##########
@@ -53,6 +54,16 @@ public class ScaleIOUtil {
// VOL-ID 6c33633100000009 MDM-ID 218ce1797566a00f
// VOL-ID 6c3362a30000000a MDM-ID 218ce1797566a00f
+ private static final String QUERY_GUID_CMD = "drv_cfg --query_guid";
+ // Sample output for cmd: drv_cfg --query_guid:
+ // B0E3BFB8-C20B-43BF-93C8-13339E85AA50
+
+ private static final String QUERY_MDMS_CMD = "drv_cfg --query_mdms";
+ // Sample output for cmd: drv_cfg --query_mdms:
+ // Retrieved 2 mdm(s)
+ // MDM-ID 3ef46cbf2aaf5d0f SDC ID 6b18479c00000003 INSTALLATION ID 68ab55462cbb3ae4 IPs [0]-x.x.x.x [1]-x.x.x.x
+ // MDM-ID 2e706b2740ec200f SDC ID 301b852c00000003 INSTALLATION ID 33f8662e7a5c1e6c IPs [0]-x.x.x.x [1]-x.x.x.x
Review Comment:
same, it's to indicate sample output for the cmd.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud merged pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
rohityadavcloud merged PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1120929837
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 3371
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1161292014
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6367)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_coverage&view=list) [15.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1161574134
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1123231165
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6367 (SL-JID-1546)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a diff in pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on code in PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#discussion_r874353922
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java:
##########
@@ -191,41 +194,59 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore)
final VolumeVO volume = volumeDao.findById(dataObject.getId());
LOGGER.debug("Revoking access for PowerFlex volume: " + volume.getPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for volume: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
- client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(volume.getPath()), sdc.getId());
+ final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
+ client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(volume.getPath()), sdcId);
} else if (DataObjectType.TEMPLATE.equals(dataObject.getType())) {
final VMTemplateStoragePoolVO templatePoolRef = vmTemplatePoolDao.findByPoolTemplate(dataStore.getId(), dataObject.getId(), null);
LOGGER.debug("Revoking access for PowerFlex template volume: " + templatePoolRef.getInstallPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for template: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
- client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(templatePoolRef.getInstallPath()), sdc.getId());
+ final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
+ client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(templatePoolRef.getInstallPath()), sdcId);
} else if (DataObjectType.SNAPSHOT.equals(dataObject.getType())) {
SnapshotInfo snapshot = (SnapshotInfo) dataObject;
LOGGER.debug("Revoking access for PowerFlex volume snapshot: " + snapshot.getPath());
- final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
- final Sdc sdc = client.getConnectedSdcByIp(host.getPrivateIpAddress());
- if (sdc == null) {
+ final String sdcId = getConnectedSdc(dataStore.getId(), host.getId());
+ if (StringUtils.isBlank(sdcId)) {
throw new CloudRuntimeException("Unable to revoke access for snapshot: " + dataObject.getId() + ", no Sdc connected with host ip: " + host.getPrivateIpAddress());
}
- client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(snapshot.getPath()), sdc.getId());
+ final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
+ client.unmapVolumeFromSdc(ScaleIOUtil.getVolumePath(snapshot.getPath()), sdcId);
}
} catch (Exception e) {
LOGGER.warn("Failed to revoke access due to: " + e.getMessage(), e);
}
}
+ private 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) {
Review Comment:
yes, any issue with storage client is caught here & logged, and returns null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a diff in pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on code in PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#discussion_r879158891
##########
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java:
##########
@@ -53,6 +54,16 @@ public class ScaleIOUtil {
// VOL-ID 6c33633100000009 MDM-ID 218ce1797566a00f
// VOL-ID 6c3362a30000000a MDM-ID 218ce1797566a00f
+ private static final String QUERY_GUID_CMD = "drv_cfg --query_guid";
+ // Sample output for cmd: drv_cfg --query_guid:
+ // B0E3BFB8-C20B-43BF-93C8-13339E85AA50
Review Comment:
> is this javadoc for the above constant?
no, just to indicate sample output for the cmd.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] acs-robot commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1161255551
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1161258121
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1161278973
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6367)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6367&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6367&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_coverage&view=list) [15.4% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6367&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1160057437
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1161505362
@rohityadavcloud a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1120961823
@sureshanaparti a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1122275252
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3379
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1123228688
Hi @rohityadavcloud No UI changes in this PR, but bot detected UI changes. Can you check/fix it. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] acs-robot commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1123226871
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1128447471
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1128424709
@sureshanaparti a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #6367: Updated PowerFlex/ScaleIO storage plugin to support separate (storage) network for Hosts(KVM)/Storage connection.
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on PR #6367:
URL: https://github.com/apache/cloudstack/pull/6367#issuecomment-1128424524
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org