Skip to content
This repository was archived by the owner on Jan 9, 2020. It is now read-only.

Add quotes around $SPARK_CLASSPATH in Dockerfile java commands #541

Merged

Conversation

tmckayus
Copy link

@tmckayus tmckayus commented Nov 3, 2017

Quotes around $SPARK_CLASSPATH in the java invocations
specified in the Dockerfiles correctly prevent the shell from expanding
wildcard paths in cases where the classpath is a single value like /opt/spark/jars/*

What changes were proposed in this pull request?

Each place where $SPARK_CLASSPATH is used as the value of the -cp flag to ${JAVA_HOME}/bin/java
in the Dockerfiles under resource-managers/kubernetes/docker-minimal-bundle/src/main/docker has
been changed to include double quotes around $SPARK_CLASSPATH

How was this patch tested?

Manually. Without the change, standard instructions for running a python example fail when the unnecessary --jars flag is left off. With the change, it works correctly.

bin/spark-submit
--deploy-mode cluster
--master k8s://https://:
--kubernetes-namespace
--conf spark.executor.instances=5
--conf spark.app.name=spark-pi
--conf spark.kubernetes.driver.docker.image=kubespark/spark-driver-py:v2.2.0-kubernetes-0.5.0
--conf spark.kubernetes.executor.docker.image=kubespark/spark-executor-py:v2.2.0-kubernetes-0.5.0
local:///opt/spark/examples/src/main/python/pi.py 10

Error from the spark driver (could be any jar, depends on order of expansion by the shell)
Error: Could not find or load main class .opt.spark.jars.RoaringBitmap-0.5.11.jar

The other images were not tested explicitly, but it's the same case.

The quotes around $SPARK_CLASSPATH in the Dockerfiles prevents
the shell from expanding wildcard paths in cases where the
classpath is a single value like /opt/spark/jars/*
@erikerlandson
Copy link
Member

@sahilprasad @mccheah @foxish ptal

@liyinan926
Copy link
Member

LGTM.

@ifilonenko
Copy link
Member

Thank you for this! @ssuchter thoughts?

@erikerlandson
Copy link
Member

Link to some back-story for reference:
#464 (comment)

@sahilprasad
Copy link

@erikerlandson @tmckayus Looks great! Good to have this resolved in a cleaner way than my PR (btw, feel free to close).

@foxish
Copy link
Member

foxish commented Nov 6, 2017

LGTM, merging EOD unless someone has objections.

@foxish foxish merged commit 8f73508 into apache-spark-on-k8s:branch-2.2-kubernetes Nov 7, 2017
@razius
Copy link

razius commented Mar 31, 2018

Would it be possible to push the built image to Docker Hub as well? I can see that the current image doesn't contain this fix.

ifilonenko pushed a commit to bloomberg/apache-spark-on-k8s that referenced this pull request Apr 22, 2019
ifilonenko pushed a commit to bloomberg/apache-spark-on-k8s that referenced this pull request Jul 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants