winterhazel commented on code in PR #13897:
URL: https://github.com/apache/cloudstack/pull/13897#discussion_r4273929651
##########
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java:
##########
@@ -453,36 +513,93 @@ 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<IpInterface> 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 (iface == null || !Boolean.TRUE.equals(iface.getEnabled())
|| !OntapStorageConstants.LIF_STATE_UP.equals(iface.getState())) {
+ continue;
+ }
+ if (iface.getIp() == null || iface.getIp().getAddress() ==
null) {
+ 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());
}
}
Review Comment:
```suggestion
return selectDataLif(response.getRecords());
} catch (Exception e) {
logger.error("Exception while retrieving network interfaces: ",
e);
throw new CloudRuntimeException("Failed to retrieve network
interfaces: " + e.getMessage());
}
}
/**
* Classifies the eligible LIFs and selects the best one using the
node-affinity tiers documented in
* {@link #getNetworkInterface()}.
*/
private Pair<String, String> selectDataLif(List<IpInterface> records) {
IpInterface currentNodeInterface = null;
IpInterface fallbackInterface = null;
for (IpInterface iface : records) {
if (!isEligibleDataLif(iface)) {
continue;
}
// LIF is homed on the aggregate's node.
if (chosenAggregateNode != null &&
chosenAggregateNode.equals(lifHomeNode(iface))) {
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 tier 1 match is found.
if (currentNodeInterface == null &&
isRunningOnChosenNode(iface)) {
currentNodeInterface = iface;
}
if (fallbackInterface == null) {
fallbackInterface = iface;
}
}
return buildDataLifResult(records.size(), currentNodeInterface,
fallbackInterface);
}
private boolean isRunningOnChosenNode(IpInterface iface) {
return chosenAggregateNode != null &&
chosenAggregateNode.equals(lifCurrentNode(iface));
}
private boolean isEligibleDataLif(IpInterface iface) {
return iface != null
&& Boolean.TRUE.equals(iface.getEnabled())
&&
OntapStorageConstants.LIF_STATE_UP.equals(iface.getState())
&& iface.getIp() != null
&& iface.getIp().getAddress() != null
&& isIPv4Address(iface.getIp().getAddress());
}
private String lifHomeNode(IpInterface iface) {
return iface.getLocation() != null &&
iface.getLocation().getHomeNode() != null
? iface.getLocation().getHomeNode().getName() : null;
}
private String lifCurrentNode(IpInterface iface) {
return iface.getLocation() != null && iface.getLocation().getNode()
!= null
? iface.getLocation().getNode().getName() : null;
}
private Pair<String, String> buildDataLifResult(int totalRecords,
IpInterface currentNodeInterface,
IpInterface
fallbackInterface) {
if (currentNodeInterface == null && fallbackInterface == null) {
throw new CloudRuntimeException("No operationally UP and enabled
LIF found for SVM '"
+ storage.getSvmName() + "' with protocol " +
storage.getProtocol()
+ " — all " + totalRecords + " LIF(s) are either
administratively disabled or operationally down");
}
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);
}
```
This section could be extracted in a few methods to improve legibility. Here
is an alternative.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
