You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by dr...@apache.org on 2017/07/24 14:37:38 UTC

[1/6] brooklyn-server git commit: make BrooklynWebServer return usable address if bound to all NICs and NICs change

Repository: brooklyn-server
Updated Branches:
  refs/heads/master 29583d8c5 -> d85ffa5b4


make BrooklynWebServer return usable address if bound to all NICs and NICs change


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/810ce622
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/810ce622
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/810ce622

Branch: refs/heads/master
Commit: 810ce622df974d71ae08127f4cbd1abf60279f33
Parents: 3647d1f
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed Jul 19 10:37:56 2017 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Sat Jul 22 03:10:08 2017 +0100

----------------------------------------------------------------------
 .../util/core/BrooklynNetworkUtils.java         |  9 +++++++--
 .../brooklyn/launcher/BrooklynWebServer.java    | 20 ++++++++++++++++----
 2 files changed, 23 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/810ce622/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java b/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java
index b1cd49a..190ad75 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java
@@ -39,9 +39,14 @@ public class BrooklynNetworkUtils {
         return LocalhostExternalIpLoader.getLocalhostIpQuicklyOrDefault();
     }
 
-    /** returns a IP address for localhost paying attention to a system property to prevent lookup in some cases */
+    /** returns an IP address for localhost,
+     * paying attention to system property 
+     * {@link BrooklynServiceAttributes#LOCALHOST_IP_ADDRESS}
+     * if set to prevent default selection when needed,
+     * otherwise finding the first bindable/reachable NIC from a system lookup which usually
+     * prefers non-loopback devices (but use the system property if if needed) */
     public static InetAddress getLocalhostInetAddress() {
         return TypeCoercions.coerce(JavaGroovyEquivalents.elvis(BrooklynServiceAttributes.LOCALHOST_IP_ADDRESS.getValue(),
-                Networking.getReachableLocalHost()), InetAddress.class);
+                Networking.getLocalHost(true, false, true, 500)), InetAddress.class);
     }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/810ce622/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java
----------------------------------------------------------------------
diff --git a/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java b/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java
index b85e79d..42f1e7f 100644
--- a/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java
+++ b/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java
@@ -23,6 +23,7 @@ import java.io.InputStream;
 import java.net.BindException;
 import java.net.InetAddress;
 import java.net.URI;
+import java.net.UnknownHostException;
 import java.security.KeyPair;
 import java.security.KeyStore;
 import java.security.KeyStoreException;
@@ -294,10 +295,12 @@ public class BrooklynWebServer {
         return actualPort;
     }
 
-    /** interface/address where this server is listening;
+    /** interface/address where this server is or will be listening;
      * if bound to 0.0.0.0 (all NICs, e.g. because security is set) this will return one NIC where this is bound */
     public InetAddress getAddress() {
-        return actualAddress;
+        if (actualAddress!=null) return actualAddress;
+        if (!shouldBindToAll()) return bindAddress;
+        return BrooklynNetworkUtils.getLocalhostInetAddress();
     }
     
     /** URL for accessing this web server (root context) */
