diff --git a/plugins/storage/volume/ontap/pom.xml b/plugins/storage/volume/ontap/pom.xml index 7af43a2325ff..f96ae2f55a27 100644 --- a/plugins/storage/volume/ontap/pom.xml +++ b/plugins/storage/volume/ontap/pom.xml @@ -33,7 +33,6 @@ 4.5.14 1.6.2 3.8.1 - 2.22.2 2.13.4 3.24.2 5.8.1 @@ -95,7 +94,7 @@ org.junit.jupiter junit-jupiter-engine - ${junit-jupiter.version} + ${cs.junit.jupiter.version} test @@ -121,7 +120,7 @@ org.assertj assertj-core - ${assertj.version} + ${cs.assertj.version} test @@ -151,7 +150,7 @@ maven-surefire-plugin - ${maven-surefire-plugin.version} + ${cs.surefire-plugin.version} **/*Test.java 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 c1cfb0a4c218..f0dc05f6e9c3 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,30 @@ 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); + } + 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); + } } else { throw new CloudRuntimeException("ONTAP details validation failed, cannot create primary storage"); } 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 d349305e5e43..c4c42369b732 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 @@ -19,13 +19,16 @@ package org.apache.cloudstack.storage.service; -import com.cloud.utils.exception.CloudRuntimeException; -import feign.FeignException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + import org.apache.cloudstack.storage.feign.FeignClientFactory; import org.apache.cloudstack.storage.feign.client.AggregateFeignClient; import org.apache.cloudstack.storage.feign.client.JobFeignClient; -import org.apache.cloudstack.storage.feign.client.NetworkFeignClient; import org.apache.cloudstack.storage.feign.client.NASFeignClient; +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; @@ -48,10 +51,10 @@ 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; /** * Storage Strategy represents the communication path for all the ONTAP storage options @@ -74,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 */ @@ -183,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; @@ -220,6 +232,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); @@ -246,7 +260,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."); @@ -274,7 +291,7 @@ public Volume createStorageVolume(String volumeName, Long size) { if (availableBytes > maxAvailableAggregateSpaceBytes) { maxAvailableAggregateSpaceBytes = availableBytes; - aggrChosen = aggr; + aggrChosen = aggrResp; } } @@ -284,6 +301,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()); @@ -457,12 +476,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:

+ *
    + *
  1. LIF whose {@code location.home_node} matches the chosen aggregate's node — no warning
    2. + *
  2. LIF currently running on that node (e.g. after failover) — returned with a warning
    3. + *
  3. Any UP and enabled LIF — returned with a warning when aggregate node is known
    4. + *
+ * + * @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<>(); @@ -480,36 +507,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..e132926dbc4b 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,9 @@ 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 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 +36,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 +121,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..2e4c281f87e0 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 @@ -47,6 +47,7 @@ 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; @@ -116,7 +117,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 +154,7 @@ void setUp() { poolDetails.put("svmName", "svm1"); poolDetails.put("protocol", "NFS3"); poolDetails.put("storageIP", "192.168.1.100"); + when(zoneScope.getScopeId()).thenReturn(1L); } @Test @@ -412,12 +414,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 +448,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 +479,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 +509,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 +536,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 +565,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 +589,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 +612,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 +621,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 +647,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 +656,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 +679,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 +688,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 +705,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 +713,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 +737,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 +746,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 +762,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 +770,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 +791,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 +800,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 +838,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 +846,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 6f09c3649757..81b7fb255d7c 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 @@ -18,8 +18,14 @@ */ package org.apache.cloudstack.storage.service; -import com.cloud.utils.exception.CloudRuntimeException; -import feign.FeignException; +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; + import org.apache.cloudstack.storage.feign.client.AggregateFeignClient; import org.apache.cloudstack.storage.feign.client.JobFeignClient; import org.apache.cloudstack.storage.feign.client.NetworkFeignClient; @@ -40,33 +46,31 @@ import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.cloudstack.storage.utils.OntapStorageConstants; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; -import org.mockito.junit.jupiter.MockitoSettings; -import org.mockito.quality.Strictness; - -import java.lang.reflect.Field; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; - import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anyString; 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; +import org.mockito.junit.jupiter.MockitoExtension; +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) @@ -221,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(); @@ -254,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() 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 + 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 @@ -375,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()); @@ -395,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 @@ -482,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 @@ -502,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 @@ -634,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, @@ -730,6 +760,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)); @@ -738,11 +770,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()); } @@ -760,6 +793,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)); @@ -768,11 +803,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 @@ -793,8 +901,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, @@ -802,6 +912,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() { @@ -820,24 +1035,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); } @@ -868,6 +1072,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