Start entrypoint bash scripts with -l

[yuvipanda]

Describe your changes

This reads any .bashrc, .profile, etc files present in user folders while starting. Matches what repo2docker does in
https://github.com/jupyterhub/repo2docker/blob/b36a6a75f54bfb47903b68671e9e157305f92fa6/repo2docker/buildpacks/python3-login.

There is a specific downside here when used with JupyterHub - users' broken .bashrc files may break these scripts from executing, and hence prevent their servers from starting.

Issue ticket if applicable

Fixes #1848

Checklist (especially for first-time contributors)

• I have performed a self-review of my code
• If it is a core feature, I have added thorough tests
• I will try not to use force-push to make the review process easier for reviewers
• I have updated the documentation for significant changes

[yuvipanda]

If we want to test this, one way would be to make a bashrc file and mount that in the user home directory, and make sure that the process started has that executed.

Probably also needs a documentation bit about breaking jupyterhub server start if your bashrc is broken, so tread carefully.

If this otherwise looks ok I'll add those!

[mathbunnyru]

There is a specific downside here when used with JupyterHub - users' broken .bashrc files may break these scripts from executing, and hence prevent their servers from starting.

I do consider this to be an upside 😃
Broken things should break, and not be silently ignored.

What bothers me more - if I'm understanding this correctly, profile scripts might me run not just once, but twice, because start-notebook.sh calls two other scripts.
Maybe we should not add this to start-notebook.sh.

[bribroder]

Hi there, I just want to chime in on how this would impact my hub community. We persist user home directories and mount them onto servers. It's not uncommon for users in my community to use tools that inject various scripts and automations, or terminal bling, into their bashrc files. These things are shockingly fragile and can malfunction due to any number of issues, including problems with python dependencies that are used by the automations in the .bashrc file--and such issues may not appear until the next server upgrade, when something gets mismatched in the user's python library or some dependency disappears. It's extremely easy for .bashrc to contain garbage. It's also hard to edit this file on a hub without access to the web terminal.

Based on @yuvipanda's comments, it sounds like users who accidentally break their .bashrc would need an administrator to get involved and fix their .bashrc file before they would be able to log in. Administrators might also need to validate .bashrc files after server upgrades to ensure nothing has broken. This would have a big impact on the maintainers of the hub, making upgrades more perilous and requiring intervention to fix broken servers, and would also degrade the experience for users--who would basically end up locked out if their server ever shuts off with garbage in the .bashrc.

[mathbunnyru]

Is there a way to read .bashrc like files, but not fail when they have some errors?

[mathbunnyru]

I actually had another idea, which might be easier to implement.
https://github.com/jupyter/docker-stacks/blob/main/base-notebook/Dockerfile#L11

We're using SHELL docker command. I think we can add -l option here and it will have desired effect (I'm not sure this will work, but I think it should).

@bribroder you problem might be solved by creating inherited image:

FROM jupyter/base-notebook

# return to the old behaviour without `-l`
SHELL ["/bin/bash", "-o", "pipefail", "-c"]

@yuvipanda @bribroder
How do you feel about this?
Yes, you will have to create inherited image, but this does seem to be a good trade-off.

[yuvipanda]

@mathbunnyru I am not 100% sure if SHELL affects entrypoint though, as the shebang in the first line is interpreted by exec directly.

I do agree with @bribroder that this adds a bunch of work to hub admins, which is making me reconsider this!

If what people want is to pass -l only to terminals opened in JupyterLab, I think there's a Jupyter Server config that specifies what command to run when starting a terminal. maybe we just set that?

[mathbunnyru]

@mathbunnyru I am not 100% sure if SHELL affects entrypoint though, as the shebang in the first line is interpreted by exec directly.

I think you're right, I wasn't able to make it work.

I do agree with @bribroder that this adds a bunch of work to hub admins, which is making me reconsider this!

If what people want is to pass -l only to terminals opened in JupyterLab, I think there's a Jupyter Server config that specifies what command to run when starting a terminal. maybe we just set that?

