Follow-up patch to ramdisk interface

* Breaks up and updates a few tests
* Switches some checks to be capability based
* Add decoratores to ramdisk deploy interface
* Switches ramdisk deploy interface to use deploy_steps.

Change-Id: Ief730a8bd9750a9ed6828506c3642f4dd7246c46

ironic/drivers/base.py

@@ -457,6 +457,7 @@ class DeployInterface(BaseInterface):
 class BootInterface(BaseInterface):
     """Interface for boot-related actions."""
     interface_type = 'boot'
+    capabilities = []
 
     @abc.abstractmethod
     def prepare_ramdisk(self, task, ramdisk_params):

ironic/drivers/modules/deploy_utils.py

@@ -370,7 +370,7 @@ def deploy_partition_image(
         partition table has not changed).
     :param configdrive: Optional. Base64 encoded Gzipped configdrive content
                         or configdrive HTTP URL.
-    :param boot_option: Can be "local" or "netboot", or "ramdisk".
+    :param boot_option: Can be "local" or "netboot".
                         "netboot" by default.
     :param boot_mode: Can be "bios" or "uefi". "bios" by default.
     :param disk_label: The disk label to be used when creating the

ironic/drivers/modules/pxe.py

@@ -32,6 +32,7 @@ from ironic.common import image_service as service
 from ironic.common import images
 from ironic.common import pxe_utils
 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 import base
@@ -212,17 +213,11 @@ def _build_instance_pxe_options(task, pxe_info):
     pxe_opts.setdefault('aki_path', 'no_kernel')
     pxe_opts.setdefault('ari_path', 'no_ramdisk')
 
-    # TODO(TheJulia): We should only do this if we have a ramdisk interface.
+    i_info = task.node.instance_info
-    # We should check the capabilities of the class, but that becomes a bit
+    try:
-    # of a pain for unit testing. We can sort this out in Stein since we will
+        pxe_opts['ramdisk_opts'] = i_info['ramdisk_kernel_arguments']
-    # need to revisit a major portion of this file to effetively begin the
+    except KeyError:
-    # ipxe boot interface promotion.
+        pass
-    if isinstance(task.driver.deploy, PXERamdiskDeploy):
-        i_info = task.node.instance_info
-        try:
-            pxe_opts['ramdisk_opts'] = i_info['ramdisk_kernel_arguments']
-        except KeyError:
-            pass
 
     return pxe_opts
 
@@ -737,24 +732,19 @@ class PXERamdiskDeploy(agent.AgentDeploy, agent.AgentDeployMixin,
                        base.DeployInterface):
 
     def validate(self, task):
-        # Initially this is likely okay, we can iterate on this and
-        # enable other drivers that have similar functionality that
-        # be invoked in a ramdisk friendly way.
-        if not isinstance(task.driver.boot, PXEBoot):
-            raise exception.InvalidParameterValue(
-                err=('Invalid configuration: The ramdisk deploy '
-                     'interface requires the pxe boot interface.'))
-        # Eventually we should be doing this.
         if 'ramdisk_boot' not in task.driver.boot.capabilities:
             raise exception.InvalidParameterValue(
                 err=('Invalid configuration: The boot interface '
                      'must have the `ramdisk_boot` capability. '
-                     'Not found.'))
+                     'You are using an incompatible boot interface.'))
         task.driver.boot.validate(task)
 
         # Validate node capabilities
         deploy_utils.validate_capabilities(task.node)
 
+    @METRICS.timer('RamdiskDeploy.deploy')
+    @base.deploy_step(priority=100)
+    @task_manager.require_exclusive_lock
     def deploy(self, task):
         if 'configdrive' in task.node.instance_info:
             LOG.warning('A configuration drive is present with '
@@ -777,18 +767,21 @@ class PXERamdiskDeploy(agent.AgentDeploy, agent.AgentDeployMixin,
         # Power-on the instance, with PXE prepared, we're done.
         manager_utils.node_power_action(task, states.POWER_ON)
         LOG.info('Deployment setup for node %s done', task.node.uuid)
-        # TODO(TheJulia): Update this in stein to support deploy steps.
+        return None
-        return states.DEPLOYDONE
 
+    @METRICS.timer('RamdiskDeploy.prepare')
+    @task_manager.require_exclusive_lock
     def prepare(self, task):
         node = task.node
         # Log a warning if the boot_option is wrong... and
         # otherwise reset it.
-        if deploy_utils.get_boot_option(node) != 'ramdisk':
+        boot_option = deploy_utils.get_boot_option(node)
+        if boot_option != 'ramdisk':
             LOG.warning('Incorrect "boot_option" set for node %(node)s '
-                        'and will be overridden to "ramdisk" as the '
+                        'and will be overridden to "ramdisk" as to '
-                        'to match the deploy interface.',
+                        'match the deploy interface. Found: %(boot_opt)s.',
-                        {'node': node.uuid})
+                        {'node': node.uuid,
+                         'boot_opt': boot_option})
             i_info = task.node.instance_info
             i_info.update({'capabilities': {'boot_option': 'ramdisk'}})
             node.instance_info = i_info

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

@@ -1467,12 +1467,10 @@ class PXEBootTestCase(db_base.DbTestCase):
                 task.node, task.context)
 
 
