Add gRPC reverse proxy server example by eottabom · Pull Request #5722 · line/armeria

eottabom · 2024-06-04T05:37:50Z

Motivation:

Add gRPC reverse proxy server example

Modifications:

Add gRPC reverse proxy server example with testcontainers.

Result:

Closes Add gRPC reverse proxy server example #2353 .

CLAassistant · 2024-06-04T05:37:55Z

All committers have signed the CLA.

github-actions · 2024-06-04T06:03:19Z

🔍 Build Scan® (commit: `0c8ecb4`)

Job name	Status	Build Scan®
build-windows-latest-jdk-21	✅	https://ge.armeria.dev/s/cl6pxeh6pzi2a
build-self-hosted-unsafe-jdk-8	✅	https://ge.armeria.dev/s/r2gsxbrrvhw2c
build-self-hosted-unsafe-jdk-21-snapshot-blockhound	✅	https://ge.armeria.dev/s/l3wmgpewtgzh2
build-self-hosted-unsafe-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/sf4jpb56eq4ec
build-self-hosted-unsafe-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/senoeher2ztfm
build-self-hosted-unsafe-jdk-17-leak	✅	https://ge.armeria.dev/s/ovnvnpjdy3kx4
build-self-hosted-unsafe-jdk-11	✅	https://ge.armeria.dev/s/s6keqs42qlb5e
build-macos-12-jdk-21	❌ (failure)	https://ge.armeria.dev/s/bvai7u6u62aqc

- style: Apply checkStyle

- fix typo

trustin · 2024-06-05T15:48:23Z

Could you check the build failures and make sure the tests are not flaky?

- fix: Modify armeria server port

trustin · 2024-06-05T16:25:56Z

    testImplementation libs.awaitility
    testImplementation libs.junit5.jupiter.api
    testImplementation libs.protobuf.java.util
+    testImplementation libs.testcontainers.junit.jupiter


Please create a separate project for the example you're writing. Don't make an example do more than one thing.

Well, am I right to understand?
Are you telling me to create and use a grpc-reverse-proxy project in the sample directory?

trustin · 2024-06-05T16:26:45Z

+    @BeforeAll
+    static void startServer() {
+        server = Server.builder()
+                .http(18080)


Please use an ephemeral port to avoid port collision

trustin · 2024-06-05T16:27:20Z

+
+    @Container
+    private static GenericContainer<?> envoy = new GenericContainer<>("envoyproxy/envoy:v1.22.0")
+            .withExposedPorts(9090)


Please use a random unused port to avoid port collision as much as possible. Use PortUtil.unusedTcpPort.

- Remove reverse proxy in grpc project - Add reverse proxy project - Add reverse proxy example

- Apply checkstyle

eottabom · 2024-06-10T04:24:23Z

It does not appear to be a problem caused by overlapping ports.
Let me find the cause again.

- Modify envoy start, stop async

- Modify timeout

- Modify envoy start, stop

eottabom · 2024-06-10T07:48:40Z

build-self-hosted-unsafe-jdk-21-snapshot-blockhound

I don't think there's a docker on the ci server

java.util.concurrent.CompletionException: java.lang.IllegalStateException: Could not find a valid Docker environment. Please see logs and check configuration

- Add try/catch CompletionException

- Apply checkstyle

trustin · 2024-06-10T15:31:41Z

+    private static final int serverPort = PortUtil.unusedTcpPort();
+    private static final int backendServerPort = PortUtil.unusedTcpPort();


In an example, we should use the fixed ports. I'd expect tests use unused random ports though.

I modified it to use a fixed port.

9424ddb

trustin · 2024-06-10T15:32:05Z

+    @BeforeAll
+    static void startServer() {
+        server = Server.builder()
+                .http(serverPort)


Specify 0 and let the OS choose the port number?

I modified it to use port 0.
ce6ab18

- test: Using server port 0

- fix: Modify default port

- test: Apply checkstyle

jrhee17 · 2024-06-14T06:09:05Z

Pushed some small commits 🙇 Let me know if any changes don't make sense

jrhee17

👍 👍 👍

minwoox

Looks great, thanks!
Could you also run this script to see if everything works correctly?
https://github.com/line/armeria-examples/blob/main/update-examples.sh
You might run this command: (I supposed armeria and armeria-examples are in the same parent directory)
./update-examples.sh 1.29.0 ../armeria

minwoox · 2024-06-19T06:57:35Z

+class GrpcEnvoyProxyTest {
+
+    // the port envoy binds to within the container
+    private static final int envoyPort = 10000;


nit:

Suggested change

private static final int envoyPort = 10000;

private static final int ENVOY_PORT = 10000;

minwoox · 2024-06-19T07:01:25Z

+    private static Server startBackendServer(int serverPort) {
+        return Server.builder()
+                .http(serverPort)
+                .service(GrpcService.builder()
+                        .addService(new HelloService())
+                        .build())
+                .build();
+    }


nit: style

Suggested change

private static Server startBackendServer(int serverPort) {

return Server.builder()

.http(serverPort)

.service(GrpcService.builder()

.addService(new HelloService())

.build())

.build();

}

private static Server startBackendServer(int serverPort) {

return Server.builder()

.http(serverPort)

.service(GrpcService.builder()

.addService(new HelloService())

.build())

.build();

}

- Apply code style - Add dependencies - Modify variable name

- Modify variable name

minwoox · 2024-06-19T08:03:47Z

                  address:
                    socket_address:
-                      address: host.testcontainers.internal
+                      address: host.docker.internal


host.docker.internal doesn't work on Linux. I think we should revert this change.
See envoyproxy/java-control-plane#303

I was merging because of a crash, but I forgot the file. I fixed it!

- Change host

eottabom · 2024-06-19T08:21:34Z

Looks great, thanks! Could you also run this script to see if everything works correctly? https://github.com/line/armeria-examples/blob/main/update-examples.sh You might run this command: (I supposed armeria and armeria-examples are in the same parent directory) ./update-examples.sh 1.29.0 ../armeria

This script is not working properly.

The following error occurs
Is there a dependency I need to add?

minwoox · 2024-06-19T08:35:12Z

This script is not working properly.

Let me see what happened. 😉

minwoox · 2024-06-19T09:41:47Z

Let me see what happened. 😉

It works correctly. The problem was just that we needed to convert the libs.testcontainers.junit.jupiter to the actual names in the script like:
-pe "s/libs.testcontainers.junit.jupiter/'org.testcontainers:junit-jupiter:$TESTCONTAINER_VERSION'/g;" \

It will be handled when we release the next version so you don't have to worry about it. 😉

minwoox

Thanks a lot, @eottabom! 😄

eottabom · 2024-06-19T09:56:33Z

Rather, I think learned a lot. Thank you. 😄

trustin

👍 🙇

ikhoon

Looks a useful example! 🙇‍♂️🙇‍♂️

eottabom requested review from ikhoon, jrhee17, minwoox and trustin as code owners June 4, 2024 05:37

eottabom force-pushed the feature/ISSUE-2353 branch 2 times, most recently from 387a425 to effa17f Compare June 4, 2024 06:33

Add gRPC reverse proxy server example

9f5e044

eottabom force-pushed the feature/ISSUE-2353 branch from effa17f to 8b01b1a Compare June 4, 2024 06:36

eottabom added 2 commits June 4, 2024 15:37

Add gRPC reverse proxy server example

4cacfc1

- style: Apply checkStyle

Add gRPC reverse proxy server example

774b34f

- fix typo

eottabom force-pushed the feature/ISSUE-2353 branch from 8b01b1a to 774b34f Compare June 4, 2024 06:38

Add gRPC reverse proxy server example

8f22629

- fix: Modify armeria server port

trustin reviewed Jun 5, 2024

View reviewed changes

eottabom added 2 commits June 10, 2024 12:24

ISSUE-2353: Add reverse proxy example

61c3429

- Remove reverse proxy in grpc project - Add reverse proxy project - Add reverse proxy example

ISSUE-2353: Add reverse proxy example

b4abca8

- Apply checkstyle

eottabom added 3 commits June 10, 2024 15:19

ISSUE-2353: Add reverse proxy example

00fa962

- Modify envoy start, stop async

ISSUE-2353: Add reverse proxy example

5759a6c

- Modify timeout

ISSUE-2353: Add reverse proxy example

7016aaa

- Modify envoy start, stop

eottabom added 2 commits June 10, 2024 17:05

ISSUE-2353: Add reverse proxy example

cd3d8e0

- Add try/catch CompletionException

ISSUE-2353: Add reverse proxy example

b1efcca

- Apply checkstyle

trustin reviewed Jun 10, 2024

View reviewed changes

eottabom added 3 commits June 11, 2024 10:08

ISSUE-2353: Add reverse proxy example

ce6ab18

- test: Using server port 0

ISSUE-2353: Add reverse proxy example

9424ddb

- fix: Modify default port

ISSUE-2353: Add reverse proxy example

4946a11

- test: Apply checkstyle

jrhee17 added 3 commits June 14, 2024 14:52

minor updates

0dd24d8

test for multiple protocols

958c0f6

remove unnecessary dependencies

4ff7116

jrhee17 added the documentation label Jun 14, 2024

jrhee17 added this to the 1.30.0 milestone Jun 14, 2024

jrhee17 added 3 commits June 14, 2024 16:57

handle flakiness

b3fd200

retry for http status unavailable

39322a3

use testcontainers host, open the host port preemptively

70a7c66

jrhee17 approved these changes Jun 14, 2024

View reviewed changes

minwoox reviewed Jun 19, 2024

View reviewed changes

eottabom added 2 commits June 19, 2024 16:15

ISSUE-2353: Add reverse proxy example

f2569dc

- Apply code style - Add dependencies - Modify variable name

ISSUE-2353: Add reverse proxy example

e1332e6

- Modify variable name

minwoox reviewed Jun 19, 2024

View reviewed changes

ISSUE-2353: Add reverse proxy example

0c8ecb4

- Change host

minwoox approved these changes Jun 19, 2024

View reviewed changes

trustin approved these changes Jun 20, 2024

View reviewed changes

ikhoon approved these changes Jun 26, 2024

View reviewed changes

ikhoon merged commit f0ec1ba into line:main Jun 26, 2024

		private static final int serverPort = PortUtil.unusedTcpPort();
		private static final int backendServerPort = PortUtil.unusedTcpPort();

	private static final int envoyPort = 10000;
	private static final int ENVOY_PORT = 10000;

Conversation

eottabom commented Jun 4, 2024

Uh oh!

CLAassistant commented Jun 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔍 Build Scan® (commit: 0c8ecb4)

Uh oh!

trustin commented Jun 5, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eottabom commented Jun 10, 2024

Uh oh!

eottabom commented Jun 10, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrhee17 commented Jun 14, 2024

Uh oh!

jrhee17 left a comment

Choose a reason for hiding this comment

Uh oh!

minwoox left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eottabom commented Jun 19, 2024

Uh oh!

minwoox commented Jun 19, 2024

Uh oh!

minwoox commented Jun 19, 2024

Uh oh!

minwoox left a comment

Choose a reason for hiding this comment

Uh oh!

eottabom commented Jun 19, 2024

Uh oh!

trustin left a comment

Choose a reason for hiding this comment

Uh oh!

ikhoon left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

CLAassistant commented Jun 4, 2024 •

edited

Loading

github-actions Bot commented Jun 4, 2024 •

edited

Loading

🔍 Build Scan® (commit: `0c8ecb4`)