-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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
[SPARK-46525][DOCKER][TESTS][FOLLOWUP] Fix docker-integration-tests on Apple Silicon for db2 and oracle with third-party docker environments #44612
Conversation
@@ -156,7 +156,7 @@ abstract class DockerJDBCIntegrationSuite | |||
.newHostConfig() | |||
.withNetworkMode("bridge") | |||
.withPrivileged(db.privileged) | |||
.withPortBindings(PortBinding.parse(s"$dockerIp:$externalPort:${db.jdbcPort}")) | |||
.withPortBindings(PortBinding.parse(s"$externalPort:${db.jdbcPort}")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Three parts binding seems not to work on Colima, removing IP part works for both
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry but it sounds like too hasty to introduce colima
as an Apache spark community recommendation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the second thought, I believe we had better document this officially in our website instead of here and collect more alternatives when we find in the future.
In short, please remove the repetitive Tips~
addition from this PR and make a PR to Spark website. In this PR, please leave a code change.
…n Apple Sillicon for db2 and oracle with colima docker environment
495b495
to
440b4df
Compare
Thanks @dongjoon-hyun. Your suggestion sounds good to me. Does the section name below for the web repo look ok to you?
|
Yes, it sounds good, @yaooqinn . |
I've noticed that this alternative was less stable than the official one. It failed from time to time. Please give me more time to test. |
Oh, thank you for spotting that. Sure, take your time~ |
Hi @dongjoon-hyun. I've found the issue which seems related to my network setting and now everything is OK. BTW, as we have a README for Kubernetes ITs, in which we also describe solutions for different backends. Thus, how about we follow it to have one for Docker ITs? And then make a spark-web site PR to point to this file like what we do for k8s? WDYT? |
It sounds good to me too.. Let me merge this PR. Please proceed for doc PRs (README and website).
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM.
Merged to master. |
Sure. Thank you @dongjoon-hyun |
…ests ### What changes were proposed in this pull request? Following the review comment #44612 (review), in this PR, we add a README for instructions on Docker integration tests ### Why are the changes needed? tips for Docker integration tests with different envrionments ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? doc build ### Was this patch authored or co-authored using generative AI tooling? no Closes #44826 from yaooqinn/SPARK-46788. Authored-by: Kent Yao <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
…e Silicon ### What changes were proposed in this pull request? This is a merged backport of SPARK-46525 with the original authorship, yaooqinn . - #44509 - #44612 - #45303 `com.spotify.docker.client` is not going to support Apple Silicons as it has already been archived and the [jnr-unixsocket](https://mvnrepository.com/artifact/com.github.jnr/jnr-unixsocket) 0.18 it uses is not compatible with Apple Silicons. If we run our docker IT tests on Apple Silicons, it will fail like ```java [info] org.apache.spark.sql.jdbc.MariaDBKrbIntegrationSuite *** ABORTED *** (2 seconds, 264 milliseconds) [info] com.spotify.docker.client.exceptions.DockerException: java.util.concurrent.ExecutionException: com.spotify.docker.client.shaded.javax.ws.rs.ProcessingException: java.lang.UnsatisfiedLinkError: could not load FFI provider jnr.ffi.provider.jffi.Provider ... [info] Cause: java.lang.IllegalStateException: Can't overwrite cause with java.lang.UnsatisfiedLinkError: java.lang.UnsatisfiedLinkError: /Users/hzyaoqin/spark/target/tmp/jffi15403099445119552969.dylib: dlopen(/Users/hzyaoqin/spark/target/tmp/jffi15403099445119552969.dylib, 0x0001): tried: '/Users/hzyaoqin/spark/target/tmp/jffi15403099445119552969.dylib' (fat file, but missing compatible architecture (have 'i386,x86_64', need 'arm64')), '/System/Volumes/Preboot/Cryptexes/OS/Users/hzyaoqin/spark/target/tmp/jffi15403099445119552969.dylib' (no such file), '/Users/hzyaoqin/spark/target/tmp/jffi15403099445119552969.dylib' (fat file, but missing compatible architecture (have 'i386,x86_64', need 'arm64')) ``` In this PR, we use its alternative to enable docker-related tests on Apple Chips ```xml <dependency> <groupId>com.github.docker-java</groupId> <artifactId>docker-java</artifactId> <scope>test</scope> </dependency> ``` ### Why are the changes needed? For developers who use Apple Silicons, w/ this patch, they can test JDBC/Docker Integration test locally instead of suffering slowness from GitHub actions. ### Does this PR introduce _any_ user-facing change? No, dev only ### How was this patch tested? Pass the CIs and do the manual test on Apple Silicon. ``` $ build/sbt -Pdocker-integration-tests 'docker-integration-tests/testOnly org.apache.spark.sql.jdbc.*MariaDB*' ... [info] All tests passed. [success] Total time: 157 s (02:37), completed Sep 27, 2024, 2:45:16 PM $ build/sbt -Pdocker-integration-tests 'docker-integration-tests/testOnly org.apache.spark.sql.jdbc.*MySQL*' ... [info] All tests passed. [success] Total time: 109 s (01:49), completed Sep 27, 2024, 2:48:47 PM ``` ### Was this patch authored or co-authored using generative AI tooling? No. Closes #48289 from dongjoon-hyun/SPARK-46525. Authored-by: Kent Yao <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
…e Silicon This is a merged backport of SPARK-46525 with the original authorship, yaooqinn . - #44509 - #44612 - #45303 `com.spotify.docker.client` is not going to support Apple Silicons as it has already been archived and the [jnr-unixsocket](https://mvnrepository.com/artifact/com.github.jnr/jnr-unixsocket) 0.18 it uses is not compatible with Apple Silicons. If we run our docker IT tests on Apple Silicons, it will fail like ```java [info] org.apache.spark.sql.jdbc.MariaDBKrbIntegrationSuite *** ABORTED *** (2 seconds, 264 milliseconds) [info] com.spotify.docker.client.exceptions.DockerException: java.util.concurrent.ExecutionException: com.spotify.docker.client.shaded.javax.ws.rs.ProcessingException: java.lang.UnsatisfiedLinkError: could not load FFI provider jnr.ffi.provider.jffi.Provider ... [info] Cause: java.lang.IllegalStateException: Can't overwrite cause with java.lang.UnsatisfiedLinkError: java.lang.UnsatisfiedLinkError: /Users/hzyaoqin/spark/target/tmp/jffi15403099445119552969.dylib: dlopen(/Users/hzyaoqin/spark/target/tmp/jffi15403099445119552969.dylib, 0x0001): tried: '/Users/hzyaoqin/spark/target/tmp/jffi15403099445119552969.dylib' (fat file, but missing compatible architecture (have 'i386,x86_64', need 'arm64')), '/System/Volumes/Preboot/Cryptexes/OS/Users/hzyaoqin/spark/target/tmp/jffi15403099445119552969.dylib' (no such file), '/Users/hzyaoqin/spark/target/tmp/jffi15403099445119552969.dylib' (fat file, but missing compatible architecture (have 'i386,x86_64', need 'arm64')) ``` In this PR, we use its alternative to enable docker-related tests on Apple Chips ```xml <dependency> <groupId>com.github.docker-java</groupId> <artifactId>docker-java</artifactId> <scope>test</scope> </dependency> ``` For developers who use Apple Silicons, w/ this patch, they can test JDBC/Docker Integration test locally instead of suffering slowness from GitHub actions. No, dev only Pass the CIs and do the manual test on Apple Silicon. ``` $ build/sbt -Pdocker-integration-tests 'docker-integration-tests/testOnly org.apache.spark.sql.jdbc.*MariaDB*' ... [info] All tests passed. [success] Total time: 157 s (02:37), completed Sep 27, 2024, 2:45:16 PM $ build/sbt -Pdocker-integration-tests 'docker-integration-tests/testOnly org.apache.spark.sql.jdbc.*MySQL*' ... [info] All tests passed. [success] Total time: 109 s (01:49), completed Sep 27, 2024, 2:48:47 PM ``` No. Closes #48289 from dongjoon-hyun/SPARK-46525. Authored-by: Kent Yao <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit f888d57) Signed-off-by: Dongjoon Hyun <[email protected]>
What changes were proposed in this pull request?
SPARK-46525 makes docker-integration-tests pass most tests on Apple Silicon except DB2 and Oracle. This PR modifies DockerJDBCIntegrationSuite to make it compatible with some of the third-party docker environments, such as the Colima docker environment. Developers can quickly bring up these tests for local testing after a simple pre-setup process.
Why are the changes needed?
Make it possible to test and debug locally for developers on Apple Silicon platforms.
Does this PR introduce any user-facing change?
no
How was this patch tested?
Was this patch authored or co-authored using generative AI tooling?
no