Merge "Clear ilo_boot_iso before deploy for glance images"

doc/source/drivers/ilo.rst

@@ -40,9 +40,9 @@ Prerequisites
   managing HP Proliant hardware.
 
   Install ``proliantutils`` [2]_ module on the Ironic conductor node. Minimum
-  version required is 2.1.0.::
+  version required is 2.1.1.::
 
-   $ pip install "proliantutils>=2.1.0"
+   $ pip install "proliantutils>=2.1.1"
 
 * ``ipmitool`` command must be present on the service node(s) where
   ``ironic-conductor`` is running. On most distros, this is provided as part

driver-requirements.txt

@@ -4,7 +4,7 @@
 # python projects they should package as optional dependencies for Ironic.
 
 # These are available on pypi
-proliantutils>=2.1.0
+proliantutils>=2.1.1
 pyghmi
 pysnmp
 python-ironic-inspector-client

ironic/drivers/modules/ilo/common.py

@@ -414,6 +414,29 @@ def setup_vmedia_for_boot(task, boot_iso, parameters=None):
     attach_vmedia(task.node, 'CDROM', boot_iso_url or boot_iso)
 
 
+def eject_vmedia_devices(task):
+    """Ejects virtual media devices.
+
+    This method ejects virtual media devices. It ignores
+    any exception encountered in the process.
+
+    :param task: a TaskManager instance containing the node to act on.
+    :returns: None
+    """
+    ilo_object = get_ilo_object(task.node)
+    for device in ('FLOPPY', 'CDROM'):
+        try:
+            ilo_object.eject_virtual_media(device)
+        except ilo_error.IloError as ilo_exception:
+            LOG.error(_LE("Error while ejecting virtual media %(device)s "
+                          "from node %(uuid)s. Error: %(error)s"),
+                      {'device': device, 'uuid': task.node.uuid,
+                       'error': ilo_exception})
+            operation = _("Eject virtual media %s") % device.lower()
+            raise exception.IloOperationError(operation=operation,
+                                              error=ilo_exception)
+
+
 def cleanup_vmedia_boot(task):
     """Cleans a node after a virtual media boot.
 
@@ -435,16 +458,7 @@ def cleanup_vmedia_boot(task):
                           "%(container)s. Error: %(error)s"),
                       {'object_name': object_name, 'container': container,
                        'error': e})
-
+    eject_vmedia_devices(task)
-    ilo_object = get_ilo_object(task.node)
-    for device in ('FLOPPY', 'CDROM'):
-        try:
-            ilo_object.eject_virtual_media(device)
-        except ilo_error.IloError as ilo_exception:
-            LOG.exception(_LE("Error while ejecting virtual media %(device)s "
-                              "from node %(uuid)s. Error: %(error)s"),
-                          {'device': device, 'uuid': task.node.uuid,
-                           'error': ilo_exception})
 
 
 def get_secure_boot_mode(task):

ironic/drivers/modules/ilo/deploy.py

@@ -273,7 +273,10 @@ def _reboot_into(task, iso, ramdisk_options):
 
 
 def _prepare_agent_vmedia_boot(task):
-    """prepare for vmedia boot."""
+    """Ejects virtual media devices and prepares for vmedia boot."""
+    # Eject all virtual media devices are we are going to use them
+    # during deploy.
+    ilo_common.eject_vmedia_devices(task)
 
     deploy_ramdisk_opts = agent.build_agent_options(task.node)
     deploy_iso = task.node.driver_info['ilo_deploy_iso']
@@ -411,6 +414,20 @@ class IloVirtualMediaIscsiDeploy(base.DeployInterface):
         """
         node = task.node
 
+        # Clear ilo_boot_iso if it's a glance image to force recreate
+        # another one again (or use existing one in glance).
+        # This is mainly for rebuild scenario.
+        if service_utils.is_glance_image(
+                node.instance_info.get('image_source')):
+            instance_info = node.instance_info
+            instance_info.pop('ilo_boot_iso', None)
+            node.instance_info = instance_info
+            node.save()
+
+        # Eject all virtual media devices are we are going to use them
+        # during deploy.
+        ilo_common.eject_vmedia_devices(task)
+
         iscsi_deploy.cache_instance_image(task.context, node)
         iscsi_deploy.check_image_size(task)
 
