Skip to content

Bugfix/cstackex 327-Concurrent iSCSI VM deployment fails intermittently: post-clone LUN resize returns 404 #118

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

Open
sp16743 wants to merge 4 commits into
base: main
Choose a base branch
from
+91 −2
Open

Bugfix/cstackex 327-Concurrent iSCSI VM deployment fails intermittently: post-clone LUN resize returns 404 #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
182b07b
Fix ONTAP LUN clone visibility using GET retry
sp16743 Oct 26, 2026
a0586de
Change the cloned LUN read failure message
sp16743 Nov 4, 2026
99194b7
Addressing Comments
sp16743 Nov 12, 2026
e68f745
Restore unused LUN endpoint
sp16743 Nov 12, 2026
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
86 changes: 84 additions & 2 deletions .../volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) {
*
* <p>The resulting LUN shares blocks with its source and is created in constant time.
* It is a sis-clone, so it stays readable after the source LUN is deleted.</p>
*
* <p>The create response includes the LUN uuid. That uuid is confirmed with repeated
* {@code GET /api/storage/luns?uuid=} calls until the response contains a record.</p>
*/
@Override
public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) {
Expand All @@ -247,14 +250,18 @@ public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume)
lunRequest.getName(),
lunRequest.getClone().getSource().getName(),
lunRequest.getClone().getSource().getUuid());
String lunUuid = null;
try {
String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword());
OntapResponse<Lun> clonedLun = sanFeignClient.createLun(authHeader, true, lunRequest);
if (clonedLun == null || CollectionUtils.isEmpty(clonedLun.getRecords())) {
if (clonedLun == null || CollectionUtils.isEmpty(clonedLun.getRecords())
|| clonedLun.getRecords().get(0) == null
|| StringUtils.isBlank(clonedLun.getRecords().get(0).getUuid())) {
logger.error("cloneCloudStackVolume: LUN clone returned no records for Lun {}", lunRequest.getName());
throw new CloudRuntimeException("Failed to clone Lun: " + lunRequest.getName());
}
Lun lun = clonedLun.getRecords().get(0);
lunUuid = clonedLun.getRecords().get(0).getUuid();
Lun lun = getLunByUuidUntilVisible(authHeader, lunUuid, lunRequest.getName());

@suryag1201 suryag1201 Nov 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

what if the get lun call failed, cloned lun will. still exists and retry will see the duplicate lun

@sp16743 sp16743 Nov 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added it.The cloned LUN is deleted ,so a subsequent CloudStack retry does not encounter a duplicate LUN name.

validateCreatedLun(lun, lunRequest.getName(), "cloneCloudStackVolume");
logger.debug("cloneCloudStackVolume: LUN cloned successfully. Lun: {}", lun);

Expand All @@ -263,17 +270,92 @@ public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume)
clonedCloudStackVolume.setVolumeInfo(cloudstackVolume.getVolumeInfo());
return clonedCloudStackVolume;
} catch (FeignException e) {
bestEffortDeleteClonedLun(lunUuid, lunRequest.getName());
logger.error("FeignException occurred while cloning LUN: {}, Status: {}, Exception: {}",
lunRequest.getName(), e.status(), e.getMessage());
throw new CloudRuntimeException("Failed to clone Lun: " + e.getMessage());
} catch (CloudRuntimeException e) {
bestEffortDeleteClonedLun(lunUuid, lunRequest.getName());
throw e;
} catch (Exception e) {
bestEffortDeleteClonedLun(lunUuid, lunRequest.getName());
logger.error("Exception occurred while cloning LUN: {}, Exception: {}", lunRequest.getName(), e.getMessage());
throw new CloudRuntimeException("Failed to clone Lun: " + e.getMessage());
}
}

/**
* Removes a clone that was created but could not be confirmed.
*
* <p>The LUN is already on ONTAP once create returns a uuid. Leaving it in place makes the
* next clone of the same name fail as a duplicate.</p>
*/
private void bestEffortDeleteClonedLun(String lunUuid, String lunName) {
if (StringUtils.isBlank(lunUuid)) {
return;
}
try {
Lun lun = new Lun();
lun.setUuid(lunUuid);
lun.setName(lunName);
CloudStackVolume deleteRequest = new CloudStackVolume();
deleteRequest.setLun(lun);
deleteCloudStackVolume(deleteRequest);
logger.info("bestEffortDeleteClonedLun: Removed leftover cloned LUN [{}] uuid [{}]",
lunName, lunUuid);
} catch (Exception cleanupEx) {
logger.warn("bestEffortDeleteClonedLun: Failed to remove leftover cloned LUN [{}] uuid [{}]: {}",
lunName, lunUuid, cleanupEx.getMessage());
}
}