@@ -421,8 +424,8 @@ public class BrooklynWebServer {
         connector.setPort(actualPort);
         server.setConnectors(new Connector[]{connector});
 
-        if (bindAddress == null || bindAddress.equals(InetAddress.getByAddress(new byte[] { 0, 0, 0, 0 }))) {
-            actualAddress = BrooklynNetworkUtils.getLocalhostInetAddress();
+        if (shouldBindToAll()) {
+            actualAddress = null;
         } else {
             actualAddress = bindAddress;
         }
@@ -468,6 +471,15 @@ public class BrooklynWebServer {
         log.info("Started Brooklyn console at "+getRootUrl()+", running " + rootWar + (allWars!=null && !allWars.isEmpty() ? " and " + wars.values() : ""));
     }
 
+    private boolean shouldBindToAll() {
+        try {
+            return bindAddress == null || bindAddress.equals(InetAddress.getByAddress(new byte[] { 0, 0, 0, 0 }));
+        } catch (UnknownHostException e) {
+            // shouldn't happen
+            throw Exceptions.propagate(e);
+        }
+    }
+
     private WebAppContext deployRestApi(WebAppContext context) {
         ImmutableList.Builder<Object> providersListBuilder = ImmutableList.builder();
         providersListBuilder.add(


[5/6] brooklyn-server git commit: clarify reuse-addr behaviour and check all nics when starting web server

Posted by dr...@apache.org.
clarify reuse-addr behaviour and check all nics when starting web server


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/19e94e93
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/19e94e93
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/19e94e93

Branch: refs/heads/master
Commit: 19e94e93130677f613443d9d3f95dc5aed7429a8
Parents: 73a3372
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Mon Jul 24 02:31:27 2017 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Mon Jul 24 02:32:32 2017 +0100

----------------------------------------------------------------------
 .../brooklyn/launcher/BrooklynWebServer.java    | 17 +++---
 .../apache/brooklyn/util/net/Networking.java    |  5 +-
 .../brooklyn/util/net/NetworkingUtilsTest.java  | 62 +++++++++++++-------
 3 files changed, 56 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/19e94e93/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java
----------------------------------------------------------------------
diff --git a/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java b/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java
index 889f4b1..fc342c1 100644
--- a/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java
+++ b/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java
@@ -38,25 +38,25 @@ import java.util.Map;
 import javax.annotation.Nullable;
 import javax.security.auth.spi.LoginModule;
 
-import com.google.common.collect.ImmutableList;
-import org.apache.brooklyn.core.BrooklynFeatureEnablement;
-import org.apache.brooklyn.rest.NopSecurityHandler;
 import org.apache.brooklyn.api.location.PortRange;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.BrooklynFeatureEnablement;
 import org.apache.brooklyn.core.BrooklynVersion;
 import org.apache.brooklyn.core.internal.BrooklynInitialization;
 import org.apache.brooklyn.core.location.PortRanges;
+import org.apache.brooklyn.core.mgmt.ShutdownHandler;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
 import org.apache.brooklyn.core.server.BrooklynServerPaths;
 import org.apache.brooklyn.core.server.BrooklynServiceAttributes;
 import org.apache.brooklyn.launcher.config.CustomResourceLocator;
 import org.apache.brooklyn.location.localhost.LocalhostMachineProvisioningLocation;
 import org.apache.brooklyn.rest.BrooklynWebConfig;
+import org.apache.brooklyn.rest.NopSecurityHandler;
 import org.apache.brooklyn.rest.RestApiSetup;
+import org.apache.brooklyn.rest.filter.CorsImplSupplierFilter;
 import org.apache.brooklyn.rest.filter.CsrfTokenFilter;
 import org.apache.brooklyn.rest.filter.EntitlementContextFilter;
-import org.apache.brooklyn.rest.filter.CorsImplSupplierFilter;
 import org.apache.brooklyn.rest.filter.HaHotCheckResourceFilter;
 import org.apache.brooklyn.rest.filter.LoggingFilter;
 import org.apache.brooklyn.rest.filter.NoCacheFilter;
@@ -66,7 +66,6 @@ import org.apache.brooklyn.rest.security.jaas.BrooklynLoginModule;
 import org.apache.brooklyn.rest.security.jaas.BrooklynLoginModule.RolePrincipal;
 import org.apache.brooklyn.rest.security.jaas.JaasUtils;
 import org.apache.brooklyn.rest.util.ManagementContextProvider;
-import org.apache.brooklyn.core.mgmt.ShutdownHandler;
 import org.apache.brooklyn.rest.util.ShutdownHandlerProvider;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
@@ -80,6 +79,7 @@ import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.io.FileUtil;
 import org.apache.brooklyn.util.javalang.Threads;
 import org.apache.brooklyn.util.logging.LoggingSetup;
+import org.apache.brooklyn.util.net.Networking;
 import org.apache.brooklyn.util.os.Os;
 import org.apache.brooklyn.util.stream.Streams;
 import org.apache.brooklyn.util.text.Identifiers;
@@ -104,6 +104,7 @@ import org.slf4j.LoggerFactory;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Splitter;
 import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Maps;
 
 /**
@@ -386,8 +387,10 @@ public class BrooklynWebServer {
             if (portRange==null) {
                 portRange = getHttpsEnabled() ? httpsPort : httpPort;
             }
-            actualPort = LocalhostMachineProvisioningLocation.obtainPort(getAddress(), portRange,
-                // allow reuse-addr so we prefer to bind to lower-numbered ports even if they are in time-wait state
+            actualPort = LocalhostMachineProvisioningLocation.obtainPort( 
+                shouldBindToAll() ? Networking.ANY_NIC : getAddress(), portRange,
+                // allow reuse-addr so we prefer to bind to lower-numbered ports even if they are in time-wait state;
+                // also allows if specific NIC specified, we can bind to it even if something else is using 0.0.0.0
                 true);
             if (actualPort == -1) 
                 throw new IllegalStateException("Unable to provision port for web console (wanted "+portRange+")");

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/19e94e93/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java b/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java
index a02dea2..6919257 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java
@@ -107,7 +107,10 @@ public class Networking {
             //192.168.0.1:X which explains the above comment (nginx sets SO_REUSEADDR as well). Moreover there
             //is no TIME_WAIT for listening sockets without any connections so why enable it at all.
             //Alex - TIME_WAIT sticks around for a while (30s or so) if a java process dies while it has
-            //connections; if we want things to aggressively try to take a port, they should use reuse-addr
+            //connections; if we want things to aggressively try to take a port, they should use reuse-addr,
+            //but probably testing against ANY_NIC unless they want to trump things listening on all interfaces.
+            //(isPortAvailable(ANY_NIC) will say false if any NIC is using it, 
+            //but isPortAvailable(SPECIFIC) may say true even if there is an ANY_NIC binding, when reuseAddr is true)
             ServerSocket ss = null;
             DatagramSocket ds = null;
             try {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/19e94e93/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java
index c56ac66..7312f6b 100644
--- a/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java
+++ b/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java
@@ -171,34 +171,56 @@ public class NetworkingUtilsTest {
             fail("This test requires that at least some ports near "+port+"+ not be in use.");
     }
 
-    @Test
-    public void testIsPortAvailableReportsFalseWhenPortIsInUse() throws Exception {
-        int port = 58767;
-        ServerSocket ss = null;
-        do {
-            port ++;
+    private int findPort() {
+        for (int port = 58767; port < 60000; port++) {
             if (Networking.isPortAvailable(port)) {
-                try {
-                    ss = new ServerSocket(port);
-                    log.info("acquired port on "+port+" for test "+JavaClassNames.niceClassAndMethod());
-                    assertFalse(Networking.isPortAvailable(port), "port mistakenly reported as available");
-                } finally {
-                    if (ss != null) {
-                        ss.close();
-                    }
-                }
+                return port;
             }
-            // repeat until we can get a port
-        } while (ss == null && port < 60000);
-        Assert.assertNotNull(ss, "could not get a port");
-        
+        }
+        throw Asserts.fail("This test requires that at least one port a little bit under port 60000 not be in use.");
+    }
+    
+    @Test
+    public void testIsPortAvailableReportsFalseWhenPortIsInUse() throws Exception {
+        int port = findPort();
+        try (ServerSocket ss = new ServerSocket(port)) {
+            log.info("acquired port on "+port+" for test "+JavaClassNames.niceClassAndMethod());
+            assertFalse(Networking.isPortAvailable(port), "port mistakenly reported as available");
+        }
+        // above will close, so below succeeds
+          
         final int portF = port;
         Asserts.succeedsEventually(new Runnable() {
             @Override public void run() {
                 assertTrue(Networking.isPortAvailable(portF), "port "+portF+" not made available afterwards");
             }});
     }
-
+    
+    @Test
+    public void testPortNotAvailableOnAnyNicWithReuseAddressWhenBoundToAnyNic() throws Exception {
+        int port = findPort();
+        try (ServerSocket ss = new ServerSocket()) {
+            ss.setReuseAddress(true);
+            ss.bind(new InetSocketAddress(Networking.ANY_NIC, port));
+            log.info("acquired port on "+port+" for test "+JavaClassNames.niceClassAndMethod());
+            assertFalse(Networking.isPortAvailable(Networking.ANY_NIC, port, true), "port mistakenly reported as available");
+            // a specific NIC *will* be available
+            //assertFalse(Networking.isPortAvailable(Networking.LOOPBACK, port, true), "port mistakenly reported as available");
+        }
+    }
+    
+    @Test
+    public void testPortNotAvailableOnAnyNicOrLoopbackWithReuseAddressWhenBoundToLoopback() throws Exception {
+        int port = findPort();
+        try (ServerSocket ss = new ServerSocket()) {
+            ss.setReuseAddress(true);
+            ss.bind(new InetSocketAddress(Networking.LOOPBACK, port));
+            log.info("acquired port on "+port+" for test "+JavaClassNames.niceClassAndMethod());
+            assertFalse(Networking.isPortAvailable(Networking.LOOPBACK, port, true), "port mistakenly reported as available");
+            assertFalse(Networking.isPortAvailable(Networking.ANY_NIC, port, true), "port mistakenly reported as available");
+        }
+    }
+    
     @Test
     public void testIsPortAvailableReportsPromptly() throws Exception {
         // repeat until we can get an available port


[3/6] brooklyn-server git commit: let brooklyn bind with reuse_addr

Posted by dr...@apache.org.
let brooklyn bind with reuse_addr

means starting, stopping, starting will pick up same address;
doesn't go to 8082 just because previous instance in TIME_WAIT state


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/73a33723
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/73a33723
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/73a33723

Branch: refs/heads/master
Commit: 73a337232f3ceafda01e4f8119463231204f3d7c
Parents: a738eb5
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed Jul 19 13:00:34 2017 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Sat Jul 22 03:10:09 2017 +0100

----------------------------------------------------------------------
 .../LocalhostMachineProvisioningLocation.java   | 22 +++++++++++++++-----
 .../brooklyn/launcher/BrooklynWebServer.java    |  4 +++-
 2 files changed, 20 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/73a33723/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocation.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocation.java b/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocation.java
index b73fd37..ae26b05 100644
--- a/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocation.java
+++ b/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocation.java
@@ -198,12 +198,15 @@ public class LocalhostMachineProvisioningLocation extends FixedListMachineProvis
        }
     }
 
-    public static synchronized boolean obtainSpecificPort(InetAddress localAddress, int portNumber) {
+    public static boolean obtainSpecificPort(InetAddress localAddress, int portNumber) {
+        return obtainSpecificPort(localAddress, portNumber, false);
+    }
+    public static synchronized boolean obtainSpecificPort(InetAddress localAddress, int portNumber, Boolean reuseAddr) {
         if (portsInUse.contains(portNumber)) {
             return false;
         } else {
             //see if it is available?
-            if (!checkPortAvailable(localAddress, portNumber)) {
+            if (!checkPortAvailable(localAddress, portNumber, reuseAddr)) {
                 return false;
             }
             portsInUse.add(portNumber);
@@ -212,18 +215,27 @@ public class LocalhostMachineProvisioningLocation extends FixedListMachineProvis
     }
     /** checks the actual availability of the port on localhost, ie by binding to it; cf {@link Networking#isPortAvailable(int)} */
     public static boolean checkPortAvailable(InetAddress localAddress, int portNumber) {
+        return checkPortAvailable(localAddress, portNumber, false);
+    }
+    public static boolean checkPortAvailable(InetAddress localAddress, int portNumber, Boolean reuseAddr) {
         if (portNumber<1024) {
             if (LOG.isDebugEnabled()) LOG.debug("Skipping system availability check for privileged localhost port "+portNumber);
             return true;
         }
-        return Networking.isPortAvailable(localAddress, portNumber);
+        return Networking.isPortAvailable(localAddress, portNumber, reuseAddr);
     }
     public static int obtainPort(PortRange range) {
-        return obtainPort(getLocalhostInetAddress(), range);
+        return obtainPort(range, false);
+    }
+    public static int obtainPort(PortRange range, Boolean reuseAddr) {    
+        return obtainPort(getLocalhostInetAddress(), range, reuseAddr);
     }
     public static int obtainPort(InetAddress localAddress, PortRange range) {
+        return obtainPort(localAddress, range, false);
+    }
+    public static int obtainPort(InetAddress localAddress, PortRange range, Boolean reuseAddr) {
         for (int p: range)
-            if (obtainSpecificPort(localAddress, p)) return p;
+            if (obtainSpecificPort(localAddress, p, reuseAddr)) return p;
         if (LOG.isDebugEnabled()) LOG.debug("unable to find port in {} on {}; returning -1", range, localAddress);
         return -1;
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/73a33723/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java
----------------------------------------------------------------------
diff --git a/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java b/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java
index 42f1e7f..889f4b1 100644
--- a/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java
+++ b/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java
@@ -386,7 +386,9 @@ public class BrooklynWebServer {
             if (portRange==null) {
                 portRange = getHttpsEnabled() ? httpsPort : httpPort;
             }
-            actualPort = LocalhostMachineProvisioningLocation.obtainPort(getAddress(), portRange);
+            actualPort = LocalhostMachineProvisioningLocation.obtainPort(getAddress(), portRange,
+                // allow reuse-addr so we prefer to bind to lower-numbered ports even if they are in time-wait state
+                true);
             if (actualPort == -1) 
                 throw new IllegalStateException("Unable to provision port for web console (wanted "+portRange+")");
         }


[4/6] brooklyn-server git commit: prefer IPv4 addresses for localhost (this is what people seem to expect)

Posted by dr...@apache.org.
prefer IPv4 addresses for localhost (this is what people seem to expect)


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/a738eb55
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/a738eb55
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/a738eb55

Branch: refs/heads/master
Commit: a738eb55f3182d81241e070fcee2b418a145d9d2
Parents: 810ce62
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed Jul 19 12:54:26 2017 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Sat Jul 22 03:10:09 2017 +0100

----------------------------------------------------------------------
 .../util/core/BrooklynNetworkUtils.java         |  4 ++--
 .../apache/brooklyn/util/net/Networking.java    | 24 ++++++++++++++------
 2 files changed, 19 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a738eb55/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java b/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java
index 190ad75..949f48f 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java
@@ -44,9 +44,9 @@ public class BrooklynNetworkUtils {
      * {@link BrooklynServiceAttributes#LOCALHOST_IP_ADDRESS}
      * if set to prevent default selection when needed,
      * otherwise finding the first bindable/reachable NIC from a system lookup which usually
-     * prefers non-loopback devices (but use the system property if if needed) */
+     * prefers IPv4 then non-loopback devices (but use the system property if if needed) */
     public static InetAddress getLocalhostInetAddress() {
         return TypeCoercions.coerce(JavaGroovyEquivalents.elvis(BrooklynServiceAttributes.LOCALHOST_IP_ADDRESS.getValue(),
-                Networking.getLocalHost(true, false, true, 500)), InetAddress.class);
+                Networking.getLocalHost(true, false, true, true, 500)), InetAddress.class);
     }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a738eb55/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java b/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java
index f0728dd..a02dea2 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java
@@ -22,6 +22,7 @@ import static com.google.common.base.Preconditions.checkArgument;
 
 import java.io.IOException;
 import java.net.DatagramSocket;
+import java.net.Inet4Address;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.NetworkInterface;
@@ -50,6 +51,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.base.Stopwatch;
 import com.google.common.base.Throwables;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Range;
 import com.google.common.collect.RangeSet;
 import com.google.common.collect.TreeRangeSet;
@@ -89,6 +91,9 @@ public class Networking {
         return isPortAvailable(ANY_NIC, port);
     }
     public static boolean isPortAvailable(InetAddress localAddress, int port) {
+        return isPortAvailable(localAddress, port, SET_REUSE_ADDRESS);
+    }
+    public static boolean isPortAvailable(InetAddress localAddress, int port, Boolean allowReuse) {
         if (port < MIN_PORT_NUMBER || port > MAX_PORT_NUMBER) {
             throw new IllegalArgumentException("Invalid start port: " + port);
         }
@@ -101,19 +106,21 @@ public class Networking {
             //Svet - SO_REUSEADDR (enabled below) will allow one socket to listen on 0.0.0.0:X and another on
             //192.168.0.1:X which explains the above comment (nginx sets SO_REUSEADDR as well). Moreover there
             //is no TIME_WAIT for listening sockets without any connections so why enable it at all.
+            //Alex - TIME_WAIT sticks around for a while (30s or so) if a java process dies while it has
+            //connections; if we want things to aggressively try to take a port, they should use reuse-addr
             ServerSocket ss = null;
             DatagramSocket ds = null;
             try {
                 // Check TCP port
                 ss = new ServerSocket();
                 ss.setSoTimeout(250);
-                if (SET_REUSE_ADDRESS!=null) { ss.setReuseAddress(SET_REUSE_ADDRESS); }
+                if (allowReuse!=null) { ss.setReuseAddress(allowReuse); }
                 ss.bind(new InetSocketAddress(localAddress, port));
 
                 // Check UDP port
                 ds = new DatagramSocket(null);
                 ds.setSoTimeout(250);
-                if (SET_REUSE_ADDRESS!=null) { ss.setReuseAddress(SET_REUSE_ADDRESS); }
+                if (allowReuse!=null) { ss.setReuseAddress(allowReuse); }
                 ds.bind(new InetSocketAddress(localAddress, port));
             } catch (IOException e) {
                 if (log.isTraceEnabled()) log.trace("Failed binding to " + localAddress + " : " + port, e);
@@ -143,7 +150,7 @@ public class Networking {
                     Enumeration<InetAddress> as = ni.getInetAddresses();
                     while (as.hasMoreElements()) {
                         InetAddress a = as.nextElement();
-                        if (!isPortAvailable(a, port)) {
+                        if (!isPortAvailable(a, port, allowReuse)) {
                             if (isAddressValid(a)) {
                                 if (log.isTraceEnabled()) log.trace("Port {} : {} @ {} is taken and the address is valid", new Object[] {a, port, nis});
                                 return false;
@@ -433,12 +440,12 @@ public class Networking {
      * use {@link #getLocalHost(boolean, boolean, boolean, int)} for full control or
      * {@link #getReachableLocalHost()} for a method with better defaults and errors */
     public static InetAddress getLocalHost() {
-        return getLocalHost(false, true, false, 0);
+        return getLocalHost(false, true, true, false, 0);
     }
     
     /** returns a validated local IP address, preferring 127.0.0.1 if it works, and failing if none found */
     public static InetAddress getReachableLocalHost() {
-        return getLocalHost(true, true, true, 250);
+        return getLocalHost(true, true, true, true, 250);
     }
     
     /**
@@ -450,12 +457,15 @@ public class Networking {
      * @param timeout how long to wait for each address (if doing checkReachable)
      * @return an {@link InetAddress} corresponding to localhost
      */
-    public static InetAddress getLocalHost(boolean checkReachable, boolean prefer127, boolean failIfNone, int timeout) {
+    public static InetAddress getLocalHost(boolean checkReachable, boolean prefer127, boolean preferIpV4, boolean failIfNone, int timeout) {
         Map<String, InetAddress> addrs = getLocalAddresses();
         MutableSet<InetAddress> candidates = new MutableSet<>();
         if (prefer127) {
             candidates.addIfNotNull(addrs.get("127.0.0.1"));
         }
+        if (preferIpV4) {
+            candidates.addAll(Iterables.filter(addrs.values(), Inet4Address.class));
+        }
         candidates.addAll(addrs.values());
         if (checkReachable) {
             for (InetAddress a: candidates) {
@@ -589,7 +599,7 @@ public class Networking {
             if (timeout>0) {
                 s.setSoTimeout(timeout);
             }
-            s.connect(new InetSocketAddress(endpoint.getHostText(), endpoint.getPort()));
+            s.connect(new InetSocketAddress(endpoint.getHostText(), endpoint.getPort()), timeout);
             closeQuietly(s);
             return true;
         } catch (Exception e) {


[2/6] brooklyn-server git commit: improve how we find localhost

Posted by dr...@apache.org.
improve how we find localhost

increasingly i'm getting unusable NICs preferred by the method we used to use,
InetAddress.getLocalhost(), and comments elsewhere eg https://stackoverflow.com/questions/5813194/inetaddress-getlocalhost-does-not-return-expected-ip-address-from-c-windows-s
suggest that wasn't a good method in any case

update most places to require a _reachable_ localhost address


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/3647d1f2
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/3647d1f2
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/3647d1f2

Branch: refs/heads/master
Commit: 3647d1f245c36386ef17cf6a75954c0801e95e9a
Parents: 9795c47
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed Jul 19 10:27:36 2017 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Sat Jul 22 03:10:08 2017 +0100

----------------------------------------------------------------------
 .../LocalhostMachineProvisioningLocation.java   |  8 +-
 .../util/core/BrooklynNetworkUtils.java         |  2 +-
 .../core/location/SimulatedLocation.java        |  2 +-
 ...ocalhostMachineProvisioningLocationTest.java |  7 +-
 .../ssh/SshMachineLocationIntegrationTest.java  |  6 +-
 .../ssh/SshMachineLocationPerformanceTest.java  |  2 +-
 .../SshMachineLocationReuseIntegrationTest.java |  4 +-
 .../location/ssh/SshMachineLocationTest.java    |  6 +-
 .../usage/JcloudsLocationUsageTrackingTest.java |  2 +-
 .../winrm/WinRmMachineLocationTest.java         |  2 +-
 .../framework/TestEndpointReachableTest.java    |  4 +-
 .../apache/brooklyn/util/net/Networking.java    | 89 ++++++++++++++++----
 .../brooklyn/util/net/NetworkingUtilsTest.java  | 22 ++++-
 .../util/net/ReachableSocketFinderTest.java     |  2 +-
 14 files changed, 115 insertions(+), 43 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocation.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocation.java b/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocation.java
index 014535b..b73fd37 100644
--- a/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocation.java
+++ b/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocation.java
@@ -177,7 +177,7 @@ public class LocalhostMachineProvisioningLocation extends FixedListMachineProvis
         for (int i=0; i<size; i++) {
             Map<Object,Object> flags2 = MutableMap.<Object,Object>builder()
                     .putAll(flags)
-                    .put("address", elvis(address, Networking.getLocalHost()))
+                    .put("address", elvis(address, getLocalhostInetAddress()))
                     .build();
             
             // copy inherited keys for ssh; 
@@ -309,17 +309,17 @@ public class LocalhostMachineProvisioningLocation extends FixedListMachineProvis
         @Override
         public LocalhostMachine configure(Map<?,?> properties) {
             if (address==null || !properties.containsKey("address"))
-                address = Networking.getLocalHost();
+                address = getLocalhostInetAddress();
             super.configure(properties);
             return this;
         }
         @Override
         public String getSubnetHostname() {
-           return Networking.getLocalHost().getHostName();
+           return Networking.getReachableLocalHost().getHostName();
         }
         @Override
         public String getSubnetIp() {
-            return Networking.getLocalHost().getHostAddress();
+            return Networking.getReachableLocalHost().getHostAddress();
         }
     }
 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java b/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java
index 33733b1..b1cd49a 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java
@@ -42,6 +42,6 @@ public class BrooklynNetworkUtils {
     /** returns a IP address for localhost paying attention to a system property to prevent lookup in some cases */
     public static InetAddress getLocalhostInetAddress() {
         return TypeCoercions.coerce(JavaGroovyEquivalents.elvis(BrooklynServiceAttributes.LOCALHOST_IP_ADDRESS.getValue(),
-                Networking.getLocalHost()), InetAddress.class);
+                Networking.getReachableLocalHost()), InetAddress.class);
     }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/core/src/test/java/org/apache/brooklyn/core/location/SimulatedLocation.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/location/SimulatedLocation.java b/core/src/test/java/org/apache/brooklyn/core/location/SimulatedLocation.java
index e9062f2..bfb48e2 100644
--- a/core/src/test/java/org/apache/brooklyn/core/location/SimulatedLocation.java
+++ b/core/src/test/java/org/apache/brooklyn/core/location/SimulatedLocation.java
@@ -51,7 +51,7 @@ public class SimulatedLocation extends AbstractLocation implements MachineProvis
 
     private static final InetAddress address;
     static {
-        address = Networking.getLocalHost();
+        address = Networking.getReachableLocalHost();
     }
 
     Iterable<Integer> permittedPorts = PortRanges.fromString("1+");

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/core/src/test/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocationTest.java b/core/src/test/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocationTest.java
index 6839afa..320c6d7 100644
--- a/core/src/test/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocationTest.java
+++ b/core/src/test/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocationTest.java
@@ -33,6 +33,7 @@ import org.apache.brooklyn.core.location.geo.HostGeoInfo;
 import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport;
 import org.apache.brooklyn.location.ssh.SshMachineLocation;
 import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
 import org.apache.brooklyn.util.net.Networking;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -76,7 +77,7 @@ public class LocalhostMachineProvisioningLocationTest extends BrooklynMgmtUnitTe
         LocalhostMachineProvisioningLocation provisioner = mgmt.getLocationManager().createLocation(LocationSpec.create(LocalhostMachineProvisioningLocation.class));
         SshMachineLocation machine = provisioner.obtain();
         assertNotNull(machine);
-        assertEquals(machine.getAddress(), Networking.getLocalHost());
+        assertEquals(machine.getAddress(), BrooklynNetworkUtils.getLocalhostInetAddress());
     }
 
     @Test
@@ -98,12 +99,12 @@ public class LocalhostMachineProvisioningLocationTest extends BrooklynMgmtUnitTe
         // first machine
         SshMachineLocation first = provisioner.obtain();
         assertNotNull(first);
-        assertEquals(first.getAddress(), Networking.getLocalHost());
+        assertEquals(first.getAddress(), Networking.getReachableLocalHost());
 
         // second machine
         SshMachineLocation second = provisioner.obtain();
         assertNotNull(second);
-        assertEquals(second.getAddress(), Networking.getLocalHost());
+        assertEquals(second.getAddress(), Networking.getReachableLocalHost());
 
         // third machine - fails
         try {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationIntegrationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationIntegrationTest.java b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationIntegrationTest.java
index 6f7b324..9ff39c1 100644
--- a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationIntegrationTest.java
+++ b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationIntegrationTest.java
@@ -87,7 +87,7 @@ public class SshMachineLocationIntegrationTest extends SshMachineLocationTest {
     @Override
     protected SshMachineLocation newHost() {
         return mgmt.getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
-                .configure("address", Networking.getLocalHost()));
+                .configure("address", Networking.getReachableLocalHost()));
     }
 
     // Overridden just to make it integration (because `newHost()` returns a real ssh'ing host)
@@ -222,7 +222,7 @@ public class SshMachineLocationIntegrationTest extends SshMachineLocationTest {
     // For issue #230
     @Test(groups = "Integration")
     public void testOverridingPropertyOnExec() throws Exception {
-        SshMachineLocation host = new SshMachineLocation(MutableMap.of("address", Networking.getLocalHost(), "sshPrivateKeyData", "wrongdata"));
+        SshMachineLocation host = new SshMachineLocation(MutableMap.of("address", Networking.getReachableLocalHost(), "sshPrivateKeyData", "wrongdata"));
         
         OutputStream outStream = new ByteArrayOutputStream();
         String expectedName = Os.user();
@@ -234,7 +234,7 @@ public class SshMachineLocationIntegrationTest extends SshMachineLocationTest {
 
     @Test(groups = "Integration", expectedExceptions={IllegalStateException.class, SshException.class})
     public void testSshRunWithInvalidUserFails() throws Exception {
-        SshMachineLocation badHost = new SshMachineLocation(MutableMap.of("user", "doesnotexist", "address", Networking.getLocalHost()));
+        SshMachineLocation badHost = new SshMachineLocation(MutableMap.of("user", "doesnotexist", "address", Networking.getReachableLocalHost()));
         badHost.execScript("mysummary", ImmutableList.of("whoami; exit"));
     }
     

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationPerformanceTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationPerformanceTest.java b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationPerformanceTest.java
index c4d7d7d..795f87f 100644
--- a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationPerformanceTest.java
+++ b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationPerformanceTest.java
@@ -111,7 +111,7 @@ public class SshMachineLocationPerformanceTest {
     public void testConsecutiveSmallCommandsWithDifferentProperties() throws Exception {
         final Map<String, ?> emptyProperties = Collections.emptyMap();
         final Map<String, ?> customProperties = MutableMap.of(
-                "address", Networking.getLocalHost(),
+                "address", Networking.getReachableLocalHost(),
                 SshTool.PROP_SESSION_TIMEOUT.getName(), 20000,
                 SshTool.PROP_CONNECT_TIMEOUT.getName(), 50000,
                 SshTool.PROP_SCRIPT_HEADER.getName(), "#!/bin/bash");

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationReuseIntegrationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationReuseIntegrationTest.java b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationReuseIntegrationTest.java
index cebd2f4..924c9c7 100644
--- a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationReuseIntegrationTest.java
+++ b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationReuseIntegrationTest.java
@@ -102,7 +102,7 @@ public class SshMachineLocationReuseIntegrationTest {
     public void setUp() throws Exception {
         managementContext = new LocalManagementContext();
         host = managementContext.getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
-                .configure("address", Networking.getLocalHost())
+                .configure("address", Networking.getReachableLocalHost())
                 .configure(SshMachineLocation.SSH_TOOL_CLASS, RecordingSshjTool.class.getName()));
     }
 
@@ -163,7 +163,7 @@ public class SshMachineLocationReuseIntegrationTest {
 
     public Map<String, Object> customSshConfigKeys() throws UnknownHostException {
         return MutableMap.<String, Object>of(
-                "address", Networking.getLocalHost(),
+                "address", Networking.getReachableLocalHost(),
                 SshTool.PROP_SESSION_TIMEOUT.getName(), 20000,
                 SshTool.PROP_CONNECT_TIMEOUT.getName(), 50000,
                 SshTool.PROP_SCRIPT_HEADER.getName(), "#!/bin/bash");

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationTest.java b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationTest.java
index e610fd2..5c6e918 100644
--- a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationTest.java
+++ b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationTest.java
@@ -196,7 +196,7 @@ public class SshMachineLocationTest extends BrooklynAppUnitTestSupport {
     @Test
     public void testConfigurePrivateAddresses() throws Exception {
         SshMachineLocation host2 = mgmt.getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
-                .configure("address", Networking.getLocalHost())
+                .configure("address", Networking.getReachableLocalHost())
                 .configure(SshMachineLocation.PRIVATE_ADDRESSES, ImmutableList.of("1.2.3.4"))
                 .configure(BrooklynConfigKeys.SKIP_ON_BOX_BASE_DIR_RESOLUTION, true));
 
@@ -211,7 +211,7 @@ public class SshMachineLocationTest extends BrooklynAppUnitTestSupport {
     @Test
     public void testGetMachineIsInessentialOnFailure() throws Exception {
         SshMachineLocation host2 = mgmt.getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
-                .configure("address", Networking.getLocalHost())
+                .configure("address", Networking.getReachableLocalHost())
                 .configure(SshMachineLocation.SSH_TOOL_CLASS, FailingSshTool.class.getName()));
 
         final Effector<MachineDetails> GET_MACHINE_DETAILS = Effectors.effector(MachineDetails.class, "getMachineDetails")
@@ -314,7 +314,7 @@ public class SshMachineLocationTest extends BrooklynAppUnitTestSupport {
     
     @Test
     public void testObtainPortDoesNotUsePreReservedPorts() {
-        host = new SshMachineLocation(MutableMap.of("address", Networking.getLocalHost(), "usedPorts", ImmutableSet.of(8000)));
+        host = new SshMachineLocation(MutableMap.of("address", Networking.getReachableLocalHost(), "usedPorts", ImmutableSet.of(8000)));
         assertEquals(host.obtainPort(PortRanges.fromString("8000")), -1);
         assertEquals(host.obtainPort(PortRanges.fromString("8000+")), 8001);
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/core/mgmt/usage/JcloudsLocationUsageTrackingTest.java
----------------------------------------------------------------------
diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/core/mgmt/usage/JcloudsLocationUsageTrackingTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/core/mgmt/usage/JcloudsLocationUsageTrackingTest.java
index 333d181..21acbc2 100644
--- a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/core/mgmt/usage/JcloudsLocationUsageTrackingTest.java
+++ b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/core/mgmt/usage/JcloudsLocationUsageTrackingTest.java
@@ -110,7 +110,7 @@ public class JcloudsLocationUsageTrackingTest extends AbstractJcloudsStubbedLive
         entity = app.createAndManageChild(EntitySpec.create(SoftwareProcessEntityTest.MyService.class));
         
         serverSocket = new ServerSocket();
-        serverSocket.bind(new InetSocketAddress(Networking.getLocalHost(), 0), 0);
+        serverSocket.bind(new InetSocketAddress(Networking.getReachableLocalHost(), 0), 0);
     }
 
     @AfterMethod(alwaysRun=true)

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/software/winrm/src/test/java/org/apache/brooklyn/location/winrm/WinRmMachineLocationTest.java
----------------------------------------------------------------------
diff --git a/software/winrm/src/test/java/org/apache/brooklyn/location/winrm/WinRmMachineLocationTest.java b/software/winrm/src/test/java/org/apache/brooklyn/location/winrm/WinRmMachineLocationTest.java
index 9fa0fe7..a258aeb 100644
--- a/software/winrm/src/test/java/org/apache/brooklyn/location/winrm/WinRmMachineLocationTest.java
+++ b/software/winrm/src/test/java/org/apache/brooklyn/location/winrm/WinRmMachineLocationTest.java
@@ -34,7 +34,7 @@ public class WinRmMachineLocationTest extends BrooklynAppUnitTestSupport {
     @Test
     public void testConfigurePrivateAddresses() throws Exception {
         WinRmMachineLocation host = mgmt.getLocationManager().createLocation(LocationSpec.create(WinRmMachineLocation.class)
-                .configure("address", Networking.getLocalHost())
+                .configure("address", Networking.getReachableLocalHost())
                 .configure(WinRmMachineLocation.PRIVATE_ADDRESSES, ImmutableList.of("1.2.3.4"))
                 .configure(BrooklynConfigKeys.SKIP_ON_BOX_BASE_DIR_RESOLUTION, true));
 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/test-framework/src/test/java/org/apache/brooklyn/test/framework/TestEndpointReachableTest.java
----------------------------------------------------------------------
diff --git a/test-framework/src/test/java/org/apache/brooklyn/test/framework/TestEndpointReachableTest.java b/test-framework/src/test/java/org/apache/brooklyn/test/framework/TestEndpointReachableTest.java
index efeb7eb..42c35dc 100644
--- a/test-framework/src/test/java/org/apache/brooklyn/test/framework/TestEndpointReachableTest.java
+++ b/test-framework/src/test/java/org/apache/brooklyn/test/framework/TestEndpointReachableTest.java
@@ -227,7 +227,7 @@ public class TestEndpointReachableTest extends BrooklynAppUnitTestSupport {
     }
 
     protected ServerSocket openServerPort() throws IOException {
-        InetAddress localAddress = Networking.getLocalHost();
+        InetAddress localAddress = Networking.getReachableLocalHost();
         return new ServerSocket(0, 1024, localAddress);
     }
 
@@ -235,7 +235,7 @@ public class TestEndpointReachableTest extends BrooklynAppUnitTestSupport {
         int startPort = 58767;
         int endPort = 60000;
         int port = startPort;
-        InetAddress localAddress = Networking.getLocalHost();
+        InetAddress localAddress = Networking.getReachableLocalHost();
         do {
             if (Networking.isPortAvailable(localAddress, port)) {
                 return HostAndPort.fromParts(localAddress.getHostAddress(), port);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java b/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java
index f944b77..f0728dd 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java
@@ -38,10 +38,7 @@ import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import java.util.regex.Pattern;
 
-import com.google.common.base.Predicate;
-import com.google.common.collect.Range;
-import com.google.common.collect.RangeSet;
-import com.google.common.collect.TreeRangeSet;
+import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.text.Identifiers;
 import org.apache.brooklyn.util.text.Strings;
@@ -50,8 +47,12 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
 import com.google.common.base.Stopwatch;
 import com.google.common.base.Throwables;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+import com.google.common.collect.TreeRangeSet;
 import com.google.common.net.HostAndPort;
 import com.google.common.primitives.UnsignedBytes;
 
@@ -427,13 +428,62 @@ public class Networking {
         }
     }
 
-    /** returns local IP address, or 127.0.0.1 if it cannot be parsed */
+    /** returns local IP address, or 127.0.0.1 if it cannot be parsed
+     * @deprecated since 0.12.0, as this method has bad defaults (fails on some machines);
+     * use {@link #getLocalHost(boolean, boolean, boolean, int)} for full control or
+     * {@link #getReachableLocalHost()} for a method with better defaults and errors */
     public static InetAddress getLocalHost() {
-        try {
-            return InetAddress.getLocalHost();
-        } catch (UnknownHostException e) {
-            InetAddress result = null;
-            result = getInetAddressWithFixedName("127.0.0.1");
+        return getLocalHost(false, true, false, 0);
+    }
+    
+    /** returns a validated local IP address, preferring 127.0.0.1 if it works, and failing if none found */
+    public static InetAddress getReachableLocalHost() {
+        return getLocalHost(true, true, true, 250);
+    }
+    
+    /**
+     * returns a local IP address, using some heuristics to determine the best one 
+     * in the case of multiple possibilities, as follows 
+     * @param checkReachable whether to ensure that the address is bindable and reachable
+     * @param prefer127 whether to prefer standard loopback address 127.0.0.1 if listed
+     * @param failIfNone whether to fail if none or simply return 127.0.0.1
+     * @param timeout how long to wait for each address (if doing checkReachable)
+     * @return an {@link InetAddress} corresponding to localhost
+     */
+    public static InetAddress getLocalHost(boolean checkReachable, boolean prefer127, boolean failIfNone, int timeout) {
+        Map<String, InetAddress> addrs = getLocalAddresses();
+        MutableSet<InetAddress> candidates = new MutableSet<>();
+        if (prefer127) {
+            candidates.addIfNotNull(addrs.get("127.0.0.1"));
+        }
+        candidates.addAll(addrs.values());
+        if (checkReachable) {
+            for (InetAddress a: candidates) {
+                try (ServerSocket ss = new ServerSocket()) {
+                    ss.bind(new InetSocketAddress(a, 0));
+                    if (isReachable(HostAndPort.fromParts(a.getHostAddress(), ss.getLocalPort()), true, timeout)) {
+                        return a;
+                    }
+                } catch (Exception e) {
+                    Exceptions.propagateIfFatal(e);
+                    // swallow and continue
+                }
+            }
+        } else {
+            if (!candidates.isEmpty()) {
+                return candidates.iterator().next();
+            }
+        }
+        if (failIfNone) {
+            throw new IllegalStateException("No reachable local addresses could be found; ensure that localhost is correctly configured on this machine"
+                + ", and if required that network security permits local access to localhost ports");
+        } else {
+            InetAddress result;
+            try {
+                result = InetAddress.getLocalHost();
+            } catch (UnknownHostException e2) {
+                result = getInetAddressWithFixedName("127.0.0.1");
+            }
             log.warn("Localhost is not resolvable; using "+result);
             return result;
         }
@@ -527,14 +577,19 @@ public class Networking {
     }
     
     public static boolean isReachable(HostAndPort endpoint) {
-        // TODO Should we create an unconnected socket, and then use the calls below (see jclouds' InetSocketAddressConnect):
-        //      socket.setReuseAddress(false);
-        //      socket.setSoLinger(false, 1);
-        //      socket.setSoTimeout(timeout);
-        //      socket.connect(socketAddress, timeout);
-        
+        return isReachable(endpoint, false, 0);
+    }
+    public static boolean isReachable(HostAndPort endpoint, boolean noReuseOrLinger, int timeout) {
         try {
-            Socket s = new Socket(endpoint.getHostText(), endpoint.getPort());
+            Socket s = new Socket();
+            if (noReuseOrLinger) {
+                s.setReuseAddress(false);
+                s.setSoLinger(false, 1);
+            }
+            if (timeout>0) {
+                s.setSoTimeout(timeout);
+            }
+            s.connect(new InetSocketAddress(endpoint.getHostText(), endpoint.getPort()));
             closeQuietly(s);
             return true;
         } catch (Exception e) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java
index 1afffd7..c56ac66 100644
--- a/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java
+++ b/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java
@@ -29,6 +29,7 @@ import java.io.IOException;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.ServerSocket;
+import java.net.Socket;
 import java.net.UnknownHostException;
 import java.util.Collection;
 import java.util.concurrent.TimeUnit;
@@ -217,13 +218,28 @@ public class NetworkingUtilsTest {
     }
 
     @Test
-    public void testIsPortAvailableValidatesAddress() throws Exception {
+    public void testCanConnectToLocalhost() throws Exception {
+        try (ServerSocket ss = new ServerSocket()) {
+            ss.bind(new InetSocketAddress(Networking.getReachableLocalHost(), 0));
+            // check we can connect to it
+            try (Socket s = new Socket()) {
+                s.setSoTimeout(500);
+                try {
+                    s.connect(ss.getLocalSocketAddress(), 500);
+                } catch (Exception e) {
+                    Assert.fail("Localhost as "+ss.getInetAddress()+" is not reachable; ensure localhost is correctly configured and addressible to use Brooklyn", e);
+                }
+            }
+        }
+    }
+        
+    @Test
+    public void testBindToLocalhostAndIsPortAvailableDetectsExplicitLocalhostBinding() throws Exception {
         ServerSocket ss = new ServerSocket();
-        ss.bind(new InetSocketAddress(InetAddress.getLocalHost(), 0));
+        ss.bind(new InetSocketAddress(Networking.getReachableLocalHost(), 0));
         int boundPort = ss.getLocalPort();
         assertTrue(ss.isBound());
         assertNotEquals(boundPort, 0);
-        //will run isAddressValid before returning
         assertFalse(Networking.isPortAvailable(boundPort));
         ss.close();
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3647d1f2/utils/common/src/test/java/org/apache/brooklyn/util/net/ReachableSocketFinderTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/net/ReachableSocketFinderTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/net/ReachableSocketFinderTest.java
index cd1dc00..fe8a90f 100644
--- a/utils/common/src/test/java/org/apache/brooklyn/util/net/ReachableSocketFinderTest.java
+++ b/utils/common/src/test/java/org/apache/brooklyn/util/net/ReachableSocketFinderTest.java
@@ -173,7 +173,7 @@ public class ReachableSocketFinderTest {
     @Test(groups="Integration")
     public void testFailsIfRealSocketUnreachable() throws Exception {
         ReachableSocketFinder realFinder = new ReachableSocketFinder();
-        HostAndPort wrongAddr = HostAndPort.fromParts(Networking.getLocalHost().getHostAddress(), findAvailablePort());
+        HostAndPort wrongAddr = HostAndPort.fromParts(Networking.getReachableLocalHost().getHostAddress(), findAvailablePort());
         
         try {
             HostAndPort result = realFinder.findOpenSocketOnNode(ImmutableList.of(wrongAddr), Duration.FIVE_SECONDS);


[6/6] brooklyn-server git commit: This closes #768

Posted by dr...@apache.org.
This closes #768


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/d85ffa5b
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/d85ffa5b
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/d85ffa5b

Branch: refs/heads/master
Commit: d85ffa5b49dd113d3bfbe0db69d42e22b79fc682
Parents: 29583d8 19e94e9
Author: Duncan Godwin <dr...@googlemail.com>
Authored: Mon Jul 24 15:37:32 2017 +0100
Committer: Duncan Godwin <dr...@googlemail.com>
Committed: Mon Jul 24 15:37:32 2017 +0100

----------------------------------------------------------------------
 .../LocalhostMachineProvisioningLocation.java   |  30 ++++--
 .../util/core/BrooklynNetworkUtils.java         |   9 +-
 .../core/location/SimulatedLocation.java        |   2 +-
 ...ocalhostMachineProvisioningLocationTest.java |   7 +-
 .../ssh/SshMachineLocationIntegrationTest.java  |   6 +-
 .../ssh/SshMachineLocationPerformanceTest.java  |   2 +-
 .../SshMachineLocationReuseIntegrationTest.java |   4 +-
 .../location/ssh/SshMachineLocationTest.java    |   6 +-
 .../brooklyn/launcher/BrooklynWebServer.java    |  37 +++++--
 .../usage/JcloudsLocationUsageTrackingTest.java |   2 +-
 .../winrm/WinRmMachineLocationTest.java         |   2 +-
 .../framework/TestEndpointReachableTest.java    |   4 +-
 .../apache/brooklyn/util/net/Networking.java    | 108 +++++++++++++++----
 .../brooklyn/util/net/NetworkingUtilsTest.java  |  84 +++++++++++----
 .../util/net/ReachableSocketFinderTest.java     |   2 +-
 15 files changed, 223 insertions(+), 82 deletions(-)
----------------------------------------------------------------------