Skip to content

[SPARK-22866] [K8S] Fix path issue in Kubernetes dockerfile #20051

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

foxish
Copy link
Contributor

@foxish foxish commented Dec 22, 2017

What changes were proposed in this pull request?

The path was recently changed in #19946, but the dockerfile was not updated.
This is a trivial 1 line fix.

How was this patch tested?

./sbin/build-push-docker-images.sh -r spark-repo -t latest build

cc/ @vanzin @mridulm @rxin @jiangxb1987 @liyinan926

The path was recently changed in apache#19946, but the dockerfile was not updated.
@@ -38,7 +38,7 @@ COPY jars /opt/spark/jars
COPY bin /opt/spark/bin
COPY sbin /opt/spark/sbin
COPY conf /opt/spark/conf
COPY dockerfiles/spark-base/entrypoint.sh /opt/
COPY kubernetes/dockerfiles/spark-base/entrypoint.sh /opt/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@foxish foxish Dec 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, to elaborate, on our fork, we used to have dockerfiles/ in the root of the built distribution. But now, it's copied to kubernetes/dockerfiles which is a more logical place, since there is a yarn/ in the distribution as well.

@mridulm
Copy link
Contributor

mridulm commented Dec 22, 2017

A grep indicates this is the full list:

  • ./resource-managers/kubernetes/docker/src/main/dockerfiles/driver/Dockerfile:# docker build -t spark-driver:latest -f dockerfiles/spark-base/Dockerfile .
  • ./resource-managers/kubernetes/docker/src/main/dockerfiles/spark-base/Dockerfile:# docker build -t spark-base:latest -f dockerfiles/spark-base/Dockerfile .
  • ./resource-managers/kubernetes/docker/src/main/dockerfiles/spark-base/Dockerfile:COPY dockerfiles/spark-base/entrypoint.sh /opt/
  • ./resource-managers/kubernetes/docker/src/main/dockerfiles/executor/Dockerfile:# docker build -t spark-executor:latest -f dockerfiles/spark-base/Dockerfile .

@foxish
Copy link
Contributor Author

foxish commented Dec 22, 2017

@mridulm thanks for looking into it! Looks like I missed updating the dockerfile comments. On it.

@foxish
Copy link
Contributor Author

foxish commented Dec 22, 2017

Done, comments updated. I'm setting up CI on the Kubernetes test infrastructure this week - should help catch these issues early on in future.

@mridulm
Copy link
Contributor

mridulm commented Dec 22, 2017

Merged, thanks @foxish

@asfgit asfgit closed this in 22e1849 Dec 22, 2017
@foxish foxish deleted the patch-1 branch December 22, 2017 05:10
@SparkQA
Copy link

SparkQA commented Dec 22, 2017

Test build #85294 has finished for PR 20051 at commit f631331.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Dec 22, 2017

Test build #85297 has finished for PR 20051 at commit 657b48a.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants