17390: Add unit tests for storage classes
authorPeter Amstutz <peter.amstutz@curii.com>
Thu, 12 Aug 2021 19:24:14 +0000 (15:24 -0400)
committerPeter Amstutz <peter.amstutz@curii.com>
Fri, 13 Aug 2021 14:34:35 +0000 (10:34 -0400)
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>

sdk/cwl/arvados_cwl/executor.py
sdk/cwl/tests/test_container.py
sdk/cwl/tests/test_submit.py
sdk/cwl/tests/wf/submit_storage_class_wf.cwl [new file with mode: 0644]

index 6d4f1bd52e11b7eed8fbc9978eaadd7f3422f45f..edb9d5b523c09bee4aa43f16705e27f2f15194d9 100644 (file)
@@ -42,7 +42,7 @@ from .context import ArvLoadingContext, ArvRuntimeContext
 from ._version import __version__
 
 from cwltool.process import shortname, UnsupportedRequirement, use_custom_schema
-from cwltool.utils import adjustFileObjs, adjustDirObjs, get_listing, visit_class
+from cwltool.utils import adjustFileObjs, adjustDirObjs, get_listing, visit_class, aslist
 from cwltool.command_line_tool import compute_checksums
 from cwltool.load_tool import load_tool
 
@@ -549,7 +549,7 @@ The 'jobs' API is no longer supported.
         if runtimeContext.submit_request_uuid and self.work_api != "containers":
             raise Exception("--submit-request-uuid requires containers API, but using '{}' api".format(self.work_api))
 
-        default_storage_classes = [k for k,v in self.api.config()["StorageClasses"].items() if v.get("Default") is True].join(",")
+        default_storage_classes = ",".join([k for k,v in self.api.config()["StorageClasses"].items() if v.get("Default") is True])
         if runtimeContext.storage_classes == "default":
             runtimeContext.storage_classes = default_storage_classes
         if runtimeContext.intermediate_storage_classes == "default":
index 09983f87a2cea8f46b8790e6749c05a5bc57d8f8..2b46b89c604cafc0f91ee3966ecd1a3c068dbad7 100644 (file)
@@ -163,7 +163,8 @@ class TestContainer(unittest.TestCase):
                         'cwd': '/var/spool/cwl',
                         'scheduling_parameters': {},
                         'properties': {},
-                        'secret_mounts': {}
+                        'secret_mounts': {},
+                        'output_storage_classes': ["default"]
                     }))
 
     # The test passes some fields in builder.resources
@@ -250,7 +251,8 @@ class TestContainer(unittest.TestCase):
                 'partitions': ['blurb']
             },
             'properties': {},
-            'secret_mounts': {}
+            'secret_mounts': {},
+            'output_storage_classes': ["default"]
         }
 
         call_body = call_kwargs.get('body', None)
@@ -379,7 +381,8 @@ class TestContainer(unittest.TestCase):
             'scheduling_parameters': {
             },
             'properties': {},
-            'secret_mounts': {}
+            'secret_mounts': {},
+            'output_storage_classes': ["default"]
         }
 
         call_body = call_kwargs.get('body', None)
@@ -463,7 +466,8 @@ class TestContainer(unittest.TestCase):
                     'cwd': '/var/spool/cwl',
                     'scheduling_parameters': {},
                     'properties': {},
-                    'secret_mounts': {}
+                    'secret_mounts': {},
+                    'output_storage_classes': ["default"]
                 }))
 
     @mock.patch("arvados.collection.Collection")
@@ -696,7 +700,8 @@ class TestContainer(unittest.TestCase):
                     'cwd': '/var/spool/cwl',
                     'scheduling_parameters': {},
                     'properties': {},
-                    'secret_mounts': {}
+                    'secret_mounts': {},
+                    'output_storage_classes': ["default"]
                 }))
 
     # The test passes no builder.resources
@@ -791,7 +796,8 @@ class TestContainer(unittest.TestCase):
                             "content": "username: user\npassword: blorp\n",
                             "kind": "text"
                         }
-                    }
+                    },
+                    'output_storage_classes': ["default"]
                 }))
 
     # The test passes no builder.resources
@@ -835,6 +841,79 @@ class TestContainer(unittest.TestCase):
         self.assertEqual(42, kwargs['body']['scheduling_parameters'].get('max_run_time'))
 
 
