Add Fabric8 Kubernetes client integration #5167

ikhoon · 2023-09-04T16:28:11Z

Motivation:

Fabric8 is one of the popular Kubernetes client implementations. It has an abstract layer for HTTP and WebSocket protocols.

If Armeria provides a Kubernetes client to better support the cloud infrastructure, other useful functions such as #4497 will be able to be implemented based on it.

Modifications:

Add kubernetes module to provide Fabric StandardHttpClient.
- ArmeriaHttpClientFactory is automatically activated via Java SPI.
- Both HTTP and WebSocket clients have been implemented in compliance with Reactive Streams specification.
- WebSocket is working over HTTP/1. WebSocket over HTTP/2 is disabled for compatibility.
- Forked test suites from the upstream repo.
Miscellaneous)
- Allow ProxyConfig to configure proxy headers.
- Allow WebSocketClient to configure HttpHeaders and RequestOptions when starting a WebSocket session.

Result:

You can use the Fabric Kubernetes client on top of the Armeria client.

Motivation: Related: fabric8io/kubernetes-client#5307 TBU

github-actions · 2023-09-04T16:45:00Z

🔍 Build Scan® (commit: `4305c06`)

Job name	Status	Build Scan®
build-windows-latest-jdk-19	✅	https://ge.armeria.dev/s/enqreuq4ehd3u
build-self-hosted-unsafe-jdk-8	✅	https://ge.armeria.dev/s/jpkjkdit2ypmc
build-self-hosted-unsafe-jdk-19-snapshot-blockhound	❌ (failure)	https://ge.armeria.dev/s/can5ak667mqcc
build-self-hosted-unsafe-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/jdbs2jtiy2tq6
build-self-hosted-unsafe-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/opzyqc5txoxc6
build-self-hosted-unsafe-jdk-17-leak	❌ (failure)	https://ge.armeria.dev/s/72amb7wj6qyru
build-self-hosted-unsafe-jdk-11	✅	https://ge.armeria.dev/s/gomdlyajyln5q
build-macos-12-jdk-19	✅	https://ge.armeria.dev/s/sg7x5hnmishhg

andreaTP · 2023-09-13T12:11:49Z

@ikhoon quick note that there are chaos tests that you can possibly re-use to test that the implementation is solid enough:
https://github.com/fabric8io/kubernetes-client/tree/main/chaos-tests

ikhoon · 2023-09-15T06:00:06Z

Thanks for the tip. I will take a look.

codecov · 2023-12-06T12:48:25Z

Codecov Report

Attention: 172 lines in your changes are missing coverage. Please review.

Comparison is base (52d130f) 0.00% compared to head (4305c06) 73.66%.
Report is 6 commits behind head on main.

Files	Patch %	Lines
...linecorp/armeria/kubernetes/it/CheckerCommand.java	0.00%	49 Missing ⚠️
...linecorp/armeria/kubernetes/it/ControlCommand.java	0.00%	49 Missing ⚠️
...ia/client/kubernetes/ArmeriaHttpClientBuilder.java	61.90%	15 Missing and 9 partials ⚠️
...rp/armeria/client/kubernetes/ArmeriaWebSocket.java	68.88%	9 Missing and 5 partials ⚠️
...p/armeria/client/kubernetes/ArmeriaHttpClient.java	83.92%	4 Missing and 5 partials ⚠️
...armeria/client/kubernetes/ArmeriaHttpResponse.java	63.63%	7 Missing and 1 partial ⚠️
...eria/client/kubernetes/ArmeriaWebSocketClient.java	86.20%	5 Missing and 3 partials ⚠️
...armeria/client/kubernetes/AsyncBodySubscriber.java	71.42%	7 Missing and 1 partial ⚠️
...a/com/linecorp/armeria/client/HttpChannelPool.java	66.66%	1 Missing ⚠️
...ecorp/armeria/client/proxy/ConnectProxyConfig.java	83.33%	0 Missing and 1 partial ⚠️
... and 1 more

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #5167       +/-   ##
===========================================
+ Coverage        0   73.66%   +73.66%     
- Complexity      0    20636    +20636     
===========================================
  Files           0     1791     +1791     
  Lines           0    76335    +76335     
  Branches        0     9716     +9716     
===========================================
+ Hits            0    56233    +56233     
- Misses          0    15448    +15448     
- Partials        0     4654     +4654

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ikhoon · 2023-12-06T09:44:56Z

kubernetes/build.gradle.kts

+    api(libs.kubernetes.client.api)
+    testImplementation(variantOf(libs.kubernetes.client.api) { classifier("tests") })
+    testImplementation(libs.kubernetes.server.mock)
+    testImplementation(libs.logback13)


Note: Logback 1.3.x is required since the Fabric Kubernetes client uses SLF4J 2.0.
Should it be nicer to exclude SLF4J 2.0 from transitive dependencies?

