Add uefi boot mode support in IloVirtualMediaIscsiDeploy

The following functionality need to be added in iLO driver:
  - Add set_boot_mode function to iLO driver to switch between
    bios and uefi boot modes.
  - Validate boot_mode for IloVirtualMediaIscsiDeploy driver.

Change-Id: Ic8496a2d97aab6634b573c85d379b446c49e04b9
Implements: blueprint uefi-boot-for-ironic

ironic/common/pxe_utils.py

@@ -22,7 +22,6 @@ from oslo.config import cfg
 from ironic.common import dhcp_factory
 from ironic.common import exception
 from ironic.common.i18n import _
-from ironic.common.i18n import _LW
 from ironic.common import utils
 from ironic.drivers import utils as driver_utils
 from ironic.openstack.common import fileutils
@@ -191,7 +190,7 @@ def create_pxe_config(task, pxe_options, template=None):
     pxe_config = _build_pxe_config(pxe_options, template)
     utils.write_to_file(pxe_config_file_path, pxe_config)
 
-    if get_node_capability(task.node, 'boot_mode') == 'uefi':
+    if driver_utils.get_node_capability(task.node, 'boot_mode') == 'uefi':
         _link_ip_address_pxe_configs(task)
     else:
         _link_mac_pxe_configs(task)
@@ -205,7 +204,7 @@ def clean_up_pxe_config(task):
     """
     LOG.debug("Cleaning up PXE config for node %s", task.node.uuid)
 
-    if get_node_capability(task.node, 'boot_mode') == 'uefi':
+    if driver_utils.get_node_capability(task.node, 'boot_mode') == 'uefi':
         api = dhcp_factory.DHCPFactory().provider
         ip_addresses = api.get_ip_addresses(task)
         if not ip_addresses:
@@ -244,7 +243,7 @@ def dhcp_options_for_instance(task):
         dhcp_opts.append({'opt_name': 'bootfile-name',
                           'opt_value': ipxe_script_url})
     else:
-        if get_node_capability(task.node, 'boot_mode') == 'uefi':
+        if driver_utils.get_node_capability(task.node, 'boot_mode') == 'uefi':
             boot_file = CONF.pxe.uefi_pxe_bootfile_name
         else:
             boot_file = CONF.pxe.pxe_bootfile_name
@@ -257,42 +256,3 @@ def dhcp_options_for_instance(task):
     dhcp_opts.append({'opt_name': 'tftp-server',
                       'opt_value': CONF.pxe.tftp_server})
     return dhcp_opts
-
-
-def get_node_capability(node, capability):
-    """Returns 'capability' value from node's 'capabilities' property.
-
-    :param node: Node object.
-    :param capability: Capability key.
-    :return: Capability value.
-             If capability is not present, then return "None"
-
-    """
-    capabilities = node.properties.get('capabilities')
-
-    if not capabilities:
-        return
-
-    for node_capability in str(capabilities).split(','):
-        parts = node_capability.split(':')
-        if len(parts) == 2 and parts[0] and parts[1]:
-            if parts[0] == capability:
-                return parts[1]
-        else:
-            LOG.warn(_LW("Ignoring malformed capability '%s'. "
-                "Format should be 'key:val'."), node_capability)
-
-
-def validate_boot_mode_capability(node):
-    """Validate the boot_mode capability set in node property.
-
-    :param node: an ironic node object.
-    :raises: InvalidParameterValue, if 'boot_mode' capability is set
-             other than 'bios' or 'uefi' or None.
-
-    """
-    boot_mode = get_node_capability(node, 'boot_mode')
-
-    if boot_mode and boot_mode not in ['bios', 'uefi']:
-        raise exception.InvalidParameterValue(_("Invalid boot_mode "
-                          "parameter '%s'.") % boot_mode)

ironic/drivers/modules/ilo/common.py

@@ -28,6 +28,7 @@ from ironic.common.i18n import _LI
 from ironic.common import images
 from ironic.common import swift
 from ironic.common import utils
+from ironic.drivers import utils as driver_utils
 from ironic.openstack.common import log as logging
 
 ilo_client = importutils.try_import('proliantutils.ilo.ribcl')
@@ -69,6 +70,11 @@ OPTIONAL_PROPERTIES = {
 }
 COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy()
 COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES)
+DEFAULT_BOOT_MODE = 'LEGACY'
+
+BOOT_MODE_GENERIC_TO_ILO = {'bios': 'legacy', 'uefi': 'uefi'}
+BOOT_MODE_ILO_TO_GENERIC = dict((v, k)
+                           for (k, v) in BOOT_MODE_GENERIC_TO_ILO.items())
 
 
 def parse_driver_info(node):
@@ -274,6 +280,63 @@ def set_boot_device(node, device):
              {'uuid': node.uuid, 'device': device})
 
 
+def set_boot_mode(node, boot_mode):
+    """Sets the node to boot using boot_mode for the next boot.
+
+    :param node: an ironic node object.
+    :param boot_mode: Next boot mode.
+    :raises: IloOperationError if setting boot mode failed.
+    """
+    ilo_object = get_ilo_object(node)
+
+    try:
+        p_boot_mode = ilo_object.get_pending_boot_mode()
+    except ilo_client.IloCommandNotSupportedError:
+        p_boot_mode = DEFAULT_BOOT_MODE
+
+    if BOOT_MODE_ILO_TO_GENERIC[p_boot_mode.lower()] == boot_mode:
+        LOG.info(_LI("Node %(uuid)s pending boot mode is %(boot_mode)s."),
+                 {'uuid': node.uuid, 'boot_mode': boot_mode})
+        return
+
+    try:
+        ilo_object.set_pending_boot_mode(
+                        BOOT_MODE_GENERIC_TO_ILO[boot_mode].upper())
+    except ilo_client.IloError as ilo_exception:
+        operation = _("Setting %s as boot mode") % boot_mode
+        raise exception.IloOperationError(operation=operation,
+                error=ilo_exception)
+
+    LOG.info(_LI("Node %(uuid)s boot mode is set to %(boot_mode)s."),
+             {'uuid': node.uuid, 'boot_mode': boot_mode})
+
+
+def update_boot_mode_capability(task):
+    """Update 'boot_mode' capability value of node's 'capabilities' property.
+
+    :param task: Task object.
+
+    """
+    ilo_object = get_ilo_object(task.node)
+
+    try:
+        p_boot_mode = ilo_object.get_pending_boot_mode()
+        if p_boot_mode == 'UNKNOWN':
+            # NOTE(faizan) ILO will return this in remote cases and mostly on
+            # the nodes which supports UEFI. Such nodes mostly comes with UEFI
+            # as default boot mode. So we will try setting bootmode to UEFI
+            # and if it fails then we fall back to BIOS boot mode.
+            ilo_object.set_pending_boot_mode('UEFI')
+            p_boot_mode = 'UEFI'
+    except ilo_client.IloCommandNotSupportedError:
+        p_boot_mode = DEFAULT_BOOT_MODE
+
+    driver_utils.rm_node_capability(task, 'boot_mode')
+
+    driver_utils.add_node_capability(task, 'boot_mode',
+        BOOT_MODE_ILO_TO_GENERIC[p_boot_mode.lower()])
+
+
 def setup_vmedia_for_boot(task, boot_iso, parameters=None):
     """Sets up the node to boot from the given ISO image.
 

ironic/drivers/modules/ilo/deploy.py

@@ -33,6 +33,7 @@ from ironic.drivers.modules import agent
 from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules.ilo import common as ilo_common
 from ironic.drivers.modules import iscsi_deploy
+from ironic.drivers import utils as driver_utils
 from ironic.openstack.common import log as logging
 
 LOG = logging.getLogger(__name__)
@@ -92,6 +93,14 @@ def _get_boot_iso(task, root_uuid):
         LOG.debug("Found boot_iso %s in Glance", boot_iso_uuid)
         return 'glance:%s' % boot_iso_uuid
 
+    # NOTE(faizan) For uefi boot_mode, operator should provide efi capable
+    # boot-iso in glance
+    if driver_utils.get_node_capability(task.node, 'boot_mode') == 'uefi':
+        LOG.error(_LE("Unable to find boot_iso in Glance, required to deploy "
+                      "node %(node)s in UEFI boot mode."),
+                  {'node': task.node.uuid})
+        return
+
     kernel_uuid = images.get_glance_image_property(task.context,
             image_uuid, 'kernel_id')
     ramdisk_uuid = images.get_glance_image_property(task.context,
@@ -238,6 +247,7 @@ class IloVirtualMediaIscsiDeploy(base.DeployInterface):
         d_info = _parse_deploy_info(task.node)
         iscsi_deploy.validate_glance_image_properties(task.context, d_info,
                                                       props)
+        driver_utils.validate_boot_mode_capability(task.node)
 
     @task_manager.require_exclusive_lock
     def deploy(self, task):
@@ -288,8 +298,13 @@ class IloVirtualMediaIscsiDeploy(base.DeployInterface):
         """Prepare the deployment environment for this task's node.
 
         :param task: a TaskManager instance containing the node to act on.
+        :raises: IloOperationError, if some operation on iLO failed.
         """
-        pass
+        boot_mode = driver_utils.get_node_capability(task.node, 'boot_mode')
+        if boot_mode is not None:
+            ilo_common.set_boot_mode(task.node, boot_mode)
+        else:
+            ilo_common.update_boot_mode_capability(task)
 
     def clean_up(self, task):
         """Clean up the deployment environment for the task's node.

ironic/drivers/modules/pxe.py

@@ -39,6 +39,7 @@ from ironic.drivers import base
 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
 from ironic.openstack.common import log as logging
 
@@ -273,8 +274,9 @@ class PXEDeploy(base.DeployInterface):
         :raises: InvalidParameterValue.
         :raises: MissingParameterValue
         """
+
         # Check the boot_mode capability parameter value.
-        pxe_utils.validate_boot_mode_capability(task.node)
+        driver_utils.validate_boot_mode_capability(task.node)
 
         if CONF.pxe.ipxe_enabled:
             if not CONF.pxe.http_url or not CONF.pxe.http_root:
@@ -282,7 +284,8 @@ class PXEDeploy(base.DeployInterface):
                     "iPXE boot is enabled but no HTTP URL or HTTP "
                     "root was specified."))
             # iPXE and UEFI should not be configured together.