We will still be breaking some users - they won't be able to launch their terminals in JupyterLab.
They will be able to use their notebooks to fix their .bashrc-like files, right?
So, If such an option really exists (sorry, I don't know a lot about terminals in Jupyter), let's add it here.
I also think, after some period of time we can actually start using -l option as you suggest in this PR - most of the users with broken .bashrc-like files will have already updated an image, launched a terminal and finally fixed their files.
I would like to ask you to check if terminal shows an error if there is a broken file (so people can easily fix the problem).

[yuvipanda]

@mathbunnyru I went through memory lane, and found https://github.com/berkeley-dsep-infra/datahub/pull/1908/files#diff-1f0c374af36c9720d2a00eddb864f4b563d38042817fd3a4f7df357c1358f3d6L2. I think the incantation is something like:

c.ServerApp.terminado_settings = {"shell_command": ["/bin/bash", "-l"]}

Unfortunately I don't think I'll be able to test it out anytime soon - think you can take a peek?

[mathbunnyru]

Sure.
But the result might surprise you.

My Dockerfile:

FROM jupyter/base-notebook

RUN printf "touch /home/${NB_USER}/file.txt" >> /home/${NB_USER}/.bashrc

I ran it using:

docker build -t test .
docker run -p 8888:8888 -it --rm test

Opened a notebook, typed !ls - it gave me Untitled.ipynb work, which is expected.
Then I opened terminal and typed ls:

(base) jovyan@6f13b113a45c:~$ ls
file.txt  Untitled.ipynb  work

So, it seems, we don't have to do anything.
I don't know why this happens though.

[yuvipanda]

That is what I'd expect though - the bashrc is not executed when the notebook is started, but is executed when a terminal is opened explicitly! This means that opening the notebook server will not crash due to bashrc problems, but opening a terminal would.

[mathbunnyru]

I didn't use your setting, this is what makes it interesting 😂

[yuvipanda]

HAHAHA ok now that is indeed interesting :) If you do a ps auxf what's the shell command being launched?

[mathbunnyru]

I'll tell you in a few hours, I don't have a 💻 with me.

[mathbunnyru]

@yuvipanda

(base) jovyan@df2ddecea7b9:~$ ps auxf
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
jovyan       1  0.0  0.0   2204   764 pts/0    Ss   20:17   0:00 tini -g -- start-notebook.sh
jovyan       7  5.1  1.1 405684 93136 pts/0    Sl+  20:17   0:00 /opt/conda/bin/python3.11 /opt/conda/bin/jupyter-lab
jovyan      67  3.0  0.0   7144  3828 pts/1    Ss   20:17   0:00  \_ /bin/bash
jovyan      95  0.0  0.0   9392  1556 pts/1    R+   20:17   0:00      \_ ps auxf

I made a simple test - I used my VM, then added touch MY_HOME_DIR_HERE/file.txt to my .bashrc, and then launched bash (without -l), and file was created.
I suppose there are different rules for .bashrc, .bash_profile and /etc/profile files.
https://www.baeldung.com/linux/bashrc-vs-bash-profile-vs-profile#2-significance-of-bashrc

[mathbunnyru]

@yuvipanda I think we might have a better solution for executing some scripts (including .bashrc-like).
We have run-hooks.sh script and Startup Hooks feature.

So, people should be able to create a script, which sources .bashrc or anything else they want.
Yes, they will have to explicitly source .bashrc-like files and create a simple script which does that (and this script should be a part of the image).

On the other hand, I think we can make run-hooks.sh continue on failure (with some message it failed).
I think it actually continues now, just without a message though and not tested.
I can add such a test easily.
This way, JupyterHub users and administrators will be happy.
I also think we can add such a script ourselves.

We already use startup hooks in one of our recipes, and it works nicely: https://jupyter-docker-stacks.readthedocs.io/en/latest/using/recipes.html#add-a-custom-conda-environment-and-jupyter-kernel
And run-hooks is tested now.

[yuvipanda]

Yeah, I agree this can probably be closed.

Thanks @bribroder for bringing in the perspective of added workload!