iLO firmware update fails with 'update_firmware_sum' clean step

Firmware update using 'update_firmware_sum' clean step fails with
an error stating that unable to connect to iLO address due to
authentication failure.
The 'update_firmware_sum' is an inband clean step wherein the
firmware ISO was inserted from IPA ramdisk. As part of security
fix to ironic the BMC credentials are no more passed to the IPA
ramdisk. The attempt to connect to iLO from IPA ramdisk fails.
The fix has been made to insert the firmware ISO from the
conductor.

Change-Id: I866330c5fc98b1c5bc042c296c3b6e76ed9fd57d
Story: 2006223
Task: 35821
(cherry picked from commit 50bfd9067fdeebcc89117ddab4fdd1573dba565e)

3 files changed, 29 insertions, 4 deletions

diff --git a/ironic/drivers/modules/ilo/management.py b/ironic/drivers/modules/ilo/management.py
index 91aa8f452..bebffc89a 100644
--- a/ironic/drivers/modules/ilo/management.py
+++ b/ironic/drivers/modules/ilo/management.py
@@ -472,6 +472,9 @@ class IloManagement(base.ManagementInterface):
             url = firmware_processor.get_swift_url(urlparse.urlparse(url))
             node.clean_step['args']['url'] = url
 
+        # Insert SPP ISO into virtual media CDROM
+        ilo_common.attach_vmedia(node, 'CDROM', url)
+
         step = node.clean_step
         return deploy_utils.agent_execute_clean_step(task, step)
 
diff --git a/ironic/tests/unit/drivers/modules/ilo/test_management.py b/ironic/tests/unit/drivers/modules/ilo/test_management.py
index 190a4b26e..8f818acc8 100644
--- a/ironic/tests/unit/drivers/modules/ilo/test_management.py
+++ b/ironic/tests/unit/drivers/modules/ilo/test_management.py
@@ -539,9 +539,12 @@ class IloManagementTestCase(test_common.BaseIloTest):
             remove_mock.assert_has_calls([mock.call(fw_loc_obj_1),
                                           mock.call(fw_loc_obj_2)])
 
+    @mock.patch.object(ilo_common, 'attach_vmedia', spec_set=True,
+                       autospec=True)
     @mock.patch.object(deploy_utils, 'agent_execute_clean_step',
                        autospec=True)
-    def test_update_firmware_sum_mode_with_component(self, execute_mock):
+    def test_update_firmware_sum_mode_with_component(
+            self, execute_mock, attach_vmedia_mock):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             execute_mock.return_value = states.CLEANWAIT
@@ -558,15 +561,19 @@ class IloManagementTestCase(test_common.BaseIloTest):
             return_value = task.driver.management.update_firmware_sum(
                 task, **firmware_update_args)
             # | THEN |
+            attach_vmedia_mock.assert_any_call(
+                task.node, 'CDROM', 'http://any_url')
             self.assertEqual(states.CLEANWAIT, return_value)
             execute_mock.assert_called_once_with(task, clean_step)
 
+    @mock.patch.object(ilo_common, 'attach_vmedia', spec_set=True,
+                       autospec=True)
     @mock.patch.object(ilo_management.firmware_processor,
                        'get_swift_url', autospec=True)
     @mock.patch.object(deploy_utils, 'agent_execute_clean_step',
                        autospec=True)
-    def test_update_firmware_sum_mode_swift_url(self, execute_mock,
-                                                swift_url_mock):
+    def test_update_firmware_sum_mode_swift_url(
+            self, execute_mock, swift_url_mock, attach_vmedia_mock):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             swift_url_mock.return_value = "http://path-to-file"
@@ -584,13 +591,18 @@ class IloManagementTestCase(test_common.BaseIloTest):
             return_value = task.driver.management.update_firmware_sum(
                 task, **firmware_update_args)
             # | THEN |
+            attach_vmedia_mock.assert_any_call(
+                task.node, 'CDROM', 'http://path-to-file')
             self.assertEqual(states.CLEANWAIT, return_value)
             self.assertEqual(task.node.clean_step['args']['url'],
                              "http://path-to-file")
 
+    @mock.patch.object(ilo_common, 'attach_vmedia', spec_set=True,
+                       autospec=True)
     @mock.patch.object(deploy_utils, 'agent_execute_clean_step',
                        autospec=True)
-    def test_update_firmware_sum_mode_without_component(self, execute_mock):
+    def test_update_firmware_sum_mode_without_component(
+            self, execute_mock, attach_vmedia_mock):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             execute_mock.return_value = states.CLEANWAIT
@@ -606,6 +618,8 @@ class IloManagementTestCase(test_common.BaseIloTest):
             return_value = task.driver.management.update_firmware_sum(
                 task, **firmware_update_args)
             # | THEN |
+            attach_vmedia_mock.assert_any_call(
+                task.node, 'CDROM', 'any_valid_url')
             self.assertEqual(states.CLEANWAIT, return_value)
             execute_mock.assert_called_once_with(task, clean_step)
 
diff --git a/releasenotes/notes/story-2006223-ilo-hpsum-firmware-update-fails-622883e4785313c1.yaml b/releasenotes/notes/story-2006223-ilo-hpsum-firmware-update-fails-622883e4785313c1.yaml
new file mode 100644
index 000000000..4a4183fe1
--- /dev/null
+++ b/releasenotes/notes/story-2006223-ilo-hpsum-firmware-update-fails-622883e4785313c1.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Fixes an issue in updating firmware using ``update_firmware_sum`` clean
+    step from management interface of ``ilo`` hardware type with an error
+    stating that unable to connect to iLO address due to authentication
+    failure. See `story 2006223
+    <https://storyboard.openstack.org/#!/story/2006223>`__ for details.