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.