Merge "Use new agent API if available"

ironic_python_agent/agent.py

@@ -336,9 +336,11 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
                 starting_interval=self.lookup_interval,
                 node_uuid=uuid)
 
+            LOG.debug('Received lookup results: %s', content)
             self.node = content['node']
+            LOG.info('Lookup succeeded, node UUID is %s', self.node['uuid'])
             hardware.cache_node(self.node)
-            self.heartbeat_timeout = content['heartbeat_timeout']
+            self.heartbeat_timeout = content['config']['heartbeat_timeout']
 
         wsgi = simple_server.make_server(
             self.listen_address[0],

ironic_python_agent/ironic_api_client.py

@@ -28,6 +28,15 @@ 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):
         self.api_url = api_url.rstrip('/')
@@ -43,32 +52,40 @@ class APIClient(object):
 
         self.encoder = encoding.RESTJSONEncoder()
 
-    def _request(self, method, path, data=None):
+    def _request(self, method, path, data=None, headers=None, **kwargs):
         request_url = '{api_url}{path}'.format(api_url=self.api_url, path=path)
 
         if data is not None:
             data = self.encoder.encode(data)
 
-        request_headers = {
+        headers = headers or {}
+        headers.update({
             'Content-Type': 'application/json',
             'Accept': 'application/json',
-        }
+        })
 
         return self.session.request(method,
                                     request_url,
-                                    headers=request_headers,
+                                    headers=headers,
-                                    data=data)
+                                    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):
-        path = '/{api_version}/nodes/{uuid}/vendor_passthru/heartbeat'.format(
+        agent_url = self._get_agent_url(advertise_address)
-            api_version=self.api_version,
-            uuid=uuid
-        )
-        data = {
-            'agent_url': self._get_agent_url(advertise_address)
-        }
         try:
-            response = self._request('POST', path, data=data)
+            response = self._heartbeat_request(uuid, agent_url)
         except Exception as e:
             raise errors.HeartbeatError(str(e))
 
@@ -93,15 +110,29 @@ class APIClient(object):
                                          'logs for details.')
         return node_content
 
-    def _do_lookup(self, hardware_info, node_uuid):
+    def _do_new_lookup(self, hardware_info, node_uuid):
-        """The actual call to lookup a node.
+        params = {
+            'addresses': ','.join(iface.mac_address
+                                  for iface in hardware_info['interfaces']
+                                  if iface.mac_address)
+        }
+        if node_uuid:
+            params['node_uuid'] = node_uuid
 
-        Should be called as a `loopingcall.BackOffLoopingCall`.
+        response = self._request('GET', self.lookup_api,
-        """
+                                 headers=self.ramdisk_api_headers,
-        path = '/{api_version}/drivers/{driver}/vendor_passthru/lookup'.format(
+                                 params=params)
-            api_version=self.api_version,
+        if response.status_code == requests.codes.NOT_FOUND:
-            driver=self.driver_name
+            # 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 = {
@@ -113,20 +144,30 @@ class APIClient(object):
 
         # 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._request('POST', path, data=data)
+            response = (self._do_new_lookup(hardware_info, node_uuid)
-        except Exception as e:
+                        if self.use_ramdisk_api
-            LOG.warning('POST failed: %s' % str(e))
+                        else self._do_passthru_lookup(hardware_info,
+                                                      node_uuid))
+        except Exception:
+            LOG.exception('Lookup failed')
             return False
 
         if response.status_code != requests.codes.OK:
-            LOG.warning('Invalid status code: %s' % response.status_code)
+            LOG.warning('Failure status code: %s', response.status_code)
             return False
 
         try:
             content = json.loads(response.content)
         except Exception as e:
-            LOG.warning('Error decoding response: %s' % str(e))
+            LOG.warning('Error decoding response: %s', e)
             return False
 
         # Check for valid response data
@@ -134,7 +175,12 @@ class APIClient(object):
             LOG.warning('Got invalid node data from the API: %s' % content)
             return False
 
-        if 'heartbeat_timeout' not in content:
+        if 'config' not in content:
+            # Old API
+            try:
+                content['config'] = {'heartbeat_timeout':
+                                     content.pop('heartbeat_timeout')}
+            except KeyError:
                 LOG.warning('Got invalid heartbeat from the API: %s' % content)
                 return False
 

ironic_python_agent/tests/unit/test_agent.py

@@ -186,8 +186,10 @@ class TestBaseAgent(test_base.BaseTestCase):
             'node': {
                 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c'
             },
+            'config': {
                 'heartbeat_timeout': 300
             }
+        }
         self.agent.run()
 
         listen_addr = ('192.0.2.1', 9999)
@@ -224,8 +226,10 @@ class TestBaseAgent(test_base.BaseTestCase):
             'node': {
                 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c'
             },
+            'config': {
                 'heartbeat_timeout': 300,
             }
+        }
         self.agent.run()
 
         listen_addr = ('192.0.2.1', 9999)
@@ -286,8 +290,10 @@ class TestBaseAgent(test_base.BaseTestCase):
             'node': {
                 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c'
             },
+            'config': {
                 'heartbeat_timeout': 300
             }
