From c6ff9c922e77633a2a52b276dc1f09a221817551 Mon Sep 17 00:00:00 2001 From: Riccardo Pittau Date: Fri, 13 Sep 2019 14:52:41 +0200 Subject: [PATCH] Use openstacksdk for ironic notifiers This patch removes the dependency from ironicclient for the ironic event notifiers in favor of openstacksdk. Also, increasing minimum required versions for mock and openstacksdk. Change-Id: Ib76e19d29f0ae3db6d181578b638da699181f60d --- lower-constraints.txt | 5 +- neutron/common/config.py | 2 + neutron/conf/common.py | 27 --------- neutron/notifiers/ironic.py | 40 +++++-------- neutron/tests/unit/notifiers/test_ironic.py | 66 ++++++--------------- requirements.txt | 2 +- test-requirements.txt | 2 +- 7 files changed, 39 insertions(+), 105 deletions(-) diff --git a/lower-constraints.txt b/lower-constraints.txt index c1ff2a479dc..03b09daea50 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -49,7 +49,7 @@ logutils==0.3.5 Mako==0.4.0 MarkupSafe==1.0 mccabe==0.2.1 -mock==2.0.0 +mock==3.0.0 monotonic==0.6;python_version<'3.3' mox3==0.20.0 msgpack-python==0.4.0 @@ -58,7 +58,7 @@ netaddr==0.7.18 netifaces==0.10.4 neutron-lib==1.29.1 openstackdocstheme==1.30.0 -openstacksdk==0.11.2 +openstacksdk==0.31.2 os-client-config==1.28.0 os-ken==0.3.0 os-service-types==1.2.0 @@ -111,7 +111,6 @@ pyroute2==0.5.3 python-dateutil==2.5.3 python-designateclient==2.7.0 python-editor==1.0.3 -python-ironicclient==2.7.0 python-keystoneclient==3.8.0 python-mimeparse==1.6.0 python-neutronclient==6.7.0 diff --git a/neutron/common/config.py b/neutron/common/config.py index 62011f2651e..3bad679aeac 100644 --- a/neutron/common/config.py +++ b/neutron/common/config.py @@ -77,6 +77,8 @@ ks_loading.register_auth_conf_options(cfg.CONF, common_config.IRONIC_CONF_SECTION) ks_loading.register_session_conf_options(cfg.CONF, common_config.IRONIC_CONF_SECTION) +ks_loading.register_adapter_conf_options(cfg.CONF, + common_config.IRONIC_CONF_SECTION) common_config.register_ironic_opts() diff --git a/neutron/conf/common.py b/neutron/conf/common.py index e25480097a1..b09e361583b 100644 --- a/neutron/conf/common.py +++ b/neutron/conf/common.py @@ -198,33 +198,6 @@ ironic_opts = [ cfg.BoolOpt('enable_notifications', default=False, help=_("Send notification events to ironic. (For example on " "relevant port status changes.)")), - cfg.StrOpt('region_name', - help=_('Name of region used to get Ironic endpoints. Useful if ' - 'keystone manages more than one region.')), - cfg.StrOpt('endpoint_type', - default='public', - choices=['public', 'admin', 'internal'], - help=_('Type of the ironic endpoint to use. This endpoint ' - 'will be looked up in the keystone catalog and should ' - 'be one of public, internal or admin.')), - cfg.StrOpt('auth_strategy', - default='keystone', - choices=('keystone', 'noauth'), - help=_('Method to use for authentication: noauth or ' - 'keystone.')), - cfg.StrOpt('ironic_url', - default='http://localhost:6385/', - help=_('Ironic API URL, used to set Ironic API URL when ' - 'auth_strategy option is noauth to work with standalone ' - 'Ironic without keystone.')), - cfg.IntOpt('retry_interval', - default=2, - help=_('Interval between retries in case of conflict error ' - '(HTTP 409).')), - cfg.IntOpt('max_retries', - default=30, - help=_('Maximum number of retries in case of conflict error ' - '(HTTP 409).')), ] diff --git a/neutron/notifiers/ironic.py b/neutron/notifiers/ironic.py index 978c0c8f4bd..f09e9398bfd 100644 --- a/neutron/notifiers/ironic.py +++ b/neutron/notifiers/ironic.py @@ -13,8 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -from ironicclient import client -from ironicclient import exc as ironic_exc from keystoneauth1 import loading as ks_loading from neutron_lib.api.definitions import port as port_def from neutron_lib.api.definitions import portbindings as portbindings_def @@ -22,6 +20,8 @@ from neutron_lib.callbacks import events from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources from neutron_lib import constants as n_const +from openstack import connection +from openstack import exceptions as os_exc from oslo_config import cfg from oslo_log import log as logging @@ -61,33 +61,23 @@ class Notifier(object): def _get_ironic_client(self): """Get Ironic client instance.""" - # NOTE: To support standalone ironic without keystone - if cfg.CONF.ironic.auth_strategy == 'noauth': - args = {'token': 'noauth', - 'endpoint': cfg.CONF.ironic.ironic_url} - else: - global IRONIC_SESSION - if not IRONIC_SESSION: - IRONIC_SESSION = self._get_session(IRONIC_CONF_SECTION) - args = {'session': IRONIC_SESSION, - 'region_name': cfg.CONF.ironic.region_name, - 'endpoint_type': cfg.CONF.ironic.endpoint_type} - args['os_ironic_api_version'] = IRONIC_API_VERSION - args['max_retries'] = cfg.CONF.ironic.max_retries - args['retry_interval'] = cfg.CONF.ironic.retry_interval - return client.Client(IRONIC_CLIENT_VERSION, **args) + global IRONIC_SESSION + + if not IRONIC_SESSION: + IRONIC_SESSION = self._get_session(IRONIC_CONF_SECTION) + + return connection.Connection( + session=IRONIC_SESSION, oslo_conf=cfg.CONF).baremetal def send_events(self, batched_events): - # NOTE(TheJulia): Friendly exception handling so operators - # can decouple updates. try: - self.irclient.events.create(events=batched_events) - except ironic_exc.NotFound: - LOG.error('The ironic API appears to not support posting events. ' - 'The API likely needs to be upgraded.') + response = self.irclient.post('/events', + json={'events': batched_events}, + microversion='1.54') + os_exc.raise_from_response(response) except Exception as e: - LOG.error('Unknown error encountered posting the event to ' - 'ironic. {error}'.format(error=e)) + LOG.exception('Error encountered posting the event to ' + 'ironic. {error}'.format(error=e)) @registry.receives(resources.PORT, [events.AFTER_UPDATE]) def process_port_update_event(self, resource, event, trigger, diff --git a/neutron/tests/unit/notifiers/test_ironic.py b/neutron/tests/unit/notifiers/test_ironic.py index 0dcea755487..94434225e34 100644 --- a/neutron/tests/unit/notifiers/test_ironic.py +++ b/neutron/tests/unit/notifiers/test_ironic.py @@ -16,10 +16,9 @@ import eventlet import mock -from ironicclient import client -from ironicclient import exc as ironic_exc from neutron_lib.api.definitions import portbindings as portbindings_def from neutron_lib import constants as n_const +from openstack import connection from neutron.notifiers import batch_notifier from neutron.notifiers import ironic @@ -40,13 +39,13 @@ def get_fake_port(): class TestIronicNotifier(base.BaseTestCase): def setUp(self): super(TestIronicNotifier, self).setUp() + with mock.patch.object(connection.Connection, 'baremetal', + autospec=False): + self.ironic_notifier = ironic.Notifier() @mock.patch.object(batch_notifier.BatchNotifier, 'queue_event', autospec=True) - @mock.patch.object(client, 'Client', autospec=False) - def test_process_port_update_event_bind_port(self, mock_client, - mock_queue_event): - self.ironic_notifier = ironic.Notifier() + def test_process_port_update_event_bind_port(self, mock_queue_event): port = get_fake_port() port.update({'status': n_const.PORT_STATUS_ACTIVE}) original_port = get_fake_port() @@ -66,10 +65,7 @@ class TestIronicNotifier(base.BaseTestCase): @mock.patch.object(batch_notifier.BatchNotifier, 'queue_event', autospec=True) - @mock.patch.object(client, 'Client', autospec=False) - def test_process_port_update_event_bind_port_err(self, mock_client, - mock_queue_event): - self.ironic_notifier = ironic.Notifier() + def test_process_port_update_event_bind_port_err(self, mock_queue_event): port = get_fake_port() port.update({'status': n_const.PORT_STATUS_ERROR}) original_port = get_fake_port() @@ -89,10 +85,7 @@ class TestIronicNotifier(base.BaseTestCase): @mock.patch.object(batch_notifier.BatchNotifier, 'queue_event', autospec=True) - @mock.patch.object(client, 'Client', autospec=False) - def test_process_port_update_event_unbind_port(self, mock_client, - mock_queue_event): - self.ironic_notifier = ironic.Notifier() + def test_process_port_update_event_unbind_port(self, mock_queue_event): port = get_fake_port() port.update({'status': n_const.PORT_STATUS_DOWN}) original_port = get_fake_port() @@ -112,10 +105,7 @@ class TestIronicNotifier(base.BaseTestCase): @mock.patch.object(batch_notifier.BatchNotifier, 'queue_event', autospec=True) - @mock.patch.object(client, 'Client', autospec=False) - def test_process_port_update_event_unbind_port_err(self, mock_client, - mock_queue_event): - self.ironic_notifier = ironic.Notifier() + def test_process_port_update_event_unbind_port_err(self, mock_queue_event): port = get_fake_port() port.update({'status': n_const.PORT_STATUS_ERROR}) original_port = get_fake_port() @@ -135,9 +125,7 @@ class TestIronicNotifier(base.BaseTestCase): @mock.patch.object(batch_notifier.BatchNotifier, 'queue_event', autospec=True) - @mock.patch.object(client, 'Client', autospec=False) - def test_process_port_delete_event(self, mock_client, mock_queue_event): - self.ironic_notifier = ironic.Notifier() + def test_process_port_delete_event(self, mock_queue_event): port = get_fake_port() self.ironic_notifier.process_port_delete_event( 'fake_resource', 'fake_event', 'fake_trigger', original_port=None, @@ -154,10 +142,7 @@ class TestIronicNotifier(base.BaseTestCase): @mock.patch.object(batch_notifier.BatchNotifier, 'queue_event', autospec=True) - @mock.patch.object(client, 'Client', autospec=False) - def test_process_port_event_empty_uuid_field(self, mock_client, - mock_queue_event): - self.ironic_notifier = ironic.Notifier() + def test_process_port_event_empty_uuid_field(self, mock_queue_event): port = get_fake_port() port.update({'device_id': ''}) self.ironic_notifier.process_port_delete_event( @@ -173,9 +158,7 @@ class TestIronicNotifier(base.BaseTestCase): 'status': 'DELETED'}) @mock.patch.object(eventlet, 'spawn_n', autospec=True) - @mock.patch.object(client, 'Client', autospec=False) - def test_queue_events(self, mock_client, mock_spawn_n): - self.ironic_notifier = ironic.Notifier() + def test_queue_events(self, mock_spawn_n): port = get_fake_port() self.ironic_notifier.process_port_delete_event( 'fake_resource', 'fake_event', 'fake_trigger', original_port=None, @@ -193,31 +176,18 @@ class TestIronicNotifier(base.BaseTestCase): 2, len(self.ironic_notifier.batch_notifier._pending_events.queue)) self.assertEqual(2, mock_spawn_n.call_count) - @mock.patch.object(client, 'Client', autospec=False) + @mock.patch.object(connection.Connection, 'baremetal', autospec=True) def test_send_events(self, mock_client): - self.ironic_notifier = ironic.Notifier() self.ironic_notifier.irclient = mock_client self.ironic_notifier.send_events(['test', 'events']) - mock_client.events.create.assert_called_with(events=['test', 'events']) + mock_client.post.assert_called_with( + '/events', json={'events': ['test', 'events']}, + microversion='1.54') - @mock.patch.object(ironic.LOG, 'error', autospec=True) - @mock.patch.object(client, 'Client', autospec=False) - def test_send_event_method_not_found(self, mock_client, mock_log): - self.ironic_notifier = ironic.Notifier() - self.ironic_notifier.irclient = mock_client - exception = ironic_exc.NotFound() - mock_client.events.create.side_effect = exception - self.ironic_notifier.send_events(['test', 'events']) - self.assertEqual(1, mock_log.call_count) - mock_log.assert_called_with('The ironic API appears to not support ' - 'posting events. The API likely needs to ' - 'be upgraded.') - - @mock.patch.object(ironic.LOG, 'error', autospec=True) - @mock.patch.object(client, 'Client', autospec=False) + @mock.patch.object(ironic.LOG, 'exception', autospec=True) + @mock.patch.object(connection.Connection, 'baremetal', autospec=True) def test_send_event_exception(self, mock_client, mock_log): - self.ironic_notifier = ironic.Notifier() self.ironic_notifier.irclient = mock_client - mock_client.events.create.side_effect = Exception() + mock_client.post.side_effect = Exception() self.ironic_notifier.send_events(['test', 'events']) self.assertEqual(1, mock_log.call_count) diff --git a/requirements.txt b/requirements.txt index 9ad1fdc50ec..4ba3f71160f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -52,7 +52,7 @@ pyroute2>=0.5.3;sys_platform!='win32' # Apache-2.0 (+ dual licensed GPL2) weakrefmethod>=1.0.2;python_version=='2.7' # PSF python-novaclient>=9.1.0 # Apache-2.0 -python-ironicclient!=2.7.1,>=2.7.0 # Apache-2.0 +openstacksdk>=0.31.2 # Apache-2.0 python-designateclient>=2.7.0 # Apache-2.0 os-xenapi>=0.3.1 # Apache-2.0 os-vif>=1.15.1 # Apache-2.0 diff --git a/test-requirements.txt b/test-requirements.txt index b8397810e8c..3aaac91e361 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -8,7 +8,7 @@ coverage!=4.4,>=4.0 # Apache-2.0 fixtures>=3.0.0 # Apache-2.0/BSD flake8-import-order==0.12 # LGPLv3 pycodestyle>=2.0.0 # MIT -mock>=2.0.0 # BSD +mock>=3.0.0 # BSD python-subunit>=1.0.0 # Apache-2.0/BSD testtools>=2.2.0 # MIT testresources>=2.0.0 # Apache-2.0/BSD