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 <ajimuharrom@uchicago.edu>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>
Co-authored-by: Ayaz Salikhov <mathbunnyru@gmail.com>

Author: 4 people

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}

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

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
 

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

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: