docker: enforce cores ResourceRequirement if requested (#1594)

Co-authored-by: Michael R. Crusoe <[email protected]>
Co-authored-by: george <[email protected]>

Author: 3 people

cwltool/argparser.py

@@ -391,6 +391,16 @@ def arg_parser() -> argparse.ArgumentParser:
         "default of 1 gigabyte to Docker's --memory option.",
     )
 
+    parser.add_argument(
+        "--strict-cpu-limit",
+        action="store_true",
+        help="When running with "
+        "software containers and the Docker engine, pass either the "
+        "calculated cpu allocation from ResourceRequirements or the "
+        "default of 1 core to Docker's --cpu option. "
+        "Requires docker version >= v1.13.",
+    )
+
     parser.add_argument(
         "--timestamps",
         action="store_true",

cwltool/context.py

@@ -151,7 +151,7 @@ def __init__(self, kwargs: Optional[Dict[str, Any]] = None) -> None:
         )  # type: Optional[Callable[[CWLObjectType], Optional[CWLObjectType]]]
         self.on_error = "stop"  # type: str
         self.strict_memory_limit = False  # type: bool
-
+        self.strict_cpu_limit = False  # type: bool
         self.cidfile_dir = None  # type: Optional[str]
         self.cidfile_prefix = None  # type: Optional[str]
 

cwltool/docker.py

@@ -2,6 +2,7 @@
 
 import csv
 import datetime
+import math
 import os
 import re
 import shutil
@@ -434,11 +435,12 @@ def create_runtime(
         for key, value in self.environment.items():
             runtime.append(f"--env={key}={value}")
 
+        res_req, _ = self.builder.get_requirement("ResourceRequirement")
+
         if runtimeContext.strict_memory_limit and not user_space_docker_cmd:
             ram = self.builder.resources["ram"]
             runtime.append("--memory=%dm" % ram)
         elif not user_space_docker_cmd:
-            res_req, _ = self.builder.get_requirement("ResourceRequirement")
             if res_req and ("ramMin" in res_req or "ramMax" in res_req):
                 _logger.warning(
                     "[job %s] Skipping Docker software container '--memory' limit "
@@ -448,5 +450,18 @@ def create_runtime(
                     "assurance.",
                     self.name,
                 )
+        if runtimeContext.strict_cpu_limit and not user_space_docker_cmd:
+            cpus = math.ceil(self.builder.resources["cores"])
+            runtime.append(f"--cpus={cpus}")
+        elif not user_space_docker_cmd:
+            if res_req and ("coresMin" in res_req or "coresMax" in res_req):
+                _logger.warning(
+                    "[job %s] Skipping Docker software container '--cpus' limit "
+                    "despite presence of ResourceRequirement with coresMin "
+                    "and/or coresMax setting. Consider running with "
+                    "--strict-cpu-limit for increased portability "
+                    "assurance.",
+                    self.name,
+                )
 
         return runtime, cidfile_path

tests/test_docker.py

@@ -1,4 +1,5 @@
 """Tests for docker engine."""
+import re
 from pathlib import Path
 from shutil import which
 
@@ -13,7 +14,7 @@ def test_docker_workflow(tmp_path: Path) -> None:
     result_code, _, stderr = get_main_output(
         [
             "--default-container",
-            "debian",
+            "docker.io/debian:stable-slim",
             "--outdir",
             str(tmp_path),
             get_data("tests/wf/hello-workflow.cwl"),
@@ -30,7 +31,7 @@ def test_docker_iwdr() -> None:
     result_code = main(
         [
             "--default-container",
-            "debian",
+            "docker.io/debian:stable-slim",
             get_data("tests/wf/iwdr-entry.cwl"),
             "--message",
             "hello",
@@ -69,3 +70,61 @@ def test_docker_file_mount() -> None:
         [get_data("tests/wf/literalfile.cwl"), get_data("tests/wf/literalfile-job.yml")]
     )
     assert result_code == 0
+
+
+@needs_docker
+def test_docker_strict_cpu_limit() -> None:
+    result_code, stdout, stderr = get_main_output(
+        [
+            "--strict-cpu-limit",
+            "--default-container",
+            "docker.io/debian:stable-slim",
+            get_data("tests/wf/cores_float.cwl"),
+        ]
+    )
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert result_code == 0
+    assert "--cpus=2" in stderr
+
+
+@needs_docker
+def test_docker_strict_memory_limit() -> None:
+    result_code, stdout, stderr = get_main_output(
+        [
+            "--strict-memory-limit",
+            "--default-container",
+            "docker.io/debian:stable-slim",
+            get_data("tests/wf/storage_float.cwl"),
+        ]
+    )
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert result_code == 0
+    assert "--memory=255m" in stderr
+
+
+@needs_docker
+def test_docker_strict_cpu_limit_warning() -> None:
+    result_code, stdout, stderr = get_main_output(
+        [
+            "--default-container",
+            "docker.io/debian:stable-slim",
+            get_data("tests/wf/cores_float.cwl"),
+        ]
+    )
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert result_code == 0
+    assert "Skipping Docker software container '--cpus' limit" in stderr
+
+
+@needs_docker
+def test_docker_strict_memory_limit_warning() -> None:
+    result_code, stdout, stderr = get_main_output(
+        [
+            "--default-container",
+            "docker.io/debian:stable-slim",
+            get_data("tests/wf/storage_float.cwl"),
+        ]
+    )
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert result_code == 0
+    assert "Skipping Docker software container '--memory' limit" in stderr

tests/test_environment.py

@@ -106,7 +106,7 @@ def HOME(v: str, env: Env) -> bool:
             "HOSTNAME": None,
         }
 
-    flags = ["--default-container=debian"]
+    flags = ["--default-container=docker.io/debian:stable-slim"]
     env_accepts_null = True
 
 
@@ -174,7 +174,7 @@ def BIND(v: str, env: Env) -> bool:
 
         return ans
 
-    flags = ["--default-container=debian", "--singularity"]
+    flags = ["--default-container=docker.io/debian:stable-slim", "--singularity"]
     env_accepts_null = True
 
 

tests/test_examples.py

@@ -1345,7 +1345,7 @@ def test_bad_userspace_runtime(factor: str) -> None:
     commands.extend(
         [
             "--user-space-docker-cmd=quaquioN",
-            "--default-container=debian",
+            "--default-container=docker.io/debian:stable-slim",
             get_data(test_file),
             get_data(job_file),
         ]
@@ -1372,7 +1372,14 @@ def test_bad_basecommand(factor: str) -> None:
 def test_bad_basecommand_docker(factor: str) -> None:
     test_file = "tests/wf/missing-tool.cwl"
     commands = factor.split()
-    commands.extend(["--debug", "--default-container", "debian", get_data(test_file)])
+    commands.extend(
+        [
+            "--debug",
+            "--default-container",
+            "docker.io/debian:stable-slim",
+            get_data(test_file),
+        ]
+    )
     error_code, stdout, stderr = get_main_output(commands)
     assert "permanentFail" in stderr, stderr
     assert error_code == 1

tests/test_singularity.py

@@ -46,7 +46,7 @@ def test_singularity_workflow(tmp_path: Path) -> None:
             [
                 "--singularity",
                 "--default-container",
-                "debian",
+                "docker.io/debian:stable-slim",
                 "--debug",
                 get_data("tests/wf/hello-workflow.cwl"),
                 "--usermessage",
@@ -62,7 +62,7 @@ def test_singularity_iwdr() -> None:
         [
             "--singularity",
             "--default-container",
-            "debian",
+            "docker.io/debian:stable-slim",
             get_data("tests/wf/iwdr-entry.cwl"),
             "--message",
             "hello",

tests/wf/cores_float.cwl

@@ -0,0 +1,20 @@
+#!/usr/bin/env cwl-runner
+class: CommandLineTool
+cwlVersion: v1.2
+
+requirements:
+  ResourceRequirement:
+      coresMin: 1.25
+      coresMax: 1.75
+
+inputs: []
+
+outputs:
+  output:
+    type: stdout
+
+baseCommand: echo
+
+stdout: cores.txt
+
+arguments: [ $(runtime.cores) ]

tests/wf/storage_float.cwl

@@ -0,0 +1,23 @@
+#!/usr/bin/env cwl-runner
+class: CommandLineTool
+cwlVersion: v1.2
+
+requirements:
+  ResourceRequirement:
+      ramMin: 254.1
+      ramMax: 254.9
+      tmpdirMin: 255.1
+      tmpdirMax: 255.9
+      outdirMin: 256.1
+      outdirMax: 256.9
+
+inputs: []
+
+outputs:
+  output: stdout
+
+baseCommand: echo
+
+stdout: values.txt
+
+arguments: [ $(runtime.ram), $(runtime.tmpdirSize), $(runtime.outdirSize) ]