Improve default handling of Singularity volumes.

My understanding is that unlike Docker, Singularity doesn't allow a subdirectory to be mounted rw and its parent to be mounted ro - it will just be ro. This PR adjusts the default volume handling for Singularity to intelligently fallback to mounting parent directories as rw if their child directories are listed as being mounted rw.

Ping @bgruening - is the above a description correct and does this approach work for you?

config/job_conf.xml.sample_advanced

@@ -443,7 +443,14 @@
         <destination id="singularity_local" runner="local">
           <param id="singularity_enabled">true</param>
           <!-- See the above documentation for docker_volumes, singularity_volumes works
-               the same way.
+               almost the same way. The only difference is that $default will expand with
+               rw directories that in Docker would expand as ro if any of subdirectories are rw.
+
+               As an example consider that Docker mounts the parent of the working directory
+               (this is known as the job directory) as ro and the working directory itself as rw.
+               This doesn't work in Singularity because if any parent directory is mounted as ro
+               none of its children will be rw. So the job directory will be mounted rw for
+               Singularity.
           -->
           <!--
           <param id="singularity_volumes">$defaults,/mnt/galaxyData/libraries:ro,/mnt/galaxyData/indices:ro</param>

lib/galaxy/tools/deps/containers.py

@@ -11,6 +11,7 @@
 
 from galaxy.util import asbool
 from galaxy.util import plugin_config
+from galaxy.util import in_directory
 
 from .container_resolvers.explicit import ExplicitContainerResolver
 from .container_resolvers.mulled import (
@@ -317,6 +318,53 @@ def containerize_command(self, command):
         """
 
 
+def preprocess_volumes(volumes_raw_str, container_type):
+    """Process Galaxy volume specification string to either Docker or Singularity specification.
+
+    Galaxy allows the mount try "default_ro" which translates to ro for Docker and
+    ro for Singularity iff no subdirectories are rw (Singularity does not allow ro
+    parent directories with rw subdirectories).
+
+    >>> preprocess_volumes("/a/b", DOCKER_CONTAINER_TYPE)
+    '/a/b:rw'
+    >>> preprocess_volumes("/a/b:ro,/a/b/c:rw", DOCKER_CONTAINER_TYPE)
+    '/a/b:ro,/a/b/c:rw'
+    >>> preprocess_volumes("/a/b:default_ro,/a/b/c:rw", DOCKER_CONTAINER_TYPE)
+    '/a/b:ro,/a/b/c:rw'
+    >>> preprocess_volumes("/a/b:default_ro,/a/b/c:rw", SINGULARITY_CONTAINER_TYPE)
+    '/a/b:rw,/a/b/c:rw'
+    """
+
+    volumes_raw_strs = [v.strip() for v in volumes_raw_str.split(",")]
+    volumes = []
+    rw_paths = []
+
+    for volume_raw_str in volumes_raw_strs:
+        volume_parts = volume_raw_str.split(":")
+        if len(volume_parts) > 2:
+            raise Exception("Unparsable volumes string in configuration [%s]" % volumes_raw_str)
+        if len(volume_parts) == 1:
+            volume_parts.append("rw")
+        volumes.append(volume_parts)
+        if volume_parts[1] == "rw":
+            rw_paths.append(volume_parts[0])
+
+    for volume in volumes:
+        path = volume[0]
+        how = volume[1]
+
+        if how == "default_ro":
+            how = "ro"
+            if container_type == SINGULARITY_CONTAINER_TYPE:
+                for rw_path in rw_paths:
+                    if in_directory(rw_path, path):
+                        how = "rw"
+
+        volume[1] = how
+
+    return ",".join([":".join(v) for v in volumes])
+
+
 class HasDockerLikeVolumes:
     """Mixin to share functionality related to Docker volume handling.
 
@@ -344,20 +392,20 @@ def add_var(name, value):
         if self.job_info.job_directory and self.job_info.job_directory_type == "pulsar":
             # We have a Pulsar job directory, so everything needed (excluding index
             # files) should be available in job_directory...
-            defaults = "$job_directory:ro,$tool_directory:ro,$job_directory/outputs:rw,$working_directory:rw"
+            defaults = "$job_directory:default_ro,$tool_directory:default_ro,$job_directory/outputs:rw,$working_directory:rw"
         else:
-            defaults = "$galaxy_root:ro,$tool_directory:ro"
+            defaults = "$galaxy_root:default_ro,$tool_directory:default_ro"
             if self.job_info.job_directory:
-                defaults += ",$job_directory:ro"
+                defaults += ",$job_directory:default_ro"
             if self.app_info.outputs_to_working_directory:
-                # Should need default_file_path (which is a course estimate given
+                # Should need default_file_path (which is of course an estimate given
                 # object stores anyway).
-                defaults += ",$working_directory:rw,$default_file_path:ro"
+                defaults += ",$working_directory:rw,$default_file_path:default_ro"
             else:
                 defaults += ",$working_directory:rw,$default_file_path:rw"
 
         if self.app_info.library_import_dir:
-            defaults += ",$library_import_dir:ro"
+            defaults += ",$library_import_dir:default_ro"
 
         # Define $defaults that can easily be extended with external library and
         # index data without deployer worrying about above details.
@@ -368,6 +416,8 @@ def add_var(name, value):
 
 class DockerContainer(Container, HasDockerLikeVolumes):
 
+    container_type = DOCKER_CONTAINER_TYPE
+
     def containerize_command(self, command):
         def prop(name, default):
             destination_name = "docker_%s" % name
@@ -390,8 +440,9 @@ def prop(name, default):
             raise Exception("Cannot containerize command [%s] without defined working directory." % working_directory)
 
         volumes_raw = self._expand_volume_str(self.destination_info.get("docker_volumes", "$defaults"))
+        preprocessed_volumes_str = preprocess_volumes(volumes_raw, self.container_type)
         # TODO: Remove redundant volumes...
-        volumes = docker_util.DockerVolume.volumes_from_str(volumes_raw)
+        volumes = docker_util.DockerVolume.volumes_from_str(preprocessed_volumes_str)
         volumes_from = self.destination_info.get("docker_volumes_from", docker_util.DEFAULT_VOLUMES_FROM)
 
         docker_host_props = dict(
@@ -454,6 +505,8 @@ def docker_cache_path(cache_directory, container_id):
 
 class SingularityContainer(Container, HasDockerLikeVolumes):
 
+    container_type = SINGULARITY_CONTAINER_TYPE
+
     def containerize_command(self, command):
         def prop(name, default):
             destination_name = "singularity_%s" % name

lib/galaxy/tools/deps/docker_util.py

@@ -28,7 +28,7 @@ def __init__(self, path, to_path=None, how=DEFAULT_VOLUME_MOUNT_TYPE):
         self.from_path = path
         self.to_path = to_path or path
         if not DockerVolume.__valid_how(how):
-            raise ValueError("Invalid way to specify docker volume %s" % how)
+            raise ValueError("Invalid way to specify Docker volume %s" % how)
         self.how = how
 
     @staticmethod
@@ -41,7 +41,7 @@ def volumes_from_str(volumes_as_str):
     @staticmethod
     def volume_from_str(as_str):
         if not as_str:
-            raise ValueError("Failed to parse docker volume from %s" % as_str)
+            raise ValueError("Failed to parse Docker volume from %s" % as_str)
         parts = as_str.split(":", 2)
         kwds = dict(path=parts[0])
         if len(parts) == 2: