You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by ramaddepally <gi...@git.apache.org> on 2018/11/16 01:04:29 UTC
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Add ability to skip building o...
GitHub user ramaddepally opened a pull request:
https://github.com/apache/spark/pull/23053
[SPARK-25957][K8S] Add ability to skip building optional k8s docker i…
…mages
## What changes were proposed in this pull request?
bin/docker-image-tool.sh tries to build all docker images (JVM, PySpark
and SparkR) by default. But not all spark distributions are built with
SparkR and hence this script will fail on such distros.
With this change,
- We should be able to skip building optional docker
images (PySpark and SparkR) by specifying -pskip or -Rskip flags.
- We autodetect if SparkR is not installed in the build and skip building
SparkR docker image.
- We skip pushing docker images that are not available locally.
## How was this patch tested?
Tested following scenarios.
- On source code and distro with SparkR support
- Run bin/docker-image-tool.sh -r <repo> -t <tag> build. Verify that JVM, PySpark and SparkR docker images are built.
- Run bin/docker-image-tool.sh -r <repo> -t <tag> -Rskip -pskip build. Verify that only JVM docker image is built. Building PySpark and SparkR images is skipped.
- On source code and distro without SparkR support
- Run bin/docker-image-tool.sh -r <repo> -t <tag> build. Verify that only JVM, PySpark docker images are built. Building SparkR image is skipped.
- On system with JVM, PySpark and SparkR images built,
- Run bin/docker-image-tool.sh -r <repo> -t <tag> push. Verify that all images are pushed to docker registry.
- On system with only JVM and PySpark images built.
- Run bin/docker-image-tool.sh -r <repo> -t <tag> push. Verify that only JVM and PySpark images are pushed. Pushing SparkR images is skipped.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ramaddepally/spark SPARK-25957
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/23053.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #23053
----
commit ae1d74376b7daa86cd44361553e48f6d508e7ae0
Author: Nagaram Prasad Addepally <ra...@...>
Date: 2018-11-15T23:51:04Z
[SPARK-25957][K8S] Add ability to skip building optional k8s docker images
bin/docker-image-tool.sh tries to build all docker images (JVM, PySpark
and SparkR) by default. But not all spark distributions are built with
SparkR and hence this script will fail on such distros.
With this change,
- We should be able to skip building optional docker
images (PySpark and SparkR) by specifying -pskip or -Rskip flags.
- We autodetect if SparkR is not installed in the build and skip building
SparkR docker image.
- We skip pushing docker images that are not available locally.
Tested following scenarios.
- On source code and distro with SparkR support
- Run bin/docker-image-tool.sh -r <repo> -t <tag> build. Verify that JVM, PySpark and SparkR docker images are built.
- Run bin/docker-image-tool.sh -r <repo> -t <tag> -Rskip -pskip build. Verify that only JVM docker image is built. Building PySpark and SparkR images is skipped.
- On source code and distro without SparkR support
- Run bin/docker-image-tool.sh -r <repo> -t <tag> build. Verify that only JVM, PySpark docker images are built. Building SparkR image is skipped.
- On system with JVM, PySpark and SparkR images built,
- Run bin/docker-image-tool.sh -r <repo> -t <tag> push. Verify that all images are pushed to docker registry.
- On system with only JVM and PySpark images built.
- Run bin/docker-image-tool.sh -r <repo> -t <tag> push. Verify that only JVM and PySpark images are pushed. Pushing SparkR images is skipped.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Add ability to skip building o...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on a diff in the pull request:
https://github.com/apache/spark/pull/23053#discussion_r234334875
--- Diff: bin/docker-image-tool.sh ---
@@ -145,6 +157,8 @@ Options:
-f file Dockerfile to build for JVM based Jobs. By default builds the Dockerfile shipped with Spark.
-p file Dockerfile to build for PySpark Jobs. Builds Python dependencies and ships with Spark.
-R file Dockerfile to build for SparkR Jobs. Builds R dependencies and ships with Spark.
+ -pskip Skip building PySpark docker image.
--- End diff --
Done. Updated the usage to document 'skip' as part of -p and -R switches.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99138/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Add ability to skip building o...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on a diff in the pull request:
https://github.com/apache/spark/pull/23053#discussion_r235108574
--- Diff: bin/docker-image-tool.sh ---
@@ -102,33 +114,37 @@ function build {
error "Failed to build Spark JVM Docker image, please refer to Docker build output for details."
fi
- docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
- -t $(image_ref spark-py) \
- -f "$PYDOCKERFILE" .
- if [ $? -ne 0 ]; then
- error "Failed to build PySpark Docker image, please refer to Docker build output for details."
+ if [ "${PYDOCKERFILE}" != "skip" ]; then
+ docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
+ -t $(image_ref spark-py) \
+ -f "$PYDOCKERFILE" .
+ if [ $? -ne 0 ]; then
+ error "Failed to build PySpark Docker image, please refer to Docker build output for details."
--- End diff --
Hi @rvesse .. script does exit with non-zero error code when error "..." is called. See error function at top of the file. It echos the message and exits with exit code 1. So indeed this script will fail in case of docker build failures.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5190/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5093/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on the issue:
https://github.com/apache/spark/pull/23053
> Can you fix the title to include the part that github wraps by default?
Done.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on the issue:
https://github.com/apache/spark/pull/23053
@vanzin .. can you please review this change?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98930/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99073/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #99073 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99073/testReport)** for PR 23053 at commit [`5ec7cb7`](https://github.com/apache/spark/commit/5ec7cb75bf476962dcabb160ff9a3672d9482bd5).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5194/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by mccheah <gi...@git.apache.org>.
Github user mccheah commented on the issue:
https://github.com/apache/spark/pull/23053
So now the utility must always build the Java image, and optionally will build python and R - is that correct? Is there any way to opt-out of building Java?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5245/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #99138 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99138/testReport)** for PR 23053 at commit [`86a11a8`](https://github.com/apache/spark/commit/86a11a88c432313b2333b183bf8b03cf3483afa8).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5245/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on the issue:
https://github.com/apache/spark/pull/23053
> Probably also want to update `docs/running-on-kubernetes.md` to make it clear that you now have to opt into building the additional language bindings
@rvesse ... Actually, the usage in `docs/running-on-kubernetes.md` is very generic. It does not even talk about language binding dockerfiles and other options this script provides. To make it simple and generic, I can just mention see `docs/running-on-kubernetes.md -h` for more details. Does that sound good?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on the issue:
https://github.com/apache/spark/pull/23053
> You need to make the appropriate change to `setup-integration-test-env.sh` to ensure that the PySpark tests pass in the integration-test suite
Thanks for pointing that out. I will update setup-integration-test-env.sh file to get integration tests working.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on the issue:
https://github.com/apache/spark/pull/23053
I have made the changes to build language binding images as optional. Can someone please review the change again? I have updated the title and description as well to reflect the changes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #98941 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98941/testReport)** for PR 23053 at commit [`8887b5a`](https://github.com/apache/spark/commit/8887b5abd426f61d004e03918445187ccc836a46).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by mccheah <gi...@git.apache.org>.
Github user mccheah commented on the issue:
https://github.com/apache/spark/pull/23053
Hm, reviewing this again I think a better way to do it is to only opt-in to building images of different bindings. So for example the Python image should only be built when `-p <file>` is specified. It seems strange that by default the utility is building all images.
Perhaps a better interface for this entry point is this:
```
bin/docker-image-tool.sh -f <path to dockerfile> -j <optional java override> -p <optional python override>
```
What does everyone think"?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on the issue:
https://github.com/apache/spark/pull/23053
Thanks for all the comments @mccheah @ifilonenko @rvesse. So it looks like ppl are in favor of opt-in behavior for the alternate language binding images. I will update this pull request accordingly in a bit.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #99073 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99073/testReport)** for PR 23053 at commit [`5ec7cb7`](https://github.com/apache/spark/commit/5ec7cb75bf476962dcabb160ff9a3672d9482bd5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/23053
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Add ability to skip building o...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on a diff in the pull request:
https://github.com/apache/spark/pull/23053#discussion_r235109184
--- Diff: bin/docker-image-tool.sh ---
@@ -41,6 +41,18 @@ function image_ref {
echo "$image"
}
+function docker_push {
+ local image_name="$1"
+ if [ ! -z $(docker images -q "$(image_ref ${image_name})") ]; then
+ docker push "$(image_ref ${image_name})"
+ if [ $? -ne 0 ]; then
+ error "Failed to push $image_name Docker image."
+ fi
+ else
+ echo "$(image_ref ${image_name}) image not found. Skipping push for this image."
--- End diff --
We do fail with non zero exit code when build or push fails. We are skipping push only in case or images not found.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/23053
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Add ability to skip building o...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on a diff in the pull request:
https://github.com/apache/spark/pull/23053#discussion_r235108873
--- Diff: bin/docker-image-tool.sh ---
@@ -41,6 +41,18 @@ function image_ref {
echo "$image"
}
+function docker_push {
+ local image_name="$1"
+ if [ ! -z $(docker images -q "$(image_ref ${image_name})") ]; then
+ docker push "$(image_ref ${image_name})"
+ if [ $? -ne 0 ]; then
+ error "Failed to push $image_name Docker image."
--- End diff --
Same comment as above. Script does exit with non-zero exit code when push fails.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98941/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Add ability to skip building o...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on a diff in the pull request:
https://github.com/apache/spark/pull/23053#discussion_r235108733
--- Diff: bin/docker-image-tool.sh ---
@@ -102,33 +114,37 @@ function build {
error "Failed to build Spark JVM Docker image, please refer to Docker build output for details."
fi
- docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
- -t $(image_ref spark-py) \
- -f "$PYDOCKERFILE" .
- if [ $? -ne 0 ]; then
- error "Failed to build PySpark Docker image, please refer to Docker build output for details."
+ if [ "${PYDOCKERFILE}" != "skip" ]; then
+ docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
+ -t $(image_ref spark-py) \
+ -f "$PYDOCKERFILE" .
+ if [ $? -ne 0 ]; then
+ error "Failed to build PySpark Docker image, please refer to Docker build output for details."
+ fi
+ else
+ echo "Skipped building PySpark docker image."
+ fi
+
+ if [ "${RDOCKERFILE}" != "skip" ]; then
+ if [ -d "${SPARK_HOME}/R/lib" ]; then
+ docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
+ -t $(image_ref spark-r) \
+ -f "$RDOCKERFILE" .
+ if [ $? -ne 0 ]; then
+ error "Failed to build SparkR Docker image, please refer to Docker build output for details."
--- End diff --
Same comment as above. Script does exit with non-zero exit code in this case.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5093/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by rvesse <gi...@git.apache.org>.
Github user rvesse commented on the issue:
https://github.com/apache/spark/pull/23053
Probably also want to update `docs/running-on-kubernetes.md` to make it clear that you now have to opt into building the additional language bindings
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Add ability to skip building o...
Posted by rvesse <gi...@git.apache.org>.
Github user rvesse commented on a diff in the pull request:
https://github.com/apache/spark/pull/23053#discussion_r234968130
--- Diff: bin/docker-image-tool.sh ---
@@ -102,33 +114,37 @@ function build {
error "Failed to build Spark JVM Docker image, please refer to Docker build output for details."
fi
- docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
- -t $(image_ref spark-py) \
- -f "$PYDOCKERFILE" .
- if [ $? -ne 0 ]; then
- error "Failed to build PySpark Docker image, please refer to Docker build output for details."
+ if [ "${PYDOCKERFILE}" != "skip" ]; then
+ docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
+ -t $(image_ref spark-py) \
+ -f "$PYDOCKERFILE" .
+ if [ $? -ne 0 ]; then
+ error "Failed to build PySpark Docker image, please refer to Docker build output for details."
+ fi
+ else
+ echo "Skipped building PySpark docker image."
+ fi
+
+ if [ "${RDOCKERFILE}" != "skip" ]; then
+ if [ -d "${SPARK_HOME}/R/lib" ]; then
+ docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
+ -t $(image_ref spark-r) \
+ -f "$RDOCKERFILE" .
+ if [ $? -ne 0 ]; then
+ error "Failed to build SparkR Docker image, please refer to Docker build output for details."
--- End diff --
Again, script should bail out i.e. `exit` if the image build fails
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by mccheah <gi...@git.apache.org>.
Github user mccheah commented on the issue:
https://github.com/apache/spark/pull/23053
My understanding is that K8s is still marked as experimental, and something like this can be changed with the expectation that we're hardening in 3.0. If anything, we should be making this breaking change now because we want to set the standard for 3.0.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by rvesse <gi...@git.apache.org>.
Github user rvesse commented on the issue:
https://github.com/apache/spark/pull/23053
@ramaddepally Sorry, clearly not reading straight today!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on the issue:
https://github.com/apache/spark/pull/23053
You need to make the appropriate change to `setup-integration-test-env.sh` to ensure that the PySpark tests pass in the integration-test suite
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5245/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on the issue:
https://github.com/apache/spark/pull/23053
> > You need to make the appropriate change to `setup-integration-test-env.sh` to ensure that the PySpark tests pass in the integration-test suite
>
> Thanks for pointing that out. I will update setup-integration-test-env.sh file to get integration tests working.
Updated the setup-integration-test-env.sh file and k8s integration-tests are successful now.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on the issue:
https://github.com/apache/spark/pull/23053
I agree with @mccheah, `-p skip` seems odd. It seems cleaner to only include i.e. `-p` flag when we desire python support.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #99072 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99072/testReport)** for PR 23053 at commit [`11080c9`](https://github.com/apache/spark/commit/11080c973407b6ffdb23a33f202a3cef0335a4f7).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5093/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5191/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5087/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Add ability to skip building o...
Posted by rvesse <gi...@git.apache.org>.
Github user rvesse commented on a diff in the pull request:
https://github.com/apache/spark/pull/23053#discussion_r234968048
--- Diff: bin/docker-image-tool.sh ---
@@ -41,6 +41,18 @@ function image_ref {
echo "$image"
}
+function docker_push {
+ local image_name="$1"
+ if [ ! -z $(docker images -q "$(image_ref ${image_name})") ]; then
+ docker push "$(image_ref ${image_name})"
+ if [ $? -ne 0 ]; then
+ error "Failed to push $image_name Docker image."
+ fi
+ else
+ echo "$(image_ref ${image_name}) image not found. Skipping push for this image."
--- End diff --
Currently the script could incorrectly go down this path if an image build fails because it doesn't exit in that case
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Add ability to skip building o...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/23053#discussion_r234325912
--- Diff: bin/docker-image-tool.sh ---
@@ -145,6 +157,8 @@ Options:
-f file Dockerfile to build for JVM based Jobs. By default builds the Dockerfile shipped with Spark.
-p file Dockerfile to build for PySpark Jobs. Builds Python dependencies and ships with Spark.
-R file Dockerfile to build for SparkR Jobs. Builds R dependencies and ships with Spark.
+ -pskip Skip building PySpark docker image.
--- End diff --
I see what you did here.
But it would be better to document this as part of the "-p" and "-R" switches.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99072/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/23053
Can you fix the title to include the part that github wraps by default?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5191/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #98930 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98930/testReport)** for PR 23053 at commit [`8887b5a`](https://github.com/apache/spark/commit/8887b5abd426f61d004e03918445187ccc836a46).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5194/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #99079 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99079/testReport)** for PR 23053 at commit [`7601cbc`](https://github.com/apache/spark/commit/7601cbcd6a1ef20be8f6f74fdaa350418665f71d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on the issue:
https://github.com/apache/spark/pull/23053
@vanzin @ifilonenko @squito ... can you of you review and commit this change if you have no more objections. This has already passed all the tests.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/5191/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #98929 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98929/testReport)** for PR 23053 at commit [`ae1d743`](https://github.com/apache/spark/commit/ae1d74376b7daa86cd44361553e48f6d508e7ae0).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on the issue:
https://github.com/apache/spark/pull/23053
> > Probably also want to update `docs/running-on-kubernetes.md` to make it clear that you now have to opt into building the additional language bindings
>
> @rvesse ... Actually, the usage in `docs/running-on-kubernetes.md` is very generic. It does not even talk about language binding dockerfiles and other options this script provides. To make it simple and generic, I can just mention see `docs/running-on-kubernetes.md -h` for more details. Does that sound good?
Never mind.. I have just added additional content to running-on-kubernetes.md to reflect the changes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on the issue:
https://github.com/apache/spark/pull/23053
> Hm, reviewing this again I think a better way to do it is to only opt-in to building images of different bindings. So for example the Python image should only be built when `-p <file>` is specified. It seems strange that by default the utility is building all images.
>
> Perhaps a better interface for this entry point is this:
>
> ```
> bin/docker-image-tool.sh -f <path to dockerfile> -j <optional java override> -p <optional python override>
> ```
> What does everyone think"?
I do not mind this approach, where we build JVM image by default and build pyspark and SparkR (and other optional images) on demand. But changing the behavior means, breaking ppl out there that are already using this script. If we are OK to do such change, then I can modify the script accordingly.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #98941 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98941/testReport)** for PR 23053 at commit [`8887b5a`](https://github.com/apache/spark/commit/8887b5abd426f61d004e03918445187ccc836a46).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5194/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Add ability to skip building o...
Posted by rvesse <gi...@git.apache.org>.
Github user rvesse commented on a diff in the pull request:
https://github.com/apache/spark/pull/23053#discussion_r234967639
--- Diff: bin/docker-image-tool.sh ---
@@ -102,33 +114,37 @@ function build {
error "Failed to build Spark JVM Docker image, please refer to Docker build output for details."
fi
- docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
- -t $(image_ref spark-py) \
- -f "$PYDOCKERFILE" .
- if [ $? -ne 0 ]; then
- error "Failed to build PySpark Docker image, please refer to Docker build output for details."
+ if [ "${PYDOCKERFILE}" != "skip" ]; then
+ docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
+ -t $(image_ref spark-py) \
+ -f "$PYDOCKERFILE" .
+ if [ $? -ne 0 ]; then
+ error "Failed to build PySpark Docker image, please refer to Docker build output for details."
--- End diff --
Script should bail out here, i.e. `exit` if the `docker build` fails
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by ifilonenko <gi...@git.apache.org>.
Github user ifilonenko commented on the issue:
https://github.com/apache/spark/pull/23053
LGTM. I will let @mccheah or @squito sign off on it with any additional comments / reviews
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #99072 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99072/testReport)** for PR 23053 at commit [`11080c9`](https://github.com/apache/spark/commit/11080c973407b6ffdb23a33f202a3cef0335a4f7).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99079/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Add ability to skip building o...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/23053#discussion_r234326069
--- Diff: bin/docker-image-tool.sh ---
@@ -102,33 +114,33 @@ function build {
error "Failed to build Spark JVM Docker image, please refer to Docker build output for details."
fi
- docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
- -t $(image_ref spark-py) \
- -f "$PYDOCKERFILE" .
+ if [ "${PYDOCKERFILE}" != "skip" ]; then
+ docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
+ -t $(image_ref spark-py) \
+ -f "$PYDOCKERFILE" .
+ if [ $? -ne 0 ]; then
+ error "Failed to build PySpark Docker image, please refer to Docker build output for details."
+ fi
+ else
+ echo "Skipped building PySpark docker image."
+ fi
+
+ if [ "${RDOCKERFILE}" != "skip" ] && [ -d "${SPARK_HOME}/R/lib" ]; then
+ docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
+ -t $(image_ref spark-r) \
+ -f "$RDOCKERFILE" .
if [ $? -ne 0 ]; then
- error "Failed to build PySpark Docker image, please refer to Docker build output for details."
+ error "Failed to build SparkR Docker image, please refer to Docker build output for details."
fi
- docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
- -t $(image_ref spark-r) \
- -f "$RDOCKERFILE" .
- if [ $? -ne 0 ]; then
- error "Failed to build SparkR Docker image, please refer to Docker build output for details."
+ else
+ echo "Skipped building SparkR docker image."
--- End diff --
If would be nice to have a more explicit message when R artifacts are not detected.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #98929 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98929/testReport)** for PR 23053 at commit [`ae1d743`](https://github.com/apache/spark/commit/ae1d74376b7daa86cd44361553e48f6d508e7ae0).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Add ability to skip building o...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on a diff in the pull request:
https://github.com/apache/spark/pull/23053#discussion_r234335016
--- Diff: bin/docker-image-tool.sh ---
@@ -102,33 +114,33 @@ function build {
error "Failed to build Spark JVM Docker image, please refer to Docker build output for details."
fi
- docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
- -t $(image_ref spark-py) \
- -f "$PYDOCKERFILE" .
+ if [ "${PYDOCKERFILE}" != "skip" ]; then
+ docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
+ -t $(image_ref spark-py) \
+ -f "$PYDOCKERFILE" .
+ if [ $? -ne 0 ]; then
+ error "Failed to build PySpark Docker image, please refer to Docker build output for details."
+ fi
+ else
+ echo "Skipped building PySpark docker image."
+ fi
+
+ if [ "${RDOCKERFILE}" != "skip" ] && [ -d "${SPARK_HOME}/R/lib" ]; then
+ docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
+ -t $(image_ref spark-r) \
+ -f "$RDOCKERFILE" .
if [ $? -ne 0 ]; then
- error "Failed to build PySpark Docker image, please refer to Docker build output for details."
+ error "Failed to build SparkR Docker image, please refer to Docker build output for details."
fi
- docker build $NOCACHEARG "${BINDING_BUILD_ARGS[@]}" \
- -t $(image_ref spark-r) \
- -f "$RDOCKERFILE" .
- if [ $? -ne 0 ]; then
- error "Failed to build SparkR Docker image, please refer to Docker build output for details."
+ else
+ echo "Skipped building SparkR docker image."
--- End diff --
Added explicit message when building image is skipped due to R artifacts not found.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Make building alternate langua...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/23053
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5088/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #99079 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99079/testReport)** for PR 23053 at commit [`7601cbc`](https://github.com/apache/spark/commit/7601cbcd6a1ef20be8f6f74fdaa350418665f71d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23053
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98929/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23053: [SPARK-25957][K8S] Add ability to skip building o...
Posted by rvesse <gi...@git.apache.org>.
Github user rvesse commented on a diff in the pull request:
https://github.com/apache/spark/pull/23053#discussion_r234968357
--- Diff: bin/docker-image-tool.sh ---
@@ -41,6 +41,18 @@ function image_ref {
echo "$image"
}
+function docker_push {
+ local image_name="$1"
+ if [ ! -z $(docker images -q "$(image_ref ${image_name})") ]; then
+ docker push "$(image_ref ${image_name})"
+ if [ $? -ne 0 ]; then
+ error "Failed to push $image_name Docker image."
--- End diff --
Script should really fail i.e. `exit` if a push fails
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by mccheah <gi...@git.apache.org>.
Github user mccheah commented on the issue:
https://github.com/apache/spark/pull/23053
Ok that makes sense. I will merge this into master then.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Add ability to skip building optional...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #98930 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98930/testReport)** for PR 23053 at commit [`8887b5a`](https://github.com/apache/spark/commit/8887b5abd426f61d004e03918445187ccc836a46).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on the issue:
https://github.com/apache/spark/pull/23053
@ifilonenko @vanzin @squito @mccheah ... can one of you please submit this change if you do not have any further concerns/comments. Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by ramaddepally <gi...@git.apache.org>.
Github user ramaddepally commented on the issue:
https://github.com/apache/spark/pull/23053
> So now the utility must always build the Java image, and optionally will build python and R - is that correct? Is there any way to opt-out of building Java?
Yes.. you are correct. We always build Java image. Java image is the base image for building python and R language binding images and it is a must build image. See https://github.com/apache/spark/blob/master/bin/docker-image-tool.sh#L92.
So we will not skip it anytime.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23053: [SPARK-25957][K8S] Make building alternate language bind...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23053
**[Test build #99138 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99138/testReport)** for PR 23053 at commit [`86a11a8`](https://github.com/apache/spark/commit/86a11a88c432313b2333b183bf8b03cf3483afa8).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org