From 6e437aa4896a7c8cd6e532d08c6d32adf8dc89a4 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Mon, 22 Jan 2024 04:47:48 +0000 Subject: [PATCH] Make `start.sh` the entrypoint (#2087) --- docs/using/common.md | 12 ++--- docs/using/selecting.md | 3 +- images/base-notebook/start-notebook.py | 5 ++- images/base-notebook/start-singleuser.py | 4 +- images/docker-stacks-foundation/Dockerfile | 3 +- images/docker-stacks-foundation/start.sh | 11 +++++ tests/R_mimetype_check.py | 8 +++- .../test_spark_notebooks.py | 2 +- tests/base-notebook/test_container_options.py | 2 +- tests/base-notebook/test_pandoc.py | 2 +- tests/base-notebook/test_start_container.py | 7 +-- .../docker-stacks-foundation/test_packages.py | 21 +++++---- .../test_python_version.py | 6 +-- tests/docker-stacks-foundation/test_units.py | 2 +- .../test_user_options.py | 45 +++++++++++++------ tests/minimal-notebook/test_nbconvert.py | 2 +- tests/package_helper.py | 2 +- tests/run_command.py | 2 +- tests/scipy-notebook/test_cython.py | 1 - tests/scipy-notebook/test_extensions.py | 2 +- tests/scipy-notebook/test_matplotlib.py | 2 +- 21 files changed, 86 insertions(+), 58 deletions(-) diff --git a/docs/using/common.md b/docs/using/common.md index 34f09e6ac9..b3b314c9f4 100644 --- a/docs/using/common.md +++ b/docs/using/common.md @@ -225,21 +225,17 @@ docker run -it --rm \ ### `start.sh` -The `start-notebook.py` script inherits most of its option handling capability from a more generic `start.sh` script. -The `start.sh` script supports all the features described above but allows you to specify an arbitrary command to execute. +Most of the configuration options in the `start-notebook.py` script are handled by an internal `start.sh` script that automatically runs before the command provided to the container +(it's set as the container entrypoint). +This allows you to specify an arbitrary command that takes advantage of all these features. For example, to run the text-based `ipython` console in a container, do the following: ```bash -docker run -it --rm quay.io/jupyter/base-notebook start.sh ipython +docker run -it --rm quay.io/jupyter/base-notebook ipython ``` This script is handy when you derive a new Dockerfile from this image and install additional Jupyter applications with subcommands like `jupyter console`, `jupyter kernelgateway`, etc. -### Others - -You can bypass the provided scripts and specify an arbitrary start command. -If you do, keep in mind that features, supported by the `start.sh` script and its kin, will not function (e.g., `GRANT_SUDO`). - ## Conda Environments The default Python 3.x [Conda environment](https://conda.io/projects/conda/en/latest/user-guide/concepts/environments.html) resides in `/opt/conda`. diff --git a/docs/using/selecting.md b/docs/using/selecting.md index 89e983e199..d44f456230 100644 --- a/docs/using/selecting.md +++ b/docs/using/selecting.md @@ -34,8 +34,7 @@ It contains: - [mamba](https://github.com/mamba-org/mamba): "reimplementation of the conda package manager in C++". We use this package manager by default when installing packages. - Unprivileged user `jovyan` (`uid=1000`, configurable, [see options in the common features section](./common.md) of this documentation) in group `users` (`gid=100`) with ownership over the `/home/jovyan` and `/opt/conda` paths -- `tini` as the container entry point -- A `start.sh` script as the default command - useful for running alternative commands in the container as applications are added (e.g. `ipython`, `jupyter kernelgateway`, `jupyter lab`) +- `tini` and a `start.sh` script as the container entry point - useful for running alternative commands in the container as applications are added (e.g. `ipython`, `jupyter kernelgateway`, `jupyter lab`) - A `run-hooks.sh` script, which can source/run files in a given directory - Options for a passwordless sudo - Common system libraries like `bzip2`, `ca-certificates`, `locales` diff --git a/images/base-notebook/start-notebook.py b/images/base-notebook/start-notebook.py index c9a899beb1..b46217e277 100755 --- a/images/base-notebook/start-notebook.py +++ b/images/base-notebook/start-notebook.py @@ -14,8 +14,8 @@ os.execvp(command[0], command) -# Wrap everything in start.sh, no matter what -command = ["/usr/local/bin/start.sh"] +# Entrypoint is start.sh +command = [] # If we want to survive restarts, tell that to start.sh if os.environ.get("RESTARTABLE") == "yes": @@ -40,4 +40,5 @@ command += sys.argv[1:] # Execute the command! +print("Executing: " + " ".join(command)) os.execvp(command[0], command) diff --git a/images/base-notebook/start-singleuser.py b/images/base-notebook/start-singleuser.py index 2dcf6c091c..8fe2ee025e 100755 --- a/images/base-notebook/start-singleuser.py +++ b/images/base-notebook/start-singleuser.py @@ -5,7 +5,8 @@ import shlex import sys -command = ["/usr/local/bin/start.sh", "jupyterhub-singleuser"] +# Entrypoint is start.sh +command = ["jupyterhub-singleuser"] # set default ip to 0.0.0.0 if "--ip=" not in os.environ.get("NOTEBOOK_ARGS", ""): @@ -20,4 +21,5 @@ command += sys.argv[1:] # Execute the command! +print("Executing: " + " ".join(command)) os.execvp(command[0], command) diff --git a/images/docker-stacks-foundation/Dockerfile b/images/docker-stacks-foundation/Dockerfile index bac0df7c36..0f21cac1a6 100644 --- a/images/docker-stacks-foundation/Dockerfile +++ b/images/docker-stacks-foundation/Dockerfile @@ -124,8 +124,7 @@ RUN set -x && \ fix-permissions "/home/${NB_USER}" # Configure container startup -ENTRYPOINT ["tini", "-g", "--"] -CMD ["start.sh"] +ENTRYPOINT ["tini", "-g", "--", "start.sh"] # Copy local files as late as possible to avoid cache busting COPY run-hooks.sh start.sh /usr/local/bin/ diff --git a/images/docker-stacks-foundation/start.sh b/images/docker-stacks-foundation/start.sh index f2108258e6..33d12d801d 100755 --- a/images/docker-stacks-foundation/start.sh +++ b/images/docker-stacks-foundation/start.sh @@ -34,6 +34,17 @@ else cmd=( "$@" ) fi +# Backwards compatibility: `start.sh` is executed by default in ENTRYPOINT +# so it should no longer be specified in CMD +if [ "${_START_SH_EXECUTED}" = "1" ]; then + _log "WARNING: start.sh is the default ENTRYPOINT, do not include it in CMD" + _log "Executing the command:" "${cmd[@]}" + exec "${cmd[@]}" +else + export _START_SH_EXECUTED=1 +fi + + # NOTE: This hook will run as the user the container was started with! # shellcheck disable=SC1091 source /usr/local/bin/run-hooks.sh /usr/local/bin/start-notebook.d diff --git a/tests/R_mimetype_check.py b/tests/R_mimetype_check.py index 71a983d37e..ba90fa51d6 100644 --- a/tests/R_mimetype_check.py +++ b/tests/R_mimetype_check.py @@ -11,10 +11,14 @@ def check_r_mimetypes(container: TrackedContainer) -> None: """Check if Rscript command can be executed""" LOGGER.info("Test that R command can be executed ...") R_MIMETYPES_CHECK_CMD = 'if (length(getOption("jupyter.plot_mimetypes")) != 5) {stop("missing jupyter.plot_mimetypes")}' + command = ["Rscript", "-e", R_MIMETYPES_CHECK_CMD] logs = container.run_and_wait( timeout=10, tty=True, - command=["Rscript", "-e", R_MIMETYPES_CHECK_CMD], + command=command, ) LOGGER.debug(f"{logs=}") - assert len(logs) == 0, f"Command {R_MIMETYPES_CHECK_CMD=} failed" + # If there is any output after this it means there was an error + assert logs.splitlines()[-1] == "Executing the command: " + " ".join( + command + ), f"Command {R_MIMETYPES_CHECK_CMD=} failed" diff --git a/tests/all-spark-notebook/test_spark_notebooks.py b/tests/all-spark-notebook/test_spark_notebooks.py index 4b2559e7cb..7e54e5b04c 100644 --- a/tests/all-spark-notebook/test_spark_notebooks.py +++ b/tests/all-spark-notebook/test_spark_notebooks.py @@ -33,7 +33,7 @@ def test_nbconvert(container: TrackedContainer, test_file: str) -> None: timeout=60, volumes={str(host_data_dir): {"bind": cont_data_dir, "mode": "ro"}}, tty=True, - command=["start.sh", "bash", "-c", command], + command=["bash", "-c", command], ) expected_file = f"{output_dir}/{test_file}.md" diff --git a/tests/base-notebook/test_container_options.py b/tests/base-notebook/test_container_options.py index 3232239bf9..b330c1ecfe 100644 --- a/tests/base-notebook/test_container_options.py +++ b/tests/base-notebook/test_container_options.py @@ -35,7 +35,7 @@ def test_nb_user_change(container: TrackedContainer) -> None: tty=True, user="root", environment=[f"NB_USER={nb_user}", "CHOWN_HOME=yes"], - command=["start.sh", "bash", "-c", "sleep infinity"], + command=["bash", "-c", "sleep infinity"], ) # Give the chown time to complete. diff --git a/tests/base-notebook/test_pandoc.py b/tests/base-notebook/test_pandoc.py index f5cce32542..3c828a3f0b 100644 --- a/tests/base-notebook/test_pandoc.py +++ b/tests/base-notebook/test_pandoc.py @@ -12,6 +12,6 @@ def test_pandoc(container: TrackedContainer) -> None: logs = container.run_and_wait( timeout=10, tty=True, - command=["start.sh", "bash", "-c", 'echo "**BOLD**" | pandoc'], + command=["bash", "-c", 'echo "**BOLD**" | pandoc'], ) assert "

BOLD

" in logs diff --git a/tests/base-notebook/test_start_container.py b/tests/base-notebook/test_start_container.py index 3fbf08aab3..729e7cacb2 100644 --- a/tests/base-notebook/test_start_container.py +++ b/tests/base-notebook/test_start_container.py @@ -53,7 +53,7 @@ def test_start_notebook( LOGGER.debug(logs) # checking that the expected command is launched assert ( - f"Executing the command: {expected_command}" in logs + f"Executing: {expected_command}" in logs ), f"Not the expected command ({expected_command}) was launched" # checking errors and warnings in logs assert "ERROR" not in logs, "ERROR(s) found in logs" @@ -76,10 +76,7 @@ def test_tini_entrypoint( https://superuser.com/questions/632979/if-i-know-the-pid-number-of-a-process-how-can-i-get-its-name """ LOGGER.info(f"Test that {command} is launched as PID {pid} ...") - running_container = container.run_detached( - tty=True, - command=["start.sh"], - ) + running_container = container.run_detached(tty=True) # Select the PID 1 and get the corresponding command cmd = running_container.exec_run(f"ps -p {pid} -o comm=") output = cmd.output.decode("utf-8").strip("\n") diff --git a/tests/docker-stacks-foundation/test_packages.py b/tests/docker-stacks-foundation/test_packages.py index 4fee6428da..a1f7f75580 100644 --- a/tests/docker-stacks-foundation/test_packages.py +++ b/tests/docker-stacks-foundation/test_packages.py @@ -21,17 +21,20 @@ Example: - $ make test/base-notebook + $ make test/docker-stacks-foundation # [...] - # tests/base-notebook/test_packages.py::test_python_packages - # ---------------------------------------------------------------------------------------------- live log setup ---------------------------------------------------------------------------------------------- - # 2023-11-04 23:59:01 [ INFO] Starting container quay.io/jupyter/base-notebook ... (package_helper.py:55) - # 2023-11-04 23:59:01 [ INFO] Running quay.io/jupyter/base-notebook with args {'detach': True, 'tty': True, 'command': ['start.sh', 'bash', '-c', 'sleep infinity']} ... (conftest.py:99) - # 2023-11-04 23:59:01 [ INFO] Grabbing the list of manually requested packages ... (package_helper.py:83) - # ---------------------------------------------------------------------------------------------- live log call ----------------------------------------------------------------------------------------------- - # 2023-11-04 23:59:02 [ INFO] Testing the import of packages ... (test_packages.py:152) - # 2023-11-04 23:59:02 [ INFO] Trying to import mamba (test_packages.py:154) + # tests/docker-stacks-foundation/test_packages.py::test_python_packages + # -------------------------------- live log setup -------------------------------- + # 2024-01-21 17:46:43 [ INFO] Starting container quay.io/jupyter/docker-stacks-foundation ... (package_helper.py:55) + # 2024-01-21 17:46:43 [ INFO] Running quay.io/jupyter/docker-stacks-foundation with args {'detach': True, 'tty': True, 'command': ['bash', '-c', 'sleep infinity']} ... (conftest.py:99) + # 2024-01-21 17:46:44 [ INFO] Grabbing the list of manually requested packages ... (package_helper.py:83) + # -------------------------------- live log call --------------------------------- + # 2024-01-21 17:46:44 [ INFO] Testing the import of packages ... (test_packages.py:151) + # 2024-01-21 17:46:44 [ INFO] Trying to import mamba (test_packages.py:153) + # 2024-01-21 17:46:44 [ INFO] Trying to import jupyter_core (test_packages.py:153) + PASSED [ 17%] + # ------------------------------ live log teardown ------------------------------- # [...] """ diff --git a/tests/docker-stacks-foundation/test_python_version.py b/tests/docker-stacks-foundation/test_python_version.py index 810bcfc42d..559853abb6 100644 --- a/tests/docker-stacks-foundation/test_python_version.py +++ b/tests/docker-stacks-foundation/test_python_version.py @@ -17,8 +17,8 @@ def test_python_version(container: TrackedContainer) -> None: tty=True, command=["python", "--version"], ) - assert logs.startswith("Python ") - full_version = logs.split()[1] + python = next(line for line in logs.splitlines() if line.startswith("Python ")) + full_version = python.split()[1] major_minor_version = full_version[: full_version.rfind(".")] assert major_minor_version == EXPECTED_PYTHON_VERSION @@ -31,4 +31,4 @@ def test_python_pinned_version(container: TrackedContainer) -> None: tty=True, command=["cat", "/opt/conda/conda-meta/pinned"], ) - assert logs.startswith(f"python {EXPECTED_PYTHON_VERSION}.*") + assert f"python {EXPECTED_PYTHON_VERSION}.*" in logs diff --git a/tests/docker-stacks-foundation/test_units.py b/tests/docker-stacks-foundation/test_units.py index 8484449826..cfdbc83dd8 100644 --- a/tests/docker-stacks-foundation/test_units.py +++ b/tests/docker-stacks-foundation/test_units.py @@ -34,5 +34,5 @@ def test_units(container: TrackedContainer) -> None: timeout=30, volumes={str(host_data_dir): {"bind": cont_data_dir, "mode": "ro"}}, tty=True, - command=["start.sh", "python", f"{cont_data_dir}/{test_file_name}"], + command=["python", f"{cont_data_dir}/{test_file_name}"], ) diff --git a/tests/docker-stacks-foundation/test_user_options.py b/tests/docker-stacks-foundation/test_user_options.py index 7463539b9d..fb2b4625a6 100644 --- a/tests/docker-stacks-foundation/test_user_options.py +++ b/tests/docker-stacks-foundation/test_user_options.py @@ -18,7 +18,7 @@ def test_uid_change(container: TrackedContainer) -> None: tty=True, user="root", environment=["NB_UID=1010"], - command=["start.sh", "bash", "-c", "id && touch /opt/conda/test-file"], + command=["bash", "-c", "id && touch /opt/conda/test-file"], ) assert "uid=1010(jovyan)" in logs @@ -30,7 +30,7 @@ def test_gid_change(container: TrackedContainer) -> None: tty=True, user="root", environment=["NB_GID=110"], - command=["start.sh", "id"], + command=["id"], ) assert "gid=110(jovyan)" in logs assert "groups=110(jovyan),100(users)" in logs @@ -43,7 +43,7 @@ def test_nb_user_change(container: TrackedContainer) -> None: tty=True, user="root", environment=[f"NB_USER={nb_user}", "CHOWN_HOME=yes"], - command=["start.sh", "bash", "-c", "sleep infinity"], + command=["bash", "-c", "sleep infinity"], ) # Give the chown time to complete. @@ -99,7 +99,6 @@ def test_chown_extra(container: TrackedContainer) -> None: "CHOWN_EXTRA_OPTS=-R", ], command=[ - "start.sh", "bash", "-c", "stat -c '%n:%u:%g' /home/jovyan/.bashrc /opt/conda/bin/jupyter", @@ -123,7 +122,7 @@ def test_chown_home(container: TrackedContainer) -> None: "NB_UID=1010", "NB_GID=101", ], - command=["start.sh", "bash", "-c", "stat -c '%n:%u:%g' /home/kitten/.bashrc"], + command=["bash", "-c", "stat -c '%n:%u:%g' /home/kitten/.bashrc"], ) assert "/home/kitten/.bashrc:1010:101" in logs @@ -135,7 +134,7 @@ def test_sudo(container: TrackedContainer) -> None: tty=True, user="root", environment=["GRANT_SUDO=yes"], - command=["start.sh", "sudo", "id"], + command=["sudo", "id"], ) assert "uid=0(root)" in logs @@ -147,7 +146,7 @@ def test_sudo_path(container: TrackedContainer) -> None: tty=True, user="root", environment=["GRANT_SUDO=yes"], - command=["start.sh", "sudo", "which", "jupyter"], + command=["sudo", "which", "jupyter"], ) assert logs.rstrip().endswith("/opt/conda/bin/jupyter") @@ -158,7 +157,7 @@ def test_sudo_path_without_grant(container: TrackedContainer) -> None: timeout=10, tty=True, user="root", - command=["start.sh", "which", "jupyter"], + command=["which", "jupyter"], ) assert logs.rstrip().endswith("/opt/conda/bin/jupyter") @@ -173,7 +172,7 @@ def test_group_add(container: TrackedContainer) -> None: no_warnings=False, user="1010:1010", group_add=["users"], # Ensures write access to /home/jovyan - command=["start.sh", "id"], + command=["id"], ) warnings = TrackedContainer.get_warnings(logs) assert len(warnings) == 1 @@ -191,7 +190,7 @@ def test_set_uid(container: TrackedContainer) -> None: timeout=5, no_warnings=False, user="1010", - command=["start.sh", "id"], + command=["id"], ) assert "uid=1010(jovyan) gid=0(root)" in logs warnings = TrackedContainer.get_warnings(logs) @@ -207,7 +206,7 @@ def test_set_uid_and_nb_user(container: TrackedContainer) -> None: user="1010", environment=["NB_USER=kitten"], group_add=["users"], # Ensures write access to /home/jovyan - command=["start.sh", "id"], + command=["id"], ) assert "uid=1010(kitten) gid=0(root)" in logs warnings = TrackedContainer.get_warnings(logs) @@ -236,7 +235,7 @@ def test_container_not_delete_bind_mount( "CHOWN_HOME=yes", ], volumes={d: {"bind": "/home/jovyan/data", "mode": "rw"}}, - command=["start.sh", "ls"], + command=["ls"], ) assert p.read_text() == "some-content" assert len(list(tmp_path.iterdir())) == 1 @@ -259,7 +258,6 @@ def test_jupyter_env_vars_to_unset( "SECRET_FRUIT=mango", ], command=[ - "start.sh", "bash", "-c", "echo I like ${FRUIT} and ${SECRET_FRUIT:-stuff}, and love ${SECRET_ANIMAL:-to keep secrets}!", @@ -284,7 +282,26 @@ def test_secure_path(container: TrackedContainer, tmp_path: pathlib.Path) -> Non tty=True, user="root", volumes={p: {"bind": "/usr/bin/python", "mode": "ro"}}, - command=["start.sh", "python", "--version"], + command=["python", "--version"], ) assert "Wrong python" not in logs assert "Python" in logs + + +def test_startsh_multiple_exec(container: TrackedContainer) -> None: + """If start.sh is executed multiple times check that configuration only occurs once.""" + logs = container.run_and_wait( + timeout=10, + no_warnings=False, + tty=True, + user="root", + environment=["GRANT_SUDO=yes"], + command=["start.sh", "sudo", "id"], + ) + assert "uid=0(root)" in logs + warnings = TrackedContainer.get_warnings(logs) + assert len(warnings) == 1 + assert ( + "WARNING: start.sh is the default ENTRYPOINT, do not include it in CMD" + in warnings[0] + ) diff --git a/tests/minimal-notebook/test_nbconvert.py b/tests/minimal-notebook/test_nbconvert.py index 115d2ff091..9c1c017be7 100644 --- a/tests/minimal-notebook/test_nbconvert.py +++ b/tests/minimal-notebook/test_nbconvert.py @@ -28,7 +28,7 @@ def test_nbconvert( timeout=30, volumes={str(host_data_dir): {"bind": cont_data_dir, "mode": "ro"}}, tty=True, - command=["start.sh", "bash", "-c", command], + command=["bash", "-c", command], ) expected_file = f"{output_dir}/{test_file}.{output_format}" assert expected_file in logs, f"Expected file {expected_file} not generated" diff --git a/tests/package_helper.py b/tests/package_helper.py index 1cfdd99d95..2fe85f5349 100644 --- a/tests/package_helper.py +++ b/tests/package_helper.py @@ -55,7 +55,7 @@ def start_container(container: TrackedContainer) -> Container: LOGGER.info(f"Starting container {container.image_name} ...") return container.run_detached( tty=True, - command=["start.sh", "bash", "-c", "sleep infinity"], + command=["bash", "-c", "sleep infinity"], ) @staticmethod diff --git a/tests/run_command.py b/tests/run_command.py index 40f343d0fb..48e3cc07cd 100644 --- a/tests/run_command.py +++ b/tests/run_command.py @@ -18,5 +18,5 @@ def run_command( return container.run_and_wait( timeout=timeout, tty=True, - command=["start.sh", "bash", "-c", command], + command=["bash", "-c", command], ) diff --git a/tests/scipy-notebook/test_cython.py b/tests/scipy-notebook/test_cython.py index 5d24e9716e..092271ba5c 100644 --- a/tests/scipy-notebook/test_cython.py +++ b/tests/scipy-notebook/test_cython.py @@ -16,7 +16,6 @@ def test_cython(container: TrackedContainer) -> None: volumes={str(host_data_dir): {"bind": cont_data_dir, "mode": "ro"}}, tty=True, command=[ - "start.sh", "bash", "-c", # We copy our data to a temporary folder to be able to modify the directory diff --git a/tests/scipy-notebook/test_extensions.py b/tests/scipy-notebook/test_extensions.py index d95f7ae599..d90cc62290 100644 --- a/tests/scipy-notebook/test_extensions.py +++ b/tests/scipy-notebook/test_extensions.py @@ -30,5 +30,5 @@ def test_check_extension(container: TrackedContainer, extension: str) -> None: container.run_and_wait( timeout=10, tty=True, - command=["start.sh", "jupyter", "labextension", "check", extension], + command=["jupyter", "labextension", "check", extension], ) diff --git a/tests/scipy-notebook/test_matplotlib.py b/tests/scipy-notebook/test_matplotlib.py index ac4fcdae15..c67f039cd3 100644 --- a/tests/scipy-notebook/test_matplotlib.py +++ b/tests/scipy-notebook/test_matplotlib.py @@ -42,7 +42,7 @@ def test_matplotlib( running_container = container.run_detached( volumes={str(host_data_dir): {"bind": cont_data_dir, "mode": "ro"}}, tty=True, - command=["start.sh", "bash", "-c", command], + command=["bash", "-c", command], ) command = f"python {cont_data_dir}/{test_file}" cmd = running_container.exec_run(command)