From 6411a570528c7ff66e71033f3876fc7958ac8b67 Mon Sep 17 00:00:00 2001 From: Eugenio Grosso Date: Mon, 20 Apr 2026 21:04:07 +0000 Subject: [PATCH 1/4] flasharray: fix delete() rejecting volumes with {name, destroyed} PATCH FlashArrayAdapter.delete() builds one FlashArrayVolume, then issues two PATCH requests on it: first to rename the volume with a timestamp suffix so a recreated volume with the same name would not collide with the destroyed copy in FlashArrays 24h recycle bin, then to mark it destroyed. The second PATCH reuses the same object, which still has the new name set, so the request body carries both name and destroyed. Purity 6.x rejects that combination with 400 Bad Request Invalid combination of parameters specified. and every CloudStack-side volume delete ends with the volume renamed (first PATCH succeeded) but not destroyed (second PATCH rejected). That leaves the volume leaking on the array and the CloudStack volume row stuck in Destroy state with no clean way forward via the UI. Rename and destroy are still issued, but as two separate PATCHes each carrying only its own field, preserving the forensic value of the timestamp suffix while working around the API restriction. --- .../adapter/flasharray/FlashArrayAdapter.java | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java b/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java index 715379daf86d..7d2874e0afb4 100644 --- a/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java +++ b/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java @@ -204,21 +204,25 @@ public void delete(ProviderAdapterContext context, ProviderAdapterDataObject dat removeVlunsAll(context, pod, dataObject.getExternalName()); String fullName = normalizeName(pod, dataObject.getExternalName()); - FlashArrayVolume volume = new FlashArrayVolume(); - - // rename as we delete so it doesn't conflict if the template or volume is ever recreated - // pure keeps the volume(s) around in a Destroyed bucket for a period of time post delete + // Rename then destroy: FlashArray keeps destroyed volumes in a recycle + // bin (default 24h) from which they can be recovered. Renaming with a + // deletion timestamp gives operators a forensic trail when browsing the + // array - they can see when each destroyed copy was deleted on the + // CloudStack side. FlashArray rejects a single PATCH that combines + // {name, destroyed}, so the rename and the destroy must be issued as + // two separate requests each carrying only its own field. String timestamp = new SimpleDateFormat("yyyyMMddHHmmss").format(new java.util.Date()); - volume.setExternalName(fullName + "-" + timestamp); + String renamedName = fullName + "-" + timestamp; try { - PATCH("/volumes?names=" + fullName, volume, new TypeReference>() { + FlashArrayVolume rename = new FlashArrayVolume(); + rename.setExternalName(renamedName); + PATCH("/volumes?names=" + fullName, rename, new TypeReference>() { }); - // now delete it with new name - volume.setDestroyed(true); - - PATCH("/volumes?names=" + fullName + "-" + timestamp, volume, new TypeReference>() { + FlashArrayVolume destroy = new FlashArrayVolume(); + destroy.setDestroyed(true); + PATCH("/volumes?names=" + renamedName, destroy, new TypeReference>() { }); } catch (CloudRuntimeException e) { if (e.toString().contains("Volume does not exist")) { From 1d77261c34eefacb9d793551eced601d5c8a4e15 Mon Sep 17 00:00:00 2001 From: Eugenio Grosso Date: Mon, 20 Apr 2026 21:05:22 +0000 Subject: [PATCH 2/4] flasharray: fix delete() failing on snapshots due to invalid rename ProviderAdapter.delete() is called for both volumes and snapshots; the adapter treated every target as a volume and applied the same rename-with-timestamp + destroy sequence. For snapshots this breaks twice: * FlashArray snapshots live at /volume-snapshots, not /volumes, so the PATCH request was going to the wrong endpoint. * The computed rename name (.-) contains a literal "." character, which violates the array name rules: Volume name must be between 1 and 63 characters (alphanumeric, _ and -), begin and end with a letter or number, and include at least one letter, _, or -. Every CloudStack-side snapshot delete therefore failed with a 400 from the array, leaving orphan snapshots on both sides. Branch early when the data object is a snapshot: issue a single PATCH to /volume-snapshots with { destroyed: true } on the original name. No rename; the suffix the array assigns (e.g. ".1") already encodes the ordering and we cannot safely append anything to it. The 24h recycle bin still applies, so operators can recover a snapshot from the array within the retention window. --- .../adapter/flasharray/FlashArrayAdapter.java | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java b/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java index 7d2874e0afb4..a9222544326b 100644 --- a/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java +++ b/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java @@ -200,9 +200,30 @@ public void detach(ProviderAdapterContext context, ProviderAdapterDataObject dat @Override public void delete(ProviderAdapterContext context, ProviderAdapterDataObject dataObject) { + String fullName = normalizeName(pod, dataObject.getExternalName()); + + // Snapshots live under /volume-snapshots and have the reserved form + // .: the FlashArray rejects renames to any name that + // includes '.' or is not pure alphanumeric+'_'+'-', so we cannot + // tag them with a timestamp on the way out. Just mark them destroyed. + if (dataObject.getType() == ProviderAdapterDataObject.Type.SNAPSHOT) { + try { + FlashArrayVolume destroy = new FlashArrayVolume(); + destroy.setDestroyed(true); + PATCH("/volume-snapshots?names=" + fullName, destroy, new TypeReference>() { + }); + } catch (CloudRuntimeException e) { + if (e.toString().contains("No such volume or snapshot") + || e.toString().contains("Volume does not exist")) { + return; + } + throw e; + } + return; + } + // first make sure we are disconnected removeVlunsAll(context, pod, dataObject.getExternalName()); - String fullName = normalizeName(pod, dataObject.getExternalName()); // Rename then destroy: FlashArray keeps destroyed volumes in a recycle // bin (default 24h) from which they can be recovered. Renaming with a From 41d00d42404184aa1b187e7b5b5b4d9a5a667e7c Mon Sep 17 00:00:00 2001 From: Eugenio Grosso Date: Tue, 21 Apr 2026 19:22:18 +0200 Subject: [PATCH 3/4] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../datastore/adapter/flasharray/FlashArrayAdapter.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java b/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java index a9222544326b..90998766e0e3 100644 --- a/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java +++ b/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java @@ -202,10 +202,11 @@ public void detach(ProviderAdapterContext context, ProviderAdapterDataObject dat public void delete(ProviderAdapterContext context, ProviderAdapterDataObject dataObject) { String fullName = normalizeName(pod, dataObject.getExternalName()); - // Snapshots live under /volume-snapshots and have the reserved form - // .: the FlashArray rejects renames to any name that - // includes '.' or is not pure alphanumeric+'_'+'-', so we cannot - // tag them with a timestamp on the way out. Just mark them destroyed. + // Snapshots live under /volume-snapshots and already use the reserved + // form .. FlashArray snapshot rename constraints do + // not let us rename them to an arbitrary timestamp-suffixed name + // before deletion, so unlike volumes we skip rename and only mark + // them destroyed. if (dataObject.getType() == ProviderAdapterDataObject.Type.SNAPSHOT) { try { FlashArrayVolume destroy = new FlashArrayVolume(); From 540d40c742fe16a4a3d3524dfdf9afb663381560 Mon Sep 17 00:00:00 2001 From: Eugenio Grosso Date: Thu, 23 Apr 2026 00:21:48 +0000 Subject: [PATCH 4/4] flasharray: address Copilot review on delete() - Use e.getMessage() instead of e.toString() in the delete-time benign-error catches so the substring check is not coupled to JVM exception formatting. - Generate the deletion-timestamp suffix in UTC via DateTimeFormatter.withZone(ZoneOffset.UTC) so the rename is stable across management servers in different timezones (and across DST changes). - Sharpen the inline comment on snapshot deletion to spell out why the rename is skipped: FlashArray volume/snapshot names must match [A-Za-z0-9_-] and start/end with an alphanumeric, so the rename target would have an embedded '.' which the array rejects. Also drop the now-unused java.text.SimpleDateFormat import. Signed-off-by: Eugenio Grosso --- .../adapter/flasharray/FlashArrayAdapter.java | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java b/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java index 90998766e0e3..5b68a4691e33 100644 --- a/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java +++ b/plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java @@ -23,7 +23,8 @@ import java.security.KeyManagementException; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; -import java.text.SimpleDateFormat; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.HashMap; import java.util.Map; @@ -88,6 +89,9 @@ public class FlashArrayAdapter implements ProviderAdapter { static final ObjectMapper mapper = new ObjectMapper(); public String pod = null; public String hostgroup = null; + private static final DateTimeFormatter DELETION_TIMESTAMP_FORMAT = + DateTimeFormatter.ofPattern("yyyyMMddHHmmss").withZone(ZoneOffset.UTC); + private String username; private String password; private String accessToken; @@ -202,11 +206,14 @@ public void detach(ProviderAdapterContext context, ProviderAdapterDataObject dat public void delete(ProviderAdapterContext context, ProviderAdapterDataObject dataObject) { String fullName = normalizeName(pod, dataObject.getExternalName()); - // Snapshots live under /volume-snapshots and already use the reserved - // form .. FlashArray snapshot rename constraints do - // not let us rename them to an arbitrary timestamp-suffixed name - // before deletion, so unlike volumes we skip rename and only mark - // them destroyed. + // Snapshots live under /volume-snapshots and already use the + // reserved form .. FlashArray volume/snapshot names + // must match [A-Za-z0-9_-] and start/end with an alphanumeric, so + // appending our usual deletion-timestamp suffix to a snapshot name + // would produce a target like ".-" - the embedded "." + // is rejected by the array. We therefore skip the rename for + // snapshots and only mark them destroyed; the array's own ".N" + // suffix already disambiguates them in the recycle bin. if (dataObject.getType() == ProviderAdapterDataObject.Type.SNAPSHOT) { try { FlashArrayVolume destroy = new FlashArrayVolume(); @@ -214,8 +221,9 @@ public void delete(ProviderAdapterContext context, ProviderAdapterDataObject dat PATCH("/volume-snapshots?names=" + fullName, destroy, new TypeReference>() { }); } catch (CloudRuntimeException e) { - if (e.toString().contains("No such volume or snapshot") - || e.toString().contains("Volume does not exist")) { + String msg = e.getMessage(); + if (msg != null && (msg.contains("No such volume or snapshot") + || msg.contains("Volume does not exist"))) { return; } throw e; @@ -233,7 +241,11 @@ public void delete(ProviderAdapterContext context, ProviderAdapterDataObject dat // CloudStack side. FlashArray rejects a single PATCH that combines // {name, destroyed}, so the rename and the destroy must be issued as // two separate requests each carrying only its own field. - String timestamp = new SimpleDateFormat("yyyyMMddHHmmss").format(new java.util.Date()); + // Use UTC so the rename suffix is stable regardless of the management + // server's local timezone or DST changes - operators correlating the + // CloudStack delete event with the array's audit log get a consistent + // wall-clock value. + String timestamp = DELETION_TIMESTAMP_FORMAT.format(java.time.Instant.now()); String renamedName = fullName + "-" + timestamp; try { @@ -247,7 +259,8 @@ public void delete(ProviderAdapterContext context, ProviderAdapterDataObject dat PATCH("/volumes?names=" + renamedName, destroy, new TypeReference>() { }); } catch (CloudRuntimeException e) { - if (e.toString().contains("Volume does not exist")) { + String msg = e.getMessage(); + if (msg != null && msg.contains("Volume does not exist")) { return; } else { throw e;