Add XDS support to gRPC extension #29251

alesj · 2022-11-14T13:20:34Z

This adds an initial XDS (proxyless) gRPC support (#28298 )

For the server side, one must explicitly enable XDS usage: quarkus.grpc.server.xds.enabled=true
For the client side, you either explicitly enable it - quarkus.grpc.clients.stub.xds.enabled=true, or you set a name resolver to xds (see example below).

New Vert.x gRPC is missing XDS support atm.

A new K8s based test(s) are added, which install Istio (a requirement) into K8s before running the tests.
(Istio == XDS Management server)
Some help will probably be needed for this to properly run on GH_Actions @iocanel @gsmet ^^

An example of XDS gRPC configuration:

# XDS
quarkus.grpc.server.xds.enabled=true
quarkus.grpc.server.port=30051

quarkus.grpc.clients.stub.name-resolver=xds
quarkus.grpc.clients.stub.host=localhost
quarkus.grpc.clients.stub.port=30051

cescoffier · 2022-11-14T13:46:08Z

@vietj here is the PR we discussed this morning.

alesj · 2022-11-15T08:25:02Z

As mentioned, I would need some help on the GH_Actions ...
@cescoffier @iocanel ^^

iocanel · 2022-11-15T10:02:17Z

As mentioned, I would need some help on the GH_Actions ... @cescoffier @iocanel ^^

I'll try to have a look at it later today!

iocanel · 2022-11-17T11:49:48Z

One issue I see is that the istio integration tests run as part of pull request validation build, which should not be the case.
Checked out the pull request and verify that this is the case for local builds too.
What I found confusing is that the same thing happens to the existing kubernetes invoker tests (they run by default).
AFAIR, the intention was that any integration tests that required kubernetes/openshfit/knative etc would have to be explicitly enabled using a profile, but this doesn't seem to be the case. @gsmet: Do I remember correctly? Is there something I miss?

iocanel · 2022-11-17T14:31:00Z

WIth a closer look, it seems that the offline-tests (default) will run but are expected to fail.
Not sure why this doesn't work in your case. It's like the invoker.buildResult is ignored for some reason.

cescoffier

The code looks ok, but the CI issue is annoying.
For now we can disable the tests but it would be very good to have a solution.

extensions/grpc/runtime/src/main/java/io/quarkus/grpc/runtime/GrpcServerRecorder.java

cescoffier · 2022-11-28T07:39:07Z

BTW, we need a bit of doc about this feature.

alesj · 2022-11-28T13:12:39Z

BTW, we need a bit of doc about this feature.

The resources on xDS are pretty limited ...
Where should we put this doc(s)?

cescoffier · 2022-11-28T13:55:19Z

I would add a link in grpc.adoc and create a new grpc-xds.adoc.

iocanel · 2022-11-28T14:55:09Z

@alesj I had the chance to look at the tests again. I think that missing piece is the the context of invoker.buildResult=fasle. This means that it is expected that the invoker command fails. However, this does not include the verification script. The script must always pass.

So, I think that we just need to exclude the particular test from the offline tests. Simplest way is to just disable them.

gsmet

As for CI, I can help but there are multiple problems that need fixing and I'm not always sure what is the intent.

Native tests

All the native tests for gRPC are now failing with issues like:

Caused by: com.oracle.graal.pointsto.constraints.UnresolvedElementException: Discovered unresolved type during parsing: io.grpc.netty.shaded.io.netty.util.internal.logging.Log4J2Logger. This error is reported at image build time because class io.grpc.netty.shaded.io.netty.util.internal.logging.Log4J2LoggerFactory is registered for linking at image build time by command line
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.phases.SharedGraphBuilderPhase$SharedBytecodeParser.reportUnresolvedElement(SharedGraphBuilderPhase.java:333)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.phases.SharedGraphBuilderPhase$SharedBytecodeParser.handleUnresolvedType(SharedGraphBuilderPhase.java:288)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.phases.SharedGraphBuilderPhase$SharedBytecodeParser.handleUnresolvedNewInstance(SharedGraphBuilderPhase.java:204)
	at jdk.internal.vm.compiler/org.graalvm.compiler.java.BytecodeParser.genNewInstance(BytecodeParser.java:4501)
	at jdk.internal.vm.compiler/org.graalvm.compiler.java.BytecodeParser.genNewInstance(BytecodeParser.java:4494)
	at jdk.internal.vm.compiler/org.graalvm.compiler.java.BytecodeParser.processBytecode(BytecodeParser.java:5291)
	at jdk.internal.vm.compiler/org.graalvm.compiler.java.BytecodeParser.iterateBytecodesForBlock(BytecodeParser.java:3385)
	... 37 more

Istio IT

I see you added a workflow fine to run it on a schedule but I also see the IT is running in the regular JVM run.
Is it supposed to work in the regular JVM run?

What exactly are you trying to achieve here?

FWIW, the existing Kubernetes tests are built to not fail if pointing to a non-available Kubernetes server. @iocanel might be able to help you on this.

Another option would be to disable the test for a normal run and only run it nightly.

Update: I see @iocanel already pushed some information.

.github/workflows/ci-istio.yml

cescoffier · 2022-12-05T07:51:27Z

@alesj any update about the documentation?

alesj · 2022-12-05T17:55:14Z

So, I think that we just need to exclude the particular test from the offline tests. Simplest way is to just disable them.

@iocanel how do I do that?

@gsmet feedback ...

All the native tests for gRPC are now failing with issues like ... Caused by: com.oracle.graal.pointsto.constraints.UnresolvedElementException: Discovered unresolved type during parsing ...

Can this be fixed with some GraalVM "substitute" code ...?

Is it supposed to work in the regular JVM run?

Probably not, as it needs Istio, which it can only get via K8s ...

Another option would be to disable the test for a normal run and only run it nightly.

Sure.
(If you let me know how to do this ...)

any update about the documentation?

@cescoffier wip ... hoping to finish them asap

iocanel · 2022-12-12T11:05:42Z

So, I think that we just need to exclude the particular test from the offline tests. Simplest way is to just disable them.

@iocanel how do I do that?

I would do that using maven profiles along with some property to complely skip these tests when offline.

gsmet · 2022-12-12T13:16:52Z

AFAICS, it's not something preexisting that is failing, it's the new stuff:

2022-12-12T02:30:46.1117701Z [INFO] --- maven-invoker-plugin:3.2.2:run (integration-tests) @ quarkus-integration-test-istio-invoker ---
2022-12-12T02:30:46.1244191Z [INFO] Building: xds-grpc/pom.xml
2022-12-12T02:30:48.6253935Z [INFO] [INFO] Scanning for projects...

I'll have a look at what you're doing exactly later today or tomorrow.

gsmet · 2022-12-13T15:21:26Z

@alesj I pushed two small commits to deal with the CI issues when offline and adjust the issue number to report the status to. I think it should solve that particular CI issue.
I haven't tested the workflow though. I suppose you have tested it in your fork?

alesj · 2022-12-13T15:31:51Z

OK, tnx!

…

On Tue, 13 Dec 2022 at 16:21, Guillaume Smet ***@***.***> wrote: @alesj <https://github.com/alesj> I pushed two small commits to deal with the CI issues when offline. I think it should solve that particular issue. — Reply to this email directly, view it on GitHub <#29251 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACRA6E6TSVQSZT24ZOFRO3WNCIABANCNFSM6AAAAAAR72EBYA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

alesj · 2022-12-19T22:08:41Z

I haven't tested the workflow though. I suppose you have tested it in your fork?

@gsmet what do you mean by workflow here?
I tested the istio test(s) locally and it worked.
I also used the Quarkus build from this xds1 branch for my demo,
showcasing xDS, and it also worked.

@gsmet I also accidentally overrode your 2 commits,
but I think I got them both back, yet it's again failing with the same "istio" error ...

Any idea what did I miss?

@cescoffier ^^

alesj · 2022-12-19T22:13:06Z

Any idea what did I miss?

Ah, just saw that I missed deleting this line

<skipInvocation>${skipTests}</skipInvocation>

cescoffier · 2023-01-09T12:34:28Z

@alesj can you rebase your PR on the latest main branch?

Add Istio support for XDS gRPC tests Split xDS support into diff module - due to grpc-netty-shaded native issue Port gsmet commits.

quarkus-bot · 2023-01-09T18:46:48Z

Failing Jobs - Building `e27dc93`

Status	Name	Step	Failures	Logs	Raw logs
✖	Devtools Tests - JDK 11	`Build`	Failures	Logs	Raw logs
✖	Devtools Tests - JDK 11 Windows	`Build`	⚠️ Check →	Logs	Raw logs
✖	Devtools Tests - JDK 17	`Build`	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
⌛	JVM Tests - JDK 11 Windows	`Build`	⚠️ Check →	Logs	Raw logs
✔️	JVM Tests - JDK 17
✔️	JVM Tests - JDK 18
✔️	Maven Tests - JDK 11
⌛	Maven Tests - JDK 11 Windows	`Build`	⚠️ Check →	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ Devtools Tests - JDK 11 #

- Failing: integration-tests/devtools

📦 integration-tests/devtools

✖ io.quarkus.devtools.codestarts.quarkus.HibernateOrmCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): HibernateOrmCodestartTest/testContent/src_main_kotlin_ilove_quark_us_MyKotlinEntity.kt] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.HibernateOrmPanacheKotlinCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): HibernateOrmPanacheKotlinCodestartTest/testContent/src_main_kotlin_ilove_quark_us_MyKotlinEntity.kt] 
Path:

⚙️ Devtools Tests - JDK 17 #

- Failing: integration-tests/devtools

📦 integration-tests/devtools

✖ io.quarkus.devtools.codestarts.quarkus.HibernateOrmCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): HibernateOrmCodestartTest/testContent/src_main_kotlin_ilove_quark_us_MyKotlinEntity.kt] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.HibernateOrmPanacheKotlinCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): HibernateOrmPanacheKotlinCodestartTest/testContent/src_main_kotlin_ilove_quark_us_MyKotlinEntity.kt] 
Path:

alesj requested a review from cescoffier November 14, 2022 13:20

quarkus-bot bot added area/grpc gRPC area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure labels Nov 14, 2022

alesj force-pushed the xds1 branch from faf94e5 to 29251ae Compare November 14, 2022 14:23

This comment has been minimized.

Sign in to view

geoand added the release/noteworthy-feature label Nov 15, 2022

cescoffier approved these changes Nov 23, 2022

View reviewed changes

extensions/grpc/runtime/src/main/java/io/quarkus/grpc/runtime/GrpcServerRecorder.java Show resolved Hide resolved

extensions/grpc/runtime/src/main/java/io/quarkus/grpc/runtime/GrpcServerRecorder.java Outdated Show resolved Hide resolved

gsmet requested changes Nov 28, 2022

