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

Use quarkus with Google Cloud Run And CloudSQL #6634

Closed
wants to merge 8 commits into from
Closed

Use quarkus with Google Cloud Run And CloudSQL #6634

wants to merge 8 commits into from

Conversation

Mihai-B
Copy link

@Mihai-B Mihai-B commented Jan 20, 2020

No description provided.

@Mihai-B Mihai-B changed the title Use quarkus with Google Cloud Run And CloudSQL #5901 Use quarkus with Google Cloud Run And CloudSQL Jan 20, 2020
@Mihai-B
Copy link
Author

Mihai-B commented Jan 20, 2020

Links to #5824

@Mihai-B Mihai-B requested a review from gastaldi January 20, 2020 11:40
Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

Added more comments. Will this work in native too?

docs/src/main/asciidoc/deploying-to-google-cloud.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/deploying-to-google-cloud.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/deploying-to-google-cloud.adoc Outdated Show resolved Hide resolved
@Mihai-B
Copy link
Author

Mihai-B commented Jan 20, 2020

@gastaldi To be honest. I compiled it in native mode. run the code. But I didn't actually connect to SQL from Cloud Run.

@Mihai-B Mihai-B requested a review from gastaldi January 20, 2020 12:16
@gastaldi
Copy link
Contributor

@Mihai-B can you make sure it works in native too?

@Mihai-B
Copy link
Author

Mihai-B commented Jan 20, 2020

@gastaldi To be honest. I compiled it in native mode. run the code. But I didn't actually connect to SQL from Cloud Run.

@Mihai-B can you make sure it works in native too?

What more should I do? I have to deploy to CloudRun in order to really test that it actually connects to CLoudSql from CloudRun

@gastaldi
Copy link
Contributor

My concern is that when a SocketFactory class is provided, the extension may need to declare that class as available for reflective access but maybe that's not necessary, hence why I am curious if compiling to native will work OOTB with your changes

@jtama
Copy link
Contributor

jtama commented Jan 20, 2020

Hi,
I already try this track but it didn't worked. The socket factory was not passed to the jdbc driver. (#5901). Have you deployed it ?

@Mihai-B
Copy link
Author

Mihai-B commented Jan 20, 2020

My concern is that when a SocketFactory class is provided, the extension may need to declare that class as available for reflective access but maybe that's not necessary, hence why I am curious if compiling to native will work OOTB with your changes

I will deploy it on CloudRun and I will notify how it works

Hi,
I already try this track but it didn't worked. The socket factory was not passed to the jdbc driver. (#5901). Have you deployed it ?

Not actually deployed, I only checked that the properties are set as jdbc properties inside the AgroalConnectionFactoryConfiguration and they are. I see no reason why they will not be set on the url itself. but I will deploy it in CloudRun and will let you guys know

@Mihai-B
Copy link
Author

Mihai-B commented Jan 20, 2020

Hi,
I already try this track but it didn't worked. The socket factory was not passed to the jdbc driver. (#5901). Have you deployed it ?

You are right, the jdbc properties are not taken into consideration, can this be Agroal bug or am I doing something wrong?

@gastaldi Do you know if thedisableSslSupport is allways true for native builds?

@gastaldi
Copy link
Contributor

gastaldi commented Jan 20, 2020

@Mihai-B I think it depends on whether there is an extension that enables SSL or not. For example, if an extension requires SSL to be enabled, it should be done like: https://github.com/quarkusio/quarkus/blob/master/extensions/jsch/deployment/src/main/java/io/quarkus/jsch/deployment/JSchProcessor.java#L18-L21

In your case, I guess the solution is to come up with a GCP extension, but I am not sure if that extension should belong to core

@Mihai-B
Copy link
Author

Mihai-B commented Jan 20, 2020

Ok, but if ssl is enabled, it sets the parameter SslEnabled=true exactly the same way I am setting the socket factory property, yet mine is not picked up.

Are we sure that disablessl flag is really set for jdbc connections?

Also: My implementation is not google cloud specific, in the end, all I am trying to do is set a connection factory to my jdbc connection

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

This will require a squash before merging + I added some comments.

@Mihai-B Mihai-B requested a review from gsmet January 21, 2020 09:16
@norrs
Copy link

norrs commented Mar 20, 2020

Would love to get some attention to getting this into a release of quarkus, @gastaldi

I've verified that it worked to mvn install @Mihai-B PR and then change my project to use the snapshot build and deploy it on cloudrun with cloudsql. This is currently using the JVM Dockerfile.

Documentation should add a link that it requires you to activate the https://console.cloud.google.com/apis/api/sqladmin.googleapis.com/overview as well when using cloudsql.

Ill see if I can get the native build going and verify that it works for native Dockerfile as well, if not someone else have verified it.

This issue is currently a huge show stopper for simply using quarkus as cloud native tool and use Google cloud platform IMO. (minimal application with access to a (cloud)SQL database.)

@norrs
Copy link

norrs commented Mar 24, 2020

@Mihai-B : MySQL error thrown in native mode is the following:

Hibernate: 
    select
        fruit0_.id as id1_0_,
        fruit0_.name as name2_0_ 
    from
        Fruit fruit0_ 
    order by
        fruit0_.name limit ?
2020-03-24 00:03:53,640 INFO  [com.goo.clo.sql.cor.CoreSocketFactory] (Agroal_8991094791) Connecting to Cloud SQL instance [xx:xx:quickstart-mysql-dev] via unix socket.
2020-03-24 00:03:53,640 WARN  [io.agr.pool] (Agroal_8991094791) Datasource '<default>': Could not create connection to database server.
2020-03-24 00:03:53,640 WARN  [org.hib.eng.jdb.spi.SqlExceptionHelper] (executor-thread-1) SQL Error: 0, SQLState: 08001
2020-03-24 00:03:53,640 ERROR [org.hib.eng.jdb.spi.SqlExceptionHelper] (executor-thread-1) Could not create connection to database server.

https://dev.mysql.com/doc/connector-j/8.0/en/connector-j-usagenotes-troubleshooting.html#qandaitem-15-1-1

EDIT: Updated log with DEBUG enabled and running locally. (so it is not due to gvisor or something like that)

2020-03-24 00:27:32,633 DEBUG [org.hib.eng.jdb.spi.SqlExceptionHelper] (executor-thread-1) Unable to acquire JDBC Connection [n/a]: java.sql.SQLNonTransientConnectionException: Could not create connection to database server.
	at com.mysql.cj.jdbc.exceptions.SQLError.createSQLException(SQLError.java:110)
	at com.mysql.cj.jdbc.exceptions.SQLError.createSQLException(SQLError.java:97)
	at com.mysql.cj.jdbc.exceptions.SQLError.createSQLException(SQLError.java:89)
	at com.mysql.cj.jdbc.exceptions.SQLError.createSQLException(SQLError.java:63)
	at com.mysql.cj.jdbc.ConnectionImpl.connectOneTryOnly(ConnectionImpl.java:1009)
	at com.mysql.cj.jdbc.ConnectionImpl.createNewIO(ConnectionImpl.java:826)
	at com.mysql.cj.jdbc.ConnectionImpl.<init>(ConnectionImpl.java:456)
	at com.mysql.cj.jdbc.ConnectionImpl.getInstance(ConnectionImpl.java:246)
	at com.mysql.cj.jdbc.NonRegisteringDriver.connect(NonRegisteringDriver.java:197)
	at io.agroal.pool.ConnectionFactory.createConnection(ConnectionFactory.java:200)
	at io.agroal.pool.ConnectionPool$CreateConnectionTask.call(ConnectionPool.java:390)
	at io.agroal.pool.ConnectionPool$CreateConnectionTask.call(ConnectionPool.java:372)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at io.agroal.pool.util.PriorityScheduledExecutor.beforeExecute(PriorityScheduledExecutor.java:65)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1146)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
	at com.oracle.svm.core.thread.JavaThreads.threadStartRoutine(JavaThreads.java:497)
	at com.oracle.svm.core.posix.thread.PosixJavaThreads.pthreadStartRoutine(PosixJavaThreads.java:193)
Caused by: java.lang.RuntimeException: Could not load platform constants for ProtocolFamily
	at jnr.constants.platform.ConstantResolver.getConstants(ConstantResolver.java:227)
	at jnr.constants.platform.ConstantResolver.lookupAndCacheConstant(ConstantResolver.java:128)
	at jnr.constants.platform.ConstantResolver.getConstant(ConstantResolver.java:116)
	at jnr.constants.platform.ConstantResolver.longValue(ConstantResolver.java:179)
	at jnr.constants.platform.ProtocolFamily.intValue(ProtocolFamily.java:48)
	at jnr.unixsocket.SockAddrUnix.setFamily(SockAddrUnix.java:55)
	at jnr.unixsocket.UnixSocketAddress.<init>(UnixSocketAddress.java:48)
	at com.google.cloud.sql.core.CoreSocketFactory.connect(CoreSocketFactory.java:179)
	at com.google.cloud.sql.mysql.SocketFactory.connect(SocketFactory.java:48)
	at com.google.cloud.sql.mysql.SocketFactory.connect(SocketFactory.java:38)
	at com.mysql.cj.protocol.a.NativeSocketConnection.connect(NativeSocketConnection.java:65)
	at com.mysql.cj.NativeSession.connect(NativeSession.java:144)
	at com.mysql.cj.jdbc.ConnectionImpl.connectOneTryOnly(ConnectionImpl.java:956)
	... 14 more

@norrs
Copy link

norrs commented Mar 24, 2020

I suspect it is related to oracle/graal#1696 , but I'm not sure how I could try to make a native image with -Dsvm.platform='org.graalvm.nativeimage.impl.InternalPlatform$LINUX_JNI_AMD64 enabled.

I tried adding it to quarkus.native.additional-build-args but it throws me Error: Could not find platform class 'org.graalvm.nativeimage.impl.InternalPlatform$LINUX_JNI_AMD64' that was specified explicitly on the command line using the system property svm.platform when trying mvn clean package -Pnative -Dquarkus.native.container-build=true -DskipTests=true.

Unsure what has to be installed inside the Dockerfile.native to gain access to JNI Linux..

@norrs
Copy link

norrs commented Mar 25, 2020

@gastaldi @Mihai-B : I would love to continue trying to get this to work, as I want to try to get with the linux JNI bindings included for graalvm compiling, and also try to see if it ie helps to disable or prefer IPv4 mode.

Do you know someone who might be able to assist me in the right direction for further debugging?

@Mihai-B
Copy link
Author

Mihai-B commented Mar 25, 2020

@norrs I made an account on Zullip, but I am not that active there. I do however see the notifications here unfortunately I don't really understand with what you need assistance with

@norrs
Copy link

norrs commented Mar 25, 2020

@Mihai-B : so I believe oracle/graal#1696 seems to be touching the same troubles I get when trying to make a native binary and run it in cloud run and connect to the cloud sql, and it throws java.lang.RuntimeException: Could not load platform constants for ProtocolFamily.

If I understand it correctly, graalvm native binary doesn't link with JNI binding towards Linux amd64 (for syscalls etc), so when we are trying to connect to the Cloud SQL via the library, it fails to create the unix socket.

I would love to try to build the native binary with JNI linux amd64 bindings included.
(I'm no expert or haven't used graalvm before, but this is my hypothesis of what is wrong - so I might be steering in totally wrong direction.. )

So I wonder how may I do this with https://quarkus.io/guides/building-native-image#creating-a-container approach.

@Mihai-B
Copy link
Author

Mihai-B commented Mar 25, 2020

In cloud run you need to add this environment variable:
DISABLE_SIGNAL_HANDLERS 1
Should work after.

@norrs
Copy link

norrs commented Mar 25, 2020

In cloud run you need to add this environment variable:
DISABLE_SIGNAL_HANDLERS 1
Should work after.

@Mihai-B : This was already set, just another value. I tried setting it to 1 just to ensure it wasn't the wrong value, but I still get the same exception thrown.
(I checked the source code here the other day and it just checks for any value is present to disable the signal handlers which is required for cloud run)

Just for clarification, it servers the webpage etc, it simply cannot connect to the cloud sql instance via unix socket which is required, and it throws the exception.

@loicmathieu
Copy link
Contributor

@Mihai-B we have multiple users asking for how to connect to Cloud SQL.
Do you plan to work again on this PR ?
Maybe we can split the additional-jdbc-properties part in a separate PR, this is what's needed to make Cloud SQL working.

@norrs
Copy link

norrs commented Jun 16, 2020

@loicmathieu : it works with JVM without this PR as mentioned at #6634 (comment) (FYI).

You are able to specify the additional jdbc properties in the connection URL (quarkus.datasource.url) when using the cloudsql driver.

JDBC URL syntax:

jdbc:mysql:///<DATABASE_NAME>?cloudSqlInstance=<INSTANCE_CONNECTION_NAME>&socketFactory=com.google.cloud.sql.mysql.SocketFactory&user=<MYSQL_USER_NAME>&password=<MYSQL_USER_PASSWORD>

Bad things is that you specify the password there as well, and this gets logged to application logs.. (when I played around with it in cloudrun)

@loicmathieu
Copy link
Contributor

@norrs

Bad things is that you specify the password there as well, and this gets logged to application logs.. (when I played around with it in cloudrun)