-            if pxe_utils.get_node_capability(task.node, 'boot_mode') == 'uefi':
+            if driver_utils.get_node_capability(task.node,
+                                                'boot_mode') == 'uefi':
                 LOG.error(_LE("UEFI boot mode is not supported with "
                               "iPXE boot enabled."))
                 raise exception.InvalidParameterValue(_(
@@ -330,7 +333,8 @@ class PXEDeploy(base.DeployInterface):
         try:
             manager_utils.node_set_boot_device(task, 'pxe', persistent=True)
         except exception.IPMIFailure:
-            if pxe_utils.get_node_capability(task.node, 'boot_mode') == 'uefi':
+            if driver_utils.get_node_capability(task.node,
+                                                'boot_mode') == 'uefi':
                 LOG.warning(_LW("ipmitool is unable to set boot device while "
                                 "the node is in UEFI boot mode."
                                 "Please set the boot device manually."))
@@ -373,7 +377,7 @@ class PXEDeploy(base.DeployInterface):
         pxe_options = _build_pxe_config_options(task.node, pxe_info,
                                                 task.context)
 
-        if pxe_utils.get_node_capability(task.node, 'boot_mode') == 'uefi':
+        if driver_utils.get_node_capability(task.node, 'boot_mode') == 'uefi':
             pxe_config_template = CONF.pxe.uefi_pxe_config_template
         else:
             pxe_config_template = CONF.pxe.pxe_config_template
@@ -461,7 +465,7 @@ class VendorPassthru(base.VendorInterface):
         try:
             pxe_config_path = pxe_utils.get_pxe_config_file_path(node.uuid)
             deploy_utils.switch_pxe_config(pxe_config_path, root_uuid,
-                            pxe_utils.get_node_capability(node, 'boot_mode'))
+                          driver_utils.get_node_capability(node, 'boot_mode'))
 
             deploy_utils.notify_deploy_complete(kwargs['address'])
 

ironic/drivers/utils.py

@@ -14,7 +14,12 @@
 
 from ironic.common import exception
 from ironic.common.i18n import _
+from ironic.common.i18n import _LW
 from ironic.drivers import base
+from ironic.openstack.common import log as logging
+
+
+LOG = logging.getLogger(__name__)
 
 
 def _raise_unsupported_error(method=None):
@@ -107,3 +112,92 @@ def get_node_mac_addresses(task):
     :returns: A list of MAC addresses in the format xx:xx:xx:xx:xx:xx.
     """
     return [p.address for p in task.ports]
+
+
+def get_node_capability(node, capability):
+    """Returns 'capability' value from node's 'capabilities' property.
+
+    :param node: Node object.
+    :param capability: Capability key.
+    :return: Capability value.
+             If capability is not present, then return "None"
+
+    """
+    capabilities = node.properties.get('capabilities')
+
+    if not capabilities:
+        return
+
+    for node_capability in capabilities.split(','):
+        parts = node_capability.split(':')
+        if len(parts) == 2 and parts[0] and parts[1]:
+            if parts[0] == capability:
+                return parts[1]
+        else:
+            LOG.warn(_LW("Ignoring malformed capability '%s'. "
+                "Format should be 'key:val'."), node_capability)
+
+
+def rm_node_capability(task, capability):
+    """Remove 'capability' from node's 'capabilities' property.
+
+    :param task: Task object.
+    :param capability: Capability key.
+
+    """
+    node = task.node
+    capabilities = node.properties.get('capabilities')
+
+    if not capabilities:
+        return
+
+    caps = []
+    for cap in capabilities.split(','):
+        parts = cap.split(':')
+        if len(parts) == 2 and parts[0] and parts[1]:
+            if parts[0] == capability:
+                continue
+        caps.append(cap)
+    new_cap_str = ",".join(caps)
+    node.properties['capabilities'] = new_cap_str if new_cap_str else None
+    node.save(task.context)
+
+
+def add_node_capability(task, capability, value):
+    """Add 'capability' to node's 'capabilities' property.
+
+    If 'capability' is already present, then a duplicate entry
+    will be added.
+
+    :param task: Task object.
+    :param capability: Capability key.
+    :param value: Capability value.
+
+    """
+    node = task.node
+    capabilities = node.properties.get('capabilities')
+
+    new_cap = ':'.join([capability, value])
+
+    if capabilities:
+        capabilities = ','.join([capabilities, new_cap])
+    else:
+        capabilities = new_cap
+
+    node.properties['capabilities'] = capabilities
+    node.save(task.context)
+
+
+def validate_boot_mode_capability(node):
+    """Validate the boot_mode capability set in node property.
+
+    :param node: an ironic node object.
+    :raises: InvalidParameterValue, if 'boot_mode' capability is set
+             other than 'bios' or 'uefi' or None.
+
+    """
+    boot_mode = get_node_capability(node, 'boot_mode')
+
+    if boot_mode and boot_mode not in ['bios', 'uefi']:
+        raise exception.InvalidParameterValue(_("Invalid boot_mode "
+                          "parameter '%s'.") % boot_mode)

ironic/tests/drivers/ilo/test_common.py

@@ -28,6 +28,7 @@ from ironic.common import utils
 from ironic.conductor import task_manager
 from ironic.db import api as dbapi
 from ironic.drivers.modules.ilo import common as ilo_common
+from ironic.drivers import utils as driver_utils
 from ironic.openstack.common import context
 from ironic.tests import base
 from ironic.tests.conductor import utils as mgr_utils
@@ -258,12 +259,111 @@ class IloCommonMethodsTestCase(base.TestCase):
 
     @mock.patch.object(ilo_common, 'get_ilo_object')
     def test_set_boot_device(self, get_ilo_object_mock):
-        ilo_object_mock = mock.MagicMock()
+        ilo_object_mock = get_ilo_object_mock.return_value
-        get_ilo_object_mock.return_value = ilo_object_mock
         ilo_common.set_boot_device(self.node, 'CDROM')
         get_ilo_object_mock.assert_called_once_with(self.node)
         ilo_object_mock.set_one_time_boot.assert_called_once_with('CDROM')
 
+    @mock.patch.object(ilo_common, 'get_ilo_object')
+    def test_set_boot_mode(self, get_ilo_object_mock):
+        ilo_object_mock = get_ilo_object_mock.return_value
+        get_pending_boot_mode_mock = ilo_object_mock.get_pending_boot_mode
+        set_pending_boot_mode_mock = ilo_object_mock.set_pending_boot_mode
+        get_pending_boot_mode_mock.return_value = 'LEGACY'
+        ilo_common.set_boot_mode(self.node, 'uefi')
+        get_ilo_object_mock.assert_called_once_with(self.node)
+        get_pending_boot_mode_mock.assert_called_once_with()
+        set_pending_boot_mode_mock.assert_called_once_with('UEFI')
+
+    @mock.patch.object(ilo_common, 'get_ilo_object')
+    def test_set_boot_mode_without_set_pending_boot_mode(self,
+                                                         get_ilo_object_mock):
+        ilo_object_mock = get_ilo_object_mock.return_value
+        get_pending_boot_mode_mock = ilo_object_mock.get_pending_boot_mode
+        get_pending_boot_mode_mock.return_value = 'LEGACY'
+        ilo_common.set_boot_mode(self.node, 'bios')
+        get_ilo_object_mock.assert_called_once_with(self.node)
+        get_pending_boot_mode_mock.assert_called_once_with()
+        self.assertFalse(ilo_object_mock.set_pending_boot_mode.called)
+
+    @mock.patch.object(ilo_common, 'ilo_client')
+    @mock.patch.object(ilo_common, 'get_ilo_object')
+    def test_set_boot_mode_with_IloOperationError(self,
+                                                  get_ilo_object_mock,
+                                                  ilo_client_mock):
+        ilo_object_mock = get_ilo_object_mock.return_value
+        get_pending_boot_mode_mock = ilo_object_mock.get_pending_boot_mode
+        get_pending_boot_mode_mock.return_value = 'UEFI'
+        set_pending_boot_mode_mock = ilo_object_mock.set_pending_boot_mode
+        ilo_client_mock.IloError = Exception
+        set_pending_boot_mode_mock.side_effect = Exception
+        self.assertRaises(exception.IloOperationError,
+                          ilo_common.set_boot_mode, self.node, 'bios')
+        get_ilo_object_mock.assert_called_once_with(self.node)
+        get_pending_boot_mode_mock.assert_called_once_with()
+
+    @mock.patch.object(driver_utils, 'rm_node_capability')
+    @mock.patch.object(driver_utils, 'add_node_capability')
+    @mock.patch.object(ilo_common, 'get_ilo_object')
+    @mock.patch.object(ilo_common, 'ilo_client')
+    def test_update_boot_mode_capability(self, ilo_client_mock,
+                                         get_ilo_object_mock,
+                                         add_node_capability_mock,
+                                         rm_node_capability_mock):
+        ilo_client_mock.IloCommandNotSupportedError = Exception
+        ilo_mock_obj = get_ilo_object_mock.return_value
+        ilo_mock_obj.get_pending_boot_mode.return_value = 'legacy'
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            ilo_common.update_boot_mode_capability(task)
+            get_ilo_object_mock.assert_called_once_with(task.node)
+            ilo_mock_obj.get_pending_boot_mode.assert_called_once_with()
+            rm_node_capability_mock.assert_called_once_with(task, 'boot_mode')
+            add_node_capability_mock.assert_called_once_with(task,
+                                                             'boot_mode',
+                                                             'bios')
+
+    @mock.patch.object(driver_utils, 'add_node_capability')
+    @mock.patch.object(ilo_common, 'get_ilo_object')
+    @mock.patch.object(ilo_common, 'ilo_client')
+    def test_update_boot_mode_capability_unknown(self, ilo_client_mock,
+                                         get_ilo_object_mock,
+                                         add_node_capability_mock):
+        ilo_client_mock.IloCommandNotSupportedError = Exception
+        ilo_mock_obj = get_ilo_object_mock.return_value
+        ilo_mock_obj.get_pending_boot_mode.return_value = 'UNKNOWN'
+        set_pending_boot_mode_mock = ilo_mock_obj.set_pending_boot_mode
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            ilo_common.update_boot_mode_capability(task)
+            get_ilo_object_mock.assert_called_once_with(task.node)
+            ilo_mock_obj.get_pending_boot_mode.assert_called_once_with()
+            set_pending_boot_mode_mock.assert_called_once_with('UEFI')
+            add_node_capability_mock.assert_called_once_with(task,
+                                                             'boot_mode',
+                                                             'uefi')
+
+    @mock.patch.object(driver_utils, 'add_node_capability')
+    @mock.patch.object(ilo_common, 'get_ilo_object')
+    @mock.patch.object(ilo_common, 'ilo_client')
+    def test_update_boot_mode_capability_legacy(self, ilo_client_mock,
+                                                get_ilo_object_mock,
+                                                add_node_capability_mock):
+        ilo_client_mock.IloCommandNotSupportedError = Exception
+        ilo_mock_obj = get_ilo_object_mock.return_value
+        ilo_mock_obj.get_pending_boot_mode.side_effect = Exception
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            ilo_common.update_boot_mode_capability(task)
+            get_ilo_object_mock.assert_called_once_with(task.node)
+            ilo_mock_obj.get_pending_boot_mode.assert_called_once_with()
+            add_node_capability_mock.assert_called_once_with(task,
+                                                             'boot_mode',
+                                                             'bios')
+
     @mock.patch.object(images, 'get_temp_url_for_glance_image')
     @mock.patch.object(ilo_common, 'attach_vmedia')
     @mock.patch.object(ilo_common, '_prepare_floppy_image')

ironic/tests/drivers/ilo/test_deploy.py

@@ -32,6 +32,7 @@ from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules.ilo import common as ilo_common
 from ironic.drivers.modules.ilo import deploy as ilo_deploy
 from ironic.drivers.modules import iscsi_deploy
+from ironic.drivers import utils as driver_utils
 from ironic.openstack.common import context
 from ironic.openstack.common import importutils
 from ironic.tests import base
@@ -77,6 +78,24 @@ class IloDeployPrivateMethodsTestCase(base.TestCase):
             boot_iso_expected = 'glance:boot-iso-uuid'
             self.assertEqual(boot_iso_expected, boot_iso_actual)
 
+    @mock.patch.object(driver_utils, 'get_node_capability')
+    @mock.patch.object(images, 'get_glance_image_property')
+    @mock.patch.object(ilo_deploy, '_parse_deploy_info')
+    def test__get_boot_iso_uefi_no_glance_image(self, deploy_info_mock,
+            image_prop_mock, get_node_cap_mock):
+        deploy_info_mock.return_value = {'image_source': 'image-uuid'}
+        image_prop_mock.return_value = None
+        get_node_cap_mock.return_value = 'uefi'
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            boot_iso_result = ilo_deploy._get_boot_iso(task, 'root-uuid')
+            deploy_info_mock.assert_called_once_with(task.node)
+            image_prop_mock.assert_called_once_with(task.context, 'image-uuid',
+                'boot_iso')
+            get_node_cap_mock.assert_called_once_with(task.node, 'boot_mode')
+            self.assertIsNone(boot_iso_result)
+
     @mock.patch.object(tempfile, 'NamedTemporaryFile')
     @mock.patch.object(images, 'create_boot_iso')
     @mock.patch.object(swift, 'SwiftAPI')
@@ -185,11 +204,12 @@ class IloVirtualMediaIscsiDeployTestCase(base.TestCase):
         self.node = obj_utils.create_test_node(self.context,
                 driver='iscsi_ilo', driver_info=INFO_DICT)
 
+    @mock.patch.object(driver_utils, 'validate_boot_mode_capability')
     @mock.patch.object(iscsi_deploy, 'validate_glance_image_properties')
     @mock.patch.object(ilo_deploy, '_parse_deploy_info')
     @mock.patch.object(iscsi_deploy, 'validate')
     def test_validate(self, validate_mock, deploy_info_mock,
-                      validate_prop_mock):
+                      validate_prop_mock, validate_boot_mode_mock):
         d_info = {'a': 'b'}
         deploy_info_mock.return_value = d_info
         with task_manager.acquire(self.context, self.node.uuid,
@@ -199,6 +219,7 @@ class IloVirtualMediaIscsiDeployTestCase(base.TestCase):
             deploy_info_mock.assert_called_once_with(task.node)
             validate_prop_mock.assert_called_once_with(task.context,
                     d_info, ['kernel_id', 'ramdisk_id'])
+            validate_boot_mode_mock.assert_called_once_with(task.node)
 
     @mock.patch.object(ilo_deploy, '_reboot_into')
     @mock.patch.object(ilo_deploy, '_get_single_nic_with_vif_port_id')

ironic/tests/drivers/test_utils.py

@@ -129,3 +129,77 @@ class UtilsTestCase(base.TestCase):
         with task_manager.acquire(self.context, self.node.uuid) as task:
             node_macs = driver_utils.get_node_mac_addresses(task)
         self.assertEqual(sorted([p.address for p in ports]), sorted(node_macs))
+
+    def test_get_node_capability(self):
+        properties = {'capabilities': 'cap1:value1,cap2:value2'}
+        self.node.properties = properties
+        expected = 'value1'
+
+        result = driver_utils.get_node_capability(self.node, 'cap1')
+        self.assertEqual(expected, result)
+
+    def test_get_node_capability_returns_none(self):
+        properties = {'capabilities': 'cap1:value1,cap2:value2'}
+        self.node.properties = properties
+
+        result = driver_utils.get_node_capability(self.node, 'capX')
+        self.assertIsNone(result)
+
+    def test_add_node_capability(self):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.properties['capabilities'] = ''
+            driver_utils.add_node_capability(task, 'boot_mode', 'bios')
+            self.assertEqual('boot_mode:bios',
+                             task.node.properties['capabilities'])
+
+    def test_add_node_capability_append(self):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.properties['capabilities'] = 'a:b,c:d'
+            driver_utils.add_node_capability(task, 'boot_mode', 'bios')
+            self.assertEqual('a:b,c:d,boot_mode:bios',
+                             task.node.properties['capabilities'])
+
+    def test_add_node_capability_append_duplicate(self):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.properties['capabilities'] = 'a:b,c:d'
+            driver_utils.add_node_capability(task, 'a', 'b')
+            self.assertEqual('a:b,c:d,a:b',
+                             task.node.properties['capabilities'])
+
+    def test_rm_node_capability(self):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.properties['capabilities'] = 'a:b'
+            driver_utils.rm_node_capability(task, 'a')
+            self.assertIsNone(task.node.properties['capabilities'])
+
+    def test_rm_node_capability_exists(self):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.properties['capabilities'] = 'a:b,c:d,x:y'
+            self.assertIsNone(driver_utils.rm_node_capability(task, 'c'))
+            self.assertEqual('a:b,x:y', task.node.properties['capabilities'])
+
+    def test_rm_node_capability_non_existent(self):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.properties['capabilities'] = 'a:b'
+            self.assertIsNone(driver_utils.rm_node_capability(task, 'x'))
+            self.assertEqual('a:b', task.node.properties['capabilities'])
+
+    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)

ironic/tests/test_pxe_utils.py

@@ -19,7 +19,6 @@ import os
 import mock
 from oslo.config import cfg
 
-from ironic.common import exception
 from ironic.common import pxe_utils
 from ironic.conductor import task_manager
 from ironic.db import api as dbapi
@@ -275,35 +274,6 @@ class TestPXEUtils(db_base.DbTestCase):
             self.assertEqual(sorted(expected_info),
                              sorted(pxe_utils.dhcp_options_for_instance(task)))
 
-    def test_get_node_capability(self):
-        properties = {'capabilities': 'cap1:value1,cap2:value2'}
-        self.node.properties = properties
-        expected = 'value1'
-
-        result = pxe_utils.get_node_capability(self.node, 'cap1')
-        self.assertEqual(expected, result)
-
-    def test_get_node_capability_returns_none(self):
-        properties = {'capabilities': 'cap1:value1,cap2:value2'}
-        self.node.properties = properties
-
-        result = pxe_utils.get_node_capability(self.node, 'capX')
-        self.assertIsNone(result)
-
-    def test_validate_boot_mode_capability(self):
-        properties = {'capabilities': 'boot_mode:uefi,cap2:value2'}
-        self.node.properties = properties
-
-        result = pxe_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,
-                          pxe_utils.validate_boot_mode_capability, self.node)
-
     @mock.patch('ironic.common.utils.rmtree_without_raise', autospec=True)
     @mock.patch('ironic.common.utils.unlink_without_raise', autospec=True)
     @mock.patch('ironic.common.dhcp_factory.DHCPFactory.provider')