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

Conversation

@erikerlandson
Copy link
Member

What changes were proposed in this pull request?

Add a common entrypoint.sh script to use as the entrypoint for the container images. This script checks to see if the container UID has an entry in /etc/passwd. If not, then it will attempt to add one. The primary motivating use case is running inside of OpenShift, which runs containers with anonymous UIDs. This PR is part of an effort to put the reference images into a state such that one can run spark-on-k8s against OpenShift out-of-the-box.

How was this patch tested?

CI to build and run integration testing with updated container images

# If there is no passwd entry for the container UID, attempt to create one
if [ -z "$uidentry" ] ; then
if [ -w /etc/passwd ] ; then
echo "$myuid:x:$myuid:$mygid:anonymous uid:$SPARK_HOME:/bin/false" >> /etc/passwd
Copy link

Choose a reason for hiding this comment

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

@erikerlandson why does OpenShift require an entry in passwd ? Is this an additional requirement unique to OpenShift when running docker images?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a Spark requirement

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem arises because OpenShift runs its containers with an anonymous UID that has no passwd entry. Spark will crash if it can't find a passwd entry.

Copy link

Choose a reason for hiding this comment

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

Can you please paste on this ticket what the exception is so we can find this discussion again in the future if necessary?

@erikerlandson
Copy link
Member Author

rerun integration test please

@erikerlandson
Copy link
Member Author

cc/ @foxish @mccheah

@erikerlandson erikerlandson mentioned this pull request Aug 1, 2017
10 tasks
@foxish
Copy link
Member

foxish commented Aug 3, 2017

This change LGTM. Any comments @mccheah?

@mccheah
Copy link

mccheah commented Aug 3, 2017

I'm not entirely convinced that if this is specific to OpenShift that it should belong in the base Spark images. How often will this be needed outside of the OpenShift use case?

@erikerlandson
Copy link
Member Author

It's hard to say. It will happen effectively all the time under OpenShift, and OpenShift(-Origin) is a major kube downstream. I'd expect it to be comparatively rare under straight kube, however it's not impossible for it to happen; containers configured to run under a UID that isn't present in /etc/passwd. My reasoning was that it could help under other scenarios, and regardless it is just a no-op if the passwd entry exists. It either helps or does no harm.

@foxish
Copy link
Member

foxish commented Aug 3, 2017

If it was any more complicated, then I'd have second thoughts, but this seems simple enough (and mostly a no-op) that I'm not opposed to it. If it were to grow in complexity, I'd say we should split the image out to publish separate OpenShift artifacts. As for this change, I'm a bit less concerned. If you feel strongly here, we can punt on this and discuss in next week's meeting.

@mccheah
Copy link

mccheah commented Aug 3, 2017

Fine to merge then

@foxish
Copy link
Member

foxish commented Aug 3, 2017

Great.. Thanks @mccheah. Merging now.

@foxish foxish merged commit fa67455 into apache-spark-on-k8s:branch-2.2-kubernetes Aug 3, 2017
ifilonenko pushed a commit to ifilonenko/spark that referenced this pull request Feb 26, 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.

4 participants