Measure Docker container memory usage (#947)

* Deprecated use of --record-container-id, container-ids are now saved by default (temporarily) or in dir specified by --cidfile-dir

Author: mr-c

cwltool/argparser.py

@@ -48,22 +48,23 @@ def arg_parser():  # type: () -> argparse.ArgumentParser
         dest="rm_container")
 
     cidgroup = parser.add_argument_group(
-        "Options for recording the Docker container identifier into a file")
+        "Options for recording the Docker container identifier into a file.")
+    # Disabled as containerid is now saved by default
     cidgroup.add_argument("--record-container-id", action="store_true",
                           default=False,
-                          help="If enabled, store the Docker container ID into a file. "
-                          "See --cidfile-dir to specify the directory.",
+                          help = argparse.SUPPRESS,
                           dest="record_container_id")
 
     cidgroup.add_argument(
-        "--cidfile-dir", type=Text, help="Directory for storing the Docker "
-        "container ID file. The default is the current directory",
+        "--cidfile-dir", type=Text, help="Store the Docker "
+        "container ID into a file in the specifed directory.",
         default=None, dest="cidfile_dir")
 
     cidgroup.add_argument(
         "--cidfile-prefix", type=Text,
         help="Specify a prefix to the container ID filename. "
-        "Final file name will be followed by a timestamp. The default is no prefix.",
+        "Final file name will be followed by a timestamp. "
+        "The default is no prefix.",
         default=None, dest="cidfile_prefix")
 
     parser.add_argument("--tmpdir-prefix", type=Text,

cwltool/context.py

@@ -122,7 +122,6 @@ def __init__(self, kwargs=None):
         self.on_error = "stop"          # type: Text
         self.strict_memory_limit = False  # type: bool
 
-        self.record_container_id = False
         self.cidfile_dir = None
         self.cidfile_prefix = None
 

cwltool/docker.py

@@ -10,7 +10,7 @@
 import tempfile
 import threading
 from io import open  # pylint: disable=redefined-builtin
-from typing import Dict, List, MutableMapping, Optional, Set
+from typing import Dict, List, MutableMapping, Optional, Set, Tuple
 
 import requests
 from typing_extensions import Text  # pylint: disable=unused-import
@@ -200,8 +200,8 @@ def append_volume(runtime, source, target, writable=False):
         # type: (List[Text], Text, Text, bool) -> None
         """Add binding arguments to the runtime list."""
         runtime.append(u"--volume={}:{}:{}".format(
-            docker_windows_path_adjust(source),
-            docker_windows_path_adjust(target), "rw" if writable else "ro"))
+            docker_windows_path_adjust(source), target,
+            "rw" if writable else "ro"))
 
     def add_file_or_directory_volume(self,
                                      runtime,         # type: List[Text]
@@ -215,9 +215,10 @@ def add_file_or_directory_volume(self,
             self.append_volume(runtime, volume.resolved, volume.target)
 
     def add_writable_file_volume(self,
-                                 runtime,         # type: List[Text]
-                                 volume,          # type: MapperEnt
-                                 host_outdir_tgt  # type: Optional[Text]
+                                 runtime,          # type: List[Text]
+                                 volume,           # type: MapperEnt
+                                 host_outdir_tgt,  # type: Optional[Text]
+                                 tmpdir_prefix     # type: Text
                                 ):  # type: (...) -> None
         """Append a writable file mapping to the runtime option list."""
         if self.inplace_update:
@@ -229,7 +230,7 @@ def add_writable_file_volume(self,
                 # which is already going to be mounted
                 shutil.copy(volume.resolved, host_outdir_tgt)
             else:
-                tmpdir = tempfile.mkdtemp(dir=self.tmpdir)
+                tmpdir = tempfile.mkdtemp(dir=tmpdir_prefix)
                 file_copy = os.path.join(
                     tmpdir, os.path.basename(volume.resolved))
                 shutil.copy(volume.resolved, file_copy)
@@ -238,16 +239,17 @@ def add_writable_file_volume(self,
             ensure_writable(host_outdir_tgt or file_copy)
 
     def add_writable_directory_volume(self,
-                                      runtime,         # type: List[Text]
-                                      volume,          # type: MapperEnt
-                                      host_outdir_tgt  # type: Optional[Text]
+                                      runtime,          # type: List[Text]
+                                      volume,           # type: MapperEnt
+                                      host_outdir_tgt,  # type: Optional[Text]
+                                      tmpdir_prefix     # type: Text
                                      ):  # type: (...) -> None
         """Append a writable directory mapping to the runtime option list."""
         if volume.resolved.startswith("_:"):
             # Synthetic directory that needs creating first
             if not host_outdir_tgt:
                 new_dir = os.path.join(
-                    tempfile.mkdtemp(dir=self.tmpdir),
+                    tempfile.mkdtemp(dir=tmpdir_prefix),
                     os.path.basename(volume.target))
                 self.append_volume(runtime, new_dir, volume.target,
                                    writable=True)
@@ -259,7 +261,7 @@ def add_writable_directory_volume(self,
                                    writable=True)
             else:
                 if not host_outdir_tgt:
-                    tmpdir = tempfile.mkdtemp(dir=self.tmpdir)
+                    tmpdir = tempfile.mkdtemp(dir=tmpdir_prefix)
                     new_dir = os.path.join(
                         tmpdir, os.path.basename(volume.resolved))
                     shutil.copytree(volume.resolved, new_dir)
@@ -270,8 +272,10 @@ def add_writable_directory_volume(self,
                     shutil.copytree(volume.resolved, host_outdir_tgt)
                 ensure_writable(host_outdir_tgt or new_dir)
 
-    def create_runtime(self, env, runtimeContext):
-        # type: (MutableMapping[Text, Text], RuntimeContext) -> List
+    def create_runtime(self,
+                       env,            # type: MutableMapping[Text, Text]
+                       runtimeContext  # type: RuntimeContext
+                      ):  # type: (...) -> Tuple[List[Text], Optional[Text]]
         any_path_okay = self.builder.get_requirement("DockerRequirement")[1] \
             or False
         user_space_docker_cmd = runtimeContext.user_space_docker_cmd
@@ -284,19 +288,18 @@ def create_runtime(self, env, runtimeContext):
                 runtime = [user_space_docker_cmd, u"run"]
         else:
             runtime = [u"docker", u"run", u"-i"]
-
-        runtime.append(u"--volume=%s:%s:rw" % (
-            docker_windows_path_adjust(os.path.realpath(self.outdir)),
-            self.builder.outdir))
-        runtime.append(u"--volume=%s:%s:rw" % (
-            docker_windows_path_adjust(os.path.realpath(self.tmpdir)), "/tmp"))
-
+        self.append_volume(runtime, os.path.realpath(self.outdir),
+                           self.builder.outdir, writable=True)
+        self.append_volume(runtime, os.path.realpath(self.tmpdir), "/tmp",
+                           writable=True)
         self.add_volumes(self.pathmapper, runtime, any_path_okay=True,
-                         secret_store=runtimeContext.secret_store)
+                         secret_store=runtimeContext.secret_store,
+                         tmpdir_prefix=runtimeContext.tmpdir_prefix)
         if self.generatemapper is not None:
             self.add_volumes(
                 self.generatemapper, runtime, any_path_okay=any_path_okay,
-                secret_store=runtimeContext.secret_store)
+                secret_store=runtimeContext.secret_store,
+                tmpdir_prefix=runtimeContext.tmpdir_prefix)
 
         if user_space_docker_cmd:
             runtime = [x.replace(":ro", "") for x in runtime]
@@ -338,25 +341,26 @@ def create_runtime(self, env, runtimeContext):
         runtime.append(u"--env=HOME=%s" % self.builder.outdir)
 
         # add parameters to docker to write a container ID file
-        if runtimeContext.record_container_id is True:
+
+        if runtimeContext.cidfile_dir:
             cidfile_dir = runtimeContext.cidfile_dir
-            if cidfile_dir is not None:
-                if not os.path.isdir(cidfile_dir):
-                    _logger.error("--cidfile-dir %s error:\n%s", cidfile_dir,
-                                  cidfile_dir + " is not a directory or "
-                                  "directory doesn't exist, please check it first")
-                    exit(2)
-                if not os.path.exists(cidfile_dir):
-                    _logger.error("--cidfile-dir %s error:\n%s", cidfile_dir,
-                                  "directory doesn't exist, please create it first")
-                    exit(2)
-            else:
-                cidfile_dir = os.getcwd()
-            cidfile_name = datetime.datetime.now().strftime("%Y%m%d%H%M%S-%f") + ".cid"
-            if runtimeContext.cidfile_prefix is not None:
-                cidfile_name = str(runtimeContext.cidfile_prefix + "-" + cidfile_name)
-            cidfile_path = os.path.join(cidfile_dir, cidfile_name)
-            runtime.append(u"--cidfile=%s" % cidfile_path)
+            if not os.path.exists(str(cidfile_dir)):
+                _logger.error("--cidfile-dir %s error:\n%s", cidfile_dir,
+                              "directory doesn't exist, please create it first")
+                exit(2)
+            if not os.path.isdir(cidfile_dir):
+                _logger.error("--cidfile-dir %s error:\n%s", cidfile_dir,
+                              cidfile_dir + " is not a directory, "
+                              "please check it first")
+                exit(2)
+        else:
+            cidfile_dir = os.getcwd()
+
+        cidfile_name = datetime.datetime.now().strftime("%Y%m%d%H%M%S-%f") + ".cid"
+        if runtimeContext.cidfile_prefix is not None:
+            cidfile_name = str(runtimeContext.cidfile_prefix + "-" + cidfile_name)
+        cidfile_path = os.path.join(cidfile_dir, cidfile_name)
+        runtime.append(u"--cidfile=%s" % cidfile_path)
 
         for key, value in self.environment.items():
             runtime.append(u"--env=%s=%s" % (key, value))
@@ -373,4 +377,4 @@ def create_runtime(self, env, runtimeContext):
                     "--strict-memory-limit for increased portability "
                     "assurance.", self.name)
 
-        return runtime
+        return runtime, cidfile_path