From 0d324bc0b38ccd9feac269d521abcf6476324e42 Mon Sep 17 00:00:00 2001 From: Muhammad Aji Muharrom Date: Tue, 24 Jan 2023 01:19:39 -0600 Subject: [PATCH] Parameterize healthcheck by internal port (#1859) * Use the JUPYTER_PORT environment variable to configure server port - Remove port setting in jupyter_server_config.py - Declare the $JUPYTER_PORT env on the base Dockerfile - Use it for HEALTHCHECK * Add test case for JUPYTER_PORT env variable * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update documentation * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update style * Better wording * Better wording * Add test for custom internal port * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Parametrize internal port test case * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Try to fix test * Better tests Co-authored-by: Muhammad Aji Muharrom Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Ayaz Salikhov Co-authored-by: Ayaz Salikhov --- base-notebook/Dockerfile | 5 +-- base-notebook/jupyter_server_config.py | 1 - docs/using/common.md | 10 +++++- tests/base-notebook/test_container_options.py | 36 +++++++++++++++++++ tests/base-notebook/test_healthcheck.py | 4 ++- 5 files changed, 51 insertions(+), 5 deletions(-) diff --git a/base-notebook/Dockerfile b/base-notebook/Dockerfile index 9804ddb25..9f3d226df 100644 --- a/base-notebook/Dockerfile +++ b/base-notebook/Dockerfile @@ -47,7 +47,8 @@ RUN mamba install --quiet --yes \ fix-permissions "${CONDA_DIR}" && \ fix-permissions "/home/${NB_USER}" -EXPOSE 8888 +ENV JUPYTER_PORT=8888 +EXPOSE $JUPYTER_PORT # Configure container startup CMD ["start-notebook.sh"] @@ -70,7 +71,7 @@ RUN sed -re "s/c.ServerApp/c.NotebookApp/g" \ # https://github.com/jupyter/docker-stacks/issues/915#issuecomment-1068528799 HEALTHCHECK --interval=5s --timeout=3s --start-period=5s --retries=3 \ CMD wget -O- --no-verbose --tries=1 --no-check-certificate \ - http${GEN_CERT:+s}://localhost:8888${JUPYTERHUB_SERVICE_PREFIX:-/}api || exit 1 + http${GEN_CERT:+s}://localhost:${JUPYTER_PORT}${JUPYTERHUB_SERVICE_PREFIX:-/}api || exit 1 # Switch back to jovyan to avoid accidental container runs as root USER ${NB_UID} diff --git a/base-notebook/jupyter_server_config.py b/base-notebook/jupyter_server_config.py index c95957cc7..679f96bee 100644 --- a/base-notebook/jupyter_server_config.py +++ b/base-notebook/jupyter_server_config.py @@ -9,7 +9,6 @@ c = get_config() # noqa: F821 c.ServerApp.ip = "0.0.0.0" -c.ServerApp.port = 8888 c.ServerApp.open_browser = False # to output both image/svg+xml and application/pdf plot formats in the notebook file diff --git a/docs/using/common.md b/docs/using/common.md index 2444495a3..bf11753b2 100644 --- a/docs/using/common.md +++ b/docs/using/common.md @@ -22,10 +22,16 @@ You can pass [Jupyter server options](https://jupyter-server.readthedocs.io/en/l 2. To set the [base URL](https://jupyter-server.readthedocs.io/en/latest/operators/public-server.html#running-the-notebook-with-a-customized-url-prefix) of the notebook server, you can run the following: ```bash - docker run -it --rm -p 8888:8888 jupyter/base-notebook \ + docker run -it --rm -p 8888:8888 --no-healthcheck jupyter/base-notebook \ start-notebook.sh --NotebookApp.base_url=/customized/url/prefix/ ``` + Note: We pass the `--no-healthcheck` parameter when setting a custom `base_url` for the Jupyter server, + because our current implementation for doing healthcheck assumes the `base_url` to be `/` (the default). + Without using this parameter, the container may run, but it's state will be "unhealthy". + Alternatively, you can [use your own command for healthcheck](https://docs.docker.com/engine/reference/run/#healthcheck) + using the `--health-cmd` parameter. + ## Docker Options You may instruct the `start-notebook.sh` script to customize the container environment before launching the notebook server. @@ -123,6 +129,8 @@ You do so by passing arguments to the `docker run` command. The variables are unset after the hooks have been executed but before the command provided to the startup script runs. - `-e NOTEBOOK_ARGS="--log-level='DEBUG' --dev-mode"` - Adds custom options to add to `jupyter` commands. This way, the user could use any option supported by `jupyter` subcommand. +- `-e JUPYTER_PORT=8117` - Changes the port in the container that Jupyter is using to the value of the `${JUPYTER_PORT}` environment variable. + This may be useful if you run multiple instances of Jupyter in swarm mode and want to use a different port for each instance. ## Startup Hooks diff --git a/tests/base-notebook/test_container_options.py b/tests/base-notebook/test_container_options.py index bb3129e66..33a856221 100644 --- a/tests/base-notebook/test_container_options.py +++ b/tests/base-notebook/test_container_options.py @@ -78,3 +78,39 @@ def test_unsigned_ssl( assert "ERROR" not in logs warnings = TrackedContainer.get_warnings(logs) assert not warnings + + +@pytest.mark.parametrize( + "env", + [ + {}, + {"JUPYTER_PORT": 1234, "DOCKER_STACKS_JUPYTER_CMD": "lab"}, + {"JUPYTER_PORT": 2345, "DOCKER_STACKS_JUPYTER_CMD": "notebook"}, + {"JUPYTER_PORT": 3456, "DOCKER_STACKS_JUPYTER_CMD": "server"}, + {"JUPYTER_PORT": 4567, "DOCKER_STACKS_JUPYTER_CMD": "nbclassic"}, + {"JUPYTER_PORT": 5678, "RESTARTABLE": "yes"}, + {"JUPYTER_PORT": 6789}, + {"JUPYTER_PORT": 7890, "DOCKER_STACKS_JUPYTER_CMD": "notebook"}, + ], +) +def test_custom_internal_port( + container: TrackedContainer, + http_client: requests.Session, + env: dict[str, str], +) -> None: + """Container should be accessible from the host + when using custom internal port""" + host_port = find_free_port() + internal_port = env.get("JUPYTER_PORT", 8888) + running_container = container.run_detached( + command=["start-notebook.sh", "--NotebookApp.token=''"], + environment=env, + ports={internal_port: host_port}, + ) + resp = http_client.get(f"http://localhost:{host_port}") + resp.raise_for_status() + logs = running_container.logs().decode("utf-8") + LOGGER.debug(logs) + assert "ERROR" not in logs + warnings = TrackedContainer.get_warnings(logs) + assert not warnings diff --git a/tests/base-notebook/test_healthcheck.py b/tests/base-notebook/test_healthcheck.py index e6260fa2f..954825c7f 100644 --- a/tests/base-notebook/test_healthcheck.py +++ b/tests/base-notebook/test_healthcheck.py @@ -17,10 +17,12 @@ [ None, ["DOCKER_STACKS_JUPYTER_CMD=lab"], - ["RESTARTABLE=yes"], ["DOCKER_STACKS_JUPYTER_CMD=notebook"], ["DOCKER_STACKS_JUPYTER_CMD=server"], ["DOCKER_STACKS_JUPYTER_CMD=nbclassic"], + ["RESTARTABLE=yes"], + ["JUPYTER_PORT=8171"], + ["JUPYTER_PORT=8117", "DOCKER_STACKS_JUPYTER_CMD=notebook"], ], ) def test_health(container: TrackedContainer, env: Optional[list[str]]) -> None: