From a7774e55ded384336f057c6cb797511dc2a9dd1a Mon Sep 17 00:00:00 2001 From: Pris Nasrat Date: Mon, 30 Jan 2023 11:17:37 -0500 Subject: [PATCH 01/12] Ensure SQLAlchemy 1.x used for hub Fixes #846 --- tljh/installer.py | 1 + tljh/requirements-base.txt | 2 ++ 2 files changed, 3 insertions(+) diff --git a/tljh/installer.py b/tljh/installer.py index 42ba070..7e9948d 100644 --- a/tljh/installer.py +++ b/tljh/installer.py @@ -121,6 +121,7 @@ def ensure_jupyterhub_package(prefix): conda.ensure_pip_packages( prefix, [ + "SQLAlchemy<2.0.0", "jupyterhub==1.*", "jupyterhub-systemdspawner==0.16.*", "jupyterhub-firstuseauthenticator==1.*", diff --git a/tljh/requirements-base.txt b/tljh/requirements-base.txt index 02df904..1985db7 100644 --- a/tljh/requirements-base.txt +++ b/tljh/requirements-base.txt @@ -5,6 +5,8 @@ # the requirements-txt-fixer pre-commit hook that sorted them and made # our integration tests fail. # +# For JupyterHub 1.x SQLAlchemy below 2.0.0 +SQLAlchemy<2.0.0 # JupyterHub + notebook package are base requirements for user environment jupyterhub==1.* notebook==6.* From 5c475b876c74422de81c89f08f41277cd4e62d5e Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 27 Nov 2022 20:08:55 +0000 Subject: [PATCH 02/12] Add debugging info to test_install permissions checks --- integration-tests/test_install.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/integration-tests/test_install.py b/integration-tests/test_install.py index 4411e10..3f53b5f 100644 --- a/integration-tests/test_install.py +++ b/integration-tests/test_install.py @@ -45,6 +45,12 @@ def test_groups_exist(group): grp.getgrnam(group) +def debug_uid_gid(): + return ( + f"uid={os.getuid()} gid={os.getgid()} euid={os.geteuid()} egid={os.getegid()}" + ) + + def permissions_test(group, path, *, readable=None, writable=None, dirs_only=False): """Run a permissions test on all files in a path path""" # start a subprocess and become nobody:group in the process @@ -88,18 +94,22 @@ def permissions_test(group, path, *, readable=None, writable=None, dirs_only=Fal # check if the path should be writable if writable is not None: if access(path, os.W_OK) != writable: + stat = os.stat(path) + info = pool.submit(debug_uid_gid).result() failures.append( - "{} {} should {}be writable by {}".format( - stat_str, path, "" if writable else "not ", group + "{} {} should {}be writable by {} [{}]".format( + stat_str, path, "" if writable else "not ", group, info ) ) # check if the path should be readable if readable is not None: if access(path, os.R_OK) != readable: + stat = os.stat(path) + info = pool.submit(debug_uid_gid).result() failures.append( - "{} {} should {}be readable by {}".format( - stat_str, path, "" if readable else "not ", group + "{} {} should {}be readable by {} [{}]".format( + stat_str, path, "" if readable else "not ", group, info ) ) # verify that we actually tested some files From 05d46e1d9b6e6974e4e56e10fb002614a2f07720 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 27 Nov 2022 20:28:16 +0000 Subject: [PATCH 03/12] integration-test.py: Add debugging info --- .github/integration-test.py | 64 +++++++++++++++++++++++++++++-------- 1 file changed, 51 insertions(+), 13 deletions(-) diff --git a/.github/integration-test.py b/.github/integration-test.py index 6d05ecb..73ea6d7 100755 --- a/.github/integration-test.py +++ b/.github/integration-test.py @@ -1,19 +1,56 @@ #!/usr/bin/env python3 import argparse +from shutil import which import subprocess +from time import time import os +def container_runtime(): + runtimes = ["podman", "docker"] + for runtime in runtimes: + if which(runtime): + return runtime + raise RuntimeError(f"No container runtime found, tried: {' '.join(runtimes)}") + + +def container_check_output(*args, **kwargs): + cmd = [container_runtime()] + list(*args) + print(f"Running {cmd} {kwargs}") + return subprocess.check_output(cmd, **kwargs) + + +def container_run(*args, **kwargs): + cmd = [container_runtime()] + list(*args) + print(f"Running {cmd} {kwargs}") + return subprocess.run(cmd, **kwargs) + + def build_systemd_image(image_name, source_path, build_args=None): """ Build docker image with systemd at source_path. Built image is tagged with image_name """ - cmd = ["docker", "build", f"-t={image_name}", source_path] + cmd = ["build", f"-t={image_name}", source_path] if build_args: cmd.extend([f"--build-arg={ba}" for ba in build_args]) - subprocess.check_call(cmd) + container_check_output(cmd) + + +def check_container_ready(container_name, timeout=60): + """ + Check if container is ready to run tests + """ + now = time() + while True: + try: + container_check_output(["exec", "-t", container_name, "id"]) + return + except subprocess.CalledProcessError: + if time() - now > timeout: + raise RuntimeError(f"Container {container_name} hasn't started") + time.sleep(5) def run_systemd_image(image_name, container_name, bootstrap_pip_spec): @@ -25,16 +62,16 @@ def run_systemd_image(image_name, container_name, bootstrap_pip_spec): Container named container_name will be started. """ cmd = [ - "docker", "run", "--privileged", - "--mount=type=bind,source=/sys/fs/cgroup,target=/sys/fs/cgroup", "--detach", f"--name={container_name}", # A bit less than 1GB to ensure TLJH runs on 1GB VMs. # If this is changed all docs references to the required memory must be changed too. "--memory=900m", ] + if container_runtime() != "podman": + cmd.append("--mount=type=bind,source=/sys/fs/cgroup,target=/sys/fs/cgroup") if bootstrap_pip_spec: cmd.append("-e") @@ -42,7 +79,7 @@ def run_systemd_image(image_name, container_name, bootstrap_pip_spec): cmd.append(image_name) - subprocess.check_call(cmd) + container_check_output(cmd) def stop_container(container_name): @@ -50,21 +87,20 @@ def stop_container(container_name): Stop & remove docker container if it exists. """ try: - subprocess.check_output( - ["docker", "inspect", container_name], stderr=subprocess.STDOUT - ) + container_check_output(["inspect", container_name], stderr=subprocess.STDOUT) except subprocess.CalledProcessError: # No such container exists, nothing to do return - subprocess.check_call(["docker", "rm", "-f", container_name]) + container_check_output(["rm", "-f", container_name]) def run_container_command(container_name, cmd): """ Run cmd in a running container with a bash shell """ - proc = subprocess.run( - ["docker", "exec", "-t", container_name, "/bin/bash", "-c", cmd], check=True + proc = container_run( + ["exec", "-t", container_name, "/bin/bash", "-c", cmd], + check=True, ) @@ -72,7 +108,7 @@ def copy_to_container(container_name, src_path, dest_path): """ Copy files from src_path to dest_path inside container_name """ - subprocess.check_call(["docker", "cp", src_path, f"{container_name}:{dest_path}"]) + container_check_output(["cp", src_path, f"{container_name}:{dest_path}"]) def run_test( @@ -84,6 +120,8 @@ def run_test( stop_container(test_name) run_systemd_image(image_name, test_name, bootstrap_pip_spec) + check_container_ready(test_name) + source_path = os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir)) copy_to_container(test_name, os.path.join(source_path, "bootstrap/."), "/srv/src") @@ -93,7 +131,7 @@ def run_test( # These logs can be very relevant to debug a container startup failure print(f"--- Start of logs from the container: {test_name}") - print(subprocess.check_output(["docker", "logs", test_name]).decode()) + print(container_check_output(["logs", test_name]).decode()) print(f"--- End of logs from the container: {test_name}") # Install TLJH from the default branch first to test upgrades From ae5fd645762b498795b43595bbdd5163cef1c2b9 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 27 Nov 2022 21:16:46 +0000 Subject: [PATCH 04/12] integration-test.py: try docker first --- .github/integration-test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/integration-test.py b/.github/integration-test.py index 73ea6d7..c6aec7f 100755 --- a/.github/integration-test.py +++ b/.github/integration-test.py @@ -7,7 +7,7 @@ import os def container_runtime(): - runtimes = ["podman", "docker"] + runtimes = ["docker", "podman"] for runtime in runtimes: if which(runtime): return runtime From 20c6a851de86984d36217fbaf6a9230bcc844a0b Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 27 Nov 2022 21:28:43 +0000 Subject: [PATCH 05/12] integration-test.py: fix time delay check --- .github/integration-test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/integration-test.py b/.github/integration-test.py index c6aec7f..0dc93e4 100755 --- a/.github/integration-test.py +++ b/.github/integration-test.py @@ -2,7 +2,7 @@ import argparse from shutil import which import subprocess -from time import time +import time import os @@ -42,13 +42,13 @@ def check_container_ready(container_name, timeout=60): """ Check if container is ready to run tests """ - now = time() + now = time.time() while True: try: container_check_output(["exec", "-t", container_name, "id"]) return except subprocess.CalledProcessError: - if time() - now > timeout: + if time.time() - now > timeout: raise RuntimeError(f"Container {container_name} hasn't started") time.sleep(5) From 6413268211eb72cb1ad3b2cc034c3ea1d69e50ba Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 27 Nov 2022 22:36:09 +0000 Subject: [PATCH 06/12] use id to debug test_install.py --- integration-tests/test_install.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/integration-tests/test_install.py b/integration-tests/test_install.py index 3f53b5f..866769e 100644 --- a/integration-tests/test_install.py +++ b/integration-tests/test_install.py @@ -46,9 +46,10 @@ def test_groups_exist(group): def debug_uid_gid(): - return ( - f"uid={os.getuid()} gid={os.getgid()} euid={os.geteuid()} egid={os.getegid()}" - ) + return subprocess.check_output("id").decode() + # return ( + # f"uid={os.getuid()} gid={os.getgid()} euid={os.geteuid()} egid={os.getegid()}" + # ) def permissions_test(group, path, *, readable=None, writable=None, dirs_only=False): From 967348069f6c002f088d280af18e60d6b2ce23be Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 27 Nov 2022 23:15:20 +0000 Subject: [PATCH 07/12] Add `inspect` to container debugging --- .github/integration-test.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/integration-test.py b/.github/integration-test.py index 0dc93e4..c67e273 100755 --- a/.github/integration-test.py +++ b/.github/integration-test.py @@ -45,9 +45,21 @@ def check_container_ready(container_name, timeout=60): now = time.time() while True: try: - container_check_output(["exec", "-t", container_name, "id"]) + out = container_check_output(["exec", "-t", container_name, "id"]) + print(out.decode()) return - except subprocess.CalledProcessError: + except subprocess.CalledProcessError as e: + print(e) + try: + out = container_check_output(["inspect", container_name]) + print(out.decode()) + except subprocess.CalledProcessError as e: + print(e) + try: + out = container_check_output(["logs", container_name]) + print(out.decode()) + except subprocess.CalledProcessError as e: + print(e) if time.time() - now > timeout: raise RuntimeError(f"Container {container_name} hasn't started") time.sleep(5) From 57b0ed89b3641e883a6fd481a7eb98b8cff5f814 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 27 Nov 2022 23:48:17 +0000 Subject: [PATCH 08/12] Try ignoring `/sys/fs/cgroup` mount --- .github/integration-test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/integration-test.py b/.github/integration-test.py index c67e273..a756711 100755 --- a/.github/integration-test.py +++ b/.github/integration-test.py @@ -82,8 +82,8 @@ def run_systemd_image(image_name, container_name, bootstrap_pip_spec): # If this is changed all docs references to the required memory must be changed too. "--memory=900m", ] - if container_runtime() != "podman": - cmd.append("--mount=type=bind,source=/sys/fs/cgroup,target=/sys/fs/cgroup") + # if container_runtime() != "podman": + # cmd.append("--mount=type=bind,source=/sys/fs/cgroup,target=/sys/fs/cgroup") if bootstrap_pip_spec: cmd.append("-e") From acd64c277e445e74e32a9cf135c25900369fd017 Mon Sep 17 00:00:00 2001 From: Pris Nasrat Date: Tue, 7 Feb 2023 10:35:19 -0500 Subject: [PATCH 09/12] Drop supplemental groups in install testing --- integration-tests/test_install.py | 1 + 1 file changed, 1 insertion(+) diff --git a/integration-tests/test_install.py b/integration-tests/test_install.py index 866769e..4dfa9eb 100644 --- a/integration-tests/test_install.py +++ b/integration-tests/test_install.py @@ -35,6 +35,7 @@ def setgroup(group): gid = grp.getgrnam(group).gr_gid uid = pwd.getpwnam("nobody").pw_uid os.setgid(gid) + os.setgroups([]) os.setuid(uid) os.environ["HOME"] = "/tmp/test-home-%i-%i" % (uid, gid) From 4eb2055e15365c87444daa57ad0a67f2b6146173 Mon Sep 17 00:00:00 2001 From: Pris Nasrat Date: Tue, 7 Feb 2023 11:41:09 -0500 Subject: [PATCH 10/12] Fix rtd build Pin Sphinx to below 6.0.0 see pydata/pydata-sphinx-theme#1094 --- docs/requirements.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/requirements.txt b/docs/requirements.txt index 7d2b038..2de526f 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,5 +1,7 @@ pydata-sphinx-theme -sphinx>=4 +# Sphix 6.0.0 breaks pydata-sphinx-theme +# See pydata/pydata-sphinx-theme#1094 +sphinx<6 sphinx_copybutton sphinx-autobuild sphinxext-opengraph From 4a701d2e1c8f27edec29db0041ba890eacafe956 Mon Sep 17 00:00:00 2001 From: Pris Nasrat Date: Thu, 9 Feb 2023 07:28:18 -0500 Subject: [PATCH 11/12] Remove commented out code in uid/gid debugging --- .github/integration-test.py | 2 -- integration-tests/test_install.py | 3 --- 2 files changed, 5 deletions(-) diff --git a/.github/integration-test.py b/.github/integration-test.py index a756711..25af580 100755 --- a/.github/integration-test.py +++ b/.github/integration-test.py @@ -82,8 +82,6 @@ def run_systemd_image(image_name, container_name, bootstrap_pip_spec): # If this is changed all docs references to the required memory must be changed too. "--memory=900m", ] - # if container_runtime() != "podman": - # cmd.append("--mount=type=bind,source=/sys/fs/cgroup,target=/sys/fs/cgroup") if bootstrap_pip_spec: cmd.append("-e") diff --git a/integration-tests/test_install.py b/integration-tests/test_install.py index 4dfa9eb..1391ddd 100644 --- a/integration-tests/test_install.py +++ b/integration-tests/test_install.py @@ -48,9 +48,6 @@ def test_groups_exist(group): def debug_uid_gid(): return subprocess.check_output("id").decode() - # return ( - # f"uid={os.getuid()} gid={os.getgid()} euid={os.geteuid()} egid={os.getegid()}" - # ) def permissions_test(group, path, *, readable=None, writable=None, dirs_only=False): From 9803df1d446fb0d419e3e72e018026a5850eb130 Mon Sep 17 00:00:00 2001 From: Pris Nasrat Date: Thu, 9 Feb 2023 10:39:02 -0500 Subject: [PATCH 12/12] Trigger CI GitHub workflow in case flaky