[SPARK-22648][K8s] Add documentation covering init containers and secrets #20059

liyinan926 · 2017-12-22T22:12:25Z

What changes were proposed in this pull request?

This PR updates the Kubernetes documentation corresponding to the following features/changes in #19954.

Ability to use remote dependencies through the init-container.
Ability to mount user-specified secrets into the driver and executor pods.

@vanzin @jiangxb1987 @foxish

…19954

foxish

Thanks for this PR @liyinan926. Left some comments, PTAL. Reviewers - the documentation is for the as-yet unmerged PR #19954. PTAL.

foxish · 2017-12-22T22:17:00Z

docs/running-on-kubernetes.md

+  <td>
+   Add the secret named <code>SecretName</code> to the executor pod on the path specified in the value. For example,
+   <code>spark.kubernetes.executor.secrets.spark-secret=/etc/secrets</code>. Note that if an init-container is used,
+   the secret will also be add to the init-container in the executor pod.


s/add/added/

foxish · 2017-12-22T22:19:04Z

docs/running-on-kubernetes.md

+  <td><code>spark.kubernetes.mountDependencies.mountTimeout</code></td>
+  <td>5 minutes</td>
+  <td>
+   Timeout before aborting the attempt to download and unpack dependencies from remote locations when initializing


Let's be more precise on what operation is happening.
"initializing" -> "when downloading and unpacking dependencies into"

foxish · 2017-12-22T22:19:39Z

docs/running-on-kubernetes.md

+  <td><code>spark.kubernetes.driver.secrets.[SecretName]</code></td>
+  <td>(none)</td>
+  <td>
+   Add the secret named <code>SecretName</code> to the driver pod on the path specified in the value. For example,


secret -> Kubernetes Secret
Please also link to the Kubernetes secrets docs page.

foxish · 2017-12-22T22:19:47Z

docs/running-on-kubernetes.md

+  <td>
+   Add the secret named <code>SecretName</code> to the driver pod on the path specified in the value. For example,
+   <code>spark.kubernetes.driver.secrets.spark-secret=/etc/secrets</code>. Note that if an init-container is used,
+   the secret will also be add to the init-container in the driver pod.


s/add/added/

foxish · 2017-12-22T22:20:10Z

docs/running-on-kubernetes.md

+  <td><code>spark.kubernetes.initContainer.image</code></td>
+  <td>(none)</td>
+  <td>
+   Container image for the init-container of the driver and executors for downloading dependencies.


Link to init-container docs.

foxish · 2017-12-22T22:22:54Z

docs/running-on-kubernetes.md

+  </td>
+</tr>
+<tr>
+  <td><code>spark.kubernetes.mountDependencies.mountTimeout</code></td>


We should rename this to mountDependencies.timeout to avoid reiterating "mount"

foxish · 2017-12-22T22:23:19Z

docs/running-on-kubernetes.md

+  </td>
+</tr>
+<tr>
+  <td><code>spark.kubernetes.initContainer.maxThreadPoolSize</code></td>


I feel like this option name needs fixing. Maybe spark.kubernetes.mountDependencies.maxThreadPoolSize?

liyinan926 · 2017-12-22T22:48:23Z

@foxish addressed your comments.

foxish

Thanks for addressing those. Sorry, missed a couple of minor ones.

foxish · 2017-12-22T22:53:55Z

docs/running-on-kubernetes.md

@@ -120,6 +120,23 @@ by their appropriate remote URIs. Also, application dependencies can be pre-moun
 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.

