Merge pull request #707 from common-workflow-language/fix-force-docker-pull

Restore --force-docker-pull.  Add caching of found docker images.

Author: mr-c

cwltool/docker.py

@@ -11,7 +11,8 @@
 
 import datetime
 import requests
-from typing import (Dict, List, Text, Any, MutableMapping)
+from typing import (Dict, List, Text, Any, MutableMapping, Set)
+import threading
 
 from .docker_id import docker_vm_id
 from .errors import WorkflowException
@@ -22,17 +23,23 @@
 
 _logger = logging.getLogger("cwltool")
 
+found_images = set()  # type: Set[Text]
+found_images_lock = threading.Lock()
 
 class DockerCommandLineJob(ContainerCommandLineJob):
 
     @staticmethod
-    def get_image(dockerRequirement, pull_image, dry_run=False):
-        # type: (Dict[Text, Text], bool, bool) -> bool
+    def get_image(dockerRequirement, pull_image, dry_run=False, force_pull=False):
+        # type: (Dict[Text, Text], bool, bool, bool) -> bool
         found = False
 
         if "dockerImageId" not in dockerRequirement and "dockerPull" in dockerRequirement:
             dockerRequirement["dockerImageId"] = dockerRequirement["dockerPull"]
 
+        with found_images_lock:
+            if dockerRequirement["dockerImageId"] in found_images:
+                return True
+
         for ln in subprocess.check_output(
                 ["docker", "images", "--no-trunc", "--all"]).decode('utf-8').splitlines():
             try:
@@ -58,7 +65,7 @@ def get_image(dockerRequirement, pull_image, dry_run=False):
             except ValueError:
                 pass
 
-        if not found and pull_image:
+        if (force_pull or not found) and pull_image:
             cmd = []  # type: List[Text]
             if "dockerPull" in dockerRequirement:
                 cmd = ["docker", "pull", str(dockerRequirement["dockerPull"])]
@@ -106,10 +113,14 @@ def get_image(dockerRequirement, pull_image, dry_run=False):
                     subprocess.check_call(cmd, stdout=sys.stderr)
                     found = True
 
+        if found:
+            with found_images_lock:
+                found_images.add(dockerRequirement["dockerImageId"])
+
         return found
 
-    def get_from_requirements(self, r, req, pull_image, dry_run=False):
-        # type: (Dict[Text, Text], bool, bool, bool) -> Text
+    def get_from_requirements(self, r, req, pull_image, dry_run=False, force_pull=False):
+        # type: (Dict[Text, Text], bool, bool, bool, bool) -> Text
         if r:
             errmsg = None
             try:
@@ -125,7 +136,7 @@ def get_from_requirements(self, r, req, pull_image, dry_run=False):
                 else:
                     return None
 
-            if self.get_image(r, pull_image, dry_run):
+            if self.get_image(r, pull_image, dry_run, force_pull=force_pull):
                 return r["dockerImageId"]
             else:
                 if req:

cwltool/job.py

@@ -337,8 +337,8 @@ class ContainerCommandLineJob(JobBase):
     __metaclass__ = ABCMeta
 
     @abstractmethod
-    def get_from_requirements(self, r, req, pull_image, dry_run=False):
-        # type: (Dict[Text, Text], bool, bool, bool) -> Text
+    def get_from_requirements(self, r, req, pull_image, dry_run=False, force_pull=False):
+        # type: (Dict[Text, Text], bool, bool, bool, bool) -> Text
         pass
 
     @abstractmethod
@@ -373,7 +373,7 @@ def run(self, pull_image=True, rm_container=True,
             try:
                 env = cast(MutableMapping[Text, Text], os.environ)
                 if docker_req and kwargs.get("use_container"):
-                    img_id = str(self.get_from_requirements(docker_req, True, pull_image))
+                    img_id = str(self.get_from_requirements(docker_req, True, pull_image, force_pull=kwargs.get("force_docker_pull")))
                 if img_id is None:
                     if self.builder.find_default_container:
                         default_container = self.builder.find_default_container()

cwltool/singularity.py

@@ -69,9 +69,13 @@ def get_image(dockerRequirement, pull_image, dry_run=False):
 
         return found
 
-    def get_from_requirements(self, r, req, pull_image, dry_run=False):
-        # type: (Dict[Text, Text], bool, bool, bool) -> Text
+    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)
+
+        if force_pull:
+            _logger.warn("--force-docker-pull currently not supported for singularity")
+
         if r:
             errmsg = None
             try: