Avoid pulling existing Singularity image #177 (#178)

adrabent · mr-c · web-flow · commit 6d0f89edad59 · 2022-11-28T18:47:32.000+01:00
Singularity: avoid hitting Docker pull rate limit 

When using Singularity and in case nodejs is not installed on your system this image is pulled from Docker every time. In case of offline processing and for often pull requests to Docker this should be avoided.

 port back expr tests from cwltool

add test for CWL_SINGULARITY_CACHE

Co-authored-by: Michael R. Crusoe &lt;1330696+mr-c@users.noreply.github.com&gt;
diff --git a/cwl_utils/sandboxjs.py b/cwl_utils/sandboxjs.py
@@ -2,6 +2,7 @@
 """Safe execution of CWL Expressions in a NodeJS sandbox."""
 import collections
 import errno
+import glob
 import json
 import os
 import re
@@ -312,28 +313,40 @@ def new_js_proc(
                     nodeimg = f"docker://{nodeimg}"
 
                 if not self.have_node_slim:
+                    singularity_cache: Optional[str] = None
                     if container_engine in ("docker", "podman"):
                         dockerimgs = subprocess.check_output(  # nosec
                             [container_engine, "images", "-q", nodeimg],
                             universal_newlines=True,
                         )
-                    elif container_engine != "singularity":
+                    elif container_engine == "singularity":
+                        singularity_cache = os.environ.get("CWL_SINGULARITY_CACHE")
+                        if singularity_cache:
+                            singularityimgs = glob.glob(
+                                singularity_cache + "/node_slim.sif"
+                            )
+                        else:
+                            singularityimgs = glob.glob(os.getcwd() + "/node_slim.sif")
+                    else:
                         raise Exception(
                             f"Unknown container_engine: {container_engine}."
                         )
                     # if output is an empty string
-                    if (
-                        container_engine == "singularity"
-                        or len(dockerimgs.split("\n")) <= 1
-                        or force_docker_pull
-                    ):
+                    need_singularity = (
+                        container_engine == "singularity" and not singularityimgs
+                    )
+                    need_docker = container_engine != "singularity" and (
+                        len(dockerimgs.split("\n")) <= 1
+                    )
+                    if need_singularity or need_docker or force_docker_pull:
                         # pull node:slim docker container
                         nodejs_pull_commands = [container_engine, "pull"]
                         if force_docker_pull:
                             nodejs_pull_commands.append("--force")
                         nodejs_pull_commands.append(nodeimg)
+                        cwd = singularity_cache if singularity_cache else os.getcwd()
                         nodejsimg = subprocess.check_output(  # nosec
-                            nodejs_pull_commands, universal_newlines=True
+                            nodejs_pull_commands, universal_newlines=True, cwd=cwd
                         )
                         _logger.debug(
                             "Pulled Docker image %s %s using %s",
diff --git a/setup.py b/setup.py
@@ -37,7 +37,7 @@
     )
     .read()
     .splitlines(),
-    tests_require=["pytest<8"],
+    tests_require=["pytest<8", "pytest-mock"],
     test_suite="tests",
     extras_require={"pretty": ["cwlformat"]},
     entry_points={
diff --git a/test-requirements.txt b/test-requirements.txt
@@ -2,3 +2,4 @@ pytest < 8
 pytest-cov
 pytest-xdist
 cwlformat
+pytest-mock >= 1.10.0
diff --git a/tests/test_js_sandbox.py b/tests/test_js_sandbox.py
@@ -0,0 +1,167 @@
+"""Test sandboxjs.py and related code."""
+import logging
+import os
+import shutil
+import threading
+from pathlib import Path
+from typing import Any, List
+
+import pytest
+
+from cwl_utils import expression, sandboxjs
+
+from .util import get_data, needs_podman, needs_singularity
+
+node_versions = [
+    ("v0.8.26\n", False),
+    ("v0.10.25\n", False),
+    ("v0.10.26\n", True),
+    ("v4.4.2\n", True),
+    ("v7.7.3\n", True),
+]
+
+
+@pytest.mark.parametrize("version,supported", node_versions)
+def test_node_version(version: str, supported: bool, mocker: Any) -> None:
+    mocked_subprocess = mocker.patch("cwl_utils.sandboxjs.subprocess")
+    mocked_subprocess.check_output = mocker.Mock(return_value=version)
+
+    assert sandboxjs.check_js_threshold_version("node") == supported
+
+
+def test_value_from_two_concatenated_expressions() -> None:
+    js_engine = sandboxjs.get_js_engine()
+    js_engine.have_node_slim = False  # type: ignore[attr-defined]
+    js_engine.localdata = threading.local()  # type: ignore[attr-defined]
+    assert (
+        expression.do_eval(
+            '$("a ")$("string")',
+            {},
+            [{"class": "InlineJavascriptRequirement"}],
+            None,
+            None,
+            {},
+            cwlVersion="v1.0",
+        )
+        == "a string"
+    )
+
+
+def hide_nodejs(temp_dir: Path) -> str:
+    """Generate a new PATH that hides node{js,}."""
+    paths: List[str] = os.environ.get("PATH", "").split(":")
+    names: List[str] = []
+    if "/bin" in paths:
+        bin_path = Path("/bin")
+        if (
+            bin_path.is_symlink()
+            and os.readlink(bin_path) == "usr/bin"
+            and "/usr/bin" in paths
+        ):
+            paths.remove("/bin")
+    for name in ("nodejs", "node"):
+        path = shutil.which(name, path=":".join(paths))
+        if path:
+            names.append(path)
+    for name in names:
+        dirname = os.path.dirname(name)
+        if dirname in paths:
+            paths.remove(dirname)
+            new_dir = temp_dir / os.path.basename(dirname)
+            new_dir.mkdir()
+            for entry in os.listdir(dirname):
+                if entry not in ("nodejs", "node"):
+                    os.symlink(os.path.join(dirname, entry), new_dir / entry)
+            paths.append(str(new_dir))
+    return ":".join(paths)
+
+
+@needs_podman
+def test_value_from_two_concatenated_expressions_podman(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Javascript test using podman."""
+    new_paths = hide_nodejs(tmp_path)
+    with monkeypatch.context() as m:
+        m.setenv("PATH", new_paths)
+        js_engine = sandboxjs.get_js_engine()
+        js_engine.have_node_slim = False  # type: ignore[attr-defined]
+        js_engine.localdata = threading.local()  # type: ignore[attr-defined]
+        assert (
+            expression.do_eval(
+                '$("a ")$("string")',
+                {},
+                [{"class": "InlineJavascriptRequirement"}],
+                None,
+                None,
+                {},
+                cwlVersion="v1.0",
+                container_engine="podman",
+            )
+            == "a string"
+        )
+
+
+@needs_singularity
+def test_value_from_two_concatenated_expressions_singularity(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Javascript test using Singularity."""
+    new_paths = hide_nodejs(tmp_path)
+    with monkeypatch.context() as m:
+        m.setenv("PATH", new_paths)
+        js_engine = sandboxjs.get_js_engine()
+        js_engine.have_node_slim = False  # type: ignore[attr-defined]
+        js_engine.localdata = threading.local()  # type: ignore[attr-defined]
+        assert (
+            expression.do_eval(
+                '$("a ")$("string")',
+                {},
+                [{"class": "InlineJavascriptRequirement"}],
+                None,
+                None,
+                {},
+                cwlVersion="v1.0",
+                container_engine="singularity",
+            )
+            == "a string"
+        )
+
+
+@needs_singularity
+def test_singularity_cache(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    """Affirm that CWL_SINGULARIT_CACHE is respected."""
+    bin_path = tmp_path / "no_nodejs"
+    bin_path.mkdir()
+    new_paths = hide_nodejs(bin_path)
+    cache_path = tmp_path / "singularity_cache"
+    cache_path.mkdir()
+    with monkeypatch.context() as m:
+        m.setenv("PATH", new_paths)
+        m.setenv("CWL_SINGULARITY_CACHE", str(cache_path))
+        js_engine = sandboxjs.get_js_engine()
+        js_engine.localdata = threading.local()  # type: ignore[attr-defined]
+        js_engine.have_node_slim = False  # type: ignore[attr-defined]
+        assert (
+            expression.do_eval(
+                "$(42*23)",
+                {},
+                [{"class": "InlineJavascriptRequirement"}],
+                None,
+                None,
+                {},
+                cwlVersion="v1.0",
+                container_engine="singularity",
+            )
+            == 42 * 23
+        )
+        assert (cache_path / "node_slim.sif").exists()
+
+
+def test_caches_js_processes(mocker: Any) -> None:
+    sandboxjs.exec_js_process("7", context="{}")
+
+    mocked_new_js_proc = mocker.patch("cwl_utils.sandboxjs.new_js_proc")
+    sandboxjs.exec_js_process("7", context="{}")
+
+    mocked_new_js_proc.assert_not_called()
diff --git a/tests/util.py b/tests/util.py
@@ -0,0 +1,41 @@
+import contextlib
+import io
+import json
+import os
+import shutil
+import subprocess
+import sys
+from pathlib import Path
+from typing import Dict, Generator, List, Mapping, Optional, Tuple, Union
+
+import pytest
+from pkg_resources import Requirement, ResolutionError, resource_filename
+
+
+def get_data(filename: str) -> str:
+    # normalizing path depending on OS or else it will cause problem when joining path
+    filename = os.path.normpath(filename)
+    filepath = None
+    try:
+        filepath = resource_filename(Requirement.parse("schema-salad"), filename)
+    except ResolutionError:
+        pass
+    if not filepath or not os.path.isfile(filepath):
+        filepath = os.path.join(os.path.dirname(__file__), os.pardir, filename)
+    return str(Path(filepath).resolve())
+
+
+needs_docker = pytest.mark.skipif(
+    not bool(shutil.which("docker")),
+    reason="Requires the docker executable on the system path.",
+)
+
+needs_singularity = pytest.mark.skipif(
+    not bool(shutil.which("singularity")),
+    reason="Requires the singularity executable on the system path.",
+)
+
+needs_podman = pytest.mark.skipif(
+    not bool(shutil.which("podman")),
+    reason="Requires the podman executable on the system path.",
+)

Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@`
`37`	`37`	`)`
`38`	`38`	`.read()`
`39`	`39`	`.splitlines(),`
`40`		`- tests_require=["pytest<8"],`
	`40`	`+ tests_require=["pytest<8", "pytest-mock"],`
`41`	`41`	`test_suite="tests",`
`42`	`42`	`extras_require={"pretty": ["cwlformat"]},`
`43`	`43`	`entry_points={`