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
foxish merged 1 commit into
apache-spark-on-k8s:branch-2.2-kubernetesfrom
tmckayus:quoteclasspath
Nov 7, 2017
Merged

Add quotes around $SPARK_CLASSPATH in Dockerfile java commands#541
foxish merged 1 commit into
apache-spark-on-k8s:branch-2.2-kubernetesfrom
tmckayus:quoteclasspath

Conversation

@tmckayus

@tmckayus tmckayus commented Nov 3, 2017

Copy link
Copy Markdown

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
Copy Markdown
Member

@sahilprasad @mccheah @foxish ptal

@liyinan926

Copy link
Copy Markdown
Member

LGTM.

@ifilonenko

Copy link
Copy Markdown
Member

Thank you for this! @ssuchter thoughts?

@erikerlandson

Copy link
Copy Markdown
Member

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

@sahilprasad

Copy link
Copy Markdown

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

@foxish

foxish commented Nov 6, 2017

Copy link
Copy Markdown
Member

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

razius commented Mar 31, 2018

Copy link
Copy Markdown

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