From 8a32d84a122b472d3bd1460144cc68a2bd0e85f3 Mon Sep 17 00:00:00 2001 From: Shivanand Tendulker Date: Tue, 16 Jul 2019 02:16:25 -0400 Subject: [PATCH] iLO firmware update fails with 'update_firmware_sum' Firmware update using 'update_firmware_sum' is an inband method wherein it tries to insert HPSUM based firmware ISO into virtual media. It errors out as Ironic has stopped passing the iLO credentials to the deploy ramdisk for security reasons. The fix has been made to insert the HPSUM firmware ISO from the ironic conductor. Change-Id: I6f11e3b749b648d16078e6dcdae1c37ac70f7b4c --- proliantutils/sum/sum_controller.py | 11 ---- .../tests/sum/test_sum_controller.py | 61 ++----------------- 2 files changed, 5 insertions(+), 67 deletions(-) diff --git a/proliantutils/sum/sum_controller.py b/proliantutils/sum/sum_controller.py index 94b2e0de..5f05cd3c 100644 --- a/proliantutils/sum/sum_controller.py +++ b/proliantutils/sum/sum_controller.py @@ -26,7 +26,6 @@ from oslo_concurrency import processutils from oslo_serialization import base64 from proliantutils import exception -from proliantutils.ilo import client from proliantutils import utils @@ -178,16 +177,6 @@ def update_firmware(node): except exception.ImageRefValidationFailed as e: raise exception.SUMOperationError(reason=e) - # Ejects the CDROM device in the iLO and inserts the SUM update ISO - # to the CDROM device. - info = node.get('driver_info') - ilo_object = client.IloClient(info.get('ilo_address'), - info.get('ilo_username'), - info.get('ilo_password')) - - ilo_object.eject_virtual_media('CDROM') - ilo_object.insert_virtual_media(sum_update_iso, 'CDROM') - # Waits for the OS to detect the disk and update the label file. SPP ISO # is identified by matching its label. time.sleep(WAIT_TIME_DISK_LABEL_TO_BE_VISIBLE) diff --git a/proliantutils/tests/sum/test_sum_controller.py b/proliantutils/tests/sum/test_sum_controller.py index ecd3bc89..fe81e09f 100644 --- a/proliantutils/tests/sum/test_sum_controller.py +++ b/proliantutils/tests/sum/test_sum_controller.py @@ -23,7 +23,6 @@ from oslo_serialization import base64 import testtools from proliantutils import exception -from proliantutils.ilo import client as ilo_client from proliantutils.sum import sum_controller from proliantutils.tests.sum import sum_sample_output as constants from proliantutils import utils @@ -162,14 +161,10 @@ class SUMFirmwareUpdateTest(testtools.TestCase): @mock.patch.object(os.path, 'exists') @mock.patch.object(os, 'mkdir') @mock.patch.object(processutils, 'execute') - @mock.patch.object(ilo_client, 'IloClient', spec_set=True, autospec=True) - def test_update_firmware(self, client_mock, execute_mock, mkdir_mock, + def test_update_firmware(self, execute_mock, mkdir_mock, exists_mock, mkdtemp_mock, rmtree_mock, listdir_mock, execute_sum_mock, verify_image_mock, validate_mock): - ilo_mock_object = client_mock.return_value - eject_media_mock = ilo_mock_object.eject_virtual_media - insert_media_mock = ilo_mock_object.insert_virtual_media execute_sum_mock.return_value = 'SUCCESS' listdir_mock.return_value = ['SPP_LABEL'] mkdtemp_mock.return_value = "/tempdir" @@ -179,9 +174,6 @@ class SUMFirmwareUpdateTest(testtools.TestCase): ret_val = sum_controller.update_firmware(self.node) - eject_media_mock.assert_called_once_with('CDROM') - insert_media_mock.assert_called_once_with('http://1.2.3.4/SPP.iso', - 'CDROM') execute_mock.assert_any_call('mount', "/dev/disk/by-label/SPP_LABEL", "/tempdir") execute_sum_mock.assert_any_call('/tempdir/hp/swpackages/hpsum', @@ -204,14 +196,10 @@ class SUMFirmwareUpdateTest(testtools.TestCase): @mock.patch.object(os.path, 'exists') @mock.patch.object(os, 'mkdir') @mock.patch.object(processutils, 'execute') - @mock.patch.object(ilo_client, 'IloClient', spec_set=True, autospec=True) - def test_update_firmware_sum(self, client_mock, execute_mock, mkdir_mock, + def test_update_firmware_sum(self, execute_mock, mkdir_mock, exists_mock, mkdtemp_mock, rmtree_mock, listdir_mock, execute_sum_mock, verify_image_mock, validate_mock): - ilo_mock_object = client_mock.return_value - eject_media_mock = ilo_mock_object.eject_virtual_media - insert_media_mock = ilo_mock_object.insert_virtual_media execute_sum_mock.return_value = 'SUCCESS' listdir_mock.return_value = ['SPP_LABEL'] mkdtemp_mock.return_value = "/tempdir" @@ -221,9 +209,6 @@ class SUMFirmwareUpdateTest(testtools.TestCase): ret_val = sum_controller.update_firmware(self.node) - eject_media_mock.assert_called_once_with('CDROM') - insert_media_mock.assert_called_once_with('http://1.2.3.4/SPP.iso', - 'CDROM') execute_mock.assert_any_call('mount', "/dev/disk/by-label/SPP_LABEL", "/tempdir") execute_sum_mock.assert_any_call('/tempdir/packages/smartupdate', @@ -250,31 +235,12 @@ class SUMFirmwareUpdateTest(testtools.TestCase): sum_controller.update_firmware, self.node) self.assertIn(value, str(exc)) - @mock.patch.object(utils, 'validate_href') - @mock.patch.object(ilo_client, 'IloClient', spec_set=True, autospec=True) - def test_update_firmware_vmedia_attach_fails(self, client_mock, - validate_mock): - ilo_mock_object = client_mock.return_value - eject_media_mock = ilo_mock_object.eject_virtual_media - value = ("Unable to attach SUM SPP iso http://1.2.3.4/SPP.iso " - "to the iLO") - eject_media_mock.side_effect = exception.IloError(value) - - exc = self.assertRaises(exception.IloError, - sum_controller.update_firmware, self.node) - self.assertEqual(value, str(exc)) - @mock.patch.object(utils, 'validate_href') @mock.patch.object(os.path, 'exists') @mock.patch.object(os, 'listdir') - @mock.patch.object(ilo_client, 'IloClient', spec_set=True, autospec=True) - def test_update_firmware_device_file_not_found(self, client_mock, + def test_update_firmware_device_file_not_found(self, listdir_mock, exists_mock, validate_mock): - ilo_mock_object = client_mock.return_value - eject_media_mock = ilo_mock_object.eject_virtual_media - insert_media_mock = ilo_mock_object.insert_virtual_media - listdir_mock.return_value = ['SPP_LABEL'] exists_mock.return_value = False @@ -284,22 +250,15 @@ class SUMFirmwareUpdateTest(testtools.TestCase): exc = self.assertRaises(exception.SUMOperationError, sum_controller.update_firmware, self.node) self.assertEqual(msg, str(exc)) - eject_media_mock.assert_called_once_with('CDROM') - insert_media_mock.assert_called_once_with('http://1.2.3.4/SPP.iso', - 'CDROM') exists_mock.assert_called_once_with("/dev/disk/by-label/SPP_LABEL") @mock.patch.object(utils, 'validate_href') @mock.patch.object(utils, 'verify_image_checksum') @mock.patch.object(os, 'listdir') @mock.patch.object(os.path, 'exists') - @mock.patch.object(ilo_client, 'IloClient', spec_set=True, autospec=True) - def test_update_firmware_invalid_checksum(self, client_mock, exists_mock, + def test_update_firmware_invalid_checksum(self, exists_mock, listdir_mock, verify_image_mock, validate_mock): - ilo_mock_object = client_mock.return_value - eject_media_mock = ilo_mock_object.eject_virtual_media - insert_media_mock = ilo_mock_object.insert_virtual_media listdir_mock.return_value = ['SPP_LABEL'] exists_mock.side_effect = [True, False] @@ -315,9 +274,6 @@ class SUMFirmwareUpdateTest(testtools.TestCase): verify_image_mock.assert_called_once_with( '/dev/disk/by-label/SPP_LABEL', '1234567890') - eject_media_mock.assert_called_once_with('CDROM') - insert_media_mock.assert_called_once_with('http://1.2.3.4/SPP.iso', - 'CDROM') exists_mock.assert_called_once_with("/dev/disk/by-label/SPP_LABEL") @mock.patch.object(utils, 'validate_href') @@ -327,14 +283,10 @@ class SUMFirmwareUpdateTest(testtools.TestCase): @mock.patch.object(os, 'mkdir') @mock.patch.object(os.path, 'exists') @mock.patch.object(os, 'listdir') - @mock.patch.object(ilo_client, 'IloClient', spec_set=True, autospec=True) - def test_update_firmware_mount_fails(self, client_mock, listdir_mock, + def test_update_firmware_mount_fails(self, listdir_mock, exists_mock, mkdir_mock, mkdtemp_mock, execute_mock, verify_image_mock, validate_mock): - ilo_mock_object = client_mock.return_value - eject_media_mock = ilo_mock_object.eject_virtual_media - insert_media_mock = ilo_mock_object.insert_virtual_media listdir_mock.return_value = ['SPP_LABEL'] exists_mock.return_value = True mkdtemp_mock.return_value = "/tempdir" @@ -345,9 +297,6 @@ class SUMFirmwareUpdateTest(testtools.TestCase): exc = self.assertRaises(exception.SUMOperationError, sum_controller.update_firmware, self.node) self.assertIn(msg, str(exc)) - eject_media_mock.assert_called_once_with('CDROM') - insert_media_mock.assert_called_once_with('http://1.2.3.4/SPP.iso', - 'CDROM') exists_mock.assert_called_once_with("/dev/disk/by-label/SPP_LABEL") @mock.patch.object(sum_controller,