+    # The test passes no builder.resources
+    # Hence the default resources will apply: {'cores': 1, 'ram': 1024, 'outdirSize': 1024, 'tmpdirSize': 1024}
+    @mock.patch("arvados.commands.keepdocker.list_images_in_arv")
+    def test_setting_storage_class(self, keepdocker):
+        arv_docker_clear_cache()
+
+        runner = mock.MagicMock()
+        runner.ignore_docker_for_reuse = False
+        runner.intermediate_output_ttl = 0
+        runner.secret_store = cwltool.secrets.SecretStore()
+
+        keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")]
+        runner.api.collections().get().execute.return_value = {
+            "portable_data_hash": "99999999999999999999999999999993+99"}
+
+        tool = cmap({
+            "inputs": [],
+            "outputs": [],
+            "baseCommand": "ls",
+            "arguments": [{"valueFrom": "$(runtime.outdir)"}],
+            "id": "#",
+            "class": "CommandLineTool",
+            "hints": [
+                {
+                    "class": "http://arvados.org/cwl#OutputStorageClass",
+                    "finalStorageClass": ["baz_sc", "qux_sc"],
+                    "intermediateStorageClass": ["foo_sc", "bar_sc"]
+                }
+            ]
+        })
+
+        loadingContext, runtimeContext = self.helper(runner, True)
+
+        arvtool = arvados_cwl.ArvadosCommandTool(runner, tool, loadingContext)
+        arvtool.formatgraph = None
+
+        for j in arvtool.job({}, mock.MagicMock(), runtimeContext):
+            j.run(runtimeContext)
+            runner.api.container_requests().create.assert_called_with(
+                body=JsonDiffMatcher({
+                    'environment': {
+                        'HOME': '/var/spool/cwl',
+                        'TMPDIR': '/tmp'
+                    },
+                    'name': 'test_run_True',
+                    'runtime_constraints': {
+                        'vcpus': 1,
+                        'ram': 1073741824
+                    },
+                    'use_existing': True,
+                    'priority': 500,
+                    'mounts': {
+                        '/tmp': {'kind': 'tmp',
+                                 "capacity": 1073741824
+                             },
+                        '/var/spool/cwl': {'kind': 'tmp',
+                                           "capacity": 1073741824 }
+                    },
+                    'state': 'Committed',
+                    'output_name': 'Output for step test_run_True',
+                    'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz',
+                    'output_path': '/var/spool/cwl',
+                    'output_ttl': 0,
+                    'container_image': '99999999999999999999999999999993+99',
+                    'command': ['ls', '/var/spool/cwl'],
+                    'cwd': '/var/spool/cwl',
+                    'scheduling_parameters': {},
+                    'properties': {},
+                    'secret_mounts': {},
+                    'output_storage_classes': ["foo_sc", "bar_sc"]
+                }))
+
+
 class TestWorkflow(unittest.TestCase):
     def setUp(self):
         cwltool.process._names = set()
@@ -972,7 +1051,8 @@ class TestWorkflow(unittest.TestCase):
                 "scheduling_parameters": {},
                 "secret_mounts": {},
                 "state": "Committed",
-                "use_existing": True
+                "use_existing": True,
+                'output_storage_classes': ["default"]
             }))
         mockc.open().__enter__().write.assert_has_calls([mock.call(subwf)])
         mockc.open().__enter__().write.assert_has_calls([mock.call(
@@ -1074,7 +1154,8 @@ class TestWorkflow(unittest.TestCase):
                 ],
                 'use_existing': True,
                 'output_name': u'Output for step echo-subwf',
-                'cwd': '/var/spool/cwl'
+                'cwd': '/var/spool/cwl',
+                'output_storage_classes': ["default"]
             }))
 
     def test_default_work_api(self):
index 6cf59f25eed87cf6cc0c1f948d18d3234081b5f7..1b646a8e4459dfe5677fb18ca997a393248b6e8f 100644 (file)
@@ -95,6 +95,11 @@ def stubs(func):
         stubs.api.containers().current().execute.return_value = {
             "uuid": stubs.fake_container_uuid,
         }
+        stubs.api.config()["StorageClasses"].items.return_value = {
+            "default": {
+                "Default": True
+            }
+        }.items()
 
         class CollectionExecute(object):
             def __init__(self, exe):
@@ -342,14 +347,6 @@ class TestSubmit(unittest.TestCase):
         cwltool.process._names = set()
         arvados_cwl.arvdocker.arv_docker_clear_cache()
 
-    @stubs
-    def test_error_when_multiple_storage_classes_specified(self, stubs):
-        storage_classes = "foo,bar"
-        exited = arvados_cwl.main(
-                ["--debug", "--storage-classes", storage_classes,
-                 "tests/wf/submit_wf.cwl", "tests/submit_test_job.json"],
-                sys.stdin, sys.stderr, api_client=stubs.api)
-        self.assertEqual(exited, 1)
 
     @mock.patch("time.sleep")
     @stubs
@@ -526,6 +523,27 @@ class TestSubmit(unittest.TestCase):
                          stubs.expect_container_request_uuid + '\n')
         self.assertEqual(exited, 0)
 
+    @stubs
+    def test_submit_multiple_storage_classes(self, stubs):
+        exited = arvados_cwl.main(
+            ["--debug", "--submit", "--no-wait", "--api=containers", "--storage-classes=foo,bar", "--intermediate-storage-classes=baz",
+                "tests/wf/submit_wf.cwl", "tests/submit_test_job.json"],
+            stubs.capture_stdout, sys.stderr, api_client=stubs.api, keep_client=stubs.keep_client)
+
+        expect_container = copy.deepcopy(stubs.expect_container_spec)
+        expect_container["command"] = ['arvados-cwl-runner', '--local', '--api=containers',
+                                       '--no-log-timestamps', '--disable-validate', '--disable-color',
+                                       '--eval-timeout=20', '--thread-count=0',
+                                       '--enable-reuse', "--collection-cache-size=256", "--debug",
+                                       "--storage-classes=foo,bar", "--intermediate-storage-classes=baz", '--on-error=continue',
+                                       '/var/lib/cwl/workflow.json#main', '/var/lib/cwl/cwl.input.json']
+
+        stubs.api.container_requests().create.assert_called_with(
+            body=JsonDiffMatcher(expect_container))
+        self.assertEqual(stubs.capture_stdout.getvalue(),
+                         stubs.expect_container_request_uuid + '\n')
+        self.assertEqual(exited, 0)
+
     @mock.patch("cwltool.task_queue.TaskQueue")
     @mock.patch("arvados_cwl.arvworkflow.ArvadosWorkflow.job")
     @mock.patch("arvados_cwl.executor.ArvCwlExecutor.make_output_collection")
@@ -568,6 +586,27 @@ class TestSubmit(unittest.TestCase):
         make_output.assert_called_with(u'Output of submit_wf.cwl', ['default'], '', 'zzzzz-4zz18-zzzzzzzzzzzzzzzz')
         self.assertEqual(exited, 0)
 
+    @mock.patch("cwltool.task_queue.TaskQueue")
+    @mock.patch("arvados_cwl.arvworkflow.ArvadosWorkflow.job")
+    @mock.patch("arvados_cwl.executor.ArvCwlExecutor.make_output_collection")
+    @stubs
+    def test_storage_class_hint_to_make_output_collection(self, stubs, make_output, job, tq):
+        final_output_c = arvados.collection.Collection()
+        make_output.return_value = ({},final_output_c)
+
+        def set_final_output(job_order, output_callback, runtimeContext):
+            output_callback("zzzzz-4zz18-zzzzzzzzzzzzzzzz", "success")
+            return []
+        job.side_effect = set_final_output
+
+        exited = arvados_cwl.main(
+            ["--debug", "--local",
+                "tests/wf/submit_storage_class_wf.cwl", "tests/submit_test_job.json"],
+            stubs.capture_stdout, sys.stderr, api_client=stubs.api, keep_client=stubs.keep_client)
+
+        make_output.assert_called_with(u'Output of submit_storage_class_wf.cwl', ['foo', 'bar'], '', 'zzzzz-4zz18-zzzzzzzzzzzzzzzz')
+        self.assertEqual(exited, 0)
+
     @stubs
     def test_submit_container_output_ttl(self, stubs):
         exited = arvados_cwl.main(
diff --git a/sdk/cwl/tests/wf/submit_storage_class_wf.cwl b/sdk/cwl/tests/wf/submit_storage_class_wf.cwl
new file mode 100644 (file)
index 0000000..c365d96
--- /dev/null
@@ -0,0 +1,30 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+# Test case for arvados-cwl-runner
+#
+# Used to test whether scanning a workflow file for dependencies
+# (e.g. submit_tool.cwl) and uploading to Keep works as intended.
+
+class: Workflow
+cwlVersion: v1.0
+$namespaces:
+  arv: "http://arvados.org/cwl#"
+hints:
+  arv:OutputStorageClass:
+    finalStorageClass: [foo, bar]
+inputs:
+  - id: x
+    type: File
+  - id: y
+    type: Directory
+  - id: z
+    type: Directory
+outputs: []
+steps:
+  - id: step1
+    in:
+      - { id: x, source: "#x" }
+    out: []
+    run: ../tool/submit_tool.cwl