From 8a85270629b80b4c4b306a9130293f25dc7a2da6 Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Mon, 16 Jul 2018 09:48:43 -0400
Subject: [PATCH] Fix and docs for advanced container options in job_conf.xml.

These options were added in #2790 but you know how @jmchilton is about documentation. The bug fix is to fix selecting the first valid option if multiple ones are present instead of just the first option which may be invalid given the other configured destination parameters.
---
 config/job_conf.xml.sample_advanced | 28 ++++++++++++++++++++++++++++
 lib/galaxy/tools/deps/containers.py | 24 ++++++++++++++----------
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/config/job_conf.xml.sample_advanced b/config/job_conf.xml.sample_advanced
index 739304f50493..1d2aa2d04662 100644
--- a/config/job_conf.xml.sample_advanced
+++ b/config/job_conf.xml.sample_advanced
@@ -528,6 +528,34 @@
                the deployer. -->
           <!-- <param id="require_container">true</param> -->
         </destination>
+        <destination id="customized_container" runner="local">
+          <!-- The above Docker and Singularity examples describe how to specify
+               default and override containers but fuller descriptions can be used
+               also to tweak extra options. Like in the above examples, "container_override"
+               will override the tool centric container resolution specified by the container
+               resolvers configuration and "containers" will provide a default if no such
+               container is found during resolution.
+
+               resolve_dependencies defaults to False, but can be set to true to use
+               dependency resolution inside the container (you'll likely want to ensure
+               Galaxy's tool dependency directory and/or Conda prefix is mounted in the
+               container if this is set. shell (defaults to /bin/sh) can be used to tell
+               Galaxy to use bash for instance in the target contanier.
+
+               If using these options, docker_enabled and/or singularity_enabled should
+               also be set to true to enable the desired container technology. If multiple
+               such containers are defined (as in the example below), the first one matching
+               the enabled container types for this destination will be used.
+          -->
+          <!-- <param id="container_override">
+              <container type="docker" shell="/bin/sh" resolve_dependencies="false">busybox:ubuntu-14.04</container>
+              <container type="singularity" shell="/bin/sh" resolve_dependencies="false">/path/to/default/container</container>
+          </param> -->
+          <!-- <param id="container">
+              <container type="docker" shell="/bin/sh" resolve_dependencies="false">busybox:ubuntu-14.04</container>
+              <container type="singularity" shell="/bin/sh" resolve_dependencies="false">/path/to/default/container</container>
+          </param> -->
+        </destination>
         <destination id="pbs" runner="pbs" tags="mycluster"/>
         <destination id="pbs_longjobs" runner="pbs" tags="mycluster,longjobs">
             <!-- Define parameters that are native to the job runner plugin. -->
diff --git a/lib/galaxy/tools/deps/containers.py b/lib/galaxy/tools/deps/containers.py
index 0edfad66ae78..d51bde357555 100644
--- a/lib/galaxy/tools/deps/containers.py
+++ b/lib/galaxy/tools/deps/containers.py
@@ -106,12 +106,18 @@ def __destination_container(container_description=None, container_id=None, conta
             )
             return container
 
+        def container_from_description_from_dicts(destination_container_dicts):
+            for destination_container_dict in destination_container_dicts:
+                container_description = ContainerDescription.from_dict(destination_container_dict)
+                if container_description:
+                    container = __destination_container(container_description)
+                    if container:
+                        return container
+
         if "container_override" in destination_info:
-            container_description = ContainerDescription.from_dict(destination_info["container_override"][0])
-            if container_description:
-                container = __destination_container(container_description)
-                if container:
-                    return container
+            container = container_from_description_from_dicts(destination_info["container_override"])
+            if container:
+                return container
 
         # If destination forcing Galaxy to use a particular container do it,
         # this is likely kind of a corner case. For instance if deployers
@@ -132,11 +138,9 @@ def __destination_container(container_description=None, container_id=None, conta
         # If we still don't have a container, check to see if any container
         # types define a default container id and use that.
         if "container" in destination_info:
-            container_description = ContainerDescription.from_dict(destination_info["container"][0])
-            if container_description:
-                container = __destination_container(container_description)
-                if container:
-                    return container
+            container = container_from_description_from_dicts(destination_info["container"])
+            if container:
+                return container
 
         for container_type in CONTAINER_CLASSES.keys():
             container_id = self.__default_container_id(container_type, destination_info)