[SPARK-22777][Scheduler] Kubernetes mode dockerfile permission and distribution #20007
Conversation
| # Only create and copy the dockerfiles directory if the kubernetes artifacts were built. | ||
| if [ -d "$SPARK_HOME"/resource-managers/kubernetes/core/target/ ]; then | ||
| mkdir -p "$DISTDIR/kubernetes/" | ||
| cp -a "$SPARK_HOME"/resource-managers/kubernetes/docker/src/main/dockerfiles "$DISTDIR/kubernetes/" |
There was a problem hiding this comment.
Could you expand more on why "-a" is needed? Thanks!
There was a problem hiding this comment.
/resource-managers/kubernetes/docker/src/main/dockerfiles is a directory? how about /resource-managers/kubernetes/docker/src/main/dockerfiles/?
| # Only create and copy the dockerfiles directory if the kubernetes artifacts were built. | ||
| if [ -d "$SPARK_HOME"/resource-managers/kubernetes/core/target/ ]; then | ||
| mkdir -p "$DISTDIR/kubernetes/" | ||
| cp -a "$SPARK_HOME"/resource-managers/kubernetes/docker/src/main/dockerfiles "$DISTDIR/kubernetes/" |
There was a problem hiding this comment.
According to build-push-docker-images.sh in #19946, seems like the directories for Dockerfile are under dockerfiles instead of kubernetes/dockerfiles. Will you update the file later?
There was a problem hiding this comment.
Yes, will be updating the docs in that PR. Thanks!
|
Yes, I'll be updating the docs. The '-a' tries to preserve permissions and
all other attributes when copying those directories (
http://man7.org/linux/man-pages/man1/cp.1.html). It's equivalent to
specifying -R and --preserve=all. It's important that the entrypoint script
remain executable or it leads to runtime errors.
…On Dec 18, 2017 5:26 AM, "Takuya UESHIN" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In dev/make-distribution.sh
<#20007 (comment)>:
> if [ -f "$SPARK_HOME"/common/network-yarn/target/scala*/spark-*-yarn-shuffle.jar ]; then
mkdir "$DISTDIR/yarn"
cp "$SPARK_HOME"/common/network-yarn/target/scala*/spark-*-yarn-shuffle.jar "$DISTDIR/yarn"
fi
+# Only create and copy the dockerfiles directory if the kubernetes artifacts were built.
+if [ -d "$SPARK_HOME"/resource-managers/kubernetes/core/target/ ]; then
+ mkdir -p "$DISTDIR/kubernetes/"
+ cp -a "$SPARK_HOME"/resource-managers/kubernetes/docker/src/main/dockerfiles "$DISTDIR/kubernetes/"
According to build-push-docker-images.sh in #19946
<#19946>, seems like the directories
for Dockerfile are under dockerfiles instead of kubernetes/dockerfiles.
Will you update the file later?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#20007 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA3U5yEjU9RXCnQ9nN_jLYVjI9B1Jcmvks5tBmgjgaJpZM4RFQlT>
.
|
|
LGTM |
|
LGTM. |
|
Test build #85059 has finished for PR 20007 at commit
|
| # Only create and copy the dockerfiles directory if the kubernetes artifacts were built. | ||
| if [ -d "$SPARK_HOME"/resource-managers/kubernetes/core/target/ ]; then | ||
| mkdir -p "$DISTDIR/kubernetes/" | ||
| cp -a "$SPARK_HOME"/resource-managers/kubernetes/docker/src/main/dockerfiles "$DISTDIR/kubernetes/" |
There was a problem hiding this comment.
/resource-managers/kubernetes/docker/src/main/dockerfiles is a directory? how about /resource-managers/kubernetes/docker/src/main/dockerfiles/?
|
700 might break openshift that doesn't run these containers as root. @erikerlandson can you verify? |
|
Yes, by default in openshift, containers run as an anonymous uid, and as group id 0. So there are a few things that need to be given access to gid 0. I asked some of our security people and they considered these mods to be acceptable. |
|
@vanzin PTAL |
|
If we wanted to lock down permissions a bit more, we might consider |
|
Merging to master. |
What changes were proposed in this pull request? This PR contains documentation on the usage of Kubernetes scheduler in Spark 2.3, and a shell script to make it easier to build docker images required to use the integration. The changes detailed here are covered by #19717 and #19468 which have merged already. How was this patch tested? The script has been in use for releases on our fork. Rest is documentation. cc rxin mateiz (shepherd) k8s-big-data SIG members & contributors: foxish ash211 mccheah liyinan926 erikerlandson ssuchter varunkatta kimoonkim tnachen ifilonenko reviewers: vanzin felixcheung jiangxb1987 mridulm TODO: - [x] Add dockerfiles directory to built distribution. (#20007) - [x] Change references to docker to instead say "container" (#19995) - [x] Update configuration table. - [x] Modify spark.kubernetes.allocation.batch.delay to take time instead of int (#20032) Author: foxish <[email protected]> Closes #19946 from foxish/update-k8s-docs.
What changes were proposed in this pull request?
How was this patch tested?
Manual testing
cc/ @ueshin @jiangxb1987 @mridulm @vanzin @rxin @liyinan926