-class PXEBootDeployTestCase(db_base.DbTestCase):
+class PXERamdiskDeployTestCase(db_base.DbTestCase):
-
-    driver = 'fake-hardware'
 
     def setUp(self):
-        super(PXEBootDeployTestCase, self).setUp()
+        super(PXERamdiskDeployTestCase, self).setUp()
         self.temp_dir = tempfile.mkdtemp()
         self.config(tftp_root=self.temp_dir, group='pxe')
         self.temp_dir = tempfile.mkdtemp()
@@ -1490,17 +1488,16 @@ class PXEBootDeployTestCase(db_base.DbTestCase):
             config_kwarg = {'enabled_%s_interfaces' % iface: [impl],
                             'default_%s_interface' % iface: impl}
             self.config(**config_kwarg)
-        self.config(enabled_hardware_types=[self.driver])
+        self.config(enabled_hardware_types=['fake-hardware'])
         instance_info = INST_INFO_DICT
         self.node = obj_utils.create_test_node(
             self.context,
-            driver=self.driver,
+            driver='fake-hardware',
             instance_info=instance_info,
             driver_info=DRV_INFO_DICT,
             driver_internal_info=DRV_INTERNAL_INFO_DICT)
         self.port = obj_utils.create_test_port(self.context,
                                                node_id=self.node.id)
-        self.config(group='conductor', api_url='http://127.0.0.1:1234/')
 
     @mock.patch.object(manager_utils, 'node_set_boot_device', autospec=True)
     @mock.patch.object(deploy_utils, 'switch_pxe_config', autospec=True)
@@ -1553,8 +1550,7 @@ class PXEBootDeployTestCase(db_base.DbTestCase):
         self.node.instance_info = i_info
         self.node.save()
         with task_manager.acquire(self.context, self.node.uuid) as task:
-            self.assertEqual(states.DEPLOYDONE,
+            self.assertIsNone(task.driver.deploy.deploy(task))
-                             task.driver.deploy.deploy(task))
             mock_image_info.assert_called_once_with(
                 task.node, task.context)
             mock_cache.assert_called_once_with(
@@ -1565,8 +1561,7 @@ class PXEBootDeployTestCase(db_base.DbTestCase):
         self.node.save()
         mock_warning.reset_mock()
         with task_manager.acquire(self.context, self.node.uuid) as task:
-            self.assertEqual(states.DEPLOYDONE,
+            self.assertIsNone(task.driver.deploy.deploy(task))
-                             task.driver.deploy.deploy(task))
             self.assertTrue(mock_warning.called)
 
     @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True)
@@ -1581,13 +1576,18 @@ class PXEBootDeployTestCase(db_base.DbTestCase):
             self.assertEqual({'boot_option': 'ramdisk'},
                              task.node.instance_info['capabilities'])
 
+    @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True)
+    def test_prepare_active(self, mock_prepare_instance):
+        node = self.node
         node.provision_state = states.ACTIVE
         node.save()
         with task_manager.acquire(self.context, node.uuid) as task:
             task.driver.deploy.prepare(task)
             mock_prepare_instance.assert_called_once_with(mock.ANY, task)
-        mock_prepare_instance.reset_mock()
 
+    @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True)
+    def test_prepare_unrescuing(self, mock_prepare_instance):
+        node = self.node
         node.provision_state = states.UNRESCUING
         node.save()
         with task_manager.acquire(self.context, node.uuid) as task:
@@ -1630,7 +1630,7 @@ class PXEBootDeployTestCase(db_base.DbTestCase):
                     default_boot_interface='fake')
         with task_manager.acquire(self.context, node.uuid) as task:
             self.assertRaisesRegexp(exception.InvalidParameterValue,
-                                    'requires the pxe boot interface',
+                                    'must have the `ramdisk_boot` capability',
                                     task.driver.deploy.validate, task)
             self.assertFalse(mock_validate_image.called)