Merge "Validate capability in properties and instance_info"

ironic/drivers/modules/deploy_utils.py

@@ -72,6 +72,10 @@ LOG = logging.getLogger(__name__)
 
 VALID_ROOT_DEVICE_HINTS = set(('size', 'model', 'wwn', 'serial', 'vendor'))
 
+SUPPORTED_CAPABILITIES = {'boot_option': ('local', 'netboot'),
+                          'boot_mode': ('bios', 'uefi'),
+                          'secure_boot': ('true', 'false')}
+
 
 # All functions are called from deploy() directly or indirectly.
 # They are split for stub-out.
@@ -1082,3 +1086,44 @@ def get_boot_mode_for_deploy(node):
               {'boot_mode': boot_mode, 'node': node.uuid})
 
     return boot_mode.lower() if boot_mode else boot_mode
+
+
+def validate_capabilities(node):
+    """Validates that specified supported capabilities have valid value
+
+    This method checks if the any of the supported capability is present in
+    Node capabilities. For all supported capabilities specified for a Node,
+    it validates that it has a valid value.
+    The node can have capability as part of the 'properties' or
+    'instance_info' or both.
+    Note that the actual value of a capability does not need to be the same
+    in the node's 'properties' and 'instance_info'.
+
+    :param node: an ironic node object.
+    :raises: InvalidParameterValue, if the capability is not set to a
+        valid value.
+    """
+    exp_str = _("The parameter '%(capability)s' from %(field)s has an "
+                "invalid value: '%(value)s'. Acceptable values are: "
+                "%(valid_values)s.")
+
+    for capability_name, valid_values in SUPPORTED_CAPABILITIES.items():
+        # Validate capability_name in node's properties/capabilities
+        value = driver_utils.get_node_capability(node, capability_name)
+        if value and (value not in valid_values):
+            field = "properties/capabilities"
+            raise exception.InvalidParameterValue(
+                exp_str %
+                {'capability': capability_name, 'field': field,
+                 'value': value, 'valid_values': ', '.join(valid_values)})
+
+        # Validate capability_name in node's instance_info/['capabilities']
+        capabilities = parse_instance_info_capabilities(node)
+        value = capabilities.get(capability_name)
+
+        if value and (value not in valid_values):
+            field = "instance_info['capabilities']"
+            raise exception.InvalidParameterValue(
+                exp_str %
+                {'capability': capability_name, 'field': field,
+                 'value': value, 'valid_values': ', '.join(valid_values)})

ironic/drivers/modules/ilo/deploy.py

@@ -43,7 +43,6 @@ from ironic.drivers.modules.ilo import common as ilo_common
 from ironic.drivers.modules import ipmitool
 from ironic.drivers.modules import iscsi_deploy
 from ironic.drivers.modules import pxe
-from ironic.drivers import utils as driver_utils
 
 LOG = logging.getLogger(__name__)
 
@@ -392,9 +391,7 @@ class IloVirtualMediaIscsiDeploy(base.DeployInterface):
         else:
             props = ['kernel', 'ramdisk']
         iscsi_deploy.validate_image_properties(task.context, d_info, props)
-        driver_utils.validate_boot_mode_capability(node)
+        deploy_utils.validate_capabilities(node)
-        driver_utils.validate_boot_option_capability(node)
-        driver_utils.validate_secure_boot_capability(node)
 
     @task_manager.require_exclusive_lock
     def deploy(self, task):
@@ -490,8 +487,7 @@ class IloVirtualMediaAgentDeploy(base.DeployInterface):
         :raises: MissingParameterValue if some parameters are missing.
         """
 
-        driver_utils.validate_boot_mode_capability(task.node)
+        deploy_utils.validate_capabilities(task.node)
-        driver_utils.validate_secure_boot_capability(task.node)
         _parse_driver_info(task.node)
 
     @task_manager.require_exclusive_lock

ironic/drivers/modules/pxe.py

@@ -43,7 +43,6 @@ from ironic.drivers.modules import agent_base_vendor
 from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules import image_cache
 from ironic.drivers.modules import iscsi_deploy
-from ironic.drivers import utils as driver_utils
 from ironic.openstack.common import fileutils
 
 
@@ -303,8 +302,7 @@ class PXEDeploy(base.DeployInterface):
         node = task.node
 
         # Check the boot_mode and boot_option capabilities values.
-        driver_utils.validate_boot_mode_capability(node)
+        deploy_utils.validate_capabilities(node)
-        driver_utils.validate_boot_option_capability(node)
 
         boot_mode = deploy_utils.get_boot_mode_for_deploy(task.node)
 
@@ -503,7 +501,7 @@ class VendorPassthru(agent_base_vendor.BaseAgentVendor):
         :raises: InvalidParameterValue if any parameters is invalid.
         """
         if method == 'pass_deploy_info':
