Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ public class AgentProperties{
*/
public static final Property<String> PRIVATE_NETWORK_DEVICE = new Property<>("private.network.device", "cloudbr1");

/**
* Private NIC IP address. If this property is commented or empty, it will be autodetected on service startup.<br>
* Data type: String.<br>
* Default value: <code>null</code>
*/
public static final Property<String> PRIVATE_NETWORK_DEVICE_ADDRESS = new Property<>("private.network.address", null, String.class);

/**
* Guest NIC device. If this property is commented, the value of the private NIC device will be used.<br>
* Data type: String.<br>
Expand Down
99 changes: 98 additions & 1 deletion core/src/main/java/com/cloud/resource/ServerResourceBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,20 @@
import java.io.File;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.net.DatagramSocket;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.NetworkInterface;
import java.net.SocketException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.LinkedList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import javax.naming.ConfigurationException;

Expand Down Expand Up @@ -75,6 +80,17 @@ public boolean configure(final String name, Map<String, Object> params) throws C

defineResourceNetworkInterfaces(params);

if (privateNic != null && !interfaceHasIpAddress(privateNic)) {
logger.info("Defined private NIC (private.network.device)) -> [{}] has no IP Address. Trying another methods.", privateNic);
privateNic = null;
}

if (privateNic == null) {
checkForPrivateInterfaceDefinedByIp(params);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this run when the bridge in private.network.device exists but has no ip? looks like it gets skipped, so the case in the description still fails

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Damans227
In my tests, private.network.device is never collected by params.get("private.network.device") even if its present on agent.properties.
On the hypervisor side, this setting works, because system vms are correctly placed on this device (that is a bridge linked to a VNI).
But as it is never resolved as private nic on the ServerResourceBase in this step, the original behavior is to jump into tryToAutoDiscoverResourcePrivateNetworkInterface() that picks the first local ip address regardless of the owner interface.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EduFrazao on kvm the bridge name comes straight from agent.properties, so a bridge that exists gets picked even with no ip. then the new lookups never run and the agent fails to start. can we also run them when the picked bridge has no usable ip?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Damans227 I've done another commit, checking for IP Address if private.network.device is found.
Is something like that you have in mind?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EduFrazao that covers it, thanks.

}
if (privateNic == null) {
tryToAutoDiscoverResourcePrivateNetworkInterfaceByRouteLookup(params);
}
if (privateNic == null) {
tryToAutoDiscoverResourcePrivateNetworkInterface();
}
Expand All @@ -90,6 +106,17 @@ public boolean configure(final String name, Map<String, Object> params) throws C
return true;
}

private boolean interfaceHasIpAddress(NetworkInterface iface) {
if (iface != null) {
String infos[] = NetUtils.getNetworkParams(privateNic);
String addr = infos != null && infos.length >= 1
? StringUtils.trimToNull(infos[0])
: null;
return addr != null;
}
return false;
}

protected void defineResourceNetworkInterfaces(Map<String, Object> params) {
String privateNic = (String) params.get("private.network.device");
privateNic = privateNic == null ? "xenbr0" : privateNic;
Expand All @@ -106,8 +133,78 @@ protected void defineResourceNetworkInterfaces(Map<String, Object> params) {
this.storageNic2 = NetUtils.getNetworkInterface(storageNic2);
}

private void checkForPrivateInterfaceDefinedByIp(Map<String, Object> params) {
final String ifAddr = StringUtils.trimToNull((String) params.get("private.network.address"));
if (ifAddr != null) {
logger.debug(String.format("Trying to use private address to resolve interface: [%s]", ifAddr));
try {
InetAddress rawAddr = InetAddress.getByAddress(InetAddress.getByName(ifAddr).getAddress());
final NetworkInterface nic = NetworkInterface.getByInetAddress(rawAddr);
if (nic != null) {
logger.info("Using NIC [{}] as private NIC. Source: InterfaceAddress [{}]", nic, ifAddr);
privateNic = nic;
} else {
logger.info("Unable to found private NIC with defined ip [{}]", ifAddr);
}
} catch (Exception e) {
// Logging only, if this method was unable to find a valid interface, iteration will be tested
logger.info("Unable to use private address to get the management interface: [{}]", e.getMessage());
}
}
}

private String extractHostAddress(String hostSpec) {
final String addr = hostSpec.contains("@") ? hostSpec.split("@")[0] : hostSpec;
return addr.trim();
}

private String[] collectMgmtHostIp(Map<String, Object> params) {
final String hosts = (String) params.get("host");
if (hosts != null && hosts.trim().length() > 0) {
logger.info(String.format("Parsing host setting: [%s]", hosts));
final Set<String> hostCollection = new LinkedHashSet<String>();
if (hosts.contains(",")) {
for(final String ip : hosts.split(",")) {
hostCollection.add(extractHostAddress(ip));
}
} else {
hostCollection.add(extractHostAddress(hosts));
}
return hostCollection.isEmpty() ? null : hostCollection.toArray(new String[0]);
}
return null;
}

private void tryToAutoDiscoverResourcePrivateNetworkInterfaceByRouteLookup(Map<String, Object> params) throws ConfigurationException {
logger.info("Trying to autodiscover this resource's private network interface by route lookup");
final String[] mgmtIps = collectMgmtHostIp(params);
if (mgmtIps == null) {
logger.info("Unable to resolve any management server ip address. Aborting private network search by route lookup.");
return;
}
for (String mgmtIp : mgmtIps) {
logger.info(String.format("Using management server IP [%s] to lookup", mgmtIp));
try {
try (DatagramSocket socket = new DatagramSocket()) {
socket.connect(new InetSocketAddress(mgmtIp, 8250));
// Asking for source address to mgmgt destination to O.S routing tables.
InetAddress localAddress = socket.getLocalAddress();
NetworkInterface nic = NetworkInterface.getByInetAddress(localAddress);
if (nic != null) {
logger.info(String.format("Using NIC [%s] as private NIC.", nic));
privateNic = nic;
break;
}
}
} catch (Throwable e) {
// Logging only, if this method was unable to find a valid interface, iteration will be tested
logger.debug(String.format("Unable to use routing table to determine private management interface: [%s]", e.getMessage()));
}
}
}

protected void tryToAutoDiscoverResourcePrivateNetworkInterface() throws ConfigurationException {
logger.info("Trying to autodiscover this resource's private network interface.");
logger.info("Trying to autodiscover this resource's private network interface by simple iteration.");

List<NetworkInterface> nics;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,8 @@ protected String getDefaultTungstenScriptsDir() {

@Override
public boolean configure(final String name, final Map<String, Object> params) throws ConfigurationException {
params.put(AgentProperties.HOST.getName(), AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HOST));
params.put(AgentProperties.PRIVATE_NETWORK_DEVICE_ADDRESS.getName(), AgentPropertiesFileHandler.getPropertyValue(AgentProperties.PRIVATE_NETWORK_DEVICE_ADDRESS));
boolean success = super.configure(name, params);
if (!success) {
return false;
Expand Down
Loading