Replace submission v1 with submission v2. #286

mccheah · 2017-05-19T23:25:05Z

Removes all of the submission V1 code and makes submission v2 the only submission mode that is used by SparkSubmit.

The package structure has been re-organized once more to remove the v1 and v2 qualifiers, changing most of the imports in the code. I also attempted to remove as much dead code as possible in files that we still use in the new submission code. Documentation has been re-written to reflect the new submission workflow that uses a resource staging server for local dependencies.

This PR also includes a sample YAML file that deploys the minimum set of resources required for a resource staging server. It includes a ConfigMap with example properties that configure the resource staging server.

mccheah · 2017-05-19T23:33:39Z

docs/running-on-kubernetes.md

+- `spark.ssl.kubernetes.resourceStagingServer.keyPasswordFile`, to provide the keyStore's key password using a file
+instead of a static value. This is useful if the key password is to be mounted into the container with a secret.
+
+Note that while the properties can be set in the ConfigMap, you will still need to consider the means of mounting the


@foxish I wonder if we can do better here. Perhaps it would be worthwhile to have a script that allows users to provide their TLS secrets from their local disk and the script then converts them to a Kubernetes secret that's mounted into the pod. Or if that's too heavyweight and we don't want to maintain that, perhaps we can have an alternate YML template that provides an outline for a Secret object and how it would be mounted into the resource staging service container.

Hmm.. I think we can wait till there is a push for using the security features extensively before making it more convenient. For now, I think this is fine.

mccheah · 2017-05-19T23:36:16Z

docs/running-on-kubernetes.md

@@ -3,23 +3,34 @@ layout: global
 title: Running Spark on Kubernetes
 ---

-Support for running on [Kubernetes](https://kubernetes.io/docs/whatisk8s/) is available in experimental status. The feature set is
-currently limited and not well-tested. This should not be used in production environments.
+Support for running on [Kubernetes](https://kubernetes.io/docs/whatisk8s/) is available in experimental status. The


@ash211 @erikerlandson @kimoonkim @lins05 @foxish

The documentation changes are by far the most important part of this change, as well as the workflow in general. This is our chance to catch any changes in the user's workflow as described in the documentation and weigh if it is still reasonable to move forward with this overhaul.

foxish · 2017-05-22T19:22:34Z

conf/kubernetes-resource-staging-server.yaml

@@ -0,0 +1,60 @@
+---
+apiVersion: extensions/v1beta1
+kind: ReplicaSet


Can we use a deployment here instead? It's a superset of the ReplicaSet in terms of features, providing rolling-update and the like. Also, I think it has recently moved to apps/v1beta1.

foxish · 2017-05-22T19:23:16Z

conf/kubernetes-resource-staging-server.yaml

+            name: spark-resource-staging-server-config
+      containers:
+        - name: spark-resource-staging-server
+          image: kubespark/spark-resource-staging-server:v2.1.0-kubernetes-0.1.0-alpha.3


Does this image exist already?

We need to publish this: https://github.com/apache-spark-on-k8s/spark/blob/branch-2.1-kubernetes/resource-managers/kubernetes/docker-minimal-bundle/src/main/docker/resource-staging-server/Dockerfile

foxish · 2017-05-22T19:24:45Z

conf/kubernetes-resource-staging-server.yaml

+        - name: spark-resource-staging-server
+          image: kubespark/spark-resource-staging-server:v2.1.0-kubernetes-0.1.0-alpha.3
+          resources:
+            requests:


Without a limit section, we will have the pods belong to the burstable QoS class. Is this the intention?

Added limits.

foxish · 2017-05-22T19:25:45Z

conf/kubernetes-resource-staging-server.yaml

+    - protocol: TCP
+      port: 10000
+      targetPort: 10000
+      nodePort: 31000


It is not typical to specify a nodeport, and should probably be called out explicitly.

This is to make the documentation and deployment instructions simpler, basically removing the steps to find the NodePort.

foxish · 2017-05-22T19:29:20Z

docs/running-on-kubernetes.md

+- `spark.ssl.kubernetes.resourceStagingServer.keyPasswordFile`, to provide the keyStore's key password using a file
+instead of a static value. This is useful if the key password is to be mounted into the container with a secret.
+
+Note that while the properties can be set in the ConfigMap, you will still need to consider the means of mounting the


Hmm.. I think we can wait till there is a push for using the security features extensively before making it more convenient. For now, I think this is fine.

foxish · 2017-05-22T19:31:09Z

Thanks for the changes. Did one cursory review. The change looks good overall; will do a more detailed pass in a day or so.

ash211 · 2017-05-22T20:33:05Z

conf/kubernetes-resource-staging-server.yaml

-apiVersion: extensions/v1beta1
-kind: ReplicaSet
+apiVersion: apps/v1beta1
+kind: Deployment


is Deployment compatible with k8s 1.5 (which as of now is "prior version" of k8s)?

Hmm.. maybe we should use extensions.v1beta1/Deployment instead of the apps group here. The apps group move happened in 1.6 (https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG.md#deployment-2). I'll verify with the team about the plan for this moving forward.

Yes, Deployment from the group extensions/v1beta1 will work on 1.5, 1.6 and 1.7, and we'll probably deprecate it in 1.8. So, I think we can use that for now, and not apps/v1beta1.

ash211 · 2017-05-22T21:36:40Z

docs/running-on-kubernetes.md

+using [kubectl](https://kubernetes.io/docs/user-guide/prereqs/). If you do not already have a working Kubernetes
+cluster, you may setup a test cluster on your local machine using
+[minikube](https://kubernetes.io/docs/getting-started-guides/minikube/).
+  * We recommend that minikube be updated to the most recent version (0.18.0 at the time of this documentation), as some


0.19.0 now -- https://github.com/kubernetes/minikube/releases

ash211 · 2017-05-22T21:38:01Z

docs/running-on-kubernetes.md

+  earlier versions may not start up the kubernetes cluster with all the necessary components.
+* You must have appropriate permissions to create and list [pods](https://kubernetes.io/docs/user-guide/pods/),
+[ConfigMaps](https://kubernetes.io/docs/tasks/configure-pod-container/configmap/) and
+[secrets](https://kubernetes.io/docs/concepts/configuration/secret/) in your cluster. You can verify that


our sample resource staging server yaml is a Deployment now -- should add that kind to this list of required perms

Strictly speaking it's not necessary to deploy the resource staging server as specified by the sample YAML file, however. We can add that separately to the documentation on running the resource staging server.

A section on "Optionality of the Resource Staging Server" would be good. I think it's basically when you have any local resources (jars or files) including the application jar, but we should spell that out

We discuss optionality here but should it be under a specific heading? https://github.com/apache-spark-on-k8s/spark/pull/286/files#diff-b5527f236b253e0d9f5db5164bdb43e9R136

I think separate heading -- it's an important distinction and determines whether the resource staging server is needed at all

ash211 · 2017-05-22T21:39:33Z

docs/running-on-kubernetes.md

-`dockerfiles/driver/Dockerfile` and `dockerfiles/executor/Dockerfile`, respectively. Use these Docker files to build the
-Docker images, and then tag them with the registry that the images should be sent to. Finally, push the images to the
-registry.
+You may also build these docker images from sources, or customize them as required. Spark distributions include the


should link customize them as required to a section about how to build a custom Docker image

We can follow up in a separate commit for documentation on creating custom Docker images.

Agreed, should do that later. That's captured as one of the items on #262 so nothing more on that for this PR

ash211 · 2017-05-22T21:42:52Z

docs/running-on-kubernetes.md

+This YAML file configures a Deployment with one pod running the resource staging server configured with a ConfigMap,
+and exposes the server through a Service with a given NodePort.
+
+To run the resource staging server with default configurations, the Kubernetes resources can simply be created:


I generally prefer avoiding the word simple in documentation -- the situation is inevitably more complicated than it seems at first and simple comes off a bit as marketing-speak

ash211 · 2017-05-22T21:43:47Z

docs/running-on-kubernetes.md

+    bin/spark-submit \
+      --deploy-mode cluster \
+      --class org.apache.spark.examples.SparkPi \
+      --master k8s://https://<k8s-apiserver-host>:<k8s-apiserver-port> \


remove the https:// here -- it looks weird and is equivalent to the k8s://-only version

ash211 · 2017-05-22T21:59:40Z

docs/running-on-kubernetes.md

-  <td><code>spark.kubernetes.driverSubmissionTimeout</code></td>
-  <td>60s</td>
+  <td><code>spark.kubernetes.resourceStagingServer.internal.uri</code></td>
+  <td>(none)</td>


defaults to the value of spark.kubernetes.resourceStagingServer.uri right?

ash211 · 2017-05-22T22:03:07Z

docs/running-on-kubernetes.md

-  <td><code>spark.kubernetes.report.interval</code></td>
-  <td><code>1s</code></td>
+  <td><code>spark.ssl.kubernetes.resourceStagingServer.internal.clientCertPem</code></td>
+  <td>(none)</td>


default of spark.ssl.kubernetes.resourceStagingServer.clientCertPem

ash211 · 2017-05-22T22:03:16Z

docs/running-on-kubernetes.md

-  <td><code>spark.kubernetes.submission.waitAppCompletion</code></td>
-  <td><code>true</code></td>
+  <td><code>spark.ssl.kubernetes.resourceStagingServer.internal.trustStoreType</code></td>
+  <td>(none)</td>


default of spark.ssl.kubernetes.resourceStagingServer.trustStoreType

ash211 · 2017-05-22T22:03:26Z

docs/running-on-kubernetes.md

-  <td><code>spark.kubernetes.driver.pod.name</code></td>
-  <td><code>(none)</code></td>
+  <td><code>spark.ssl.kubernetes.resourceStagingServer.internal.trustStorePassword</code></td>
+  <td>(none)</td>


default of spark.ssl.kubernetes.resourceStagingServer.trustStorePassword

ash211 · 2017-05-22T22:03:37Z

docs/running-on-kubernetes.md

-  <td><code>spark.kubernetes.driver.service.exposeUiPort</code></td>
-  <td><code>false</code></td>
+  <td><code>spark.ssl.kubernetes.resourceStagingServer.internal.trustStore</code></td>
+  <td>(none)</td>


default of spark.ssl.kubernetes.resourceStagingServer.trustStore

mccheah · 2017-05-22T22:24:07Z

rerun unit tests please

ash211

LGTM

@foxish can you take a look at this today/tomorrow?

foxish · 2017-05-23T14:10:58Z

LGTM from my end. I think a couple of @ash211's comments still need addressing however.

ash211 · 2017-05-23T18:37:28Z

I think that covers it -- will merge when the tests succeed. Thanks for the review @foxish !

* Replace submission v1 with submission v2. * Address documentation changes. * Fix documentation

mccheah force-pushed the remove-submission-v1 branch from df72c0e to ab07326 Compare May 19, 2017 23:25

mccheah commented May 19, 2017

View reviewed changes

foxish reviewed May 22, 2017

View reviewed changes

ash211 reviewed May 22, 2017

View reviewed changes

Replace submission v1 with submission v2.

7cb0420

mccheah force-pushed the remove-submission-v1 branch from ced7990 to 7cb0420 Compare May 22, 2017 21:36

mccheah changed the base branch from logging-pod-status-v2 to branch-2.1-kubernetes May 22, 2017 21:36

ash211 reviewed May 22, 2017

View reviewed changes

This was referenced May 22, 2017

Revisit submission process after alpha release #167

Closed

Simplify the way docker images are specified #289

Open

Address documentation changes.

ce8e578

ash211 approved these changes May 23, 2017

View reviewed changes

Fix documentation

267ab46

ash211 merged commit 8f3d965 into branch-2.1-kubernetes May 23, 2017

ash211 deleted the remove-submission-v1 branch May 23, 2017 19:57

foxish pushed a commit that referenced this pull request Jul 24, 2017

Replace submission v1 with submission v2. (#286)

cc5eb85

* Replace submission v1 with submission v2. * Address documentation changes. * Fix documentation

puneetloya pushed a commit to puneetloya/spark that referenced this pull request Mar 11, 2019

Replace submission v1 with submission v2. (apache-spark-on-k8s#286)

053bdd0

* Replace submission v1 with submission v2. * Address documentation changes. * Fix documentation

Replace submission v1 with submission v2. #286

Replace submission v1 with submission v2. #286

Conversation

mccheah commented May 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mccheah May 19, 2017 • edited Loading

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

Choose a reason for hiding this comment

foxish commented May 22, 2017

Choose a reason for hiding this comment

foxish May 22, 2017 • edited Loading

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mccheah commented May 22, 2017

ash211 left a comment

Choose a reason for hiding this comment

foxish commented May 23, 2017

ash211 commented May 23, 2017

mccheah May 19, 2017 •

edited

Loading

foxish May 22, 2017 •

edited

Loading