diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index 29b4adbc..c37211e3 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -75,6 +75,8 @@ jobs: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} with: fail_ci_if_error: true + # Work around https://github.com/codecov/codecov-action/issues/1876 + use_pypi: true tox-style: name: CI linters via Tox diff --git a/src/cwl_utils/image_puller.py b/src/cwl_utils/image_puller.py index 5604002d..a7c3ee70 100644 --- a/src/cwl_utils/image_puller.py +++ b/src/cwl_utils/image_puller.py @@ -2,9 +2,12 @@ """Classes for docker-extract.""" import logging +import os +import shutil import subprocess # nosec from abc import ABC, abstractmethod from pathlib import Path +from uuid import uuid4 from .singularity import get_version as get_singularity_version from .singularity import is_version_2_6 as is_singularity_version_2_6 @@ -22,7 +25,11 @@ def __init__( cmd: str, force_pull: bool, ) -> None: - """Create an ImagePuller.""" + """ + Create an ImagePuller. + + req already contains any tag that will be used. + """ self.req = req self.save_directory = save_directory self.cmd = cmd @@ -37,10 +44,17 @@ def save_docker_image(self) -> None: """Download and save the image to disk.""" @staticmethod - def _run_command_pull(cmd_pull: list[str]) -> None: + def _run_command_pull( + cmd_pull: list[str], + env_pull: dict[str, str] | None = None, + ) -> None: try: subprocess.run( # nosec - cmd_pull, check=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT + cmd_pull, + env=env_pull, + check=True, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, ) except subprocess.CalledProcessError as err: if err.output: @@ -84,16 +98,31 @@ def save_docker_image(self) -> None: class SingularityImagePuller(ImagePuller): - """Pull docker image with Singularity.""" + """ + Pull docker image with Singularity. - CHARS_TO_REPLACE = ["/", ":"] - NEW_CHAR = "_" + The image req may not contain a protocol. - def get_image_name(self) -> str: - """Determine the file name appropriate to the installed version of Singularity.""" - image_name = self.req - for char in self.CHARS_TO_REPLACE: - image_name = image_name.replace(char, self.NEW_CHAR) + The image req, if it refers to a Docker image, may or may not contain a + tag. + """ + + CHARS_TO_REPLACE = ["_", "/"] + NEW_STRINGS = ["___", "_s_"] + + def _image_to_filename( + self, + image_name: str, + to_replace: list[str], + replacements: list[str], + ) -> str: + """ + Get the filename for an image, using the given replacements to escape it. + + The filename will be appropriate for the current Singularity. + """ + for char, replacement in zip(to_replace, replacements): + image_name = image_name.replace(char, replacement) if is_singularity_version_2_6(): suffix = ".img" elif is_singularity_version_3_or_newer(): @@ -102,32 +131,188 @@ def get_image_name(self) -> str: raise Exception( f"Don't know how to handle this version of singularity: {get_singularity_version()}." ) - return f"{image_name}{suffix}" + filename = f"{image_name}{suffix}" + return filename + + def _could_be_current_image(self, filename: str) -> bool: + """ + Check if a path could belong to the current image name encoding scheme. + + This allows us to be backward-compatible with most existing cached + images, without risking treating cache entries created under the new + scheme as belonging to different images under older schemes. + + Is not guaranteed to be a tight bound: may return True for things that + can't actually be generated under the new scheme, but will never + return False for things that can. + """ + for replacement in self.NEW_STRINGS: + # Remove anything the new scheme generates involving replaceable + # characters. + filename = filename.replace(replacement, "") + for remaining in self.CHARS_TO_REPLACE: + if remaining in filename: + # We have something that can't have been generated under the + # new scheme. + return False + # If we don't see anything we can't make, we can probably make this path. + return True + + def get_image_name(self) -> str: + """Determine the file name appropriate to the installed version of Singularity.""" + return self._image_to_filename( + self.req, self.CHARS_TO_REPLACE, self.NEW_STRINGS + ) + + def get_alternate_image_names(self) -> list[str]: + """ + Determine filenames used by previous versions of cwltool or cwl-utils. + + These should be checked for the image and used if it exists there, + instead of pulling it again. + + These cover cwltool 3.2.20260720092025 and cwl-utils 0.42. + + If an image name could potentially also belong to some image under the + current scheme, it will not appear here. + """ + image_name = self.req + possibilities = [ + # Check the path cwl-utils 0.42 uses, with underscores for slashes + # and colons. + self._image_to_filename(image_name, ["/", ":"], ["_", "_"]), + # Check the path cwltool 3.2.20260720092025 uses, with _latest + # potentially appended and then only slashes replaced. + self._image_to_filename( + (image_name + "_latest") if ":" not in image_name else image_name, + ["/"], + ["_"], + ), + ] + possibilities = [ + p for p in possibilities if not self._could_be_current_image(p) + ] + return possibilities + + def find_destination_path(self) -> Path: + """Find the path where the image belongs.""" + save_directory: str | Path + if self.save_directory: + save_directory = self.save_directory + return Path(save_directory, self.get_image_name()) + + def _promote(self, source: Path, target: Path) -> None: + """ + Promote an image from an alternate path to a main path. + + Will hardlink source at target if possible, and copy it there + otherwise. + """ + try: + target.hardlink_to(source) + except NotImplementedError: + # Use a temporary file to make sure the replacement is atomic. + # Don't use mkstemp because we might want the file to be readable + # by other users. + temp_target = target.with_suffix(f".tmp.{uuid4()}") + shutil.copy(source, temp_target) + temp_target.replace(target) + + def save_docker_image_from_cache( + self, + target: Path, + search_paths: list[Path] | None = None, + ) -> bool: + """ + Put the image we need at our destination path, if we have it locally. + + :param target: The destination path. + + Checks target, plus under save_directory, plus inder the directories in + search_paths if provided. + + :returns: True if the image was found and put in place, and False otherwise. + + If force_pull is set, does nothing and returns False. + """ + if self.force_pull: + # Never use the cache, always pull. + return False - def save_docker_image(self) -> None: - """Pull down the Docker software container image and save it in the Singularity image format.""" save_directory: str | Path if self.save_directory: save_directory = self.save_directory - target = Path(save_directory, self.get_image_name()) - if target.exists() and not self.force_pull: + if target.exists(): _LOGGER.info(f"Already cached {self.req} with Singularity.") + return True + # Otherwise check other paths old versions may have placed it at. + + # We want to find any of these names + names = [target.name] + self.get_alternate_image_names() + + # Recursively look under any of these paths + to_search: list[Path | str] = [save_directory] + if search_paths: + to_search.extend(search_paths) + + # Find the source file to promote to the target path + source: Path | None = None + for search_path in to_search: + for dirpath, _subdirs, files in os.walk(search_path): + # We need to check our filenames in priority order + file_set = set(files) + for wanted in names: + if wanted in file_set: + path = Path(dirpath) / wanted + if os.path.isfile(path): + _LOGGER.info( + "Using local copy of Singularity image %s found in %s", + wanted, + dirpath, + ) + source = path + break + if source is not None: + break + if source is not None: + break + + if source: + self._promote(source, target) + return True + return False + + def save_docker_image(self) -> None: + """ + Pull down the Docker software container image and save it in the Singularity image format. + + Uses the cache if possible. + """ + target = self.find_destination_path() + if self.save_docker_image_from_cache(target): return + _LOGGER.info(f"Pulling {self.req} with Singularity...") cmd_pull = [ self.cmd, "pull", ] + env_pull = os.environ.copy() + + if is_singularity_version_2_6(): + env_pull["SINGULARITY_PULLFOLDER"] = str(target.parent) + target_name = target.name + else: + target_name = str(target) + if self.force_pull: cmd_pull.append("--force") cmd_pull.extend( [ "--name", - str(target), + target_name, f"docker://{self.req}", ] ) - ImagePuller._run_command_pull(cmd_pull) - _LOGGER.info( - f"Image successfully pulled: {save_directory}/{self.get_image_name()}" - ) + ImagePuller._run_command_pull(cmd_pull, env_pull) + _LOGGER.info(f"Image successfully pulled: {target}") diff --git a/src/cwl_utils/tests/test_image_puller.py b/src/cwl_utils/tests/test_image_puller.py new file mode 100644 index 00000000..122c1f9f --- /dev/null +++ b/src/cwl_utils/tests/test_image_puller.py @@ -0,0 +1,83 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Tests for classes for docker-extract.""" + +from cwl_utils.image_puller import SingularityImagePuller +from cwl_utils.singularity import get_version as get_singularity_version +from cwl_utils.singularity import is_version_2_6 as is_singularity_version_2_6 +from cwl_utils.singularity import ( + is_version_3_or_newer as is_singularity_version_3_or_newer, +) + +from .util import needs_singularity + + +@needs_singularity +class TestSingularityImagePuller: + """Tests for SingularityImagePuller.""" + + def test_get_image_name_matches_cwltool(self) -> None: + """Make sure image names generated match those expected by cwltool.""" + if is_singularity_version_2_6(): + suffix = ".img" + elif is_singularity_version_3_or_newer(): + suffix = ".sif" + else: + raise Exception( + f"Don't know how to handle this version of singularity: {get_singularity_version()}." + ) + + def get_name(s: str) -> str: + return SingularityImagePuller(s, None, "", False).get_image_name() + + assert get_name("some_name/repo:123") == f"some___name_s_repo:123{suffix}" + assert get_name("some/name_repo:123") == f"some_s_name___repo:123{suffix}" + + # We have to include the normal name here because if there aren't + # slashes or underscores and a tag is included we generate the same + # names for the same images under the new and old schemes. + + def test_get_image_names_match_old_cwltool(self) -> None: + """ + Check image names against cwltool. + + Make sure main and alternate image names tried include those previously + used by cwltool 3.2.20260720092025. + """ + if is_singularity_version_2_6(): + suffix = ".img" + elif is_singularity_version_3_or_newer(): + suffix = ".sif" + else: + raise Exception( + f"Don't know how to handle this version of singularity: {get_singularity_version()}." + ) + + def get_names(s: str) -> list[str]: + puller = SingularityImagePuller(s, None, "", False) + return [puller.get_image_name()] + puller.get_alternate_image_names() + + assert f"debian:stable-slim{suffix}" in get_names("debian:stable-slim") + assert f"quay.io_user_image_latest{suffix}" in get_names("quay.io/user/image") + + def test_get_image_names_match_old_cwl_utils(self) -> None: + """ + Check image names against cwl-utils. + + Make sure main and alternate image names tried include those previously + used by cwl-utils 0.42 + """ + if is_singularity_version_2_6(): + suffix = ".img" + elif is_singularity_version_3_or_newer(): + suffix = ".sif" + else: + raise Exception( + f"Don't know how to handle this version of singularity: {get_singularity_version()}." + ) + + def get_names(s: str) -> list[str]: + puller = SingularityImagePuller(s, None, "", False) + return [puller.get_image_name()] + puller.get_alternate_image_names() + + assert f"debian_stable-slim{suffix}" in get_names("debian:stable-slim") + assert f"quay.io_user_image{suffix}" in get_names("quay.io/user/image")