Merge "Remove old lookup/heartbeat from IPA"

ironic_python_agent/agent.py

@@ -159,7 +159,7 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
 
     def __init__(self, api_url, advertise_address, listen_address,
                  ip_lookup_attempts, ip_lookup_sleep, network_interface,
-                 lookup_timeout, lookup_interval, driver_name, standalone,
+                 lookup_timeout, lookup_interval, standalone,
                  hardware_initialization_delay=0):
         super(IronicPythonAgent, self).__init__()
         self.ext_mgr = extension.ExtensionManager(
@@ -169,9 +169,7 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
             invoke_kwds={'agent': self},
         )
         self.api_url = api_url
-        self.driver_name = driver_name
+        self.api_client = ironic_api_client.APIClient(self.api_url)
-        self.api_client = ironic_api_client.APIClient(self.api_url,
-                                                      self.driver_name)
         self.listen_address = listen_address
         self.advertise_address = advertise_address
         self.version = pkg_resources.get_distribution('ironic-python-agent')\

ironic_python_agent/cmd/agent.py

@@ -44,6 +44,5 @@ def run():
                             CONF.network_interface,
                             CONF.lookup_timeout,
                             CONF.lookup_interval,
-                            CONF.driver_name,
                             CONF.standalone,
                             CONF.hardware_initialization_delay).run()

ironic_python_agent/config.py

@@ -82,11 +82,6 @@ cli_opts = [
                     'doubled after each failure until timeout is '
                     'exceeded.'),
 
-    cfg.StrOpt('driver_name',
-               default=APARAMS.get('ipa-driver-name', 'agent_ipmitool'),
-               deprecated_name='driver-name',
-               help='The Ironic driver in use for this node'),
-
     cfg.FloatOpt('lldp_timeout',
                  default=APARAMS.get('ipa-lldp-timeout',
                                      APARAMS.get('lldp-timeout', 30.0)),

ironic_python_agent/ironic_api_client.py

@@ -27,20 +27,12 @@ LOG = log.getLogger(__name__)
 
 class APIClient(object):
     api_version = 'v1'
-    payload_version = '2'
-    use_ramdisk_api = True
     lookup_api = '/%s/lookup' % api_version
     heartbeat_api = '/%s/heartbeat/{uuid}' % api_version
-    # TODO(dtanstur): drop support for old passthru in Ocata
-    lookup_passthru = ('/%s/drivers/{driver}/vendor_passthru/lookup'
-                       % api_version)
-    heartbeat_passthru = ('/%s/nodes/{uuid}/vendor_passthru/heartbeat'
-                          % api_version)
     ramdisk_api_headers = {'X-OpenStack-Ironic-API-Version': '1.22'}
 
-    def __init__(self, api_url, driver_name):
+    def __init__(self, api_url):
         self.api_url = api_url.rstrip('/')
-        self.driver_name = driver_name
 
         # Only keep alive a maximum of 2 connections to the API. More will be
         # opened if they are needed, but they will be closed immediately after
@@ -70,22 +62,12 @@ class APIClient(object):
                                     data=data,
                                     **kwargs)
 
-    def _heartbeat_request(self, uuid, agent_url):
-        if self.use_ramdisk_api:
-            path = self.heartbeat_api.format(uuid=uuid)
-            data = {'callback_url': agent_url}
-            headers = self.ramdisk_api_headers
-        else:
-            path = self.heartbeat_passthru.format(uuid=uuid)
-            data = {'agent_url': agent_url}
-            headers = None
-
-        return self._request('POST', path, data=data, headers=headers)
-
     def heartbeat(self, uuid, advertise_address):
-        agent_url = self._get_agent_url(advertise_address)
+        path = self.heartbeat_api.format(uuid=uuid)
+        data = {'callback_url': self._get_agent_url(advertise_address)}
         try:
-            response = self._heartbeat_request(uuid, agent_url)
+            response = self._request('POST', path, data=data,
+                                     headers=self.ramdisk_api_headers)
         except Exception as e:
             raise errors.HeartbeatError(str(e))
 
@@ -110,7 +92,11 @@ class APIClient(object):
                                          'logs for details.')
         return node_content
 
-    def _do_new_lookup(self, hardware_info, node_uuid):
+    def _do_lookup(self, hardware_info, node_uuid):
+        """The actual call to lookup a node.
+
+        Should be called as a `loopingcall.BackOffLoopingCall`.
+        """
         params = {
             'addresses': ','.join(iface.mac_address
                                   for iface in hardware_info['interfaces']
@@ -119,45 +105,10 @@ class APIClient(object):
         if node_uuid:
             params['node_uuid'] = node_uuid
 
-        response = self._request('GET', self.lookup_api,
-                                 headers=self.ramdisk_api_headers,
-                                 params=params)
-        if response.status_code in (requests.codes.NOT_FOUND,
-                                    requests.codes.UNAUTHORIZED,
-                                    requests.codes.NOT_ACCEPTABLE):
-            # Assume that new API is not available and retry
-            LOG.warning('New API is not available, falling back to old '
-                        'agent vendor passthru')
-            self.use_ramdisk_api = False
-            return self._do_passthru_lookup(hardware_info, node_uuid)
-
-        return response
-
-    def _do_passthru_lookup(self, hardware_info, node_uuid):
-        path = self.lookup_passthru.format(driver=self.driver_name)
-        # This hardware won't be saved on the node currently, because of
-        # how driver_vendor_passthru is implemented (no node saving).
-        data = {
-            'version': self.payload_version,
-            'inventory': hardware_info
-        }
-        if node_uuid:
-            data['node_uuid'] = node_uuid
-
-        # Make the POST, make sure we get back normal data/status codes and
-        # content
-        return self._request('POST', path, data=data)
-
-    def _do_lookup(self, hardware_info, node_uuid):
-        """The actual call to lookup a node.
-
-        Should be called as a `loopingcall.BackOffLoopingCall`.
-        """
         try:
-            response = (self._do_new_lookup(hardware_info, node_uuid)
+            response = self._request('GET', self.lookup_api,
-                        if self.use_ramdisk_api
+                                     headers=self.ramdisk_api_headers,
-                        else self._do_passthru_lookup(hardware_info,
+                                     params=params)
-                                                      node_uuid))
         except Exception:
             LOG.exception('Lookup failed')
             return False

ironic_python_agent/tests/functional/base.py

@@ -46,7 +46,6 @@ class FunctionalBase(test_base.BaseTestCase):
             network_interface=None,
             lookup_timeout=300,
             lookup_interval=1,
-            driver_name='agent_ipmitool',
             standalone=True)
         self.process = multiprocessing.Process(
             target=self.agent.run)

ironic_python_agent/tests/unit/test_agent.py

@@ -144,7 +144,6 @@ class TestBaseAgent(test_base.BaseTestCase):
                                              'eth0',
                                              300,
                                              1,
-                                             'agent_ipmitool',
                                              False)
         self.agent.ext_mgr = extension.ExtensionManager.\
             make_test_instance([extension.Extension('fake', None,
@@ -438,7 +437,6 @@ class TestAdvertiseAddress(test_base.BaseTestCase):
             network_interface=None,
             lookup_timeout=300,
             lookup_interval=1,
-            driver_name='agent_ipmitool',
             standalone=False)
 
     def test_advertise_address_provided(self, mock_exec, mock_gethostbyname):

ironic_python_agent/tests/unit/test_ironic_api_client.py

@@ -17,14 +17,12 @@ import json
 import mock
 from oslo_service import loopingcall
 from oslotest import base as test_base
-import requests
 
 from ironic_python_agent import errors
 from ironic_python_agent import hardware
 from ironic_python_agent import ironic_api_client
 
 API_URL = 'http://agent-api.ironic.example.org/'
-DRIVER = 'agent_ipmitool'
 
 
 class FakeResponse(object):
@@ -38,7 +36,7 @@ class FakeResponse(object):
 class TestBaseIronicPythonAgent(test_base.BaseTestCase):
     def setUp(self):
         super(TestBaseIronicPythonAgent, self).setUp()
-        self.api_client = ironic_api_client.APIClient(API_URL, DRIVER)
+        self.api_client = ironic_api_client.APIClient(API_URL)
         self.hardware_info = {
             'interfaces': [
                 hardware.NetworkInterface(
@@ -76,24 +74,6 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
         self.assertEqual('POST', request_args[0])
         self.assertEqual(API_URL + heartbeat_path, request_args[1])
 
-    def test_successful_heartbeat_old_api(self):
-        response = FakeResponse(status_code=202)
-
-        self.api_client.session.request = mock.Mock()
-        self.api_client.session.request.return_value = response
-
-        self.api_client.use_ramdisk_api = False
-        self.api_client.heartbeat(
-            uuid='deadbeef-dabb-ad00-b105-f00d00bab10c',
-            advertise_address=('192.0.2.1', '9999')
-        )
-
-        heartbeat_path = ('v1/nodes/deadbeef-dabb-ad00-b105-f00d00bab10c/'
-                          'vendor_passthru/heartbeat')
-        request_args = self.api_client.session.request.call_args[0]
-        self.assertEqual('POST', request_args[0])
-        self.assertEqual(API_URL + heartbeat_path, request_args[1])
-
     def test_heartbeat_requests_exception(self):
         self.api_client.session.request = mock.Mock()
         self.api_client.session.request.side_effect = Exception('api is down!')
@@ -178,7 +158,6 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
         params = self.api_client.session.request.call_args[1]['params']
         self.assertEqual({'addresses': '00:0c:29:8c:11:b1,00:0c:29:8c:11:b2'},
                          params)
-        self.assertTrue(self.api_client.use_ramdisk_api)
 
     def test_do_lookup_with_uuid(self):
         response = FakeResponse(status_code=200, content={
@@ -262,104 +241,3 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
                                            node_uuid=None)
 
         self.assertFalse(error)
-
-    def _test_do_lookup_fallback(self, error_code):
-        response0 = FakeResponse(status_code=error_code)
-        response = FakeResponse(status_code=200, content={
-            'node': {
-                'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c'
-            },
-            'heartbeat_timeout': 300
-        })
-
-        self.api_client.session.request = mock.Mock()
-        self.api_client.session.request.side_effect = [response0,
-                                                       response]
-
-        self.assertRaises(loopingcall.LoopingCallDone,
-                          self.api_client._do_lookup,
-                          hardware_info=self.hardware_info,
-                          node_uuid=None)
-
-        url = '{api_url}v1/drivers/{driver}/vendor_passthru/lookup'.format(
-            api_url=API_URL, driver=DRIVER)
-        self.assertEqual(2, self.api_client.session.request.call_count)
-        request_args = self.api_client.session.request.call_args_list[1][0]
-        self.assertEqual('POST', request_args[0])
-        self.assertEqual(url, request_args[1])
-
-        data = self.api_client.session.request.call_args_list[1][1]['data']
-        content = json.loads(data)
-        self.assertEqual(self.api_client.payload_version, content['version'])
-        self.assertEqual({
-            u'interfaces': [
-                {
-                    u'mac_address': u'00:0c:29:8c:11:b1',
-                    u'name': u'eth0',
-                    u'ipv4_address': None,
-                    u'switch_chassis_descr': None,
-                    u'switch_port_descr': None,
-                    u'has_carrier': True,
-                    u'lldp': None,
-                    u'vendor': u'0x15b3',
-                    u'product': u'0x1014'
-                },
-                {
-                    u'mac_address': u'00:0c:29:8c:11:b2',
-                    u'name': u'eth1',
-                    u'ipv4_address': None,
-                    u'switch_chassis_descr': None,
-                    u'switch_port_descr': None,
-                    u'has_carrier': True,
-                    u'lldp': [[1, u'04885a92ec5459'],
-                              [2, u'0545746865726e6574312f3138']],
-                    u'vendor': u'0x15b3',
-                    u'product': u'0x1014'
-                }
-            ],
-            u'cpu': {
-                u'model_name': u'Awesome Jay CPU x10 9001',
-                u'frequency': u'9001',
-                u'count': u'10',
-                u'architecture': u'ARMv9',
-                u'flags': [],
-            },
-            u'disks': [
-                {
-                    u'model': u'small',
-                    u'name': u'/dev/sdj',
-                    u'rotational': False,
-                    u'size': u'9001',
-                    u'serial': None,
-                    u'wwn': None,
-                    u'wwn_with_extension': None,
-                    u'wwn_vendor_extension': None,
-                    u'vendor': None,
-                },
-                {
-                    u'model': u'big',
-                    u'name': u'/dev/hdj',
-                    u'rotational': False,
-                    u'size': u'9002',
-                    u'serial': None,
-                    u'wwn': None,
-                    u'wwn_with_extension': None,
-                    u'wwn_vendor_extension': None,
-                    u'vendor': None,
-                }
-            ],
-            u'memory': {
-                u'total': u'8675309',
-                u'physical_mb': u'8675'
-            },
-        }, content['inventory'])
-        self.assertFalse(self.api_client.use_ramdisk_api)
-
-    def test_do_lookup_fallback_not_found(self):
-        self._test_do_lookup_fallback(error_code=requests.codes.NOT_FOUND)
-
-    def test_do_lookup_fallback_unauthorized(self):
-        self._test_do_lookup_fallback(error_code=requests.codes.UNAUTHORIZED)
-
-    def test_do_lookup_fallback_not_acceptable(self):
-        self._test_do_lookup_fallback(error_code=requests.codes.NOT_ACCEPTABLE)

releasenotes/notes/remove-vendor-passthru-eda3519c322eb4e2.yaml

@@ -0,0 +1,10 @@
+---
+upgrade:
+  - Ironic Python Agent no longer supports Ironic API lookup and heartbeat
+    actions via agent vendor passthru methods,
+    and uses only the new, top-level lookup and heartbeat endpoints of
+    Ironic API v1.22 (introduced in Newton release, Ironic version 6.1.0).
+
+    This effectively means that Ironic Python Agent becomes incompatible
+    with Ironic API < 1.22 (Ironic version 6.0.0 or older,
+    integrated OpenStack release Mitaka or older).