This is wy we need the support for additional-jdbc-properties at the datasource level. I suspect those will not be logged to application logs.

@Mihai-B
Copy link
Author

Mihai-B commented Jun 17, 2020

@Mihai-B we have multiple users asking for how to connect to Cloud SQL.
Do you plan to work again on this PR ?
Maybe we can split the additional-jdbc-properties part in a separate PR, this is what's needed to make Cloud SQL working.

I noticed that the connection process to Cloud Run has changed(if I am not mistaken?). I am planning to work on it, I just don't have the time right now. Realistically in 3 weeks I can start working on it but not sooner :(
I could use some help TBH

@loicmathieu
Copy link
Contributor

@Mihai-B thanks for the heads up.

If you don't have a lot of time, you can split your work as I suggested.

If you need help, don't hesitate to reach us on Zulip: https://quarkusio.zulipchat.com/#narrow/stream/187038-dev

@xource-tech
Copy link

@Mihai-B @loicmathieu Any updates regarding this? Seems to me this is required for running any application with a db connection in Google Cloud Run instance, since they have no IP?

@loicmathieu
Copy link
Contributor

@Mihai-B as you seems to don't have time to handle this, can I took some of your code from this PR to provides the capability to add additional properties to a Quarkus datasource so that we can support Cloud SQL ?

@gastaldi
Copy link
Contributor

gastaldi commented Oct 8, 2020

I think we'd want to have the additionalJdbcProperties feature + a guide explaining how to configure Quarkus to work with Cloud SQL?

@loicmathieu if you can work on this, feel free to close this pull-request

gastaldi added a commit to gastaldi/quarkus that referenced this pull request Oct 9, 2020
Some environments use these properties for additional metadata (eg. in Google Cloud SQL)

Closes quarkusio#6634

Co-authored-by: Mihai B <[email protected]>
@gastaldi
Copy link
Contributor

gastaldi commented Oct 9, 2020

I created #12639 for the additional JDBC properties support, but the Google Cloud SQL guide needs to be in a different PR

gastaldi added a commit to gastaldi/quarkus that referenced this pull request Oct 12, 2020
Some environments use these properties for additional metadata (eg. in Google Cloud SQL)

Closes quarkusio#6634

Co-authored-by: Mihai B <[email protected]>
@gsmet gsmet added the triage/invalid This doesn't seem right label Oct 13, 2020
gsmet pushed a commit to gsmet/quarkus that referenced this pull request Oct 13, 2020
Some environments use these properties for additional metadata (eg. in Google Cloud SQL)

Closes quarkusio#6634

Co-authored-by: Mihai B <[email protected]>
xumk pushed a commit to xumk/quarkus that referenced this pull request Oct 14, 2020
Some environments use these properties for additional metadata (eg. in Google Cloud SQL)

Closes quarkusio#6634

Co-authored-by: Mihai B <[email protected]>
@Mihai-B
Copy link
Author

Mihai-B commented Oct 15, 2020

Perfect. Apperantly there is a way you can connect to to Google Cloud Sql from CloudRun with a private IP. I am trying this to see how it works. if I can get it working I will submit a tutorial

@Mihai-B
Copy link
Author

Mihai-B commented Oct 15, 2020

@Mihai-B as you seems to don't have time to handle this, can I took some of your code from this PR to provides the capability to add additional properties to a Quarkus datasource so that we can support Cloud SQL ?

Not sure if I am to late, but offcourse, you can do whatever you want with the code. If we can make it work, then I am happy :)

@jtama
Copy link
Contributor

jtama commented Oct 15, 2020

Perfect. Apperantly there is a way you can connect to to Google Cloud Sql from CloudRun with a private IP. I am trying this to see how it works. if I can get it working I will submit a tutorial

You could also use public IP with no firewall rule which should work with native build if this still the main issue with socket proxy.

@nvrs
Copy link

nvrs commented Sep 6, 2021

FYI, I managed to make this work with the latest quarkus release and GraalVM 21.2.0.r16 in the following way:

  1. Added the maven dependency for google-cloud-graalvm-support into my project.
  2. Added the property -Dquarkus.native.additional-build-args="--initialize-at-run-time=jdk.internal.platform.cgroupv2.CgroupV2Subsystem" in the package maven command.

The produced binary now works fine on Google Cloud Run using postgres Cloud SQL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation triage/invalid This doesn't seem right triage/needs-rebase This PR needs to be rebased first because it has merge conflicts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants