-
Notifications
You must be signed in to change notification settings - Fork 52
[SPARK-52542] Use /nonexistent instead of nonexistent /home/spark
#87
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
Conversation
/nonexistent instead of nonexistent /opt/spark/nonexistent instead of nonexistent /home/spark
|
It seems that |
|
It's the flakiness of |
e07cb60 to
f7ab79e
Compare
f7ab79e to
287d96c
Compare
|
Rebased to the master to bring the ASF Mirror patch. |
|
cc @Yikun , @yaooqinn , @viirya , @HyukjinKwon , @LuciferYang . |
viirya
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable. Just wonder if it is possible to break downstream images if any?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dongjoon-hyun Thanks for catching this and fixing it, I'm OK with this changes:
- IIRC, this behavior introduced on #11, I revisit with it. The PR were addressing
rootuser problem and meet DOI requirements. - Consider the home path (
/home/spark) is non-exists, so it's safe to switch/nonexistent, thenon-existsalso be used in nginx docker official image - The default workspace is
/opt/spark/work-diralso no impacts on. - This PR change the behavior,
-
- It will raise different error (with different path info) if
cd ~, I think it's OK.
- It will raise different error (with different path info) if
-
- It will have behavior change when user
cd ~if user mkdir the path/home/sparkin thier downstreamdockerfilewhich based on spark-docker base image. Users should usecd /home/sparkinstead.
- It will have behavior change when user
-
- Maybe we should doc this behavior changes in release note or somewhere
cc @yosifkit @tianon It is better to make the DOI maintainers aware of this and get their blessing to avoid getting some unexpected reviews on the spark docker version upgrade PR.
|
Thank you, @viirya , @HyukjinKwon , @yaooqinn , @LuciferYang , @Yikun . This PR only affected Apache Spark-maintained images. A user who builds a docker image from Apache Spark binary via |
|
I think the documentation on https://hub.docker.com/r/apache/spark/ might need to be updated. If I run the example: then it works, but I get: This happens for any command. Do I need to modify my Docker command to use it properly now with this change? |
|
To @DenWav , your question is completely irrelevant to this PR because the situation happened with the previous status |
|
Just FYI,
docker run -it -u root apache/spark /opt/spark/bin/spark-shell |
|
You can also add an anonymous volume or a |
What changes were proposed in this pull request?
This PR aims to use
/nonexistentexplicitly instead of nonexistent/home/sparkbecause the current status is misleading.Please note that SPARK-40528 introduced
useradd --systemwhich createdsparkuser with a non-existent/home/sparkdirectory from the beginning of this repository,spark-docker.[SPARK-40528] Support dockerfile template #12
spark-docker/Dockerfile.template
Lines 21 to 22 in c264d48
Rejected Alternatives
HOMEto/opt/sparklike Apache Spark behavior. However, it's also different fromWORKDIR(/opt/spark/work-dir)./home/spark, but it could be more vulnerable than AS-IS status. Forsystemaccount,/nonexistentis frequently used as the security practice to prevent any side effects ofHOMEdirectory.Why are the changes needed?
Apache Spark 3.3.3
Apache Spark 3.4.4
Apache Spark 3.5.6
Apache Spark 4.0.0
Does this PR introduce any user-facing change?
No behavior change because it doesn't exist already.
How was this patch tested?
Manual review.