[SPARK-33759][K8S] docker entrypoint should using spark-class for spark executor

[dungdm93]

Signed-off-by: Đặng Minh Dũng dungdm93@live.com

What changes were proposed in this pull request?

In docker entrypoint.sh, spark executor should using spark-class instead of pure java command.

spark/resource-managers/kubernetes/docker/src/main/dockerfiles/spark/entrypoint.sh

Lines 70 to 102 in 8b97b19

	case "$1" in
	driver)
	shift 1
	CMD=(
	"$SPARK_HOME/bin/spark-submit"
	--conf "spark.driver.bindAddress=$SPARK_DRIVER_BIND_ADDRESS"
	--deploy-mode client
	"$@"
	)
	;;
	executor)
	shift 1
	CMD=(
	${JAVA_HOME}/bin/java
	"${SPARK_EXECUTOR_JAVA_OPTS[@]}"
	-Xms$SPARK_EXECUTOR_MEMORY
	-Xmx$SPARK_EXECUTOR_MEMORY
	-cp "$SPARK_CLASSPATH:$SPARK_DIST_CLASSPATH"
	org.apache.spark.executor.CoarseGrainedExecutorBackend
	--driver-url $SPARK_DRIVER_URL
	--executor-id $SPARK_EXECUTOR_ID
	--cores $SPARK_EXECUTOR_CORES
	--app-id $SPARK_APPLICATION_ID
	--hostname $SPARK_EXECUTOR_POD_IP
	--resourceProfileId $SPARK_RESOURCE_PROFILE_ID
	)
	;;
	*)
	echo "Non-spark-on-k8s command provided, proceeding in pass-through mode..."
	CMD=("$@")
	;;
	esac

Why are the changes needed?

In docker entrypoint.sh, spark driver using spark-submit command but spark executor using pure java command which don't load spark-env.sh in $SPARK_HOME/conf directory.
This can lead configuration mismatch between driver and executors in the cases spark-env.sh contains something like custom envvars or pre-start hooks

Does this PR introduce any user-facing change?

N/A

How was this patch tested?

[HyukjinKwon]

@dungdm93, can you file a JIRA and keep the format in the PR title? See also http://spark.apache.org/contributing.html

[HyukjinKwon]

ok to test

[HyukjinKwon]

cc @vanzin FYI

[SparkQA]

Test build #132712 has finished for PR 30738 at commit e03b4b1.

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

[SparkQA]

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37315/

[SparkQA]

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37315/

[HyukjinKwon]

WDYT @dongjoon-hyun, @tgravescs, @holdenk ?

[tgravescs]

I thought most of our launcher use straight java. ie Yarn, standalone, etc. They setup the env as necessary for the executor side.
Generally speaking many times the env is not be the same on the executor side as the driver side as well, or the files might not be on the executor, only on the launcher. You want to use a consistent env so that is passed to the executors.

So without more detail I'm against the change, but I'm not as familiar with the k8s side here so if there is something broken in the executor env can you please describe in more detail as to what is happening so we can decide on an appropriate fix.

[dungdm93]

@tgravescs I understand that env could be passed to driver & executors in k8s. But the main focus of this change is supporting pre-start hooks.

For example, in our in-house DWH system, we have a s3-compatible service with self-signed certificate. So custom CA need to be imported into both driver & executor pods.
After some short of research, we decided to wrote a scrip like keytool -import ... in spark-env.sh, then mounted it into spark pods. Then this time I realize that spark driver load that script, but spark executors are not, this is because spark executor use straight java.

[dungdm93]

FYI, we already use spark-class for executor in production since last 6 months. And now I decided to contribute it to upstream project.

[tgravescs]

so that seems like a very specific use case that could very well break other people or put more requirements on the container anyway.
Can you use the ExecutorPlugin to load the certs?
I'm not familiar with your setup or to much of the k8s secret handling but I assume this doesn't work for you either: http://spark.apache.org/docs/latest/running-on-kubernetes.html#secret-management?

[HyukjinKwon]

Let's drop this anyway since it looks going to break other users' applications very easily given the discussion above.

[tgravescs]

It would be good to hear back and see if there is an alternate solution to the problem

[dungdm93]