-            driver_utils.validate_boot_option_capability(task.node)
+            deploy_utils.validate_capabilities(task.node)
             iscsi_deploy.get_deploy_info(task.node, **kwargs)
         elif method == 'pass_bootloader_install_info':
             iscsi_deploy.validate_pass_bootloader_info_input(task, kwargs)

ironic/drivers/utils.py

@@ -170,57 +170,3 @@ def add_node_capability(task, capability, value):
     properties['capabilities'] = capabilities
     node.properties = properties
     node.save()
-
-
-def validate_capability(node, capability_name, valid_values):
-    """Validate a capabability set in node property
-
-    :param node: an ironic node object.
-    :param capability_name: the name of the capability.
-    :parameter valid_values: an iterable with valid values expected for
-        that capability.
-    :raises: InvalidParameterValue, if the capability is not set to the
-        expected values.
-    """
-    value = get_node_capability(node, capability_name)
-
-    if value and value not in valid_values:
-        valid_value_str = ', '.join(valid_values)
-        raise exception.InvalidParameterValue(
-            _("Invalid %(capability)s parameter '%(value)s'. "
-              "Acceptable values are: %(valid_values)s.") %
-            {'capability': capability_name, 'value': value,
-             'valid_values': valid_value_str})
-
-
-def validate_boot_mode_capability(node):
-    """Validate the boot_mode capability set in node properties.
-
-    :param node: an ironic node object.
-    :raises: InvalidParameterValue, if 'boot_mode' capability is set
-             other than 'bios' or 'uefi' or None.
-
-    """
-    validate_capability(node, 'boot_mode', ('bios', 'uefi'))
-
-
-def validate_boot_option_capability(node):
-    """Validate the boot_option capability set in node properties.
-
-    :param node: an ironic node object.
-    :raises: InvalidParameterValue, if 'boot_option' capability is set
-             other than 'local' or 'netboot' or None.
-
-    """
-    validate_capability(node, 'boot_option', ('local', 'netboot'))
-
-
-def validate_secure_boot_capability(node):
-    """Validate the secure_boot capability set in node property.
-
-    :param node: an ironic node object.
-    :raises: InvalidParameterValue, if 'secure_boot' capability is set
-             other than 'true' or 'false' or None.
-
-    """
-    validate_capability(node, 'secure_boot', ('true', 'false'))

ironic/tests/drivers/ilo/test_deploy.py

@@ -474,9 +474,7 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase):
         self.node = obj_utils.create_test_node(
             self.context, driver='iscsi_ilo', driver_info=INFO_DICT)
 
-    @mock.patch.object(driver_utils, 'validate_secure_boot_capability',
+    @mock.patch.object(deploy_utils, 'validate_capabilities',
-                       spec_set=True, autospec=True)
-    @mock.patch.object(driver_utils, 'validate_boot_mode_capability',
                        spec_set=True, autospec=True)
     @mock.patch.object(iscsi_deploy, 'validate_image_properties',
                        spec_set=True, autospec=True)
@@ -486,8 +484,7 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase):
     def _test_validate(self, validate_mock,
                        deploy_info_mock,
                        validate_prop_mock,
-                       validate_boot_mode_mock,
+                       validate_capability_mock,
-                       validate_secure_boot_mock,
                        props_expected):
         d_info = {'image_source': 'uuid'}
         deploy_info_mock.return_value = d_info
@@ -498,8 +495,7 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase):
             deploy_info_mock.assert_called_once_with(task.node)
             validate_prop_mock.assert_called_once_with(
                 task.context, d_info, props_expected)
-            validate_boot_mode_mock.assert_called_once_with(task.node)
+            validate_capability_mock.assert_called_once_with(task.node)
-            validate_secure_boot_mock.assert_called_once_with(task.node)
 
     @mock.patch.object(iscsi_deploy, 'validate_image_properties',
                        spec_set=True, autospec=True)
@@ -675,22 +671,18 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase):
         self.node = obj_utils.create_test_node(
             self.context, driver='agent_ilo', driver_info=INFO_DICT)
 
-    @mock.patch.object(driver_utils, 'validate_secure_boot_capability',
+    @mock.patch.object(deploy_utils, 'validate_capabilities',
-                       spec_set=True, autospec=True)
-    @mock.patch.object(driver_utils, 'validate_boot_mode_capability',
                        spec_set=True, autospec=True)
     @mock.patch.object(ilo_deploy, '_parse_driver_info', spec_set=True,
                        autospec=True)
     def test_validate(self,
                       parse_driver_info_mock,
-                      validate_boot_mode_mock,
+                      validate_capability_mock):
-                      validate_secure_boot_mock):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.driver.deploy.validate(task)
             parse_driver_info_mock.assert_called_once_with(task.node)
