Skip to content

Conversation

@jkleckner
Copy link

What changes were proposed in this pull request?

Change entrypoint.sh script for the kubernetes manager image to reference /usr/sbin/tini

Why are the changes needed?

This makes running commands like /bin/bash via pass-through work.

This was missing from #26046

Does this PR introduce any user-facing change?

It makes pass-through work.

How was this patch tested?

I built an image and verified that the following worked:
docker run -it --rm image:version /bin/bash

…bernetes fix entrypoint

The pass through feature of the entrypoint.sh script needs to change
from /sbin/tini => /usr/bin/tini
@viirya
Copy link
Member

viirya commented Oct 29, 2019

ok to test

*)
echo "Non-spark-on-k8s command provided, proceeding in pass-through mode..."
exec /sbin/tini -s -- "$@"
exec /usr/bin/tini -s -- "$@"
Copy link
Member

Choose a reason for hiding this comment

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

Good catch! Thanks!

@viirya
Copy link
Member

viirya commented Oct 29, 2019

cc @dongjoon-hyun

@viirya
Copy link
Member

viirya commented Oct 29, 2019

@jkleckner The JIRA number in the title seems wrong, can you fix it? Please also modify the title to make it not truncated.

@SparkQA
Copy link

SparkQA commented Oct 29, 2019

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/17786/

@SparkQA
Copy link

SparkQA commented Oct 29, 2019

Test build #112856 has finished for PR 26296 at commit 17f80a5.

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

@SparkQA
Copy link

SparkQA commented Oct 29, 2019

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/17786/

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-38938][K8S][2.4] Move to supported OpenJDK docker image for Ku… [SPARK-38938][K8S][2.4][FOLLOWUP] Move to supported OpenJDK docker image for Ku… Oct 29, 2019
@dongjoon-hyun dongjoon-hyun changed the title [SPARK-38938][K8S][2.4][FOLLOWUP] Move to supported OpenJDK docker image for Ku… [SPARK-38938][K8S][2.4][FOLLOWUP] Use /usr/bin/tini instead of /sbin/tini Oct 29, 2019
@dongjoon-hyun
Copy link
Member

Thank you for the followup, @jkleckner and @viirya .

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-38938][K8S][2.4][FOLLOWUP] Use /usr/bin/tini instead of /sbin/tini [SPARK-28938][K8S][2.4][FOLLOWUP] Use /usr/bin/tini instead of /sbin/tini Oct 29, 2019
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Merged to branch-2.4.

dongjoon-hyun pushed a commit that referenced this pull request Oct 29, 2019
…in/tini`

### What changes were proposed in this pull request?

Change entrypoint.sh script for the kubernetes manager image to reference /usr/sbin/tini

### Why are the changes needed?

This makes running commands like /bin/bash via pass-through work.

This was missing from #26046

### Does this PR introduce any user-facing change?

It makes pass-through work.

### How was this patch tested?

I built an image and verified that the following worked:
`docker run -it --rm  image:version   /bin/bash`

Closes #26296 from jkleckner/fix-pass-through-38938.

Authored-by: Jim Kleckner <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
@dongjoon-hyun
Copy link
Member

Thank you for the followup, @jkleckner and @viirya .

@jkleckner
Copy link
Author

Wow, thanks for the fast action!

@jkleckner jkleckner deleted the fix-pass-through-38938 branch October 29, 2019 18:25
jkleckner pushed a commit to jkleckner/spark-on-k8s-operator that referenced this pull request Dec 6, 2019
As outlined in kubeflow#669 this is needed to run properly because it includes the
unreleased fixes from apache/spark#26046 and apache/spark#26296 which are
needed for GKE authentication to work properly.
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.

4 participants