@tgravescs I don't know about ExecutorPlugin, which plugin should I used?
As you mention about Secret Management, I do use that, but this only mount k8s secret/configmap into executor pod, not import cert file into truststore. So, we need this feature for the second step.

[dungdm93]

@HyukjinKwon I don't understand how this change could break other users' applications.
Because if there is something wrong in spark-env.sh, it should break spark driver first. And it happens even without this change.

[dungdm93]

@tgravescs There are an other way is create an startup-hooks script in docker-entrypoint.sh like some docker official images do, e.g: mysql, postgres,...

[tgravescs]

its a matter of the confs and environment and what is in the docker images and keeping things consistent. If I create a docker image that contains the spark and spark confs with a bunch of things set in spark-env, but then when I launch my job I change the configs or specifically launch it with different config files its still going to pick up the spark-env.sh from the docker image. It shouldn't do that to keep things consistent and get reliable results.

There is a driver and executor plugin that run at launch, its a developer api so probably not great docs on it:
https://github.com/apache/spark/blob/master/core/src/main/java/org/apache/spark/api/plugin/SparkPlugin.java

it does run after start though so I'm not sure exactly when you need this to run.

[holdenk]

@dungdm93 could you use Spark's pod templates to specify a Kubernetes a pre-start hook (e.g https://www.decodingdevops.com/kubernetes-pod-lifecycle-tutorial/ )?

[dungdm93]

@tgravescs here is an example of my deployment.

apiVersion: sparkoperator.k8s.io/v1beta2
kind: SparkApplication
metadata:
  name: pyspark-pi
  namespace: default
spec:
  type: Python
  sparkVersion: "3.0.0"
  pythonVersion: "3"
  mode: cluster
  image: hub.acme.corp/library/pyspark:v3.0.0
  imagePullPolicy: Always
  mainApplicationFile: local://path/to/python/main.py
  hadoopConf:
    fs.s3a.impl:              org.apache.hadoop.fs.s3a.S3AFileSystem
    fs.s3a.endpoint:          https://minio.acme.corp:443/
    fs.s3a.path.style.access: "true"
    fs.s3a.access.key:        sample-access-key
    fs.s3a.secret.key:        sample-secret-key
    # other config here
  sparkConfigMap: spark-conf
  driver:
    cores: 1
    coreLimit: "1200m"
    memory: "512m"
    serviceAccount: spark
  executor:
    instances: 1
    cores: 1
    memory: "512m"
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: spark-conf
  namespace: default
data:
  spark-env.sh: |
    keytool -import -alias "acme.corp" -noprompt -file $SPARK_CONF_DIR/ca.crt -keystore $JAVA_HOME/lib/security/cacerts -storepass changeit
  ca.crt: |
    -----BEGIN CERTIFICATE-----
    ....
    -----END CERTIFICATE-----

A few points worth noting:

• We use PySpark instead of Scala/Java, so write an ExecutorPlugin seem overkilled.
• Spark configs, spark-env as well as ca.crt are mounted into spark pods at runtime instead of bundle it into docker image. Is that your concern?

@holdenk thanks for you suggestion. I used to use that feature but it fails in some situations because Kubernetes don't guarantee that the hook will execute before the container entrypoint. You could checkout k8s documentation here.

[dongjoon-hyun]

Hi, @dungdm93 . Apache Spark distributions provide docker files and build scripts instead of docker image. It seems that you can do the following to achieve your use cases. How do you think about that? It's just one line before you build your docker image.

$ sed -i.bak 's/${JAVA_HOME}\/bin\/java/\"\$SPARK_HOME\/bin\/spark-class\"/' kubernetes/dockerfiles/spark/entrypoint.sh
$ bin/docker-image-tool.sh -p kubernetes/dockerfiles/spark/bindings/python/Dockerfile -n build

[dungdm93]

@dongjoon-hyun Yes, It's OK.
So feel free to close this MR if it is not suitable for you.

[dongjoon-hyun]

Or, you may want to switch this PR to support -e option at docker-image-tool.sh to support a custom entrypoint.sh file. I believe we can accept that approach, @dungdm93 .

[AmplabJenkins]

Can one of the admins verify this patch?

[dongjoon-hyun]

I close this PR for now. Please feel free to reopen this if there is any change.

Happy New Year!