From 65ddfc91e4e6099965e60f9f0f71f00b3dc5d120 Mon Sep 17 00:00:00 2001 From: Brandon Walker <43654521+misterbrandonwalker@users.noreply.github.com> Date: Tue, 5 Dec 2023 09:23:11 -0600 Subject: [PATCH] singularity: support dockerFile (#1938) * allow user to set temp dir with APPTAINER_TMPDIR * singularity build: automatically try fakeroot mode if proot is missing --------- Co-authored-by: Brandon Walker Co-authored-by: Michael R. Crusoe --- .github/workflows/ci-tests.yml | 4 +- cwltool/singularity.py | 43 ++++++++++--- mypy-stubs/spython/main/__init__.pyi | 9 +++ mypy-stubs/spython/main/base/__init__.pyi | 3 + mypy-stubs/spython/main/build.pyi | 23 +++++++ .../spython/main/parse/parsers/base.pyi | 14 ++++ .../spython/main/parse/parsers/docker.pyi | 7 ++ mypy-stubs/spython/main/parse/recipe.pyi | 19 ++++++ .../spython/main/parse/writers/base.pyi | 6 ++ .../main/parse/writers/singularity.pyi | 10 +++ requirements.txt | 1 + tests/test_tmpdir.py | 64 ++++++++++++++++++- tox.ini | 2 + 13 files changed, 193 insertions(+), 12 deletions(-) create mode 100644 mypy-stubs/spython/main/__init__.pyi create mode 100644 mypy-stubs/spython/main/base/__init__.pyi create mode 100644 mypy-stubs/spython/main/build.pyi create mode 100644 mypy-stubs/spython/main/parse/parsers/base.pyi create mode 100644 mypy-stubs/spython/main/parse/parsers/docker.pyi create mode 100644 mypy-stubs/spython/main/parse/recipe.pyi create mode 100644 mypy-stubs/spython/main/parse/writers/base.pyi create mode 100644 mypy-stubs/spython/main/parse/writers/singularity.pyi diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index cc16b032d..13deb5393 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -77,7 +77,7 @@ jobs: key: mypy-${{ env.py-semver }} - name: Test with tox - run: tox + run: APPTAINER_TMPDIR=${RUNNER_TEMP} tox - name: Upload coverage to Codecov if: ${{ matrix.step == 'unit' }} @@ -156,7 +156,7 @@ jobs: chmod a-w . - name: run tests - run: make test + run: APPTAINER_TMPDIR=${RUNNER_TEMP} make test conformance_tests: diff --git a/cwltool/singularity.py b/cwltool/singularity.py index 1a194fc94..2f590a140 100644 --- a/cwltool/singularity.py +++ b/cwltool/singularity.py @@ -10,6 +10,9 @@ from typing import Callable, Dict, List, MutableMapping, Optional, Tuple, cast from schema_salad.sourceline import SourceLine +from spython.main import Client +from spython.main.parse.parsers.docker import DockerParser +from spython.main.parse.writers.singularity import SingularityWriter from .builder import Builder from .context import RuntimeContext @@ -140,6 +143,7 @@ def __init__( def get_image( dockerRequirement: Dict[str, str], pull_image: bool, + tmp_outdir_prefix: str, force_pull: bool = False, ) -> bool: """ @@ -162,7 +166,35 @@ def get_image( elif is_version_2_6() and "SINGULARITY_PULLFOLDER" in os.environ: cache_folder = os.environ["SINGULARITY_PULLFOLDER"] - if "dockerImageId" not in dockerRequirement and "dockerPull" in dockerRequirement: + if "dockerFile" in dockerRequirement: + if cache_folder is None: # if environment variables were not set + cache_folder = create_tmp_dir(tmp_outdir_prefix) + + absolute_path = os.path.abspath(cache_folder) + dockerfile_path = os.path.join(absolute_path, "Dockerfile") + singularityfile_path = dockerfile_path + ".def" + # if you do not set APPTAINER_TMPDIR will crash + # WARNING: 'nodev' mount option set on /tmp, it could be a + # source of failure during build process + # FATAL: Unable to create build: 'noexec' mount option set on + # /tmp, temporary root filesystem won't be usable at this location + with open(dockerfile_path, "w") as dfile: + dfile.write(dockerRequirement["dockerFile"]) + + singularityfile = SingularityWriter(DockerParser(dockerfile_path).parse()).convert() + with open(singularityfile_path, "w") as file: + file.write(singularityfile) + + os.environ["APPTAINER_TMPDIR"] = absolute_path + singularity_options = ["--fakeroot"] if not shutil.which("proot") else [] + Client.build( + recipe=singularityfile_path, + build_folder=absolute_path, + sudo=False, + options=singularity_options, + ) + found = True + elif "dockerImageId" not in dockerRequirement and "dockerPull" in dockerRequirement: match = re.search(pattern=r"([a-z]*://)", string=dockerRequirement["dockerPull"]) img_name = _normalize_image_id(dockerRequirement["dockerPull"]) candidates.append(img_name) @@ -243,13 +275,6 @@ def get_image( check_call(cmd, stdout=sys.stderr) # nosec found = True - elif "dockerFile" in dockerRequirement: - raise SourceLine( - dockerRequirement, "dockerFile", WorkflowException, debug - ).makeError( - "dockerFile is not currently supported when using the " - "Singularity runtime for Docker containers." - ) elif "dockerLoad" in dockerRequirement: if is_version_3_1_or_newer(): if "dockerImageId" in dockerRequirement: @@ -298,7 +323,7 @@ def get_from_requirements( if not bool(shutil.which("singularity")): raise WorkflowException("singularity executable is not available") - if not self.get_image(cast(Dict[str, str], r), pull_image, force_pull): + if not self.get_image(cast(Dict[str, str], r), pull_image, tmp_outdir_prefix, force_pull): raise WorkflowException("Container image {} not found".format(r["dockerImageId"])) return os.path.abspath(cast(str, r["dockerImageId"])) diff --git a/mypy-stubs/spython/main/__init__.pyi b/mypy-stubs/spython/main/__init__.pyi new file mode 100644 index 000000000..adced1f3a --- /dev/null +++ b/mypy-stubs/spython/main/__init__.pyi @@ -0,0 +1,9 @@ +from typing import Iterator, Optional + +from .base import Client as _BaseClient +from .build import build as base_build + +class _Client(_BaseClient): + build = base_build + +Client = _Client() diff --git a/mypy-stubs/spython/main/base/__init__.pyi b/mypy-stubs/spython/main/base/__init__.pyi new file mode 100644 index 000000000..111997914 --- /dev/null +++ b/mypy-stubs/spython/main/base/__init__.pyi @@ -0,0 +1,3 @@ +class Client: + def __init__(self) -> None: ... + def version(self) -> str: ... diff --git a/mypy-stubs/spython/main/build.pyi b/mypy-stubs/spython/main/build.pyi new file mode 100644 index 000000000..098ba3436 --- /dev/null +++ b/mypy-stubs/spython/main/build.pyi @@ -0,0 +1,23 @@ +from typing import Iterator, Optional + +from .base import Client + +def build( + self: Client, + recipe: Optional[str] = ..., + image: Optional[str] = ..., + isolated: Optional[bool] = ..., + sandbox: Optional[bool] = ..., + writable: Optional[bool] = ..., + build_folder: Optional[str] = ..., + robot_name: Optional[bool] = ..., + ext: Optional[str] = ..., + sudo: Optional[bool] = ..., + stream: Optional[bool] = ..., + force: Optional[bool] = ..., + options: Optional[list[str]] | None = ..., + quiet: Optional[bool] = ..., + return_result: Optional[bool] = ..., + sudo_options: Optional[str | list[str]] = ..., + singularity_options: Optional[list[str]] = ..., +) -> tuple[str, Iterator[str]]: ... diff --git a/mypy-stubs/spython/main/parse/parsers/base.pyi b/mypy-stubs/spython/main/parse/parsers/base.pyi new file mode 100644 index 000000000..23eef9975 --- /dev/null +++ b/mypy-stubs/spython/main/parse/parsers/base.pyi @@ -0,0 +1,14 @@ +import abc + +from ..recipe import Recipe + +class ParserBase(metaclass=abc.ABCMeta): + filename: str + lines: list[str] + args: dict[str, str] + active_layer: str + active_layer_num: int + recipe: dict[str, Recipe] + def __init__(self, filename: str, load: bool = ...) -> None: ... + @abc.abstractmethod + def parse(self) -> dict[str, Recipe]: ... diff --git a/mypy-stubs/spython/main/parse/parsers/docker.pyi b/mypy-stubs/spython/main/parse/parsers/docker.pyi new file mode 100644 index 000000000..8adb8a547 --- /dev/null +++ b/mypy-stubs/spython/main/parse/parsers/docker.pyi @@ -0,0 +1,7 @@ +from ..recipe import Recipe +from .base import ParserBase as ParserBase + +class DockerParser(ParserBase): + name: str + def __init__(self, filename: str = ..., load: bool = ...) -> None: ... + def parse(self) -> dict[str, Recipe]: ... diff --git a/mypy-stubs/spython/main/parse/recipe.pyi b/mypy-stubs/spython/main/parse/recipe.pyi new file mode 100644 index 000000000..dabd4ebc5 --- /dev/null +++ b/mypy-stubs/spython/main/parse/recipe.pyi @@ -0,0 +1,19 @@ +from typing import Optional + +class Recipe: + cmd: Optional[str] + comments: list[str] + entrypoint: Optional[str] + environ: list[str] + files: list[str] + layer_files: dict[str, str] + install: list[str] + labels: list[str] + ports: list[str] + test: Optional[str] + volumes: list[str] + workdir: Optional[str] + layer: int + fromHeader: Optional[str] + source: Optional[Recipe] + def __init__(self, recipe: Optional[Recipe] = ..., layer: int = ...) -> None: ... diff --git a/mypy-stubs/spython/main/parse/writers/base.pyi b/mypy-stubs/spython/main/parse/writers/base.pyi new file mode 100644 index 000000000..3b4fe12da --- /dev/null +++ b/mypy-stubs/spython/main/parse/writers/base.pyi @@ -0,0 +1,6 @@ +from ..recipe import Recipe + +class WriterBase: + recipe: dict[str, Recipe] + def __init__(self, recipe: dict[str, Recipe] | None = ...) -> None: ... + def write(self, output_file: str | None = ..., force: bool = ...) -> None: ... diff --git a/mypy-stubs/spython/main/parse/writers/singularity.pyi b/mypy-stubs/spython/main/parse/writers/singularity.pyi new file mode 100644 index 000000000..c80198461 --- /dev/null +++ b/mypy-stubs/spython/main/parse/writers/singularity.pyi @@ -0,0 +1,10 @@ +from typing import Optional + +from ..recipe import Recipe +from .base import WriterBase as WriterBase + +class SingularityWriter(WriterBase): + name: str + def __init__(self, recipe: Optional[dict[str, Recipe]] = ...) -> None: ... + def validate(self) -> None: ... + def convert(self, runscript: str = ..., force: bool = ...) -> str: ... diff --git a/requirements.txt b/requirements.txt index d1ecc2b4c..036c4eed6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,3 +12,4 @@ pydot>=1.4.1 argcomplete>=1.12.0 pyparsing!=3.0.2 # breaks --print-dot (pydot) https://github.com/pyparsing/pyparsing/issues/319 cwl-utils>=0.32 +spython>=0.3.0 diff --git a/tests/test_tmpdir.py b/tests/test_tmpdir.py index 14caecb3c..57d693c2c 100644 --- a/tests/test_tmpdir.py +++ b/tests/test_tmpdir.py @@ -1,5 +1,7 @@ """Test that all temporary directories respect the --tmpdir-prefix and --tmp-outdir-prefix options.""" +import os import re +import shutil import subprocess import sys from pathlib import Path @@ -17,11 +19,12 @@ from cwltool.job import JobBase from cwltool.main import main from cwltool.pathmapper import MapperEnt +from cwltool.singularity import SingularityCommandLineJob from cwltool.stdfsaccess import StdFsAccess from cwltool.update import INTERNAL_VERSION, ORIGINAL_CWLVERSION from cwltool.utils import create_tmp_dir -from .util import get_data, get_main_output, needs_docker +from .util import get_data, get_main_output, needs_docker, needs_singularity def test_docker_commandLineTool_job_tmpdir_prefix(tmp_path: Path) -> None: @@ -164,6 +167,65 @@ def test_dockerfile_tmpdir_prefix(tmp_path: Path, monkeypatch: pytest.MonkeyPatc assert (subdir / "Dockerfile").exists() +@needs_singularity +def test_dockerfile_singularity_build(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + """Test that SingularityCommandLineJob.get_image builds a Dockerfile with Singularity.""" + tmppath = Path(os.environ.get("APPTAINER_TMPDIR", tmp_path)) + # some HPC not allowed to execute on /tmp so allow user to define temp path with APPTAINER_TMPDIR + # FATAL: Unable to create build: 'noexec' mount option set on /tmp, temporary root filesystem + monkeypatch.setattr(target=subprocess, name="check_call", value=lambda *args, **kwargs: True) + (tmppath / "out").mkdir(exist_ok=True) + tmp_outdir_prefix = tmppath / "out" / "1" + (tmppath / "3").mkdir(exist_ok=True) + tmpdir_prefix = str(tmppath / "3" / "ttmp") + runtime_context = RuntimeContext( + {"tmpdir_prefix": tmpdir_prefix, "user_space_docker_cmd": None} + ) + builder = Builder( + {}, + [], + [], + {}, + schema.Names(), + [], + [], + {}, + None, + None, + StdFsAccess, + StdFsAccess(""), + None, + 0.1, + True, + False, + False, + "no_listing", + runtime_context.get_outdir(), + runtime_context.get_tmpdir(), + runtime_context.get_stagedir(), + INTERNAL_VERSION, + "singularity", + ) + + assert SingularityCommandLineJob( + builder, {}, CommandLineTool.make_path_mapper, [], [], "" + ).get_image( + { + "class": "DockerRequirement", + "dockerFile": "FROM debian:stable-slim", + }, + pull_image=True, + tmp_outdir_prefix=str(tmp_outdir_prefix), + force_pull=True, + ) + children = sorted(tmp_outdir_prefix.parent.glob("*")) + subdir = tmppath / children[0] + children = sorted(subdir.glob("*.sif")) + image_path = children[0] + assert image_path.exists() + shutil.rmtree(subdir) + + def test_docker_tmpdir_prefix(tmp_path: Path) -> None: """Test that DockerCommandLineJob respects temp directory directives.""" (tmp_path / "3").mkdir() diff --git a/tox.ini b/tox.ini index 31b0fd91a..cd7fc0702 100644 --- a/tox.ini +++ b/tox.ini @@ -39,6 +39,8 @@ passenv = CI GITHUB_* PROOT_NO_SECCOMP + APPTAINER_TMPDIR + SINGULARITY_FAKEROOT extras = py3{8,9,10,11,12}-unit: deps