+### Using Remote Dependencies
+When there are application dependencies hosted in remote locations like HDFS or HTTP servers, the driver and executor pods need a Kubernetes [init-container](https://kubernetes.io/docs/concepts/workloads/pods/init-containers/) for downloading the dependencies so the driver and executor containers can use them locally. This requires users to specify the container image for the init-container using the configuration property `spark.kubernetes.initContainer.image`. For example, users simply add the following option to the `spark-submit` command to specify the init-container image:


This and below text should be broken up into multiple lines.

Maybe we should include 2-3 examples of remote file usage - ideally, showing that one can use http, hdfs, gcs, s3 in dependencies.

Do we need to break them into lines? I thought this should be automatically wrapped when being viewed.

Regarding examples, I can add one spark-submit example showing how to use remote jars/files on http/https and hdfs. But gcs requires the connector in the init-container, which is non-trivial. I'm not sure about s3. I think we should avoid doing so.

HDFS or HTTP sound good. We can cover GCS elsewhere. Line breaks were for ease of reviewing by others (being able to comment on individual lines) and for consistency with the rest of the docs.

liyinan926 · 2017-12-22T23:15:55Z

Updated in fbb2112.

vanzin

Looks ok. Can you use "k8s" to save space in the PR title? And also make it self-contained instead of referencing another PR?

e.g.

"Add documentation covering init containers and secrets."

vanzin · 2017-12-23T00:03:03Z

docs/running-on-kubernetes.md

+```
+
+## Secret Management
+In some cases, a Spark application may need to use some credentials, e.g., for accessing data on a secured HDFS cluster


I'd rewrite this.

"Kubernetes Secrets can be used to provide credentials for a Spark application to access secured services. To mount secrets into a driver container, ..."

vanzin · 2017-12-23T00:05:51Z

docs/running-on-kubernetes.md

+  <td><code>spark.kubernetes.mountDependencies.maxThreadPoolSize</code></td>
+  <td>5</td>
+  <td>
+   Maximum size of the thread pool for downloading remote dependencies into the driver and executor pods.


I'd clarify this controls how many downloads happen simultaneously; could even change the name of the config to reflect that.

SparkQA · 2017-12-23T01:06:26Z

Test build #85321 has finished for PR 20059 at commit c0a659a.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-12-23T02:12:50Z

Test build #85323 has finished for PR 20059 at commit a6a6660.

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

SparkQA · 2017-12-23T02:37:13Z

Test build #85325 has finished for PR 20059 at commit fbb2112.

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

SparkQA · 2017-12-23T06:36:28Z

Test build #85331 has finished for PR 20059 at commit f23bf0f.

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

SparkQA · 2017-12-25T21:46:56Z

Test build #85385 has finished for PR 20059 at commit 818abaf.

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

jiangxb1987

mostly LGTM

jiangxb1987 · 2017-12-26T12:34:56Z

docs/running-on-kubernetes.md

+</tr>
+<tr>
+  <td><code>spark.kubernetes.executor.secrets.[SecretName]</code></td>
+  <td>5</td>


what's the meaning of 5 here?

Hmm, copy and paste error. Fixed.

SparkQA · 2017-12-26T20:35:13Z

Test build #85411 has finished for PR 20059 at commit 08486e8.

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

ueshin · 2017-12-27T06:13:35Z

docs/running-on-kubernetes.md

+    --files hdfs://host:port/path/to/file1,hdfs://host:port/path/to/file2
+    --conf spark.executor.instances=5 \
+    --conf spark.kubernetes.driver.docker.image=<driver-image> \
+    --conf spark.kubernetes.executor.docker.image=<executor-image> \


container.image instead of docker.image. We need to modify line 79-80 as well.

ueshin · 2017-12-27T06:27:33Z

docs/running-on-kubernetes.md

+</tr>
+<tr>
+  <td><code>spark.kubernetes.mountDependencies.timeout</code></td>
+  <td>300 seconds</td>


300s instead of 300 seconds, which should be the form we can specify to the config string.

SparkQA · 2017-12-27T08:05:01Z

Test build #85428 has finished for PR 20059 at commit f4b5c03.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

jiangxb1987 · 2017-12-27T08:44:13Z

retest this please

SparkQA · 2017-12-27T12:26:32Z

Test build #85431 has finished for PR 20059 at commit f4b5c03.

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

SparkQA · 2017-12-27T22:50:40Z

Test build #85440 has finished for PR 20059 at commit 453a3db.

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

ueshin · 2017-12-28T04:53:11Z

Thanks! merging to master.

[SPARK-22648][Kubernetes] Update documentation to cover features in #…

c0a659a

…19954

foxish reviewed Dec 22, 2017

View reviewed changes

Addressed comments

fbb2112

vanzin reviewed Dec 23, 2017

View reviewed changes

liyinan926 changed the title ~~[SPARK-22648][Kubernetes] Update documentation to cover features in #19954~~ [SPARK-22648][K8s] Add documentation covering init containers and secrets Dec 23, 2017

Addressed more comments

f23bf0f

Update the unit of one configuration property

818abaf

jiangxb1987 reviewed Dec 26, 2017

View reviewed changes

Fixed the default value of a config property

08486e8

ueshin reviewed Dec 27, 2017

View reviewed changes

Addressed more comments

f4b5c03

Fixed some formatting

453a3db

asfgit closed this in ded6d27 Dec 28, 2017

[SPARK-22648][K8s] Add documentation covering init containers and secrets #20059

[SPARK-22648][K8s] Add documentation covering init containers and secrets #20059

Conversation

liyinan926 commented Dec 22, 2017

What changes were proposed in this pull request?

foxish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liyinan926 commented Dec 22, 2017

foxish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

foxish Dec 22, 2017 • edited Loading

Choose a reason for hiding this comment

liyinan926 commented Dec 22, 2017

vanzin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 23, 2017

SparkQA commented Dec 23, 2017

SparkQA commented Dec 23, 2017

SparkQA commented Dec 23, 2017

SparkQA commented Dec 25, 2017

jiangxb1987 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 27, 2017

jiangxb1987 commented Dec 27, 2017

SparkQA commented Dec 27, 2017

SparkQA commented Dec 27, 2017

ueshin commented Dec 28, 2017

foxish Dec 22, 2017 •

edited

Loading