OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH) {
+ normalized = normalized.substring(0, OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH);
+ }
+ return normalized;
+ }
+
+ /**
+ * Builds an ONTAP-safe snapshot name that preserves the CloudStack UI snapshot name
+ * and appends a uniqueness suffix.
+ */
+ public static String buildOntapSnapshotName(String cloudStackSnapshotName, String uniquenessSuffix) {
+ String normalizedBase = (cloudStackSnapshotName == null || cloudStackSnapshotName.trim().isEmpty())
+ ? "snapshot"
+ : getOntapSnapshotName(cloudStackSnapshotName);
+ String suffix = (uniquenessSuffix == null || uniquenessSuffix.isEmpty())
+ ? ""
+ : "_" + uniquenessSuffix.replaceAll("[^a-zA-Z0-9_]", "_");
+ int maxLength = OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH;
+ int maxBaseLength = maxLength - suffix.length();
+ if (maxBaseLength <= 0) {
+ return normalizedBase.substring(0, maxLength);
+ }
+ if (normalizedBase.length() > maxBaseLength) {
+ normalizedBase = normalizedBase.substring(0, maxBaseLength);
+ }
+ return normalizedBase + suffix;
+ }
+
+ /**
+ * Extracts a resource UUID from an ONTAP job description path.
+ *
+ * Example: {@code POST /api/application/consistency-groups/{cg}/snapshots/{uuid}}
+ * with {@code pathSegment} {@code "/snapshots/"} returns the snapshot UUID.
+ */
+ public static String extractUuidFromOntapJobDescription(String description, String pathSegment) {
+ if (description == null || pathSegment == null || pathSegment.isEmpty()) {
+ return null;
+ }
+ int idx = description.indexOf(pathSegment);
+ if (idx < 0) {
+ return null;
+ }
+ String remainder = description.substring(idx + pathSegment.length()).trim();
+ if (remainder.isEmpty()) {
+ return null;
+ }
+ int queryIdx = remainder.indexOf('?');
+ if (queryIdx >= 0) {
+ remainder = remainder.substring(0, queryIdx);
+ }
+ int slashIdx = remainder.indexOf('/');
+ if (slashIdx >= 0) {
+ remainder = remainder.substring(0, slashIdx);
+ }
+ return remainder.isEmpty() ? null : remainder;
+ }
+
}
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java
index 7fa80a0b3fae..702a3aa5e414 100644
--- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java
+++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java
@@ -20,21 +20,29 @@
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.concurrent.TimeUnit;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
+import com.cloud.utils.StringUtils;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotOptions;
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient;
-import org.apache.cloudstack.storage.feign.model.CliSnapshotRestoreRequest;
+import org.apache.cloudstack.storage.feign.model.ConsistencyGroup;
+import org.apache.cloudstack.storage.feign.model.ConsistencyGroupSnapshot;
+import org.apache.cloudstack.storage.feign.model.ConsistencyGroupVolume;
+import org.apache.cloudstack.storage.feign.model.ConsistencyGroupVolumeProvisioningOptions;
import org.apache.cloudstack.storage.feign.model.FlexVolSnapshot;
+import org.apache.cloudstack.storage.feign.model.Svm;
+import org.apache.cloudstack.storage.feign.model.Job;
import org.apache.cloudstack.storage.feign.model.response.JobResponse;
import org.apache.cloudstack.storage.feign.model.response.OntapResponse;
import org.apache.cloudstack.storage.service.StorageStrategy;
@@ -72,25 +80,22 @@
import org.apache.cloudstack.storage.utils.OntapStorageConstants;
/**
- * VM Snapshot strategy for NetApp ONTAP managed storage using FlexVolume-level snapshots.
+ * VM Snapshot strategy for NetApp ONTAP managed storage using temporary consistency-group orchestration.
*
* This strategy handles VM-level (instance) snapshots for VMs whose volumes
- * reside on ONTAP managed primary storage. Instead of creating per-file clones
- * (the old approach), it takes ONTAP FlexVolume-level snapshots via the
- * ONTAP REST API ({@code POST /api/storage/volumes/{uuid}/snapshots}).
- *
- * Key Advantage:
- * When multiple CloudStack disks (ROOT + DATA) reside on the same ONTAP
- * FlexVolume, a single FlexVolume snapshot atomically captures all of them.
- * This is both faster and more storage-efficient than per-file clones.
+ * reside on ONTAP managed primary storage. When VM volumes span multiple FlexVols,
+ * snapshot creation is coordinated through a temporary ONTAP consistency group (CG)
+ * and two-phase snapshot flow (start + commit). When all volumes share a single FlexVol,
+ * a direct FlexVol snapshot is used instead.
*
* Flow:
*
* - Group all VM volumes by their parent FlexVolume UUID
* - Freeze the VM via QEMU guest agent ({@code fsfreeze}) — if quiesce requested
- * - For each unique FlexVolume, create one ONTAP snapshot
+ * - If VM spans multiple FlexVolumes: create temporary CG, start + commit CG snapshot (two-phase)
+ * - If VM spans a single FlexVolume: create one FlexVol snapshot directly (no CG overhead)
* - Thaw the VM
- * - Record FlexVolume → snapshot UUID mappings in {@code vm_snapshot_details}
+ * - Resolve FlexVolume → snapshot UUID mappings and persist in {@code vm_snapshot_details}
*
*
* Metadata in vm_snapshot_details:
@@ -251,12 +256,14 @@ boolean allVolumesOnOntapManagedStorage(long vmId) {
/**
* Takes a VM-level snapshot by freezing the VM, creating ONTAP FlexVolume-level
- * snapshots (one per unique FlexVolume), and then thawing the VM.
+ * snapshot(s), and then thawing the VM.
*
* Volumes are grouped by their parent FlexVolume UUID (from storage pool details).
- * For each unique FlexVolume, exactly one ONTAP snapshot is created via
- * {@code POST /api/storage/volumes/{uuid}/snapshots}. This means if a VM has
- * ROOT and DATA disks on the same FlexVolume, only one snapshot is created.
+ * When the VM spans more than one unique FlexVolume, a temporary ONTAP
+ * consistency group is used with two-phase snapshot semantics (start + commit) so
+ * all FlexVols are captured at the same point in time. When all VM volumes reside
+ * on a single FlexVolume, a direct per-FlexVol snapshot is taken instead —
+ * CG orchestration is unnecessary in that case.
*
* Memory Snapshots Not Supported: This strategy only supports disk-only
* (crash-consistent) snapshots. Memory snapshots (snapshotmemory=true) are rejected
@@ -286,7 +293,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
FreezeThawVMAnswer thawAnswer = null;
long startFreeze = 0;
- // Track which FlexVolume snapshots were created (for rollback)
+ // Track which FlexVolume snapshots were created (for rollback and detail persistence)
List createdSnapshots = new ArrayList<>();
boolean result = false;
@@ -338,7 +345,8 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
CreateVMSnapshotCommand ccmd = new CreateVMSnapshotCommand(
userVm.getInstanceName(), userVm.getUuid(), target, volumeTOs, guestOS.getDisplayName());
- logger.info("takeVMSnapshot: Creating ONTAP FlexVolume VM Snapshot for VM [{}] with quiesce={}", userVm.getInstanceName(), quiesceVm);
+ logger.info("takeVMSnapshot: Creating ONTAP VM snapshot for VM [{}] with quiesce={}",
+ userVm.getInstanceName(), quiesceVm);
// Prepare volume info list and calculate sizes
for (VolumeObjectTO volumeObjectTO : volumeTOs) {
@@ -375,56 +383,20 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
userVm.getInstanceName(), quiesceVm, vmIsRunning);
}
- // ── Step 2: Create FlexVolume-level snapshots ──
+ // ── Step 2: Create FlexVolume-level snapshot(s) ──
try {
String snapshotNameBase = buildSnapshotName(vmSnapshot);
- for (Map.Entry entry : flexVolGroups.entrySet()) {
- String flexVolUuid = entry.getKey();
- FlexVolGroupInfo groupInfo = entry.getValue();
- long startSnapshot = System.nanoTime();
-
- // Build storage strategy from pool details to get the feign client
- StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(groupInfo.poolDetails);
- SnapshotFeignClient snapshotClient = storageStrategy.getSnapshotFeignClient();
- String authHeader = storageStrategy.getAuthHeader();
-
- // Use the same snapshot name for all FlexVolumes in this VM snapshot
- // (each FlexVolume gets its own independent snapshot with this name)
- FlexVolSnapshot snapshotRequest = new FlexVolSnapshot(snapshotNameBase,
- "CloudStack VM snapshot " + vmSnapshot.getName() + " for VM " + userVm.getInstanceName());
-
- logger.info("takeVMSnapshot: Creating ONTAP FlexVolume snapshot [{}] on FlexVol UUID [{}] covering {} volume(s)",
- snapshotNameBase, flexVolUuid, groupInfo.volumeIds.size());
-
- JobResponse jobResponse = snapshotClient.createSnapshot(authHeader, flexVolUuid, snapshotRequest);
- if (jobResponse == null || jobResponse.getJob() == null) {
- throw new CloudRuntimeException("Failed to initiate FlexVolume snapshot on FlexVol UUID [" + flexVolUuid + "]");
- }
-
- // Poll for job completion
- Boolean jobSucceeded = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 30, 2000);
- if (!jobSucceeded) {
- throw new CloudRuntimeException("FlexVolume snapshot job failed on FlexVol UUID [" + flexVolUuid + "]");
- }
-
- // Retrieve the created snapshot UUID by name
- String snapshotUuid = resolveSnapshotUuid(snapshotClient, authHeader, flexVolUuid, snapshotNameBase);
-
- String protocol = groupInfo.poolDetails.get(OntapStorageConstants.PROTOCOL);
-
- // Create one detail per CloudStack volume in this FlexVol group (for single-file restore during revert)
- for (Long volumeId : groupInfo.volumeIds) {
- String volumePath = resolveVolumePathOnOntap(volumeId, protocol, groupInfo.poolDetails);
- FlexVolSnapshotDetail detail = new FlexVolSnapshotDetail(
- flexVolUuid, snapshotUuid, snapshotNameBase, volumePath, groupInfo.poolId, protocol);
- createdSnapshots.add(detail);
- }
-
- logger.info("takeVMSnapshot: ONTAP FlexVolume snapshot [{}] (uuid={}) on FlexVol [{}] completed in {} ms. Covers volumes: {}",
- snapshotNameBase, snapshotUuid, flexVolUuid,
- TimeUnit.MILLISECONDS.convert(System.nanoTime() - startSnapshot, TimeUnit.NANOSECONDS),
- groupInfo.volumeIds);
+ // CG orchestration is only required when VM disks span multiple FlexVols.
+ // A single FlexVol already provides atomic capture for all volumes on that FlexVol.
+ if (flexVolGroups.size() > 1) {
+ logger.info("takeVMSnapshot: VM [{}] spans {} FlexVol(s); using temporary CG two-phase snapshot flow",
+ userVm.getInstanceName(), flexVolGroups.size());
+ createVmSnapshotsViaTemporaryCg(vmSnapshot, userVm, flexVolGroups, snapshotNameBase, createdSnapshots);
+ } else {
+ logger.info("takeVMSnapshot: VM [{}] spans a single FlexVol; using direct FlexVol snapshot flow",
+ userVm.getInstanceName());
+ createVmSnapshotsViaSingleFlexVol(vmSnapshot, userVm, flexVolGroups, snapshotNameBase, createdSnapshots);
}
} finally {
// ── Step 3: Thaw the VM (only if it was frozen, always even on error) ──
@@ -456,7 +428,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
answer.setVolumeTOs(volumeTOs);
processAnswer(vmSnapshotVO, userVm, answer, null);
- logger.info("takeVMSnapshot: ONTAP FlexVolume VM Snapshot [{}] created successfully for VM [{}] ({} FlexVol snapshot(s))",
+ logger.info("takeVMSnapshot: ONTAP VM Snapshot [{}] created successfully for VM [{}] ({} detail row(s))",
vmSnapshot.getName(), userVm.getInstanceName(), createdSnapshots.size());
long newChainSize = 0;
@@ -668,16 +640,140 @@ Map groupVolumesByFlexVol(List volumeT
}
/**
- * Builds a deterministic, ONTAP-safe snapshot name for a VM snapshot.
- * Format: {@code vmsnap__}
+ * Creates VM snapshot artifacts via direct FlexVol snapshot API.
+ *
+ * Used when all VM volumes map to a single FlexVol. In that case a CG is not
+ * needed because one FlexVol snapshot already captures every disk atomically.
*/
- String buildSnapshotName(VMSnapshot vmSnapshot) {
- String name = "vmsnap_" + vmSnapshot.getId() + "_" + System.currentTimeMillis();
- // ONTAP snapshot names: max 255 chars, must start with letter, only alphanumeric and underscores
- if (name.length() > OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH) {
- name = name.substring(0, OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH);
+ void createVmSnapshotsViaSingleFlexVol(VMSnapshot vmSnapshot, UserVm userVm,
+ Map flexVolGroups,
+ String snapshotNameBase,
+ List createdSnapshots) {
+ for (Map.Entry entry : flexVolGroups.entrySet()) {
+ String flexVolUuid = entry.getKey();
+ FlexVolGroupInfo groupInfo = entry.getValue();
+ long startSnapshot = System.nanoTime();
+
+ StorageStrategy storageStrategy = resolveStorageStrategy(groupInfo.poolDetails);
+ SnapshotFeignClient snapshotClient = storageStrategy.getSnapshotFeignClient();
+ String authHeader = storageStrategy.getAuthHeader();
+
+ FlexVolSnapshot snapshotRequest = new FlexVolSnapshot(snapshotNameBase,
+ "CloudStack VM snapshot " + vmSnapshot.getName() + " for VM " + userVm.getInstanceName());
+
+ logger.info("takeVMSnapshot: [FlexVol] Creating snapshot [{}] on FlexVol UUID [{}] covering {} volume(s)",
+ snapshotNameBase, flexVolUuid, groupInfo.volumeIds.size());
+
+ JobResponse jobResponse = snapshotClient.createSnapshot(authHeader, flexVolUuid, snapshotRequest);
+ if (jobResponse == null || jobResponse.getJob() == null) {
+ throw new CloudRuntimeException("Failed to initiate FlexVolume snapshot on FlexVol UUID [" + flexVolUuid + "]");
+ }
+
+ Boolean jobSucceeded = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 30, 2000);
+ if (!jobSucceeded) {
+ throw new CloudRuntimeException("FlexVolume snapshot job failed on FlexVol UUID [" + flexVolUuid + "]");
+ }
+
+ String snapshotUuid = resolveSnapshotUuid(snapshotClient, authHeader, flexVolUuid, snapshotNameBase);
+ String protocol = groupInfo.poolDetails.get(OntapStorageConstants.PROTOCOL);
+
+ for (Long volumeId : groupInfo.volumeIds) {
+ String volumePath = resolveVolumePathOnOntap(volumeId, protocol, groupInfo.poolDetails);
+ createdSnapshots.add(new FlexVolSnapshotDetail(
+ flexVolUuid, snapshotUuid, snapshotNameBase, volumePath, groupInfo.poolId, protocol));
+ }
+
+ logger.debug("takeVMSnapshot: [FlexVol] Snapshot [{}] (uuid={}) on FlexVol [{}] completed in {} ms. Covers volumes: {}",
+ snapshotNameBase, snapshotUuid, flexVolUuid,
+ TimeUnit.MILLISECONDS.convert(System.nanoTime() - startSnapshot, TimeUnit.NANOSECONDS),
+ groupInfo.volumeIds);
}
- return name;
+ }
+
+ /**
+ * Creates VM snapshot artifacts via temporary consistency-group two-phase flow.
+ *
+ * Used when VM volumes span multiple FlexVols and require a consistent
+ * point-in-time capture across all participating FlexVolumes.
+ */
+ void createVmSnapshotsViaTemporaryCg(VMSnapshot vmSnapshot, UserVm userVm,
+ Map flexVolGroups,
+ String snapshotNameBase,
+ List createdSnapshots) {
+ String tempCgName = buildTemporaryConsistencyGroupName(vmSnapshot);
+ String tempCgUuid = null;
+ String cgSnapshotUuid = null;
+ long cgFlowStart = System.nanoTime();
+
+ // All volumes in a VM snapshot belong to ONTAP-managed pools and share the same ONTAP credentials.
+ // Use any one FlexVol group to obtain strategy/client objects for this operation.
+ FlexVolGroupInfo referenceGroup = flexVolGroups.values().iterator().next();
+ StorageStrategy storageStrategy = resolveStorageStrategy(referenceGroup.poolDetails);
+ SnapshotFeignClient snapshotClient = storageStrategy.getSnapshotFeignClient();
+ String authHeader = storageStrategy.getAuthHeader();
+
+ try {
+ logger.info("takeVMSnapshot: [CG:Create] Creating temporary consistency group [{}] for VM [{}] over {} FlexVol(s)",
+ tempCgName, userVm.getInstanceName(), flexVolGroups.size());
+ tempCgUuid = createTemporaryConsistencyGroup(snapshotClient, storageStrategy, authHeader, tempCgName,
+ resolveConsistencyGroupScope(flexVolGroups), flexVolGroups.keySet());
+
+ logger.info("takeVMSnapshot: [CG:Start] Starting phase-1 snapshot [{}] for temporary consistency group [{}]",
+ snapshotNameBase, tempCgUuid);
+ cgSnapshotUuid = resolveStartedConsistencyGroupSnapshotUuid(snapshotClient, storageStrategy,
+ authHeader, tempCgUuid, snapshotNameBase);
+
+ logger.info("takeVMSnapshot: [CG:Commit] Committing phase-2 snapshot [{}] (uuid={}) for temporary consistency group [{}]",
+ snapshotNameBase, cgSnapshotUuid, tempCgUuid);
+ commitConsistencyGroupSnapshot(snapshotClient, storageStrategy, authHeader, tempCgUuid, cgSnapshotUuid);
+
+ // Resolve per-FlexVol snapshot UUIDs and build one detail entry per CloudStack volume.
+ for (Map.Entry entry : flexVolGroups.entrySet()) {
+ String flexVolUuid = entry.getKey();
+ FlexVolGroupInfo groupInfo = entry.getValue();
+ String snapshotUuid = resolveSnapshotUuid(snapshotClient, authHeader, flexVolUuid, snapshotNameBase);
+ String protocol = groupInfo.poolDetails.get(OntapStorageConstants.PROTOCOL);
+
+ for (Long volumeId : groupInfo.volumeIds) {
+ String volumePath = resolveVolumePathOnOntap(volumeId, protocol, groupInfo.poolDetails);
+ createdSnapshots.add(new FlexVolSnapshotDetail(
+ flexVolUuid, snapshotUuid, snapshotNameBase, volumePath, groupInfo.poolId, protocol));
+ }
+
+ logger.debug("takeVMSnapshot: [CG:Resolve] Snapshot [{}] resolved to FlexVol snapshot uuid [{}] for FlexVol [{}], volumes={}",
+ snapshotNameBase, snapshotUuid, flexVolUuid, groupInfo.volumeIds);
+ }
+ } finally {
+ // CG is only a transaction boundary; remove it after commit/failure and keep snapshots intact.
+ if (tempCgUuid != null) {
+ try {
+ logger.info("takeVMSnapshot: [CG:Cleanup] Deleting temporary consistency group [{}]", tempCgUuid);
+ deleteTemporaryConsistencyGroup(snapshotClient, storageStrategy, authHeader, tempCgUuid);
+ } catch (Exception cleanupEx) {
+ logger.warn("takeVMSnapshot: Failed to delete temporary consistency group [{}]: {}",
+ tempCgUuid, cleanupEx.getMessage());
+ }
+ }
+ }
+
+ logger.info("takeVMSnapshot: Temporary consistency-group two-phase flow completed for VM [{}] in {} ms. CG snapshot uuid={}, detail rows={}",
+ userVm.getInstanceName(),
+ TimeUnit.MILLISECONDS.convert(System.nanoTime() - cgFlowStart, TimeUnit.NANOSECONDS),
+ cgSnapshotUuid, createdSnapshots.size());
+ }
+
+ /**
+ * Builds an ONTAP-safe snapshot name from the CloudStack VM snapshot UI name.
+ */
+ String buildSnapshotName(VMSnapshot vmSnapshot) {
+ return OntapStorageUtils.buildOntapSnapshotName(vmSnapshot.getName(), "vm" + vmSnapshot.getId());
+ }
+
+ /**
+ * Wrapper for static utility to simplify unit testing and keep call sites explicit.
+ */
+ StorageStrategy resolveStorageStrategy(Map poolDetails) {
+ return OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails);
}
/**
@@ -695,6 +791,227 @@ String resolveSnapshotUuid(SnapshotFeignClient client, String authHeader,
return response.getRecords().get(0).getUuid();
}
+ /**
+ * Builds a deterministic temporary CG name for the VM snapshot transaction.
+ */
+ String buildTemporaryConsistencyGroupName(VMSnapshot vmSnapshot) {
+ return OntapStorageConstants.ONTAP_TEMP_CG_PREFIX + vmSnapshot.getId();
+ }
+
+ /**
+ * Validates and returns the ONTAP scope for a temporary consistency group.
+ *
+ * CG membership requires all FlexVols on the same ONTAP management endpoint and SVM.
+ * SVM name alone is not sufficient — different clusters may reuse names such as {@code vs0}.
+ * Identity uses {@code storageIP} plus {@code svmUUID} when persisted, otherwise
+ * {@code storageIP} plus {@code svmName} for legacy pools.
+ */
+ ConsistencyGroupScope resolveConsistencyGroupScope(Map flexVolGroups) {
+ ConsistencyGroupScope scope = null;
+ for (FlexVolGroupInfo group : flexVolGroups.values()) {
+ ConsistencyGroupScope candidate = consistencyGroupScopeFromPoolDetails(group.poolDetails, group.poolId);
+ if (scope == null) {
+ scope = candidate;
+ } else if (!scope.matches(candidate)) {
+ throw new CloudRuntimeException("ONTAP consistency groups require all VM volumes on the same "
+ + "ONTAP cluster and SVM. Found [" + scope + "] and [" + candidate + "]");
+ }
+ }
+ return scope;
+ }
+
+ ConsistencyGroupScope consistencyGroupScopeFromPoolDetails(Map poolDetails, long poolId) {
+ String storageIp = poolDetails.get(OntapStorageConstants.STORAGE_IP);
+ if (StringUtils.isBlank(storageIp)) {
+ throw new CloudRuntimeException("ONTAP storage management IP not found in pool details for pool ["
+ + poolId + "]");
+ }
+ String svmName = poolDetails.get(OntapStorageConstants.SVM_NAME);
+ if (StringUtils.isBlank(svmName)) {
+ throw new CloudRuntimeException("SVM name not found in pool details for pool [" + poolId + "]");
+ }
+ String svmUuid = poolDetails.get(OntapStorageConstants.SVM_UUID);
+ return new ConsistencyGroupScope(storageIp.trim(), svmName.trim(),
+ svmUuid != null && !svmUuid.trim().isEmpty() ? svmUuid.trim() : null);
+ }
+
+ /**
+ * Creates a temporary consistency group for the involved FlexVol UUIDs and returns its UUID.
+ */
+ String createTemporaryConsistencyGroup(SnapshotFeignClient client, StorageStrategy storageStrategy,
+ String authHeader, String cgName, ConsistencyGroupScope cgScope,
+ Set flexVolUuids) {
+ if (cgScope == null) {
+ throw new CloudRuntimeException("ONTAP consistency group scope is required to create CG [" + cgName + "]");
+ }
+
+ List volumeRefs = new ArrayList<>();
+ for (String flexVolUuid : flexVolUuids) {
+ ConsistencyGroupVolumeProvisioningOptions provisioningOptions =
+ new ConsistencyGroupVolumeProvisioningOptions(OntapStorageConstants.CG_VOLUME_PROVISIONING_ACTION_ADD);
+
+ ConsistencyGroupVolume volumeRef = new ConsistencyGroupVolume();
+ volumeRef.setUuid(flexVolUuid);
+ volumeRef.setProvisioningOptions(provisioningOptions);
+ volumeRefs.add(volumeRef);
+ }
+
+ ConsistencyGroup payload = new ConsistencyGroup();
+ payload.setName(cgName);
+ payload.setSvm(cgScope.toOntapSvm());
+ payload.setVolumes(volumeRefs);
+
+ JobResponse response = client.createConsistencyGroup(authHeader, payload);
+ storageStrategy.pollJobIfPresent(response, "create temporary consistency group " + cgName);
+
+ String cgUuid = resolveConsistencyGroupUuidByName(client, authHeader, cgName, cgScope);
+ if (cgUuid == null || cgUuid.isEmpty()) {
+ throw new CloudRuntimeException("Unable to resolve temporary consistency group UUID for [" + cgName + "]");
+ }
+ return cgUuid;
+ }
+
+ /**
+ * Starts phase-1 of the two-phase CG snapshot and returns the CG snapshot UUID when ONTAP exposes it in the job record.
+ */
+ String startConsistencyGroupSnapshot(SnapshotFeignClient client, StorageStrategy storageStrategy,
+ String authHeader, String cgUuid, String snapshotName) {
+ ConsistencyGroupSnapshot payload = new ConsistencyGroupSnapshot(snapshotName, "start");
+ JobResponse response = client.createConsistencyGroupSnapshot(authHeader, cgUuid, payload);
+ Job completedJob = storageStrategy.pollJobIfPresentAndGetCompletedJob(response,
+ "start CG snapshot " + snapshotName + " for " + cgUuid);
+ if (completedJob == null) {
+ return null;
+ }
+ String snapshotUuid = OntapStorageUtils.extractUuidFromOntapJobDescription(
+ completedJob.getDescription(), "/snapshots/");
+ if (snapshotUuid != null) {
+ logger.info("takeVMSnapshot: [CG:Start] Resolved CG snapshot UUID [{}] from ONTAP job for snapshot [{}]",
+ snapshotUuid, snapshotName);
+ }
+ return snapshotUuid;
+ }
+
+ /**
+ * Commits phase-2 of the started CG snapshot.
+ */
+ void commitConsistencyGroupSnapshot(SnapshotFeignClient client, StorageStrategy storageStrategy,
+ String authHeader, String cgUuid, String snapshotUuid) {
+ ConsistencyGroupSnapshot payload = new ConsistencyGroupSnapshot();
+ payload.setAction("commit");
+ JobResponse response = client.commitConsistencyGroupSnapshot(authHeader, cgUuid, snapshotUuid, payload);
+ storageStrategy.pollJobIfPresent(response, "commit CG snapshot " + snapshotUuid + " for " + cgUuid);
+ }
+
+ /**
+ * Deletes the temporary consistency group used as a transaction boundary.
+ */
+ void deleteTemporaryConsistencyGroup(SnapshotFeignClient client, StorageStrategy storageStrategy,
+ String authHeader, String cgUuid) {
+ JobResponse response = client.deleteConsistencyGroup(authHeader, cgUuid);
+ storageStrategy.pollJobIfPresent(response, "delete temporary consistency group " + cgUuid);
+ }
+
+ /**
+ * Resolves consistency group UUID by name within the given ONTAP cluster/SVM scope.
+ */
+ String resolveConsistencyGroupUuidByName(SnapshotFeignClient client, String authHeader,
+ String cgName, ConsistencyGroupScope cgScope) {
+ Map query = new HashMap<>();
+ query.put("name", cgName);
+ cgScope.applySvmQueryFilter(query);
+ query.put("fields", "uuid,name");
+ OntapResponse response = client.getConsistencyGroups(authHeader, query);
+ if (response == null || response.getRecords() == null || response.getRecords().isEmpty()) {
+ return null;
+ }
+ ConsistencyGroup consistencyGroup = response.getRecords().get(0);
+ return consistencyGroup != null ? consistencyGroup.getUuid() : null;
+ }
+
+ /**
+ * Resolves the started CG snapshot UUID after phase-1, using the job record when available and polling GET otherwise.
+ */
+ String resolveStartedConsistencyGroupSnapshotUuid(SnapshotFeignClient client, StorageStrategy storageStrategy,
+ String authHeader, String cgUuid, String snapshotName) {
+ String snapshotUuidFromJob = startConsistencyGroupSnapshot(client, storageStrategy, authHeader, cgUuid, snapshotName);
+ if (snapshotUuidFromJob != null && !snapshotUuidFromJob.isEmpty()) {
+ return snapshotUuidFromJob;
+ }
+ return resolveConsistencyGroupSnapshotUuid(client, storageStrategy, authHeader, cgUuid, snapshotName);
+ }
+
+ /**
+ * Resolves consistency group snapshot UUID by name with retries (ONTAP list can lag behind job success).
+ */
+ String resolveConsistencyGroupSnapshotUuid(SnapshotFeignClient client, StorageStrategy storageStrategy,
+ String authHeader, String cgUuid, String snapshotName) {
+ int maxRetries = OntapStorageConstants.ONTAP_CG_SNAPSHOT_RESOLVE_MAX_RETRIES;
+ int pollIntervalMs = OntapStorageConstants.ONTAP_CG_SNAPSHOT_RESOLVE_POLL_INTERVAL_MS;
+
+ for (int attempt = 1; attempt <= maxRetries; attempt++) {
+ String snapshotUuid = lookupConsistencyGroupSnapshotUuid(client, authHeader, cgUuid, snapshotName);
+ if (snapshotUuid != null) {
+ if (attempt > 1) {
+ logger.info("takeVMSnapshot: [CG:Resolve] CG snapshot [{}] resolved on attempt {}/{}",
+ snapshotName, attempt, maxRetries);
+ }
+ return snapshotUuid;
+ }
+ if (attempt < maxRetries) {
+ logger.debug("takeVMSnapshot: [CG:Resolve] CG snapshot [{}] not yet visible in CG [{}], retry {}/{}",
+ snapshotName, cgUuid, attempt, maxRetries);
+ try {
+ Thread.sleep(pollIntervalMs);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new CloudRuntimeException("Interrupted while resolving CG snapshot [" + snapshotName + "]", e);
+ }
+ }
+ }
+
+ throw new CloudRuntimeException("Unable to resolve consistency group snapshot UUID for snapshot [" +
+ snapshotName + "] in CG [" + cgUuid + "] after " + maxRetries + " attempts");
+ }
+
+ /**
+ * Single GET attempt: try to match by name,
+ * then fall back to listing all CG snapshots in this group (And it would be one
+ * always since workflow is keep deleting the CG).
+ */
+ String lookupConsistencyGroupSnapshotUuid(SnapshotFeignClient client, String authHeader,
+ String cgUuid, String snapshotName) {
+ Map query = new HashMap<>();
+ query.put("name", snapshotName);
+ query.put("fields", "uuid,name");
+ OntapResponse response = client.getConsistencyGroupSnapshots(authHeader, cgUuid, query);
+ String snapshotUuid = findConsistencyGroupSnapshotUuidInRecords(response, snapshotName);
+ if (snapshotUuid != null) {
+ return snapshotUuid;
+ }
+
+ Map listAllQuery = new HashMap<>();
+ listAllQuery.put("fields", "uuid,name");
+ OntapResponse allSnapshots = client.getConsistencyGroupSnapshots(authHeader, cgUuid, listAllQuery);
+ return findConsistencyGroupSnapshotUuidInRecords(allSnapshots, snapshotName);
+ }
+
+ private String findConsistencyGroupSnapshotUuidInRecords(OntapResponse response,
+ String snapshotName) {
+ if (response == null || response.getRecords() == null || response.getRecords().isEmpty()) {
+ return null;
+ }
+ for (ConsistencyGroupSnapshot record : response.getRecords()) {
+ if (record != null && snapshotName.equals(record.getName())) {
+ String uuid = record.getUuid();
+ if (uuid != null && !uuid.isEmpty()) {
+ return uuid;
+ }
+ }
+ }
+ return null;
+ }
+
/**
* Resolves the ONTAP-side path of a CloudStack volume within its FlexVolume.
*
@@ -735,7 +1052,7 @@ String resolveVolumePathOnOntap(Long volumeId, String protocol, Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(detail.poolId);
- StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails);
+ StorageStrategy storageStrategy = resolveStorageStrategy(poolDetails);
SnapshotFeignClient client = storageStrategy.getSnapshotFeignClient();
String authHeader = storageStrategy.getAuthHeader();
@@ -757,36 +1074,52 @@ void rollbackFlexVolSnapshot(FlexVolSnapshotDetail detail) {
* Since there is one detail row per CloudStack volume, multiple rows may reference
* the same FlexVol + snapshot combination. This method deduplicates to delete each
* underlying ONTAP snapshot only once.
+ *
+ * Detail rows are removed only after the underlying ONTAP snapshot delete succeeds
+ * (or was already deleted for the same FlexVol+snapshot pair in this pass). If delete
+ * throws, the detail row is retained so a retry can still find the ONTAP snapshot.
*/
void deleteFlexVolSnapshots(List flexVolDetails) {
- // Track which FlexVol+Snapshot pairs have already been deleted
Map deletedSnapshots = new HashMap<>();
+ CloudRuntimeException deleteFailure = null;
for (VMSnapshotDetailsVO detailVO : flexVolDetails) {
FlexVolSnapshotDetail detail = FlexVolSnapshotDetail.parse(detailVO.getValue());
String dedupeKey = detail.flexVolUuid + "::" + detail.snapshotUuid;
- // Only delete the ONTAP snapshot once per FlexVol+Snapshot pair
- if (!deletedSnapshots.containsKey(dedupeKey)) {
- Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(detail.poolId);
- StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails);
- SnapshotFeignClient client = storageStrategy.getSnapshotFeignClient();
- String authHeader = storageStrategy.getAuthHeader();
+ try {
+ if (!deletedSnapshots.containsKey(dedupeKey)) {
+ Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(detail.poolId);
+ StorageStrategy storageStrategy = resolveStorageStrategy(poolDetails);
- logger.info("deleteFlexVolSnapshots: Deleting ONTAP FlexVol snapshot [{}] (uuid={}) on FlexVol [{}]",
- detail.snapshotName, detail.snapshotUuid, detail.flexVolUuid);
+ logger.info("deleteFlexVolSnapshots: Deleting ONTAP FlexVol snapshot [{}] (uuid={}) on FlexVol [{}]",
+ detail.snapshotName, detail.snapshotUuid, detail.flexVolUuid);
- JobResponse jobResponse = client.deleteSnapshot(authHeader, detail.flexVolUuid, detail.snapshotUuid);
- if (jobResponse != null && jobResponse.getJob() != null) {
- storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 30, 2000);
- }
+ storageStrategy.deleteFlexVolSnapshotForCloudStackVolume(
+ detail.flexVolUuid, detail.snapshotUuid, detail.snapshotName);
- deletedSnapshots.put(dedupeKey, Boolean.TRUE);
- logger.info("deleteFlexVolSnapshots: Deleted ONTAP FlexVol snapshot [{}] on FlexVol [{}]", detail.snapshotName, detail.flexVolUuid);
+ deletedSnapshots.put(dedupeKey, Boolean.TRUE);
+ logger.info("deleteFlexVolSnapshots: Deleted ONTAP FlexVol snapshot [{}] on FlexVol [{}]",
+ detail.snapshotName, detail.flexVolUuid);
+ }
+ } catch (Exception e) {
+ logger.error("deleteFlexVolSnapshots: Failed to delete ONTAP FlexVol snapshot [{}] (uuid={}) "
+ + "on FlexVol [{}] for detail [{}]: {}",
+ detail.snapshotName, detail.snapshotUuid, detail.flexVolUuid, detailVO.getId(), e.getMessage(), e);
+ if (deleteFailure == null) {
+ deleteFailure = e instanceof CloudRuntimeException
+ ? (CloudRuntimeException) e
+ : new CloudRuntimeException("Failed to delete ONTAP FlexVol snapshot: " + e.getMessage(), e);
+ }
+ } finally {
+ if (deletedSnapshots.containsKey(dedupeKey)) {
+ vmSnapshotDetailsDao.remove(detailVO.getId());
+ }
}
+ }
- // Always remove the DB detail row
- vmSnapshotDetailsDao.remove(detailVO.getId());
+ if (deleteFailure != null) {
+ throw deleteFailure;
}
}
@@ -818,41 +1151,24 @@ void revertFlexVolSnapshots(List flexVolDetails) {
}
Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(detail.poolId);
- StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails);
- SnapshotFeignClient snapshotClient = storageStrategy.getSnapshotFeignClient();
- String authHeader = storageStrategy.getAuthHeader();
+ StorageStrategy storageStrategy = resolveStorageStrategy(poolDetails);
- // Get SVM name and FlexVolume name from pool details
- String svmName = poolDetails.get(OntapStorageConstants.SVM_NAME);
String flexVolName = poolDetails.get(OntapStorageConstants.VOLUME_NAME);
-
- if (svmName == null || svmName.isEmpty()) {
- throw new CloudRuntimeException("SVM name not found in pool details for pool [" + detail.poolId + "]");
- }
if (flexVolName == null || flexVolName.isEmpty()) {
throw new CloudRuntimeException("FlexVolume name not found in pool details for pool [" + detail.poolId + "]");
}
- // The path must start with "/" for the ONTAP CLI API
String ontapFilePath = detail.volumePath.startsWith("/") ? detail.volumePath : "/" + detail.volumePath;
logger.info("revertFlexVolSnapshots: Restoring volume [{}] from FlexVol snapshot [{}] on FlexVol [{}] (protocol={})",
ontapFilePath, detail.snapshotName, flexVolName, detail.protocol);
- // Use CLI-based restore API: POST /api/private/cli/volume/snapshot/restore-file
- CliSnapshotRestoreRequest restoreRequest = new CliSnapshotRestoreRequest(
- svmName, flexVolName, detail.snapshotName, ontapFilePath);
+ JobResponse jobResponse = storageStrategy.revertSnapshotForCloudStackVolume(
+ detail.snapshotName, detail.flexVolUuid, detail.snapshotUuid,
+ detail.volumePath, null, flexVolName);
- JobResponse jobResponse = snapshotClient.restoreFileFromSnapshotCli(authHeader, restoreRequest);
-
- if (jobResponse != null && jobResponse.getJob() != null) {
- Boolean success = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 60, 2000);
- if (!success) {
- throw new CloudRuntimeException("Snapshot file restore failed for volume path [" +
- ontapFilePath + "] from snapshot [" + detail.snapshotName +
- "] on FlexVol [" + flexVolName + "]");
- }
- }
+ storageStrategy.executeCliSfsrRestore(jobResponse,
+ "VM snapshot file restore for path [" + ontapFilePath + "] from snapshot [" + detail.snapshotName + "]");
logger.info("revertFlexVolSnapshots: Successfully restored volume [{}] from snapshot [{}] on FlexVol [{}]",
ontapFilePath, detail.snapshotName, flexVolName);
@@ -877,6 +1193,69 @@ static class FlexVolGroupInfo {
}
}
+ /**
+ * Identifies the ONTAP cluster management endpoint and SVM for CG operations.
+ */
+ static class ConsistencyGroupScope {
+ final String storageIp;
+ final String svmName;
+ final String svmUuid;
+
+ ConsistencyGroupScope(String storageIp, String svmName, String svmUuid) {
+ this.storageIp = storageIp;
+ this.svmName = svmName;
+ this.svmUuid = svmUuid;
+ }
+
+ boolean matches(ConsistencyGroupScope other) {
+ return other != null && identityKey().equals(other.identityKey());
+ }
+
+ String identityKey() {
+ if (svmUuid != null && !svmUuid.isEmpty()) {
+ return storageIp + "|" + svmUuid;
+ }
+ return storageIp + "|" + svmName;
+ }
+
+ Map toOntapSvmReference() {
+ Svm svm = toOntapSvm();
+ Map svmRef = new LinkedHashMap<>();
+ if (svm.getUuid() != null && !svm.getUuid().isEmpty()) {
+ svmRef.put("uuid", svm.getUuid());
+ } else {
+ svmRef.put("name", svm.getName());
+ }
+ return svmRef;
+ }
+
+ Svm toOntapSvm() {
+ Svm svm = new Svm();
+ if (svmUuid != null && !svmUuid.isEmpty()) {
+ svm.setUuid(svmUuid);
+ } else {
+ svm.setName(svmName);
+ }
+ return svm;
+ }
+
+ void applySvmQueryFilter(Map query) {
+ if (svmUuid != null && !svmUuid.isEmpty()) {
+ query.put("svm.uuid", svmUuid);
+ } else {
+ query.put("svm.name", svmName);
+ }
+ }
+
+ @Override
+ public String toString() {
+ if (svmUuid != null && !svmUuid.isEmpty()) {
+ return "storageIP=" + storageIp + ", svmUUID=" + svmUuid + ", svmName=" + svmName;
+ }
+ return "storageIP=" + storageIp + ", svmName=" + svmName;
+ }
+ }
+
/**
* Holds the metadata for a single volume's FlexVolume snapshot entry (used during create and for
* serialization/deserialization to/from vm_snapshot_details).
diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java
index 3c139e23cb88..571002df2a7f 100644
--- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java
+++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java
@@ -134,6 +134,7 @@ void testGetCapabilities() {
// so StorageSystemSnapshotStrategy handles snapshot backup to secondary storage
assertEquals(Boolean.TRUE.toString(), capabilities.get("STORAGE_SYSTEM_SNAPSHOT"));
assertEquals(Boolean.TRUE.toString(), capabilities.get("CAN_CREATE_VOLUME_FROM_SNAPSHOT"));
+ assertEquals(Boolean.TRUE.toString(), capabilities.get("CAN_REVERT_VOLUME_TO_SNAPSHOT"));
}
@Test
diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java
index 880615636fa3..070c352a7620 100644
--- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java
+++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java
@@ -27,6 +27,7 @@
import org.apache.cloudstack.storage.feign.client.JobFeignClient;
import org.apache.cloudstack.storage.feign.client.NetworkFeignClient;
import org.apache.cloudstack.storage.feign.client.SANFeignClient;
+import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient;
import org.apache.cloudstack.storage.feign.client.SvmFeignClient;
import org.apache.cloudstack.storage.feign.client.VolumeFeignClient;
import org.apache.cloudstack.storage.feign.model.Aggregate;
@@ -56,6 +57,7 @@
import org.mockito.Mock;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -89,6 +91,9 @@ public class StorageStrategyTest {
@Mock
private SANFeignClient sanFeignClient;
+ @Mock
+ private SnapshotFeignClient snapshotFeignClient;
+
private TestableStorageStrategy storageStrategy;
// Concrete implementation for testing abstract class
@@ -99,7 +104,8 @@ public TestableStorageStrategy(OntapStorage ontapStorage,
SvmFeignClient svmFeignClient,
JobFeignClient jobFeignClient,
NetworkFeignClient networkFeignClient,
- SANFeignClient sanFeignClient) {
+ SANFeignClient sanFeignClient,
+ SnapshotFeignClient snapshotFeignClient) {
super(ontapStorage);
// Use reflection to replace the private Feign client fields with mocked ones
injectMockedClient("aggregateFeignClient", aggregateFeignClient);
@@ -108,6 +114,7 @@ public TestableStorageStrategy(OntapStorage ontapStorage,
injectMockedClient("jobFeignClient", jobFeignClient);
injectMockedClient("networkFeignClient", networkFeignClient);
injectMockedClient("sanFeignClient", sanFeignClient);
+ injectMockedClient("snapshotFeignClient", snapshotFeignClient);
}
private void injectMockedClient(String fieldName, Object mockedClient) {
@@ -193,7 +200,7 @@ void setUp() {
// For testing, we'll need to mock the FeignClientFactory behavior
storageStrategy = new TestableStorageStrategy(ontapStorage,
aggregateFeignClient, volumeFeignClient, svmFeignClient,
- jobFeignClient, networkFeignClient, sanFeignClient);
+ jobFeignClient, networkFeignClient, sanFeignClient, snapshotFeignClient);
}
// ========== connect() Tests ==========
@@ -232,42 +239,7 @@ public void testConnect_positive() {
}
@Test
- public void testConnect_succeedsWhenAggregateSpaceBelowPoolCapacity() {
- // Regression: connect() must validate connectivity/SVM/aggregate-state ONLY.
- // Capacity is validated per-volume in createStorageVolume(name, size). Previously
- // connect() compared aggregate free space against the whole storage pool size
- // (storage.getSize()), which incorrectly failed data-path operations (volume/LUN
- // create, grant/revoke access, delete) once the pool FlexVolume already existed.
- Svm svm = new Svm();
- svm.setName("svm1");
- svm.setState(OntapStorageConstants.RUNNING);
- svm.setNfsEnabled(true);
-
- Aggregate aggregate = new Aggregate();
- aggregate.setName("aggr1");
- aggregate.setUuid("aggr-uuid-1");
- svm.setAggregates(List.of(aggregate));
-
- OntapResponse svmResponse = new OntapResponse<>();
- svmResponse.setRecords(List.of(svm));
-
- when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
-
- // Aggregate is ONLINE but has far less free space than the configured pool size (5GB).
- Aggregate aggregateDetail = mock(Aggregate.class);
- when(aggregateDetail.getName()).thenReturn("aggr1");
- when(aggregateDetail.getUuid()).thenReturn("aggr-uuid-1");
- when(aggregateDetail.getState()).thenReturn(Aggregate.StateEnum.ONLINE);
- when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"))).thenReturn(aggregateDetail);
-
- // Execute & Verify - connect() should succeed regardless of available space.
- boolean result = storageStrategy.connect();
- assertTrue(result, "connect() should succeed for an online aggregate even when its free space is below the pool capacity");
- }
-
- @Test
- public void testConnect_noOnlineAggregates() {
- // Setup - aggregate assigned to the SVM exists but is not ONLINE
+ public void testConnect_operationsOnly_skipsAggregateValidation() {
Svm svm = new Svm();
svm.setName("svm1");
svm.setState(OntapStorageConstants.RUNNING);
@@ -283,15 +255,10 @@ public void testConnect_noOnlineAggregates() {
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
- Aggregate aggregateDetail = mock(Aggregate.class);
- when(aggregateDetail.getName()).thenReturn("aggr1");
- when(aggregateDetail.getUuid()).thenReturn("aggr-uuid-1");
- when(aggregateDetail.getState()).thenReturn(null); // not online
- when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"))).thenReturn(aggregateDetail);
+ boolean result = storageStrategy.connect(false);
- // Execute & Verify
- CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> storageStrategy.connect());
- assertTrue(ex.getMessage().contains("No suitable aggregates found"));
+ assertTrue(result);
+ verify(aggregateFeignClient, never()).getAggregateByUUID(anyString(), anyString());
}
@Test
@@ -355,7 +322,7 @@ public void testConnect_iscsiNotEnabled() {
"svm1", 5000000000L, ProtocolType.ISCSI);
storageStrategy = new TestableStorageStrategy(iscsiStorage,
aggregateFeignClient, volumeFeignClient, svmFeignClient,
- jobFeignClient, networkFeignClient, sanFeignClient);
+ jobFeignClient, networkFeignClient, sanFeignClient, snapshotFeignClient);
Svm svm = new Svm();
svm.setName("svm1");
@@ -705,7 +672,7 @@ public void testGetStoragePath_iscsi() {
"svm1", null, ProtocolType.ISCSI);
storageStrategy = new TestableStorageStrategy(iscsiStorage,
aggregateFeignClient, volumeFeignClient, svmFeignClient,
- jobFeignClient, networkFeignClient, sanFeignClient);
+ jobFeignClient, networkFeignClient, sanFeignClient, snapshotFeignClient);
IscsiService.IscsiServiceTarget target = new IscsiService.IscsiServiceTarget();
target.setName("iqn.1992-08.com.netapp:sn.123456:vs.1");
@@ -735,7 +702,7 @@ public void testGetStoragePath_iscsi_noService() {
"svm1", null, ProtocolType.ISCSI);
storageStrategy = new TestableStorageStrategy(iscsiStorage,
aggregateFeignClient, volumeFeignClient, svmFeignClient,
- jobFeignClient, networkFeignClient, sanFeignClient);
+ jobFeignClient, networkFeignClient, sanFeignClient, snapshotFeignClient);
OntapResponse emptyResponse = new OntapResponse<>();
emptyResponse.setRecords(new ArrayList<>());
@@ -756,7 +723,7 @@ public void testGetStoragePath_iscsi_noTargetIqn() {
"svm1", null, ProtocolType.ISCSI);
storageStrategy = new TestableStorageStrategy(iscsiStorage,
aggregateFeignClient, volumeFeignClient, svmFeignClient,
- jobFeignClient, networkFeignClient, sanFeignClient);
+ jobFeignClient, networkFeignClient, sanFeignClient, snapshotFeignClient);
IscsiService iscsiService = new IscsiService();
iscsiService.setTarget(null);
@@ -806,7 +773,7 @@ public void testGetNetworkInterface_iscsi() {
"svm1", null, ProtocolType.ISCSI);
storageStrategy = new TestableStorageStrategy(iscsiStorage,
aggregateFeignClient, volumeFeignClient, svmFeignClient,
- jobFeignClient, networkFeignClient, sanFeignClient);
+ jobFeignClient, networkFeignClient, sanFeignClient, snapshotFeignClient);
IpInterface.IpInfo ipInfo = new IpInterface.IpInfo();
ipInfo.setAddress("192.168.1.51");
diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java
index b069ab7246a0..a3ffed1fb0f7 100644
--- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java
+++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java
@@ -21,12 +21,17 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -44,6 +49,14 @@
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient;
+import org.apache.cloudstack.storage.feign.model.ConsistencyGroup;
+import org.apache.cloudstack.storage.feign.model.ConsistencyGroupSnapshot;
+import org.apache.cloudstack.storage.feign.model.FlexVolSnapshot;
+import org.apache.cloudstack.storage.feign.model.Job;
+import org.apache.cloudstack.storage.feign.model.response.JobResponse;
+import org.apache.cloudstack.storage.feign.model.response.OntapResponse;
+import org.apache.cloudstack.storage.service.StorageStrategy;
import org.apache.cloudstack.storage.to.VolumeObjectTO;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -127,6 +140,10 @@ class OntapVMSnapshotStrategyTest {
private VolumeDataFactory volumeDataFactory;
@Mock
private VolumeDetailsDao volumeDetailsDao;
+ @Mock
+ private StorageStrategy storageStrategy;
+ @Mock
+ private SnapshotFeignClient snapshotFeignClient;
@Spy
@InjectMocks
@@ -226,14 +243,18 @@ void testCanHandle_AllocatedDiskType_VmxenHypervisor_ReturnsCantHandle() {
}
@Test
- void testCanHandle_AllocatedDiskType_VmNotRunning_ReturnsCantHandle() {
+ void testCanHandle_AllocatedDiskType_VmStopped_ReturnsHighest() {
UserVmVO userVm = createMockUserVm(Hypervisor.HypervisorType.KVM, VirtualMachine.State.Stopped);
when(userVmDao.findById(VM_ID)).thenReturn(userVm);
VMSnapshotVO vmSnapshot = createMockVmSnapshot(VMSnapshot.State.Allocated, VMSnapshot.Type.Disk);
+ VolumeVO vol = createMockVolume(VOLUME_ID_1, POOL_ID_1);
+ when(volumeDao.findByInstance(VM_ID)).thenReturn(Collections.singletonList(vol));
+ StoragePoolVO pool = createOntapManagedPool(POOL_ID_1);
+ when(storagePool.findById(POOL_ID_1)).thenReturn(pool);
StrategyPriority result = strategy.canHandle(vmSnapshot);
- assertEquals(StrategyPriority.CANT_HANDLE, result);
+ assertEquals(StrategyPriority.HIGHEST, result);
}
@Test
@@ -532,6 +553,86 @@ void testGroupVolumesByFlexVol_VolumeNotFound_ThrowsException() {
() -> strategy.groupVolumesByFlexVol(Collections.singletonList(volumeTO1)));
}
+ @Test
+ void testCreateTemporaryConsistencyGroup_includesSvmName() {
+ SnapshotFeignClient client = mock(SnapshotFeignClient.class);
+ StorageStrategy storageStrategy = mock(StorageStrategy.class);
+ when(client.createConsistencyGroup(any(), any())).thenReturn(createJobResponse("job-cg-create"));
+ OntapResponse cgResponse = new OntapResponse<>();
+ ConsistencyGroup cgRecord = new ConsistencyGroup();
+ cgRecord.setUuid("cg-uuid-1");
+ cgResponse.setRecords(Collections.singletonList(cgRecord));
+ when(client.getConsistencyGroups(any(), any())).thenReturn(cgResponse);
+
+ String cgUuid = strategy.createTemporaryConsistencyGroup(client, storageStrategy, "auth",
+ "cg-name", new OntapVMSnapshotStrategy.ConsistencyGroupScope("10.0.0.1", "vs0", "svm-uuid-1"),
+ java.util.Set.of("flexvol-uuid-1", "flexvol-uuid-2"));
+
+ assertEquals("cg-uuid-1", cgUuid);
+ org.mockito.ArgumentCaptor payloadCaptor = org.mockito.ArgumentCaptor.forClass(ConsistencyGroup.class);
+ verify(client).createConsistencyGroup(eq("auth"), payloadCaptor.capture());
+ ConsistencyGroup payload = payloadCaptor.getValue();
+ assertEquals("cg-name", payload.getName());
+ assertEquals("svm-uuid-1", payload.getSvm().getUuid());
+ assertEquals(2, payload.getVolumes().size());
+ assertEquals(OntapStorageConstants.CG_VOLUME_PROVISIONING_ACTION_ADD,
+ payload.getVolumes().get(0).getProvisioningOptions().getAction());
+ }
+
+ @Test
+ void testResolveConsistencyGroupScope_rejectsDifferentStorageIpWithSameSvmName() {
+ Map groups = new HashMap<>();
+ Map poolDetails1 = new HashMap<>();
+ poolDetails1.put(OntapStorageConstants.STORAGE_IP, "10.1.1.1");
+ poolDetails1.put(OntapStorageConstants.SVM_NAME, "vs0");
+ groups.put("flexvol-uuid-1", new OntapVMSnapshotStrategy.FlexVolGroupInfo(poolDetails1, POOL_ID_1));
+
+ Map poolDetails2 = new HashMap<>();
+ poolDetails2.put(OntapStorageConstants.STORAGE_IP, "10.2.2.2");
+ poolDetails2.put(OntapStorageConstants.SVM_NAME, "vs0");
+ groups.put("flexvol-uuid-2", new OntapVMSnapshotStrategy.FlexVolGroupInfo(poolDetails2, POOL_ID_2));
+
+ assertThrows(CloudRuntimeException.class, () -> strategy.resolveConsistencyGroupScope(groups));
+ }
+
+ @Test
+ void testResolveConsistencyGroupScope_acceptsSameClusterAndSvmUuid() {
+ Map groups = new HashMap<>();
+ Map poolDetails1 = new HashMap<>();
+ poolDetails1.put(OntapStorageConstants.STORAGE_IP, "10.1.1.1");
+ poolDetails1.put(OntapStorageConstants.SVM_NAME, "vs0");
+ poolDetails1.put(OntapStorageConstants.SVM_UUID, "svm-uuid-shared");
+ groups.put("flexvol-uuid-1", new OntapVMSnapshotStrategy.FlexVolGroupInfo(poolDetails1, POOL_ID_1));
+
+ Map poolDetails2 = new HashMap<>();
+ poolDetails2.put(OntapStorageConstants.STORAGE_IP, "10.1.1.1");
+ poolDetails2.put(OntapStorageConstants.SVM_NAME, "vs0");
+ poolDetails2.put(OntapStorageConstants.SVM_UUID, "svm-uuid-shared");
+ groups.put("flexvol-uuid-2", new OntapVMSnapshotStrategy.FlexVolGroupInfo(poolDetails2, POOL_ID_2));
+
+ OntapVMSnapshotStrategy.ConsistencyGroupScope scope = strategy.resolveConsistencyGroupScope(groups);
+ assertEquals("svm-uuid-shared", scope.svmUuid);
+ assertEquals("10.1.1.1", scope.storageIp);
+ }
+
+ @Test
+ void testResolveConsistencyGroupScope_rejectsDifferentSvmUuidOnSameCluster() {
+ Map groups = new HashMap<>();
+ Map poolDetails1 = new HashMap<>();
+ poolDetails1.put(OntapStorageConstants.STORAGE_IP, "10.1.1.1");
+ poolDetails1.put(OntapStorageConstants.SVM_NAME, "vs0");
+ poolDetails1.put(OntapStorageConstants.SVM_UUID, "svm-uuid-1");
+ groups.put("flexvol-uuid-1", new OntapVMSnapshotStrategy.FlexVolGroupInfo(poolDetails1, POOL_ID_1));
+
+ Map poolDetails2 = new HashMap<>();
+ poolDetails2.put(OntapStorageConstants.STORAGE_IP, "10.1.1.1");
+ poolDetails2.put(OntapStorageConstants.SVM_NAME, "vs0");
+ poolDetails2.put(OntapStorageConstants.SVM_UUID, "svm-uuid-2");
+ groups.put("flexvol-uuid-2", new OntapVMSnapshotStrategy.FlexVolGroupInfo(poolDetails2, POOL_ID_2));
+
+ assertThrows(CloudRuntimeException.class, () -> strategy.resolveConsistencyGroupScope(groups));
+ }
+
// ══════════════════════════════════════════════════════════════════════════
// Tests: FlexVolSnapshotDetail parse/toString
// ══════════════════════════════════════════════════════════════════════════
@@ -593,10 +694,11 @@ void testFlexVolSnapshotDetail_Parse5Parts_ThrowsException() {
void testBuildSnapshotName_Format() {
VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class);
when(vmSnapshot.getId()).thenReturn(SNAPSHOT_ID);
+ when(vmSnapshot.getName()).thenReturn("UI VM Snapshot");
String name = strategy.buildSnapshotName(vmSnapshot);
- assertEquals(true, name.startsWith("vmsnap_200_"));
+ assertEquals(true, name.startsWith("UI_VM_Snapshot_vm200"));
assertEquals(true, name.length() <= OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH);
}
@@ -732,6 +834,86 @@ void testTakeVMSnapshot_OperationTimeout_ThrowsCloudRuntimeException() throws Ex
assertEquals(true, ex.getMessage().contains("timed out"));
}
+ @Test
+ void testTakeVMSnapshot_SingleFlexVolSuccess_UsesDirectSnapshotNotCg() throws Exception {
+ VMSnapshotVO vmSnapshot = createTakeSnapshotVmSnapshot();
+ setupTakeSnapshotCommon(vmSnapshot);
+ setupSingleVolumeForTakeSnapshot();
+
+ String snapshotName = strategy.buildSnapshotName(vmSnapshot);
+ setupSingleFlexVolFlowMocks(snapshotName);
+
+ FreezeThawVMAnswer freezeAnswer = mock(FreezeThawVMAnswer.class);
+ when(freezeAnswer.getResult()).thenReturn(true);
+ FreezeThawVMAnswer thawAnswer = mock(FreezeThawVMAnswer.class);
+ when(thawAnswer.getResult()).thenReturn(true);
+ when(agentMgr.send(eq(HOST_ID), any(FreezeThawVMCommand.class)))
+ .thenReturn(freezeAnswer)
+ .thenReturn(thawAnswer);
+
+ strategy.takeVMSnapshot(vmSnapshot);
+
+ verify(snapshotFeignClient, times(1)).createSnapshot(any(), eq("flexvol-uuid-1"), any());
+ verify(snapshotFeignClient, never()).createConsistencyGroup(any(), any());
+ verify(snapshotFeignClient, never()).createConsistencyGroupSnapshot(any(), any(), any());
+ verify(snapshotFeignClient, never()).commitConsistencyGroupSnapshot(any(), any(), any(), any());
+ verify(snapshotFeignClient, never()).deleteConsistencyGroup(any(), any());
+ verify(vmSnapshotDetailsDao, atLeastOnce()).persist(any(VMSnapshotDetailsVO.class));
+ }
+
+ @Test
+ void testTakeVMSnapshot_TemporaryCgTwoPhaseSuccess_PersistsDetailsAndCleansUpCg() throws Exception {
+ VMSnapshotVO vmSnapshot = createTakeSnapshotVmSnapshot();
+ setupTakeSnapshotCommon(vmSnapshot);
+ setupMultiFlexVolForTakeSnapshot();
+
+ String snapshotName = strategy.buildSnapshotName(vmSnapshot);
+ setupTemporaryCgFlowMocks(snapshotName);
+
+ FreezeThawVMAnswer freezeAnswer = mock(FreezeThawVMAnswer.class);
+ when(freezeAnswer.getResult()).thenReturn(true);
+ FreezeThawVMAnswer thawAnswer = mock(FreezeThawVMAnswer.class);
+ when(thawAnswer.getResult()).thenReturn(true);
+ when(agentMgr.send(eq(HOST_ID), any(FreezeThawVMCommand.class)))
+ .thenReturn(freezeAnswer)
+ .thenReturn(thawAnswer);
+
+ strategy.takeVMSnapshot(vmSnapshot);
+
+ verify(snapshotFeignClient, times(1)).createConsistencyGroup(any(), any());
+ verify(snapshotFeignClient, times(1)).createConsistencyGroupSnapshot(any(), eq("cg-uuid-1"), any());
+ verify(snapshotFeignClient, times(1)).commitConsistencyGroupSnapshot(any(), eq("cg-uuid-1"), eq("cg-snap-uuid-1"), any());
+ verify(snapshotFeignClient, times(1)).deleteConsistencyGroup(any(), eq("cg-uuid-1"));
+ verify(vmSnapshotDetailsDao, atLeastOnce()).persist(any(VMSnapshotDetailsVO.class));
+ }
+
+ @Test
+ void testTakeVMSnapshot_TemporaryCgStartFails_TransitionsToOperationFailed() throws Exception {
+ VMSnapshotVO vmSnapshot = createTakeSnapshotVmSnapshot();
+ setupTakeSnapshotCommon(vmSnapshot);
+ setupMultiFlexVolForTakeSnapshot();
+
+ String snapshotName = strategy.buildSnapshotName(vmSnapshot);
+ setupTemporaryCgFlowMocks(snapshotName);
+ when(snapshotFeignClient.createConsistencyGroupSnapshot(any(), eq("cg-uuid-1"), any()))
+ .thenThrow(new CloudRuntimeException("start phase failed"));
+
+ FreezeThawVMAnswer freezeAnswer = mock(FreezeThawVMAnswer.class);
+ when(freezeAnswer.getResult()).thenReturn(true);
+ FreezeThawVMAnswer thawAnswer = mock(FreezeThawVMAnswer.class);
+ when(thawAnswer.getResult()).thenReturn(true);
+ when(agentMgr.send(eq(HOST_ID), any(FreezeThawVMCommand.class)))
+ .thenReturn(freezeAnswer)
+ .thenReturn(thawAnswer);
+ when(vmSnapshotDetailsDao.listDetails(SNAPSHOT_ID)).thenReturn(Collections.emptyList());
+ doReturn(true).when(vmSnapshotHelper).vmSnapshotStateTransitTo(any(), eq(VMSnapshot.Event.OperationFailed));
+
+ assertThrows(CloudRuntimeException.class, () -> strategy.takeVMSnapshot(vmSnapshot));
+
+ verify(snapshotFeignClient, times(1)).deleteConsistencyGroup(any(), eq("cg-uuid-1"));
+ verify(vmSnapshotHelper, atLeastOnce()).vmSnapshotStateTransitTo(any(), eq(VMSnapshot.Event.OperationFailed));
+ }
+
// ══════════════════════════════════════════════════════════════════════════
// Tests: Quiesce Behavior
// ══════════════════════════════════════════════════════════════════════════
@@ -746,20 +928,9 @@ void testTakeVMSnapshot_QuiesceFalse_SkipsFreezeThaw() throws Exception {
setupTakeSnapshotCommon(vmSnapshot);
setupSingleVolumeForTakeSnapshot();
+ setupSingleFlexVolFlowMocks(strategy.buildSnapshotName(vmSnapshot));
- // The FlexVolume snapshot flow will try to call Utility.getStrategyByStoragePoolDetails
- // which is a static method that makes real connections. We expect this to fail in unit tests.
- // The important thing is that freeze/thaw was NOT called before the failure.
- when(vmSnapshotDetailsDao.listDetails(SNAPSHOT_ID)).thenReturn(Collections.emptyList());
- doReturn(true).when(vmSnapshotHelper).vmSnapshotStateTransitTo(any(), eq(VMSnapshot.Event.OperationFailed));
-
- // Since Utility.getStrategyByStoragePoolDetails is static and creates real Feign clients,
- // this will fail. We just verify that freeze was never called.
- try {
- strategy.takeVMSnapshot(vmSnapshot);
- } catch (Exception e) {
- // Expected — static utility can't be mocked in unit test
- }
+ strategy.takeVMSnapshot(vmSnapshot);
// No freeze/thaw commands should be sent when quiesce is false
verify(agentMgr, never()).send(eq(HOST_ID), any(FreezeThawVMCommand.class));
@@ -790,16 +961,9 @@ void testTakeVMSnapshot_WithParentSnapshot_SetsParentId() throws Exception {
when(agentMgr.send(eq(HOST_ID), any(FreezeThawVMCommand.class)))
.thenReturn(freezeAnswer)
.thenReturn(thawAnswer);
+ setupSingleFlexVolFlowMocks(strategy.buildSnapshotName(vmSnapshot));
- when(vmSnapshotDetailsDao.listDetails(SNAPSHOT_ID)).thenReturn(Collections.emptyList());
- doReturn(true).when(vmSnapshotHelper).vmSnapshotStateTransitTo(any(), eq(VMSnapshot.Event.OperationFailed));
-
- // FlexVol snapshot flow will fail on static method, but parent should already be set
- try {
- strategy.takeVMSnapshot(vmSnapshot);
- } catch (Exception e) {
- // Expected
- }
+ strategy.takeVMSnapshot(vmSnapshot);
// Verify parent was set on the VM snapshot before the FlexVol snapshot attempt
verify(vmSnapshot).setParent(199L);
@@ -820,15 +984,9 @@ void testTakeVMSnapshot_WithNoParentSnapshot_SetsParentNull() throws Exception {
when(agentMgr.send(eq(HOST_ID), any(FreezeThawVMCommand.class)))
.thenReturn(freezeAnswer)
.thenReturn(thawAnswer);
+ setupSingleFlexVolFlowMocks(strategy.buildSnapshotName(vmSnapshot));
- when(vmSnapshotDetailsDao.listDetails(SNAPSHOT_ID)).thenReturn(Collections.emptyList());
- doReturn(true).when(vmSnapshotHelper).vmSnapshotStateTransitTo(any(), eq(VMSnapshot.Event.OperationFailed));
-
- try {
- strategy.takeVMSnapshot(vmSnapshot);
- } catch (Exception e) {
- // Expected
- }
+ strategy.takeVMSnapshot(vmSnapshot);
verify(vmSnapshot).setParent(null);
}
@@ -866,6 +1024,9 @@ private UserVmVO setupTakeSnapshotCommon(VMSnapshotVO vmSnapshot) throws Excepti
when(vmSnapshotDao.findCurrentSnapshotByVmId(VM_ID)).thenReturn(null);
doReturn(true).when(vmSnapshotHelper).vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.CreateRequested);
+ doNothing().when(strategy).processAnswer(any(), any(), any(), any());
+ doNothing().when(strategy).publishUsageEvent(any(), any(), any(), any());
+ doNothing().when(strategy).publishUsageEvent(any(), any(), any(), anyLong(), anyLong());
return userVm;
}
@@ -880,6 +1041,7 @@ private void setupSingleVolumeForTakeSnapshot() {
VolumeVO volumeVO = mock(VolumeVO.class);
when(volumeVO.getId()).thenReturn(VOLUME_ID_1);
when(volumeVO.getPoolId()).thenReturn(POOL_ID_1);
+ when(volumeVO.getPath()).thenReturn("volume-301.qcow2");
when(volumeVO.getVmSnapshotChainSize()).thenReturn(null);
when(volumeDao.findById(VOLUME_ID_1)).thenReturn(volumeVO);
@@ -899,4 +1061,139 @@ private void setupSingleVolumeForTakeSnapshot() {
when(volumeInfo.getName()).thenReturn("vol-1");
when(volumeDataFactory.getVolume(VOLUME_ID_1)).thenReturn(volumeInfo);
}
+
+ private void setupMultiFlexVolForTakeSnapshot() {
+ VolumeObjectTO volumeTO1 = mock(VolumeObjectTO.class);
+ when(volumeTO1.getId()).thenReturn(VOLUME_ID_1);
+ when(volumeTO1.getSize()).thenReturn(10737418240L);
+ VolumeObjectTO volumeTO2 = mock(VolumeObjectTO.class);
+ when(volumeTO2.getId()).thenReturn(VOLUME_ID_2);
+ when(volumeTO2.getSize()).thenReturn(10737418240L);
+ List volumeTOs = Arrays.asList(volumeTO1, volumeTO2);
+ when(vmSnapshotHelper.getVolumeTOList(VM_ID)).thenReturn(volumeTOs);
+
+ VolumeVO volumeVO1 = mock(VolumeVO.class);
+ when(volumeVO1.getId()).thenReturn(VOLUME_ID_1);
+ when(volumeVO1.getPoolId()).thenReturn(POOL_ID_1);
+ when(volumeVO1.getPath()).thenReturn("volume-301.qcow2");
+ when(volumeVO1.getVmSnapshotChainSize()).thenReturn(null);
+ when(volumeDao.findById(VOLUME_ID_1)).thenReturn(volumeVO1);
+
+ VolumeVO volumeVO2 = mock(VolumeVO.class);
+ when(volumeVO2.getId()).thenReturn(VOLUME_ID_2);
+ when(volumeVO2.getPoolId()).thenReturn(POOL_ID_2);
+ when(volumeVO2.getPath()).thenReturn("volume-302.qcow2");
+ when(volumeVO2.getVmSnapshotChainSize()).thenReturn(null);
+ when(volumeDao.findById(VOLUME_ID_2)).thenReturn(volumeVO2);
+
+ Map poolDetails1 = new HashMap<>();
+ poolDetails1.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-1");
+ poolDetails1.put(OntapStorageConstants.USERNAME, "admin");
+ poolDetails1.put(OntapStorageConstants.PASSWORD, "pass");
+ poolDetails1.put(OntapStorageConstants.STORAGE_IP, "10.0.0.1");
+ poolDetails1.put(OntapStorageConstants.SVM_NAME, "svm1");
+ poolDetails1.put(OntapStorageConstants.SVM_UUID, "svm-uuid-shared");
+ poolDetails1.put(OntapStorageConstants.SIZE, "107374182400");
+ poolDetails1.put(OntapStorageConstants.PROTOCOL, "NFS3");
+ when(storagePoolDetailsDao.listDetailsKeyPairs(POOL_ID_1)).thenReturn(poolDetails1);
+
+ Map poolDetails2 = new HashMap<>();
+ poolDetails2.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-2");
+ poolDetails2.put(OntapStorageConstants.USERNAME, "admin");
+ poolDetails2.put(OntapStorageConstants.PASSWORD, "pass");
+ poolDetails2.put(OntapStorageConstants.STORAGE_IP, "10.0.0.1");
+ poolDetails2.put(OntapStorageConstants.SVM_NAME, "svm1");
+ poolDetails2.put(OntapStorageConstants.SVM_UUID, "svm-uuid-shared");
+ poolDetails2.put(OntapStorageConstants.SIZE, "107374182400");
+ poolDetails2.put(OntapStorageConstants.PROTOCOL, "NFS3");
+ when(storagePoolDetailsDao.listDetailsKeyPairs(POOL_ID_2)).thenReturn(poolDetails2);
+
+ VolumeInfo volumeInfo1 = mock(VolumeInfo.class);
+ when(volumeInfo1.getId()).thenReturn(VOLUME_ID_1);
+ when(volumeDataFactory.getVolume(VOLUME_ID_1)).thenReturn(volumeInfo1);
+ VolumeInfo volumeInfo2 = mock(VolumeInfo.class);
+ when(volumeInfo2.getId()).thenReturn(VOLUME_ID_2);
+ when(volumeDataFactory.getVolume(VOLUME_ID_2)).thenReturn(volumeInfo2);
+ }
+
+ private JobResponse createJobResponse(String uuid) {
+ Job job = new Job();
+ job.setUuid(uuid);
+ JobResponse response = new JobResponse();
+ response.setJob(job);
+ return response;
+ }
+
+ private void setupSingleFlexVolFlowMocks(String snapshotName) {
+ doReturn(storageStrategy).when(strategy).resolveStorageStrategy(any());
+ when(storageStrategy.getSnapshotFeignClient()).thenReturn(snapshotFeignClient);
+ when(storageStrategy.getAuthHeader()).thenReturn("Basic dGVzdDp0ZXN0");
+ when(storageStrategy.jobPollForSuccess(any(), anyInt(), anyInt())).thenReturn(true);
+
+ when(snapshotFeignClient.createSnapshot(any(), eq("flexvol-uuid-1"), any()))
+ .thenReturn(createJobResponse("job-fv-snap"));
+
+ OntapResponse flexVolSnapshots = new OntapResponse<>();
+ FlexVolSnapshot flexVolSnapshot = new FlexVolSnapshot();
+ flexVolSnapshot.setUuid("fv-snap-uuid-1");
+ flexVolSnapshot.setName(snapshotName);
+ flexVolSnapshots.setRecords(Collections.singletonList(flexVolSnapshot));
+ when(snapshotFeignClient.getSnapshots(any(), eq("flexvol-uuid-1"), any()))
+ .thenReturn(flexVolSnapshots);
+ }
+
+ private void setupTemporaryCgFlowMocks(String snapshotName) {
+ doReturn(storageStrategy).when(strategy).resolveStorageStrategy(any());
+ when(storageStrategy.getSnapshotFeignClient()).thenReturn(snapshotFeignClient);
+ when(storageStrategy.getAuthHeader()).thenReturn("Basic dGVzdDp0ZXN0");
+ when(storageStrategy.jobPollForSuccess(any(), anyInt(), anyInt())).thenReturn(true);
+ when(storageStrategy.pollJobIfPresentAndGetCompletedJob(any(), any())).thenAnswer(invocation -> {
+ Job completedJob = new Job();
+ completedJob.setState(OntapStorageConstants.JOB_SUCCESS);
+ String operationName = invocation.getArgument(1);
+ if (operationName != null && operationName.startsWith("start CG snapshot")) {
+ completedJob.setDescription(
+ "POST /api/application/consistency-groups/cg-uuid-1/snapshots/cg-snap-uuid-1");
+ }
+ return completedJob;
+ });
+
+ when(snapshotFeignClient.createConsistencyGroup(any(), any())).thenReturn(createJobResponse("job-cg-create"));
+ OntapResponse cgResponse = new OntapResponse<>();
+ ConsistencyGroup cgRecord = new ConsistencyGroup();
+ cgRecord.setUuid("cg-uuid-1");
+ cgResponse.setRecords(Collections.singletonList(cgRecord));
+ when(snapshotFeignClient.getConsistencyGroups(any(), any())).thenReturn(cgResponse);
+
+ when(snapshotFeignClient.createConsistencyGroupSnapshot(any(), eq("cg-uuid-1"), any()))
+ .thenReturn(createJobResponse("job-cg-start"));
+ OntapResponse cgSnapshotResponse = new OntapResponse<>();
+ ConsistencyGroupSnapshot cgSnapshotRecord = new ConsistencyGroupSnapshot();
+ cgSnapshotRecord.setUuid("cg-snap-uuid-1");
+ cgSnapshotRecord.setName(snapshotName);
+ cgSnapshotResponse.setRecords(Collections.singletonList(cgSnapshotRecord));
+ when(snapshotFeignClient.getConsistencyGroupSnapshots(any(), eq("cg-uuid-1"), any()))
+ .thenReturn(cgSnapshotResponse);
+ when(snapshotFeignClient.commitConsistencyGroupSnapshot(any(), eq("cg-uuid-1"), eq("cg-snap-uuid-1"), any()))
+ .thenReturn(createJobResponse("job-cg-commit"));
+
+ when(snapshotFeignClient.deleteConsistencyGroup(any(), eq("cg-uuid-1")))
+ .thenReturn(createJobResponse("job-cg-delete"));
+
+ OntapResponse flexVolSnapshots = new OntapResponse<>();
+ FlexVolSnapshot flexVolSnapshot = new FlexVolSnapshot();
+ flexVolSnapshot.setUuid("fv-snap-uuid-1");
+ flexVolSnapshot.setName(snapshotName);
+ flexVolSnapshots.setRecords(Collections.singletonList(flexVolSnapshot));
+ when(snapshotFeignClient.getSnapshots(any(), eq("flexvol-uuid-1"), any()))
+ .thenReturn(flexVolSnapshots);
+
+ OntapResponse flexVolSnapshots2 = new OntapResponse<>();
+ FlexVolSnapshot flexVolSnapshot2 = new FlexVolSnapshot();
+ flexVolSnapshot2.setUuid("fv-snap-uuid-2");
+ flexVolSnapshot2.setName(snapshotName);
+ flexVolSnapshots2.setRecords(Collections.singletonList(flexVolSnapshot2));
+ when(snapshotFeignClient.getSnapshots(any(), eq("flexvol-uuid-2"), any()))
+ .thenReturn(flexVolSnapshots2);
+ }
}
diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
index dc33a4442a33..a10cb59c1058 100755
--- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
+++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
@@ -51,6 +51,7 @@
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
@@ -1634,9 +1635,18 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
boolean isKvmAndFileBasedStorage = isHypervisorKvmAndFileBasedStorage(volume, storagePool);
boolean backupSnapToSecondary = isBackupSnapshotToSecondaryForZone(volume.getDataCenterId());
+
StoragePoolType poolType = volume.getStoragePoolType();
- if (isKvmAndFileBasedStorage && backupSnapToSecondary) {
+ updateSnapshotPayload(volume.getPoolId(), payload, isKvmAndFileBasedStorage, poolType, clusterId);
+
+ // NetApp ONTAP managed PRIMARY snapshots remain on primary/array storage (FlexVol).
+ // They must not use secondary archive bookkeeping (postSnapshotDirectlyToSecondary) or a physical
+ // secondary copy — delete is handled via StorageSystemSnapshotStrategy → driver deleteAsync.
+ boolean archiveSnapshotToSecondary = backupSnapToSecondary
+ && !isManagedPrimaryLocationSnapshot(storagePool, payload);
+
+ if (isKvmAndFileBasedStorage && archiveSnapshotToSecondary) {
DataStore imageStore = snapshotSrv.findSnapshotImageStore(snapshot);
if (imageStore == null) {
throw new CloudRuntimeException(String.format("Could not find any secondary storage to allocate snapshot [%s].", snapshot));
@@ -1644,7 +1654,6 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
snapshot.setImageStore(imageStore);
}
- updateSnapshotPayload(volume.getPoolId(), payload, isKvmAndFileBasedStorage, poolType, clusterId);
snapshot.addPayload(payload);
try {
@@ -1659,7 +1668,7 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
SnapshotInfo snapshotOnPrimary = snapshotStrategy.takeSnapshot(snapshot);
- if (backupSnapToSecondary) {
+ if (archiveSnapshotToSecondary) {
if (!isKvmAndFileBasedStorage) {
backupSnapshotToSecondary(payload.getAsyncBackup(), snapshotStrategy, snapshotOnPrimary, payload.getZoneIds(), payload.getStoragePoolIds());
if (!payload.getAsyncBackup() && ClvmPoolManager.isClvmPoolType(storagePool.getPoolType())) {
@@ -1669,7 +1678,15 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
postSnapshotDirectlyToSecondary(snapshot, snapshotOnPrimary, snapshotId);
}
} else {
- logger.debug("Skipping backup of snapshot [{}] to secondary due to configuration [{}].", snapshotOnPrimary.getUuid(), SnapshotInfo.BackupSnapshotAfterTakingSnapshot.key());
+ if (backupSnapToSecondary && isManagedPrimaryLocationSnapshot(storagePool, payload)) {
+ logger.info("takeSnapshot: snapshot [{}] on NetApp ONTAP managed primary pool [{}] with locationType=PRIMARY — "
+ + "keeping snapshot on primary/array storage only; not archiving to secondary "
+ + "(backup.snapshot.after.take is ignored for this snapshot class)",
+ snapshotId, storagePool.getId());
+ } else {
+ logger.debug("Skipping backup of snapshot [{}] to secondary due to configuration [{}].",
+ snapshotOnPrimary.getUuid(), SnapshotInfo.BackupSnapshotAfterTakingSnapshot.key());
+ }
if (CollectionUtils.isNotEmpty(payload.getStoragePoolIds()) && payload.getAsyncBackup()) {
snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.COPY);
@@ -1685,10 +1702,10 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
snapshotZoneDao.addSnapshotToZone(snapshotId, snapshot.getDataCenterId());
DataStoreRole dataStoreRole;
- if (payload.getAsyncBackup() && backupSnapToSecondary && !isKvmAndFileBasedStorage) {
+ if (payload.getAsyncBackup() && archiveSnapshotToSecondary && !isKvmAndFileBasedStorage) {
dataStoreRole = DataStoreRole.Primary;
} else {
- dataStoreRole = backupSnapToSecondary ? snapshotHelper.getDataStoreRole(snapshot) : DataStoreRole.Primary;
+ dataStoreRole = archiveSnapshotToSecondary ? snapshotHelper.getDataStoreRole(snapshot) : DataStoreRole.Primary;
}
List snapshotStoreRefs = _snapshotStoreDao.listReadyBySnapshot(snapshotId, dataStoreRole);
@@ -1704,7 +1721,7 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), storeResourceType, volume.getSize() - snapshotStoreRef.getPhysicalSize());
if (!payload.getAsyncBackup()) {
- if (backupSnapToSecondary) {
+ if (archiveSnapshotToSecondary) {
copyNewSnapshotToZones(snapshotId, snapshot.getDataCenterId(), payload.getZoneIds());
}
if (CollectionUtils.isNotEmpty(payload.getStoragePoolIds())) {
@@ -1734,6 +1751,12 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
return snapshot;
}
+ /**
+ * KVM file-based fast-path: records snapshot on image store without copying bytes, then drops the
+ * primary {@code snapshot_data_store} row. Used only for non-managed primary storage when
+ * {@code backup.snapshot.after.take} is enabled. NetApp ONTAP managed PRIMARY snapshots never
+ * call this method — see {@link #isManagedPrimaryLocationSnapshot}.
+ */
private void postSnapshotDirectlyToSecondary(SnapshotInfo snapshot, SnapshotInfo snapshotOnPrimary, Long snapshotId) {
logger.debug("{} was directly copied to secondary storage because the hypervisor is KVM, the primary storage is file-based and the [{}] configuration" +
" is set to true.", snapshot.getSnapshotVO().toString(), SnapshotInfo.BackupSnapshotAfterTakingSnapshot);
@@ -1750,6 +1773,23 @@ private void postSnapshotDirectlyToSecondary(SnapshotInfo snapshot, SnapshotInfo
snapshotDetailsDao.removeDetail(snapshotOnPrimary.getId(), AsyncJob.Constants.MS_ID);
}
+ /**
+ * Returns true when a volume snapshot on NetApp ONTAP is explicitly kept on managed primary/array storage.
+ *
+ * For ONTAP managed pools, {@link #updateSnapshotPayload} defaults {@code locationType} to
+ * {@link Snapshot.LocationType#PRIMARY}. ONTAP volume snapshots therefore stay on the FlexVol
+ * on primary — they are not moved or mirrored to secondary storage. The primary
+ * {@code snapshot_data_store} row must remain so volume-snapshot DELETE uses
+ * {@code StorageSystemSnapshotStrategy} and the primary datastore driver.
+ *
+ * Other managed storage providers are not affected by this check.
+ */
+ private boolean isManagedPrimaryLocationSnapshot(StoragePool storagePool, CreateSnapshotPayload payload) {
+ return storagePool != null && storagePool.isManaged()
+ && DataStoreProvider.ONTAP_PLUGIN_NAME.equals(storagePool.getStorageProviderName())
+ && Snapshot.LocationType.PRIMARY.equals(payload.getLocationType());
+ }
+
@Override
public boolean isHypervisorKvmAndFileBasedStorage(VolumeInfo volumeInfo, StoragePool storagePool) {
Set fileBasedStores = Set.of(Storage.StoragePoolType.SharedMountPoint, Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.Filesystem);
From 31e1a643e1080868c8e7d21e728d4ff342e874cb Mon Sep 17 00:00:00 2001
From: sandeeplocharla <85344604+sandeeplocharla@users.noreply.github.com>
Date: Fri, 7 Aug 2026 11:42:03 +0530
Subject: [PATCH 5/7] CSTACKEX-127: Primary storage-pool is getting created
even if desired data LIFs are not reachable (#76)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Choosing IpInterface based on its status and affinity to the chosen
aggregate
This PR...
1. Fixed an issue in picking an unavailable IP while creating the
storage pool.
2. When CloudStack creates an ONTAP primary storage pool, it now picks
the best available network interface (LIF) using a priority-based
selection:
- Best case: Uses a LIF homed on the same node as the storage aggregate
— optimal I/O, no warning
- Degraded case: All home-node LIFs are down but a failover LIF is
running on that node — pool is created, admin is warned
- Fallback case: No LIF at all on the aggregate's node, pool is created
using a LIF from a different node, admin is warned with a latency note
- Failure case: No usable LIF anywhere, pool creation fails with a clear
error
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] Build/CI
- [ ] Test (unit or integration test code)
- [X] Major
- [ ] Minor
- [ ] BLOCKER
- [ ] Critical
- [X] Major
- [ ] Minor
- [ ] Trivial
Note: The following images have been captured for NFS3, the same would
be the case for iSCSI.
Clearly, by the virtue of free space available, the plugin would choose
`sti246_vsim_ocvs040d_aggr1` by default.
**Scenario-1 [pool_P1]: Happy path; No LIFs were down.**
The first best available LIF with current node and home node matching
with the chosen node has been picked.
**Scenario-2 [pool_P2_1]: LIFs on `040d` node were down; with one LIF
whose current node: `040d`, while its home node: `040c`**
**Scenario-3 [pool_P3]: None of the `040d` node LIFs are UP. First best
available LIF is picked from `040c`.**
---
.../feign/client/AggregateFeignClient.java | 7 +-
.../storage/feign/model/Aggregate.java | 56 ++-
.../storage/feign/model/IpInterface.java | 88 +++-
.../OntapPrimaryDatastoreLifecycle.java | 85 ++--
.../storage/listener/OntapHostListener.java | 4 -
.../storage/service/StorageStrategy.java | 132 ++++--
.../storage/utils/OntapStorageConstants.java | 7 +
.../storage/utils/OntapStorageUtils.java | 19 +-
.../OntapPrimaryDatastoreLifecycleTest.java | 266 +++++++++++-
.../storage/service/StorageStrategyTest.java | 377 +++++++++++++++---
10 files changed, 899 insertions(+), 142 deletions(-)
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/AggregateFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/AggregateFeignClient.java
index f756c3d32f18..7e026b0a6b19 100644
--- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/AggregateFeignClient.java
+++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/AggregateFeignClient.java
@@ -19,10 +19,14 @@
package org.apache.cloudstack.storage.feign.client;
+import java.util.Map;
+
import org.apache.cloudstack.storage.feign.model.Aggregate;
import org.apache.cloudstack.storage.feign.model.response.OntapResponse;
+
import feign.Headers;
import feign.Param;
+import feign.QueryMap;
import feign.RequestLine;
public interface AggregateFeignClient {
@@ -33,5 +37,6 @@ public interface AggregateFeignClient {
@RequestLine("GET /api/storage/aggregates/{uuid}")
@Headers({"Authorization: {authHeader}"})
- Aggregate getAggregateByUUID(@Param("authHeader") String authHeader, @Param("uuid") String uuid);
+ Aggregate getAggregateByUUID(@Param("authHeader") String authHeader, @Param("uuid") String uuid,
+ @QueryMap Map queryParams);
}
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Aggregate.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Aggregate.java
index 8ac1717604a5..7b57be59ec25 100644
--- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Aggregate.java
+++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Aggregate.java
@@ -19,14 +19,14 @@
package org.apache.cloudstack.storage.feign.model;
+import java.util.Objects;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonValue;
-import java.util.Objects;
-
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_NULL)
public class Aggregate {
@@ -77,6 +77,17 @@ public int hashCode() {
@JsonProperty("space")
private AggregateSpace space = null;
+ @JsonProperty("node")
+ private Node node = null;
+
+
+ public Node getNode() {
+ return node;
+ }
+
+ public void setNode(Node node) {
+ this.node = node;
+ }
public Aggregate name(String name) {
this.name = name;
@@ -107,10 +118,18 @@ public StateEnum getState() {
return state;
}
+ public void setState(StateEnum state) {
+ this.state = state;
+ }
+
public AggregateSpace getSpace() {
return space;
}
+ public void setSpace(AggregateSpace space) {
+ this.space = space;
+ }
+
public Double getAvailableBlockStorageSpace() {
if (space != null && space.blockStorage != null) {
return space.blockStorage.available;
@@ -148,9 +167,32 @@ public String toString() {
return "DiskAggregates [name=" + name + ", uuid=" + uuid + "]";
}
+ @JsonIgnoreProperties(ignoreUnknown = true)
+ @JsonInclude(JsonInclude.Include.NON_NULL)
+ public static class Node {
+ @JsonProperty("name")
+ private String name;
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
+ }
+
public static class AggregateSpace {
@JsonProperty("block_storage")
private AggregateSpaceBlockStorage blockStorage = null;
+
+ public AggregateSpaceBlockStorage getBlockStorage() {
+ return blockStorage;
+ }
+
+ public void setBlockStorage(AggregateSpaceBlockStorage blockStorage) {
+ this.blockStorage = blockStorage;
+ }
}
public static class AggregateSpaceBlockStorage {
@@ -160,6 +202,14 @@ public static class AggregateSpaceBlockStorage {
private Double size = null;
@JsonProperty("used")
private Double used = null;
+
+ public Double getAvailable() {
+ return available;
+ }
+
+ public void setAvailable(Double available) {
+ this.available = available;
+ }
}
}
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/IpInterface.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/IpInterface.java
index c15798a42b70..8070763285c8 100644
--- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/IpInterface.java
+++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/IpInterface.java
@@ -19,13 +19,13 @@
package org.apache.cloudstack.storage.feign.model;
+import java.util.List;
+import java.util.Objects;
+
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
-import java.util.List;
-import java.util.Objects;
-
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_NULL)
public class IpInterface {
@@ -44,6 +44,15 @@ public class IpInterface {
@JsonProperty("services")
private List services;
+ @JsonProperty("state")
+ private String state;
+
+ @JsonProperty("enabled")
+ private Boolean enabled;
+
+ @JsonProperty("location")
+ private Location location;
+
// Getters and setters
public String getUuid() {
return uuid;
@@ -85,6 +94,30 @@ public void setServices(List services) {
this.services = services;
}
+ public String getState() {
+ return state;
+ }
+
+ public void setState(String state) {
+ this.state = state;
+ }
+
+ public Boolean getEnabled() {
+ return enabled;
+ }
+
+ public void setEnabled(Boolean enabled) {
+ this.enabled = enabled;
+ }
+
+ public Location getLocation() {
+ return location;
+ }
+
+ public void setLocation(Location location) {
+ this.location = location;
+ }
+
@Override
public boolean equals(Object o) {
if (this == o) {
@@ -98,12 +131,14 @@ public boolean equals(Object o) {
Objects.equals(name, that.name) &&
Objects.equals(ip, that.ip) &&
Objects.equals(svm, that.svm) &&
- Objects.equals(services, that.services);
+ Objects.equals(services, that.services) &&
+ Objects.equals(state, that.state) &&
+ Objects.equals(enabled, that.enabled);
}
@Override
public int hashCode() {
- return Objects.hash(uuid, name, ip, svm, services);
+ return Objects.hash(uuid, name, ip, svm, services, state, enabled);
}
@Override
@@ -114,9 +149,52 @@ public String toString() {
", ip=" + ip +
", svm=" + svm +
", services=" + services +
+ ", state='" + state + '\'' +
+ ", enabled=" + enabled +
'}';
}
+ @JsonIgnoreProperties(ignoreUnknown = true)
+ @JsonInclude(JsonInclude.Include.NON_NULL)
+ public static class Node {
+ @JsonProperty("name")
+ private String name;
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
+ }
+
+ @JsonIgnoreProperties(ignoreUnknown = true)
+ @JsonInclude(JsonInclude.Include.NON_NULL)
+ public static class Location {
+ @JsonProperty("home_node")
+ private Node homeNode;
+
+ @JsonProperty("node")
+ private Node node;
+
+ public Node getHomeNode() {
+ return homeNode;
+ }
+
+ public void setHomeNode(Node homeNode) {
+ this.homeNode = homeNode;
+ }
+
+ public Node getNode() {
+ return node;
+ }
+
+ public void setNode(Node node) {
+ this.node = node;
+ }
+ }
+
// Nested class for IP information
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_NULL)
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java
index 32127b010572..dfd6552d1f64 100755
--- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java
+++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java
@@ -19,20 +19,15 @@
package org.apache.cloudstack.storage.lifecycle;
-import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
-import com.cloud.agent.api.StoragePoolInfo;
-import com.cloud.dc.ClusterVO;
-import com.cloud.dc.dao.ClusterDao;
-import com.cloud.exception.InvalidParameterValueException;
-import com.cloud.host.HostVO;
-import com.cloud.hypervisor.Hypervisor;
-import com.cloud.resource.ResourceManager;
-import com.cloud.storage.Storage;
-import com.cloud.storage.StorageManager;
-import com.cloud.storage.StoragePool;
-import com.cloud.storage.StoragePoolAutomation;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.google.common.base.Preconditions;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+
+import javax.inject.Inject;
+
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
@@ -40,10 +35,11 @@
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreLifeCycle;
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreParameters;
+import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDetailsDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.datastore.lifecycle.BasePrimaryDataStoreLifeCycleImpl;
import org.apache.cloudstack.storage.feign.model.OntapStorage;
@@ -59,13 +55,21 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
-import javax.inject.Inject;
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.UUID;
+import com.cloud.agent.api.StoragePoolInfo;
+import com.cloud.alert.AlertManager;
+import com.cloud.dc.ClusterVO;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.host.HostVO;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.resource.ResourceManager;
+import com.cloud.storage.Storage;
+import com.cloud.storage.StorageManager;
+import com.cloud.storage.StoragePool;
+import com.cloud.storage.StoragePoolAutomation;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.google.common.base.Preconditions;
public class OntapPrimaryDatastoreLifecycle extends BasePrimaryDataStoreLifeCycleImpl implements PrimaryDataStoreLifeCycle {
@Inject private ClusterDao _clusterDao;
@@ -76,6 +80,7 @@ public class OntapPrimaryDatastoreLifecycle extends BasePrimaryDataStoreLifeCycl
@Inject private StoragePoolAutomation _storagePoolAutomation;
@Inject private PrimaryDataStoreDao storagePoolDao;
@Inject private StoragePoolDetailsDao storagePoolDetailsDao;
+ @Inject private AlertManager _alertMgr;
private static final Logger logger = LogManager.getLogger(OntapPrimaryDatastoreLifecycle.class);
private static final long ONTAP_MIN_VOLUME_SIZE_IN_BYTES = 1677721600L;
@@ -135,13 +140,6 @@ public DataStore initialize(Map dsInfos) {
StorageStrategy storageStrategy = StorageProviderFactory.getStrategy(ontapStorage);
boolean isValid = storageStrategy.connect();
if (isValid) {
- // Get the DataLIF for data access
- String dataLIF = storageStrategy.getNetworkInterface();
- if (dataLIF == null || dataLIF.isEmpty()) {
- throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP, cannot create primary storage");
- }
- logger.info("Using Data LIF for storage access: " + dataLIF);
- details.put(OntapStorageConstants.DATA_LIF, dataLIF);
if (storageStrategy.getResolvedSvmUuid() != null && !storageStrategy.getResolvedSvmUuid().isEmpty()) {
details.put(OntapStorageConstants.SVM_UUID, storageStrategy.getResolvedSvmUuid());
}
@@ -160,6 +158,15 @@ public DataStore initialize(Map dsInfos) {
logger.error("Exception occurred while creating ONTAP volume: " + storagePoolName, e);
throw new CloudRuntimeException("Failed to create ONTAP volume: " + storagePoolName + ". Error: " + e.getMessage(), e);
}
+
+ Pair lifResult;
+ try {
+ lifResult = storageStrategy.getNetworkInterface();
+ } catch (Exception e) {
+ logger.error("Exception occurred while retrieving network interface for pool: " + storagePoolName, e);
+ throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP: " + e.getMessage(), e);
+ }
+ processDataLifSelection(lifResult, details, storagePoolName, zoneId, podId);
} else {
throw new CloudRuntimeException("ONTAP details validation failed, cannot create primary storage");
}
@@ -275,6 +282,26 @@ private long validateInitializeInputs(Long capacityBytes, Long podId, Long clust
return capacityBytes;
}
+ private void processDataLifSelection(Pair lifResult, Map details,
+ String storagePoolName, Long zoneId, Long podId) {
+ String dataLIF = lifResult.first();
+ if (dataLIF == null || dataLIF.isEmpty()) {
+ throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP, cannot create primary storage");
+ }
+ logger.info("Using Data LIF for storage access: " + dataLIF);
+ details.put(OntapStorageConstants.DATA_LIF, dataLIF);
+
+ // Persist LIF warning as a pool detail and fire a storage alert so the user is informed
+ if (lifResult.second() != null) {
+ String lifWarning = lifResult.second();
+ details.put(OntapStorageConstants.LIF_WARNING, lifWarning);
+ logger.warn("LIF selection warning for pool '" + storagePoolName + "': " + lifWarning);
+ String alertSubject = "ONTAP Storage Pool '" + storagePoolName + "': "
+ + lifWarning.split(OntapStorageConstants.SEMICOLON)[0].trim();
+ OntapStorageUtils.sendStorageAlert(_alertMgr, zoneId, podId, alertSubject, lifWarning);
+ }
+ }
+
@Override
public boolean attachCluster(DataStore dataStore, ClusterScope scope) {
logger.debug("In attachCluster for ONTAP primary storage");
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java
index 993e2d182804..f750e2904023 100644
--- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java
+++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java
@@ -177,10 +177,6 @@ private void updateNfsExportPolicyForConnectedHostIfNeeded(long poolId, long hos
return;
}
- if (host == null) {
- throw new CloudRuntimeException("Host was not found with id: " + hostId);
- }
-
if (!isNfs3EnabledOnHost(host)) {
throw new CloudRuntimeException("NFS protocol is not enabled on host with id: " + hostId);
}
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java
index 600ff87fe49d..0fba42b3fb7f 100644
--- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java
+++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java
@@ -51,10 +51,7 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
+import com.cloud.utils.Pair;
import com.cloud.utils.exception.CloudRuntimeException;
import feign.FeignException;
@@ -80,6 +77,12 @@ public abstract class StorageStrategy {
protected OntapStorage storage;
+ /**
+ * Holds the node name of the aggregate chosen during createStorageVolume().
+ * Used by getNetworkInterface() to prefer a LIF homed on the same node.
+ */
+ private String chosenAggregateNode;
+
/**
* Presents aggregate object for the unified storage, not eligible for disaggregated
*/
@@ -189,7 +192,10 @@ private void validateAndSelectAggregatesForVolumeCreation(String authHeader, Str
}
for (Aggregate aggr : aggrs) {
logger.debug("Found aggregate: " + aggr.getName() + " with UUID: " + aggr.getUuid());
- Aggregate aggrResp = aggregateFeignClient.getAggregateByUUID(authHeader, aggr.getUuid());
+ Aggregate aggrResp = aggregateFeignClient.getAggregateByUUID(authHeader, aggr.getUuid(),
+ Map.of(OntapStorageConstants.FIELDS, OntapStorageConstants.AGGREGATE_NODE
+ + OntapStorageConstants.COMMA + OntapStorageConstants.AGGREGATE_SPACE
+ + OntapStorageConstants.COMMA + OntapStorageConstants.STATE));
if (aggrResp == null) {
logger.warn("Aggregate details response is null for aggregate " + aggr.getName() + ". Skipping.");
continue;
@@ -204,7 +210,6 @@ private void validateAndSelectAggregatesForVolumeCreation(String authHeader, Str
}
logger.info("Selected aggregate: " + aggr.getName() + " for volume operations.");
this.aggregates = List.of(aggr);
- break;
}
if (this.aggregates == null || this.aggregates.isEmpty()) {
logger.error("No suitable aggregates found on SVM " + svmName + " for volume creation.");
@@ -226,6 +231,8 @@ private void validateAndSelectAggregatesForVolumeCreation(String authHeader, Str
public Volume createStorageVolume(String volumeName, Long size) {
logger.info("Creating volume: " + volumeName + " of size: " + size + " bytes");
+ this.chosenAggregateNode = null;
+
String svmName = storage.getSvmName();
if (aggregates == null || aggregates.isEmpty()) {
logger.error("No aggregates available to create volume on SVM " + svmName);
@@ -252,7 +259,10 @@ public Volume createStorageVolume(String volumeName, Long size) {
Aggregate aggrChosen = null;
for (Aggregate aggr : aggregates) {
logger.debug("Found aggregate: " + aggr.getName() + " with UUID: " + aggr.getUuid());
- Aggregate aggrResp = aggregateFeignClient.getAggregateByUUID(authHeader, aggr.getUuid());
+ Aggregate aggrResp = aggregateFeignClient.getAggregateByUUID(authHeader, aggr.getUuid(),
+ Map.of(OntapStorageConstants.FIELDS, OntapStorageConstants.AGGREGATE_NODE
+ + OntapStorageConstants.COMMA + OntapStorageConstants.AGGREGATE_SPACE
+ + OntapStorageConstants.COMMA + OntapStorageConstants.STATE));
if (aggrResp == null) {
logger.warn("Aggregate details response is null for aggregate " + aggr.getName() + ". Skipping.");
@@ -280,7 +290,7 @@ public Volume createStorageVolume(String volumeName, Long size) {
if (availableBytes > maxAvailableAggregateSpaceBytes) {
maxAvailableAggregateSpaceBytes = availableBytes;
- aggrChosen = aggr;
+ aggrChosen = aggrResp;
}
}
@@ -290,6 +300,8 @@ public Volume createStorageVolume(String volumeName, Long size) {
}
logger.info("Selected aggregate: " + aggrChosen.getName() + " for volume operations.");
+ this.chosenAggregateNode = aggrChosen.getNode() != null ? aggrChosen.getNode().getName() : null;
+
Aggregate aggr = new Aggregate();
aggr.setName(aggrChosen.getName());
aggr.setUuid(aggrChosen.getUuid());
@@ -467,12 +479,20 @@ public String getStoragePath() {
/**
- * Get the network ip interface
+ * Selects the best available data LIF for storage I/O, preferring one homed on the same node
+ * as the chosen aggregate to avoid inter-node traffic.
*
- * @return the network interface ip as a String
+ * Selection order:
+ *
+ * - LIF whose {@code location.home_node} matches the chosen aggregate's node — no warning
+ * - LIF currently running on that node (e.g. after failover) — returned with a warning
+ * - Any UP and enabled LIF — returned with a warning when aggregate node is known
+ *
+ *
+ * @return {@link Pair} where {@code first()} is the LIF's IP address and {@code second()} is
+ * a warning message (null when no warning)
*/
-
- public String getNetworkInterface() {
+ public Pair getNetworkInterface() {
String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword());
try {
Map queryParams = new HashMap<>();
@@ -490,36 +510,90 @@ public String getNetworkInterface() {
throw new CloudRuntimeException("Unsupported protocol: " + storage.getProtocol());
}
}
- queryParams.put(OntapStorageConstants.FIELDS, OntapStorageConstants.IP_ADDRESS);
+ queryParams.put(OntapStorageConstants.FIELDS,
+ OntapStorageConstants.IP_ADDRESS + OntapStorageConstants.COMMA
+ + OntapStorageConstants.STATE + OntapStorageConstants.COMMA
+ + OntapStorageConstants.LIF_ENABLED + OntapStorageConstants.COMMA
+ + OntapStorageConstants.LIF_LOCATION_HOME_NODE + OntapStorageConstants.COMMA
+ + OntapStorageConstants.LIF_LOCATION_NODE);
queryParams.put(OntapStorageConstants.RETURN_RECORDS, OntapStorageConstants.TRUE);
OntapResponse response =
networkFeignClient.getNetworkIpInterfaces(authHeader, queryParams);
- if (response != null && response.getRecords() != null && !response.getRecords().isEmpty()) {
- IpInterface ipInterface = null;
- // For simplicity, return the first interface's name (Of IPv4 type for NFS3)
- if (storage.getProtocol() == ProtocolType.ISCSI) {
- ipInterface = response.getRecords().get(0);
- } else if (storage.getProtocol() == ProtocolType.NFS3) {
- for (IpInterface iface : response.getRecords()) {
- if (iface.getIp().getAddress().contains(".")) {
- ipInterface = iface;
- break;
+ if (response == null || response.getRecords() == null || response.getRecords().isEmpty()) {
+ throw new CloudRuntimeException("No network interfaces found for SVM " + storage.getSvmName() +
+ " for protocol " + storage.getProtocol());
+ }
+
+ IpInterface currentNodeInterface = null;
+ IpInterface fallbackInterface = null;
+
+ for (IpInterface iface : response.getRecords()) {
+ if (!Boolean.TRUE.equals(iface.getEnabled()) || !OntapStorageConstants.LIF_STATE_UP.equals(iface.getState())) {
+ continue;
+ }
+ if (!isIPv4Address(iface.getIp().getAddress())) {
+ continue;
+ }
+ if (chosenAggregateNode != null) {
+ // LIF is homed on the aggregate's node
+ String homeNode = iface.getLocation() != null && iface.getLocation().getHomeNode() != null
+ ? iface.getLocation().getHomeNode().getName() : null;
+ if (chosenAggregateNode.equals(homeNode)) {
+ return new Pair<>(iface.getIp().getAddress(), null);
+ }
+ // LIF has failed over and is currently running on the aggregate's node
+ // (home_node differs). Keep as a candidate; returned with a warning if no match is found earlier.
+ if (currentNodeInterface == null) {
+ String currentNode = iface.getLocation() != null && iface.getLocation().getNode() != null
+ ? iface.getLocation().getNode().getName() : null;
+ if (chosenAggregateNode.equals(currentNode)) {
+ currentNodeInterface = iface;
}
}
}
+ if (fallbackInterface == null) {
+ fallbackInterface = iface;
+ }
+ }
- logger.info("Retrieved network interface: " + ipInterface.getIp().getAddress());
- return ipInterface.getIp().getAddress();
- } else {
- throw new CloudRuntimeException("No network interfaces found for SVM " + storage.getSvmName() +
- " for protocol " + storage.getProtocol());
+ if (currentNodeInterface == null && fallbackInterface == null) {
+ throw new CloudRuntimeException("No operationally UP and enabled LIF found for SVM '"
+ + storage.getSvmName() + "' with protocol " + storage.getProtocol()
+ + " — all " + response.getRecords().size() + " LIF(s) are either administratively disabled or operationally down");
}
- } catch (FeignException.FeignClientException e) {
+
+ if (currentNodeInterface != null) {
+ String ip = currentNodeInterface.getIp().getAddress();
+ String warning = "No home-node LIF found for aggregate node '" + chosenAggregateNode
+ + "'; using LIF '" + ip + "' currently running on that node (home node LIF may be down).";
+ logger.warn(warning);
+ return new Pair<>(ip, warning);
+ }
+
+ String ip = fallbackInterface.getIp().getAddress();
+ if (chosenAggregateNode == null) {
+ return new Pair<>(ip, null);
+ }
+ String warning = "No operational LIF found on aggregate's home node '" + chosenAggregateNode
+ + "'; using fallback LIF '" + ip + "' on a different node."
+ + " I/O will traverse an inter-node path, increasing latency.";
+ logger.warn(warning);
+ return new Pair<>(ip, warning);
+ } catch (Exception e) {
logger.error("Exception while retrieving network interfaces: ", e);
throw new CloudRuntimeException("Failed to retrieve network interfaces: " + e.getMessage());
}
}
+ /**
+ * Returns true if the given IP address string is an IPv4 address.
+ * IPv6 addresses contain colons; IPv4 addresses do not.
+ * To extend LIF selection to support IPv6, update this method and its call site in getNetworkInterface().
+ */
+ private boolean isIPv4Address(String address) {
+ return address != null && !address.contains(":");
+ }
+
/**
* Method encapsulates the behavior based on the opted protocol in subclasses.
* it is going to mimic
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 da9f00331f90..5ef662dd8528 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
@@ -67,9 +67,16 @@ public class OntapStorageConstants {
public static final String INITIATORS = "initiators";
public static final String AGGREGATES = "aggregates";
public static final String STATE = "state";
+ public static final String AGGREGATE_NODE = "node";
+ public static final String AGGREGATE_SPACE = "space";
public static final String DATA_NFS = "data_nfs";
public static final String DATA_ISCSI = "data_iscsi";
public static final String IP_ADDRESS = "ip.address";
+ public static final String LIF_ENABLED = "enabled";
+ public static final String LIF_STATE_UP = "up";
+ public static final String LIF_LOCATION_HOME_NODE = "location.home_node.name";
+ public static final String LIF_LOCATION_NODE = "location.node.name";
+ public static final String LIF_WARNING = "ONTAP_LIF_WARNING";
public static final String SERVICES = "services";
public static final String RETURN_RECORDS = "return_records";
public static final String SVM = "svm";
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java
index 1ada832ea952..b8b390026186 100644
--- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java
+++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java
@@ -19,9 +19,10 @@
package org.apache.cloudstack.storage.utils;
-import com.cloud.exception.InvalidParameterValueException;
-import com.cloud.utils.StringUtils;
-import com.cloud.utils.exception.CloudRuntimeException;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+
+import feign.FeignException;
import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.feign.model.Lun;
@@ -36,8 +37,10 @@
import org.apache.logging.log4j.Logger;
import org.springframework.util.Base64Utils;
-import java.nio.charset.StandardCharsets;
-import java.util.Map;
+import com.cloud.alert.AlertManager;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.utils.StringUtils;
+import com.cloud.utils.exception.CloudRuntimeException;
public class OntapStorageUtils {
@@ -119,6 +122,12 @@ public static String getOSTypeFromHypervisor(String hypervisorType) {
}
}
+ public static void sendStorageAlert(AlertManager alertMgr, Long zoneId, Long podId, String subject, String body) {
+ if (alertMgr != null) {
+ alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_STORAGE_MISC, zoneId != null ? zoneId : 0L, podId, subject, body);
+ }
+ }
+
/**
* Returns a connected {@link StorageStrategy} for operations on an existing pool (snapshots,
* delete, revert, grant/revoke). Does not require aggregate free space for the full pool size.
diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java
index 751b864ecfcc..ed538de4a49c 100644
--- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java
+++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java
@@ -44,9 +44,11 @@
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
import org.apache.cloudstack.storage.service.model.AccessGroup;
import com.cloud.hypervisor.Hypervisor;
+import com.cloud.alert.AlertManager;
import java.util.Map;
import java.util.List;
import java.util.ArrayList;
+import com.cloud.utils.Pair;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
@@ -54,6 +56,7 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.withSettings;
+import static org.mockito.ArgumentMatchers.contains;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -92,6 +95,9 @@ public class OntapPrimaryDatastoreLifecycleTest {
@Mock
private PrimaryDataStoreDao storagePoolDao;
+ @Mock
+ private AlertManager _alertMgr;
+
// Mock object that implements both DataStore and PrimaryDataStoreInfo
// This is needed because attachCluster(DataStore) casts DataStore to PrimaryDataStoreInfo internally
private DataStore dataStore;
@@ -116,7 +122,7 @@ void setUp() {
when(_clusterDao.findById(1L)).thenReturn(clusterVO);
when(storageStrategy.connect()).thenReturn(true);
- when(storageStrategy.getNetworkInterface()).thenReturn("testNetworkInterface");
+ when(storageStrategy.getNetworkInterface()).thenReturn(new Pair<>("testNetworkInterface", null));
Volume volume = new Volume();
volume.setUuid("test-volume-uuid");
@@ -153,6 +159,7 @@ void setUp() {
poolDetails.put("svmName", "svm1");
poolDetails.put("protocol", "NFS3");
poolDetails.put("storageIP", "192.168.1.100");
+ when(zoneScope.getScopeId()).thenReturn(1L);
}
@Test
@@ -404,6 +411,235 @@ public void testInitialize_unexpectedDetailKey() {
assertTrue(ex.getMessage().contains("Unexpected ONTAP detail key in URL"));
}
+ @Test
+ public void testInitialize_dataLifWithWarning() {
+ // Test when getNetworkInterface returns a warning in the Pair's second value
+ // This exercises the processDataLifSelection path for non-null warning
+ HashMap detailsMap = new HashMap<>();
+ detailsMap.put(OntapStorageConstants.USERNAME, "testUser");
+ detailsMap.put(OntapStorageConstants.PASSWORD, "testPassword");
+ detailsMap.put(OntapStorageConstants.STORAGE_IP, "10.10.10.10");
+ detailsMap.put(OntapStorageConstants.SVM_NAME, "vs0");
+ detailsMap.put(OntapStorageConstants.PROTOCOL, "NFS3");
+
+ Map dsInfos = new HashMap<>();
+ dsInfos.put("zoneId", 1L);
+ dsInfos.put("podId", 1L);
+ dsInfos.put("clusterId", 1L);
+ dsInfos.put("name", "testStoragePool");
+ dsInfos.put("providerName", "testProvider");
+ dsInfos.put("capacityBytes", 200000L);
+ dsInfos.put("managed", true);
+ dsInfos.put("tags", "testTag");
+ dsInfos.put("isTagARule", false);
+ dsInfos.put("details", detailsMap);
+
+ String warningMessage = "LIF on node-b; expected on node-a;Details about LIF failover";
+ when(storageStrategy.getNetworkInterface()).thenReturn(new Pair<>("10.0.0.1", warningMessage));
+
+ try (MockedStatic storageProviderFactory = Mockito.mockStatic(StorageProviderFactory.class);
+ MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) {
+ storageProviderFactory.when(() -> StorageProviderFactory.getStrategy(any())).thenReturn(storageStrategy);
+ ontapPrimaryDatastoreLifecycle.initialize(dsInfos);
+
+ // Verify alert was sent with warning message
+ utilityMock.verify(() -> OntapStorageUtils.sendStorageAlert(eq(_alertMgr), eq(1L), eq(1L),
+ contains("LIF on node-b"), eq(warningMessage)), times(1));
+ }
+ }
+
+ @Test
+ public void testInitialize_nullDataLif() {
+ // Test when lifResult.first() returns null
+ HashMap detailsMap = new HashMap<>();
+ detailsMap.put(OntapStorageConstants.USERNAME, "testUser");
+ detailsMap.put(OntapStorageConstants.PASSWORD, "testPassword");
+ detailsMap.put(OntapStorageConstants.STORAGE_IP, "10.10.10.10");
+ detailsMap.put(OntapStorageConstants.SVM_NAME, "vs0");
+ detailsMap.put(OntapStorageConstants.PROTOCOL, "NFS3");
+
+ Map dsInfos = new HashMap<>();
+ dsInfos.put("zoneId", 1L);
+ dsInfos.put("podId", 1L);
+ dsInfos.put("clusterId", 1L);
+ dsInfos.put("name", "testStoragePool");
+ dsInfos.put("providerName", "testProvider");
+ dsInfos.put("capacityBytes", 200000L);
+ dsInfos.put("managed", true);
+ dsInfos.put("tags", "testTag");
+ dsInfos.put("isTagARule", false);
+ dsInfos.put("details", detailsMap);
+
+ when(storageStrategy.getNetworkInterface()).thenReturn(new Pair<>(null, null));
+
+ try (MockedStatic storageProviderFactory = Mockito.mockStatic(StorageProviderFactory.class)) {
+ storageProviderFactory.when(() -> StorageProviderFactory.getStrategy(any())).thenReturn(storageStrategy);
+ Exception ex = assertThrows(CloudRuntimeException.class, () -> ontapPrimaryDatastoreLifecycle.initialize(dsInfos));
+ assertTrue(ex.getMessage().contains("Failed to retrieve Data LIF from ONTAP, cannot create primary storage"));
+ }
+ }
+
+ @Test
+ public void testInitialize_emptyDataLif() {
+ // Test when lifResult.first() returns empty string
+ HashMap detailsMap = new HashMap<>();
+ detailsMap.put(OntapStorageConstants.USERNAME, "testUser");
+ detailsMap.put(OntapStorageConstants.PASSWORD, "testPassword");
+ detailsMap.put(OntapStorageConstants.STORAGE_IP, "10.10.10.10");
+ detailsMap.put(OntapStorageConstants.SVM_NAME, "vs0");
+ detailsMap.put(OntapStorageConstants.PROTOCOL, "NFS3");
+
+ Map dsInfos = new HashMap<>();
+ dsInfos.put("zoneId", 1L);
+ dsInfos.put("podId", 1L);
+ dsInfos.put("clusterId", 1L);
+ dsInfos.put("name", "testStoragePool");
+ dsInfos.put("providerName", "testProvider");
+ dsInfos.put("capacityBytes", 200000L);
+ dsInfos.put("managed", true);
+ dsInfos.put("tags", "testTag");
+ dsInfos.put("isTagARule", false);
+ dsInfos.put("details", detailsMap);
+
+ when(storageStrategy.getNetworkInterface()).thenReturn(new Pair<>("", null));
+
+ try (MockedStatic storageProviderFactory = Mockito.mockStatic(StorageProviderFactory.class)) {
+ storageProviderFactory.when(() -> StorageProviderFactory.getStrategy(any())).thenReturn(storageStrategy);
+ Exception ex = assertThrows(CloudRuntimeException.class, () -> ontapPrimaryDatastoreLifecycle.initialize(dsInfos));
+ assertTrue(ex.getMessage().contains("Failed to retrieve Data LIF from ONTAP, cannot create primary storage"));
+ }
+ }
+
+ @Test
+ public void testInitialize_getNetworkInterfaceException() {
+ // Test when getNetworkInterface throws an exception
+ HashMap detailsMap = new HashMap<>();
+ detailsMap.put(OntapStorageConstants.USERNAME, "testUser");
+ detailsMap.put(OntapStorageConstants.PASSWORD, "testPassword");
+ detailsMap.put(OntapStorageConstants.STORAGE_IP, "10.10.10.10");
+ detailsMap.put(OntapStorageConstants.SVM_NAME, "vs0");
+ detailsMap.put(OntapStorageConstants.PROTOCOL, "NFS3");
+
+ Map dsInfos = new HashMap<>();
+ dsInfos.put("zoneId", 1L);
+ dsInfos.put("podId", 1L);
+ dsInfos.put("clusterId", 1L);
+ dsInfos.put("name", "testStoragePool");
+ dsInfos.put("providerName", "testProvider");
+ dsInfos.put("capacityBytes", 200000L);
+ dsInfos.put("managed", true);
+ dsInfos.put("tags", "testTag");
+ dsInfos.put("isTagARule", false);
+ dsInfos.put("details", detailsMap);
+
+ when(storageStrategy.getNetworkInterface()).thenThrow(new RuntimeException("ONTAP API error"));
+
+ try (MockedStatic storageProviderFactory = Mockito.mockStatic(StorageProviderFactory.class)) {
+ storageProviderFactory.when(() -> StorageProviderFactory.getStrategy(any())).thenReturn(storageStrategy);
+ Exception ex = assertThrows(CloudRuntimeException.class, () -> ontapPrimaryDatastoreLifecycle.initialize(dsInfos));
+ assertTrue(ex.getMessage().contains("Failed to retrieve Data LIF from ONTAP"));
+ assertTrue(ex.getCause() != null && ex.getCause().getMessage().contains("ONTAP API error"));
+ }
+ }
+
+ @Test
+ public void testInitialize_volumeCreationFailure_nullVolume() {
+ // Test when createStorageVolume returns null
+ HashMap detailsMap = new HashMap<>();
+ detailsMap.put(OntapStorageConstants.USERNAME, "testUser");
+ detailsMap.put(OntapStorageConstants.PASSWORD, "testPassword");
+ detailsMap.put(OntapStorageConstants.STORAGE_IP, "10.10.10.10");
+ detailsMap.put(OntapStorageConstants.SVM_NAME, "vs0");
+ detailsMap.put(OntapStorageConstants.PROTOCOL, "NFS3");
+
+ Map dsInfos = new HashMap<>();
+ dsInfos.put("zoneId", 1L);
+ dsInfos.put("podId", 1L);
+ dsInfos.put("clusterId", 1L);
+ dsInfos.put("name", "testStoragePool");
+ dsInfos.put("providerName", "testProvider");
+ dsInfos.put("capacityBytes", 200000L);
+ dsInfos.put("managed", true);
+ dsInfos.put("tags", "testTag");
+ dsInfos.put("isTagARule", false);
+ dsInfos.put("details", detailsMap);
+
+ when(storageStrategy.createStorageVolume(any(), any())).thenReturn(null);
+
+ try (MockedStatic storageProviderFactory = Mockito.mockStatic(StorageProviderFactory.class)) {
+ storageProviderFactory.when(() -> StorageProviderFactory.getStrategy(any())).thenReturn(storageStrategy);
+ Exception ex = assertThrows(CloudRuntimeException.class, () -> ontapPrimaryDatastoreLifecycle.initialize(dsInfos));
+ assertTrue(ex.getMessage().contains("Failed to create ONTAP volume"));
+ }
+ }
+
+ @Test
+ public void testInitialize_volumeCreationException() {
+ // Test when createStorageVolume throws an exception
+ HashMap detailsMap = new HashMap<>();
+ detailsMap.put(OntapStorageConstants.USERNAME, "testUser");
+ detailsMap.put(OntapStorageConstants.PASSWORD, "testPassword");
+ detailsMap.put(OntapStorageConstants.STORAGE_IP, "10.10.10.10");
+ detailsMap.put(OntapStorageConstants.SVM_NAME, "vs0");
+ detailsMap.put(OntapStorageConstants.PROTOCOL, "NFS3");
+
+ Map dsInfos = new HashMap<>();
+ dsInfos.put("zoneId", 1L);
+ dsInfos.put("podId", 1L);
+ dsInfos.put("clusterId", 1L);
+ dsInfos.put("name", "testStoragePool");
+ dsInfos.put("providerName", "testProvider");
+ dsInfos.put("capacityBytes", 200000L);
+ dsInfos.put("managed", true);
+ dsInfos.put("tags", "testTag");
+ dsInfos.put("isTagARule", false);
+ dsInfos.put("details", detailsMap);
+
+ when(storageStrategy.createStorageVolume(any(), any())).thenThrow(new RuntimeException("Volume creation failed"));
+
+ try (MockedStatic storageProviderFactory = Mockito.mockStatic(StorageProviderFactory.class)) {
+ storageProviderFactory.when(() -> StorageProviderFactory.getStrategy(any())).thenReturn(storageStrategy);
+ Exception ex = assertThrows(CloudRuntimeException.class, () -> ontapPrimaryDatastoreLifecycle.initialize(dsInfos));
+ assertTrue(ex.getMessage().contains("Failed to create ONTAP volume"));
+ assertTrue(ex.getCause() != null && ex.getCause().getMessage().contains("Volume creation failed"));
+ }
+ }
+
+ @Test
+ public void testInitialize_positiveWithDetailAssertions() {
+ // Enhanced positive test that verifies DATA_LIF detail is persisted and host is set correctly
+ HashMap detailsMap = new HashMap<>();
+ detailsMap.put(OntapStorageConstants.USERNAME, "testUser");
+ detailsMap.put(OntapStorageConstants.PASSWORD, "testPassword");
+ detailsMap.put(OntapStorageConstants.STORAGE_IP, "10.10.10.10");
+ detailsMap.put(OntapStorageConstants.SVM_NAME, "vs0");
+ detailsMap.put(OntapStorageConstants.PROTOCOL, "NFS3");
+
+ Map dsInfos = new HashMap<>();
+ dsInfos.put("zoneId", 1L);
+ dsInfos.put("podId", 1L);
+ dsInfos.put("clusterId", 1L);
+ dsInfos.put("name", "testStoragePool");
+ dsInfos.put("providerName", "testProvider");
+ dsInfos.put("capacityBytes", 200000L);
+ dsInfos.put("managed", true);
+ dsInfos.put("tags", "testTag");
+ dsInfos.put("isTagARule", false);
+ dsInfos.put("details", detailsMap);
+
+ String expectedDataLif = "192.168.1.100";
+ when(storageStrategy.getNetworkInterface()).thenReturn(new Pair<>(expectedDataLif, null));
+ when(storageStrategy.getStoragePath()).thenReturn("/vol/testVolume");
+
+ try (MockedStatic storageProviderFactory = Mockito.mockStatic(StorageProviderFactory.class)) {
+ storageProviderFactory.when(() -> StorageProviderFactory.getStrategy(any())).thenReturn(storageStrategy);
+ ontapPrimaryDatastoreLifecycle.initialize(dsInfos);
+
+ // Verify that createPrimaryDataStore was called and host parameter contains the DATA_LIF
+ verify(_dataStoreHelper, times(1)).createPrimaryDataStore(any());
+ }
+ }
+
// ========== attachCluster Tests ==========
@Test
@@ -412,12 +648,12 @@ public void testAttachCluster_positive() throws Exception {
when(_resourceMgr.getEligibleUpAndEnabledHostsInClusterForStorageConnection(any()))
.thenReturn(mockHosts);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
- when(_dataStoreHelper.attachCluster(any(DataStore.class))).thenReturn(dataStore);
try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachCluster(any(DataStore.class))).thenReturn(dataStore);
// Mock successful host connections
when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong())).thenReturn(true);
@@ -446,12 +682,12 @@ public void testAttachCluster_withSingleHost() throws Exception {
when(_resourceMgr.getEligibleUpAndEnabledHostsInClusterForStorageConnection(any()))
.thenReturn(singleHost);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
- when(_dataStoreHelper.attachCluster(any(DataStore.class))).thenReturn(dataStore);
try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachCluster(any(DataStore.class))).thenReturn(dataStore);
when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong())).thenReturn(true);
// Execute
@@ -477,12 +713,12 @@ public void testAttachCluster_withMultipleHosts() throws Exception {
when(_resourceMgr.getEligibleUpAndEnabledHostsInClusterForStorageConnection(any()))
.thenReturn(mockHosts);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
- when(_dataStoreHelper.attachCluster(any(DataStore.class))).thenReturn(dataStore);
try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachCluster(any(DataStore.class))).thenReturn(dataStore);
when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong())).thenReturn(true);
// Execute
@@ -507,6 +743,7 @@ public void testAttachCluster_hostConnectionFailure() throws Exception {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachCluster(any(DataStore.class))).thenReturn(dataStore);
// Mock host connection failure for first host
when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong()))
@@ -533,12 +770,12 @@ public void testAttachCluster_emptyHostList() throws Exception {
when(_resourceMgr.getEligibleUpAndEnabledHostsInClusterForStorageConnection(any()))
.thenReturn(emptyHosts);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
- when(_dataStoreHelper.attachCluster(any(DataStore.class))).thenReturn(dataStore);
try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachCluster(any(DataStore.class))).thenReturn(dataStore);
// Execute
boolean result = ontapPrimaryDatastoreLifecycle.attachCluster(
@@ -562,6 +799,7 @@ public void testAttachCluster_secondHostConnectionFails() throws Exception {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachCluster(any(DataStore.class))).thenReturn(dataStore);
// Mock: first host succeeds, second host fails
when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong()))
@@ -585,12 +823,12 @@ public void testAttachCluster_createAccessGroupCalled() throws Exception {
when(_resourceMgr.getEligibleUpAndEnabledHostsInClusterForStorageConnection(any()))
.thenReturn(mockHosts);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
- when(_dataStoreHelper.attachCluster(any(DataStore.class))).thenReturn(dataStore);
try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachCluster(any(DataStore.class))).thenReturn(dataStore);
when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong())).thenReturn(true);
// Execute
@@ -608,7 +846,6 @@ public void testAttachCluster_createAccessGroupCalled() throws Exception {
@Test
public void testAttachZone_positive() throws Exception {
// Setup
- when(zoneScope.getScopeId()).thenReturn(1L);
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
.thenReturn(mockHosts);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
@@ -618,6 +855,7 @@ public void testAttachZone_positive() throws Exception {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore);
// Mock successful host connections
when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong())).thenReturn(true);
@@ -643,7 +881,6 @@ public void testAttachZone_withSingleHost() throws Exception {
List singleHost = new ArrayList<>();
singleHost.add(mockHosts.get(0));
- when(zoneScope.getScopeId()).thenReturn(1L);
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
.thenReturn(singleHost);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
@@ -653,6 +890,7 @@ public void testAttachZone_withSingleHost() throws Exception {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore);
when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong())).thenReturn(true);
// Execute
@@ -675,7 +913,6 @@ public void testAttachZone_withMultipleHosts() throws Exception {
host3.setClusterId(1L);
mockHosts.add(host3);
- when(zoneScope.getScopeId()).thenReturn(1L);
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
.thenReturn(mockHosts);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
@@ -685,6 +922,7 @@ public void testAttachZone_withMultipleHosts() throws Exception {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore);
when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong())).thenReturn(true);
// Execute
@@ -701,7 +939,6 @@ public void testAttachZone_withMultipleHosts() throws Exception {
@Test
public void testAttachZone_hostConnectionFailure() throws Exception {
// Setup
- when(zoneScope.getScopeId()).thenReturn(1L);
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
.thenReturn(mockHosts);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
@@ -710,6 +947,7 @@ public void testAttachZone_hostConnectionFailure() throws Exception {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore);
// Mock host connection failure for first host
when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong()))
@@ -733,7 +971,6 @@ public void testAttachZone_emptyHostList() throws Exception {
// Setup - no hosts in zone
List emptyHosts = new ArrayList<>();
- when(zoneScope.getScopeId()).thenReturn(1L);
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
.thenReturn(emptyHosts);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
@@ -743,6 +980,7 @@ public void testAttachZone_emptyHostList() throws Exception {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore);
// Execute
boolean result = ontapPrimaryDatastoreLifecycle.attachZone(
@@ -758,7 +996,6 @@ public void testAttachZone_emptyHostList() throws Exception {
@Test
public void testAttachZone_secondHostConnectionFails() throws Exception {
// Setup
- when(zoneScope.getScopeId()).thenReturn(1L);
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
.thenReturn(mockHosts);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
@@ -767,6 +1004,7 @@ public void testAttachZone_secondHostConnectionFails() throws Exception {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore);
// Mock: first host succeeds, second host fails
when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong()))
@@ -787,7 +1025,6 @@ public void testAttachZone_secondHostConnectionFails() throws Exception {
@Test
public void testAttachZone_createAccessGroupCalled() throws Exception {
// Setup
- when(zoneScope.getScopeId()).thenReturn(1L);
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
.thenReturn(mockHosts);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
@@ -797,6 +1034,7 @@ public void testAttachZone_createAccessGroupCalled() throws Exception {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
+ when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore);
when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong())).thenReturn(true);
// Execute
@@ -834,7 +1072,6 @@ public void testAttachZone_nullHypervisorThrowsException() {
@Test
public void testAttachZone_kvmHypervisorSetsAndUpdatesPool() throws Exception {
// KVM hypervisorType should be set on the pool and persisted via storagePoolDao.update
- when(zoneScope.getScopeId()).thenReturn(1L);
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
.thenReturn(mockHosts);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
@@ -843,7 +1080,6 @@ public void testAttachZone_kvmHypervisorSetsAndUpdatesPool() throws Exception {
try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) {
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
.thenReturn(storageStrategy);
- when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null);
when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong())).thenReturn(true);
boolean result = ontapPrimaryDatastoreLifecycle.attachZone(
diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java
index 070c352a7620..eb3bbff3cfa4 100644
--- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java
+++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java
@@ -19,7 +19,10 @@
package org.apache.cloudstack.storage.service;
import java.lang.reflect.Field;
+import java.nio.charset.Charset;
import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
@@ -56,8 +59,6 @@
import static org.mockito.ArgumentMatchers.eq;
import org.mockito.Mock;
import static org.mockito.Mockito.atLeastOnce;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -65,9 +66,11 @@
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;
+import com.cloud.utils.Pair;
import com.cloud.utils.exception.CloudRuntimeException;
import feign.FeignException;
+import feign.Request;
@ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.LENIENT)
@@ -222,13 +225,8 @@ public void testConnect_positive() {
svmResponse.setRecords(List.of(svm));
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
- Aggregate aggregateDetail = mock(Aggregate.class);
- when(aggregateDetail.getName()).thenReturn("aggr1");
- when(aggregateDetail.getUuid()).thenReturn("aggr-uuid-1");
- when(aggregateDetail.getState()).thenReturn(Aggregate.StateEnum.ONLINE);
- when(aggregateDetail.getSpace()).thenReturn(mock(Aggregate.AggregateSpace.class));
- when(aggregateDetail.getAvailableBlockStorageSpace()).thenReturn(10000000000.0);
- when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"))).thenReturn(aggregateDetail);
+ Aggregate aggregateDetail = buildAggregate("aggr1", "aggr-uuid-1", 10000000000.0);
+ when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap())).thenReturn(aggregateDetail);
// Execute
boolean result = storageStrategy.connect();
@@ -255,10 +253,47 @@ public void testConnect_operationsOnly_skipsAggregateValidation() {
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
+ // Aggregate is ONLINE but has far less free space than the configured pool size (5GB).
+ Aggregate aggregateDetail = buildAggregate("aggr1", "aggr-uuid-1", 1000000.0); // only 1MB free
+ when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap())).thenReturn(aggregateDetail);
+
+ // Execute & Verify - connect(false) should succeed regardless of available space.
+ boolean result = storageStrategy.connect(false);
+ assertTrue(result, "connect() should succeed for an online aggregate even when its free space is below the pool capacity");
+ }
+
+ @Test
+ public void testConnect_noOnlineAggregates() {
+ // Setup - aggregate assigned to the SVM exists but is not ONLINE
+ Svm svm = new Svm();
+ svm.setName("svm1");
+ svm.setState(OntapStorageConstants.RUNNING);
+ svm.setNfsEnabled(true);
+
+ Aggregate aggregate = new Aggregate();
+ aggregate.setName("aggr1");
+ aggregate.setUuid("aggr-uuid-1");
+ svm.setAggregates(List.of(aggregate));
+
+ OntapResponse svmResponse = new OntapResponse<>();
+ svmResponse.setRecords(List.of(svm));
+
+ when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
+
+ Aggregate aggregateDetail = new Aggregate();
+ aggregateDetail.setName("aggr1");
+ aggregateDetail.setUuid("aggr-uuid-1");
+ aggregateDetail.setState(null); // not online
+ when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap())).thenReturn(aggregateDetail);
+
+ // Execute & Verify
+ CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> storageStrategy.connect());
+ assertTrue(ex.getMessage().contains("No suitable aggregates found"));
boolean result = storageStrategy.connect(false);
assertTrue(result);
- verify(aggregateFeignClient, never()).getAggregateByUUID(anyString(), anyString());
+ // connect(true) called getAggregateByUUID once; connect(false) must not add more calls
+ verify(aggregateFeignClient, times(1)).getAggregateByUUID(anyString(), anyString(), anyMap());
}
@Test
@@ -376,8 +411,10 @@ public void testConnect_nullSvmResponse() {
@Test
public void testConnect_invalidCredentials() {
// Setup - ONTAP rejects the supplied username/password with HTTP 401 Unauthorized.
+ Map> emptyHeaders = Collections.emptyMap();
+ Request dummyReq = Request.create(Request.HttpMethod.GET, "http://test", emptyHeaders, (byte[]) null, (Charset) null);
when(svmFeignClient.getSvmResponse(anyMap(), anyString()))
- .thenThrow(mock(FeignException.Unauthorized.class));
+ .thenThrow(new FeignException.Unauthorized("Unauthorized", dummyReq, null));
// Execute & Verify - connect() must surface a clear "invalid credentials" error.
CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> storageStrategy.connect());
@@ -396,14 +433,8 @@ public void testCreateStorageVolume_positive() {
storageStrategy.connect();
// Setup aggregate details
- Aggregate aggregateDetail = mock(Aggregate.class);
- when(aggregateDetail.getName()).thenReturn("aggr1");
- when(aggregateDetail.getUuid()).thenReturn("aggr-uuid-1");
- when(aggregateDetail.getState()).thenReturn(Aggregate.StateEnum.ONLINE);
- when(aggregateDetail.getSpace()).thenReturn(mock(Aggregate.AggregateSpace.class)); // Mock non-null space
- when(aggregateDetail.getAvailableBlockStorageSpace()).thenReturn(10000000000.0);
-
- when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1")))
+ Aggregate aggregateDetail = buildAggregate("aggr1", "aggr-uuid-1", 10000000000.0);
+ when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap()))
.thenReturn(aggregateDetail);
// Setup job response
@@ -483,12 +514,12 @@ public void testCreateStorageVolume_aggregateNotOnline() {
setupSuccessfulConnect();
storageStrategy.connect();
- Aggregate aggregateDetail = mock(Aggregate.class);
- when(aggregateDetail.getName()).thenReturn("aggr1");
- when(aggregateDetail.getUuid()).thenReturn("aggr-uuid-1");
- when(aggregateDetail.getState()).thenReturn(null); // null state to simulate offline
+ Aggregate aggregateDetail = new Aggregate();
+ aggregateDetail.setName("aggr1");
+ aggregateDetail.setUuid("aggr-uuid-1");
+ aggregateDetail.setState(null); // null state to simulate offline
- when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1")))
+ when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap()))
.thenReturn(aggregateDetail);
// Execute & Verify
@@ -503,13 +534,9 @@ public void testCreateStorageVolume_insufficientSpace() {
setupSuccessfulConnect();
storageStrategy.connect();
- Aggregate aggregateDetail = mock(Aggregate.class);
- when(aggregateDetail.getName()).thenReturn("aggr1");
- when(aggregateDetail.getUuid()).thenReturn("aggr-uuid-1");
- when(aggregateDetail.getState()).thenReturn(Aggregate.StateEnum.ONLINE);
- when(aggregateDetail.getAvailableBlockStorageSpace()).thenReturn(1000000.0); // Only 1MB available
+ Aggregate aggregateDetail = buildAggregate("aggr1", "aggr-uuid-1", 1000000.0); // Only 1MB available
- when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1")))
+ when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap()))
.thenReturn(aggregateDetail);
// Execute & Verify
@@ -635,8 +662,10 @@ public void testDeleteStorageVolume_feignException() {
volume.setName("test-volume");
volume.setUuid("vol-uuid-1");
+ Map> emptyHeaders = Collections.emptyMap();
+ Request dummyReq = Request.create(Request.HttpMethod.DELETE, "http://test", emptyHeaders, (byte[]) null, (Charset) null);
when(volumeFeignClient.deleteVolume(anyString(), eq("vol-uuid-1")))
- .thenThrow(mock(FeignException.FeignClientException.class));
+ .thenThrow(new FeignException.FeignClientException(500, "error", dummyReq, null));
// Execute & Verify
Exception ex = assertThrows(CloudRuntimeException.class,
@@ -750,6 +779,8 @@ public void testGetNetworkInterface_nfs() {
IpInterface ipInterface = new IpInterface();
ipInterface.setIp(ipInfo);
+ ipInterface.setState(OntapStorageConstants.LIF_STATE_UP);
+ ipInterface.setEnabled(true);
OntapResponse interfaceResponse = new OntapResponse<>();
interfaceResponse.setRecords(List.of(ipInterface));
@@ -758,11 +789,12 @@ public void testGetNetworkInterface_nfs() {
.thenReturn(interfaceResponse);
// Execute
- String result = storageStrategy.getNetworkInterface();
+ Pair result = storageStrategy.getNetworkInterface();
// Verify
assertNotNull(result);
- assertEquals("192.168.1.50", result);
+ assertEquals("192.168.1.50", result.first());
+ assertTrue(result.second() == null, "Expect no warning when a suitable LIF is found");
verify(networkFeignClient, times(1)).getNetworkIpInterfaces(anyString(), anyMap());
}
@@ -780,6 +812,8 @@ public void testGetNetworkInterface_iscsi() {
IpInterface ipInterface = new IpInterface();
ipInterface.setIp(ipInfo);
+ ipInterface.setState(OntapStorageConstants.LIF_STATE_UP);
+ ipInterface.setEnabled(true);
OntapResponse interfaceResponse = new OntapResponse<>();
interfaceResponse.setRecords(List.of(ipInterface));
@@ -788,11 +822,84 @@ public void testGetNetworkInterface_iscsi() {
.thenReturn(interfaceResponse);
// Execute
- String result = storageStrategy.getNetworkInterface();
+ Pair result = storageStrategy.getNetworkInterface();
// Verify
assertNotNull(result);
- assertEquals("192.168.1.51", result);
+ assertEquals("192.168.1.51", result.first());
+ assertTrue(result.second() == null, "Expect no warning when a suitable LIF is found");
+ }
+
+ @Test
+ public void testGetNetworkInterface_nfs_lifDown() {
+ // LIF exists but is operationally down — should fail
+ IpInterface.IpInfo ipInfo = new IpInterface.IpInfo();
+ ipInfo.setAddress("192.168.1.50");
+
+ IpInterface ipInterface = new IpInterface();
+ ipInterface.setIp(ipInfo);
+ ipInterface.setState("down");
+ ipInterface.setEnabled(true);
+
+ OntapResponse interfaceResponse = new OntapResponse<>();
+ interfaceResponse.setRecords(List.of(ipInterface));
+
+ when(networkFeignClient.getNetworkIpInterfaces(anyString(), anyMap()))
+ .thenReturn(interfaceResponse);
+
+ Exception ex = assertThrows(CloudRuntimeException.class,
+ () -> storageStrategy.getNetworkInterface());
+ assertTrue(ex.getMessage().contains("operationally UP and enabled"));
+ }
+
+ @Test
+ public void testGetNetworkInterface_nfs_lifDisabled() {
+ // LIF exists but is administratively disabled — should fail
+ IpInterface.IpInfo ipInfo = new IpInterface.IpInfo();
+ ipInfo.setAddress("192.168.1.50");
+
+ IpInterface ipInterface = new IpInterface();
+ ipInterface.setIp(ipInfo);
+ ipInterface.setState(OntapStorageConstants.LIF_STATE_UP);
+ ipInterface.setEnabled(false);
+
+ OntapResponse interfaceResponse = new OntapResponse<>();
+ interfaceResponse.setRecords(List.of(ipInterface));
+
+ when(networkFeignClient.getNetworkIpInterfaces(anyString(), anyMap()))
+ .thenReturn(interfaceResponse);
+
+ Exception ex = assertThrows(CloudRuntimeException.class,
+ () -> storageStrategy.getNetworkInterface());
+ assertTrue(ex.getMessage().contains("operationally UP and enabled"));
+ }
+
+ @Test
+ public void testGetNetworkInterface_iscsi_lifDown() {
+ // iSCSI LIF exists but is operationally down — should fail
+ OntapStorage iscsiStorage = new OntapStorage("admin", "password", "192.168.1.100",
+ "svm1", null, ProtocolType.ISCSI);
+ storageStrategy = new TestableStorageStrategy(iscsiStorage,
+ aggregateFeignClient, volumeFeignClient, svmFeignClient,
+ jobFeignClient, networkFeignClient, sanFeignClient, snapshotFeignClient);
+
+ IpInterface.IpInfo ipInfo = new IpInterface.IpInfo();
+ ipInfo.setAddress("192.168.1.51");
+
+ IpInterface ipInterface = new IpInterface();
+ ipInterface.setIp(ipInfo);
+ ipInterface.setState("down");
+ ipInterface.setEnabled(true);
+
+ OntapResponse interfaceResponse = new OntapResponse<>();
+ interfaceResponse.setRecords(List.of(ipInterface));
+
+ when(networkFeignClient.getNetworkIpInterfaces(anyString(), anyMap()))
+ .thenReturn(interfaceResponse);
+
+ Exception ex = assertThrows(CloudRuntimeException.class,
+ () -> storageStrategy.getNetworkInterface());
+ assertTrue(ex.getMessage().contains("operationally UP and enabled"));
}
@Test
@@ -813,8 +920,10 @@ public void testGetNetworkInterface_noInterfaces() {
@Test
public void testGetNetworkInterface_feignException() {
// Setup
+ Map> emptyHeaders = Collections.emptyMap();
+ Request dummyReq = Request.create(Request.HttpMethod.GET, "http://test", emptyHeaders, (byte[]) null, (Charset) null);
when(networkFeignClient.getNetworkIpInterfaces(anyString(), anyMap()))
- .thenThrow(mock(FeignException.FeignClientException.class));
+ .thenThrow(new FeignException.FeignClientException(500, "error", dummyReq, null));
// Execute & Verify
Exception ex = assertThrows(CloudRuntimeException.class,
@@ -822,6 +931,111 @@ public void testGetNetworkInterface_feignException() {
assertTrue(ex.getMessage().contains("Failed to retrieve network interfaces"));
}
+ // ========== getNetworkInterface() Node-Affinity Tests ==========
+
+ /**
+ * Tier 1: LIF homed on the same node as the chosen aggregate — selected without warning.
+ */
+ @Test
+ public void testGetNetworkInterface_nfs_tier1_homeNodeMatch() {
+ injectChosenAggregateNode(storageStrategy, "node-a");
+
+ IpInterface lif = buildLif("10.0.0.1", OntapStorageConstants.LIF_STATE_UP, true, "node-a", "node-a");
+ when(networkFeignClient.getNetworkIpInterfaces(anyString(), anyMap()))
+ .thenReturn(wrapLifs(List.of(lif)));
+
+ Pair result = storageStrategy.getNetworkInterface();
+
+ assertEquals("10.0.0.1", result.first());
+ assertTrue(result.second() == null, "Tier 1 should produce no warning");
+ }
+
+ /**
+ * Tier 2: No home-node match, but another UP LIF is currently running on the target node (failover).
+ * The result should carry a warning.
+ */
+ @Test
+ public void testGetNetworkInterface_nfs_tier2_currentNodeMatch() {
+ injectChosenAggregateNode(storageStrategy, "node-a");
+
+ // home node = node-b, currently running on node-a after failover
+ IpInterface lif = buildLif("10.0.0.2", OntapStorageConstants.LIF_STATE_UP, true, "node-b", "node-a");
+ when(networkFeignClient.getNetworkIpInterfaces(anyString(), anyMap()))
+ .thenReturn(wrapLifs(List.of(lif)));
+
+ Pair result = storageStrategy.getNetworkInterface();
+
+ assertEquals("10.0.0.2", result.first());
+ assertTrue(result.second() != null, "Tier 2 should produce a warning");
+ assertTrue(result.second().contains("node-a"));
+ }
+
+ /**
+ * Tier 3 fallback: No LIF matches the target node in either home_node or current node.
+ * First UP/enabled LIF used; result carries a warning directing the user to create a
+ * LIF on the correct node.
+ */
+ @Test
+ public void testGetNetworkInterface_nfs_tier3_crossNodeFallback() {
+ injectChosenAggregateNode(storageStrategy, "node-a");
+
+ // Both home_node and current node are node-b — no affinity to node-a
+ IpInterface lif = buildLif("10.0.0.3", OntapStorageConstants.LIF_STATE_UP, true, "node-b", "node-b");
+ when(networkFeignClient.getNetworkIpInterfaces(anyString(), anyMap()))
+ .thenReturn(wrapLifs(List.of(lif)));
+
+ Pair result = storageStrategy.getNetworkInterface();
+
+ assertEquals("10.0.0.3", result.first());
+ assertTrue(result.second() != null, "Tier 3 fallback should produce a warning");
+ assertTrue(result.second().contains("node-a"),
+ "Warning should mention the expected node");
+ assertTrue(result.second().contains("10.0.0.3"),
+ "Warning should mention the fallback LIF IP");
+ }
+
+ /**
+ * When chosenAggregateNode is null (volume not yet created / no aggregate info),
+ * any UP/enabled LIF is returned without warning.
+ */
+ @Test
+ public void testGetNetworkInterface_nfs_noAggregateNode_noWarning() {
+ // chosenAggregateNode is null by default — no node affinity context
+ IpInterface lif = buildLif("10.0.0.4", OntapStorageConstants.LIF_STATE_UP, true, "node-a", "node-a");
+ when(networkFeignClient.getNetworkIpInterfaces(anyString(), anyMap()))
+ .thenReturn(wrapLifs(List.of(lif)));
+
+ Pair result = storageStrategy.getNetworkInterface();
+
+ assertEquals("10.0.0.4", result.first());
+ // With no chosenAggregateNode, tier 1/2 selection is skipped — result falls through to tier 3
+ // but since there's no "expected node" in the warning message (chosenAggregateNode is null),
+ // the message text will still contain "null" — we simply verify no exception is thrown and IP is correct.
+ // (Tier 3 warning is generated when chosenAggregateNode != null; here it is null so no warning)
+ assertTrue(result.second() == null, "No warning when chosenAggregateNode is null");
+ }
+
+ /**
+ * Tier-1 LIF is down; Tier-2 LIF matches the current node and should be selected with a warning.
+ */
+ @Test
+ public void testGetNetworkInterface_nfs_tier1Down_tier2Used() {
+ injectChosenAggregateNode(storageStrategy, "node-a");
+
+ // Tier 1 candidate: home_node = node-a but operationally DOWN
+ IpInterface lifDown = buildLif("10.0.0.5", "down", true, "node-a", "node-a");
+ // Tier 2 candidate: home_node = node-b, currently on node-a
+ IpInterface lifFailover = buildLif("10.0.0.6", OntapStorageConstants.LIF_STATE_UP, true, "node-b", "node-a");
+
+ when(networkFeignClient.getNetworkIpInterfaces(anyString(), anyMap()))
+ .thenReturn(wrapLifs(List.of(lifDown, lifFailover)));
+
+ Pair result = storageStrategy.getNetworkInterface();
+
+ assertEquals("10.0.0.6", result.first());
+ assertTrue(result.second() != null, "Should warn that the home-node LIF is not in use");
+ }
+
// ========== Helper Methods ==========
private void setupSuccessfulConnect() {
@@ -840,24 +1054,13 @@ private void setupSuccessfulConnect() {
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
- Aggregate aggregateDetail = mock(Aggregate.class);
- when(aggregateDetail.getName()).thenReturn("aggr1");
- when(aggregateDetail.getUuid()).thenReturn("aggr-uuid-1");
- when(aggregateDetail.getState()).thenReturn(Aggregate.StateEnum.ONLINE);
- when(aggregateDetail.getSpace()).thenReturn(mock(Aggregate.AggregateSpace.class));
- when(aggregateDetail.getAvailableBlockStorageSpace()).thenReturn(10000000000.0);
- when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"))).thenReturn(aggregateDetail);
+ Aggregate aggregateDetail = buildAggregate("aggr1", "aggr-uuid-1", 10000000000.0);
+ when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap())).thenReturn(aggregateDetail);
}
private void setupAggregateForVolumeCreation() {
- Aggregate aggregateDetail = mock(Aggregate.class);
- when(aggregateDetail.getName()).thenReturn("aggr1");
- when(aggregateDetail.getUuid()).thenReturn("aggr-uuid-1");
- when(aggregateDetail.getState()).thenReturn(Aggregate.StateEnum.ONLINE);
- when(aggregateDetail.getSpace()).thenReturn(mock(Aggregate.AggregateSpace.class)); // Mock non-null space
- when(aggregateDetail.getAvailableBlockStorageSpace()).thenReturn(10000000000.0);
-
- when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1")))
+ Aggregate aggregateDetail = buildAggregate("aggr1", "aggr-uuid-1", 10000000000.0);
+ when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap()))
.thenReturn(aggregateDetail);
}
@@ -888,6 +1091,78 @@ private void setupSuccessfulJobCreation() {
.thenReturn(volumeResponse);
}
+ /**
+ * Injects a value into the private {@code chosenAggregateNode} field of StorageStrategy
+ * so node-affinity tests can exercise all three selection tiers without having to drive
+ * the full {@code createStorageVolume()} flow.
+ */
+ private static void injectChosenAggregateNode(StorageStrategy strategy, String nodeName) {
+ try {
+ Field field = StorageStrategy.class.getDeclaredField("chosenAggregateNode");
+ field.setAccessible(true);
+ field.set(strategy, nodeName);
+ } catch (NoSuchFieldException | IllegalAccessException e) {
+ throw new RuntimeException("Failed to inject chosenAggregateNode", e);
+ }
+ }
+
+ /**
+ * Builds an {@link IpInterface} with all node-affinity fields populated.
+ *
+ * @param ip the LIF's IP address (IPv4 for NFS3 selection to work)
+ * @param state operational state (e.g. "up" or "down")
+ * @param enabled administrative state
+ * @param homeNode name of the node the LIF is homed to
+ * @param currentNode name of the node the LIF is currently running on
+ */
+ private static IpInterface buildLif(String ip, String state, boolean enabled,
+ String homeNode, String currentNode) {
+ IpInterface.IpInfo ipInfo = new IpInterface.IpInfo();
+ ipInfo.setAddress(ip);
+
+ IpInterface.Node homeNodeObj = new IpInterface.Node();
+ homeNodeObj.setName(homeNode);
+
+ IpInterface.Node currentNodeObj = new IpInterface.Node();
+ currentNodeObj.setName(currentNode);
+
+ IpInterface.Location location = new IpInterface.Location();
+ location.setHomeNode(homeNodeObj);
+ location.setNode(currentNodeObj);
+
+ IpInterface lif = new IpInterface();
+ lif.setIp(ipInfo);
+ lif.setState(state);
+ lif.setEnabled(enabled);
+ lif.setLocation(location);
+ return lif;
+ }
+
+ private static OntapResponse wrapLifs(List lifs) {
+ OntapResponse response = new OntapResponse<>();
+ response.setRecords(lifs);
+ return response;
+ }
+
+ /**
+ * Creates a real {@link Aggregate} with nested space information so tests can avoid
+ * {@code mock(Aggregate.class)} which fails on JDK 26+ due to Byte Buddy limitations.
+ */
+ private static Aggregate buildAggregate(String name, String uuid, double availableBytes) {
+ Aggregate.AggregateSpaceBlockStorage blockStorage = new Aggregate.AggregateSpaceBlockStorage();
+ blockStorage.setAvailable(availableBytes);
+
+ Aggregate.AggregateSpace space = new Aggregate.AggregateSpace();
+ space.setBlockStorage(blockStorage);
+
+ Aggregate agg = new Aggregate();
+ agg.setName(name);
+ agg.setUuid(uuid);
+ agg.setState(Aggregate.StateEnum.ONLINE);
+ agg.setSpace(space);
+ return agg;
+ }
+
// ========== pollJobIfPresent / executeCliSfsrRestore Tests ==========
@Test
From 9f3a4907b34e3a723f86232e8ae80be536408b2b Mon Sep 17 00:00:00 2001
From: Sathvika
Date: Wed, 22 Jul 2026 14:08:36 +0530
Subject: [PATCH 6/7] =?UTF-8?q?CSTACKEX-212:=20fix=20for=20snapshot=20fail?=
=?UTF-8?q?ure=20for=20attached=20cs=20volumes=20nfs=20an=E2=80=A6=20(#77)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
### Description
Fix snapshot failure for CloudStack volumes attached to running VMs on
ONTAP primary storage (both NFS3 and iSCSI protocols).
This PR...
When a volume was created and attached to a running VM in a single
step,by enabling create on storage and choose the storage pool tag the
volume format was not being set correctly. The format is now determined
by the hypervisor type (KVM → QCOW2) in ontapdriver via
[getImageFormatByHypervisor(HypervisorType] mirroring the
[getSupportedImageFormatForCluster] in VolumeOrchestrator file.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] Build/CI
- [ ] Test (unit or integration test code)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [X] Major
- [ ] Minor
- [ ] Trivial
### How Has This Been Tested?
Tested on a dev setup against these scenarios:
Scenario A — Attach data disk to running VM, then snapshot
- Create ONTAP primary storage pool (NFS3 or iSCSI)
- Deploy VM with data disk using pool-tagged disk offering → VM reaches
Running state
- Create volume attached to the running VM by enabling create on storage
and choose the storage pool tag
- Take snapshot of attached volume — ✅ succeeds (was failing before fix)
Scenario B — Attach volume to root-disk-only VM, then snapshot
- Create ONTAP primary storage pool (NFS3 or iSCSI)
- Deploy VM without data disk → VM reaches Running state
- Create and attach volume to the running VM by enabling create on
storage and choose the storage pool tag
- Take snapshot of attached volume — ✅ succeeds (was failing before fix)
scenarios-C-create a volume on storage pool but not attach to any vm
- Create ONTAP primary storage pool (NFS3 or iSCSI)
- Create volume by enabling create on storage and choose the storage
pool tag
- Take snapshot- — ✅ succeeds (was failing before fix)
### Screenshots (if appropriate):
snapshots for when cs volume is attached to NFS and ISCSI instance that
has both root and data disk and create on storage enabled:
verifying on ontap and db:
snapshots for when cs volume is attached to NFS and ISCSI instance that
has only root and also case when they are not attached to any instance
and create on storage enabled:
#### How did you try to break this feature and the system with this
change?
---
.../storage/driver/OntapPrimaryDatastoreDriver.java | 9 +++++++++
.../storage/driver/OntapPrimaryDatastoreDriverTest.java | 5 +++++
2 files changed, 14 insertions(+)
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java
index 5e7a80b1af7c..738d31d6d4ee 100644
--- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java
+++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java
@@ -26,6 +26,7 @@
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.host.Host;
import com.cloud.host.HostVO;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.storage.Storage;
import com.cloud.storage.StoragePool;
import com.cloud.storage.Volume;
@@ -159,6 +160,8 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
volumeVO.setPoolType(storagePool.getPoolType());
volumeVO.setPoolId(storagePool.getId());
+ volumeVO.setFormat(getImageFormatByHypervisor(storagePool.getHypervisor()));
+ logger.info("createAsync: Volume format set to [{}] for hypervisor [{}]", volumeVO.getFormat(), storagePool.getHypervisor());
if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) {
String lunName = created != null && created.getLun() != null ? created.getLun().getName() : null;
@@ -1004,6 +1007,12 @@ private String buildSnapshotName(String cloudStackSnapshotName, long snapshotId)
}
+ private Storage.ImageFormat getImageFormatByHypervisor(HypervisorType hypervisorType) {
+ if (HypervisorType.KVM.equals(hypervisorType)) {
+ return Storage.ImageFormat.QCOW2;
+ }
+ throw new CloudRuntimeException("Unsupported hypervisor [" + hypervisorType + "] for ONTAP image format resolution");
+ }
/**
* Persists snapshot metadata in snapshot_details table.
*
diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java
index 571002df2a7f..bad8168ba86d 100644
--- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java
+++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java
@@ -21,6 +21,7 @@
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.host.Host;
import com.cloud.host.HostVO;
+import com.cloud.hypervisor.Hypervisor;
import com.cloud.storage.ScopeType;
import com.cloud.storage.Storage;
import com.cloud.storage.VolumeVO;
@@ -167,6 +168,7 @@ void testCreateAsync_VolumeWithISCSI_Success() {
when(storagePoolDao.findById(1L)).thenReturn(storagePool);
when(storagePool.getId()).thenReturn(1L);
when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem);
+ when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails);
when(volumeDao.findById(100L)).thenReturn(volumeVO);
@@ -202,6 +204,7 @@ void testCreateAsync_VolumeWithISCSI_Success() {
verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_UUID), eq("lun-uuid-123"), eq(false));
verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_NAME), eq("/vol/vol1/lun1"), eq(false));
+ verify(volumeVO).setFormat(Storage.ImageFormat.QCOW2);
verify(volumeDao).update(eq(100L), any(VolumeVO.class));
}
}
@@ -220,6 +223,7 @@ void testCreateAsync_VolumeWithNFS_Success() {
when(storagePoolDao.findById(1L)).thenReturn(storagePool);
when(storagePool.getId()).thenReturn(1L);
when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem);
+ when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM);
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails);
when(volumeDao.findById(100L)).thenReturn(volumeVO);
when(volumeVO.getId()).thenReturn(100L);
@@ -244,6 +248,7 @@ void testCreateAsync_VolumeWithNFS_Success() {
CreateCmdResult result = resultCaptor.getValue();
assertNotNull(result);
assertTrue(result.isSuccess());
+ verify(volumeVO).setFormat(Storage.ImageFormat.QCOW2);
verify(volumeDao).update(eq(100L), any(VolumeVO.class));
}
}
From 10e6f1c66ecbc8ddeb8e384ffbdca57b176692f2 Mon Sep 17 00:00:00 2001
From: Rajiv Jain
Date: Wed, 29 Jul 2026 13:00:58 +0530
Subject: [PATCH 7/7] =?UTF-8?q?CSTACKEX-158:=20if=20ontap=20snapshot=20are?=
=?UTF-8?q?=20already=20delete=20from=20ontap=20side,=20d=E2=80=A6=20(#82)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
…Deletion of CS side of snapshot should not fail on not finding ontap
snapshot.
This PR...
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] Build/CI
- [ ] Test (unit or integration test code)
- [ ] Major
- [x] Minor
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [x] Minor
- [ ] Trivial
Test -1: Ran VM snapshot delete operation when the respective snapshot
is not available at ONTAP, it passed.
Test -2: Ran VM snapshot delete operation when the respective snapshot
is available at ONTAP; it passed
Test -3: Ran cloudstack volume snapshot delete workflow when the
respective snapshot is not available at ONTAP, it passed.
Test -4: Ran cloudstack volume snapshot delete workflow when the
respective snapshot is available at ONTAP, it passed.
change?
---
.../driver/OntapPrimaryDatastoreDriver.java | 21 +---------
.../storage/service/StorageStrategy.java | 42 ++++++++++++-------
.../storage/utils/OntapStorageUtils.java | 28 +++++++++++++
.../storage/utils/OntapStorageUtilsTest.java | 14 +++++++
4 files changed, 71 insertions(+), 34 deletions(-)
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java
index 738d31d6d4ee..d6b7b089d6bf 100644
--- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java
+++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java
@@ -316,7 +316,7 @@ private void deleteCloudStackVolumeSnapshot(SnapshotInfo snapshotInfo, CommandRe
commandResult.setSuccess(true);
commandResult.setResult(null);
} catch (Exception e) {
- if (isSnapshotNotFoundError(e)) {
+ if (OntapStorageUtils.isOntapObjectNotFoundError(e)) {
logger.warn("deleteCloudStackVolumeSnapshot: ONTAP snapshot for CloudStack snapshot [{}] "
+ "already absent (idempotent success): {}", snapshotId, e.getMessage());
commandResult.setSuccess(true);
@@ -330,25 +330,6 @@ private void deleteCloudStackVolumeSnapshot(SnapshotInfo snapshotInfo, CommandRe
}
}
- /**
- * Returns true when the exception indicates the ONTAP snapshot was already removed.
- * Delete is idempotent: a missing backend snapshot is treated as success.
- */
- private boolean isSnapshotNotFoundError(Throwable error) {
- if (error == null) {
- return false;
- }
- String message = error.getMessage();
- if (message != null) {
- String lower = message.toLowerCase();
- if (lower.contains("404") || lower.contains("not found") || lower.contains("does not exist")
- || lower.contains("entry doesn't exist")) {
- return true;
- }
- }
- return isSnapshotNotFoundError(error.getCause());
- }
-
private long resolveSnapshotPoolId(String poolIdStr, long snapshotId) {
if (poolIdStr != null && !poolIdStr.isEmpty()) {
return Long.parseLong(poolIdStr);
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java
index 0fba42b3fb7f..ac142edf57ae 100644
--- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java
+++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java
@@ -889,21 +889,35 @@ public void deleteFlexVolSnapshotForCloudStackVolume(String flexVolUuid, String
logger.info("deleteFlexVolSnapshotForCloudStackVolume: issuing ONTAP REST delete for snapshot [{}] "
+ "(uuid={}) on FlexVol [{}]", snapshotName, snapshotUuid, flexVolUuid);
- JobResponse jobResponse = snapshotFeignClient.deleteSnapshot(getAuthHeader(), flexVolUuid, snapshotUuid);
-
- if (jobResponse == null || jobResponse.getJob() == null) {
- logger.debug("deleteFlexVolSnapshotForCloudStackVolume: no async job returned for snapshot [{}] "
- + "(uuid={}); treating HTTP success as completion", snapshotName, snapshotUuid);
- } else {
- logger.debug("deleteFlexVolSnapshotForCloudStackVolume: polling ONTAP delete job [{}] for snapshot [{}]",
- jobResponse.getJob().getUuid(), snapshotName);
- }
+ try {
+ JobResponse jobResponse = snapshotFeignClient.deleteSnapshot(getAuthHeader(), flexVolUuid, snapshotUuid);
+
+ if (jobResponse == null || jobResponse.getJob() == null) {
+ logger.debug("deleteFlexVolSnapshotForCloudStackVolume: no async job returned for snapshot [{}] "
+ + "(uuid={}); treating HTTP success as completion", snapshotName, snapshotUuid);
+ } else {
+ logger.debug("deleteFlexVolSnapshotForCloudStackVolume: polling ONTAP delete job [{}] for snapshot [{}]",
+ jobResponse.getJob().getUuid(), snapshotName);
+ }
- pollJobIfPresent(jobResponse, "delete FlexVol snapshot [" + snapshotName + "] uuid [" + snapshotUuid + "]",
- OntapStorageConstants.ONTAP_SNAPSHOT_DELETE_JOB_MAX_RETRIES,
- OntapStorageConstants.ONTAP_SNAPSHOT_DELETE_JOB_POLL_INTERVAL_MS);
+ pollJobIfPresent(jobResponse, "delete FlexVol snapshot [" + snapshotName + "] uuid [" + snapshotUuid + "]",
+ OntapStorageConstants.ONTAP_SNAPSHOT_DELETE_JOB_MAX_RETRIES,
+ OntapStorageConstants.ONTAP_SNAPSHOT_DELETE_JOB_POLL_INTERVAL_MS);
- logger.info("deleteFlexVolSnapshotForCloudStackVolume: ONTAP FlexVol snapshot [{}] (uuid={}) removed from [{}]",
- snapshotName, snapshotUuid, flexVolUuid);
+ logger.info("deleteFlexVolSnapshotForCloudStackVolume: ONTAP FlexVol snapshot [{}] (uuid={}) removed from [{}]",
+ snapshotName, snapshotUuid, flexVolUuid);
+ } catch (Exception e) {
+ if (OntapStorageUtils.isOntapObjectNotFoundError(e)) {
+ logger.warn("deleteFlexVolSnapshotForCloudStackVolume: ONTAP snapshot [{}] (uuid={}) on FlexVol [{}] "
+ + "already absent; treating delete as success: {}", snapshotName, snapshotUuid, flexVolUuid,
+ e.getMessage());
+ return;
+ }
+ if (e instanceof CloudRuntimeException) {
+ throw (CloudRuntimeException) e;
+ }
+ throw new CloudRuntimeException("Failed to delete ONTAP FlexVol snapshot [" + snapshotName + "]: "
+ + e.getMessage(), e);
+ }
}
}
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java
index b8b390026186..7f09b5584b40 100644
--- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java
+++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java
@@ -248,4 +248,32 @@ public static String extractUuidFromOntapJobDescription(String description, Stri
return remainder.isEmpty() ? null : remainder;
}
+ /**
+ * Returns true when the exception indicates the ONTAP Object was already removed.
+ * Delete workflows treat a missing backend object as idempotent success.
+ */
+ public static boolean isOntapObjectNotFoundError(Throwable error) {
+ if (error == null) {
+ return false;
+ }
+ if(error instanceof FeignException) {
+ FeignException feignException = (FeignException) error;
+ if (feignException.status() == 404) {
+ return true;
+ }
+ }
+ String message = error.getMessage();
+ if (message != null) {
+ String lower = message.toLowerCase();
+ if (lower.contains("404") || lower.contains("not found") || lower.contains("does not exist")
+ || lower.contains("entry doesn't exist")) {
+ return true;
+ }
+ } else {
+ logger.warn("Error message is null for exception: {}", error.getClass().getName());
+ return false;
+ }
+ return false;
+ }
+
}
diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java
index 372a75ad257d..ebe7da25ed12 100644
--- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java
+++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java
@@ -18,9 +18,11 @@
*/
package org.apache.cloudstack.storage.utils;
+import com.cloud.utils.exception.CloudRuntimeException;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class OntapStorageUtilsTest {
@@ -79,4 +81,16 @@ public void getIgroupName_truncates_whenOneCharOverMaxLength() {
assertEquals(OntapStorageConstants.IGROUP_NAME_MAX_LENGTH, result.length());
}
+
+ @Test
+ public void isOntapSnapshotNotFoundError_matchesEntryDoesNotExist() {
+ CloudRuntimeException ex = new CloudRuntimeException("Job failed with error: entry doesn't exist");
+ assertTrue(OntapStorageUtils.isOntapObjectNotFoundError(ex));
+ }
+
+ @Test
+ public void isOntapSnapshotNotFoundError_rejectsUnrelatedErrors() {
+ assertFalse(OntapStorageUtils.isOntapObjectNotFoundError(
+ new CloudRuntimeException("Job failed with error: permission denied")));
+ }
}