Skip to content
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

[SPARK-22960][K8S] Revert use of ARG base_image in images #20170

Closed
wants to merge 1 commit into from
Closed

[SPARK-22960][K8S] Revert use of ARG base_image in images #20170

wants to merge 1 commit into from

Conversation

liyinan926
Copy link
Contributor

What changes were proposed in this pull request?

This PR reverts the ARG base_image before FROM in the images of driver, executor, and init-container, introduced in #20154. The reason is Docker versions before 17.06 do not support this use (ARG before FROM).

How was this patch tested?

Tested manually.

@vanzin @foxish @kimoonkim

@foxish
Copy link
Contributor

foxish commented Jan 5, 2018

LGTM, thanks!

@vanzin
Copy link
Contributor

vanzin commented Jan 5, 2018

LGTM. I'll merge when tests finish even though this doesn't affect them.

@vanzin
Copy link
Contributor

vanzin commented Jan 6, 2018

Tests are taking to long...

Merging to master / 2.3.

asfgit pushed a commit that referenced this pull request Jan 6, 2018
## What changes were proposed in this pull request?

This PR reverts the `ARG base_image` before `FROM` in the images of driver, executor, and init-container, introduced in #20154. The reason is Docker versions before 17.06 do not support this use (`ARG` before `FROM`).

## How was this patch tested?

Tested manually.

vanzin foxish kimoonkim

Author: Yinan Li <liyinan926@gmail.com>

Closes #20170 from liyinan926/master.

(cherry picked from commit bf65cd3)
Signed-off-by: Marcelo Vanzin <vanzin@cloudera.com>
@asfgit asfgit closed this in bf65cd3 Jan 6, 2018
@SparkQA
Copy link

SparkQA commented Jan 6, 2018

Test build #85734 has finished for PR 20170 at commit 2f8d0b9.

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

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

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

+1

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.

5 participants