Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve testing gRPC services with random ports #34135

Merged
merged 1 commit into from
Jun 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.quarkus.grpc.runtime;

import static io.quarkus.grpc.runtime.GrpcSslUtils.applySslOptions;
import static io.quarkus.grpc.runtime.GrpcTestPortUtils.testPort;

import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -57,6 +58,7 @@
import io.quarkus.runtime.RuntimeValue;
import io.quarkus.runtime.ShutdownContext;
import io.quarkus.runtime.annotations.Recorder;
import io.quarkus.vertx.http.runtime.PortSystemProperties;
import io.vertx.core.AbstractVerticle;
import io.vertx.core.AsyncResult;
import io.vertx.core.DeploymentOptions;
Expand Down Expand Up @@ -226,7 +228,7 @@ private void prodStart(GrpcContainer grpcContainer, Vertx vertx, GrpcServerConfi

private void postStartup(GrpcServerConfiguration configuration, GrpcBuilderProvider<?> provider, boolean test) {
initHealthStorage();
int port = test ? configuration.testPort : configuration.port;
int port = test ? testPort(configuration) : configuration.port;
Copy link
Contributor

Choose a reason for hiding this comment

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

Well, this was easy. :-)
Are we sure we captured all such log outputs -- with this "new" port?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's an output in buildServer but that's before the server is started - so before the port is chosen.

String msg = "Started ";
if (provider != null)
msg += provider.serverInfo(configuration.host, port, configuration);
Expand Down Expand Up @@ -555,6 +557,7 @@ private class GrpcServerVerticle extends AbstractVerticle {
private final GrpcBuilderProvider provider;
private final LaunchMode launchMode;
private final Map<String, List<String>> blockingMethodsPerService;
private volatile PortSystemProperties portSystemProperties;

private Server grpcServer;

Expand Down Expand Up @@ -591,6 +594,11 @@ public void start(Promise<Void> startPromise) {
}
startPromise.fail(effectiveCause);
} else {
int actualPort = grpcServer.getPort();
if (actualPort != portToServer.getKey()) {
portSystemProperties = new PortSystemProperties();
portSystemProperties.set("grpc.server", actualPort, launchMode);
}
startPromise.complete();
grpcVerticleCount.incrementAndGet();
}
Expand All @@ -600,6 +608,11 @@ public void start(Promise<Void> startPromise) {
vertx.executeBlocking((Handler<Promise<Void>>) event -> {
try {
grpcServer.start();
int actualPort = grpcServer.getPort();
if (actualPort != portToServer.getKey()) {
portSystemProperties = new PortSystemProperties();
portSystemProperties.set("grpc.server", actualPort, launchMode);
}
startPromise.complete();
} catch (Exception e) {
LOGGER.error("Unable to start gRPC server", e);
Expand All @@ -623,6 +636,9 @@ public void stop(Promise<Void> stopPromise) {
stopPromise.complete();
grpcVerticleCount.decrementAndGet();
}
if (portSystemProperties != null) {
portSystemProperties.restore();
}
});
} else {
try {
Expand All @@ -635,6 +651,10 @@ public void stop(Promise<Void> stopPromise) {
} catch (Exception e) {
LOGGER.errorf(e, "Unable to stop the gRPC server gracefully");
stopPromise.fail(e);
} finally {
if (portSystemProperties != null) {
portSystemProperties.restore();
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package io.quarkus.grpc.runtime;

import java.util.List;

import org.eclipse.microprofile.config.ConfigProvider;

import io.quarkus.grpc.runtime.config.GrpcServerConfiguration;
import io.quarkus.grpc.runtime.config.SslServerConfig;
import io.vertx.core.http.ClientAuth;

public final class GrpcTestPortUtils {
private GrpcTestPortUtils() {
}

public static int testPort(GrpcServerConfiguration serverConfiguration) {
if (serverConfiguration.useSeparateServer) {
if (serverConfiguration.testPort == 0) {
return testPort("grpc.server");
}
return serverConfiguration.testPort;
}
if (isHttpsConfigured(serverConfiguration.ssl) || !serverConfiguration.plainText) {
int httpsTestPort = port("quarkus.http.test-ssl-port");
if (httpsTestPort == 0) {
return testPort("https");
}
return httpsTestPort;
}
return testPort("http");
}

private static boolean isHttpsConfigured(SslServerConfig ssl) {
return ssl.certificate.isPresent() || ssl.key.isPresent() || ssl.keyStore.isPresent()
|| ssl.keyStoreType.isPresent() || ssl.keyStorePassword.isPresent() || ssl.trustStore.isPresent()
|| ssl.trustStoreType.isPresent() || ssl.cipherSuites.isPresent() || ssl.clientAuth != ClientAuth.NONE
|| !isDefaultProtocols(ssl.protocols);
}

private static boolean isDefaultProtocols(List<String> protocols) {
return protocols.size() == 2 && protocols.contains("TLSv1.3") && protocols.contains("TLSv1.2");
}

private static int testPort(String subProperty) {
return port("quarkus." + subProperty + ".test-port");
}

private static int port(String property) {
return ConfigProvider.getConfig().getValue(property, Integer.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static io.grpc.internal.GrpcUtil.DEFAULT_MAX_HEADER_LIST_SIZE;
import static io.grpc.internal.GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE;
import static io.grpc.netty.NettyChannelBuilder.DEFAULT_FLOW_CONTROL_WINDOW;
import static io.quarkus.grpc.runtime.GrpcTestPortUtils.testPort;
import static io.quarkus.grpc.runtime.config.GrpcClientConfiguration.DNS;

import java.io.IOException;
Expand Down Expand Up @@ -101,6 +102,10 @@ public static Channel createChannel(String name, Set<String> perClientIntercepto
throw new IllegalStateException("gRPC client " + name + " is missing configuration.");
}

if (LaunchMode.current() == LaunchMode.TEST) {
config.port = testPort(configProvider.getServerConfiguration());
}

GrpcBuilderProvider provider = GrpcBuilderProvider.findChannelBuilderProvider(config);

boolean vertxGrpc = config.useQuarkusGrpcClient;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package io.quarkus.vertx.http.runtime;

import java.util.HashMap;
import java.util.Map;
import java.util.Objects;

import io.quarkus.runtime.LaunchMode;

public class PortSystemProperties {
private final Map<String, String> portPropertiesToRestore = new HashMap<>();

public void set(String subProperty, int actualPort, LaunchMode launchMode) {
String portPropertyValue = String.valueOf(actualPort);
//we always set the .port property, even if we are in test mode, so this will always
//reflect the current port
String portPropertyName = "quarkus." + subProperty + ".port";
String prevPortPropertyValue = System.setProperty(portPropertyName, portPropertyValue);
if (!Objects.equals(prevPortPropertyValue, portPropertyValue)) {
portPropertiesToRestore.put(portPropertyName, prevPortPropertyValue);
}
if (launchMode == LaunchMode.TEST) {
//we also set the test-port property in a test
String testPropName = "quarkus." + subProperty + ".test-port";
String prevTestPropPrevValue = System.setProperty(testPropName, portPropertyValue);
if (!Objects.equals(prevTestPropPrevValue, portPropertyValue)) {
portPropertiesToRestore.put(testPropName, prevTestPropPrevValue);
}
}
if (launchMode.isDevOrTest()) {
// set the profile property as well to make sure we don't have any inconsistencies
portPropertyName = "%" + launchMode.getDefaultProfile() + "." + portPropertyName;
prevPortPropertyValue = System.setProperty(portPropertyName, portPropertyValue);
if (!Objects.equals(prevPortPropertyValue, portPropertyValue)) {
portPropertiesToRestore.put(portPropertyName, prevPortPropertyValue);
}
}
}

public void restore() {
portPropertiesToRestore.forEach((key, value) -> {
if (value == null) {
System.clearProperty(key);
} else {
System.setProperty(key, value);
}
});
portPropertiesToRestore.clear();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,8 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.TreeMap;
import java.util.concurrent.*;
Expand Down Expand Up @@ -1023,7 +1021,7 @@ private static class WebDeploymentVerticle extends AbstractVerticle implements R
private final LaunchMode launchMode;
private volatile boolean clearHttpProperty = false;
private volatile boolean clearHttpsProperty = false;
private volatile Map<String, String> portPropertiesToRestore;
private volatile PortSystemProperties portSystemProperties;
private final HttpConfiguration.InsecureRequests insecureRequests;
private final HttpConfiguration quarkusConfig;
private final AtomicInteger connectionCount;
Expand Down Expand Up @@ -1188,31 +1186,8 @@ public void handle(AsyncResult<HttpServer> event) {
actualHttpPort = actualPort;
schema = "http";
}
portPropertiesToRestore = new HashMap<>();
String portPropertyValue = String.valueOf(actualPort);
//we always set the .port property, even if we are in test mode, so this will always
//reflect the current port
String portPropertyName = "quarkus." + schema + ".port";
String prevPortPropertyValue = System.setProperty(portPropertyName, portPropertyValue);
if (!Objects.equals(prevPortPropertyValue, portPropertyValue)) {
portPropertiesToRestore.put(portPropertyName, prevPortPropertyValue);
}
if (launchMode == LaunchMode.TEST) {
//we also set the test-port property in a test
String testPropName = "quarkus." + schema + ".test-port";
nahguam marked this conversation as resolved.
Show resolved Hide resolved
String prevTestPropPrevValue = System.setProperty(testPropName, portPropertyValue);
if (!Objects.equals(prevTestPropPrevValue, portPropertyValue)) {
portPropertiesToRestore.put(testPropName, prevTestPropPrevValue);
}
}
if (launchMode.isDevOrTest()) {
// set the profile property as well to make sure we don't have any inconsistencies
portPropertyName = propertyWithProfilePrefix(portPropertyName);
prevPortPropertyValue = System.setProperty(portPropertyName, portPropertyValue);
if (!Objects.equals(prevPortPropertyValue, portPropertyValue)) {
portPropertiesToRestore.put(portPropertyName, prevPortPropertyValue);
}
}
portSystemProperties = new PortSystemProperties();
portSystemProperties.set(schema, actualPort, launchMode);
}
startFuture.complete(null);
}
Expand Down Expand Up @@ -1256,14 +1231,8 @@ public void stop(Promise<Void> stopFuture) {
System.clearProperty(propertyWithProfilePrefix(portPropertyName));
}
}
if (portPropertiesToRestore != null) {
for (Map.Entry<String, String> entry : portPropertiesToRestore.entrySet()) {
if (entry.getValue() == null) {
System.clearProperty(entry.getKey());
} else {
System.setProperty(entry.getKey(), entry.getValue());
}
}
if (portSystemProperties != null) {
portSystemProperties.restore();
}

stopFuture.complete();
Expand Down
65 changes: 65 additions & 0 deletions integration-tests/grpc-test-random-port/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<parent>
<artifactId>quarkus-integration-tests-parent</artifactId>
<groupId>io.quarkus</groupId>
<version>999-SNAPSHOT</version>
</parent>

<artifactId>quarkus-integration-test-grpc-test-random-port</artifactId>
<name>Quarkus - Integration Tests - gRPC - Test Random Port</name>

<dependencies>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-grpc</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-junit5</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>

<!-- Minimal test dependencies to *-deployment artifacts for consistent build order -->
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-grpc-deployment</artifactId>
<version>${project.version}</version>
<type>pom</type>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-maven-plugin</artifactId>
<executions>
<execution>
<goals>
<goal>generate-code</goal>
<goal>build</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package io.quarkus.grpc.examples.hello;

import java.util.concurrent.atomic.AtomicInteger;

import examples.HelloReply;
import examples.HelloRequest;
import examples.MutinyGreeterGrpc;
import io.quarkus.grpc.GrpcService;
import io.smallrye.mutiny.Uni;

@GrpcService
public class HelloWorldService extends MutinyGreeterGrpc.GreeterImplBase {

AtomicInteger counter = new AtomicInteger();

@Override
public Uni<HelloReply> sayHello(HelloRequest request) {
int count = counter.incrementAndGet();
String name = request.getName();
return Uni.createFrom().item("Hello " + name)
.map(res -> HelloReply.newBuilder().setMessage(res).setCount(count).build());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@

syntax = "proto2";

option java_multiple_files = true;
option java_package = "examples";
option java_outer_classname = "HelloWorldProto";
option objc_class_prefix = "HLW";

package helloworld;

service Greeter {
rpc SayHello (HelloRequest) returns (HelloReply) {}
}

message HelloRequest {
required string name = 1;
}

message HelloReply {
required string message = 1;
optional int32 count = 2;
}
Loading