-            validate_boot_mode_mock.assert_called_once_with(task.node)
+            validate_capability_mock.assert_called_once_with(task.node)
-            validate_secure_boot_mock.assert_called_once_with(task.node)
 
     @mock.patch.object(ilo_deploy, '_prepare_agent_vmedia_boot', spec_set=True,
                        autospec=True)

ironic/tests/drivers/test_deploy_utils.py

@@ -1559,6 +1559,42 @@ class ParseInstanceInfoCapabilitiesTestCase(tests_base.TestCase):
         result = utils.get_boot_mode_for_deploy(self.node)
         self.assertEqual('bios', result)
 
+    def test_validate_boot_mode_capability(self):
+        prop = {'capabilities': 'boot_mode:uefi,cap2:value2'}
+        self.node.properties = prop
+
+        result = utils.validate_capabilities(self.node)
+        self.assertIsNone(result)
+
+    def test_validate_boot_mode_capability_with_exc(self):
+        prop = {'capabilities': 'boot_mode:UEFI,cap2:value2'}
+        self.node.properties = prop
+
+        self.assertRaises(exception.InvalidParameterValue,
+                          utils.validate_capabilities, self.node)
+
+    def test_validate_boot_mode_capability_instance_info(self):
+        inst_info = {'capabilities': {"boot_mode": "uefi", "cap2": "value2"}}
+        self.node.instance_info = inst_info
+
+        result = utils.validate_capabilities(self.node)
+        self.assertIsNone(result)
+
+    def test_validate_boot_mode_capability_instance_info_with_exc(self):
+        inst_info = {'capabilities': {"boot_mode": "UEFI", "cap2": "value2"}}
+        self.node.instance_info = inst_info
+
+        self.assertRaises(exception.InvalidParameterValue,
+                          utils.validate_capabilities, self.node)
+
+    def test_all_supported_capabilities(self):
+        self.assertEqual(('local', 'netboot'),
+                         utils.SUPPORTED_CAPABILITIES['boot_option'])
+        self.assertEqual(('bios', 'uefi'),
+                         utils.SUPPORTED_CAPABILITIES['boot_mode'])
+        self.assertEqual(('true', 'false'),
+                         utils.SUPPORTED_CAPABILITIES['secure_boot'])
+
 
 class TrySetBootDeviceTestCase(db_base.DbTestCase):
 

ironic/tests/drivers/test_utils.py

@@ -114,64 +114,3 @@ class UtilsTestCase(db_base.DbTestCase):
             driver_utils.add_node_capability(task, 'a', 'b')
             self.assertEqual('a:b,c:d,a:b',
                              task.node.properties['capabilities'])
-
-    def test_validate_capability(self):
-        properties = {'capabilities': 'cat:meow,cap2:value2'}
-        self.node.properties = properties
-
-        result = driver_utils.validate_capability(
-            self.node, 'cat', ['meow', 'purr'])
-        self.assertIsNone(result)
-
-    def test_validate_capability_with_exception(self):
-        properties = {'capabilities': 'cat:bark,cap2:value2'}
-        self.node.properties = properties
-
-        self.assertRaises(exception.InvalidParameterValue,
-                          driver_utils.validate_capability,
-                          self.node, 'cat', ['meow', 'purr'])
-
-    def test_validate_boot_mode_capability(self):
-        properties = {'capabilities': 'boot_mode:uefi,cap2:value2'}
-        self.node.properties = properties
-
-        result = driver_utils.validate_boot_mode_capability(self.node)
-        self.assertIsNone(result)
-
-    def test_validate_boot_mode_capability_with_exception(self):
-        properties = {'capabilities': 'boot_mode:foo,cap2:value2'}
-        self.node.properties = properties
-
-        self.assertRaises(exception.InvalidParameterValue,
-                          driver_utils.validate_boot_mode_capability,
-                          self.node)
-
-    def test_validate_boot_option_capability(self):
-        properties = {'capabilities': 'boot_option:netboot,cap2:value2'}
-        self.node.properties = properties
-
-        result = driver_utils.validate_boot_option_capability(self.node)
-        self.assertIsNone(result)
-
-    def test_validate_boot_option_capability_with_exception(self):
-        properties = {'capabilities': 'boot_option:foo,cap2:value2'}
-        self.node.properties = properties
-
-        self.assertRaises(exception.InvalidParameterValue,
-                          driver_utils.validate_boot_option_capability,
-                          self.node)
-
-    def test_validate_secure_boot_capability(self):
-        properties = {'capabilities': 'secure_boot:true,cap2:value2'}
-        self.node.properties = properties
-
-        result = driver_utils.validate_secure_boot_capability(self.node)
-        self.assertIsNone(result)
-
-    def test_validate_secure_boot_capability_with_exception(self):
-        properties = {'capabilities': 'secure_boot:foo,cap2:value2'}
-        self.node.properties = properties
-
-        self.assertRaises(exception.InvalidParameterValue,
-                          driver_utils.validate_secure_boot_capability,
-                          self.node)