search

apache-spark-on-k8s / spark

Apache Spark enhanced with native Kubernetes scheduler back-end: NOTE this repository is being ARCHIVED as all new development for the kubernetes scheduler back-end is now on https://github.com/apache/spark/
https://spark.apache.org/
Apache License 2.0
612 stars 118 forks source link

Allow user-specified environment variables and secrets in the init-container #564

Closed liyinan926 closed 6 years ago

liyinan926 commented 6 years ago

What changes were proposed in this pull request?

This PR allows setting user-specified environment variables and mounting user-specified secrets in the init-container. The driver init-container gets user-specified environment variables and secrets for the driver, whereas the executor init-container gets those for the executors. This is for #562 to enable the init-container to be able to download remote dependencies from Hadoop-compatible sources, e.g., HDFS, GCS, and S3.

How was this patch tested?

Unit tests.

@mccheah @foxish @kimoonkim

foxish commented 6 years ago

Should we also have secret injection the same way?

liyinan926 commented 6 years ago

Yes, we also need that. Do you prefer doing both in the same PR or doing secrets in a separate one?

foxish commented 6 years ago

The change LGTM

foxish commented 6 years ago

Question - in some sense, the init container is an implementation detail of the executor, can we assume that the executor pod's envs and secrets should just carry over to the init container? Might let us avoid introducing new options. WDYT?

liyinan926 commented 6 years ago

That's a good question. It depends on if there are use cases that would require the init-container to have secrets/envs different than the driver/executor containers.

foxish commented 6 years ago

cc/ @erikerlandson @mccheah @ash211 In terms of usage, do you see practical cases where we may want to differentiate between the init container and the executor?

erikerlandson commented 6 years ago

I'm not aware of any use cases for special init-container env. For that matter, inheriting from executor settings doesn't even preclude additional settings if they ever did become necessary in the future.

liyinan926 commented 6 years ago

Sounds good. So we will use the relevant driver properties for the driver init-container and executor properties for the executor init-container.

foxish commented 6 years ago

Sounds good. So we will use the relevant driver properties for the driver init-container and executor properties for the executor init-container.

SGTM

liyinan926 commented 6 years ago

Updated per our discussion.

foxish commented 6 years ago

@erikerlandson @ifilonenko @mccheah can you review this please?

liyinan926 commented 6 years ago

PR updated to also mount user-specified secrets. PTAL. Thanks!

liyinan926 commented 6 years ago

Any other comments? Will merge by EOD Monday if no objection.