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

fix hub permission issue #91

Merged
merged 3 commits into from
Sep 16, 2024
Merged

fix hub permission issue #91

merged 3 commits into from
Sep 16, 2024

Conversation

Tianhao-Gu
Copy link
Collaborator

The spark_user, which runs the JupyterHub, needs access to users' directories to update Jupyter runtime data. However, despite setting directory permissions to 770 and assigning the correct group to spark_user, it still encounters access issues. As a temporary solution, we have set the directories to 777, hoping this will be resolved when we switch to Docker spawner.

spark_user@618d542b608c:/jupyterhub/users_home$ ls -ll
total 12
drwxr-xr-x 6 spark_user spark      4096 Sep 13 00:50 root
drwxrwx--- 9 spark_user spark      4096 Sep 13 04:33 spark_user
drwxrwx--- 6 tian.gu    jupyterhub 4096 Sep 14 03:34 tian.gu
spark_user@618d542b608c:/jupyterhub/users_home$ cd tian.gu/
bash: cd: tian.gu/: Permission denied
spark_user@618d542b608c:/jupyterhub/users_home$ groups spark_user
spark_user : spark jupyterhub

Copy link

codecov bot commented Sep 14, 2024

Codecov Report

Attention: Patch coverage is 26.66667% with 11 lines in your changes missing coverage. Please review.

Project coverage is 68.37%. Comparing base (412092e) to head (c4e6cb3).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/jupyterhub_config/custom_spawner.py 26.66% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #91      +/-   ##
==========================================
- Coverage   72.48%   68.37%   -4.12%     
==========================================
  Files           5        5              
  Lines         229      234       +5     
==========================================
- Hits          166      160       -6     
- Misses         63       74      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


# TODO: Set directory permissions to 750 or 700 or switch to use docker spawner
# Set directory permissions to 777: Owner (rwx), Group (rwx), Others (rwx)
subprocess.run(['sudo', 'chmod', '-R', '777', user_dir], check=True)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

After exploring DockerSpawner, I'm confident that switching to it will resolve the issue, as each user's directory is mounted individually per container.

src/jupyterhub_config/custom_spawner.py Outdated Show resolved Hide resolved
@@ -237,11 +201,6 @@ def test_ensure_user_directory_reuse_existing(mock_chown, mock_chmod, caplog):
spawner = VirtualEnvSpawner()
spawner._ensure_user_directory(user_dir, username)

# Check that mkdir, chown, and chmod were not called since directory exists
assert user_dir.exists()
Copy link
Member

Choose a reason for hiding this comment

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

Seems like you should keep this line

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍

Copy link
Member

@MrCreosote MrCreosote left a comment

Choose a reason for hiding this comment

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

Seems reasonable for now given this is all going to change drastically with the spawner setup

@Tianhao-Gu Tianhao-Gu merged commit 68bd53c into main Sep 16, 2024
7 of 10 checks passed
@Tianhao-Gu Tianhao-Gu deleted the dev_jupyterhub branch September 16, 2024 18:24
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.

2 participants