Merge pull request #715 from common-workflow-language/singularity_tighten

Tighten our use of Singularity

Author: mr-c

cwltool/resolver.py

@@ -1,29 +1,38 @@
 from __future__ import absolute_import
 import logging
 import os
+import sys
 
+if sys.version_info < (3, 4):
+    from pathlib2 import Path
+else:
+    from pathlib import Path
 from six.moves import urllib
 
-from schema_salad.ref_resolver import file_uri
 
 _logger = logging.getLogger("cwltool")
 
 
 def resolve_local(document_loader, uri):
-    if uri.startswith("/"):
-        return None
-    shares = [os.environ.get("XDG_DATA_HOME", os.path.join(os.path.expanduser('~'), ".local", "share"))]
-    shares.extend(os.environ.get("XDG_DATA_DIRS", "/usr/local/share/:/usr/share/").split(":"))
-    shares = [os.path.join(s, "commonwl", uri) for s in shares]
-    shares.insert(0, os.path.join(os.getcwd(), uri))
+    if uri.startswith("/") and os.path.exists(uri):
+        return Path(uri).as_uri()
+    if os.path.exists(urllib.parse.urlparse(
+            urllib.parse.urldefrag(
+                "{}/{}".format(Path.cwd().as_uri(), uri))[0])[2]):
+        return "{}/{}".format(Path.cwd().as_uri(), uri)
+    sharepaths = [os.environ.get("XDG_DATA_HOME", os.path.join(
+        os.path.expanduser('~'), ".local", "share"))]
+    sharepaths.extend(os.environ.get(
+        "XDG_DATA_DIRS", "/usr/local/share/:/usr/share/").split(":"))
+    shares = [os.path.join(s, "commonwl", uri) for s in sharepaths]
 
     _logger.debug("Search path is %s", shares)
 
-    for s in shares:
-        if os.path.exists(s):
-            return file_uri(s)
-        if os.path.exists("%s.cwl" % s):
-            return file_uri(s)
+    for path in shares:
+        if os.path.exists(path):
+            return Path(uri).as_uri()
+        if os.path.exists("{}.cwl".format(path)):
+            return Path("{}.cwl".format(path)).as_uri()
     return None
 
 
@@ -32,7 +41,6 @@ def tool_resolver(document_loader, uri):
         ret = r(document_loader, uri)
         if ret is not None:
             return ret
-    return file_uri(os.path.abspath(uri), split_frag=True)
 
 
 ga4gh_tool_registries = ["https://dockstore.org:8443"]

cwltool/singularity.py

@@ -8,7 +8,7 @@
 import sys
 from io import open
 
-from typing import (Dict, List, Text, MutableMapping, Any)
+from typing import (Dict, List, Text, Optional, MutableMapping, Any)
 
 from .errors import WorkflowException
 from .job import ContainerCommandLineJob
@@ -21,34 +21,52 @@
 
 class SingularityCommandLineJob(ContainerCommandLineJob):
     @staticmethod
-    def get_image(dockerRequirement, pull_image, dry_run=False):
-        # type: (Dict[Text, Text], bool, bool) -> bool
+    def get_image(dockerRequirement,  # type: Dict[Text, Text]
+                  pull_image,         # type: bool
+                  dry_run=False       # type: bool
+                 ):
+        # type: (...) -> bool
+        """
+        Acquire the software container image in the specified dockerRequirement
+        using Singularity and returns the success as a bool. Updates the
+        provided dockerRequirement with the specific dockerImageId to the full
+        path of the local image, if found. Likewise the
+        dockerRequirement['dockerPull'] is updated to a docker:// URI if needed.
+        """
         found = False
 
         if "dockerImageId" not in dockerRequirement and "dockerPull" in dockerRequirement:
             match = re.search(pattern=r'([a-z]*://)', string=dockerRequirement["dockerPull"])
             if match:
                 dockerRequirement["dockerImageId"] = re.sub(pattern=r'([a-z]*://)', repl=r'',
                                                             string=dockerRequirement["dockerPull"])
-                dockerRequirement["dockerImageId"] = re.sub(pattern=r'[:/]', repl=r'-',
-                                                            string=dockerRequirement["dockerImageId"]) + ".img"
+                dockerRequirement["dockerImageId"] = re.sub(
+                    pattern=r'[:/]', repl=r'-', string=dockerRequirement["dockerImageId"]) + ".img"
             else:
-                dockerRequirement["dockerImageId"] = re.sub(pattern=r'[:/]', repl=r'-',
-                                                            string=dockerRequirement["dockerPull"]) + ".img"
+                dockerRequirement["dockerImageId"] = re.sub(
+                    pattern=r'[:/]', repl=r'-', string=dockerRequirement["dockerPull"]) + ".img"
                 dockerRequirement["dockerPull"] = "docker://" + dockerRequirement["dockerPull"]
 
         # check if Singularity image is available in $SINGULARITY_CACHEDIR
-        if "SINGULARITY_CACHEDIR" in os.environ \
-        and os.path.isfile(os.path.join(os.environ["SINGULARITY_CACHEDIR"], dockerRequirement["dockerImageId"])):
-            _logger.info("Using local copy of Singularity image found in $SINGULARITY_CACHEDIR")
-            dockerRequirement["dockerImageId"] = os.path.join(os.environ["SINGULARITY_CACHEDIR"], dockerRequirement["dockerImageId"])
+        if "SINGULARITY_CACHEDIR" in os.environ and os.path.isfile(
+                os.path.join(os.environ["SINGULARITY_CACHEDIR"],
+                             dockerRequirement["dockerImageId"])):
+            _logger.info("Using local copy of Singularity image found in "
+                         "$SINGULARITY_CACHEDIR")
+            dockerRequirement["dockerImageId"] = os.path.join(
+                os.environ["SINGULARITY_CACHEDIR"],
+                dockerRequirement["dockerImageId"])
             found = True
 
         # check if Singularity image is available in $SINGULARITY_PULLFOLDER
-        elif "SINGULARITY_PULLFOLDER" in os.environ \
-        and os.path.isfile(os.path.join(os.environ["SINGULARITY_PULLFOLDER"], dockerRequirement["dockerImageId"])):
-            _logger.info("Using local copy of Singularity image found in $SINGULARITY_PULLFOLDER")
-            dockerRequirement["dockerImageId"] = os.path.join(os.environ["SINGULARITY_PULLFOLDER"], dockerRequirement["dockerImageId"])
+        elif "SINGULARITY_PULLFOLDER" in os.environ and os.path.isfile(
+                os.path.join(os.environ["SINGULARITY_PULLFOLDER"],
+                             dockerRequirement["dockerImageId"])):
+            _logger.info("Using local copy of Singularity image found in "
+                         "$SINGULARITY_PULLFOLDER")
+            dockerRequirement["dockerImageId"] = os.path.join(
+                os.environ["SINGULARITY_PULLFOLDER"],
+                dockerRequirement["dockerImageId"])
             found = True
 
         # check if Singularity image is available in current working directory
@@ -69,21 +87,31 @@ def get_image(dockerRequirement, pull_image, dry_run=False):
 
         return found
 
-    def get_from_requirements(self, r, req, pull_image, dry_run=False, force_pull=False):
-        # type: (Dict[Text, Text], bool, bool, bool, bool) -> Text
-        # returns the filename of the Singularity image (e.g. hello-world-latest.img)
+    def get_from_requirements(self,
+                              r,                # type: Optional[Dict[Text, Text]]
+                              req,              # type: bool
+                              pull_image,       # type: bool
+                              dry_run=False,    # type: bool
+                              force_pull=False  # type: bool
+                             ):
+        # type: (...) -> Text
+        """
+        Returns the filename of the Singularity image (e.g.
+        hello-world-latest.img).
+        """
 
         if force_pull:
-            _logger.warn("--force-docker-pull currently not supported for singularity")
+            _logger.warning("--force-docker-pull currently not supported for "
+                            "singularity")
 
         if r:
             errmsg = None
             try:
                 subprocess.check_output(["singularity", "--version"])
-            except subprocess.CalledProcessError as e:
-                errmsg = "Cannot execute 'singularity --version' " + Text(e)
-            except OSError as e:
-                errmsg = "'singularity' executable not found: " + Text(e)
+            except subprocess.CalledProcessError as err:
+                errmsg = "Cannot execute 'singularity --version' {}".format(err)
+            except OSError as err:
+                errmsg = "'singularity' executable not found: {}".format(err)
 
             if errmsg:
                 if req:
