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:42 UTC

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

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