Skip to content

Commit fe6074b

Browse files
Addressed review comments
1 parent 009ea78 commit fe6074b

2 files changed

Lines changed: 21 additions & 14 deletions

File tree

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@
6868
import com.cloud.storage.StorageManager;
6969
import com.cloud.storage.StoragePool;
7070
import com.cloud.storage.StoragePoolAutomation;
71-
import com.cloud.utils.Pair;
7271
import com.cloud.utils.exception.CloudRuntimeException;
7372
import com.google.common.base.Preconditions;
7473

@@ -153,14 +152,16 @@ public DataStore initialize(Map<String, Object> dsInfos) {
153152
+ ". Error: " + e.getMessage(), e);
154153
}
155154

156-
Pair<String, String> lifResult;
155+
Map<String, String> lifResult;
157156
try {
158157
lifResult = storageStrategy.getNetworkInterface(aggregate);
159158
} catch (Exception e) {
160159
logger.error("Exception occurred while retrieving network interface for pool: " + storagePoolName, e);
161160
throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP: " + e.getMessage(), e);
162161
}
163-
processDataLifSelection(lifResult, details, storagePoolName, zoneId, podId);
162+
String dataLif = lifResult.get(OntapStorageConstants.DATA_LIF);
163+
String lifWarning = lifResult.get(OntapStorageConstants.LIF_WARNING);
164+
processDataLifSelection(dataLif, lifWarning, details, storagePoolName, zoneId, podId);
164165

165166
logger.info("Creating ONTAP volume '" + storagePoolName + "' with size: " + capacityBytes + " bytes (" +
166167
(capacityBytes / (1024 * 1024 * 1024)) + " GB)");
@@ -290,18 +291,16 @@ private void validateInitializeInputs(Long capacityBytes, Long podId, Long clust
290291
}
291292
}
292293

293-
private void processDataLifSelection(Pair<String, String> lifResult, Map<String, String> details,
294+
private void processDataLifSelection(String dataLIF, String lifWarning, Map<String, String> details,
294295
String storagePoolName, Long zoneId, Long podId) {
295-
String dataLIF = lifResult.first();
296296
if (dataLIF == null || dataLIF.isEmpty()) {
297297
throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP, cannot create primary storage");
298298
}
299299
logger.info("Using Data LIF for storage access: " + dataLIF);
300300
details.put(OntapStorageConstants.DATA_LIF, dataLIF);
301301

302302
// Persist LIF warning as a pool detail and fire a storage alert so the user is informed
303-
if (lifResult.second() != null) {
304-
String lifWarning = lifResult.second();
303+
if (lifWarning != null) {
305304
details.put(OntapStorageConstants.LIF_WARNING, lifWarning);
306305
logger.warn("LIF selection warning for pool '" + storagePoolName + "': " + lifWarning);
307306
String alertSubject = "ONTAP Storage Pool '" + storagePoolName + "': "

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@
5959
import org.apache.logging.log4j.LogManager;
6060
import org.apache.logging.log4j.Logger;
6161

62-
import com.cloud.utils.Pair;
6362
import com.cloud.utils.exception.CloudRuntimeException;
6463

6564
import feign.FeignException;
@@ -675,10 +674,10 @@ public String getStoragePath() {
675674
*
676675
* @param aggregate the aggregate previously selected via {@link #chooseAggregate(Long)};
677676
* must include a node name for LIF affinity
678-
* @return {@link Pair} where {@code first()} is the LIF's IP address and {@code second()} is
679-
* a warning message (null when no warning)
677+
* @return map with {@link OntapStorageConstants#DATA_LIF} set to the LIF IP address, and
678+
* optionally {@link OntapStorageConstants#LIF_WARNING} when a non-ideal LIF was selected
680679
*/
681-
public Pair<String, String> getNetworkInterface(Aggregate aggregate) {
680+
public Map<String, String> getNetworkInterface(Aggregate aggregate) {
682681
if (aggregate == null || aggregate.getNode() == null || aggregate.getNode().getName() == null
683682
|| aggregate.getNode().getName().isEmpty()) {
684683
throw new CloudRuntimeException("Aggregate with a node name is required to select a network interface");
@@ -729,7 +728,7 @@ public Pair<String, String> getNetworkInterface(Aggregate aggregate) {
729728
String homeNode = iface.getLocation() != null && iface.getLocation().getHomeNode() != null
730729
? iface.getLocation().getHomeNode().getName() : null;
731730
if (aggregateNode.equals(homeNode)) {
732-
return new Pair<>(iface.getIp().getAddress(), null);
731+
return networkInterfaceResult(iface.getIp().getAddress(), null);
733732
}
734733
// LIF has failed over and is currently running on the aggregate's node
735734
// (home_node differs). Keep as a candidate; returned with a warning if no match is found earlier.
@@ -756,15 +755,15 @@ public Pair<String, String> getNetworkInterface(Aggregate aggregate) {
756755
String warning = "No home-node LIF found for aggregate node '" + aggregateNode
757756
+ "'; using LIF '" + ip + "' currently running on that node (home node LIF may be down).";
758757
logger.warn(warning);
759-
return new Pair<>(ip, warning);
758+
return networkInterfaceResult(ip, warning);
760759
}
761760

762761
String ip = fallbackInterface.getIp().getAddress();
763762
String warning = "No operational LIF found on aggregate's home node '" + aggregateNode
764763
+ "'; using fallback LIF '" + ip + "' on a different node."
765764
+ " I/O will traverse an inter-node path, increasing latency.";
766765
logger.warn(warning);
767-
return new Pair<>(ip, warning);
766+
return networkInterfaceResult(ip, warning);
768767
} catch (CloudRuntimeException e) {
769768
throw e;
770769
} catch (Exception e) {
@@ -773,6 +772,15 @@ public Pair<String, String> getNetworkInterface(Aggregate aggregate) {
773772
}
774773
}
775774

775+
private Map<String, String> networkInterfaceResult(String address, String warning) {
776+
Map<String, String> result = new HashMap<>();
777+
result.put(OntapStorageConstants.DATA_LIF, address);
778+
if (warning != null) {
779+
result.put(OntapStorageConstants.LIF_WARNING, warning);
780+
}
781+
return result;
782+
}
783+
776784
/**
777785
* Returns true if the given IP address string is an IPv4 address.
778786
* IPv6 addresses contain colons; IPv4 addresses do not.

0 commit comments

Comments
 (0)