diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 9f3fb185dc..1abb75fe10 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -69,6 +69,7 @@ from ironic.conductor import notification_utils as notify_utils from ironic.conductor import steps as conductor_steps from ironic.conductor import task_manager from ironic.conductor import utils +from ironic.conductor import verify from ironic.conf import CONF from ironic.drivers import base as drivers_base from ironic import objects @@ -1200,52 +1201,6 @@ class ConductorManager(base_manager.BaseConductorManager): self._spawn_worker, cleaning.continue_node_clean, task) - @task_manager.require_exclusive_lock - def _do_node_verify(self, task): - """Internal method to perform power credentials verification.""" - node = task.node - LOG.debug('Starting power credentials verification for node %s', - node.uuid) - - error = None - try: - task.driver.power.validate(task) - except Exception as e: - error = (_('Failed to validate power driver interface for node ' - '%(node)s. Error: %(msg)s') % - {'node': node.uuid, 'msg': e}) - log_traceback = not isinstance(e, exception.IronicException) - LOG.error(error, exc_info=log_traceback) - else: - try: - power_state = task.driver.power.get_power_state(task) - except Exception as e: - error = (_('Failed to get power state for node ' - '%(node)s. Error: %(msg)s') % - {'node': node.uuid, 'msg': e}) - log_traceback = not isinstance(e, exception.IronicException) - LOG.error(error, exc_info=log_traceback) - - if error is None: - # Retrieve BIOS config settings for this node - utils.node_cache_bios_settings(task, node) - # Cache the vendor if possible - utils.node_cache_vendor(task) - # Cache also boot_mode and secure_boot states - utils.node_cache_boot_mode(task) - - if power_state != node.power_state: - old_power_state = node.power_state - node.power_state = power_state - task.process_event('done') - notify_utils.emit_power_state_corrected_notification( - task, old_power_state) - else: - task.process_event('done') - else: - node.last_error = error - task.process_event('fail') - @METRICS.timer('ConductorManager.do_provisioning_action') @messaging.expected_exceptions(exception.NoFreeConductorWorker, exception.NodeLocked, @@ -1293,7 +1248,7 @@ class ConductorManager(base_manager.BaseConductorManager): task.process_event( 'manage', callback=self._spawn_worker, - call_args=(self._do_node_verify, task), + call_args=(verify.do_node_verify, task), err_handler=utils.provisioning_error_handler) return diff --git a/ironic/conductor/verify.py b/ironic/conductor/verify.py new file mode 100644 index 0000000000..3c40c1e50b --- /dev/null +++ b/ironic/conductor/verify.py @@ -0,0 +1,71 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Functionality related to verify steps.""" + +from oslo_log import log + +from ironic.common import exception +from ironic.common.i18n import _ +from ironic.conductor import notification_utils as notify_utils +from ironic.conductor import task_manager +from ironic.conductor import utils + +LOG = log.getLogger(__name__) + + +@task_manager.require_exclusive_lock +def do_node_verify(task): + """Internal method to perform power credentials verification.""" + node = task.node + LOG.debug('Starting power credentials verification for node %s', + node.uuid) + + error = None + try: + task.driver.power.validate(task) + except Exception as e: + error = (_('Failed to validate power driver interface for node ' + '%(node)s. Error: %(msg)s') % + {'node': node.uuid, 'msg': e}) + log_traceback = not isinstance(e, exception.IronicException) + LOG.error(error, exc_info=log_traceback) + else: + try: + power_state = task.driver.power.get_power_state(task) + except Exception as e: + error = (_('Failed to get power state for node ' + '%(node)s. Error: %(msg)s') % + {'node': node.uuid, 'msg': e}) + log_traceback = not isinstance(e, exception.IronicException) + LOG.error(error, exc_info=log_traceback) + if error is None: + # NOTE(janders) this can eventually move to driver-specific + # verify steps, will leave this for a follow-up change. + # Retrieve BIOS config settings for this node + utils.node_cache_bios_settings(task, node) + # Cache the vendor if possible + utils.node_cache_vendor(task) + # Cache also boot_mode and secure_boot states + utils.node_cache_boot_mode(task) + if power_state != node.power_state: + old_power_state = node.power_state + node.power_state = power_state + task.process_event('done') + notify_utils.emit_power_state_corrected_notification( + task, old_power_state) + + else: + task.process_event('done') + else: + node.last_error = error + task.process_event('fail') diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 1603b19be3..50d1d151d1 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -50,6 +50,7 @@ from ironic.conductor import notification_utils from ironic.conductor import steps as conductor_steps from ironic.conductor import task_manager from ironic.conductor import utils as conductor_utils +from ironic.conductor import verify from ironic.db import api as dbapi from ironic.drivers import base as drivers_base from ironic.drivers.modules import fake @@ -2695,7 +2696,7 @@ class DoProvisioningActionTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(states.MANAGEABLE, node.target_provision_state) self.assertIsNone(node.last_error) mock_spawn.assert_called_with(self.service, - self.service._do_node_verify, mock.ANY) + verify.do_node_verify, mock.ANY) @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', autospec=True) @@ -3380,149 +3381,6 @@ class DoNodeRescueTestCase(mgr_utils.CommonMixIn, mgr_utils.ServiceSetUpMixin, task.node.fault) -@mgr_utils.mock_record_keepalive -class DoNodeVerifyTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): - @mock.patch.object(conductor_utils, 'node_cache_vendor', autospec=True) - @mock.patch('ironic.objects.node.NodeCorrectedPowerStateNotification', - autospec=True) - @mock.patch('ironic.drivers.modules.fake.FakePower.get_power_state', - autospec=True) - @mock.patch('ironic.drivers.modules.fake.FakePower.validate', - autospec=True) - def test__do_node_verify(self, mock_validate, mock_get_power_state, - mock_notif, mock_cache_vendor): - self._start_service() - mock_get_power_state.return_value = states.POWER_OFF - # Required for exception handling - mock_notif.__name__ = 'NodeCorrectedPowerStateNotification' - node = obj_utils.create_test_node( - self.context, driver='fake-hardware', - provision_state=states.VERIFYING, - target_provision_state=states.MANAGEABLE, - last_error=None, - power_state=states.NOSTATE) - - with task_manager.acquire( - self.context, node['id'], shared=False) as task: - self.service._do_node_verify(task) - mock_cache_vendor.assert_called_once_with(task) - - self._stop_service() - - # 1 notification should be sent - - # baremetal.node.power_state_corrected.success - mock_notif.assert_called_once_with(publisher=mock.ANY, - event_type=mock.ANY, - level=mock.ANY, - payload=mock.ANY) - mock_notif.return_value.emit.assert_called_once_with(mock.ANY) - - node.refresh() - - mock_validate.assert_called_once_with(mock.ANY, task) - mock_get_power_state.assert_called_once_with(mock.ANY, task) - - self.assertEqual(states.MANAGEABLE, node.provision_state) - self.assertIsNone(node.target_provision_state) - self.assertIsNone(node.last_error) - self.assertEqual(states.POWER_OFF, node.power_state) - - @mock.patch('ironic.drivers.modules.fake.FakePower.get_power_state', - autospec=True) - @mock.patch('ironic.drivers.modules.fake.FakePower.validate', - autospec=True) - def test__do_node_verify_validation_fails(self, mock_validate, - mock_get_power_state): - self._start_service() - node = obj_utils.create_test_node( - self.context, driver='fake-hardware', - provision_state=states.VERIFYING, - target_provision_state=states.MANAGEABLE, - last_error=None, - power_state=states.NOSTATE) - - mock_validate.side_effect = RuntimeError("boom") - - with task_manager.acquire( - self.context, node['id'], shared=False) as task: - self.service._do_node_verify(task) - - self._stop_service() - node.refresh() - - mock_validate.assert_called_once_with(mock.ANY, task) - - self.assertEqual(states.ENROLL, node.provision_state) - self.assertIsNone(node.target_provision_state) - self.assertTrue(node.last_error) - self.assertFalse(mock_get_power_state.called) - - @mock.patch('ironic.drivers.modules.fake.FakePower.get_power_state', - autospec=True) - @mock.patch('ironic.drivers.modules.fake.FakePower.validate', - autospec=True) - def test__do_node_verify_get_state_fails(self, mock_validate, - mock_get_power_state): - self._start_service() - node = obj_utils.create_test_node( - self.context, driver='fake-hardware', - provision_state=states.VERIFYING, - target_provision_state=states.MANAGEABLE, - last_error=None, - power_state=states.NOSTATE) - - mock_get_power_state.side_effect = RuntimeError("boom") - - with task_manager.acquire( - self.context, node['id'], shared=False) as task: - self.service._do_node_verify(task) - - self._stop_service() - node.refresh() - - mock_get_power_state.assert_called_once_with(mock.ANY, task) - - self.assertEqual(states.ENROLL, node.provision_state) - self.assertIsNone(node.target_provision_state) - self.assertTrue(node.last_error) - - @mock.patch.object(conductor_utils, 'LOG', autospec=True) - @mock.patch('ironic.drivers.modules.fake.FakePower.validate', - autospec=True) - @mock.patch('ironic.drivers.modules.fake.FakeBIOS.cache_bios_settings', - autospec=True) - def _test__do_node_cache_bios(self, mock_bios, mock_validate, - mock_log, - enable_unsupported=False, - enable_exception=False): - if enable_unsupported: - mock_bios.side_effect = exception.UnsupportedDriverExtension('') - elif enable_exception: - mock_bios.side_effect = exception.IronicException('test') - node = obj_utils.create_test_node( - self.context, driver='fake-hardware', - provision_state=states.VERIFYING, - target_provision_state=states.MANAGEABLE) - with task_manager.acquire( - self.context, node.uuid, shared=False) as task: - self.service._do_node_verify(task) - mock_bios.assert_called_once_with(mock.ANY, task) - mock_validate.assert_called_once_with(mock.ANY, task) - if enable_exception: - mock_log.exception.assert_called_once_with( - 'Caching of bios settings failed on node {}.' - .format(node.uuid)) - - def test__do_node_cache_bios(self): - self._test__do_node_cache_bios() - - def test__do_node_cache_bios_exception(self): - self._test__do_node_cache_bios(enable_exception=True) - - def test__do_node_cache_bios_unsupported(self): - self._test__do_node_cache_bios(enable_unsupported=True) - - @mgr_utils.mock_record_keepalive class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn, db_base.DbTestCase): diff --git a/ironic/tests/unit/conductor/test_verify.py b/ironic/tests/unit/conductor/test_verify.py new file mode 100644 index 0000000000..0e29f87f59 --- /dev/null +++ b/ironic/tests/unit/conductor/test_verify.py @@ -0,0 +1,176 @@ +# coding=utf-8 + +# Copyright 2013 Hewlett-Packard Development Company, L.P. +# Copyright 2013 International Business Machines Corporation +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Test class for Ironic ManagerService.""" + +from unittest import mock + +from oslo_config import cfg + +from ironic.common import exception +from ironic.common import states +from ironic.conductor import task_manager +from ironic.conductor import utils as conductor_utils +from ironic.conductor import verify +from ironic.tests.unit.conductor import mgr_utils +from ironic.tests.unit.db import base as db_base +from ironic.tests.unit.objects import utils as obj_utils + +CONF = cfg.CONF + + +@mgr_utils.mock_record_keepalive +class DoNodeVerifyTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): + @mock.patch.object(conductor_utils, 'node_cache_vendor', autospec=True) + @mock.patch('ironic.objects.node.NodeCorrectedPowerStateNotification', + autospec=True) + @mock.patch('ironic.drivers.modules.fake.FakePower.get_power_state', + autospec=True) + @mock.patch('ironic.drivers.modules.fake.FakePower.validate', + autospec=True) + def test__do_node_verify(self, mock_validate, mock_get_power_state, + mock_notif, mock_cache_vendor): + self._start_service() + mock_get_power_state.return_value = states.POWER_OFF + # Required for exception handling + mock_notif.__name__ = 'NodeCorrectedPowerStateNotification' + node = obj_utils.create_test_node( + self.context, driver='fake-hardware', + provision_state=states.VERIFYING, + target_provision_state=states.MANAGEABLE, + last_error=None, + power_state=states.NOSTATE) + + with task_manager.acquire( + self.context, node['id'], shared=False) as task: + verify.do_node_verify(task) + + self._stop_service() + + # 1 notification should be sent - + # baremetal.node.power_state_corrected.success + mock_notif.assert_called_once_with(publisher=mock.ANY, + event_type=mock.ANY, + level=mock.ANY, + payload=mock.ANY) + mock_notif.return_value.emit.assert_called_once_with(mock.ANY) + + node.refresh() + + mock_validate.assert_called_once_with(mock.ANY, task) + mock_get_power_state.assert_called_once_with(mock.ANY, task) + + self.assertEqual(states.MANAGEABLE, node.provision_state) + self.assertIsNone(node.target_provision_state) + self.assertIsNone(node.last_error) + self.assertEqual(states.POWER_OFF, node.power_state) + + @mock.patch('ironic.drivers.modules.fake.FakePower.get_power_state', + autospec=True) + @mock.patch('ironic.drivers.modules.fake.FakePower.validate', + autospec=True) + def test__do_node_verify_validation_fails(self, mock_validate, + mock_get_power_state): + self._start_service() + node = obj_utils.create_test_node( + self.context, driver='fake-hardware', + provision_state=states.VERIFYING, + target_provision_state=states.MANAGEABLE, + last_error=None, + power_state=states.NOSTATE) + + mock_validate.side_effect = RuntimeError("boom") + + with task_manager.acquire( + self.context, node['id'], shared=False) as task: + verify.do_node_verify(task) + + self._stop_service() + node.refresh() + + mock_validate.assert_called_once_with(mock.ANY, task) + + self.assertEqual(states.ENROLL, node.provision_state) + self.assertIsNone(node.target_provision_state) + self.assertTrue(node.last_error) + self.assertFalse(mock_get_power_state.called) + + @mock.patch('ironic.drivers.modules.fake.FakePower.get_power_state', + autospec=True) + @mock.patch('ironic.drivers.modules.fake.FakePower.validate', + autospec=True) + def test__do_node_verify_get_state_fails(self, mock_validate, + mock_get_power_state): + self._start_service() + node = obj_utils.create_test_node( + self.context, driver='fake-hardware', + provision_state=states.VERIFYING, + target_provision_state=states.MANAGEABLE, + last_error=None, + power_state=states.NOSTATE) + + mock_get_power_state.side_effect = RuntimeError("boom") + + with task_manager.acquire( + self.context, node['id'], shared=False) as task: + verify.do_node_verify(task) + + self._stop_service() + node.refresh() + + mock_get_power_state.assert_called_once_with(mock.ANY, task) + + self.assertEqual(states.ENROLL, node.provision_state) + self.assertIsNone(node.target_provision_state) + self.assertTrue(node.last_error) + + @mock.patch.object(conductor_utils, 'LOG', autospec=True) + @mock.patch('ironic.drivers.modules.fake.FakePower.validate', + autospec=True) + @mock.patch('ironic.drivers.modules.fake.FakeBIOS.cache_bios_settings', + autospec=True) + def _test__do_node_cache_bios(self, mock_bios, mock_validate, + mock_log, + enable_unsupported=False, + enable_exception=False): + if enable_unsupported: + mock_bios.side_effect = exception.UnsupportedDriverExtension('') + elif enable_exception: + mock_bios.side_effect = exception.IronicException('test') + node = obj_utils.create_test_node( + self.context, driver='fake-hardware', + provision_state=states.VERIFYING, + target_provision_state=states.MANAGEABLE) + with task_manager.acquire( + self.context, node.uuid, shared=False) as task: + verify.do_node_verify(task) + mock_bios.assert_called_once_with(mock.ANY, task) + mock_validate.assert_called_once_with(mock.ANY, task) + if enable_exception: + mock_log.exception.assert_called_once_with( + 'Caching of bios settings failed on node {}.' + .format(node.uuid)) + + def test__do_node_cache_bios(self): + self._test__do_node_cache_bios() + + def test__do_node_cache_bios_exception(self): + self._test__do_node_cache_bios(enable_exception=True) + + def test__do_node_cache_bios_unsupported(self): + self._test__do_node_cache_bios(enable_unsupported=True)