diff --git a/devstack/lib/ironic b/devstack/lib/ironic index b0181f899c..582d24c12e 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -1333,6 +1333,9 @@ function configure_ironic { fi # NOTE(vsaienko) Add stack to libvirt group when installing without nova. if ! is_service_enabled nova; then + # Disable power state change callbacks to nova. + iniset $IRONIC_CONF_FILE nova send_power_notifications false + add_user_to_group $STACK_USER $LIBVIRT_GROUP # This is the basic set of devices allowed / required by all virtual machines. @@ -1412,7 +1415,7 @@ function configure_ironic_conductor { # NOTE(pas-ha) service_catalog section is used to discover # ironic API endpoint from keystone catalog - local client_sections="neutron swift glance inspector cinder service_catalog json_rpc" + local client_sections="neutron swift glance inspector cinder service_catalog json_rpc nova" for conf_section in $client_sections; do configure_client_for $conf_section done diff --git a/doc/source/admin/index.rst b/doc/source/admin/index.rst index 6c348620b8..855eaef69c 100644 --- a/doc/source/admin/index.rst +++ b/doc/source/admin/index.rst @@ -31,6 +31,7 @@ the services. Security Windows Images Troubleshooting FAQ + Power Sync with the Compute Service .. toctree:: :hidden: diff --git a/doc/source/admin/power-sync.rst b/doc/source/admin/power-sync.rst new file mode 100644 index 0000000000..f19ff6c1b3 --- /dev/null +++ b/doc/source/admin/power-sync.rst @@ -0,0 +1,75 @@ +=================================== +Power Sync with the Compute Service +=================================== + +Baremetal Power Sync +==================== +Each Baremetal conductor process runs a periodic task which synchronizes the +power state of the nodes between its database and the actual hardware. If the +value of the :oslo.config:option:`conductor.force_power_state_during_sync` +option is set to ``true`` the power state in the database will be forced on +the hardware and if it is set to ``false`` the hardware state will be forced +on the database. If this periodic task is enabled, it runs at an interval +defined by the :oslo.config:option:`conductor.sync_power_state_interval` config +option for those nodes which are not in maintenance. + +Compute-Baremetal Power Sync +============================ +Each ``nova-compute`` process in the Compute service runs a periodic task which +synchronizes the power state of servers between its database and the compute +driver. If enabled, it runs at an interval defined by the +`sync_power_state_interval` config option on the ``nova-compute`` process. +In case of the compute driver being baremetal driver, this sync will happen +between the databases of the compute and baremetal services. Since the sync +happens on the ``nova-compute`` process, the state in the compute database +will be forced on the baremetal database in case of inconsistencies. Hence a +node which was put down using the compute service API cannot be brought up +through the baremetal service API since the power sync task will regard the +compute service's knowledge of the power state as the source of truth. In order +to get around this disadvantage of the compute-baremetal power sync, +baremetal service does power state change callbacks to the compute service +using external events. + +Power State Change Callbacks to the Compute Service +--------------------------------------------------- + +Whenever the Baremetal service changes the power state of a node, it can issue +a notification to the Compute service. The Compute service will consume this +notification and update the power state of the instance in its database. +By conveying all the power state changes to the compute service, the baremetal +service becomes the source of truth thus preventing the compute service from +forcing wrong power states on the physical instance during the +compute-baremetal power sync. It also adds the possibility of bringing +up/down a physical instance through the baremetal service API even if it was +put down/up through the compute service API. + +This change requires the :oslo.config:group:`nova` section and the necessary +authentication options like the :oslo.config:option:`nova.auth_url` to be +defined in the configuration file of the baremetal service. If it is not +configured the baremetal service will not be able to send notifications to the +compute service and it will fall back to the behaviour of the compute service +forcing power states on the baremetal service during the power sync. +See :oslo.config:group:`nova` group for more details on the available config +options. + +In case of baremetal stand alone deployments where there is no compute service +running, the :oslo.config:option:`nova.send_power_notifications` config option +should be set to ``False`` to disable power state change callbacks to the +compute service. + +.. note:: + The baremetal service sends notifications to the compute service only if + the target power state is ``power on`` or ``power off``. Other error + and ``None`` states will be ignored. In situations where the power state + change is originally coming from the compute service, the notification + will still be sent by the baremetal service and it will be a no-op on the + compute service side with a debug log stating the node is already powering + on/off. + +.. note:: + Although an exclusive lock is used when sending notifications to the + compute service, there can still be a race condition if the + compute-baremetal power sync happens to happen a nano-second before the + power state change event is received from the baremetal service in which + case the power state from compute service's database will be forced on the + node. diff --git a/ironic/common/nova.py b/ironic/common/nova.py new file mode 100644 index 0000000000..d7d04d8ac3 --- /dev/null +++ b/ironic/common/nova.py @@ -0,0 +1,116 @@ +# 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. + +from keystoneauth1 import exceptions as kaexception +from oslo_log import log + +from ironic.common import keystone +from ironic.common import states +from ironic.conf import CONF + + +LOG = log.getLogger(__name__) + +NOVA_API_VERSION = "2.1" +NOVA_API_MICROVERSION = '2.76' +_NOVA_ADAPTER = None + + +def _get_nova_adapter(): + global _NOVA_ADAPTER + if not _NOVA_ADAPTER: + _NOVA_ADAPTER = keystone.get_adapter( + 'nova', + session=keystone.get_session('nova'), + auth=keystone.get_auth('nova'), + version=NOVA_API_VERSION) + return _NOVA_ADAPTER + + +def _get_power_update_event(server_uuid, target_power_state): + return {'name': 'power-update', + 'server_uuid': server_uuid, + 'tag': target_power_state} + + +def _send_event(context, event, api_version=None): + """Sends an event to Nova conveying power state change. + + :param context: + request context, + instance of ironic.common.context.RequestContext + :param event: + A "power-update" event for nova to act upon. + :param api_version: + api version of nova + :returns: + A boolean which indicates if the event was sent and received + successfully. + """ + + try: + nova = _get_nova_adapter() + response = nova.post( + '/os-server-external-events', json={'events': [event]}, + microversion=api_version, global_request_id=context.global_id, + raise_exc=False) + except kaexception.ClientException as ex: + LOG.warning('Could not connect to Nova to send a power notification, ' + 'please check configuration. %s', ex) + return False + + try: + if response.status_code >= 400: + LOG.warning('Failed to notify nova on event: %s. %s.', + event, response.text) + return False + resp_event = response.json()['events'][0] + code = resp_event['code'] + except Exception as e: + LOG.error('Invalid response %s returned from nova for power-update ' + 'event %s. %s.', response, event, e) + return False + + if code >= 400: + LOG.warning('Nova event: %s returned with failed status.', resp_event) + else: + LOG.debug('Nova event response: %s.', resp_event) + return True + + +def power_update(context, server_uuid, target_power_state): + """Creates and sends power state change for the provided server_uuid. + + :param context: + request context, + instance of ironic.common.context.RequestContext + :param server_uuid: + The uuid of the node whose power state changed. + :param target_power_state: + Targeted power state change i.e "POWER_ON" or "POWER_OFF" + :returns: + A boolean which indicates if the power update was executed + successfully (mainly for testing purposes). + """ + if not CONF.nova.send_power_notifications: + return False + + if target_power_state == states.POWER_ON: + target_power_state = "POWER_ON" + elif target_power_state == states.POWER_OFF: + target_power_state = "POWER_OFF" + else: + LOG.error('Invalid Power State %s.', target_power_state) + return False + event = _get_power_update_event(server_uuid, target_power_state) + result = _send_event(context, event, api_version=NOVA_API_MICROVERSION) + return result diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index cea2afe12b..2085ed5be8 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -63,6 +63,7 @@ from ironic.common.glance_service import service_utils as glance_utils from ironic.common.i18n import _ from ironic.common import images from ironic.common import network +from ironic.common import nova from ironic.common import release_mappings as versions from ironic.common import states from ironic.common import swift @@ -1832,6 +1833,9 @@ class ConductorManager(base_manager.BaseConductorManager): "with actual power state '%(state)s'.", {'node': node.uuid, 'state': actual_power_state}) if old_power_state != actual_power_state: + if node.instance_uuid: + nova.power_update( + task.context, node.instance_uuid, node.power_state) notify_utils.emit_power_state_corrected_notification( task, old_power_state) @@ -4017,6 +4021,9 @@ def handle_sync_power_state_max_retries_exceeded(task, actual_power_state, node.fault = faults.POWER_FAILURE node.save() if old_power_state != actual_power_state: + if node.instance_uuid: + nova.power_update( + task.context, node.instance_uuid, node.power_state) notify_utils.emit_power_state_corrected_notification( task, old_power_state) LOG.error(msg) @@ -4096,6 +4103,9 @@ def do_sync_power_state(task, count): {'node': node.uuid, 'state': power_state}) node.power_state = power_state node.save() + if node.instance_uuid: + nova.power_update( + task.context, node.instance_uuid, node.power_state) notify_utils.emit_power_state_corrected_notification( task, None) return 0 @@ -4130,6 +4140,9 @@ def do_sync_power_state(task, count): 'state': node.power_state}) node.power_state = power_state node.save() + if node.instance_uuid: + nova.power_update( + task.context, node.instance_uuid, node.power_state) notify_utils.emit_power_state_corrected_notification( task, old_power_state) diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 27f589b949..4cea2f2c75 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -29,6 +29,7 @@ from ironic.common import exception from ironic.common import faults from ironic.common.i18n import _ from ironic.common import network +from ironic.common import nova from ironic.common import states from ironic.conductor import notification_utils as notify_utils from ironic.conductor import task_manager @@ -304,6 +305,9 @@ def node_power_action(task, new_state, timeout=None): node['power_state'] = target_state node['target_power_state'] = states.NOSTATE node.save() + if node.instance_uuid: + nova.power_update( + task.context, node.instance_uuid, target_state) notify_utils.emit_power_set_notification( task, fields.NotificationLevel.INFO, fields.NotificationStatus.END, new_state) diff --git a/ironic/conf/__init__.py b/ironic/conf/__init__.py index f53b02b010..9243cbbe3c 100644 --- a/ironic/conf/__init__.py +++ b/ironic/conf/__init__.py @@ -39,6 +39,7 @@ from ironic.conf import json_rpc from ironic.conf import metrics from ironic.conf import metrics_statsd from ironic.conf import neutron +from ironic.conf import nova from ironic.conf import pxe from ironic.conf import redfish from ironic.conf import service_catalog @@ -72,6 +73,7 @@ json_rpc.register_opts(CONF) metrics.register_opts(CONF) metrics_statsd.register_opts(CONF) neutron.register_opts(CONF) +nova.register_opts(CONF) pxe.register_opts(CONF) redfish.register_opts(CONF) service_catalog.register_opts(CONF) diff --git a/ironic/conf/nova.py b/ironic/conf/nova.py new file mode 100644 index 0000000000..9fc8c1c52e --- /dev/null +++ b/ironic/conf/nova.py @@ -0,0 +1,34 @@ +# 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. + +from oslo_config import cfg + +from ironic.common.i18n import _ +from ironic.conf import auth + +opts = [ + cfg.BoolOpt('send_power_notifications', + default=True, + help=_('When set to True, it will enable the support ' + 'for power state change callbacks to nova. This ' + 'option should be set to False in deployments ' + 'that do not have the openstack compute service.')) +] + + +def register_opts(conf): + conf.register_opts(opts, group='nova') + auth.register_auth_opts(conf, 'nova', service_type='compute') + + +def list_opts(): + return auth.add_auth_opts(opts, service_type='compute') diff --git a/ironic/conf/opts.py b/ironic/conf/opts.py index 4d28d90a12..61e6fe8958 100644 --- a/ironic/conf/opts.py +++ b/ironic/conf/opts.py @@ -55,6 +55,7 @@ _opts = [ ('metrics', ironic.conf.metrics.opts), ('metrics_statsd', ironic.conf.metrics_statsd.opts), ('neutron', ironic.conf.neutron.list_opts()), + ('nova', ironic.conf.nova.list_opts()), ('pxe', ironic.conf.pxe.opts), ('service_catalog', ironic.conf.service_catalog.list_opts()), ('snmp', ironic.conf.snmp.opts), diff --git a/ironic/tests/unit/common/test_nova.py b/ironic/tests/unit/common/test_nova.py new file mode 100644 index 0000000000..e961cca0dc --- /dev/null +++ b/ironic/tests/unit/common/test_nova.py @@ -0,0 +1,209 @@ +# 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. + + +import ddt +from keystoneauth1 import exceptions as kaexception +import mock +import requests + + +from ironic.common import context +from ironic.common import keystone +from ironic.common import nova +from ironic.tests import base + + +@mock.patch.object(keystone, 'get_session', autospec=True) +@mock.patch.object(keystone, 'get_adapter', autospec=True) +class TestNovaAdapter(base.TestCase): + + def test_get_nova_adapter(self, mock_adapter, mock_nova_session): + nova._NOVA_ADAPTER = None + mock_session_obj = mock.Mock() + expected = {'session': mock_session_obj, + 'auth': None, + 'version': "2.1"} + mock_nova_session.return_value = mock_session_obj + nova._get_nova_adapter() + mock_nova_session.assert_called_once_with('nova') + mock_adapter.assert_called_once_with(group='nova', **expected) + + """Check if existing adapter is used.""" + mock_nova_session.reset_mock() + nova._get_nova_adapter() + mock_nova_session.assert_not_called() + + +@ddt.ddt +@mock.patch.object(nova, 'LOG', autospec=True) +class NovaApiTestCase(base.TestCase): + def setUp(self): + super(NovaApiTestCase, self).setUp() + + self.api = nova + self.ctx = context.get_admin_context() + + @ddt.data({'events': [{'status': 'completed', + 'tag': 'POWER_OFF', + 'name': 'power-update', + 'server_uuid': '1234', + 'code': 200}]}, + {'events': [{'code': 404}]}, + {'events': [{'code': 400}]}) + @mock.patch.object(nova, '_get_nova_adapter') + def test_power_update(self, nova_result, mock_adapter, mock_log): + server_ids = ['server-id-1', 'server-id-2'] + nova_adapter = mock.Mock() + with mock.patch.object(nova_adapter, 'post') as mock_post_event: + post_resp_mock = requests.Response() + + def json_func(): + return nova_result + post_resp_mock.json = json_func + post_resp_mock.status_code = 200 + mock_adapter.return_value = nova_adapter + mock_post_event.return_value = post_resp_mock + for server in server_ids: + result = self.api.power_update(self.ctx, server, 'power on') + self.assertTrue(result) + + mock_adapter.assert_has_calls([mock.call(), mock.call()]) + req_url = '/os-server-external-events' + mock_post_event.assert_has_calls([ + mock.call(req_url, + json={'events': [{'name': 'power-update', + 'server_uuid': 'server-id-1', + 'tag': 'POWER_ON'}]}, + microversion='2.76', + global_request_id=self.ctx.global_id, + raise_exc=False), + mock.call(req_url, + json={'events': [{'name': 'power-update', + 'server_uuid': 'server-id-2', + 'tag': 'POWER_ON'}]}, + microversion='2.76', + global_request_id=self.ctx.global_id, + raise_exc=False) + ]) + if nova_result['events'][0]['code'] != 200: + expected = ('Nova event: %s returned with failed status.', + nova_result['events'][0]) + mock_log.warning.assert_called_with(*expected) + else: + expected = ("Nova event response: %s.", nova_result['events'][0]) + mock_log.debug.assert_called_with(*expected) + + @mock.patch.object(nova, '_get_nova_adapter') + def test_invalid_power_update(self, mock_adapter, mock_log): + nova_adapter = mock.Mock() + with mock.patch.object(nova_adapter, 'post') as mock_post_event: + result = self.api.power_update(self.ctx, 'server', None) + self.assertFalse(result) + expected = ('Invalid Power State %s.', None) + mock_log.error.assert_called_once_with(*expected) + + mock_adapter.assert_not_called() + mock_post_event.assert_not_called() + + def test_power_update_failed(self, mock_log): + nova_adapter = nova._get_nova_adapter() + event = [{'name': 'power-update', + 'server_uuid': 'server-id-1', + 'tag': 'POWER_OFF'}] + nova_result = requests.Response() + with mock.patch.object(nova_adapter, 'post') as mock_post_event: + for stat_code in (500, 404, 207): + mock_log.reset_mock() + nova_result.status_code = stat_code + type(nova_result).text = mock.PropertyMock(return_value="blah") + if stat_code == 207: + def json_func(): + return {'events': [{}]} + nova_result.json = json_func + mock_post_event.return_value = nova_result + result = self.api.power_update( + self.ctx, 'server-id-1', 'power off') + self.assertFalse(result) + if stat_code == 207: + expected = ('Invalid response %s returned from nova for ' + 'power-update event %s. %s.') + self.assertIn(expected, mock_log.error.call_args[0][0]) + else: + expected = ("Failed to notify nova on event: %s. %s.", + event[0], "blah") + mock_log.warning.assert_called_once_with(*expected) + + mock_post_event.assert_has_calls([ + mock.call('/os-server-external-events', + json={'events': event}, + microversion='2.76', + global_request_id=self.ctx.global_id, + raise_exc=False) + ]) + + @ddt.data({'events': [{}]}, + {'events': []}, + {'events': None}, + {}) + @mock.patch.object(nova, '_get_nova_adapter') + def test_power_update_invalid_reponse_format(self, nova_result, + mock_adapter, mock_log): + nova_adapter = mock.Mock() + with mock.patch.object(nova_adapter, 'post') as mock_post_event: + post_resp_mock = requests.Response() + + def json_func(): + return nova_result + + post_resp_mock.json = json_func + post_resp_mock.status_code = 207 + mock_adapter.return_value = nova_adapter + mock_post_event.return_value = post_resp_mock + result = self.api.power_update(self.ctx, 'server-id-1', 'power on') + self.assertFalse(result) + + mock_adapter.assert_has_calls([mock.call()]) + req_url = '/os-server-external-events' + mock_post_event.assert_has_calls([ + mock.call(req_url, + json={'events': [{'name': 'power-update', + 'server_uuid': 'server-id-1', + 'tag': 'POWER_ON'}]}, + microversion='2.76', + global_request_id=self.ctx.global_id, + raise_exc=False), + ]) + self.assertIn('Invalid response', mock_log.error.call_args[0][0]) + + @mock.patch.object(keystone, 'get_adapter', autospec=True) + def test_power_update_failed_no_nova(self, mock_adapter, mock_log): + self.config(send_power_notifications=False, group="nova") + result = self.api.power_update(self.ctx, 'server-id-1', 'power off') + self.assertFalse(result) + mock_adapter.assert_not_called() + + @mock.patch.object(nova, '_get_nova_adapter') + def test_power_update_failed_no_nova_auth_url(self, mock_adapter, + mock_log): + server = 'server-id-1' + emsg = 'An auth plugin is required to determine endpoint URL' + side_effect = kaexception.MissingAuthPlugin(emsg) + mock_nova = mock.Mock() + mock_adapter.return_value = mock_nova + mock_nova.post.side_effect = side_effect + result = self.api.power_update(self.ctx, server, 'power off') + msg = ('Could not connect to Nova to send a power notification, ' + 'please check configuration. %s', side_effect) + self.assertFalse(result) + mock_log.warning.assert_called_once_with(*msg) + mock_adapter.assert_called_once_with() diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 44a4c3e61c..1622aac180 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -37,6 +37,7 @@ from ironic.common import boot_devices from ironic.common import driver_factory from ironic.common import exception from ironic.common import images +from ironic.common import nova from ironic.common import states from ironic.common import swift from ironic.conductor import manager @@ -6371,7 +6372,7 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase): self.power = self.driver.power self.node = obj_utils.create_test_node( self.context, driver='fake-hardware', maintenance=False, - provision_state=states.AVAILABLE) + provision_state=states.AVAILABLE, instance_uuid=uuidutils.uuid) self.task = mock.Mock(spec_set=['context', 'driver', 'node', 'upgrade_lock', 'shared']) self.task.context = self.context @@ -6407,7 +6408,8 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase): self.assertFalse(node_power_action.called) self.assertFalse(self.task.upgrade_lock.called) - def test_state_not_set(self, node_power_action): + @mock.patch.object(nova, 'power_update', autospec=True) + def test_state_not_set(self, mock_power_update, node_power_action): self._do_sync_power_state(None, states.POWER_ON) self.power.validate.assert_called_once_with(self.task) @@ -6415,6 +6417,8 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase): self.assertFalse(node_power_action.called) self.assertEqual(states.POWER_ON, self.node.power_state) self.task.upgrade_lock.assert_called_once_with() + mock_power_update.assert_called_once_with( + self.task.context, self.node.instance_uuid, states.POWER_ON) def test_validate_fail(self, node_power_action): self._do_sync_power_state(None, states.POWER_ON, @@ -6445,7 +6449,8 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase): self.assertEqual(1, self.service.power_state_sync_count[self.node.uuid]) - def test_state_changed_no_sync(self, node_power_action): + @mock.patch.object(nova, 'power_update', autospec=True) + def test_state_changed_no_sync(self, mock_power_update, node_power_action): self._do_sync_power_state(states.POWER_ON, states.POWER_OFF) self.assertFalse(self.power.validate.called) @@ -6453,9 +6458,13 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase): self.assertFalse(node_power_action.called) self.assertEqual(states.POWER_OFF, self.node.power_state) self.task.upgrade_lock.assert_called_once_with() + mock_power_update.assert_called_once_with( + self.task.context, self.node.instance_uuid, states.POWER_OFF) @mock.patch('ironic.objects.node.NodeCorrectedPowerStateNotification') - def test_state_changed_no_sync_notify(self, mock_notif, node_power_action): + @mock.patch.object(nova, 'power_update', autospec=True) + def test_state_changed_no_sync_notify(self, mock_power_update, mock_notif, + node_power_action): # Required for exception handling mock_notif.__name__ = 'NodeCorrectedPowerStateNotification' @@ -6481,6 +6490,8 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase): notif_args, 'ironic-conductor', CONF.host, 'baremetal.node.power_state_corrected.success', obj_fields.NotificationLevel.INFO) + mock_power_update.assert_called_once_with( + self.task.context, self.node.instance_uuid, states.POWER_OFF) def test_state_changed_sync(self, node_power_action): self.config(force_power_state_during_sync=True, group='conductor') @@ -6508,7 +6519,8 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase): self.assertEqual(1, self.service.power_state_sync_count[self.node.uuid]) - def test_max_retries_exceeded(self, node_power_action): + @mock.patch.object(nova, 'power_update', autospec=True) + def test_max_retries_exceeded(self, mock_power_update, node_power_action): self.config(force_power_state_during_sync=True, group='conductor') self.config(power_state_sync_max_retries=1, group='conductor') @@ -6526,8 +6538,11 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase): self.assertTrue(self.node.maintenance) self.assertIsNotNone(self.node.maintenance_reason) self.assertEqual('power failure', self.node.fault) + mock_power_update.assert_called_once_with( + self.task.context, self.node.instance_uuid, states.POWER_OFF) - def test_max_retries_exceeded2(self, node_power_action): + @mock.patch.object(nova, 'power_update', autospec=True) + def test_max_retries_exceeded2(self, mock_power_update, node_power_action): self.config(force_power_state_during_sync=True, group='conductor') self.config(power_state_sync_max_retries=2, group='conductor') @@ -6546,9 +6561,13 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase): self.service.power_state_sync_count[self.node.uuid]) self.assertTrue(self.node.maintenance) self.assertEqual('power failure', self.node.fault) + mock_power_update.assert_called_once_with( + self.task.context, self.node.instance_uuid, states.POWER_OFF) @mock.patch('ironic.objects.node.NodeCorrectedPowerStateNotification') - def test_max_retries_exceeded_notify(self, mock_notif, node_power_action): + @mock.patch.object(nova, 'power_update', autospec=True) + def test_max_retries_exceeded_notify(self, mock_power_update, + mock_notif, node_power_action): self.config(force_power_state_during_sync=True, group='conductor') self.config(power_state_sync_max_retries=1, group='conductor') # Required for exception handling @@ -6570,6 +6589,8 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase): notif_args, 'ironic-conductor', CONF.host, 'baremetal.node.power_state_corrected.success', obj_fields.NotificationLevel.INFO) + mock_power_update.assert_called_once_with( + self.task.context, self.node.instance_uuid, states.POWER_OFF) def test_retry_then_success(self, node_power_action): self.config(force_power_state_during_sync=True, group='conductor') @@ -6993,8 +7014,10 @@ class ManagerPowerRecoveryTestCase(mgr_utils.CommonMixIn, @mock.patch.object(notification_utils, 'emit_power_state_corrected_notification') - def test_node_recovery_success(self, notify_mock, get_nodeinfo_mock, - mapped_mock, acquire_mock): + @mock.patch.object(nova, 'power_update', autospec=True) + def test_node_recovery_success(self, mock_power_update, notify_mock, + get_nodeinfo_mock, mapped_mock, + acquire_mock): self.node.power_state = states.POWER_ON get_nodeinfo_mock.return_value = self._get_nodeinfo_list_response() mapped_mock.return_value = True @@ -7019,6 +7042,8 @@ class ManagerPowerRecoveryTestCase(mgr_utils.CommonMixIn, self.assertIsNone(self.node.maintenance_reason) self.assertEqual(states.POWER_OFF, self.node.power_state) notify_mock.assert_called_once_with(self.task, states.POWER_ON) + mock_power_update.assert_called_once_with( + self.task.context, self.node.instance_uuid, states.POWER_OFF) def test_node_recovery_failed(self, get_nodeinfo_mock, mapped_mock, acquire_mock): diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index 5c5fb68b31..0606dd5101 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -21,6 +21,7 @@ from ironic.common import boot_modes from ironic.common import exception from ironic.common import network from ironic.common import neutron +from ironic.common import nova from ironic.common import states from ironic.conductor import rpcapi from ironic.conductor import task_manager @@ -176,7 +177,9 @@ class NodePowerActionTestCase(db_base.DbTestCase): @mock.patch('ironic.objects.node.NodeSetPowerStateNotification') @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True) - def test_node_power_action_power_on_notify(self, get_power_mock, + @mock.patch.object(nova, 'power_update', autospec=True) + def test_node_power_action_power_on_notify(self, mock_power_update, + get_power_mock, mock_notif): """Test node_power_action to power on node and send notification.""" self.config(notification_level='info') @@ -186,6 +189,7 @@ class NodePowerActionTestCase(db_base.DbTestCase): node = obj_utils.create_test_node(self.context, uuid=uuidutils.generate_uuid(), driver='fake-hardware', + instance_uuid=uuidutils.uuid, power_state=states.POWER_OFF) task = task_manager.TaskManager(self.context, node.uuid) @@ -214,6 +218,8 @@ class NodePowerActionTestCase(db_base.DbTestCase): 'ironic-conductor', CONF.host, 'baremetal.node.power_set.end', obj_fields.NotificationLevel.INFO) + mock_power_update.assert_called_once_with( + task.context, node.instance_uuid, states.POWER_ON) @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True) def test_node_power_action_power_off(self, get_power_mock): diff --git a/lower-constraints.txt b/lower-constraints.txt index 77ddd3e26a..10d98049f6 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -4,6 +4,7 @@ Babel==2.3.4 bandit==1.1.0 bashate==0.5.1 coverage==4.0 +ddt==1.0.1 doc8==0.6.0 eventlet==0.18.2 fixtures==3.0.0 @@ -15,7 +16,7 @@ iso8601==0.1.11 Jinja2==2.10 jsonpatch==1.16 jsonschema==2.6.0 -keystoneauth1==3.11.0 +keystoneauth1==3.15.0 keystonemiddleware==4.17.0 mock==3.0.0 openstackdocstheme==1.20.0 diff --git a/releasenotes/notes/bp-nova-support-instance-power-update-49c531ef13982e62.yaml b/releasenotes/notes/bp-nova-support-instance-power-update-49c531ef13982e62.yaml new file mode 100644 index 0000000000..c478527f62 --- /dev/null +++ b/releasenotes/notes/bp-nova-support-instance-power-update-49c531ef13982e62.yaml @@ -0,0 +1,31 @@ +--- +features: + - | + Adds power state change callbacks of an instance to the Compute service by + performing API notifications. This is configurable through the + ``nova.send_power_notifications`` config option. Whenever there is a change + in the power state of a physical instance (for example a "power on" or + "power off" API command is issued or during the periodic power state + synchronization between nova and ironic) the Baremetal service will create + and send a ``power-update`` external event to the Compute service which will + cause the power state of the instance to be updated in its database. It + also adds the possibility of bringing up/down a physical instance through + the Baremetal service API even if it was put down/up through the Compute + service API. +fixes: + - | + By immediately conveying all the power state changes (note that the + Baremetal service only sends requests to the Compute service if the target + power state is either "power on" or "power off") of an instance through + external events to the Compute service, the Baremetal service becomes the + source of truth thus preventing the Compute service from forcing wrong + power states on the instance during the periodic power state + synchronization between nova and ironic. An exception would be if a race + condition were to occur due to the nova-ironic power sync task happening + a nano-second before the power state change event is received from the + Baremetal service in which case the nova instance state will be forced + on the baremetal node. +upgrade: + - | + In order to support power state change call backs to nova, the ``[nova]`` + section must be configured in the baremetal service configuration. \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index e543ea71d6..44c4d681b7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,7 +10,7 @@ WebOb>=1.7.1 # MIT python-cinderclient!=4.0.0,>=3.3.0 # Apache-2.0 python-neutronclient>=6.7.0 # Apache-2.0 python-glanceclient>=2.8.0 # Apache-2.0 -keystoneauth1>=3.11.0 # Apache-2.0 +keystoneauth1>=3.15.0 # Apache-2.0 ironic-lib>=2.17.1 # Apache-2.0 python-swiftclient>=3.2.0 # Apache-2.0 pytz>=2013.6 # MIT diff --git a/test-requirements.txt b/test-requirements.txt index 6640ba8bbf..ab53cb70f6 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -3,6 +3,7 @@ # process, which may cause wedges in the gate later. hacking>=1.0.0,<1.1.0 # Apache-2.0 coverage!=4.4,>=4.0 # Apache-2.0 +ddt>=1.0.1 # MIT doc8>=0.6.0 # Apache-2.0 fixtures>=3.0.0 # Apache-2.0/BSD mock>=3.0.0 # BSD