@@ -503,6 +520,7 @@ class IloVirtualMediaAgentDeploy(base.DeployInterface):
             image.
         :raises: IloOperationError, if some operation on iLO fails.
         """
+
         _prepare_agent_vmedia_boot(task)
 
         return states.DEPLOYWAIT

ironic/tests/drivers/ilo/test_common.py

@@ -426,18 +426,16 @@ class IloCommonMethodsTestCase(db_base.DbTestCase):
             attach_vmedia_mock.assert_called_once_with(task.node, 'CDROM',
                                                        boot_iso)
 
-    @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
+    @mock.patch.object(ilo_common, 'eject_vmedia_devices',
-                       autospec=True)
+                       spec_set=True, autospec=True)
     @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True)
     @mock.patch.object(ilo_common, '_get_floppy_image_name', spec_set=True,
                        autospec=True)
     def test_cleanup_vmedia_boot(self, get_name_mock, swift_api_mock,
-                                 get_ilo_object_mock):
+                                 eject_mock):
         swift_obj_mock = swift_api_mock.return_value
         CONF.ilo.swift_ilo_container = 'ilo_cont'
 
-        ilo_object_mock = mock.MagicMock(spec=['eject_virtual_media'])
-        get_ilo_object_mock.return_value = ilo_object_mock
         get_name_mock.return_value = 'image-node-uuid'
 
         with task_manager.acquire(self.context, self.node.uuid,
@@ -445,9 +443,36 @@ class IloCommonMethodsTestCase(db_base.DbTestCase):
             ilo_common.cleanup_vmedia_boot(task)
             swift_obj_mock.delete_object.assert_called_once_with(
                 'ilo_cont', 'image-node-uuid')
+            eject_mock.assert_called_once_with(task)
+
+    @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
+                       autospec=True)
+    def test_eject_vmedia_devices(self, get_ilo_object_mock):
+        ilo_object_mock = mock.MagicMock(spec=['eject_virtual_media'])
+        get_ilo_object_mock.return_value = ilo_object_mock
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            ilo_common.eject_vmedia_devices(task)
             ilo_object_mock.eject_virtual_media.assert_any_call('CDROM')
             ilo_object_mock.eject_virtual_media.assert_any_call('FLOPPY')
 
+    @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
+                       autospec=True)
+    def test_eject_vmedia_devices_raises(
+            self, get_ilo_object_mock):
+        ilo_object_mock = mock.MagicMock(spec=['eject_virtual_media'])
+        get_ilo_object_mock.return_value = ilo_object_mock
+        exc = ilo_error.IloError('error')
+        ilo_object_mock.eject_virtual_media.side_effect = exc
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            self.assertRaises(exception.IloOperationError,
+                              ilo_common.eject_vmedia_devices,
+                              task)
+
+            ilo_object_mock.eject_virtual_media.assert_called_once_with(
+                'FLOPPY')
+
     @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
                        autospec=True)
     def test_get_secure_boot_mode(self,

ironic/tests/drivers/ilo/test_deploy.py

@@ -283,18 +283,23 @@ class IloDeployPrivateMethodsTestCase(db_base.DbTestCase):
                                                          boot_devices.CDROM)
             node_power_action_mock.assert_called_once_with(task, states.REBOOT)
 
+    @mock.patch.object(ilo_common, 'eject_vmedia_devices',
+                       spec_set=True, autospec=True)
     @mock.patch.object(ilo_deploy, '_reboot_into', spec_set=True,
                        autospec=True)
     @mock.patch.object(agent, 'build_agent_options', spec_set=True,
                        autospec=True)
     def test__prepare_agent_vmedia_boot(self, build_options_mock,
-                                        reboot_into_mock):
+                                        reboot_into_mock, eject_mock):
         deploy_opts = {'a': 'b'}
         build_options_mock.return_value = deploy_opts
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.node.driver_info['ilo_deploy_iso'] = 'deploy-iso-uuid'
+
             ilo_deploy._prepare_agent_vmedia_boot(task)
+
+            eject_mock.assert_called_once_with(task)
             build_options_mock.assert_called_once_with(task.node)
             reboot_into_mock.assert_called_once_with(task,
                                                      'deploy-iso-uuid',
@@ -565,6 +570,8 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase):
         is_glance_image_mock.return_value = False
         self._test_validate(props_expected=['kernel', 'ramdisk'])
 
+    @mock.patch.object(ilo_common, 'eject_vmedia_devices',
+                       spec_set=True, autospec=True)
     @mock.patch.object(ilo_deploy, '_reboot_into', spec_set=True,
                        autospec=True)
     @mock.patch.object(deploy_utils, 'get_single_nic_with_vif_port_id',
@@ -577,13 +584,23 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase):
                        autospec=True)
     @mock.patch.object(iscsi_deploy, 'cache_instance_image', spec_set=True,
                        autospec=True)
-    def test_deploy(self,
+    def _test_deploy(self,
-                    cache_instance_image_mock,
+                     cache_instance_image_mock,
-                    check_image_size_mock,
+                     check_image_size_mock,
-                    build_opts_mock,
+                     build_opts_mock,
-                    agent_options_mock,
+                     agent_options_mock,
-                    get_nic_mock,
+                     get_nic_mock,
-                    reboot_into_mock):
+                     reboot_into_mock,
+                     eject_mock,
+                     ilo_boot_iso,
+                     image_source
+                     ):
+        instance_info = self.node.instance_info
+        instance_info['ilo_boot_iso'] = ilo_boot_iso
+        instance_info['image_source'] = image_source
+        self.node.instance_info = instance_info
+        self.node.save()
+
         deploy_opts = {'a': 'b'}
         agent_options_mock.return_value = {
             'ipa-api-url': 'http://1.2.3.4:6385'}
@@ -595,6 +612,7 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase):
             task.node.driver_info['ilo_deploy_iso'] = 'deploy-iso'
             returned_state = task.driver.deploy.deploy(task)
 
+            eject_mock.assert_called_once_with(task)
             cache_instance_image_mock.assert_called_once_with(task.context,
                                                               task.node)
             check_image_size_mock.assert_called_once_with(task)
@@ -607,6 +625,21 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase):
 
         self.assertEqual(states.DEPLOYWAIT, returned_state)
 
+    def test_deploy_glance_image(self):
+        self._test_deploy(
+            ilo_boot_iso='swift:abcdef',
+            image_source='6b2f0c0c-79e8-4db6-842e-43c9764204af')
+        self.node.refresh()
+        self.assertNotIn('ilo_boot_iso', self.node.instance_info)
+
+    def test_deploy_not_a_glance_image(self):
+        self._test_deploy(
+            ilo_boot_iso='http://mybootiso',
+            image_source='http://myimage')
+        self.node.refresh()
+        self.assertEqual('http://mybootiso',
+                         self.node.instance_info['ilo_boot_iso'])
+
     @mock.patch.object(ilo_deploy, '_update_secure_boot_mode', spec_set=True,
                        autospec=True)
     @mock.patch.object(manager_utils, 'node_power_action', spec_set=True,