Would it be possible? If so, yes I think we should exclude it.

jrhee17

Looks good overall! Left some minor questions 🙇

.github/workflows/e2e-chaos-tests.yml

jrhee17 · 2023-12-12T09:45:31Z

it/kubernetes-chaos-tests/README.md

+
+```bash
+eval $(minikube -p minikube docker-env)
+./gradlew :it:kubernetes-chaos-tests:k8sBuild :it:kubernetes-chaos-tests:test


Just plainly running this fails due to the gradle junit timeout (although I saw the @Timeout annotation in the test). Were you able to get this to run in your environment?

org.awaitility.core.ConditionTimeoutException: Condition with com.linecorp.armeria.kubernetes.it.ChaosIT was not fulfilled within 1 minutes.

https://ge.armeria.dev/s/hkphofabit2xo

Yes, I've tested locally and debugged many problems.
The test might be failed due to a wrong setup.
Would you paste the output of docker images?

kubernetes/src/test/java/com/linecorp/armeria/client/kubernetes/ArmeriaHttpClientTest.java

kubernetes/src/test/java/com/linecorp/armeria/client/kubernetes/ArmeriaHttpInterceptorTest.java

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/AsyncBodySubscriber.java

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/ArmeriaHttpClient.java

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/ArmeriaHttpClientFactory.java

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/ArmeriaHttpClientBuilder.java

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/ArmeriaWebSocketClient.java

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/ArmeriaWebSocket.java

jrhee17

Basically looks good to me! Can you check the CI failures?

minwoox

Looks so nice! 🎉 🎉 🎉
I didn't see the chaos part though. 😅

core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/ArmeriaWebSocketClient.java

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/ArmeriaHttpClientBuilder.java

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/ArmeriaWebSocket.java

.../test/java/com/linecorp/armeria/client/kubernetes/ArmeriaHttpHttpLoggingInterceptorTest.java

...st/java/com/linecorp/armeria/client/kubernetes/ArmeriaHttpClientNewWebSocketBuilderTest.java

minwoox

Will review once more after #5357 is merged. Thanks! 🎉

minwoox · 2024-01-22T08:54:54Z

kubernetes/build.gradle.kts

+    api(libs.kubernetes.client.api)
+    testImplementation(variantOf(libs.kubernetes.client.api) { classifier("tests") })
+    testImplementation(libs.kubernetes.server.mock)
+    testImplementation(libs.logback13)


Would it be possible? If so, yes I think we should exclude it.

ikhoon · 2024-01-24T14:34:55Z

Would it be possible? If so, yes I think we should exclude it.

Excluded via exclusions in dependencies.toml

runtimeClasspath - Runtime classpath of source set 'main'.
+--- project :core
|    +--- org.slf4j:slf4j-api:1.7.36
|    +--- com.fasterxml.jackson.core:jackson-core -> 2.15.3
|    |    \--- com.fasterxml.jackson:jackson-bom:2.15.3
|    |         +--- com.fasterxml.jackson.core:jackson-annotations:2.15.3 (c)
|    |         +--- com.fasterxml.jackson.core:jackson-core:2.15.3 (c)
|    |         +--- com.fasterxml.jackson.core:jackson-databind:2.15.3 (c)
...
+--- it.unimi.dsi:fastutil:8.5.12
+--- com.google.guava:guava:32.1.3-jre (*)
+--- com.google.code.findbugs:jsr305:3.0.2
+--- org.jctools:jctools-core:4.0.1
+--- org.slf4j:slf4j-api:1.7.36
+--- org.reflections:reflections:0.9.11 (*)
\--- io.projectreactor.tools:blockhound:1.0.8.RELEASE

.github/workflows/e2e-chaos-tests.yml

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/AsyncBodySubscriber.java

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/ArmeriaWebSocket.java

jrhee17

Looks very nice, only left nit questions! Thanks @ikhoon 🙇 👍 🙇

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/ArmeriaWebSocket.java

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/AsyncBodySubscriber.java

minwoox

Still LGTM. 👍
Please address the comment from @jrhee17. 😉

it/kubernetes-chaos-tests/build.gradle

ikhoon · 2024-01-29T05:41:17Z

Thanks for the thorough review! 🙇‍♂️

Add Fabric8 Kubernetes client integration

4ee3c2b

Motivation: Related: fabric8io/kubernetes-client#5307 TBU

ikhoon added the new feature label Sep 4, 2023

ikhoon added 2 commits October 10, 2023 10:29

[wip] Adding chaos tests

ce29470

Merge branch 'main' into fabric-kubernetes-client

5b78411

Add e2e-chaos-test

115ef47

ikhoon force-pushed the fabric-kubernetes-client branch from 4a4d504 to 115ef47 Compare December 11, 2023 08:44

ikhoon commented Dec 11, 2023

View reviewed changes

ikhoon marked this pull request as ready for review December 11, 2023 08:45

ikhoon requested review from jrhee17, minwoox and trustin as code owners December 11, 2023 08:45

ikhoon added this to the 1.27.0 milestone Dec 11, 2023

jrhee17 reviewed Dec 15, 2023

View reviewed changes

ikhoon added 2 commits December 26, 2023 21:20

Address comments by @jrhee17

78f1650

Add GE access token

1c4decb

ikhoon mentioned this pull request Dec 27, 2023

Introduce the capability to aggregate WebSocket continuation frames. #5357

Merged

jrhee17 reviewed Jan 10, 2024

View reviewed changes

minwoox reviewed Jan 11, 2024

View reviewed changes

ikhoon added 5 commits January 18, 2024 17:37

Oops...

5b48946

Address comments by @minwoox

17d4cf9

Address comments by @jrhee17

d0831fb

ArmeriaHttpClient is hidden

5e1e998

expectContinue() test is disabled

2546673

minwoox approved these changes Jan 22, 2024

View reviewed changes

ikhoon added 2 commits January 24, 2024 21:01

Merge branch 'main' into fabric-kubernetes-client

d60d084

Enable aggregateContinuation

6471623

ikhoon and others added 3 commits January 24, 2024 22:26

Merge branch 'main' into fabric-kubernetes-client

09e0dd7

Exclude slf4j dependencies from Kubernetes

dbc462a

lint

93c42dc

jrhee17 reviewed Jan 25, 2024

View reviewed changes

jrhee17 approved these changes Jan 25, 2024

View reviewed changes

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/ArmeriaWebSocket.java Show resolved Hide resolved

kubernetes/src/main/java/com/linecorp/armeria/client/kubernetes/AsyncBodySubscriber.java Outdated Show resolved Hide resolved

minwoox approved these changes Jan 26, 2024

View reviewed changes

ikhoon added 2 commits January 26, 2024 14:03

Address comments by @jrhee17

d6258b6

remove unused imports

41176fd

ikhoon commented Jan 26, 2024

View reviewed changes

it/kubernetes-chaos-tests/build.gradle Outdated Show resolved Hide resolved

Update it/kubernetes-chaos-tests/build.gradle

4305c06

ikhoon merged commit d2afcd9 into line:main Jan 29, 2024
15 of 17 checks passed

ikhoon deleted the fabric-kubernetes-client branch June 28, 2024 10:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Fabric8 Kubernetes client integration #5167

Add Fabric8 Kubernetes client integration #5167

ikhoon commented Sep 4, 2023 •

edited

Loading

github-actions bot commented Sep 4, 2023 •

edited

Loading

andreaTP commented Sep 13, 2023

ikhoon commented Sep 15, 2023

codecov bot commented Dec 6, 2023 •

edited

Loading

ikhoon Dec 6, 2023

minwoox Jan 22, 2024

jrhee17 left a comment

jrhee17 Dec 12, 2023

ikhoon Dec 26, 2023

jrhee17 left a comment

minwoox left a comment

minwoox left a comment

minwoox Jan 22, 2024

ikhoon commented Jan 24, 2024

jrhee17 left a comment

minwoox left a comment

ikhoon commented Jan 29, 2024

Add Fabric8 Kubernetes client integration #5167

Add Fabric8 Kubernetes client integration #5167

Conversation

ikhoon commented Sep 4, 2023 • edited Loading

github-actions bot commented Sep 4, 2023 • edited Loading

🔍 Build Scan® (commit: 4305c06)

andreaTP commented Sep 13, 2023

ikhoon commented Sep 15, 2023

codecov bot commented Dec 6, 2023 • edited Loading

Codecov Report

ikhoon Dec 6, 2023

Choose a reason for hiding this comment

minwoox Jan 22, 2024

Choose a reason for hiding this comment

jrhee17 left a comment

Choose a reason for hiding this comment

jrhee17 Dec 12, 2023

Choose a reason for hiding this comment

ikhoon Dec 26, 2023

Choose a reason for hiding this comment

jrhee17 left a comment

Choose a reason for hiding this comment

minwoox left a comment

Choose a reason for hiding this comment

minwoox left a comment

Choose a reason for hiding this comment

minwoox Jan 22, 2024

Choose a reason for hiding this comment

ikhoon commented Jan 24, 2024

jrhee17 left a comment

Choose a reason for hiding this comment

minwoox left a comment

Choose a reason for hiding this comment

ikhoon commented Jan 29, 2024

ikhoon commented Sep 4, 2023 •

edited

Loading

github-actions bot commented Sep 4, 2023 •

edited

Loading

🔍 Build Scan® (commit: `4305c06`)

codecov bot commented Dec 6, 2023 •

edited

Loading