/**
* Confirms a clone uuid from the collection GET record.
*
* <p>{@code GET /api/storage/luns?uuid=} returns no records until the clone is visible.
* Empty responses and 404s are retried. Any other GET failure is returned to the caller.</p>
*/
private Lun getLunByUuidUntilVisible(String authHeader, String lunUuid, String lunName) {
Map<String, Object> queryParams = Map.of(OntapStorageConstants.UUID, lunUuid);
for (int attempt = 1; attempt <= OntapStorageConstants.ONTAP_LUN_CLONE_GET_MAX_RETRIES; attempt++) {
try {
OntapResponse<Lun> lunResponse = sanFeignClient.getLunResponse(authHeader, queryParams);
if (lunResponse != null && !CollectionUtils.isEmpty(lunResponse.getRecords())
&& lunResponse.getRecords().get(0) != null) {
Lun lun = lunResponse.getRecords().get(0);
if (StringUtils.isBlank(lun.getUuid())) {
lun.setUuid(lunUuid);
}
if (lun.getName() == null) {
lun.setName(lunName);
}
logger.debug("cloneCloudStackVolume: LUN uuid [{}] is visible on attempt {}/{}",
lunUuid, attempt, OntapStorageConstants.ONTAP_LUN_CLONE_GET_MAX_RETRIES);
return lun;
}
logger.info("cloneCloudStackVolume: LUN uuid [{}] is not visible yet (attempt {}/{})",
lunUuid, attempt, OntapStorageConstants.ONTAP_LUN_CLONE_GET_MAX_RETRIES);
} catch (FeignException e) {
if (e.status() != 404) {
logger.error("cloneCloudStackVolume: GET LUN uuid [{}] failed, Status: {}, Exception: {}",
lunUuid, e.status(), e.getMessage());
throw new CloudRuntimeException("Failed to fetch cloned Lun: " + e.getMessage());
}
logger.info("cloneCloudStackVolume: GET LUN uuid [{}] returned 404 (attempt {}/{})",
lunUuid, attempt, OntapStorageConstants.ONTAP_LUN_CLONE_GET_MAX_RETRIES);
}
if (attempt < OntapStorageConstants.ONTAP_LUN_CLONE_GET_MAX_RETRIES) {
try {
Thread.sleep(OntapStorageConstants.ONTAP_LUN_CLONE_GET_POLL_INTERVAL_MS);
} catch (InterruptedException interrupted) {
Thread.currentThread().interrupt();
throw new CloudRuntimeException("Interrupted while waiting for cloned Lun " + lunName, interrupted);
}
}
}
throw new CloudRuntimeException("Failed to read cloned Lun " + lunName);
}

/**
* Ensures ONTAP returned a usable LUN identity from create/clone. Callers in the datastore
* driver rely on non-null name and uuid, so reject incomplete records at the Feign boundary.
Expand Down
3 changes: 3 additions & 0 deletions ...volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@ public class OntapStorageConstants {
public static final int ONTAP_VOLUME_JOB_POLL_INTERVAL_MS = 1000;
public static final int ONTAP_FLEXVOL_JOB_POLL_INTERVAL_MS = 2000;
public static final int ONTAP_FLEXVOL_RESOLVE_MAX_RETRIES = 30;
/** GET-by-UUID retries after LUN clone response. */
public static final int ONTAP_LUN_CLONE_GET_MAX_RETRIES = 10;
public static final int ONTAP_LUN_CLONE_GET_POLL_INTERVAL_MS = 1000;

/** vm_snapshot_details key for ONTAP FlexVolume-level VM snapshots. */
public static final String ONTAP_FLEXVOL_SNAPSHOT = "ontapFlexVolSnapshot";
Expand Down
4 changes: 4 additions & 0 deletions ...ume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1025,13 +1025,15 @@ void testCloneCloudStackVolume_Success() {

when(sanFeignClient.createLun(eq(authHeader), eq(true), any(Lun.class)))
.thenReturn(response);
when(sanFeignClient.getLunResponse(eq(authHeader), anyMap())).thenReturn(response);

CloudStackVolume result = unifiedSANStrategy.cloneCloudStackVolume(request);

assertNotNull(result);
assertEquals("cloned-lun-uuid", result.getLun().getUuid());
assertSame(volumeInfo, result.getVolumeInfo());
verify(sanFeignClient).createLun(eq(authHeader), eq(true), any(Lun.class));
verify(sanFeignClient).getLunResponse(eq(authHeader), anyMap());
}
}

Expand Down Expand Up @@ -1074,6 +1076,7 @@ void testCloneCloudStackVolume_SnapshotSource_SendsSnapshotQualifiedName() {
utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password"))
.thenReturn(authHeader);
when(sanFeignClient.createLun(eq(authHeader), eq(true), any(Lun.class))).thenReturn(response);
when(sanFeignClient.getLunResponse(eq(authHeader), anyMap())).thenReturn(response);

CloudStackVolume result = unifiedSANStrategy.cloneCloudStackVolume(request);

Expand Down Expand Up @@ -1334,6 +1337,7 @@ void testCloneCloudStackVolume_FeignException_ThrowsException() {

assertThrows(CloudRuntimeException.class,
() -> unifiedSANStrategy.cloneCloudStackVolume(request));
verify(sanFeignClient, never()).deleteLun(any(), any(), anyMap());
}
}

Expand Down
Loading