+        }
         self.agent.run()
 
         listen_addr = ('192.0.2.1', 9999)
@@ -394,8 +400,10 @@ class TestAgentStandalone(test_base.BaseTestCase):
             'node': {
                 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c'
             },
+            'config': {
                 'heartbeat_timeout': 300
             }
+        }
         self.agent.run()
 
         listen_addr = ('192.0.2.1', 9999)

ironic_python_agent/tests/unit/test_ironic_api_client.py

@@ -67,8 +67,25 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
             advertise_address=('192.0.2.1', '9999')
         )
 
-        heartbeat_path = 'v1/nodes/deadbeef-dabb-ad00-b105-f00d00bab10c/' \
+        heartbeat_path = 'v1/heartbeat/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_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])
@@ -109,8 +126,10 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
             'node': {
                 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c'
             },
+            'config': {
                 'heartbeat_timeout': 300
             }
+        }
         lookup_mock.side_effect = loopingcall.LoopingCallDone(
             retvalue=content)
         returned_content = self.api_client.lookup_node(
@@ -135,7 +154,9 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
             'node': {
                 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c'
             },
+            'config': {
                 'heartbeat_timeout': 300
+            }
         })
 
         self.api_client.session.request = mock.Mock()
@@ -146,74 +167,41 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
                           hardware_info=self.hardware_info,
                           node_uuid=None)
 
-        url = '{api_url}v1/drivers/{driver}/vendor_passthru/lookup'.format(
+        url = '{api_url}v1/lookup'.format(api_url=API_URL)
-            api_url=API_URL, driver=DRIVER)
         request_args = self.api_client.session.request.call_args[0]
-        self.assertEqual('POST', request_args[0])
+        self.assertEqual('GET', request_args[0])
         self.assertEqual(url, request_args[1])
+        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)
 
-        data = self.api_client.session.request.call_args[1]['data']
+    def test_do_lookup_with_uuid(self):
-        content = json.loads(data)
+        response = FakeResponse(status_code=200, content={
-        self.assertNotIn('node_uuid', content)
+            'node': {
-        self.assertEqual(self.api_client.payload_version, content['version'])
+                'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c'
-        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,
             },
-                {
+            'config': {
-                    u'mac_address': u'00:0c:29:8c:11:b2',
+                'heartbeat_timeout': 300
-                    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'cpu': {
+
-                u'model_name': u'Awesome Jay CPU x10 9001',
+        self.api_client.session.request = mock.Mock()
-                u'frequency': u'9001',
+        self.api_client.session.request.return_value = response
-                u'count': u'10',
+
-                u'architecture': u'ARMv9',
+        self.assertRaises(loopingcall.LoopingCallDone,
-                u'flags': [],
+                          self.api_client._do_lookup,
-            },
+                          hardware_info=self.hardware_info,
-            u'disks': [
+                          node_uuid='someuuid')
-                {
+
-                    u'model': u'small',
+        url = '{api_url}v1/lookup'.format(api_url=API_URL)
-                    u'name': u'/dev/sdj',
+        request_args = self.api_client.session.request.call_args[0]
-                    u'rotational': False,
+        self.assertEqual('GET', request_args[0])
-                    u'size': u'9001',
+        self.assertEqual(url, request_args[1])
-                    u'serial': None,
+        params = self.api_client.session.request.call_args[1]['params']
-                    u'wwn': None,
+        self.assertEqual({'addresses': '00:0c:29:8c:11:b1,00:0c:29:8c:11:b2',
-                    u'wwn_with_extension': None,
+                          'node_uuid': 'someuuid'},
-                    u'wwn_vendor_extension': None,
+                         params)
-                    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'])
 
     def test_do_lookup_bad_response_code(self):
         response = FakeResponse(status_code=400, content={
@@ -271,7 +259,8 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
 
         self.assertFalse(error)
 
-    def test_do_lookup_with_node_uuid(self):
+    def test_do_lookup_fallback(self):
+        response0 = FakeResponse(status_code=404)
         response = FakeResponse(status_code=200, content={
             'node': {
                 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c'
@@ -280,22 +269,23 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
         })
 
         self.api_client.session.request = mock.Mock()
-        self.api_client.session.request.return_value = response
+        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='uuid')
+                          node_uuid=None)
 
         url = '{api_url}v1/drivers/{driver}/vendor_passthru/lookup'.format(
             api_url=API_URL, driver=DRIVER)
-        request_args = self.api_client.session.request.call_args[0]
+        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[1]['data']
+        data = self.api_client.session.request.call_args_list[1][1]['data']
         content = json.loads(data)
-        self.assertEqual('uuid', content['node_uuid'])
         self.assertEqual(self.api_client.payload_version, content['version'])
         self.assertEqual({
             u'interfaces': [
@@ -355,3 +345,4 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
                 u'physical_mb': u'8675'
             },
         }, content['inventory'])
+        self.assertFalse(self.api_client.use_ramdisk_api)

releasenotes/notes/new-agent-api-afbe7391493749be.yaml

@@ -0,0 +1,3 @@
+---
+features:
+  - Use new Ironic agent API (added in API version 1.22) when it's available.