View reviewed changes

.github/workflows/ci-istio.yml Outdated Show resolved Hide resolved

alesj mentioned this pull request Dec 7, 2022

Provide grpc-xds artifact w/o the need of grpc-netty-shaded grpc/grpc-java#9736

Closed

alesj force-pushed the xds1 branch from 29251ae to d0894e9 Compare December 11, 2022 21:57

quarkus-bot bot added the area/dependencies Pull requests that update a dependency file label Dec 11, 2022

This comment has been minimized.

Sign in to view

alesj force-pushed the xds1 branch from 1385704 to b789c47 Compare December 19, 2022 10:03

This comment has been minimized.

Sign in to view

alesj force-pushed the xds1 branch from b789c47 to 9a5e057 Compare December 19, 2022 16:14

This comment has been minimized.

Sign in to view

alesj force-pushed the xds1 branch from 9a5e057 to 295a8a3 Compare December 19, 2022 22:11

This comment has been minimized.

Sign in to view

alesj force-pushed the xds1 branch 4 times, most recently from 882169d to ca1fcd3 Compare January 5, 2023 14:27

This comment has been minimized.

Sign in to view

alesj force-pushed the xds1 branch from ca1fcd3 to c32cc9d Compare January 9, 2023 06:04

This comment has been minimized.

Sign in to view

alesj force-pushed the xds1 branch from c32cc9d to 45b79f9 Compare January 9, 2023 13:09

Add XDS gRPC support

e27dc93

Add Istio support for XDS gRPC tests Split xDS support into diff module - due to grpc-netty-shaded native issue Port gsmet commits.

alesj force-pushed the xds1 branch from 45b79f9 to e27dc93 Compare January 9, 2023 13:09

cescoffier merged commit d438bc5 into quarkusio:main Jan 10, 2023

quarkus-bot bot added this to the 2.16 - main milestone Jan 10, 2023

cescoffier mentioned this pull request Jan 16, 2023

gRPC - add support for proxyless istio (gRPC extension) #28298

Closed

Sgitario mentioned this pull request Jan 20, 2023

Force quarkus grpc xds dependency for the istio integration test #30498

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add XDS support to gRPC extension #29251

Add XDS support to gRPC extension #29251

alesj commented Nov 14, 2022

cescoffier commented Nov 14, 2022

This comment has been minimized.

alesj commented Nov 15, 2022

iocanel commented Nov 15, 2022

iocanel commented Nov 17, 2022

iocanel commented Nov 17, 2022

cescoffier left a comment

cescoffier commented Nov 28, 2022

alesj commented Nov 28, 2022

cescoffier commented Nov 28, 2022

iocanel commented Nov 28, 2022

gsmet left a comment

cescoffier commented Dec 5, 2022

alesj commented Dec 5, 2022 •

edited

Loading

This comment has been minimized.

iocanel commented Dec 12, 2022

gsmet commented Dec 12, 2022

gsmet commented Dec 13, 2022 •

edited

Loading

alesj commented Dec 13, 2022 via email

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

alesj commented Dec 19, 2022

alesj commented Dec 19, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

cescoffier commented Jan 9, 2023

quarkus-bot bot commented Jan 9, 2023

Add XDS support to gRPC extension #29251

Add XDS support to gRPC extension #29251

Conversation

alesj commented Nov 14, 2022

cescoffier commented Nov 14, 2022

This comment has been minimized.

alesj commented Nov 15, 2022

iocanel commented Nov 15, 2022

iocanel commented Nov 17, 2022

iocanel commented Nov 17, 2022

cescoffier left a comment

Choose a reason for hiding this comment

cescoffier commented Nov 28, 2022

alesj commented Nov 28, 2022

cescoffier commented Nov 28, 2022

iocanel commented Nov 28, 2022

gsmet left a comment

Choose a reason for hiding this comment

Native tests

Istio IT

cescoffier commented Dec 5, 2022

alesj commented Dec 5, 2022 • edited Loading

This comment has been minimized.

iocanel commented Dec 12, 2022

gsmet commented Dec 12, 2022

gsmet commented Dec 13, 2022 • edited Loading

alesj commented Dec 13, 2022 via email

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

alesj commented Dec 19, 2022

alesj commented Dec 19, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

cescoffier commented Jan 9, 2023

quarkus-bot bot commented Jan 9, 2023

Failing Jobs - Building e27dc93

Failures

⚙️ Devtools Tests - JDK 11 #

📦 integration-tests/devtools

⚙️ Devtools Tests - JDK 17 #

📦 integration-tests/devtools

alesj commented Dec 5, 2022 •

edited

Loading

gsmet commented Dec 13, 2022 •

edited

Loading

Failing Jobs - Building `e27dc93`