jupyterhub / the-littlest-jupyterhub

Simple JupyterHub distribution for 1-100 users on a single server
https://tljh.jupyter.org
BSD 3-Clause "New" or "Revised" License
1.02k stars 338 forks source link

Update systemdspawner from version 0.17.* to >=1.0.1,<2 #915

Closed consideRatio closed 1 year ago

consideRatio commented 1 year ago

The breaking changes in SystemdSpawner are listed as:

  • Systemd v243+ is now required, and v245+ is recommended. Systemd v245 is available in for example Ubuntu 20.04+, Debian 11+, and Rocky/CentOS 9+.
  • Python 3.8+, JupyterHub 2.3.0+, and Tornado 5.1+ is now required.
  • SystemdSpawner.disable_user_sudo (influences systemd's NoNewPrivileges) now defaults to True, making the installation more secure by default.

I think only the disable_user_sudo=True as default change is the only possibly breaking change. I've added a commit to make admin users get it set to False and non-admin users get it set to True explicitly in TLJH now. This leads to the following breaking change:

If a non-admin jupyterhub user has been granted sudo permissions, without being granted jupyterhub admin status, then the user would then not be able to sudo any more.

I suggest that we suggest in the changelog that such users should also be granted jupyterhub admin status. This is something the user could acquire with sudo anyhow.

consideRatio commented 1 year ago

I found logs finally, this is what happens.

Jun 06 23:49:49 6950396480e0 systemd[1]: Started /opt/tljh/hub/bin/jupyterhub-singleuser.
Jun 06 23:49:50 6950396480e0 jupyterhub-singleuser[3077]: Traceback (most recent call last):
Jun 06 23:49:50 6950396480e0 jupyterhub-singleuser[3077]:   File "/opt/tljh/hub/bin/jupyterhub-singleuser", line 5, in <module>
Jun 06 23:49:50 6950396480e0 jupyterhub-singleuser[3077]:     from jupyterhub.singleuser import main
Jun 06 23:49:50 6950396480e0 jupyterhub-singleuser[3077]:   File "/opt/tljh/hub/lib/python3.9/site-packages/jupyterhub/singleuser/__init__.py", line 17, in <module>
Jun 06 23:49:50 6950396480e0 jupyterhub-singleuser[3077]:     from .mixins import HubAuthenticatedHandler, make_singleuser_app
Jun 06 23:49:50 6950396480e0 jupyterhub-singleuser[3077]:   File "/opt/tljh/hub/lib/python3.9/site-packages/jupyterhub/singleuser/mixins.py", line 48, in <module>
Jun 06 23:49:50 6950396480e0 jupyterhub-singleuser[3077]:     from ._disable_user_config import _disable_user_config, _exclude_home
Jun 06 23:49:50 6950396480e0 jupyterhub-singleuser[3077]:   File "/opt/tljh/hub/lib/python3.9/site-packages/jupyterhub/singleuser/_disable_user_config.py", line 24, in <module>
Jun 06 23:49:50 6950396480e0 jupyterhub-singleuser[3077]:     from jupyter_core import paths
Jun 06 23:49:50 6950396480e0 jupyterhub-singleuser[3077]: ModuleNotFoundError: No module named 'jupyter_core'
Jun 06 23:49:50 6950396480e0 systemd[1]: jupyter-6a7ae9387dd3057f.service: Main process exited, code=exited, status=1/FAILURE
Jun 06 23:49:50 6950396480e0 systemd[1]: jupyter-6a7ae9387dd3057f.service: Failed with result 'exit-code'.

Is this a path issue perhaps?

Oh! Why is jupyterhub-singleuser started from the hub env and not the user env?

@minrk do you have a guess about this?

manics commented 1 year ago

Is the path to the user env definitely passed to the spawner (either using PATH, or passing the full path to jupyterhub-singleuser)? Do the logs contain the SystemdSpawner/UserCreatingSpawner configuration?

minrk commented 1 year ago

Definitely a PATH issue, because the command should be /opt/tljh/user/bin/jupyterhub-singleuser not /opt/tljh/hub/bin/jupyterhub-singleuser. My guess is it's https://github.com/jupyterhub/systemdspawner/commit/eb3d26dfbf429624f0bace02ae413e5083f6d529 removing the invocation of /bin/bash, which may have modified PATH via profiles to put the user env ahead, but I'm not sure.

I believe the fix should be for the Spawner to ensure {USER_ENV_PREFIX}/bin is prepended to PATH, but this is supposedly done here, and applied here. Seems like one or both of those is not having the intended effect. It's not obvious to me what's responsible, though.

Can you dump the systemd unit file?

consideRatio commented 1 year ago

Can you dump the systemd unit file?

Hmmm hmmm, the user servers are started as "transient units", not having a unit registered etc i think. I'm not sure how to do things.

@minrk the logs was found from #916 in https://github.com/jupyterhub/the-littlest-jupyterhub/actions/runs/5194421002/jobs/9366061173#step:5:319 (not in the "show logs" step, but from the in-the-middle-of-the-test run step where the created user is known)

minrk commented 1 year ago

I'm not sure how to do things.

I think it's worth adding some debug logging to systemdspawner.

I was able to run some local tests to do some debugging, and I believe it comes down to the fact that systemd itself resolves executables to absolute paths before loading EnvironmentFile, so Spawner.cmd needs to be resolves to an absolute path.

systemd-run asserts this as well:

If a command is run as service unit, the first argument needs to be an absolute program path.

so I'm not quite sure why it behaves weird instead of failing because the condition isn't met. It also appears to be using the calling process's $PATH, because it's finding the executable in tljh/hub/bin, rather than raising command not found. I

Options include:

All of these should be done in systemdspawner. I'll have a look at implementing the first option, which I think makes the most sense.

minrk commented 1 year ago

https://github.com/jupyterhub/systemdspawner/pull/129

minrk commented 1 year ago

fwiw, this is how I did my debug setup:

.github/integration-test.py build-image
export BOOTSTRAP_PIP_SPEC=git+https://github.com/jupyterhub/the-littlest-jupyterhub.git@refs/pull/915/merge
.github/integration-test.py run-test basic-tests               --bootstrap-pip-spec "$BOOTSTRAP_PIP_SPEC" test_hub.py # (once)

# start an interactive shell in the container
docker exec -it basic-tests bash

# on host, copy systemdspawner into container
docker cp -r . basic-tests:/srv/src/systemdspawner

# back in container
cd srv/src
/opt/hub/bin/pip install -e ./systemdspawner
tljh-config reload hub
/opt/hub/bin/pytest -vsx ./integration-tests/test_hub.py

# check logs
journalctl

and repeat the docker cp and reload hub when I update, since the path isn't mounted (I could have created the container by hand with a mount, but running the integration test startup was the easiest way to make sure bootstrap matched the test environment)

consideRatio commented 1 year ago

Tests are passing, this works now! Thank you @minrk for working this in systemdspawner!