COMPUTE-1764_no_selector_extra_args

(feat) all - account for the cloned jobs and the runtime specs there

Author: aogorodnikov-dna

src/python/dxpy/cli/parsers.py

@@ -235,14 +235,31 @@ def check_system_requirements(sys_reqs, context="systemRequirements"):
         if 'systemRequirements' in args.extra_args:
             check_system_requirements(args.extra_args['systemRequirements'])
 
+        # Check systemRequirementsByExecutable (double-nested structure: executable -> entrypoint -> requirements)
+        if 'systemRequirementsByExecutable' in args.extra_args:
+            sys_reqs_by_exec = args.extra_args['systemRequirementsByExecutable']
+            if isinstance(sys_reqs_by_exec, dict):
+                for executable_id, entrypoints in sys_reqs_by_exec.items():
+                    if isinstance(entrypoints, dict):
+                        check_system_requirements(entrypoints, f"systemRequirementsByExecutable.{executable_id}")
+
         # Also check regionalOptions
         if 'regionalOptions' in args.extra_args:
             regional_opts = args.extra_args['regionalOptions']
             if isinstance(regional_opts, dict):
                 for region, region_spec in regional_opts.items():
-                    if isinstance(region_spec, dict) and 'systemRequirements' in region_spec:
-                        check_system_requirements(region_spec['systemRequirements'],
-                                                 f"regionalOptions.{region}.systemRequirements")
+                    if isinstance(region_spec, dict):
+                        if 'systemRequirements' in region_spec:
+                            check_system_requirements(region_spec['systemRequirements'],
+                                                     f"regionalOptions.{region}.systemRequirements")
+                        # Also check systemRequirementsByExecutable within regionalOptions
+                        if 'systemRequirementsByExecutable' in region_spec:
+                            sys_reqs_by_exec = region_spec['systemRequirementsByExecutable']
+                            if isinstance(sys_reqs_by_exec, dict):
+                                for executable_id, entrypoints in sys_reqs_by_exec.items():
+                                    if isinstance(entrypoints, dict):
+                                        check_system_requirements(entrypoints,
+                                                                f"regionalOptions.{region}.systemRequirementsByExecutable.{executable_id}")
 
 exec_input_args = argparse.ArgumentParser(add_help=False)
 exec_input_args.add_argument('-i', '--input', help=fill('An input to be added using "<input name>[:<class>]=<input value>" (provide "class" if there is no input spec; it can be any job IO class, e.g. "string", "array:string", or "array"; if "class" is "array" or not specified, the value will be attempted to be parsed as JSON and is otherwise treated as a string)', width_adjustment=-24), action='append')

src/python/dxpy/scripts/dx.py

@@ -3191,27 +3191,38 @@ def run_body(args, executable, dest_proj, dest_path, preset_inputs=None, input_n
         # however when cloning from an analysis, the temporary workflow already has the cloned spec as its default, so no need to merge 1) and 2) here
         cloned_system_requirements = copy.deepcopy(args.cloned_job_desc).get("systemRequirements", {})
         cloned_instance_type = SystemRequirementsDict.from_sys_requirements(cloned_system_requirements, _type='instanceType')
+        cloned_instance_type_selector = SystemRequirementsDict.from_sys_requirements(cloned_system_requirements, _type='instanceTypeSelector')
         cloned_cluster_spec = SystemRequirementsDict.from_sys_requirements(cloned_system_requirements, _type='clusterSpec')
         cloned_fpga_driver = SystemRequirementsDict.from_sys_requirements(cloned_system_requirements, _type='fpgaDriver')
         cloned_nvidia_driver = SystemRequirementsDict.from_sys_requirements(cloned_system_requirements, _type='nvidiaDriver')
         cloned_system_requirements_by_executable = args.cloned_job_desc.get("mergedSystemRequirementsByExecutable", {}) or {}
     else:
         cloned_system_requirements = {}
-        cloned_instance_type, cloned_cluster_spec, cloned_fpga_driver, cloned_nvidia_driver = (
-            SystemRequirementsDict({}), SystemRequirementsDict({}), SystemRequirementsDict({}), SystemRequirementsDict({}))
+        cloned_instance_type, cloned_instance_type_selector, cloned_cluster_spec, cloned_fpga_driver, cloned_nvidia_driver = (
+            SystemRequirementsDict({}), SystemRequirementsDict({}), SystemRequirementsDict({}), SystemRequirementsDict({}), SystemRequirementsDict({}))
         cloned_system_requirements_by_executable = {}
 
     # convert runtime --instance-type into mapping {entrypoint:{'instanceType':xxx}}
     # here the args.instance_type no longer contains specifications for stage sys reqs
     if args.instance_type:
+        # Runtime --instance-type overrides instanceTypeSelector from the executable's runSpec
+        # When instanceType is specified at runtime, instanceTypeSelector should not be passed to the API
         requested_instance_type = SystemRequirementsDict.from_instance_type(args.instance_type)
         if not isinstance(args.instance_type, basestring):
             requested_instance_type = SystemRequirementsDict(merge(cloned_instance_type.as_dict(), requested_instance_type.as_dict()))
     else:
+        # If no runtime instanceType, use cloned instanceType (not instanceTypeSelector)
+        # instanceTypeSelector from the executable's runSpec will be resolved by the API
         requested_instance_type = cloned_instance_type
 
     # convert runtime --instance-count into mapping {entrypoint:{'clusterSpec':{'initialInstanceCount': N}}})
     if args.instance_count:
+        # Validate that we're not mixing clusterSpec with instanceTypeSelector
+        # clusterSpec and instanceTypeSelector are mutually exclusive at build time
+        # If runtime provides instance-count, we should not have instanceTypeSelector from cloned job
+        if cloned_instance_type_selector.as_dict():
+            raise err_exit("Cannot specify --instance-count when cloning a job that uses instanceTypeSelector. "
+                          "instanceTypeSelector and clusterSpec are mutually exclusive.")
         # retrieve the full cluster spec defined in executable's runSpec.systemRequirements
         # and overwrite the field initialInstanceCount with the runtime mapping
         requested_instance_count = SystemRequirementsDict.from_instance_count(args.instance_count)        
@@ -3232,8 +3243,17 @@ def run_body(args, executable, dest_proj, dest_path, preset_inputs=None, input_n
     requested_fpga_driver = cloned_fpga_driver
     requested_nvidia_driver = cloned_nvidia_driver
 
+    # Validate mutual exclusivity: instanceTypeSelector and clusterSpec cannot coexist
+    # Note: instanceType can override instanceTypeSelector, so we only check clusterSpec here
+    # instanceTypeSelector is build-time only and should never be in runtime systemRequirements
+    if cloned_instance_type_selector.as_dict() and requested_cluster_spec.as_dict():
+        raise err_exit("Cannot combine clusterSpec with instanceTypeSelector. "
+                      "instanceTypeSelector and clusterSpec are mutually exclusive.")
+
     # combine the requested instance type, full cluster spec, fpga spec, nvidia spec
     # into the runtime systemRequirements
+    # Note: instanceTypeSelector is NOT included here as it's build-time only
+    # If runtime instanceType was provided, it overrides instanceTypeSelector
     requested_system_requirements = (requested_instance_type + requested_cluster_spec + requested_fpga_driver +
                                      requested_nvidia_driver).as_dict()
 

src/python/dxpy/system_requirements.py

@@ -83,9 +83,9 @@ def from_sys_requirements(cls, system_requirements, _type='all'):
         """
         Returns SystemRequirementsDict encapsulating system requirements.
         It can extract only entrypoints with specific fields ('clusterSpec',
-        'instanceType', etc), depending on the value of _type.
+        'instanceType', 'instanceTypeSelector', etc), depending on the value of _type.
         """
-        allowed_types = ['all', 'clusterSpec', 'instanceType', 'fpgaDriver', 'nvidiaDriver']
+        allowed_types = ['all', 'clusterSpec', 'instanceType', 'instanceTypeSelector', 'fpgaDriver', 'nvidiaDriver']
         if _type not in (allowed_types):
             raise DXError("Expected '_type' to be one of the following: {}".format(allowed_types))
 

src/python/test/test_dxclient.py

@@ -3305,6 +3305,21 @@ def test_dx_run_extra_args_instanceTypeSelector_rejected(self):
             run('dx run ' + applet_id + ' --extra-args ' +
                 '\'{"regionalOptions": {"aws:us-east-1": {"systemRequirements": {"*": {"instanceTypeSelector": {"allowedInstanceTypes": ["mem2_ssd1_v2_x2"]}}}}}}\'')
 
+        # Test with systemRequirementsByExecutable
+        with self.assertSubprocessFailure(stderr_regexp='instanceTypeSelector cannot be specified in runtime', exit_code=3):
+            run('dx run ' + applet_id + ' --extra-args ' +
+                '\'{"systemRequirementsByExecutable": {"' + applet_id + '": {"main": {"instanceTypeSelector": {"allowedInstanceTypes": ["mem2_ssd1_v2_x2"]}}}}}\'')
+
+        # Test with systemRequirementsByExecutable with wildcard entry point
+        with self.assertSubprocessFailure(stderr_regexp='instanceTypeSelector cannot be specified in runtime', exit_code=3):
+            run('dx run ' + applet_id + ' --extra-args ' +
+                '\'{"systemRequirementsByExecutable": {"' + applet_id + '": {"*": {"instanceTypeSelector": {"allowedInstanceTypes": ["mem2_ssd1_v2_x2"]}}}}}\'')
+
+        # Test with regionalOptions.systemRequirementsByExecutable
+        with self.assertSubprocessFailure(stderr_regexp='instanceTypeSelector cannot be specified in runtime', exit_code=3):
+            run('dx run ' + applet_id + ' --extra-args ' +
+                '\'{"regionalOptions": {"aws:us-east-1": {"systemRequirementsByExecutable": {"' + applet_id + '": {"main": {"instanceTypeSelector": {"allowedInstanceTypes": ["mem2_ssd1_v2_x2"]}}}}}}}\'')
+
     def test_dx_run_sys_reqs(self):
         app_spec = {"project": self.project,
                     "dxapi": "1.0.0",
@@ -3403,6 +3418,64 @@ def test_dx_run_clone_nvidia_driver(self):
         cloned_job_desc = dxpy.api.job_describe(cloned_job_id_nvidia_override)
         assert cloned_job_desc["systemRequirements"]["*"]["nvidiaDriver"] == build_nvidia_version
 
+    def test_dx_run_clone_with_instance_type_selector(self):
+        """
+        Test that when cloning a job from an applet that uses instanceTypeSelector:
+        1. Cloning without runtime args works - instanceTypeSelector is preserved
+        2. Cloning with --instance-type works - runtime instanceType overrides instanceTypeSelector
+        3. Cloning with --instance-count fails - clusterSpec and instanceTypeSelector are mutually exclusive
+        4. Job description contains instanceTypeSelector in systemRequirements
+        """
+        # Create an applet with instanceTypeSelector
+        applet_id = dxpy.api.applet_new({"project": self.project,
+                                         "dxapi": "1.0.0",
+                                         "runSpec": {"interpreter": "bash",
+                                                     "distribution": "Ubuntu",
+                                                     "release": "20.04",
+                                                     "version": "0",
+                                                     "code": "echo 'hello'",
+                                                     "systemRequirements": {
+                                                         "*": {
+                                                             "instanceTypeSelector": {
+                                                                 "allowedInstanceTypes": ["mem1_ssd1_x2", "mem2_ssd1_x2"]
+                                                             }
+                                                         }
+                                                     }}
+                                         })['id']
+
+        # Run the applet to create the origin job
+        # The API will resolve instanceTypeSelector to an actual instanceType
+        origin_job_id = run(f"dx run {applet_id} --brief -y").strip().split('\n')[-1]
+
+        # Verify the origin job description contains instanceTypeSelector
+        origin_job_desc = dxpy.api.job_describe(origin_job_id)
+        assert "systemRequirements" in origin_job_desc
+        assert "*" in origin_job_desc["systemRequirements"]
+        assert "instanceTypeSelector" in origin_job_desc["systemRequirements"]["*"]
+        assert "allowedInstanceTypes" in origin_job_desc["systemRequirements"]["*"]["instanceTypeSelector"]
+
+        # Clone without runtime args - should work, instanceTypeSelector is preserved
+        cloned_job_id_1 = run(f"dx run --clone {origin_job_id} --brief -y").strip()
+        assert cloned_job_id_1.startswith("job-")
+        cloned_job_desc_1 = dxpy.api.job_describe(cloned_job_id_1)
+        # Verify instanceTypeSelector is in the cloned job's systemRequirements
+        assert "instanceTypeSelector" in cloned_job_desc_1["systemRequirements"]["*"]
+
+        # Clone with --instance-type - should work, runtime instanceType overrides instanceTypeSelector
+        cloned_job_id_2 = run(f"dx run --clone {origin_job_id} --instance-type mem2_hdd2_x4 --brief -y").strip()
+        assert cloned_job_id_2.startswith("job-")
+        cloned_job_desc_2 = dxpy.api.job_describe(cloned_job_id_2)
+        # Verify instanceType is used instead of instanceTypeSelector
+        assert "instanceType" in cloned_job_desc_2["systemRequirements"]["*"]
+        assert cloned_job_desc_2["systemRequirements"]["*"]["instanceType"] == "mem2_hdd2_x4"
+        # instanceTypeSelector should not be in the runtime systemRequirements when overridden
+        # TODO uncomment later - for now this is not included in the backend behaviour
+        # assert "instanceTypeSelector" not in cloned_job_desc_2["systemRequirements"]["*"]
+
+        # Clone with --instance-count - should fail (clusterSpec and instanceTypeSelector are mutually exclusive)
+        with self.assertSubprocessFailure(stderr_regexp='Cannot specify --instance-count.*instanceTypeSelector', exit_code=3):
+            run(f"dx run --clone {origin_job_id} --instance-count 3 --brief -y")
+
     def test_dx_run_clone(self):
         applet_id = dxpy.api.applet_new({"project": self.project,
                                          "dxapi": "1.0.0",