From d693d4c06c1ba89332ef8f3f672809b3cc81f8c9 Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Wed, 12 Jun 2019 11:27:41 +0200 Subject: [PATCH] Support power state change callbacks to nova using ksa_adapter Add power state change callbacks of an instance to nova by performing API requests. Whenever there is a change in the power state of a physical instance (example a "power on" or "power off" IPMI command is issued or the periodic ``_sync_power_states`` task detects a change in power state) ironic will create and send a ``power-update`` external event to nova using which nova will update the power state of the instance in its database. By conveying the power state changes to nova, ironic becomes the source of truth thus preventing nova from forcing wrong power states on the instance during the nova-ironic periodic sync. It also adds the possibility of bringing up/down a physical instance through the ironic API even if it was put down/up through the nova API. Note that ironic only sends requests to nova if the target power state is either "power on" or "power off". Other error states will be ignored. In cases where the power state change is originally coming from nova, the event will still be created and sent to nova and on the nova side it will be a no-op with a debug log saying the node is already powering on/off. NOTE: Although an exclusive lock (task_manager.upgrade_lock() method) is used when calling the nova API to send events, there can still be a race condition if the nova-ironic power sync happens to happen a nano-second before the power state change event is received from ironic in which case the nova state will be forced on the node. Credit for introducing ksa adapter: Eric Fried Depends-On: https://review.opendev.org/#/c/645611/ Part of blueprint nova-support-instance-power-update Story: 2004969 Task: 29424 Change-Id: I6d105524e1645d9a40dfeae2850c33cf2d110826 --- devstack/lib/ironic | 5 +- doc/source/admin/index.rst | 1 + doc/source/admin/power-sync.rst | 75 +++++++ ironic/common/nova.py | 116 ++++++++++ ironic/conductor/manager.py | 13 ++ ironic/conductor/utils.py | 4 + ironic/conf/__init__.py | 2 + ironic/conf/nova.py | 34 +++ ironic/conf/opts.py | 1 + ironic/tests/unit/common/test_nova.py | 209 ++++++++++++++++++ ironic/tests/unit/conductor/test_manager.py | 43 +++- ironic/tests/unit/conductor/test_utils.py | 8 +- lower-constraints.txt | 3 +- ...nstance-power-update-49c531ef13982e62.yaml | 31 +++ requirements.txt | 2 +- test-requirements.txt | 1 + 16 files changed, 535 insertions(+), 13 deletions(-) create mode 100644 doc/source/admin/power-sync.rst create mode 100644 ironic/common/nova.py create mode 100644 ironic/conf/nova.py create mode 100644 ironic/tests/unit/common/test_nova.py create mode 100644 releasenotes/notes/bp-nova-support-instance-power-update-49c531ef13982e62.yaml diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 1292390c0c..b88bf28cf2 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -1316,6 +1316,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. @@ -1395,7 +1398,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 8c28307ce2..2abc73f640 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 @@ -1821,6 +1822,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) @@ -3992,6 +3996,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) @@ -4071,6 +4078,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 @@ -4105,6 +4115,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 c555701c34..8fa9a74393 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 1a6063c127..0e5519e64f 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 @@ -6260,7 +6261,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 @@ -6296,7 +6297,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) @@ -6304,6 +6306,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, @@ -6334,7 +6338,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) @@ -6342,9 +6347,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' @@ -6370,6 +6379,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') @@ -6397,7 +6408,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') @@ -6415,8 +6427,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') @@ -6435,9 +6450,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 @@ -6459,6 +6478,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') @@ -6882,8 +6903,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 @@ -6908,6 +6931,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 dc2e4b3d0a..f2438e39f2 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 7241fd3a52..fd3cee362b 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 1ac808ad49..8b9438c039 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