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

@@ -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

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()
 
 

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).')),
 ]
 
 

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
+        global IRONIC_SESSION
-        if cfg.CONF.ironic.auth_strategy == 'noauth':
+
-            args = {'token': 'noauth',
+        if not IRONIC_SESSION:
-                    'endpoint': cfg.CONF.ironic.ironic_url}
+            IRONIC_SESSION = self._get_session(IRONIC_CONF_SECTION)
-        else:
+
-            global IRONIC_SESSION
+        return connection.Connection(
-            if not IRONIC_SESSION:
+            session=IRONIC_SESSION, oslo_conf=cfg.CONF).baremetal
-                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)
 
     def send_events(self, batched_events):
-        # NOTE(TheJulia): Friendly exception handling so operators
-        # can decouple updates.
         try:
-            self.irclient.events.create(events=batched_events)
+            response = self.irclient.post('/events',
-        except ironic_exc.NotFound:
+                                          json={'events': batched_events},
-            LOG.error('The ironic API appears to not support posting events. '
+                                          microversion='1.54')
-                      'The API likely needs to be upgraded.')
+            os_exc.raise_from_response(response)
         except Exception as e:
-            LOG.error('Unknown error encountered posting the event to '
+            LOG.exception('Error encountered posting the event to '
-                      'ironic. {error}'.format(error=e))
+                          'ironic. {error}'.format(error=e))
 
     @registry.receives(resources.PORT, [events.AFTER_UPDATE])
     def process_port_update_event(self, resource, event, trigger,

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_queue_event):
-    def test_process_port_update_event_bind_port(self, mock_client,
-                                                 mock_queue_event):
-        self.ironic_notifier = ironic.Notifier()
         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_queue_event):
-    def test_process_port_update_event_bind_port_err(self, mock_client,
-                                                     mock_queue_event):
-        self.ironic_notifier = ironic.Notifier()
         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_queue_event):
-    def test_process_port_update_event_unbind_port(self, mock_client,
-                                                   mock_queue_event):
-        self.ironic_notifier = ironic.Notifier()
         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_queue_event):
-    def test_process_port_update_event_unbind_port_err(self, mock_client,
-                                                       mock_queue_event):
-        self.ironic_notifier = ironic.Notifier()
         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_queue_event):
-    def test_process_port_delete_event(self, mock_client, mock_queue_event):
-        self.ironic_notifier = ironic.Notifier()
         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_queue_event):
-    def test_process_port_event_empty_uuid_field(self, mock_client,
-                                                 mock_queue_event):
-        self.ironic_notifier = ironic.Notifier()
         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_spawn_n):
-    def test_queue_events(self, mock_client, mock_spawn_n):
-        self.ironic_notifier = ironic.Notifier()
         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(ironic.LOG, 'exception', autospec=True)
-    @mock.patch.object(client, 'Client', autospec=False)
+    @mock.patch.object(connection.Connection, 'baremetal', autospec=True)
-    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)
     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)

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

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