diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index e0166cbf2829..b0f69c0468d5 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java @@ -231,6 +231,9 @@ public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { * *

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.

+ * + *

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.

*/ @Override public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) { @@ -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 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()); validateCreatedLun(lun, lunRequest.getName(), "cloneCloudStackVolume"); logger.debug("cloneCloudStackVolume: LUN cloned successfully. Lun: {}", lun); @@ -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. + * + *

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.

+ */ + 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. + * + *

{@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.

+ */ + private Lun getLunByUuidUntilVisible(String authHeader, String lunUuid, String lunName) { + Map queryParams = Map.of(OntapStorageConstants.UUID, lunUuid); + for (int attempt = 1; attempt <= OntapStorageConstants.ONTAP_LUN_CLONE_GET_MAX_RETRIES; attempt++) { + try { + OntapResponse 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. diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index 6fb66d20c215..0f3fff1f80a6 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -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"; diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java index 0ffb0166d960..36108532db63 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java @@ -1025,6 +1025,7 @@ 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); @@ -1032,6 +1033,7 @@ void testCloneCloudStackVolume_Success() { 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()); } } @@ -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); @@ -1334,6 +1337,7 @@ void testCloneCloudStackVolume_FeignException_ThrowsException() { assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.cloneCloudStackVolume(request)); + verify(sanFeignClient, never()).deleteLun(any(), any(), anyMap()); } }