From 5847431e9cbf5705f5bf5b1b52d5cd50d27e6f87 Mon Sep 17 00:00:00 2001 From: Hugo Nicodemos Date: Fri, 1 Dec 2017 11:00:24 -0300 Subject: [PATCH] Fix persistent information when getting boot device This patches fixes the get_boot_device methods for ``oneview`` hardware type to consider onetime boot option on the machine's iLO. if onetime boot is set to 'Once' on iLO, the method will return persistent as False (not persistent), True otherwise (persistent). Change-Id: I48d8b01ff37263a19c889f242a7ce2a99eee597e Closes-Bug: #1706725 --- ironic/drivers/modules/oneview/management.py | 31 ++++++- .../modules/oneview/test_management.py | 80 +++++++++++++++---- ...evice-not-persistent-de6159d8d2b60656.yaml | 7 ++ 3 files changed, 100 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/fix-get-boot-device-not-persistent-de6159d8d2b60656.yaml diff --git a/ironic/drivers/modules/oneview/management.py b/ironic/drivers/modules/oneview/management.py index 42ea2af85e..1812cea630 100644 --- a/ironic/drivers/modules/oneview/management.py +++ b/ironic/drivers/modules/oneview/management.py @@ -48,6 +48,10 @@ BOOT_DEVICE_MAP_ILO = { BOOT_DEVICE_MAP_ILO_REV = { v: k for k, v in BOOT_DEVICE_MAP_ILO.items()} +ILO_SYSTEM_PATH = "/rest/v1/Systems/1" + +ILO_REQUEST_HEADERS = {"Content-Type": "application/json"} + def set_onetime_boot(task): """Set onetime boot to server hardware. @@ -71,16 +75,15 @@ def set_onetime_boot(task): server_hardware = task.node.driver_info.get('server_hardware_uri') ilo_client = common.get_ilorest_client(server_hardware) boot_device = BOOT_DEVICE_MAP_ILO.get(boot_device) - path = '/rest/v1/Systems/1' body = { "Boot": { "BootSourceOverrideTarget": boot_device, "BootSourceOverrideEnabled": "Once" } } - headers = {"Content-Type": "application/json"} try: - ilo_client.patch(path=path, body=body, headers=headers) + ilo_client.patch(path=ILO_SYSTEM_PATH, body=body, + headers=ILO_REQUEST_HEADERS) except Exception as e: msg = (_("Error while trying to set onetime boot on Server Hardware: " "%(sh_uri)s. Error: %(error)s") % @@ -88,6 +91,26 @@ def set_onetime_boot(task): raise exception.OneViewError(error=msg) +def _is_onetime_boot(task): + """Check onetime boot from server hardware. + + Check if the onetime boot option of an OneView server hardware + is set to 'Once' in iLO. + + :param task: a task from TaskManager. + :returns: Boolean value. True if onetime boot is 'Once' + False otherwise. + :raises: AttributeError if Boot is None. + """ + server_hardware = task.node.driver_info.get('server_hardware_uri') + ilo_client = common.get_ilorest_client(server_hardware) + response = ilo_client.get(path=ILO_SYSTEM_PATH, + headers=ILO_REQUEST_HEADERS) + boot = response.dict.get('Boot') + onetime_boot = boot.get('BootSourceOverrideEnabled') + return onetime_boot == 'Once' + + def set_boot_device(task): """Sets the boot device for a node. @@ -279,7 +302,7 @@ class OneViewManagement(base.ManagementInterface): boot_device = { 'boot_device': BOOT_DEVICE_MAP_ONEVIEW_REV.get(primary_device), - 'persistent': True, + 'persistent': not _is_onetime_boot(task) } return boot_device diff --git a/ironic/tests/unit/drivers/modules/oneview/test_management.py b/ironic/tests/unit/drivers/modules/oneview/test_management.py index e09deb0b89..940decb13a 100644 --- a/ironic/tests/unit/drivers/modules/oneview/test_management.py +++ b/ironic/tests/unit/drivers/modules/oneview/test_management.py @@ -79,14 +79,12 @@ class OneViewManagementDriverFunctionsTestCase(db_base.DbTestCase): client.server_profiles.get.return_value = server_profile boot_device_map_ilo = management.BOOT_DEVICE_MAP_ILO boot_device = boot_device_map_ilo.get(boot_devices.PXE) - path = '/rest/v1/Systems/1' body = { "Boot": { "BootSourceOverrideTarget": boot_device, "BootSourceOverrideEnabled": "Once" } } - headers = {"Content-Type": "application/json"} with task_manager.acquire(self.context, self.node.uuid) as task: driver_info = task.node.driver_info profile_uri = driver_info.get('applied_server_profile_uri') @@ -100,7 +98,10 @@ class OneViewManagementDriverFunctionsTestCase(db_base.DbTestCase): update.assert_called_once_with(server_profile, profile_uri) patch = ilo_client.patch patch.assert_called_once_with( - path=path, body=body, headers=headers) + path=management.ILO_SYSTEM_PATH, + body=body, + headers=management.ILO_REQUEST_HEADERS + ) driver_internal_info = task.node.driver_internal_info self.assertNotIn('next_boot_device', driver_internal_info) @@ -155,14 +156,12 @@ class OneViewManagementDriverFunctionsTestCase(db_base.DbTestCase): self, mock_iloclient, mock_get_ov_client): ilo_client = mock_iloclient() boot_device = management.BOOT_DEVICE_MAP_ILO.get(boot_devices.DISK) - path = '/rest/v1/Systems/1' body = { "Boot": { "BootSourceOverrideTarget": boot_device, "BootSourceOverrideEnabled": "Once" } } - headers = {"Content-Type": "application/json"} with task_manager.acquire(self.context, self.node.uuid) as task: driver_internal_info = task.node.driver_internal_info next_boot_device = {'boot_device': 'disk', 'persistent': False} @@ -170,8 +169,49 @@ class OneViewManagementDriverFunctionsTestCase(db_base.DbTestCase): task.node.driver_internal_info = driver_internal_info management.set_onetime_boot(task) self.assertTrue(mock_iloclient.called) - ilo_client.patch.assert_called_with( - path=path, body=body, headers=headers) + ilo_client.patch.assert_called_once_with( + path=management.ILO_SYSTEM_PATH, + body=body, + headers=management.ILO_REQUEST_HEADERS + ) + + @mock.patch.object(common, 'get_ilorest_client') + def test__is_onetime_boot_true( + self, mock_iloclient, mock_get_ov_client): + + class RestResponse(object): + @property + def dict(self): + return {'Boot': {'BootSourceOverrideEnabled': "Once"}} + + ilo_client = mock_iloclient() + ilo_client.get.return_value = RestResponse() + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertTrue(management._is_onetime_boot(task)) + self.assertTrue(mock_iloclient.called) + ilo_client.get.assert_called_with( + path=management.ILO_SYSTEM_PATH, + headers=management.ILO_REQUEST_HEADERS + ) + + @mock.patch.object(common, 'get_ilorest_client') + def test__is_onetime_boot_false( + self, mock_iloclient, mock_get_ov_client): + + class RestResponse(object): + @property + def dict(self): + return {'Boot': {'BootSourceOverrideEnabled': "Disabled"}} + + ilo_client = mock_iloclient() + ilo_client.get.return_value = RestResponse() + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertFalse(management._is_onetime_boot(task)) + self.assertTrue(mock_iloclient.called) + ilo_client.get.assert_called_with( + path=management.ILO_SYSTEM_PATH, + headers=management.ILO_REQUEST_HEADERS + ) class OneViewManagementDriverTestCase(db_base.DbTestCase): @@ -276,21 +316,26 @@ class OneViewManagementDriverTestCase(db_base.DbTestCase): ) @mock.patch.object(common, 'get_hponeview_client') - def test_get_boot_device(self, mock_get_ov_client): - client = mock_get_ov_client() + @mock.patch.object(common, 'get_ilorest_client') + def test_get_boot_device(self, mock_iloclient, mock_get_ov_client): + ilo_client = mock_iloclient() + oneview_client = mock_get_ov_client() device_mapping = management.BOOT_DEVICE_MAP_ONEVIEW.items() with task_manager.acquire(self.context, self.node.uuid) as task: # For each known device on OneView, Ironic should return its # counterpart value for ironic_device, oneview_device in device_mapping: profile = {'boot': {'order': [oneview_device]}} - client.server_profiles.get.return_value = profile + oneview_client.server_profiles.get.return_value = profile expected = {'boot_device': ironic_device, 'persistent': True} response = self.driver.management.get_boot_device(task) self.assertEqual(expected, response) - self.assertTrue(client.server_profiles.get.called) + self.assertTrue(oneview_client.server_profiles.get.called) + self.assertTrue(ilo_client.get.called) - def test_get_boot_device_from_next_boot_device(self): + @mock.patch.object(common, 'get_ilorest_client') + def test_get_boot_device_from_next_boot_device(self, mock_iloclient): + ilo_client = mock_iloclient() with task_manager.acquire(self.context, self.node.uuid) as task: driver_internal_info = task.node.driver_internal_info next_boot_device = {'boot_device': boot_devices.DISK, @@ -303,10 +348,13 @@ class OneViewManagementDriverTestCase(db_base.DbTestCase): } response = self.driver.management.get_boot_device(task) self.assertEqual(expected_response, response) + self.assertFalse(ilo_client.get.called) @mock.patch.object(common, 'get_hponeview_client') - def test_get_boot_device_fail(self, mock_get_ov_client): + @mock.patch.object(common, 'get_ilorest_client') + def test_get_boot_device_fail(self, mock_iloclient, mock_get_ov_client): client = mock_get_ov_client() + ilo_client = mock_iloclient() exc = client_exception.HPOneViewException() client.server_profiles.get.side_effect = exc with task_manager.acquire(self.context, self.node.uuid) as task: @@ -316,14 +364,18 @@ class OneViewManagementDriverTestCase(db_base.DbTestCase): task ) self.assertTrue(client.server_profiles.get.called) + self.assertFalse(ilo_client.get.called) - def test_get_boot_device_unknown_device(self): + @mock.patch.object(common, 'get_ilorest_client') + def test_get_boot_device_unknown_device(self, mock_iloclient): + ilo_client = mock_iloclient() with task_manager.acquire(self.context, self.node.uuid) as task: self.assertRaises( exception.InvalidParameterValue, task.driver.management.get_boot_device, task ) + self.assertFalse(ilo_client.get.called) def test_get_sensors_data_not_implemented(self): with task_manager.acquire(self.context, self.node.uuid) as task: diff --git a/releasenotes/notes/fix-get-boot-device-not-persistent-de6159d8d2b60656.yaml b/releasenotes/notes/fix-get-boot-device-not-persistent-de6159d8d2b60656.yaml new file mode 100644 index 0000000000..3300e3ae27 --- /dev/null +++ b/releasenotes/notes/fix-get-boot-device-not-persistent-de6159d8d2b60656.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + The ``oneview`` management interface now correctly detects whether the + current boot device setting is persistent at the machine's iLO. + Previously it always returned ``True``. + See https://bugs.launchpad.net/ironic/+bug/1706725 for details.