From 1a3c48a50091878ed9dca1b5db7aa6d051d30fa8 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 21 Mar 2023 14:21:00 +0100 Subject: [PATCH 01/14] Update base user environment to mambaforge 22.11.1-4 shift some duplicated code into utility functions and constants --- tests/test_conda.py | 26 +++++------- tljh/conda.py | 41 +++++++++++++++---- tljh/installer.py | 98 +++++++++++++++++++++++++++++++-------------- tljh/utils.py | 9 +++++ 4 files changed, 120 insertions(+), 54 deletions(-) diff --git a/tests/test_conda.py b/tests/test_conda.py index a13ab39..d38a85a 100644 --- a/tests/test_conda.py +++ b/tests/test_conda.py @@ -2,6 +2,7 @@ Test conda commandline wrappers """ from tljh import conda +from tljh import installer import os import pytest import subprocess @@ -13,25 +14,20 @@ def prefix(): """ Provide a temporary directory with a mambaforge conda environment """ - # see https://github.com/conda-forge/miniforge/releases - mambaforge_version = "4.10.3-7" - if os.uname().machine == "aarch64": - installer_sha256 = ( - "ac95f137b287b3408e4f67f07a284357b1119ee157373b788b34e770ef2392b2" - ) - elif os.uname().machine == "x86_64": - installer_sha256 = ( - "fc872522ec427fcab10167a93e802efaf251024b58cc27b084b915a9a73c4474" - ) - installer_url = "https://github.com/conda-forge/miniforge/releases/download/{v}/Mambaforge-{v}-Linux-{arch}.sh".format( - v=mambaforge_version, arch=os.uname().machine - ) + machine = os.uname().machine + installer_url, checksum = installer._mambaforge_url() with tempfile.TemporaryDirectory() as tmpdir: with conda.download_miniconda_installer( - installer_url, installer_sha256 + installer_url, checksum ) as installer_path: conda.install_miniconda(installer_path, tmpdir) - conda.ensure_conda_packages(tmpdir, ["conda==4.10.3"]) + conda.ensure_conda_packages( + tmpdir, + [ + f"conda=={installer.MAMBAFORGE_CONDA_VERSION}", + f"mamba=={installer.MAMBAFORGE_MAMBA_VERSION}", + ], + ) yield tmpdir diff --git a/tljh/conda.py b/tljh/conda.py index 88923f6..7aa864a 100644 --- a/tljh/conda.py +++ b/tljh/conda.py @@ -8,8 +8,8 @@ import hashlib import contextlib import tempfile import requests -from distutils.version import LooseVersion as V from tljh import utils +from tljh.utils import parse_version as V def sha256_file(fname): @@ -29,19 +29,44 @@ def check_miniconda_version(prefix, version): """ Return true if a miniconda install with version exists at prefix """ + versions = get_mamba_versions(prefix) + if "conda" not in versions: + return False + + return V(versions["conda"]) >= V(version) + + +def get_mamba_versions(prefix): + """Parse `mamba --version` output into a dict + + which looks like: + + mamba 1.1.0 + conda 22.11.1 + + into: + + { + "mamba": "1.1.0", + "conda": "22.11.1", + } + """ + versions = {} try: - installed_version = ( + out = ( subprocess.check_output( - [os.path.join(prefix, "bin", "conda"), "-V"], stderr=subprocess.STDOUT + [os.path.join(prefix, "bin", "mamba"), "--version"], + stderr=subprocess.STDOUT, ) .decode() .strip() - .split()[1] ) - return V(installed_version) >= V(version) except (subprocess.CalledProcessError, FileNotFoundError): - # Conda doesn't exist - return False + return versions + for line in out.strip().splitlines(): + pkg, version = line.split() + versions[pkg] = version + return versions @contextlib.contextmanager @@ -53,7 +78,7 @@ def download_miniconda_installer(installer_url, sha256sum): of given version, verifies the sha256sum & provides path to it to the `with` block to run. """ - with tempfile.NamedTemporaryFile("wb") as f: + with tempfile.NamedTemporaryFile("wb", suffix=".sh") as f: f.write(requests.get(installer_url).content) # Remain in the NamedTemporaryFile context, but flush changes, see: # https://docs.python.org/3/library/os.html#os.fsync diff --git a/tljh/installer.py b/tljh/installer.py index 7e9948d..c11eb3e 100644 --- a/tljh/installer.py +++ b/tljh/installer.py @@ -26,6 +26,7 @@ from tljh import ( traefik, user, ) + from .config import ( CONFIG_DIR, CONFIG_FILE, @@ -34,6 +35,7 @@ from .config import ( STATE_DIR, USER_ENV_PREFIX, ) +from .utils import parse_version as V from .yaml import yaml HERE = os.path.abspath(os.path.dirname(__file__)) @@ -154,58 +156,92 @@ def ensure_usergroups(): f.write("Defaults exempt_group = jupyterhub-admins\n") +# Install mambaforge using an installer from +# https://github.com/conda-forge/miniforge/releases +MAMBAFORGE_VERSION = "22.11.1-4" +# sha256 checksums +MAMBAFORGE_CHECKSUMS = { + "aarch64": "96191001f27e0cc76612d4498d34f9f656d8a7dddee44617159e42558651479c", + "x86_64": "16c7d256de783ceeb39970e675efa4a8eb830dcbb83187f1197abfea0bf07d30", +} +# run `mamba --version` to get the conda and mamba versions +# conda/mamba will be _upgraded_ to these versions, if they differ from what's in +# the mambaforge distribution +MAMBAFORGE_MAMBA_VERSION = "1.1.0" +MAMBAFORGE_CONDA_VERSION = "22.11.1" + + +def _mambaforge_url(version=MAMBAFORGE_VERSION, arch=None): + """Return (URL, checksum) for mambaforge download for a given version and arch + + Default values provided for both version and arch + """ + if arch is None: + arch = os.uname().machine + installer_url = "https://github.com/conda-forge/miniforge/releases/download/{v}/Mambaforge-{v}-Linux-{arch}.sh".format( + v=version, + arch=arch, + ) + # Check system architecture, set appropriate installer checksum + checksum = MAMBAFORGE_CHECKSUMS.get(arch) + if not checksum: + raise ValueError( + f"Unsupported architecture: {arch}. TLJH only supports {','.join(MAMBAFORGE_CHECKSUMS.keys())}" + ) + return installer_url, checksum + + def ensure_user_environment(user_requirements_txt_file): """ Set up user conda environment with required packages """ logger.info("Setting up user environment...") + # note: these must be in descending order + conda_upgrade_versions = { + # format: "conda version": (conda_version, mamba_version), + # mambaforge 4.10.3-7 (2023-03-21) + "22.11.1": (MAMBAFORGE_CONDA_VERSION, MAMBAFORGE_MAMBA_VERSION), + # tljh up to 0.2.0 (2021-10-18) + "4.10.3": ("4.10.3", "0.16.0"), + # very old versions, do these still work? + "4.7.10": ("4.8.1", "0.16.0"), + "4.5.4": ("4.5.8", "0.16.0"), + } - miniconda_old_version = "4.5.4" - miniconda_new_version = "4.7.10" - # Install mambaforge using an installer from - # https://github.com/conda-forge/miniforge/releases - mambaforge_new_version = "4.10.3-7" - # Check system architecture, set appropriate installer checksum - if os.uname().machine == "aarch64": - installer_sha256 = ( - "ac95f137b287b3408e4f67f07a284357b1119ee157373b788b34e770ef2392b2" - ) - elif os.uname().machine == "x86_64": - installer_sha256 = ( - "fc872522ec427fcab10167a93e802efaf251024b58cc27b084b915a9a73c4474" - ) # Check OS, set appropriate string for conda installer path if os.uname().sysname != "Linux": raise OSError("TLJH is only supported on Linux platforms.") - # Then run `mamba --version` to get the conda and mamba versions - # Keep these in sync with tests/test_conda.py::prefix - mambaforge_conda_new_version = "4.10.3" - mambaforge_mamba_version = "0.16.0" + found_conda = False + have_versions = conda.get_mamba_versions(USER_ENV_PREFIX) + have_conda_version = have_versions.get("conda") + if have_conda_version: + for check_version, conda_mamba_version in conda_upgrade_versions.items(): + if V(have_conda_version) >= V(check_version): + found_conda = True + conda_version, mamba_version = conda_mamba_version + break - if conda.check_miniconda_version(USER_ENV_PREFIX, mambaforge_conda_new_version): - conda_version = "4.10.3" - elif conda.check_miniconda_version(USER_ENV_PREFIX, miniconda_new_version): - conda_version = "4.8.1" - elif conda.check_miniconda_version(USER_ENV_PREFIX, miniconda_old_version): - conda_version = "4.5.8" - # If no prior miniconda installation is found, we can install a newer version - else: + if not found_conda: + if os.path.exists(USER_ENV_PREFIX): + logger.warning( + f"Found prefix at {USER_ENV_PREFIX}, but too old or missing conda/mamba ({have_versions}). Rebuilding env from scratch!!" + ) + # FIXME: should this fail? I'm not sure how destructive it is logger.info("Downloading & setting up user environment...") - installer_url = "https://github.com/conda-forge/miniforge/releases/download/{v}/Mambaforge-{v}-Linux-{arch}.sh".format( - v=mambaforge_new_version, arch=os.uname().machine - ) + installer_url, installer_sha256 = _mambaforge_url() with conda.download_miniconda_installer( installer_url, installer_sha256 ) as installer_path: conda.install_miniconda(installer_path, USER_ENV_PREFIX) - conda_version = "4.10.3" + conda_version = MAMBAFORGE_CONDA_VERSION + mamba_version = MAMBAFORGE_MAMBA_VERSION conda.ensure_conda_packages( USER_ENV_PREFIX, [ # Conda's latest version is on conda much more so than on PyPI. "conda==" + conda_version, - "mamba==" + mambaforge_mamba_version, + "mamba==" + MAMBAFORGE_MAMBA_VERSION, ], ) diff --git a/tljh/utils.py b/tljh/utils.py index 0b61da6..a78fb80 100644 --- a/tljh/utils.py +++ b/tljh/utils.py @@ -59,3 +59,12 @@ def get_plugin_manager(): pm.load_setuptools_entrypoints("tljh") return pm + + +def parse_version(version_string): + """Parse version string to tuple + + Finds all numbers and returns a tuple of ints + _very_ loose version parsing, like the old distutils.version.LooseVersion + """ + return tuple(int(part) for part in version_string.split(".")) From 6d0c1cbf63606444c077281b6b482ff5c3372cbd Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 21 Mar 2023 15:59:37 +0100 Subject: [PATCH 02/14] Remove very old conda versions from installer.py - 4.7.10 was Python 3.7 (no longer supported) - 4.5.4 was Python 3.6 --- tljh/installer.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tljh/installer.py b/tljh/installer.py index c11eb3e..52f956c 100644 --- a/tljh/installer.py +++ b/tljh/installer.py @@ -201,11 +201,8 @@ def ensure_user_environment(user_requirements_txt_file): # format: "conda version": (conda_version, mamba_version), # mambaforge 4.10.3-7 (2023-03-21) "22.11.1": (MAMBAFORGE_CONDA_VERSION, MAMBAFORGE_MAMBA_VERSION), - # tljh up to 0.2.0 (2021-10-18) + # tljh up to 0.2.0 (since 2021-10-18) "4.10.3": ("4.10.3", "0.16.0"), - # very old versions, do these still work? - "4.7.10": ("4.8.1", "0.16.0"), - "4.5.4": ("4.5.8", "0.16.0"), } # Check OS, set appropriate string for conda installer path From 594b61003f731df80ca1f754d58ef3bb68787996 Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 23 Mar 2023 12:30:35 +0100 Subject: [PATCH 03/14] add some logging to conda setup --- tljh/conda.py | 13 ++++++++++++- tljh/installer.py | 5 ++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/tljh/conda.py b/tljh/conda.py index 7aa864a..6f1f9ce 100644 --- a/tljh/conda.py +++ b/tljh/conda.py @@ -6,8 +6,12 @@ import subprocess import json import hashlib import contextlib +import logging import tempfile +import time + import requests + from tljh import utils from tljh.utils import parse_version as V @@ -78,12 +82,19 @@ def download_miniconda_installer(installer_url, sha256sum): of given version, verifies the sha256sum & provides path to it to the `with` block to run. """ + logger = logging.getLogger("tljh") + logger.info(f"Downloading conda installer {installer_url}") with tempfile.NamedTemporaryFile("wb", suffix=".sh") as f: - f.write(requests.get(installer_url).content) + tic = time.perf_counter() + r = requests.get(installer_url) + r.raise_for_status() + f.write(r.content) # Remain in the NamedTemporaryFile context, but flush changes, see: # https://docs.python.org/3/library/os.html#os.fsync f.flush() os.fsync(f.fileno()) + t = time.perf_counter() - tic + logger.info(f"Downloaded conda installer {installer_url} in {t:.1f}s") if sha256_file(f.name) != sha256sum: raise Exception("sha256sum hash mismatch! Downloaded file corrupted") diff --git a/tljh/installer.py b/tljh/installer.py index 52f956c..df94592 100644 --- a/tljh/installer.py +++ b/tljh/installer.py @@ -212,6 +212,9 @@ def ensure_user_environment(user_requirements_txt_file): have_versions = conda.get_mamba_versions(USER_ENV_PREFIX) have_conda_version = have_versions.get("conda") if have_conda_version: + logger.info( + f"Found prefix at {USER_ENV_PREFIX}, with conda/mamba({have_versions})" + ) for check_version, conda_mamba_version in conda_upgrade_versions.items(): if V(have_conda_version) >= V(check_version): found_conda = True @@ -238,7 +241,7 @@ def ensure_user_environment(user_requirements_txt_file): [ # Conda's latest version is on conda much more so than on PyPI. "conda==" + conda_version, - "mamba==" + MAMBAFORGE_MAMBA_VERSION, + "mamba==" + mamba_version, ], ) From 4d42f24e480b7fa875f0545430b426ae252d9915 Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 23 Mar 2023 12:34:44 +0100 Subject: [PATCH 04/14] test ensure_user_environment verify behavior for: - current version (no change) - old, supported version (upgrade, but not too far) - too old, re-run installer - directory exists, no conda --- tests/test_installer.py | 114 ++++++++++++++++++++++++++++++++++++++++ tljh/conda.py | 2 +- tljh/installer.py | 2 +- tljh/utils.py | 5 +- 4 files changed, 120 insertions(+), 3 deletions(-) diff --git a/tests/test_installer.py b/tests/test_installer.py index 9b42d70..bf06aae 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -1,10 +1,16 @@ """ Unit test functions in installer.py """ +import json import os +from unittest import mock +from subprocess import run, PIPE + import pytest +from tljh import conda from tljh import installer +from tljh.utils import parse_version as V from tljh.yaml import yaml @@ -36,3 +42,111 @@ def test_ensure_admins(tljh_dir, admins, expected_config): # verify the list was flattened assert config["users"]["admin"] == expected_config + + +def setup_conda(distro, version, prefix): + """Install mambaforge or miniconda in a prefix""" + if distro == "mambaforge": + installer_url, _ = installer._mambaforge_url(version) + elif distro == "miniconda": + arch = os.uname().machine + installer_url = ( + f"https://repo.anaconda.com/miniconda/Miniconda3-{version}-Linux-{arch}.sh" + ) + else: + raise ValueError(f"{distro=} must be 'miniconda' or 'mambaforge'") + with conda.download_miniconda_installer(installer_url, None) as installer_path: + conda.install_miniconda(installer_path, str(prefix)) + + +@pytest.fixture +def user_env_prefix(tmp_path): + user_env_prefix = tmp_path / "user_env" + with mock.patch.object(installer, "USER_ENV_PREFIX", str(user_env_prefix)): + yield user_env_prefix + + +@pytest.mark.parametrize( + "distro, version, conda_version, mamba_version", + [ + ( + None, + None, + installer.MAMBAFORGE_CONDA_VERSION, + installer.MAMBAFORGE_MAMBA_VERSION, + ), + ( + "exists", + None, + installer.MAMBAFORGE_CONDA_VERSION, + installer.MAMBAFORGE_MAMBA_VERSION, + ), + ( + "mambaforge", + "22.11.1-4", + installer.MAMBAFORGE_CONDA_VERSION, + installer.MAMBAFORGE_MAMBA_VERSION, + ), + ("mambaforge", "4.10.3-7", "4.10.3", "0.16.0"), + ( + "miniconda", + "4.7.10", + installer.MAMBAFORGE_CONDA_VERSION, + installer.MAMBAFORGE_MAMBA_VERSION, + ), + ( + "miniconda", + "4.5.1", + installer.MAMBAFORGE_CONDA_VERSION, + installer.MAMBAFORGE_MAMBA_VERSION, + ), + ], +) +def test_ensure_user_environment( + user_env_prefix, + distro, + version, + conda_version, + mamba_version, +): + if version and V(version) < V("4.10.1") and os.uname().machine == "aarch64": + pytest.skip(f"Miniconda {version} not available for aarch64") + canary_file = user_env_prefix / "test-file.txt" + canary_package = "types-backports_abc" + if distro: + if distro == "exists": + user_env_prefix.mkdir() + else: + setup_conda(distro, version, user_env_prefix) + # install a noarch: python package that won't be used otherwise + # should depend on Python, so it will interact with possible upgrades + run( + [str(user_env_prefix / "bin/conda"), "install", "-y", canary_package], + input="", + check=True, + ) + + # make a file not managed by conda, to check for wipeouts + with canary_file.open("w") as f: + f.write("I'm here\n") + + installer.ensure_user_environment("") + p = run( + [str(user_env_prefix / "bin/conda"), "list", "--json"], + stdout=PIPE, + text=True, + check=True, + ) + package_list = json.loads(p.stdout) + packages = {package["name"]: package for package in package_list} + if distro: + # make sure we didn't wipe out files + assert canary_file.exists() + if distro != "exists": + # make sure we didn't delete the installed package + assert canary_package in packages + + assert "conda" in packages + assert packages["conda"]["version"] == conda_version + assert "mamba" in packages + assert packages["mamba"]["version"] == mamba_version diff --git a/tljh/conda.py b/tljh/conda.py index 6f1f9ce..19dc937 100644 --- a/tljh/conda.py +++ b/tljh/conda.py @@ -96,7 +96,7 @@ def download_miniconda_installer(installer_url, sha256sum): t = time.perf_counter() - tic logger.info(f"Downloaded conda installer {installer_url} in {t:.1f}s") - if sha256_file(f.name) != sha256sum: + if sha256sum and sha256_file(f.name) != sha256sum: raise Exception("sha256sum hash mismatch! Downloaded file corrupted") yield f.name diff --git a/tljh/installer.py b/tljh/installer.py index df94592..b463ff1 100644 --- a/tljh/installer.py +++ b/tljh/installer.py @@ -224,7 +224,7 @@ def ensure_user_environment(user_requirements_txt_file): if not found_conda: if os.path.exists(USER_ENV_PREFIX): logger.warning( - f"Found prefix at {USER_ENV_PREFIX}, but too old or missing conda/mamba ({have_versions}). Rebuilding env from scratch!!" + f"Found prefix at {USER_ENV_PREFIX}, but too old or missing conda/mamba ({have_versions}). Upgrading from mambaforge." ) # FIXME: should this fail? I'm not sure how destructive it is logger.info("Downloading & setting up user environment...") diff --git a/tljh/utils.py b/tljh/utils.py index a78fb80..d4bf127 100644 --- a/tljh/utils.py +++ b/tljh/utils.py @@ -2,6 +2,7 @@ Miscellaneous functions useful in at least two places unrelated to each other """ import logging +import re import subprocess # Copied into bootstrap/bootstrap.py. Make sure these two copies are exactly the same! @@ -67,4 +68,6 @@ def parse_version(version_string): Finds all numbers and returns a tuple of ints _very_ loose version parsing, like the old distutils.version.LooseVersion """ - return tuple(int(part) for part in version_string.split(".")) + # return a tuple of all the numbers in the version string + # always succeeds, even if passed nonsense + return tuple(int(part) for part in re.findall(r"\d+", version_string)) From a24e038136b819783ae2ee16586720f511db3938 Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 23 Mar 2023 15:39:46 +0100 Subject: [PATCH 05/14] get canary package from conda-forge --- tests/test_installer.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/test_installer.py b/tests/test_installer.py index bf06aae..97524d6 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -121,7 +121,13 @@ def test_ensure_user_environment( # install a noarch: python package that won't be used otherwise # should depend on Python, so it will interact with possible upgrades run( - [str(user_env_prefix / "bin/conda"), "install", "-y", canary_package], + [ + str(user_env_prefix / "bin/conda"), + "install", + "-y", + "-c" "conda-forge", + canary_package, + ], input="", check=True, ) From 755d0d02fbe4cbbc0fff2ca7cdc17ed7220c3fa7 Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 23 Mar 2023 16:26:34 +0100 Subject: [PATCH 06/14] avoid auto-updating conda in test env because then we don't get the version we expect to test with! --- tests/test_installer.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_installer.py b/tests/test_installer.py index 97524d6..861c947 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -57,6 +57,19 @@ def setup_conda(distro, version, prefix): raise ValueError(f"{distro=} must be 'miniconda' or 'mambaforge'") with conda.download_miniconda_installer(installer_url, None) as installer_path: conda.install_miniconda(installer_path, str(prefix)) + # avoid auto-updating conda when we install other packages + run( + [ + str(prefix / "bin/conda"), + "config", + "--system", + "--set", + "auto_update_conda", + "false", + ], + input="", + check=True, + ) @pytest.fixture From 6b0b5f2998eaafa27d34c57f1a037ea861291365 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 24 Mar 2023 11:36:36 +0100 Subject: [PATCH 07/14] add bzip2 to test env required for testing against miniconda 4.5 --- .github/workflows/unit-test.yaml | 1 + integration-tests/Dockerfile | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/unit-test.yaml b/.github/workflows/unit-test.yaml index a0acde8..3c51a98 100644 --- a/.github/workflows/unit-test.yaml +++ b/.github/workflows/unit-test.yaml @@ -61,6 +61,7 @@ jobs: apt-get update apt-get install --yes \ python3-venv \ + bzip2 \ git python3 -m venv /srv/venv diff --git a/integration-tests/Dockerfile b/integration-tests/Dockerfile index 447bcb7..22cd2d4 100644 --- a/integration-tests/Dockerfile +++ b/integration-tests/Dockerfile @@ -8,6 +8,7 @@ RUN export DEBIAN_FRONTEND=noninteractive \ && apt-get update \ && apt-get install --yes \ systemd \ + bzip2 \ curl \ git \ sudo \ From 5980cb3ef22221b1f20c20a2e8928ef297f01668 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 24 Mar 2023 11:58:32 +0100 Subject: [PATCH 08/14] log commands before they start (at debug level) so you know what you're waiting for --- tljh/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tljh/utils.py b/tljh/utils.py index d4bf127..8ab1ca8 100644 --- a/tljh/utils.py +++ b/tljh/utils.py @@ -25,10 +25,11 @@ def run_subprocess(cmd, *args, **kwargs): and failed output directly to the user's screen """ logger = logging.getLogger("tljh") + printable_command = " ".join(cmd) + logger.debug("Running %s", printable_command) proc = subprocess.run( cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, *args, **kwargs ) - printable_command = " ".join(cmd) if proc.returncode != 0: # Our process failed! Show output to the user logger.error( From b5a6b3f590cd77dc9cdb1ce3fbfaafd3ad7a3680 Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 27 Mar 2023 13:10:31 +0200 Subject: [PATCH 09/14] Avoid downgrading user-env conda/mamba - Only check lower bound on conda/mamba, upgrade unbounded if not matched (let conda apply upper bound according to existing pins, such as Python) - handle missing mamba - avoid upgrading Python by aborting the install, instead of keeping old envs - minimum supported Python for user env is 3.9 - Fix output reporting of conda install step (no need for json capture when we don't parse the output - exit codes will do) --- dev-requirements.txt | 1 + tests/test_conda.py | 7 -- tests/test_installer.py | 169 +++++++++++++++++++++++++++------------- tljh/conda.py | 81 +++++-------------- tljh/installer.py | 105 +++++++++++++++---------- 5 files changed, 200 insertions(+), 163 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 0c50f19..ec27a97 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,3 +1,4 @@ +packaging pytest pytest-cov pytest-mock diff --git a/tests/test_conda.py b/tests/test_conda.py index d38a85a..46b7cac 100644 --- a/tests/test_conda.py +++ b/tests/test_conda.py @@ -21,13 +21,6 @@ def prefix(): installer_url, checksum ) as installer_path: conda.install_miniconda(installer_path, tmpdir) - conda.ensure_conda_packages( - tmpdir, - [ - f"conda=={installer.MAMBAFORGE_CONDA_VERSION}", - f"mamba=={installer.MAMBAFORGE_MAMBA_VERSION}", - ], - ) yield tmpdir diff --git a/tests/test_installer.py b/tests/test_installer.py index 861c947..dcffa71 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -6,11 +6,12 @@ import os from unittest import mock from subprocess import run, PIPE +from packaging.version import parse as V +from packaging.specifiers import SpecifierSet import pytest from tljh import conda from tljh import installer -from tljh.utils import parse_version as V from tljh.yaml import yaml @@ -48,13 +49,18 @@ def setup_conda(distro, version, prefix): """Install mambaforge or miniconda in a prefix""" if distro == "mambaforge": installer_url, _ = installer._mambaforge_url(version) + elif distro == "miniforge": + installer_url, _ = installer._mambaforge_url(version) + installer_url = installer_url.replace("Mambaforge", "Miniforge3") elif distro == "miniconda": arch = os.uname().machine installer_url = ( f"https://repo.anaconda.com/miniconda/Miniconda3-{version}-Linux-{arch}.sh" ) else: - raise ValueError(f"{distro=} must be 'miniconda' or 'mambaforge'") + raise ValueError( + f"{distro=} must be 'miniconda' or 'mambaforge' or 'miniforge'" + ) with conda.download_miniconda_installer(installer_url, None) as installer_path: conda.install_miniconda(installer_path, str(prefix)) # avoid auto-updating conda when we install other packages @@ -79,77 +85,127 @@ def user_env_prefix(tmp_path): yield user_env_prefix +def _specifier(version): + """Convert version string to SpecifierSet + + If just a version number, add == to make it a specifier + + Any missing fields are replaced with .* + + If it's already a specifier string, pass it directly to SpecifierSet + + e.g. + + - 3.7 -> ==3.7.* + - 1.2.3 -> ==1.2.3 + """ + if version[0].isdigit(): + # it's a version number, not a specifier + if version.count(".") < 2: + # pad missing fields + version += ".*" + version = f"=={version}" + return SpecifierSet(version) + + @pytest.mark.parametrize( - "distro, version, conda_version, mamba_version", + "distro, distro_version, expected_versions", [ + # No previous install, start fresh ( None, None, - installer.MAMBAFORGE_CONDA_VERSION, - installer.MAMBAFORGE_MAMBA_VERSION, - ), - ( - "exists", - None, - installer.MAMBAFORGE_CONDA_VERSION, - installer.MAMBAFORGE_MAMBA_VERSION, + { + "python": "3.10.*", + "conda": "22.11.1", + "mamba": "1.1.0", + }, ), + # previous install, 1.0 ( "mambaforge", "22.11.1-4", - installer.MAMBAFORGE_CONDA_VERSION, - installer.MAMBAFORGE_MAMBA_VERSION, + { + "python": "3.10.*", + "conda": "22.11.1", + "mamba": "1.1.0", + }, ), - ("mambaforge", "4.10.3-7", "4.10.3", "0.16.0"), + # 0.2 install, no upgrade needed + ( + "mambaforge", + "4.10.3-7", + { + "conda": "4.10.3", + "mamba": "0.16.0", + "python": "3.9.*", + }, + ), + # simulate missing mamba + # will be installed but not pinned + # to avoid conflicts + ( + "miniforge", + "4.10.3-7", + { + "conda": "4.10.3", + "mamba": ">=1.1.0", + "python": "3.9.*", + }, + ), + # too-old Python (3.7), abort ( "miniconda", "4.7.10", - installer.MAMBAFORGE_CONDA_VERSION, - installer.MAMBAFORGE_MAMBA_VERSION, - ), - ( - "miniconda", - "4.5.1", - installer.MAMBAFORGE_CONDA_VERSION, - installer.MAMBAFORGE_MAMBA_VERSION, + ValueError, ), ], ) def test_ensure_user_environment( user_env_prefix, distro, - version, - conda_version, - mamba_version, + distro_version, + expected_versions, ): - if version and V(version) < V("4.10.1") and os.uname().machine == "aarch64": - pytest.skip(f"Miniconda {version} not available for aarch64") + if ( + distro_version + and V(distro_version) < V("4.10.1") + and os.uname().machine == "aarch64" + ): + pytest.skip(f"{distro} {distro_version} not available for aarch64") canary_file = user_env_prefix / "test-file.txt" canary_package = "types-backports_abc" if distro: - if distro == "exists": - user_env_prefix.mkdir() - else: - setup_conda(distro, version, user_env_prefix) - # install a noarch: python package that won't be used otherwise - # should depend on Python, so it will interact with possible upgrades - run( - [ - str(user_env_prefix / "bin/conda"), - "install", - "-y", - "-c" "conda-forge", - canary_package, - ], - input="", - check=True, - ) + setup_conda(distro, distro_version, user_env_prefix) + # install a noarch: python package that won't be used otherwise + # should depend on Python, so it will interact with possible upgrades + pkgs = [canary_package] + run( + [ + str(user_env_prefix / "bin/conda"), + "install", + "-S", + "-y", + "-c", + "conda-forge", + ] + + pkgs, + input="", + check=True, + ) # make a file not managed by conda, to check for wipeouts with canary_file.open("w") as f: f.write("I'm here\n") - installer.ensure_user_environment("") + if isinstance(expected_versions, type) and issubclass(expected_versions, Exception): + exc_class = expected_versions + with pytest.raises(exc_class): + installer.ensure_user_environment("") + return + else: + installer.ensure_user_environment("") + p = run( [str(user_env_prefix / "bin/conda"), "list", "--json"], stdout=PIPE, @@ -158,14 +214,23 @@ def test_ensure_user_environment( ) package_list = json.loads(p.stdout) packages = {package["name"]: package for package in package_list} + if distro: # make sure we didn't wipe out files assert canary_file.exists() - if distro != "exists": - # make sure we didn't delete the installed package - assert canary_package in packages + # make sure we didn't delete the installed package + assert canary_package in packages - assert "conda" in packages - assert packages["conda"]["version"] == conda_version - assert "mamba" in packages - assert packages["mamba"]["version"] == mamba_version + for pkg, version in expected_versions.items(): + assert pkg in packages + assert V(packages[pkg]["version"]) in _specifier(version) + + +def test_ensure_user_environment_no_clobber(user_env_prefix): + # don't clobber existing user-env dir if it's non-empty and not a conda install + user_env_prefix.mkdir() + canary_file = user_env_prefix / "test-file.txt" + with canary_file.open("w") as f: + pass + with pytest.raises(OSError): + installer.ensure_user_environment("") diff --git a/tljh/conda.py b/tljh/conda.py index 19dc937..4a08fce 100644 --- a/tljh/conda.py +++ b/tljh/conda.py @@ -13,7 +13,6 @@ import time import requests from tljh import utils -from tljh.utils import parse_version as V def sha256_file(fname): @@ -29,47 +28,20 @@ def sha256_file(fname): return hash_sha256.hexdigest() -def check_miniconda_version(prefix, version): - """ - Return true if a miniconda install with version exists at prefix - """ - versions = get_mamba_versions(prefix) - if "conda" not in versions: - return False - - return V(versions["conda"]) >= V(version) - - -def get_mamba_versions(prefix): - """Parse `mamba --version` output into a dict - - which looks like: - - mamba 1.1.0 - conda 22.11.1 - - into: - - { - "mamba": "1.1.0", - "conda": "22.11.1", - } - """ +def get_conda_package_versions(prefix): + """Get conda package versions, via `conda list --json`""" versions = {} try: - out = ( - subprocess.check_output( - [os.path.join(prefix, "bin", "mamba"), "--version"], - stderr=subprocess.STDOUT, - ) - .decode() - .strip() + out = subprocess.check_output( + [os.path.join(prefix, "bin", "conda"), "list", "--json"], + text=True, ) except (subprocess.CalledProcessError, FileNotFoundError): return versions - for line in out.strip().splitlines(): - pkg, version = line.split() - versions[pkg] = version + + packages = json.loads(out) + for package in packages: + versions[package["name"]] = package["version"] return versions @@ -133,39 +105,24 @@ def ensure_conda_packages(prefix, packages): Note that conda seem to update dependencies by default, so there is probably no need to have a update parameter exposed for this function. """ - conda_executable = [os.path.join(prefix, "bin", "mamba")] + conda_executable = os.path.join(prefix, "bin", "mamba") + if not os.path.isfile(conda_executable): + # fallback on conda if mamba is not present (e.g. for mamba to install itself) + conda_executable = os.path.join(prefix, "bin", "conda") + abspath = os.path.abspath(prefix) - # Let subprocess errors propagate - # Explicitly do *not* capture stderr, since that's not always JSON! - # Scripting conda is a PITA! - # FIXME: raise different exception when using - raw_output = subprocess.check_output( - conda_executable - + [ + + utils.run_subprocess( + [ + conda_executable, "install", "-c", "conda-forge", # Make customizable if we ever need to - "--json", "--prefix", abspath, ] - + packages - ).decode() - # `conda install` outputs JSON lines for fetch updates, - # and a undelimited output at the end. There is no reasonable way to - # parse this outside of this kludge. - filtered_output = "\n".join( - [ - l - for l in raw_output.split("\n") - # Sometimes the JSON messages start with a \x00. The lstrip removes these. - # conda messages seem to randomly throw \x00 in places for no reason - if not l.lstrip("\x00").startswith('{"fetch"') - ] + + packages, ) - output = json.loads(filtered_output.lstrip("\x00")) - if "success" in output and output["success"] == True: - return fix_permissions(prefix) diff --git a/tljh/installer.py b/tljh/installer.py index b463ff1..90232d6 100644 --- a/tljh/installer.py +++ b/tljh/installer.py @@ -164,11 +164,15 @@ MAMBAFORGE_CHECKSUMS = { "aarch64": "96191001f27e0cc76612d4498d34f9f656d8a7dddee44617159e42558651479c", "x86_64": "16c7d256de783ceeb39970e675efa4a8eb830dcbb83187f1197abfea0bf07d30", } -# run `mamba --version` to get the conda and mamba versions -# conda/mamba will be _upgraded_ to these versions, if they differ from what's in -# the mambaforge distribution -MAMBAFORGE_MAMBA_VERSION = "1.1.0" -MAMBAFORGE_CONDA_VERSION = "22.11.1" + +# minimum versions of packages +MINIMUM_VERSIONS = { + # if conda/mamba are lower than this, upgrade them before installing the user packages + "mamba": "0.16.0", + "conda": "4.10", + # minimum Python version (if not matched, abort to avoid big disruptive updates) + "python": "3.9", +} def _mambaforge_url(version=MAMBAFORGE_VERSION, arch=None): @@ -196,54 +200,71 @@ def ensure_user_environment(user_requirements_txt_file): Set up user conda environment with required packages """ logger.info("Setting up user environment...") - # note: these must be in descending order - conda_upgrade_versions = { - # format: "conda version": (conda_version, mamba_version), - # mambaforge 4.10.3-7 (2023-03-21) - "22.11.1": (MAMBAFORGE_CONDA_VERSION, MAMBAFORGE_MAMBA_VERSION), - # tljh up to 0.2.0 (since 2021-10-18) - "4.10.3": ("4.10.3", "0.16.0"), - } - # Check OS, set appropriate string for conda installer path if os.uname().sysname != "Linux": raise OSError("TLJH is only supported on Linux platforms.") - found_conda = False - have_versions = conda.get_mamba_versions(USER_ENV_PREFIX) - have_conda_version = have_versions.get("conda") - if have_conda_version: - logger.info( - f"Found prefix at {USER_ENV_PREFIX}, with conda/mamba({have_versions})" - ) - for check_version, conda_mamba_version in conda_upgrade_versions.items(): - if V(have_conda_version) >= V(check_version): - found_conda = True - conda_version, mamba_version = conda_mamba_version - break - if not found_conda: - if os.path.exists(USER_ENV_PREFIX): - logger.warning( - f"Found prefix at {USER_ENV_PREFIX}, but too old or missing conda/mamba ({have_versions}). Upgrading from mambaforge." - ) - # FIXME: should this fail? I'm not sure how destructive it is + # Check the existing environment for what to do + package_versions = conda.get_conda_package_versions(USER_ENV_PREFIX) + + # Case 1: no existing environment + if not package_versions: + # 1a. no environment, but prefix exists. + # Abort to avoid clobbering something we don't recognize + if os.path.exists(USER_ENV_PREFIX) and os.listdir(USER_ENV_PREFIX): + msg = f"Found non-empty directory that is not a conda install in {USER_ENV_PREFIX}. Please remove it (or rename it to preserve files) and run tljh again." + logger.error(msg) + raise OSError(msg) + + # 1b. No environment, directory empty or doesn't exist + # start fresh install logger.info("Downloading & setting up user environment...") installer_url, installer_sha256 = _mambaforge_url() with conda.download_miniconda_installer( installer_url, installer_sha256 ) as installer_path: conda.install_miniconda(installer_path, USER_ENV_PREFIX) - conda_version = MAMBAFORGE_CONDA_VERSION - mamba_version = MAMBAFORGE_MAMBA_VERSION + package_versions = conda.get_conda_package_versions(USER_ENV_PREFIX) + # quick sanity check: we should have conda and mamba! + assert "conda" in package_versions + assert "mamba" in package_versions - conda.ensure_conda_packages( - USER_ENV_PREFIX, - [ - # Conda's latest version is on conda much more so than on PyPI. - "conda==" + conda_version, - "mamba==" + mamba_version, - ], - ) + # next, check Python + python_version = package_versions["python"] + logger.debug(f"Found python={python_version} in {USER_ENV_PREFIX}") + if V(python_version) < V(MINIMUM_VERSIONS["python"]): + msg = ( + f"TLJH requires Python >={MINIMUM_VERSIONS['python']}, found python={python_version} in {USER_ENV_PREFIX}." + f"\nPlease upgrade Python (may be highly disruptive!), or remove/rename {USER_ENV_PREFIX} to allow TLJH to make a fresh install." + f"\nYou can use `{USER_ENV_PREFIX}/bin/conda list` to save your current list of packages." + ) + logger.error(msg) + raise ValueError(msg) + + # at this point, we know we have an env ready with conda and are going to start installing + # first, check if we should upgrade/install conda and/or mamba + to_upgrade = [] + for pkg in ("conda", "mamba"): + version = package_versions.get(pkg) + min_version = MINIMUM_VERSIONS[pkg] + if not version: + logger.warning(f"{USER_ENV_PREFIX} is missing {pkg}, installing it...") + to_upgrade.append(pkg) + else: + logger.debug(f"Found {pkg}=={version} in {USER_ENV_PREFIX}") + if V(version) < V(min_version): + logger.info( + f"{USER_ENV_PREFIX} has {pkg}=={version}, it will be upgraded to {pkg}>={min_version}" + ) + to_upgrade.append(pkg) + + if to_upgrade: + conda.ensure_conda_packages( + USER_ENV_PREFIX, + # we _could_ explicitly pin Python here, + # but conda already does this by default + to_upgrade, + ) conda.ensure_pip_requirements( USER_ENV_PREFIX, From 9b940c34fde0f8cf90f4455e2b7018bc33356893 Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 27 Mar 2023 15:43:46 +0200 Subject: [PATCH 10/14] don't let conda install wait for input --- tljh/conda.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tljh/conda.py b/tljh/conda.py index 4a08fce..206e4df 100644 --- a/tljh/conda.py +++ b/tljh/conda.py @@ -116,12 +116,14 @@ def ensure_conda_packages(prefix, packages): [ conda_executable, "install", + "-y", "-c", "conda-forge", # Make customizable if we ever need to "--prefix", abspath, ] + packages, + input="", ) fix_permissions(prefix) From 6883316c363fe55fef0606dfe6ec924e5b21bb5c Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sat, 15 Apr 2023 11:06:29 +0200 Subject: [PATCH 11/14] Update mambaforge from 22.11.1-4 to 23.1.0-1 --- tests/test_installer.py | 10 +++++----- tljh/installer.py | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_installer.py b/tests/test_installer.py index dcffa71..a99db48 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -117,8 +117,8 @@ def _specifier(version): None, { "python": "3.10.*", - "conda": "22.11.1", - "mamba": "1.1.0", + "conda": "23.1.0", + "mamba": "1.4.1", }, ), # previous install, 1.0 @@ -127,8 +127,8 @@ def _specifier(version): "22.11.1-4", { "python": "3.10.*", - "conda": "22.11.1", - "mamba": "1.1.0", + "conda": "23.1.0", + "mamba": "1.4.1", }, ), # 0.2 install, no upgrade needed @@ -149,7 +149,7 @@ def _specifier(version): "4.10.3-7", { "conda": "4.10.3", - "mamba": ">=1.1.0", + "mamba": ">=1.4.1", "python": "3.9.*", }, ), diff --git a/tljh/installer.py b/tljh/installer.py index 90232d6..bbd0753 100644 --- a/tljh/installer.py +++ b/tljh/installer.py @@ -158,11 +158,11 @@ def ensure_usergroups(): # Install mambaforge using an installer from # https://github.com/conda-forge/miniforge/releases -MAMBAFORGE_VERSION = "22.11.1-4" +MAMBAFORGE_VERSION = "23.1.0-1" # sha256 checksums MAMBAFORGE_CHECKSUMS = { - "aarch64": "96191001f27e0cc76612d4498d34f9f656d8a7dddee44617159e42558651479c", - "x86_64": "16c7d256de783ceeb39970e675efa4a8eb830dcbb83187f1197abfea0bf07d30", + "aarch64": "d9d89c9e349369702171008d9ee7c5ce80ed420e5af60bd150a3db4bf674443a", + "x86_64": "cfb16c47dc2d115c8b114280aa605e322173f029fdb847a45348bf4bd23c62ab", } # minimum versions of packages From 438df10e1a19dca7a855aea75064b3cd86bded4e Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Mon, 17 Apr 2023 10:26:38 +0200 Subject: [PATCH 12/14] Fix tests for update of mambaforge from 22.11.1-4 to 23.1.0-1 --- tests/test_installer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_installer.py b/tests/test_installer.py index a99db48..e7af2ca 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -124,7 +124,7 @@ def _specifier(version): # previous install, 1.0 ( "mambaforge", - "22.11.1-4", + "23.1.0-1", { "python": "3.10.*", "conda": "23.1.0", From f13a31c20c0e8e4b5e9faea9b4c7a238e2b8362b Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 17 Apr 2023 10:56:24 +0200 Subject: [PATCH 13/14] update upgrade expectation for test_installer it may not upgrade all the way to 1.4 if other packages are pinned in the base env --- tests/test_installer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_installer.py b/tests/test_installer.py index e7af2ca..7675bd0 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -149,7 +149,7 @@ def _specifier(version): "4.10.3-7", { "conda": "4.10.3", - "mamba": ">=1.4.1", + "mamba": ">=1.1.0", "python": "3.9.*", }, ), From 2fcd8efde3b983959b501bd5ff9bcc3183a46284 Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 17 Apr 2023 11:00:18 +0200 Subject: [PATCH 14/14] increase test timeout 15 minutes isn't always enough for integration tests --- .github/workflows/integration-test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/integration-test.yaml b/.github/workflows/integration-test.yaml index 4c04c11..1b7939e 100644 --- a/.github/workflows/integration-test.yaml +++ b/.github/workflows/integration-test.yaml @@ -69,7 +69,7 @@ jobs: run: | pytest --verbose --maxfail=2 --color=yes --durations=10 --capture=no \ integration-tests/test_bootstrap.py - timeout-minutes: 15 + timeout-minutes: 20 env: # integration-tests/test_bootstrap.py will build and start containers # based on this environment variable. This is similar to how