From f6ec07d87d20b0aed2be53c4c1413b780aef016f Mon Sep 17 00:00:00 2001 From: Carl Mastrangelo Date: Wed, 6 Feb 2019 15:49:59 -0800 Subject: [PATCH] core,netty: expose listening on multiple ports --- core/src/main/java/io/grpc/Server.java | 18 ++++++++- .../io/grpc/inprocess/InProcessServer.java | 9 +++-- .../java/io/grpc/internal/InternalServer.java | 13 ++++--- .../java/io/grpc/internal/ServerImpl.java | 38 ++++++++++++++++--- .../grpc/inprocess/InProcessServerTest.java | 2 +- .../java/io/grpc/internal/ServerImplTest.java | 16 ++++---- .../integration/AbstractInteropTest.java | 4 +- .../integration/AutoWindowSizingOnTest.java | 2 +- .../testing/integration/Http2NettyTest.java | 8 ++-- .../testing/integration/Http2OkHttpTest.java | 15 +++++--- .../integration/TransportCompressionTest.java | 2 +- .../main/java/io/grpc/netty/NettyServer.java | 31 +++++++-------- .../io/grpc/netty/NettyServerBuilder.java | 14 ++++--- .../grpc/netty/NettyClientTransportTest.java | 12 +++--- .../io/grpc/netty/NettyServerBuilderTest.java | 3 +- .../java/io/grpc/netty/NettyServerTest.java | 13 +++---- .../io/grpc/netty/NettyTransportTest.java | 9 ++--- .../io/grpc/okhttp/OkHttpTransportTest.java | 6 +-- .../testing/AbstractTransportTest.java | 25 +++++++++--- .../io/grpc/internal/testing/TestUtils.java | 4 +- 20 files changed, 152 insertions(+), 92 deletions(-) diff --git a/core/src/main/java/io/grpc/Server.java b/core/src/main/java/io/grpc/Server.java index a9f0f40c83..569326f229 100644 --- a/core/src/main/java/io/grpc/Server.java +++ b/core/src/main/java/io/grpc/Server.java @@ -17,6 +17,7 @@ package io.grpc; import java.io.IOException; +import java.net.SocketAddress; import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; @@ -41,8 +42,10 @@ public abstract class Server { /** * Returns the port number the server is listening on. This can return -1 if there is no actual * port or the result otherwise does not make sense. Result is undefined after the server is - * terminated. + * terminated. If there are multiple possible ports, this will return one arbitrarily. + * Implementations are encouraged to return the same port on each call. * + * @see #getListenSockets() * @throws IllegalStateException if the server has not yet been started. * @since 1.0.0 */ @@ -50,6 +53,19 @@ public abstract class Server { return -1; } + /** + * Returns a list of listening sockets for this server. May be different than the originally + * requested sockets (e.g. listening on port '0' may end up listening on a different port). + * The list is unmodifiable. + * + * @throws IllegalStateException if the server has not yet been started. + * @since 1.19.0 + */ + @ExperimentalApi("https://github.com/grpc/grpc-java/issues/FIXME") + public List getListenSockets() { + throw new UnsupportedOperationException(); + } + /** * Returns all services registered with the server, or an empty list if not supported by the * implementation. diff --git a/core/src/main/java/io/grpc/inprocess/InProcessServer.java b/core/src/main/java/io/grpc/inprocess/InProcessServer.java index b66e95b0a2..a422506f95 100644 --- a/core/src/main/java/io/grpc/inprocess/InProcessServer.java +++ b/core/src/main/java/io/grpc/inprocess/InProcessServer.java @@ -27,6 +27,7 @@ import io.grpc.internal.ObjectPool; import io.grpc.internal.ServerListener; import io.grpc.internal.ServerTransportListener; import java.io.IOException; +import java.net.SocketAddress; import java.util.Collections; import java.util.List; import java.util.concurrent.ConcurrentHashMap; @@ -77,13 +78,13 @@ final class InProcessServer implements InternalServer { } @Override - public int getPort() { - return -1; + public SocketAddress getListenSocketAddress() { + return new InProcessSocketAddress(name); } @Override - public List> getListenSockets() { - return Collections.emptyList(); + public InternalInstrumented getListenSocketStats() { + return null; } @Override diff --git a/core/src/main/java/io/grpc/internal/InternalServer.java b/core/src/main/java/io/grpc/internal/InternalServer.java index ebe62881e2..c3a1092342 100644 --- a/core/src/main/java/io/grpc/internal/InternalServer.java +++ b/core/src/main/java/io/grpc/internal/InternalServer.java @@ -19,7 +19,8 @@ package io.grpc.internal; import io.grpc.InternalChannelz.SocketStats; import io.grpc.InternalInstrumented; import java.io.IOException; -import java.util.List; +import java.net.SocketAddress; +import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; /** @@ -46,13 +47,13 @@ public interface InternalServer { void shutdown(); /** - * Returns what underlying port the server is listening on, or -1 if the port number is not - * available or does not make sense. + * Returns the listening socket address. May change after {@link start(ServerListener)} is + * called. */ - int getPort(); + SocketAddress getListenSocketAddress(); /** - * Returns the listen sockets of this server. May return an empty list but never returns null. + * Returns the listen socket stats of this server. May return {@code null}. */ - List> getListenSockets(); + @Nullable InternalInstrumented getListenSocketStats(); } diff --git a/core/src/main/java/io/grpc/internal/ServerImpl.java b/core/src/main/java/io/grpc/internal/ServerImpl.java index 260c85a80e..d08abf7b52 100644 --- a/core/src/main/java/io/grpc/internal/ServerImpl.java +++ b/core/src/main/java/io/grpc/internal/ServerImpl.java @@ -38,6 +38,7 @@ import io.grpc.DecompressorRegistry; import io.grpc.HandlerRegistry; import io.grpc.InternalChannelz; import io.grpc.InternalChannelz.ServerStats; +import io.grpc.InternalChannelz.SocketStats; import io.grpc.InternalInstrumented; import io.grpc.InternalLogId; import io.grpc.InternalServerInterceptors; @@ -51,6 +52,8 @@ import io.grpc.ServerTransportFilter; import io.grpc.Status; import java.io.IOException; import java.io.InputStream; +import java.net.InetSocketAddress; +import java.net.SocketAddress; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -136,9 +139,8 @@ public final class ServerImpl extends io.grpc.Server implements InternalInstrume Preconditions.checkNotNull(transportServers, "transportServers"); Preconditions.checkArgument(!transportServers.isEmpty(), "no servers provided"); this.transportServers = new ArrayList<>(transportServers); - // TODO(notcarl): concatenate all listening ports in the Log Id. this.logId = - InternalLogId.allocate("Server", String.valueOf(transportServers.get(0).getPort())); + InternalLogId.allocate("Server", String.valueOf(getListenSocketsIgnoringLifecycle())); // Fork from the passed in context so that it does not propagate cancellation, it only // inherits values. this.rootContext = Preconditions.checkNotNull(rootContext, "rootContext").fork(); @@ -181,21 +183,41 @@ public final class ServerImpl extends io.grpc.Server implements InternalInstrume } } + @Override public int getPort() { synchronized (lock) { checkState(started, "Not started"); checkState(!terminated, "Already terminated"); for (InternalServer ts : transportServers) { - int port = ts.getPort(); - if (port != -1) { - return port; + SocketAddress addr = ts.getListenSocketAddress(); + if (addr instanceof InetSocketAddress) { + return ((InetSocketAddress) addr).getPort(); } } return -1; } } + @Override + public List getListenSockets() { + synchronized (lock) { + checkState(started, "Not started"); + checkState(!terminated, "Already terminated"); + return getListenSocketsIgnoringLifecycle(); + } + } + + private List getListenSocketsIgnoringLifecycle() { + synchronized (lock) { + List addrs = new ArrayList<>(transportServers.size()); + for (InternalServer ts : transportServers) { + addrs.add(ts.getListenSocketAddress()); + } + return Collections.unmodifiableList(addrs); + } + } + @Override public List getServices() { List fallbackServices = fallbackRegistry.getServices(); @@ -602,7 +624,11 @@ public final class ServerImpl extends io.grpc.Server implements InternalInstrume public ListenableFuture getStats() { ServerStats.Builder builder = new ServerStats.Builder(); for (InternalServer ts : transportServers) { - builder.addListenSockets(ts.getListenSockets()); + // TODO(carl-mastrangelo): remove the list and just add directly. + InternalInstrumented stats = ts.getListenSocketStats(); + if (stats != null ) { + builder.addListenSockets(Collections.singletonList(stats)); + } } serverCallTracer.updateBuilder(builder); SettableFuture ret = SettableFuture.create(); diff --git a/core/src/test/java/io/grpc/inprocess/InProcessServerTest.java b/core/src/test/java/io/grpc/inprocess/InProcessServerTest.java index c988770541..541c4f6985 100644 --- a/core/src/test/java/io/grpc/inprocess/InProcessServerTest.java +++ b/core/src/test/java/io/grpc/inprocess/InProcessServerTest.java @@ -38,7 +38,7 @@ public class InProcessServerTest { InProcessServer s = new InProcessServer(builder, Collections.emptyList()); - Truth.assertThat(s.getPort()).isEqualTo(-1); + Truth.assertThat(s.getListenSocketAddress()).isEqualTo(new InProcessSocketAddress("name")); } @Test diff --git a/core/src/test/java/io/grpc/internal/ServerImplTest.java b/core/src/test/java/io/grpc/internal/ServerImplTest.java index 3363ea4bdd..8951f6e795 100644 --- a/core/src/test/java/io/grpc/internal/ServerImplTest.java +++ b/core/src/test/java/io/grpc/internal/ServerImplTest.java @@ -83,6 +83,7 @@ import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.net.InetSocketAddress; import java.net.SocketAddress; import java.util.Arrays; import java.util.Collections; @@ -1070,15 +1071,16 @@ public class ServerImplTest { @Test public void getPort() throws Exception { + final InetSocketAddress addr = new InetSocketAddress(65535); transportServer = new SimpleServer() { @Override - public int getPort() { - return 65535; + public SocketAddress getListenSocketAddress() { + return addr; } }; createAndStartServer(); - assertThat(server.getPort()).isEqualTo(65535); + assertThat(server.getPort()).isEqualTo(addr.getPort()); } @Test @@ -1397,13 +1399,13 @@ public class ServerImplTest { } @Override - public int getPort() { - return -1; + public SocketAddress getListenSocketAddress() { + return new InetSocketAddress(12345); } @Override - public List> getListenSockets() { - return Collections.emptyList(); + public InternalInstrumented getListenSocketStats() { + return null; } @Override diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java b/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java index 7b6b7d19e8..c1032221d9 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java @@ -246,8 +246,8 @@ public abstract class AbstractInteropTest { } @VisibleForTesting - final int getPort() { - return server.getPort(); + final SocketAddress getListenAddress() { + return server.getListenSockets().iterator().next(); } protected ManagedChannel channel; diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/AutoWindowSizingOnTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/AutoWindowSizingOnTest.java index 68b081a960..1260966674 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/AutoWindowSizingOnTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/AutoWindowSizingOnTest.java @@ -43,7 +43,7 @@ public class AutoWindowSizingOnTest extends AbstractInteropTest { @Override protected ManagedChannel createChannel() { - NettyChannelBuilder builder = NettyChannelBuilder.forAddress("localhost", getPort()) + NettyChannelBuilder builder = NettyChannelBuilder.forAddress(getListenAddress()) .negotiationType(NegotiationType.PLAINTEXT) .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE); io.grpc.internal.TestingAccessor.setStatsImplementation( diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java index 5164feabf0..dd0f4ea17f 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java @@ -62,7 +62,7 @@ public class Http2NettyTest extends AbstractInteropTest { protected ManagedChannel createChannel() { try { NettyChannelBuilder builder = NettyChannelBuilder - .forAddress(TestUtils.testServerAddress(getPort())) + .forAddress(TestUtils.testServerAddress((InetSocketAddress) getListenAddress())) .flowControlWindow(65 * 1024) .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) .sslContext(GrpcSslContexts @@ -80,18 +80,18 @@ public class Http2NettyTest extends AbstractInteropTest { } @Test - public void remoteAddr() throws Exception { + public void remoteAddr() { InetSocketAddress isa = (InetSocketAddress) obtainRemoteClientAddr(); assertEquals(InetAddress.getLoopbackAddress(), isa.getAddress()); // It should not be the same as the server - assertNotEquals(getPort(), isa.getPort()); + assertNotEquals(((InetSocketAddress) getListenAddress()).getPort(), isa.getPort()); } @Test public void localAddr() throws Exception { InetSocketAddress isa = (InetSocketAddress) obtainLocalClientAddr(); assertEquals(InetAddress.getLoopbackAddress(), isa.getAddress()); - assertEquals(getPort(), isa.getPort()); + assertEquals(((InetSocketAddress) getListenAddress()).getPort(), isa.getPort()); } @Test diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java index 0f99b001ac..4830fa9014 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java @@ -39,6 +39,7 @@ import io.netty.handler.ssl.SslContextBuilder; import io.netty.handler.ssl.SslProvider; import io.netty.handler.ssl.SupportedCipherSuiteFilter; import java.io.IOException; +import java.net.InetSocketAddress; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; @@ -91,13 +92,14 @@ public class Http2OkHttpTest extends AbstractInteropTest { } private OkHttpChannelBuilder createChannelBuilder() { - OkHttpChannelBuilder builder = OkHttpChannelBuilder.forAddress("localhost", getPort()) + int port = ((InetSocketAddress) getListenAddress()).getPort(); + OkHttpChannelBuilder builder = OkHttpChannelBuilder.forAddress("localhost", port) .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) .connectionSpec(new ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS) .cipherSuites(TestUtils.preferredTestCiphers().toArray(new String[0])) .build()) .overrideAuthority(GrpcUtil.authorityFromHostAndPort( - TestUtils.TEST_SERVER_HOST, getPort())); + TestUtils.TEST_SERVER_HOST, port)); io.grpc.internal.TestingAccessor.setStatsImplementation( builder, createClientCensusStatsModule()); try { @@ -135,9 +137,10 @@ public class Http2OkHttpTest extends AbstractInteropTest { @Test public void wrongHostNameFailHostnameVerification() throws Exception { + int port = ((InetSocketAddress) getListenAddress()).getPort(); ManagedChannel channel = createChannelBuilder() .overrideAuthority(GrpcUtil.authorityFromHostAndPort( - BAD_HOSTNAME, getPort())) + BAD_HOSTNAME, port)) .build(); TestServiceGrpc.TestServiceBlockingStub blockingStub = TestServiceGrpc.newBlockingStub(channel); @@ -157,9 +160,10 @@ public class Http2OkHttpTest extends AbstractInteropTest { @Test public void hostnameVerifierWithBadHostname() throws Exception { + int port = ((InetSocketAddress) getListenAddress()).getPort(); ManagedChannel channel = createChannelBuilder() .overrideAuthority(GrpcUtil.authorityFromHostAndPort( - BAD_HOSTNAME, getPort())) + BAD_HOSTNAME, port)) .hostnameVerifier(new HostnameVerifier() { @Override public boolean verify(String hostname, SSLSession session) { @@ -177,9 +181,10 @@ public class Http2OkHttpTest extends AbstractInteropTest { @Test public void hostnameVerifierWithCorrectHostname() throws Exception { + int port = ((InetSocketAddress) getListenAddress()).getPort(); ManagedChannel channel = createChannelBuilder() .overrideAuthority(GrpcUtil.authorityFromHostAndPort( - TestUtils.TEST_SERVER_HOST, getPort())) + TestUtils.TEST_SERVER_HOST, port)) .hostnameVerifier(new HostnameVerifier() { @Override public boolean verify(String hostname, SSLSession session) { diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/TransportCompressionTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/TransportCompressionTest.java index 955cf5d5e0..e99ba561b2 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/TransportCompressionTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/TransportCompressionTest.java @@ -123,7 +123,7 @@ public class TransportCompressionTest extends AbstractInteropTest { @Override protected ManagedChannel createChannel() { - NettyChannelBuilder builder = NettyChannelBuilder.forAddress("localhost", getPort()) + NettyChannelBuilder builder = NettyChannelBuilder.forAddress(getListenAddress()) .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) .decompressorRegistry(decompressors) .compressorRegistry(compressors) diff --git a/netty/src/main/java/io/grpc/netty/NettyServer.java b/netty/src/main/java/io/grpc/netty/NettyServer.java index 26e6d494f5..fac8f81a3c 100644 --- a/netty/src/main/java/io/grpc/netty/NettyServer.java +++ b/netty/src/main/java/io/grpc/netty/NettyServer.java @@ -23,7 +23,6 @@ import static io.netty.channel.ChannelOption.SO_KEEPALIVE; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.SettableFuture; import io.grpc.InternalChannelz; @@ -52,11 +51,11 @@ import io.netty.util.ReferenceCounted; import io.netty.util.concurrent.Future; import io.netty.util.concurrent.GenericFutureListener; import java.io.IOException; -import java.net.InetSocketAddress; import java.net.SocketAddress; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -94,9 +93,8 @@ class NettyServer implements InternalServer, InternalWithLogId { private final TransportTracer.Factory transportTracerFactory; private final InternalChannelz channelz; // Only modified in event loop but safe to read any time. Set at startup and unset at shutdown. - // In the future we may have >1 listen socket. - private volatile ImmutableList> listenSockets - = ImmutableList.of(); + private final AtomicReference> listenSocketStats = + new AtomicReference<>(); NettyServer( SocketAddress address, Class channelType, @@ -139,20 +137,17 @@ class NettyServer implements InternalServer, InternalWithLogId { } @Override - public int getPort() { + public SocketAddress getListenSocketAddress() { if (channel == null) { - return -1; + // server is not listening/bound yet, just return the original port. + return address; } - SocketAddress localAddr = channel.localAddress(); - if (!(localAddr instanceof InetSocketAddress)) { - return -1; - } - return ((InetSocketAddress) localAddr).getPort(); + return channel.localAddress(); } @Override - public List> getListenSockets() { - return listenSockets; + public InternalInstrumented getListenSocketStats() { + return listenSocketStats.get(); } @Override @@ -260,7 +255,7 @@ class NettyServer implements InternalServer, InternalWithLogId { @Override public void run() { InternalInstrumented listenSocket = new ListenSocket(channel); - listenSockets = ImmutableList.of(listenSocket); + listenSocketStats.set(listenSocket); channelz.addListenSocket(listenSocket); } }); @@ -283,10 +278,10 @@ class NettyServer implements InternalServer, InternalWithLogId { if (!future.isSuccess()) { log.log(Level.WARNING, "Error shutting down server", future.cause()); } - for (InternalInstrumented listenSocket : listenSockets) { - channelz.removeListenSocket(listenSocket); + InternalInstrumented stats = listenSocketStats.getAndSet(null); + if (stats != null) { + channelz.removeListenSocket(stats); } - listenSockets = null; synchronized (NettyServer.this) { listener.serverShutdown(); } diff --git a/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java b/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java index 51ccc2a5c2..6a694ad132 100644 --- a/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java +++ b/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java @@ -17,12 +17,12 @@ package io.grpc.netty; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; import static io.grpc.internal.GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE; import static io.grpc.internal.GrpcUtil.DEFAULT_SERVER_KEEPALIVE_TIMEOUT_NANOS; import static io.grpc.internal.GrpcUtil.DEFAULT_SERVER_KEEPALIVE_TIME_NANOS; import static io.grpc.internal.GrpcUtil.SERVER_KEEPALIVE_TIME_NANOS_DISABLED; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.grpc.ExperimentalApi; @@ -121,11 +121,13 @@ public final class NettyServerBuilder extends AbstractServerImplBuilder(CantConstructChannel.class), @@ -539,7 +539,7 @@ public class NettyClientTransportTest { @Test public void getAttributes_negotiatorHandler() throws Exception { - address = TestUtils.testServerAddress(12345); + address = TestUtils.testServerAddress(new InetSocketAddress(12345)); authority = GrpcUtil.authorityFromHostAndPort(address.getHostString(), address.getPort()); NettyClientTransport transport = newTransport(new NoopProtocolNegotiator()); @@ -550,7 +550,7 @@ public class NettyClientTransportTest { @Test public void getEagAttributes_negotiatorHandler() throws Exception { - address = TestUtils.testServerAddress(12345); + address = TestUtils.testServerAddress(new InetSocketAddress(12345)); authority = GrpcUtil.authorityFromHostAndPort(address.getHostString(), address.getPort()); NoopProtocolNegotiator npn = new NoopProtocolNegotiator(); @@ -654,7 +654,7 @@ public class NettyClientTransportTest { private void startServer(int maxStreamsPerConnection, int maxHeaderListSize) throws IOException { server = new NettyServer( - TestUtils.testServerAddress(0), + TestUtils.testServerAddress(new InetSocketAddress(0)), NioServerSocketChannel.class, new HashMap, Object>(), group, group, negotiator, @@ -667,7 +667,7 @@ public class NettyClientTransportTest { MAX_CONNECTION_AGE_NANOS_DISABLED, MAX_CONNECTION_AGE_GRACE_NANOS_INFINITE, true, 0, channelz); server.start(serverListener); - address = TestUtils.testServerAddress(server.getPort()); + address = TestUtils.testServerAddress((InetSocketAddress) server.getListenSocketAddress()); authority = GrpcUtil.authorityFromHostAndPort(address.getHostString(), address.getPort()); } diff --git a/netty/src/test/java/io/grpc/netty/NettyServerBuilderTest.java b/netty/src/test/java/io/grpc/netty/NettyServerBuilderTest.java index cf5f1d6b59..d5e906a0a8 100644 --- a/netty/src/test/java/io/grpc/netty/NettyServerBuilderTest.java +++ b/netty/src/test/java/io/grpc/netty/NettyServerBuilderTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.truth.Truth; import io.grpc.ServerStreamTracer.Factory; import io.netty.handler.ssl.SslContext; +import java.net.InetSocketAddress; import java.util.List; import java.util.concurrent.TimeUnit; import org.junit.Rule; @@ -43,7 +44,7 @@ public class NettyServerBuilderTest { @Test public void createMultipleServers() { - builder.addPort(8081); + builder.addListenAddress(new InetSocketAddress(8081)); List servers = builder.buildTransportServers(ImmutableList.of()); Truth.assertThat(servers).hasSize(2); diff --git a/netty/src/test/java/io/grpc/netty/NettyServerTest.java b/netty/src/test/java/io/grpc/netty/NettyServerTest.java index 28d31c2436..cc1880cdac 100644 --- a/netty/src/test/java/io/grpc/netty/NettyServerTest.java +++ b/netty/src/test/java/io/grpc/netty/NettyServerTest.java @@ -16,7 +16,6 @@ package io.grpc.netty; -import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.common.truth.Truth.assertThat; import static io.grpc.InternalChannelz.id; import static org.junit.Assert.assertEquals; @@ -86,7 +85,7 @@ public class NettyServerTest { }); // Check that we got an actual port. - assertThat(ns.getPort()).isGreaterThan(0); + assertThat(((InetSocketAddress) ns.getListenSocketAddress()).getPort()).isGreaterThan(0); // Cleanup ns.shutdown(); @@ -114,7 +113,7 @@ public class NettyServerTest { true, 0, // ignore channelz); - assertThat(ns.getPort()).isEqualTo(-1); + assertThat(ns.getListenSocketAddress()).isEqualTo(addr); } @Test(timeout = 60000) @@ -170,7 +169,7 @@ public class NettyServerTest { }); Socket socket = new Socket(); - socket.connect(new InetSocketAddress("localhost", ns.getPort()), /* timeout= */ 8000); + socket.connect(ns.getListenSocketAddress(), /* timeout= */ 8000); countDownLatch.await(); socket.close(); @@ -213,14 +212,14 @@ public class NettyServerTest { shutdownCompleted.set(null); } }); - assertThat(ns.getPort()).isGreaterThan(0); + assertThat(((InetSocketAddress) ns.getListenSocketAddress()).getPort()).isGreaterThan(0); - InternalInstrumented listenSocket = getOnlyElement(ns.getListenSockets()); + InternalInstrumented listenSocket = ns.getListenSocketStats(); assertSame(listenSocket, channelz.getSocket(id(listenSocket))); // very basic sanity check of the contents SocketStats socketStats = listenSocket.getStats().get(); - assertEquals(ns.getPort(), ((InetSocketAddress) socketStats.local).getPort()); + assertEquals(ns.getListenSocketAddress(), socketStats.local); assertNull(socketStats.remote); // TODO(zpencer): uncomment when sock options are exposed diff --git a/netty/src/test/java/io/grpc/netty/NettyTransportTest.java b/netty/src/test/java/io/grpc/netty/NettyTransportTest.java index 798bd71730..5e24ca6384 100644 --- a/netty/src/test/java/io/grpc/netty/NettyTransportTest.java +++ b/netty/src/test/java/io/grpc/netty/NettyTransportTest.java @@ -58,7 +58,7 @@ public class NettyTransportTest extends AbstractTransportTest { protected List newServer( List streamTracerFactories) { return NettyServerBuilder - .forPort(0) + .forAddress(new InetSocketAddress("localhost", 0)) .flowControlWindow(65 * 1024) .setTransportTracerFactory(fakeClockTransportTracer) .buildTransportServers(streamTracerFactories); @@ -68,7 +68,7 @@ public class NettyTransportTest extends AbstractTransportTest { protected List newServer( int port, List streamTracerFactories) { return NettyServerBuilder - .forPort(port) + .forAddress(new InetSocketAddress("localhost", port)) .flowControlWindow(65 * 1024) .setTransportTracerFactory(fakeClockTransportTracer) .buildTransportServers(streamTracerFactories); @@ -76,7 +76,7 @@ public class NettyTransportTest extends AbstractTransportTest { @Override protected String testAuthority(InternalServer server) { - return "localhost:" + server.getPort(); + return "localhost:" + server.getListenSocketAddress(); } @Override @@ -91,9 +91,8 @@ public class NettyTransportTest extends AbstractTransportTest { @Override protected ManagedClientTransport newClientTransport(InternalServer server) { - int port = server.getPort(); return clientFactory.newClientTransport( - new InetSocketAddress("localhost", port), + server.getListenSocketAddress(), new ClientTransportFactory.ClientTransportOptions() .setAuthority(testAuthority(server))); } diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java index ab7b704495..f7e4589b29 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java @@ -66,7 +66,7 @@ public class OkHttpTransportTest extends AbstractTransportTest { int port, List streamTracerFactories) { return AccessProtectedHack.serverBuilderBuildTransportServer( NettyServerBuilder - .forPort(port) + .forAddress(new InetSocketAddress(port)) .flowControlWindow(65 * 1024), streamTracerFactories, fakeClockTransportTracer); @@ -74,12 +74,12 @@ public class OkHttpTransportTest extends AbstractTransportTest { @Override protected String testAuthority(InternalServer server) { - return "thebestauthority:" + server.getPort(); + return "thebestauthority:" + server.getListenSocketAddress(); } @Override protected ManagedClientTransport newClientTransport(InternalServer server) { - int port = server.getPort(); + int port = ((InetSocketAddress) server.getListenSocketAddress()).getPort(); return clientFactory.newClientTransport( new InetSocketAddress("localhost", port), new ClientTransportFactory.ClientTransportOptions() diff --git a/testing/src/main/java/io/grpc/internal/testing/AbstractTransportTest.java b/testing/src/main/java/io/grpc/internal/testing/AbstractTransportTest.java index d7ef06448a..2d450494d6 100644 --- a/testing/src/main/java/io/grpc/internal/testing/AbstractTransportTest.java +++ b/testing/src/main/java/io/grpc/internal/testing/AbstractTransportTest.java @@ -74,6 +74,7 @@ import io.grpc.internal.TransportTracer; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.net.InetSocketAddress; import java.net.SocketAddress; import java.util.Arrays; import java.util.List; @@ -364,7 +365,11 @@ public abstract class AbstractTransportTest { public void serverAlreadyListening() throws Exception { client = null; server.start(serverListener); - int port = server.getPort(); + int port = -1; + SocketAddress addr = server.getListenSocketAddress(); + if (addr instanceof InetSocketAddress) { + port = ((InetSocketAddress) addr).getPort(); + } InternalServer server2 = Iterables.getOnlyElement(newServer(port, Arrays.asList(serverStreamTracerFactory))); thrown.expect(IOException.class); @@ -374,7 +379,11 @@ public abstract class AbstractTransportTest { @Test public void openStreamPreventsTermination() throws Exception { server.start(serverListener); - int port = server.getPort(); + int port = -1; + SocketAddress addr = server.getListenSocketAddress(); + if (addr instanceof InetSocketAddress) { + port = ((InetSocketAddress) addr).getPort(); + } client = newClientTransport(server); startTransport(client, mockClientTransportListener); MockServerTransportListener serverTransportListener @@ -1800,15 +1809,19 @@ public abstract class AbstractTransportTest { SocketAddress clientAddress = serverStream.getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR); SocketStats clientSocketStats = client.getStats().get(); - assertEquals(clientAddress, clientSocketStats.local); - assertEquals(serverAddress, clientSocketStats.remote); + assertEquals( + "clientLocal " + clientStream.getAttributes(), clientAddress, clientSocketStats.local); + assertEquals( + "clientRemote " + clientStream.getAttributes(), serverAddress, clientSocketStats.remote); // very basic sanity check that socket options are populated assertNotNull(clientSocketStats.socketOptions.lingerSeconds); assertTrue(clientSocketStats.socketOptions.others.containsKey("SO_SNDBUF")); SocketStats serverSocketStats = serverTransportListener.transport.getStats().get(); - assertEquals(serverAddress, serverSocketStats.local); - assertEquals(clientAddress, serverSocketStats.remote); + assertEquals( + "serverLocal " + serverStream.getAttributes(), serverAddress, serverSocketStats.local); + assertEquals( + "serverRemote " + serverStream.getAttributes(), clientAddress, serverSocketStats.remote); // very basic sanity check that socket options are populated assertNotNull(serverSocketStats.socketOptions.lingerSeconds); assertTrue(serverSocketStats.socketOptions.others.containsKey("SO_SNDBUF")); diff --git a/testing/src/main/java/io/grpc/internal/testing/TestUtils.java b/testing/src/main/java/io/grpc/internal/testing/TestUtils.java index dff8cc5d49..470281595a 100644 --- a/testing/src/main/java/io/grpc/internal/testing/TestUtils.java +++ b/testing/src/main/java/io/grpc/internal/testing/TestUtils.java @@ -67,11 +67,11 @@ public class TestUtils { * Creates a new {@link InetSocketAddress} on localhost that overrides the host with * {@link #TEST_SERVER_HOST}. */ - public static InetSocketAddress testServerAddress(int port) { + public static InetSocketAddress testServerAddress(InetSocketAddress originalSockAddr) { try { InetAddress inetAddress = InetAddress.getByName("localhost"); inetAddress = InetAddress.getByAddress(TEST_SERVER_HOST, inetAddress.getAddress()); - return new InetSocketAddress(inetAddress, port); + return new InetSocketAddress(inetAddress, originalSockAddr.getPort()); } catch (UnknownHostException e) { throw new RuntimeException(e); }