[SPARK-22962][K8S] Fail fast if submission client local files are used #20320

liyinan926 · 2018-01-18T20:42:40Z

What changes were proposed in this pull request?

In the Kubernetes mode, fails fast in the submission process if any submission client local dependencies are used as the use case is not supported yet.

How was this patch tested?

Unit tests, integration tests, and manual tests.

@vanzin @foxish

jiangxb1987

LGTM only some nits.

jiangxb1987 · 2018-01-18T20:50:08Z

...rnetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/DriverConfigOrchestrator.scala

      .map(_.split(","))
      .getOrElse(Array.empty[String])

+    if (existSubmissionLocalFiles(sparkJars) || existSubmissionLocalFiles(sparkFiles)) {


We can add a TODO here if this is planned to be supported in the future.

Done, and also created https://issues.apache.org/jira/browse/SPARK-23153.

jiangxb1987 · 2018-01-18T20:50:37Z

...rnetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/DriverConfigOrchestrator.scala


+    if (existSubmissionLocalFiles(sparkJars) || existSubmissionLocalFiles(sparkFiles)) {
+      throw new SparkException("The Kubernetes mode does not yet support application " +
+        "dependencies local to the submission client. It currently only allows application" +


nit: extra space in the end of line

SparkQA · 2018-01-18T20:55:36Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/12/

foxish

+1 on docs change for 2.3 modulo minor comments.
Code changes ok if we also re-validate with manual tests against http (we have a caveat with our integration tests not testing this correctly yet), gcs and HDFS. Also ok with dropping the code change for 2.3, since it's a usability and not a functionality improvement.

foxish · 2018-01-18T20:49:12Z

docs/running-on-kubernetes.md

-`SPARK_EXTRA_CLASSPATH` environment variable in your Dockerfiles.
+`SPARK_EXTRA_CLASSPATH` environment variable in your Dockerfiles. The `local://` scheme is also required when referring to
+dependencies in custom-built Docker images in `spark-submit`. Note that using application dependencies local to the submission
+client is currently not yet supported.


"application dependencies from the local file system"?

foxish · 2018-01-18T20:49:37Z

docs/running-on-kubernetes.md

 by their appropriate remote URIs. Also, application dependencies can be pre-mounted into custom-built Docker images.
 Those dependencies can be added to the classpath by referencing them with `local://` URIs and/or setting the
-`SPARK_EXTRA_CLASSPATH` environment variable in your Dockerfiles.
+`SPARK_EXTRA_CLASSPATH` environment variable in your Dockerfiles. The `local://` scheme is also required when referring to


The point above already covers that local:// is needed with custom-built images.

Yes, but that's about adding to the classpath. I wanted to make it more specific and clearer.

SparkQA · 2018-01-18T21:02:07Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/12/

SparkQA · 2018-01-18T21:02:25Z

Test build #86354 has finished for PR 20320 at commit df9cdfb.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

liyinan926 · 2018-01-18T21:44:10Z

Regarding manual tests, our integration tests cover local:// and http:// (at least for files). I will do some manual tests on http:// for jars and gs://.

SparkQA · 2018-01-18T21:45:53Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/14/

SparkQA · 2018-01-18T21:51:30Z

Test build #86357 has finished for PR 20320 at commit 8e1fffe.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-01-18T21:51:56Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/14/

liyinan926 · 2018-01-18T22:07:20Z

@foxish Manual tests to verify that using http:///https://, gcs://, and local:// dependencies works fine. Also verified that using file:// dependencies results in submission failure due to the exception.

foxish

Question - is referencing a local file as a main app resource (/home/../spark-examples.jar for example) need to be dealt with separately?

foxish · 2018-01-18T22:09:05Z

...rnetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/DriverConfigOrchestrator.scala

      .map(_.split(","))
      .getOrElse(Array.empty[String])

+    // TODO(SPARK-23153): remote once submission client local dependencies are supported.


s/remote/remove/

foxish · 2018-01-18T22:12:50Z

...rnetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/DriverConfigOrchestrator.scala


+    // TODO(SPARK-23153): remote once submission client local dependencies are supported.
+    if (existSubmissionLocalFiles(sparkJars) || existSubmissionLocalFiles(sparkFiles)) {
+      throw new SparkException("The Kubernetes mode does not yet support application " +


I'd shorten this to just "Kubernetes mode does not support referencing application dependencies in the local file system".

liyinan926 · 2018-01-18T22:15:39Z

Actually main application jar gets added to spark.jars automatically by the submission process so are treated the same as other jars on spark.jars.

foxish · 2018-01-18T22:19:41Z

Suspected that it would use the same code path - might be worth a manual test of that as well.
LGTM from my end.

liyinan926 · 2018-01-18T22:21:41Z

The manual tests I did actually use a main app jar located on gcs and http. To be specific and for record, I did the following tests:

Using a gs:// main application jar and a http:// dependency jar. Succeeded.
Using a https:// main application jar and a http:// dependency jar. Succeeded.
Using a local:// main application jar. Succeeded.
Using a file:// main application jar. Failed.
Using a file:// dependency jar. Failed.

SparkQA · 2018-01-18T22:30:27Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/17/

SparkQA · 2018-01-18T22:31:44Z

Test build #86359 has finished for PR 20320 at commit 649e34d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

jiangxb1987 · 2018-01-18T22:36:07Z

LGTM

SparkQA · 2018-01-18T22:41:48Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-spark-integration/17/

vanzin · 2018-01-18T22:44:06Z

Merging to master / 2.3.

## What changes were proposed in this pull request? In the Kubernetes mode, fails fast in the submission process if any submission client local dependencies are used as the use case is not supported yet. ## How was this patch tested? Unit tests, integration tests, and manual tests. vanzin foxish Author: Yinan Li <[email protected]> Closes #20320 from liyinan926/master. (cherry picked from commit 5d7c4ba) Signed-off-by: Marcelo Vanzin <[email protected]>

[SPARK-22962][K8S] Fail fast if submission client local files are used

df9cdfb

jiangxb1987 approved these changes Jan 18, 2018

View reviewed changes

foxish reviewed Jan 18, 2018

View reviewed changes

Addressed comments

8e1fffe

foxish reviewed Jan 18, 2018

View reviewed changes

More comments

649e34d

asfgit closed this in 5d7c4ba Jan 18, 2018

[SPARK-22962][K8S] Fail fast if submission client local files are used #20320

[SPARK-22962][K8S] Fail fast if submission client local files are used #20320

Uh oh!

Conversation

liyinan926 commented Jan 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

jiangxb1987 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jan 18, 2018

Uh oh!

foxish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jan 18, 2018

Uh oh!

SparkQA commented Jan 18, 2018

Uh oh!

liyinan926 commented Jan 18, 2018

Uh oh!

SparkQA commented Jan 18, 2018

Uh oh!

SparkQA commented Jan 18, 2018

Uh oh!

SparkQA commented Jan 18, 2018

Uh oh!

liyinan926 commented Jan 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

foxish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liyinan926 commented Jan 18, 2018

Uh oh!

foxish commented Jan 18, 2018

Uh oh!

liyinan926 commented Jan 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Jan 18, 2018

Uh oh!

SparkQA commented Jan 18, 2018

Uh oh!

jiangxb1987 commented Jan 18, 2018

Uh oh!

SparkQA commented Jan 18, 2018

Uh oh!

vanzin commented Jan 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

liyinan926 commented Jan 18, 2018 •

edited

Loading

liyinan926 commented Jan 18, 2018 •

edited

Loading

liyinan926 commented Jan 18, 2018 •

edited

Loading