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 <[email protected]> Authored: Mon Jul 24 02:31:27 2017 +0100 Committer: Alex Heneveld <[email protected]> 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
