COMPUTE-1703_instance_type_selector_validation

(feat) app_builder.py, test_dxpy.py - added validation for the applet build checking mutual exclusivity in the applet spec for selector and instanceType and clusterSpec

Author: aogorodnikov-dna

src/python/dxpy/app_builder.py

@@ -64,10 +64,47 @@ class AppBuilderException(Exception):
     """
     pass
 
+def _validate_system_requirements(system_requirements):
+    """
+    Validates systemRequirements section of dxapp.json for instanceTypeSelector constraints.
+
+    :param system_requirements: The systemRequirements section from runSpec
+    :type system_requirements: dict
+    :raises AppBuilderException: If validation fails
+    """
+    if not isinstance(system_requirements, dict):
+        return
+
+    for entry_point, requirements in system_requirements.items():
+        if not isinstance(requirements, dict):
+            continue
+
+        has_instance_type = 'instanceType' in requirements
+        has_instance_type_selector = 'instanceTypeSelector' in requirements
+        has_cluster_spec = 'clusterSpec' in requirements
+
+        # Check mutual exclusivity: instanceType and instanceTypeSelector
+        if has_instance_type and has_instance_type_selector:
+            raise AppBuilderException(
+                "instanceType and instanceTypeSelector keywords are mutually exclusive "
+                "in systemRequirements for entry point '{}'".format(entry_point)
+            )
+
+        # Check mutual exclusivity: instanceTypeSelector and clusterSpec
+        if has_instance_type_selector and has_cluster_spec:
+            raise AppBuilderException(
+                "instanceTypeSelector and clusterSpec keywords are mutually exclusive "
+                "in systemRequirements for entry point '{}'".format(entry_point)
+            )
+
 def _validate_applet_spec(applet_spec):
     if 'runSpec' not in applet_spec:
         raise AppBuilderException("Required field 'runSpec' not found in dxapp.json")
 
+    # Validate systemRequirements for instanceTypeSelector constraints
+    if 'systemRequirements' in applet_spec.get('runSpec', {}):
+        _validate_system_requirements(applet_spec['runSpec']['systemRequirements'])
+
 def _validate_app_spec(app_spec):
     pass
 

src/python/test/test_dxpy.py

@@ -3191,6 +3191,118 @@ def test_assert_consistent_regions(self):
         with self.assertRaises(app_builder.AppBuilderException):
             assert_consistent_regions({"aws:us-east-1": None}, ["azure:westus"], app_builder.AppBuilderException)
 
+    def test_validate_system_requirements_instance_type_selector(self):
+        """Test that instanceTypeSelector validation works correctly"""
+        validate = app_builder._validate_system_requirements
+
+        # Valid cases - should not raise exceptions
+
+        # instanceTypeSelector alone is valid
+        validate({
+            "main": {
+                "instanceTypeSelector": {
+                    "allowedInstanceTypes": ["mem1_ssd1_x4", "mem1_ssd1_x8"]
+                }
+            }
+        })
+
+        # instanceType alone is valid
+        validate({
+            "main": {
+                "instanceType": "mem1_ssd1_x4"
+            }
+        })
+
+        # clusterSpec alone is valid
+        validate({
+            "main": {
+                "clusterSpec": {
+                    "type": "generic",
+                    "numInstances": 3
+                }
+            }
+        })
+
+        # instanceType and clusterSpec together is valid
+        validate({
+            "main": {
+                "instanceType": "mem1_ssd1_x4",
+                "clusterSpec": {
+                    "type": "generic",
+                    "numInstances": 3
+                }
+            }
+        })
+
+        # Empty requirements is valid
+        validate({})
+
+        # None requirements is valid
+        validate(None)
+
+        # Invalid cases - should raise AppBuilderException
+
+        # instanceType and instanceTypeSelector are mutually exclusive
+        with self.assertRaises(app_builder.AppBuilderException) as cm:
+            validate({
+                "main": {
+                    "instanceType": "mem1_ssd1_x4",
+                    "instanceTypeSelector": {
+                        "allowedInstanceTypes": ["mem1_ssd1_x4", "mem1_ssd1_x8"]
+                    }
+                }
+            })
+        self.assertIn("mutually exclusive", str(cm.exception))
+        self.assertIn("instanceType", str(cm.exception))
+        self.assertIn("instanceTypeSelector", str(cm.exception))
+
+        # instanceTypeSelector and clusterSpec are mutually exclusive
+        with self.assertRaises(app_builder.AppBuilderException) as cm:
+            validate({
+                "main": {
+                    "instanceTypeSelector": {
+                        "allowedInstanceTypes": ["mem1_ssd1_x4", "mem1_ssd1_x8"]
+                    },
+                    "clusterSpec": {
+                        "type": "generic",
+                        "numInstances": 3
+                    }
+                }
+            })
+        self.assertIn("mutually exclusive", str(cm.exception))
+        self.assertIn("instanceTypeSelector", str(cm.exception))
+        self.assertIn("clusterSpec", str(cm.exception))
+
+        # Test with wildcard entry point
+        with self.assertRaises(app_builder.AppBuilderException) as cm:
+            validate({
+                "*": {
+                    "instanceType": "mem1_ssd1_x4",
+                    "instanceTypeSelector": {
+                        "allowedInstanceTypes": ["mem1_ssd1_x4"]
+                    }
+                }
+            })
+        self.assertIn("mutually exclusive", str(cm.exception))
+
+        # Test with multiple entry points - one invalid
+        with self.assertRaises(app_builder.AppBuilderException) as cm:
+            validate({
+                "main": {
+                    "instanceTypeSelector": {
+                        "allowedInstanceTypes": ["mem1_ssd1_x4"]
+                    }
+                },
+                "process": {
+                    "instanceType": "mem1_ssd1_x4",
+                    "instanceTypeSelector": {
+                        "allowedInstanceTypes": ["mem1_ssd1_x8"]
+                    }
+                }
+            })
+        self.assertIn("mutually exclusive", str(cm.exception))
+        self.assertIn("process", str(cm.exception))
+
 class TestWorkflowBuilderUtils(testutil.DXTestCaseBuildWorkflows):
     def setUp(self):
         super(TestWorkflowBuilderUtils, self).setUp()