Remove check for UEFI + Whole disk images

This patch is removing a conditional preventing whole disk images from
being deployed in UEFI mode without local boot.

This conditional is an inconsistency because whole disk images being
deployed in non-UEFI mode don't need to explicitly set the boot_option
from "local", also, in Ironic when deploying a whole disk image we
already assume local boot by default.

Change-Id: I678e3547397eac3199d8ff670fe281e20b2cd2c0
Closes-Bug: #1627022

ironic/drivers/modules/ilo/deploy.py

@@ -32,7 +32,6 @@ from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules.ilo import boot as ilo_boot
 from ironic.drivers.modules.ilo import common as ilo_common
 from ironic.drivers.modules import iscsi_deploy
-from ironic.drivers.modules import pxe
 
 LOG = logging.getLogger(__name__)
 
@@ -366,16 +365,6 @@ class IloPXEDeploy(IloIscsiDeployMixin, iscsi_deploy.ISCSIDeploy):
         if task.node.provision_state == states.DEPLOYING:
             _prepare_node_for_deploy(task)
 
-            # Check if 'boot_option' is compatible with 'boot_mode' and image.
-            # Whole disk image deploy is not supported in UEFI boot mode if
-            # 'boot_option' is not 'local'.
-            # If boot_mode is not set in the node properties/capabilities then
-            # PXEDeploy.validate() would pass.
-            # Boot mode gets updated in prepare stage. It is possible that the
-            # deploy boot mode is 'uefi' after call to update_boot_mode().
-            # Hence a re-check is required here.
-            pxe.validate_boot_option_for_uefi(task.node)
-
         super(IloPXEDeploy, self).prepare(task)
 
     @METRICS.timer('IloPXEDeploy.deploy')

ironic/drivers/modules/pxe.py

@@ -26,7 +26,7 @@ from ironic.common import boot_devices
 from ironic.common import dhcp_factory
 from ironic.common import exception
 from ironic.common.glance_service import service_utils
-from ironic.common.i18n import _, _LE, _LW
+from ironic.common.i18n import _, _LW
 from ironic.common import image_service as service
 from ironic.common import images
 from ironic.common import pxe_utils
@@ -215,30 +215,6 @@ def _build_pxe_config_options(task, pxe_info):
     return pxe_options
 
 
-@METRICS.timer('validate_boot_option_for_uefi')
-def validate_boot_option_for_uefi(node):
-    """In uefi boot mode, validate if the boot option is compatible.
-
-    This method raises exception if whole disk image being deployed
-    in UEFI boot mode without 'boot_option' being set to 'local'.
-
-    :param node: a single Node.
-    :raises: InvalidParameterValue
-    """
-    boot_mode = deploy_utils.get_boot_mode_for_deploy(node)
-    boot_option = deploy_utils.get_boot_option(node)
-    if (boot_mode == 'uefi' and
-            node.driver_internal_info.get('is_whole_disk_image') and
-            boot_option != 'local'):
-        LOG.error(_LE("Whole disk image with netboot is not supported in UEFI "
-                      "boot mode."))
-        raise exception.InvalidParameterValue(_(
-            "Conflict: Whole disk image being used for deploy, but "
-            "cannot be used with node %(node_uuid)s configured to use "
-            "UEFI boot with netboot option") %
-            {'node_uuid': node.uuid})
-
-
 @METRICS.timer('validate_boot_option_for_trusted_boot')
 def validate_boot_parameters_for_trusted_boot(node):
     """Check if boot parameters are valid for trusted boot."""
@@ -333,9 +309,6 @@ class PXEBoot(base.BootInterface):
                 _("Node %s does not have any port associated with it.")
                 % node.uuid)
 
-        # Get the boot_mode capability value.
-        boot_mode = deploy_utils.get_boot_mode_for_deploy(node)
-
         if CONF.pxe.ipxe_enabled:
             if (not CONF.deploy.http_url or
                 not CONF.deploy.http_root):
@@ -343,9 +316,6 @@ class PXEBoot(base.BootInterface):
                     "iPXE boot is enabled but no HTTP URL or HTTP "
                     "root was specified."))
 
-        if boot_mode == 'uefi':
-            validate_boot_option_for_uefi(node)
-
         # Check the trusted_boot capabilities value.
         deploy_utils.validate_capabilities(node)
         if deploy_utils.is_trusted_boot_requested(node):

ironic/tests/unit/drivers/modules/ilo/test_deploy.py

@@ -25,6 +25,7 @@ from ironic.common import image_service
 from ironic.common import states
 from ironic.conductor import task_manager
 from ironic.conductor import utils as manager_utils
+from ironic.conf import CONF
 from ironic.drivers.modules import agent
 from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules.ilo import common as ilo_common
@@ -686,19 +687,18 @@ class IloPXEDeployTestCase(db_base.DbTestCase):
                        autospec=True)
     @mock.patch.object(ilo_deploy, '_prepare_node_for_deploy', spec_set=True,
                        autospec=True)
-    def test_prepare_uefi_whole_disk_image_fail(self,
-                                                prepare_node_for_deploy_mock,
-                                                pxe_prepare_mock):
+    def test_prepare_whole_disk_image_uefi(self, prepare_node_for_deploy_mock,
+                                           pxe_prepare_mock):
+        CONF.set_override('default_boot_option', 'netboot', 'deploy')
         self.node.provision_state = states.DEPLOYING
         self.node.save()
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.node.properties['capabilities'] = 'boot_mode:uefi'
             task.node.driver_internal_info['is_whole_disk_image'] = True
-            self.assertRaises(exception.InvalidParameterValue,
-                              task.driver.deploy.prepare, task)
+            task.driver.deploy.prepare(task)
             prepare_node_for_deploy_mock.assert_called_once_with(task)
-            self.assertFalse(pxe_prepare_mock.called)
+            pxe_prepare_mock.assert_called_once_with(mock.ANY, task)
 
     @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'deploy', spec_set=True,
                        autospec=True)

ironic/tests/unit/drivers/modules/test_pxe.py

@@ -450,48 +450,6 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
                                                  list(fake_pxe_info.values()),
                                                  True)
 
-    @mock.patch.object(pxe.LOG, 'error', autospec=True)
-    def test_validate_boot_option_for_uefi_exc(self, mock_log):
-        properties = {'capabilities': 'boot_mode:uefi'}
-        instance_info = {"boot_option": "netboot"}
-        self.node.properties = properties
-        self.node.instance_info['capabilities'] = instance_info
-        self.node.driver_internal_info['is_whole_disk_image'] = True
-        self.assertRaises(exception.InvalidParameterValue,
-                          pxe.validate_boot_option_for_uefi,
-                          self.node)
-        self.assertTrue(mock_log.called)
-
-    @mock.patch.object(pxe.LOG, 'error', autospec=True)
-    def test_validate_boot_option_for_uefi_noexc_one(self, mock_log):
-        properties = {'capabilities': 'boot_mode:uefi'}
-        instance_info = {"boot_option": "local"}
-        self.node.properties = properties
-        self.node.instance_info['capabilities'] = instance_info
-        self.node.driver_internal_info['is_whole_disk_image'] = True
-        pxe.validate_boot_option_for_uefi(self.node)
-        self.assertFalse(mock_log.called)
-
-    @mock.patch.object(pxe.LOG, 'error', autospec=True)
-    def test_validate_boot_option_for_uefi_noexc_two(self, mock_log):
-        properties = {'capabilities': 'boot_mode:bios'}
-        instance_info = {"boot_option": "local"}
-        self.node.properties = properties
-        self.node.instance_info['capabilities'] = instance_info
-        self.node.driver_internal_info['is_whole_disk_image'] = True
-        pxe.validate_boot_option_for_uefi(self.node)
-        self.assertFalse(mock_log.called)
-
-    @mock.patch.object(pxe.LOG, 'error', autospec=True)
-    def test_validate_boot_option_for_uefi_noexc_three(self, mock_log):
-        properties = {'capabilities': 'boot_mode:uefi'}
-        instance_info = {"boot_option": "local"}
-        self.node.properties = properties
-        self.node.instance_info['capabilities'] = instance_info
-        self.node.driver_internal_info['is_whole_disk_image'] = False
-        pxe.validate_boot_option_for_uefi(self.node)
-        self.assertFalse(mock_log.called)
-
     @mock.patch.object(pxe.LOG, 'error', autospec=True)
     def test_validate_boot_parameters_for_trusted_boot_one(self, mock_log):
         properties = {'capabilities': 'boot_mode:uefi'}
@@ -636,17 +594,6 @@ class PXEBootTestCase(db_base.DbTestCase):
             self.assertRaises(exception.MissingParameterValue,
                               task.driver.boot.validate, task)
 
-    def test_validate_fail_invalid_config_uefi_whole_disk_image(self):
-        properties = {'capabilities': 'boot_mode:uefi,boot_option:netboot'}
-        instance_info = {"boot_option": "netboot"}
-        with task_manager.acquire(self.context, self.node.uuid,
-                                  shared=True) as task:
-            task.node.properties = properties
-            task.node.instance_info['capabilities'] = instance_info
-            task.node.driver_internal_info['is_whole_disk_image'] = True
-            self.assertRaises(exception.InvalidParameterValue,
-                              task.driver.boot.validate, task)
-
     def test_validate_fail_no_port(self):
         new_node = obj_utils.create_test_node(
             self.context,

releasenotes/notes/check-for-whole-disk-image-uefi-3bf2146588de2423.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - Remove a check that was preventing whole disk images to be deployed
+    in UEFI mode without explicitly setting the boot_option capability to
+    "local". For whole disk images Ironic already assumes booting from
+    the hard-drive by default.