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

Add support for fetching application dependencies from HDFS #585

Closed hex108 closed 5 years ago

hex108 commented 6 years ago

What changes were proposed in this pull request?

Fix #584

How was this patch tested?

Manual tests as in #584.

liyinan926 commented 6 years ago

We already have org.apache.spark.deploy.k8s. HadoopConfBootstrapImpl that has the logic for mounting the HADOOP_FILE_VOLUME into a container.

hex108 commented 6 years ago

hi @liyinan926 , if I understand correctly, HadoopConfBootstrapImpl is used to mount Hadoop conf to driver/executor pod's main container, and now we only mount Hadoop conf to driver/executor container. However we also need mount it to init container to fetch HDFS dependencies.

liyinan926 commented 6 years ago

@hex108 Yes, it is for mounting the Hadoop conf ConfigMap into the driver and executor containers. My point is the logic is duplicated, so should be extracted out and shared. For example, we have MountSecretBootstrap that is shared for mounting secrets into the main and init containers in both the driver and executor pods. I would suggest renaming MountHadoopConfStep to something that has Bootstrap in the name and using it in places where mounting the ConfigMap is needed.

hex108 commented 6 years ago

@liyinan926 Thanks, I get it now. I'll refactor it.

ifilonenko commented 6 years ago

If it is alright, I am currently working on integration tests for hdfs (secure and non), after a few LGTM and after passing the tests that I write, we can merge this PR, after resolving comments. Want to mitigate changes to HDFS until there are integration test in place to validate the logic e2e.

Furthermore this PR is missing unit tests, can you please add?

Thanks for this work!