From dad3f70956102bf7fd3f1cabc0e2212df5bfa06c Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 30 Nov 2020 14:11:35 +0530 Subject: [PATCH 1/2] Fix failure in validating IP address in case of multiple Management Servers --- .../resource/NfsSecondaryStorageResource.java | 61 ++++++++++--------- 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java index ab98a812580f..d0fd4836b7f6 100644 --- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java @@ -46,6 +46,7 @@ import java.nio.file.Path; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -2674,36 +2675,40 @@ private void addRouteToInternalIpOrCidr(String localgw, String eth1ip, String et s_logger.debug("addRouteToInternalIp: destIp is null"); return; } - if (!NetUtils.isValidIp4(destIpOrCidr) && !NetUtils.isValidIp4Cidr(destIpOrCidr)) { - s_logger.warn(" destIp is not a valid ip address or cidr destIp=" + destIpOrCidr); - return; - } - boolean inSameSubnet = false; - if (NetUtils.isValidIp4(destIpOrCidr)) { - if (eth1ip != null && eth1mask != null) { - inSameSubnet = NetUtils.sameSubnet(eth1ip, destIpOrCidr, eth1mask); + List ips = new ArrayList<>(); + ips.addAll(Arrays.asList(destIpOrCidr.split(","))); + for (String ip : ips) { + if (!NetUtils.isValidIp4(ip) && !NetUtils.isValidIp4Cidr(ip)) { + s_logger.warn(" destIp is not a valid ip address or cidr destIp=" + ip); + return; + } + boolean inSameSubnet = false; + if (NetUtils.isValidIp4(ip)) { + if (eth1ip != null && eth1mask != null) { + inSameSubnet = NetUtils.sameSubnet(eth1ip, ip, eth1mask); + } else { + s_logger.warn("addRouteToInternalIp: unable to determine same subnet: _eth1ip=" + eth1ip + ", dest ip=" + ip + ", _eth1mask=" + eth1mask); + } } else { - s_logger.warn("addRouteToInternalIp: unable to determine same subnet: _eth1ip=" + eth1ip + ", dest ip=" + destIpOrCidr + ", _eth1mask=" + eth1mask); + inSameSubnet = NetUtils.isNetworkAWithinNetworkB(ip, NetUtils.ipAndNetMaskToCidr(eth1ip, eth1mask)); + } + if (inSameSubnet) { + s_logger.info("addRouteToInternalIp: dest ip " + ip + " is in the same subnet as eth1 ip " + eth1ip); + return; + } + Script command = new Script("/bin/bash", s_logger); + command.add("-c"); + command.add("ip route delete " + ip); + command.execute(); + command = new Script("/bin/bash", s_logger); + command.add("-c"); + command.add("ip route add " + ip + " via " + localgw); + String result = command.execute(); + if (result != null) { + s_logger.warn("Error in configuring route to internal ip err=" + result); + } else { + s_logger.info("addRouteToInternalIp: added route to internal ip=" + ip + " via " + localgw); } - } else { - inSameSubnet = NetUtils.isNetworkAWithinNetworkB(destIpOrCidr, NetUtils.ipAndNetMaskToCidr(eth1ip, eth1mask)); - } - if (inSameSubnet) { - s_logger.debug("addRouteToInternalIp: dest ip " + destIpOrCidr + " is in the same subnet as eth1 ip " + eth1ip); - return; - } - Script command = new Script("/bin/bash", s_logger); - command.add("-c"); - command.add("ip route delete " + destIpOrCidr); - command.execute(); - command = new Script("/bin/bash", s_logger); - command.add("-c"); - command.add("ip route add " + destIpOrCidr + " via " + localgw); - String result = command.execute(); - if (result != null) { - s_logger.warn("Error in configuring route to internal ip err=" + result); - } else { - s_logger.debug("addRouteToInternalIp: added route to internal ip=" + destIpOrCidr + " via " + localgw); } } From 5b165bc96809008b90df03232fd8f02d49c0acfb Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Tue, 1 Dec 2020 14:12:03 +0530 Subject: [PATCH 2/2] refactor code --- .../resource/NfsSecondaryStorageResource.java | 67 +++++++++---------- 1 file changed, 32 insertions(+), 35 deletions(-) diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java index d0fd4836b7f6..f4ae30e3e8f8 100644 --- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java @@ -46,7 +46,6 @@ import java.nio.file.Path; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -2588,8 +2587,10 @@ public boolean configure(String name, Map params) throws Configu if (_inSystemVM) { _localgw = (String)params.get("localgw"); if (_localgw != null) { // can only happen inside service vm - String mgmtHost = (String)params.get("host"); - addRouteToInternalIpOrCidr(_localgw, _eth1ip, _eth1mask, mgmtHost); + String mgmtHosts = (String)params.get("host"); + for (final String mgmtHost : mgmtHosts.split(",")) { + addRouteToInternalIpOrCidr(_localgw, _eth1ip, _eth1mask, mgmtHost); + } String internalDns1 = (String)params.get("internaldns1"); if (internalDns1 == null) { @@ -2675,40 +2676,36 @@ private void addRouteToInternalIpOrCidr(String localgw, String eth1ip, String et s_logger.debug("addRouteToInternalIp: destIp is null"); return; } - List ips = new ArrayList<>(); - ips.addAll(Arrays.asList(destIpOrCidr.split(","))); - for (String ip : ips) { - if (!NetUtils.isValidIp4(ip) && !NetUtils.isValidIp4Cidr(ip)) { - s_logger.warn(" destIp is not a valid ip address or cidr destIp=" + ip); - return; - } - boolean inSameSubnet = false; - if (NetUtils.isValidIp4(ip)) { - if (eth1ip != null && eth1mask != null) { - inSameSubnet = NetUtils.sameSubnet(eth1ip, ip, eth1mask); - } else { - s_logger.warn("addRouteToInternalIp: unable to determine same subnet: _eth1ip=" + eth1ip + ", dest ip=" + ip + ", _eth1mask=" + eth1mask); - } - } else { - inSameSubnet = NetUtils.isNetworkAWithinNetworkB(ip, NetUtils.ipAndNetMaskToCidr(eth1ip, eth1mask)); - } - if (inSameSubnet) { - s_logger.info("addRouteToInternalIp: dest ip " + ip + " is in the same subnet as eth1 ip " + eth1ip); - return; - } - Script command = new Script("/bin/bash", s_logger); - command.add("-c"); - command.add("ip route delete " + ip); - command.execute(); - command = new Script("/bin/bash", s_logger); - command.add("-c"); - command.add("ip route add " + ip + " via " + localgw); - String result = command.execute(); - if (result != null) { - s_logger.warn("Error in configuring route to internal ip err=" + result); + if (!NetUtils.isValidIp4(destIpOrCidr) && !NetUtils.isValidIp4Cidr(destIpOrCidr)) { + s_logger.warn(" destIp is not a valid ip address or cidr destIp=" + destIpOrCidr); + return; + } + boolean inSameSubnet = false; + if (NetUtils.isValidIp4(destIpOrCidr)) { + if (eth1ip != null && eth1mask != null) { + inSameSubnet = NetUtils.sameSubnet(eth1ip, destIpOrCidr, eth1mask); } else { - s_logger.info("addRouteToInternalIp: added route to internal ip=" + ip + " via " + localgw); + s_logger.warn("addRouteToInternalIp: unable to determine same subnet: _eth1ip=" + eth1ip + ", dest ip=" + destIpOrCidr + ", _eth1mask=" + eth1mask); } + } else { + inSameSubnet = NetUtils.isNetworkAWithinNetworkB(destIpOrCidr, NetUtils.ipAndNetMaskToCidr(eth1ip, eth1mask)); + } + if (inSameSubnet) { + s_logger.debug("addRouteToInternalIp: dest ip " + destIpOrCidr + " is in the same subnet as eth1 ip " + eth1ip); + return; + } + Script command = new Script("/bin/bash", s_logger); + command.add("-c"); + command.add("ip route delete " + destIpOrCidr); + command.execute(); + command = new Script("/bin/bash", s_logger); + command.add("-c"); + command.add("ip route add " + destIpOrCidr + " via " + localgw); + String result = command.execute(); + if (result != null) { + s_logger.warn("Error in configuring route to internal ip err=" + result); + } else { + s_logger.debug("addRouteToInternalIp: added route to internal ip=" + destIpOrCidr + " via " + localgw); } }