Skip to content

PowerFlex/ScaleIO client initialization, authentication and command execution improvements #12391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
sureshanaparti wants to merge 1 commit into
base: 4.22
Choose a base branch
from
Draft

PowerFlex/ScaleIO client initialization, authentication and command execution improvements #12391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,13 @@ protected T valueInGlobalOrAvailableParentScope(Scope scope, Long id) {
return value();
}

/**
* Use {@link ConfigKey#valueInScope(Scope, Long)} instead.
*/
public T valueInDomain(Long domainId) {
return valueInScope(Scope.Domain, domainId);
}
Comment on lines +403 to +408
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation comment is misleading. It states "Use ConfigKey#valueInScope(Scope, Long) instead" which suggests this method is deprecated, but there is no @deprecated annotation. If this is a deprecated method meant to guide users to the preferred API, it should have the @deprecated annotation. If it's just a convenience method, the documentation should clarify that rather than suggesting users should use a different method.

Copilot uses AI. Check for mistakes.

public T valueInScope(Scope scope, Long id) {
if (id == null) {
return value();
Expand Down
7 changes: 4 additions & 3 deletions ...rvisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ public void testUnprepareStorageClient_MDMNotAdded() {
details.put(ScaleIOGatewayClient.STORAGE_POOL_MDMS, "1.1.1.1,2.2.2.2");
when(Script.runSimpleBashScriptForExitValue(Mockito.eq("systemctl status scini"))).thenReturn(3);
when(Script.runSimpleBashScriptForExitValue(Mockito.eq("systemctl is-enabled scini"))).thenReturn(0);
when(Script.runSimpleBashScript(Mockito.eq("/opt/emc/scaleio/sdc/bin/drv_cfg --query_mdms|grep 1.1.1.1"))).thenReturn("MDM-ID 71fd458f0775010f SDC ID 4421a91a00000000 INSTALLATION ID 204930df2cbcaf8e IPs [0]-3.3.3.3 [1]-4.4.4.4");
when(Script.runSimpleBashScript(Mockito.eq("/opt/emc/scaleio/sdc/bin/drv_cfg --query_mdms|grep 1.1.1.1"), Mockito.eq(ScaleIOUtil.DEFAULT_TIMEOUT_MS)))
.thenReturn("MDM-ID 71fd458f0775010f SDC ID 4421a91a00000000 INSTALLATION ID 204930df2cbcaf8e IPs [0]-3.3.3.3 [1]-4.4.4.4");

Pair<Boolean, String> result = scaleIOStorageAdaptor.unprepareStorageClient(poolUuid, details);

Expand All @@ -196,11 +197,11 @@ public void testUnprepareStorageClient_RemoveMDMFailed() {
when(Script.runSimpleBashScriptForExitValue(Mockito.eq("systemctl is-enabled scini"))).thenReturn(0);
when(Script.executeCommand(Mockito.eq("sed -i '/1.1.1.1\\,/d' /etc/emc/scaleio/drv_cfg.txt"))).thenReturn(new Pair<>(null, null));
when(Script.runSimpleBashScriptForExitValue(Mockito.eq("systemctl restart scini"))).thenReturn(0);
when(Script.runSimpleBashScript(Mockito.eq("/opt/emc/scaleio/sdc/bin/drv_cfg --query_mdms|grep 1.1.1.1"))).thenReturn("MDM-ID 71fd458f0775010f SDC ID 4421a91a00000000 INSTALLATION ID 204930df2cbcaf8e IPs [0]-1.1.1.1 [1]-2.2.2.2");
when(Script.runSimpleBashScript(Mockito.eq("/opt/emc/scaleio/sdc/bin/drv_cfg --query_mdms --file /etc/emc/scaleio/drv_cfg.txt|grep 1.1.1.1"), Mockito.eq(ScaleIOUtil.DEFAULT_TIMEOUT_MS)))
.thenReturn("MDM-ID 71fd458f0775010f SDC ID 4421a91a00000000 INSTALLATION ID 204930df2cbcaf8e IPs [0]-1.1.1.1 [1]-2.2.2.2");
when(Script.executeCommand(Mockito.eq("/opt/emc/scaleio/sdc/bin/drv_cfg"))).thenReturn(new Pair<>(null, null));
when(Script.executeCommand(Mockito.eq("/opt/emc/scaleio/sdc/bin/drv_cfg --query_vols"))).thenReturn(new Pair<>("", null));


Pair<Boolean, String> result = scaleIOStorageAdaptor.unprepareStorageClient(poolUuid, details);

Assert.assertFalse(result.first());
Expand Down
13 changes: 6 additions & 7 deletions ...ypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ScaleIOStoragePoolTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,8 @@ public void testSdcIdAttribute() {
details.put(ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID, systemId);

try (MockedStatic<Script> ignored = Mockito.mockStatic(Script.class)) {
when(Script.runSimpleBashScript(
"/opt/emc/scaleio/sdc/bin/drv_cfg --query_mdms|grep 218ce1797566a00f|awk '{print $5}'")).thenReturn(
sdcId);
when(Script.runSimpleBashScript(Mockito.eq("/opt/emc/scaleio/sdc/bin/drv_cfg --query_mdms --file /etc/emc/scaleio/drv_cfg.txt|grep 218ce1797566a00f|awk '{print $5}'"), Mockito.eq(ScaleIOUtil.DEFAULT_TIMEOUT_MS)))
.thenReturn(sdcId);

ScaleIOStoragePool pool1 = new ScaleIOStoragePool(uuid, "192.168.1.19", 443, "a519be2f00000000", type,
details, adapter);
Expand All @@ -116,10 +115,10 @@ public void testSdcGuidAttribute() {
details.put(ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID, systemId);

try (MockedStatic<Script> ignored = Mockito.mockStatic(Script.class)) {
when(Script.runSimpleBashScript(
"/opt/emc/scaleio/sdc/bin/drv_cfg --query_mdms|grep 218ce1797566a00f|awk '{print $5}'")).thenReturn(
null);
when(Script.runSimpleBashScript("/opt/emc/scaleio/sdc/bin/drv_cfg --query_guid")).thenReturn(sdcGuid);
when(Script.runSimpleBashScript(Mockito.eq("/opt/emc/scaleio/sdc/bin/drv_cfg --query_mdms --file /etc/emc/scaleio/drv_cfg.txt|grep 218ce1797566a00f|awk '{print $5}'"), Mockito.eq(ScaleIOUtil.DEFAULT_TIMEOUT_MS)))
.thenReturn(null);
when(Script.runSimpleBashScript(Mockito.eq("/opt/emc/scaleio/sdc/bin/drv_cfg --query_guid --file /etc/emc/scaleio/drv_cfg.txt"), Mockito.eq(ScaleIOUtil.DEFAULT_TIMEOUT_MS)))
.thenReturn(sdcGuid);

ScaleIOStoragePool pool1 = new ScaleIOStoragePool(uuid, "192.168.1.19", 443, "a519be2f00000000", type,
details, adapter);
Expand Down
89 changes: 48 additions & 41 deletions ...va/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientConnectionPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@

package org.apache.cloudstack.storage.datastore.client;

import java.net.URISyntaxException;
import java.security.KeyManagementException;
import java.security.NoSuchAlgorithmException;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;

import com.cloud.storage.StoragePool;
import com.cloud.utils.exception.CloudRuntimeException;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
Expand All @@ -36,9 +36,9 @@
public class ScaleIOGatewayClientConnectionPool {
protected Logger logger = LogManager.getLogger(getClass());

private ConcurrentHashMap<Long, ScaleIOGatewayClient> gatewayClients;

private Map<Long, ScaleIOGatewayClient> gatewayClients;
private static final ScaleIOGatewayClientConnectionPool instance;
private final Object lock = new Object();

static {
instance = new ScaleIOGatewayClientConnectionPool();
Expand All @@ -49,69 +49,76 @@ public static ScaleIOGatewayClientConnectionPool getInstance() {
}

private ScaleIOGatewayClientConnectionPool() {
gatewayClients = new ConcurrentHashMap<Long, ScaleIOGatewayClient>();
gatewayClients = new ConcurrentHashMap<>();
}

public ScaleIOGatewayClient getClient(StoragePool storagePool,
StoragePoolDetailsDao storagePoolDetailsDao)
throws NoSuchAlgorithmException, KeyManagementException, URISyntaxException {
StoragePoolDetailsDao storagePoolDetailsDao) {
return getClient(storagePool.getId(), storagePool.getUuid(), storagePoolDetailsDao);
}


public ScaleIOGatewayClient getClient(DataStore dataStore,
StoragePoolDetailsDao storagePoolDetailsDao)
throws NoSuchAlgorithmException, KeyManagementException, URISyntaxException {
StoragePoolDetailsDao storagePoolDetailsDao) {
return getClient(dataStore.getId(), dataStore.getUuid(), storagePoolDetailsDao);
}


private ScaleIOGatewayClient getClient(Long storagePoolId, String storagePoolUuid,
StoragePoolDetailsDao storagePoolDetailsDao)
throws NoSuchAlgorithmException, KeyManagementException, URISyntaxException {
StoragePoolDetailsDao storagePoolDetailsDao) {

Preconditions.checkArgument(storagePoolId != null && storagePoolId > 0,
"Invalid storage pool id");

ScaleIOGatewayClient client = null;
synchronized (gatewayClients) {
client = gatewayClients.get(storagePoolId);
if (client == null) {
String url = null;
StoragePoolDetailVO urlDetail = storagePoolDetailsDao.findDetail(storagePoolId, ScaleIOGatewayClient.GATEWAY_API_ENDPOINT);
if (urlDetail != null) {
url = urlDetail.getValue();
}
String username = null;
StoragePoolDetailVO encryptedUsernameDetail = storagePoolDetailsDao.findDetail(storagePoolId, ScaleIOGatewayClient.GATEWAY_API_USERNAME);
if (encryptedUsernameDetail != null) {
final String encryptedUsername = encryptedUsernameDetail.getValue();
username = DBEncryptionUtil.decrypt(encryptedUsername);
logger.debug("Getting ScaleIO client for {} ({})", storagePoolId, storagePoolUuid);

ScaleIOGatewayClient client = gatewayClients.get(storagePoolId);
if (client == null) {
logger.debug("Before acquiring lock to create ScaleIO client for {} ({})", storagePoolId, storagePoolUuid);
synchronized (lock) {
logger.debug("Acquired lock to create ScaleIO client for {} ({})", storagePoolId, storagePoolUuid);
client = gatewayClients.get(storagePoolId);
if (client == null) {
logger.debug("Initializing ScaleIO client for {} ({})", storagePoolId, storagePoolUuid);

String url = Optional.ofNullable(storagePoolDetailsDao.findDetail(storagePoolId, ScaleIOGatewayClient.GATEWAY_API_ENDPOINT))
.map(StoragePoolDetailVO::getValue)
.orElse(null);

String username = Optional.ofNullable(storagePoolDetailsDao.findDetail(storagePoolId, ScaleIOGatewayClient.GATEWAY_API_USERNAME))
.map(StoragePoolDetailVO::getValue)
.map(DBEncryptionUtil::decrypt)
.orElse(null);

String password = Optional.ofNullable(storagePoolDetailsDao.findDetail(storagePoolId, ScaleIOGatewayClient.GATEWAY_API_PASSWORD))
.map(StoragePoolDetailVO::getValue)
.map(DBEncryptionUtil::decrypt)
.orElse(null);

int clientTimeout = StorageManager.STORAGE_POOL_CLIENT_TIMEOUT.valueIn(storagePoolId);
int clientMaxConnections = StorageManager.STORAGE_POOL_CLIENT_MAX_CONNECTIONS.valueIn(storagePoolId);

try {
client = new ScaleIOGatewayClientImpl(url, username, password, false, clientTimeout, clientMaxConnections);
logger.debug("Created ScaleIO client for the storage pool [id: {}, uuid: {}]", storagePoolId, storagePoolUuid);
gatewayClients.put(storagePoolId, client);
} catch (Exception e) {
String msg = String.format("Failed to create ScaleIO client for the storage pool [id: %d, uuid: %s]", storagePoolId, storagePoolUuid);
throw new CloudRuntimeException(msg, e);
}
}
String password = null;
StoragePoolDetailVO encryptedPasswordDetail = storagePoolDetailsDao.findDetail(storagePoolId, ScaleIOGatewayClient.GATEWAY_API_PASSWORD);
if (encryptedPasswordDetail != null) {
final String encryptedPassword = encryptedPasswordDetail.getValue();
password = DBEncryptionUtil.decrypt(encryptedPassword);
}
final int clientTimeout = StorageManager.STORAGE_POOL_CLIENT_TIMEOUT.valueIn(storagePoolId);
final int clientMaxConnections = StorageManager.STORAGE_POOL_CLIENT_MAX_CONNECTIONS.valueIn(storagePoolId);

client = new ScaleIOGatewayClientImpl(url, username, password, false, clientTimeout, clientMaxConnections);
gatewayClients.put(storagePoolId, client);
logger.debug("Added gateway client for the storage pool [id: {}, uuid: {}]", storagePoolId, storagePoolUuid);
}
}

logger.debug("Returning ScaleIO client for {} ({})", storagePoolId, storagePoolUuid);
return client;
}

public boolean removeClient(DataStore dataStore) {
Preconditions.checkArgument(dataStore != null && dataStore.getId() > 0,
"Invalid storage pool id");

ScaleIOGatewayClient client = null;
synchronized (gatewayClients) {
ScaleIOGatewayClient client;
synchronized (lock) {
client = gatewayClients.remove(dataStore.getId());
}

Expand Down
22 changes: 16 additions & 6 deletions ...rc/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public class ScaleIOGatewayClientImpl implements ScaleIOGatewayClient {

private String username;
private String password;
private String sessionKey = null;
private String sessionKey;

// The session token is valid for 8 hours from the time it was created, unless there has been no activity for 10 minutes
// Reference: https://cpsdocs.dellemc.com/bundle/PF_REST_API_RG/page/GUID-92430F19-9F44-42B6-B898-87D5307AE59B.html
Expand All @@ -102,7 +102,7 @@ public class ScaleIOGatewayClientImpl implements ScaleIOGatewayClient {
private static final long MAX_IDLE_TIME_IN_MILLISECS = MAX_IDLE_TIME_IN_MINS * 60 * 1000;
private static final long BUFFER_TIME_IN_MILLISECS = 30 * 1000; // keep 30 secs buffer before the expiration (to avoid any last-minute operations)

private boolean authenticating = false;
private volatile boolean authenticating = false;
private long createTime = 0;
private long lastUsedTime = 0;

Expand Down Expand Up @@ -142,7 +142,6 @@ public ScaleIOGatewayClientImpl(final String url, final String username, final S
this.username = username;
this.password = password;

authenticate();
logger.debug("API client for the PowerFlex gateway " + apiURI.getHost() + " is created successfully, with max connections: "
+ maxConnections + " and timeout: " + timeout + " secs");
}
Expand Down Expand Up @@ -181,7 +180,7 @@ private synchronized void authenticate() {
long now = System.currentTimeMillis();
createTime = lastUsedTime = now;
} catch (final IOException e) {
logger.error("Failed to authenticate PowerFlex API Gateway " + apiURI.getHost() + " due to: " + e.getMessage() + getConnectionManagerStats());
logger.error("Failed to authenticate PowerFlex API Gateway " + apiURI.getHost() + " due to: " + e.getMessage() + getConnectionManagerStats(), e);
throw new CloudRuntimeException("Failed to authenticate PowerFlex API Gateway " + apiURI.getHost() + " due to: " + e.getMessage());
} finally {
authenticating = false;
Expand All @@ -199,6 +198,10 @@ private synchronized void renewClientSessionOnExpiry() {
}

private boolean isSessionExpired() {
if (sessionKey == null) {
logger.debug("Session never created for the Gateway " + apiURI.getHost());
return true;
}
long now = System.currentTimeMillis() + BUFFER_TIME_IN_MILLISECS;
if ((now - createTime) > MAX_VALID_SESSION_TIME_IN_MILLISECS) {
logger.debug("Session expired for the Gateway " + apiURI.getHost() + ", token is invalid after " + MAX_VALID_SESSION_TIME_IN_HRS
Expand Down Expand Up @@ -281,7 +284,11 @@ private <T> T get(final String path, final Class<T> type, final boolean renewAnd
HttpResponse response = null;
boolean responseConsumed = false;
try {
while (authenticating); // wait for authentication request (if any) to complete (and to pick the new session key)
while (authenticating) { // wait for authentication request (if any)
// to complete (and to pick the new session key)
Thread.yield();
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The busy-wait loop with Thread.yield() can cause excessive CPU usage when waiting for authentication to complete. Thread.yield() is a hint to the scheduler but provides no guaranteed waiting period, so the loop may spin very rapidly consuming CPU cycles. Consider using a small Thread.sleep() instead, or better yet, using proper synchronization primitives like CountDownLatch or wait/notify to eliminate busy-waiting entirely.

Suggested change
Thread.yield();
try {
Thread.sleep(10L);
} catch (InterruptedException ie) {
Thread.currentThread().interrupt();
break;
}

Copilot uses AI. Check for mistakes.
}

final HttpGet request = new HttpGet(apiURI.toString() + path);
request.setHeader(HttpHeaders.AUTHORIZATION, "Basic " + Base64.getEncoder().encodeToString((this.username + ":" + this.sessionKey).getBytes()));
logger.debug("Sending GET request: " + request.toString());
Expand Down Expand Up @@ -316,7 +323,10 @@ private <T> T post(final String path, final Object obj, final Class<T> type, fin
HttpResponse response = null;
boolean responseConsumed = false;
try {
while (authenticating); // wait for authentication request (if any) to complete (and to pick the new session key)
while (authenticating) { // wait for authentication request (if any)
// to complete (and to pick the new session key)
Thread.yield();
}
final HttpPost request = new HttpPost(apiURI.toString() + path);
request.setHeader(HttpHeaders.AUTHORIZATION, "Basic " + Base64.getEncoder().encodeToString((this.username + ":" + this.sessionKey).getBytes()));
request.setHeader("content-type", "application/json");
Expand Down
10 changes: 5 additions & 5 deletions ...in/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,15 +153,15 @@ public ScaleIOPrimaryDataStoreDriver() {
sdcManager = new ScaleIOSDCManagerImpl();
}

ScaleIOGatewayClient getScaleIOClient(final StoragePool storagePool) throws Exception {
ScaleIOGatewayClient getScaleIOClient(final StoragePool storagePool) {
return ScaleIOGatewayClientConnectionPool.getInstance().getClient(storagePool, storagePoolDetailsDao);
}

ScaleIOGatewayClient getScaleIOClient(final DataStore dataStore) throws Exception {
ScaleIOGatewayClient getScaleIOClient(final DataStore dataStore) {
return ScaleIOGatewayClientConnectionPool.getInstance().getClient(dataStore, storagePoolDetailsDao);
}

private boolean setVolumeLimitsOnSDC(VolumeVO volume, Host host, DataStore dataStore, Long iopsLimit, Long bandwidthLimitInKbps) throws Exception {
private boolean setVolumeLimitsOnSDC(VolumeVO volume, Host host, DataStore dataStore, Long iopsLimit, Long bandwidthLimitInKbps) {
sdcManager = ComponentContext.inject(sdcManager);
final String sdcId = sdcManager.prepareSDC(host, dataStore);
if (StringUtils.isBlank(sdcId)) {
Expand All @@ -173,7 +173,7 @@ private boolean setVolumeLimitsOnSDC(VolumeVO volume, Host host, DataStore dataS
return client.mapVolumeToSdcWithLimits(ScaleIOUtil.getVolumePath(volume.getPath()), sdcId, iopsLimit, bandwidthLimitInKbps);
}

private boolean setVolumeLimitsFromDetails(VolumeVO volume, Host host, DataStore dataStore) throws Exception {
private boolean setVolumeLimitsFromDetails(VolumeVO volume, Host host, DataStore dataStore) {
Long bandwidthLimitInKbps = 0L; // Unlimited
// Check Bandwidth Limit parameter in volume details
final VolumeDetailVO bandwidthVolumeDetail = volumeDetailsDao.findDetail(volume.getId(), Volume.BANDWIDTH_LIMIT_IN_MBPS);
Expand Down Expand Up @@ -997,7 +997,7 @@ private Host findEndpointForVolumeOperation(DataObject srcData) {
return host;
}

public void updateSnapshotsAfterCopyVolume(DataObject srcData, DataObject destData) throws Exception {
public void updateSnapshotsAfterCopyVolume(DataObject srcData, DataObject destData) {
final long srcVolumeId = srcData.getId();
DataStore srcStore = srcData.getDataStore();
final ScaleIOGatewayClient client = getScaleIOClient(srcStore);
Expand Down
Loading
Loading