From 182b07b218b0a76c4e918410c62a8c332507181a Mon Sep 17 00:00:00 2001 From: sp16743 Date: Mon, 26 Oct 2026 19:28:17 +0530 Subject: [PATCH 1/4] Fix ONTAP LUN clone visibility using GET retry --- .../storage/feign/client/SANFeignClient.java | 2 +- .../storage/service/UnifiedSANStrategy.java | 56 ++++++++++++++++++- .../storage/utils/OntapStorageConstants.java | 3 + .../service/UnifiedSANStrategyTest.java | 3 + 4 files changed, 60 insertions(+), 4 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java index 8a60a9a3e2bb..0ac03645b454 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java @@ -46,7 +46,7 @@ public interface SANFeignClient { @Headers({"Authorization: {authHeader}"}) OntapResponse getLunResponse(@Param("authHeader") String authHeader, @QueryMap Map queryMap); - @RequestLine("GET /{uuid}") + @RequestLine("GET /api/storage/luns/{uuid}") @Headers({"Authorization: {authHeader}"}) Lun getLunByUUID(@Param("authHeader") String authHeader, @Param("uuid") String uuid); 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..cf944c6d016b 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 while ONTAP returns 404.

*/ @Override public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) { @@ -249,12 +252,15 @@ public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) lunRequest.getClone().getSource().getUuid()); try { String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); - OntapResponse clonedLun = sanFeignClient.createLun(authHeader, true, lunRequest); - if (clonedLun == null || CollectionUtils.isEmpty(clonedLun.getRecords())) { + OntapResponse created = sanFeignClient.createLun(authHeader, true, lunRequest); + if (created == null || created.getRecords() == null || created.getRecords().isEmpty() + || created.getRecords().get(0) == null + || StringUtils.isBlank(created.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); + String lunUuid = created.getRecords().get(0).getUuid(); + Lun lun = getLunByUuidUntilVisible(authHeader, lunUuid, lunRequest.getName()); validateCreatedLun(lun, lunRequest.getName(), "cloneCloudStackVolume"); logger.debug("cloneCloudStackVolume: LUN cloned successfully. Lun: {}", lun); @@ -274,6 +280,50 @@ public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) } } + /** + * Confirms a clone uuid by reading that LUN directly. + * + *

{@code GET /api/storage/luns/{uuid}} returns 404 until the clone is visible. Those + * 404s are retried. Any other GET failure is returned to the caller.

+ */ + private Lun getLunByUuidUntilVisible(String authHeader, String lunUuid, String lunName) { + for (int attempt = 1; attempt <= OntapStorageConstants.ONTAP_LUN_CLONE_GET_MAX_RETRIES; attempt++) { + try { + Lun lun = sanFeignClient.getLunByUUID(authHeader, lunUuid); + if (lun != null) { + 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("Cloned Lun uuid was not visible for: " + lunUuid); + } + /** * 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..d61dd406fcb8 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 = 5; + 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..f1a2cff2f1d9 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.getLunByUUID(eq(authHeader), eq("cloned-lun-uuid"))).thenReturn(clonedLun); 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).getLunByUUID(eq(authHeader), eq("cloned-lun-uuid")); } } @@ -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.getLunByUUID(eq(authHeader), eq("new-lun-uuid"))).thenReturn(createdLun); CloudStackVolume result = unifiedSANStrategy.cloneCloudStackVolume(request); From a0586dea5d94bb3d04faa8f9da595136d7c91f72 Mon Sep 17 00:00:00 2001 From: sp16743 Date: Wed, 4 Nov 2026 21:23:45 +0530 Subject: [PATCH 2/4] Change the cloned LUN read failure message --- .../apache/cloudstack/storage/service/UnifiedSANStrategy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 cf944c6d016b..19cdce601208 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 @@ -321,7 +321,7 @@ private Lun getLunByUuidUntilVisible(String authHeader, String lunUuid, String l } } } - throw new CloudRuntimeException("Cloned Lun uuid was not visible for: " + lunUuid); + throw new CloudRuntimeException("Failed to read cloned Lun " + lunName); } /** From 99194b7577c3d209d20d1cfff70bba7f6ec2d3d5 Mon Sep 17 00:00:00 2001 From: sp16743 Date: Thu, 12 Nov 2026 18:28:42 +0530 Subject: [PATCH 3/4] Addressing Comments --- .../storage/service/UnifiedSANStrategy.java | 54 +++++++++++++++---- .../storage/utils/OntapStorageConstants.java | 2 +- .../service/UnifiedSANStrategyTest.java | 7 +-- 3 files changed, 48 insertions(+), 15 deletions(-) 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 19cdce601208..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 @@ -233,7 +233,7 @@ public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { * 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 while ONTAP returns 404.

+ * {@code GET /api/storage/luns?uuid=} calls until the response contains a record.

*/ @Override public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) { @@ -250,16 +250,17 @@ 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 created = sanFeignClient.createLun(authHeader, true, lunRequest); - if (created == null || created.getRecords() == null || created.getRecords().isEmpty() - || created.getRecords().get(0) == null - || StringUtils.isBlank(created.getRecords().get(0).getUuid())) { + OntapResponse clonedLun = sanFeignClient.createLun(authHeader, true, lunRequest); + 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()); } - String lunUuid = created.getRecords().get(0).getUuid(); + 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); @@ -269,28 +270,59 @@ 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()); } } /** - * Confirms a clone uuid by reading that LUN directly. + * Removes a clone that was created but could not be confirmed. * - *

{@code GET /api/storage/luns/{uuid}} returns 404 until the clone is visible. Those - * 404s are retried. Any other GET failure is returned to the caller.

+ *

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 { - Lun lun = sanFeignClient.getLunByUUID(authHeader, lunUuid); - if (lun != null) { + 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); } 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 d61dd406fcb8..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 @@ -176,7 +176,7 @@ public class OntapStorageConstants { 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 = 5; + 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. */ 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 f1a2cff2f1d9..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,7 +1025,7 @@ void testCloneCloudStackVolume_Success() { when(sanFeignClient.createLun(eq(authHeader), eq(true), any(Lun.class))) .thenReturn(response); - when(sanFeignClient.getLunByUUID(eq(authHeader), eq("cloned-lun-uuid"))).thenReturn(clonedLun); + when(sanFeignClient.getLunResponse(eq(authHeader), anyMap())).thenReturn(response); CloudStackVolume result = unifiedSANStrategy.cloneCloudStackVolume(request); @@ -1033,7 +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).getLunByUUID(eq(authHeader), eq("cloned-lun-uuid")); + verify(sanFeignClient).getLunResponse(eq(authHeader), anyMap()); } } @@ -1076,7 +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.getLunByUUID(eq(authHeader), eq("new-lun-uuid"))).thenReturn(createdLun); + when(sanFeignClient.getLunResponse(eq(authHeader), anyMap())).thenReturn(response); CloudStackVolume result = unifiedSANStrategy.cloneCloudStackVolume(request); @@ -1337,6 +1337,7 @@ void testCloneCloudStackVolume_FeignException_ThrowsException() { assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.cloneCloudStackVolume(request)); + verify(sanFeignClient, never()).deleteLun(any(), any(), anyMap()); } } From e68f74509e12c999ae47b6d432b874d7b0930151 Mon Sep 17 00:00:00 2001 From: sp16743 Date: Thu, 12 Nov 2026 18:54:32 +0530 Subject: [PATCH 4/4] Restore unused LUN endpoint --- .../apache/cloudstack/storage/feign/client/SANFeignClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java index 0ac03645b454..8a60a9a3e2bb 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java @@ -46,7 +46,7 @@ public interface SANFeignClient { @Headers({"Authorization: {authHeader}"}) OntapResponse getLunResponse(@Param("authHeader") String authHeader, @QueryMap Map queryMap); - @RequestLine("GET /api/storage/luns/{uuid}") + @RequestLine("GET /{uuid}") @Headers({"Authorization: {authHeader}"}) Lun getLunByUUID(@Param("authHeader") String authHeader, @Param("uuid") String uuid);