@@ -95,7 +123,8 @@ def get_from_requirements(self, r, req, pull_image, dry_run=False, force_pull=Fa
                 return os.path.abspath(r["dockerImageId"])
             else:
                 if req:
-                    raise WorkflowException(u"Container image %s not found" % r["dockerImageId"])
+                    raise WorkflowException(u"Container image {} not "
+                                            "found".format(r["dockerImageId"]))
 
         return None
 
@@ -104,10 +133,10 @@ def add_volumes(self, pathmapper, runtime, stage_output):
 
         host_outdir = self.outdir
         container_outdir = self.builder.outdir
-        for src, vol in pathmapper.items():
+        for _, vol in pathmapper.items():
             if not vol.staged:
                 continue
-            if stage_output:
+            if stage_output and not vol.target.startswith(container_outdir):
                 containertgt = container_outdir + vol.target[len(host_outdir):]
             else:
                 containertgt = vol.target
@@ -119,13 +148,15 @@ def add_volumes(self, pathmapper, runtime, stage_output):
             if vol.type in ("File", "Directory"):
                 if not vol.resolved.startswith("_:"):
                     runtime.append(u"--bind")
-                    runtime.append("%s:%s:ro" % (
-                    docker_windows_path_adjust(vol.resolved), docker_windows_path_adjust(containertgt)))
+                    runtime.append("{}:{}:ro".format(
+                        docker_windows_path_adjust(vol.resolved),
+                        docker_windows_path_adjust(containertgt)))
             elif vol.type == "WritableFile":
                 if self.inplace_update:
                     runtime.append(u"--bind")
-                    runtime.append("%s:%s:rw" % (
-                    docker_windows_path_adjust(vol.resolved), docker_windows_path_adjust(containertgt)))
+                    runtime.append("{}:{}:rw".format(
+                        docker_windows_path_adjust(vol.resolved),
+                        docker_windows_path_adjust(containertgt)))
                 else:
                     shutil.copy(vol.resolved, host_outdir_tgt)
                     ensure_writable(host_outdir_tgt)
@@ -135,28 +166,40 @@ def add_volumes(self, pathmapper, runtime, stage_output):
                 else:
                     if self.inplace_update:
                         runtime.append(u"--bind")
-                        runtime.append("%s:%s:rw" % (
-                        docker_windows_path_adjust(vol.resolved), docker_windows_path_adjust(containertgt)))
+                        runtime.append("{}:{}:rw".format(
+                            docker_windows_path_adjust(vol.resolved),
+                            docker_windows_path_adjust(containertgt)))
                     else:
                         shutil.copytree(vol.resolved, vol.target)
             elif vol.type == "CreateFile":
                 createtmp = os.path.join(host_outdir, os.path.basename(vol.target))
-                with open(createtmp, "wb") as f:
-                    f.write(vol.resolved.encode("utf-8"))
+                with open(createtmp, "wb") as tmp:
+                    tmp.write(vol.resolved.encode("utf-8"))
                 runtime.append(u"--bind")
-                runtime.append(
-                    "%s:%s:ro" % (docker_windows_path_adjust(createtmp), docker_windows_path_adjust(vol.target)))
-
-    def create_runtime(self, env, rm_container=True, record_container_id=False, cidfile_dir="",
-                       cidfile_prefix="", **kwargs):
-        # type: (MutableMapping[Text, Text], bool, bool, Text, Text, **Any) -> List
-
-        runtime = [u"singularity", u"--quiet", u"exec"]
+                runtime.append("{}:{}:ro".format(
+                    docker_windows_path_adjust(createtmp),
+                    docker_windows_path_adjust(vol.target)))
+
+    def create_runtime(self,
+                       env,                        # type: MutableMapping[Text, Text]
+                       rm_container=True,          # type: bool
+                       record_container_id=False,  # type: bool
+                       cidfile_dir="",             # type: Text
+                       cidfile_prefix="",          # type: Text
+                       **kwargs
+                      ):
+        # type: (...) -> List
+        """ Returns the Singularity runtime list of commands and options."""
+
+        runtime = [u"singularity", u"--quiet", u"exec", u"--contain", u"--pid",
+                   u"--ipc"]  # , u"--userns"]
         runtime.append(u"--bind")
-        runtime.append(
-            u"%s:%s:rw" % (docker_windows_path_adjust(os.path.realpath(self.outdir)), self.builder.outdir))
+        runtime.append(u"{}:{}:rw".format(
+            docker_windows_path_adjust(os.path.realpath(self.outdir)),
+            self.builder.outdir))
         runtime.append(u"--bind")
-        runtime.append(u"%s:%s:rw" % (docker_windows_path_adjust(os.path.realpath(self.tmpdir)), "/tmp"))
+        runtime.append(u"{}:{}:rw".format(
+            docker_windows_path_adjust(os.path.realpath(self.tmpdir)), "/tmp"))
 
         self.add_volumes(self.pathmapper, runtime, stage_output=False)
         if self.generatemapper:
@@ -167,11 +210,13 @@ def create_runtime(self, env, rm_container=True, record_container_id=False, cidf
 
         if kwargs.get("custom_net", None) is not None:
             raise UnsupportedRequirement(
-                "Singularity implementation does not support networking")
+                "Singularity implementation does not support custom networking")
+        elif kwargs.get("disable_net", None):
+            runtime.append(u"--net")
 
         env["SINGULARITYENV_TMPDIR"] = "/tmp"
         env["SINGULARITYENV_HOME"] = self.builder.outdir
 
-        for t, v in self.environment.items():
-            env["SINGULARITYENV_" + t] = v
+        for name, value in self.environment.items():
+            env["SINGULARITYENV_{}".format(name)] = value
         return runtime

requirements.txt

@@ -5,3 +5,4 @@ rdflib-jsonld==0.4.0
 shellescape==3.4.1
 schema-salad>=2.6,<3
 typing==3.5.3
+pathlib2; python_version<"3"

setup.py

@@ -58,6 +58,7 @@
           'six >= 1.8.0',
       ],
       extras_require={
+          ':python_version<"3"': [ 'pathlib2' ],
           'deps': ["galaxy-lib >= 17.09.3"]
       },
       python_requires='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, <4',

tox.ini

@@ -48,7 +48,9 @@ deps =
 [testenv:py27-pipconflictchecker]
 commands = pipconflictchecker
 whitelist_externals = pipconflictchecker
-deps = pip-conflict-checker
+deps =
+  pip-conflict-checker
+  pip==9.0.3
 
 [testenv:py27-lint-readme]
 commands = python setup.py check -r -s