From 09265ba4b53a186eca21e11bd6ddab2480292cb8 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 27 Jun 2016 16:53:31 +0200 Subject: [PATCH] Use new agent API if available Falls back to vendor passthru on receiving 404. Also fixes logging around lookup: log traceback on unexpected exceptions, log successful lookup and replace % with , Change-Id: I7160c99ca63585fc333482fa578fdf5e0962b2b6 Depends-On: I9080c07b03103cd7a323e2fc01be821733b07eea Partial-Bug: #1570841 --- ironic_python_agent/agent.py | 4 +- ironic_python_agent/ironic_api_client.py | 104 +++++++++---- ironic_python_agent/tests/unit/test_agent.py | 16 +- .../tests/unit/test_ironic_api_client.py | 139 ++++++++---------- .../notes/new-agent-api-afbe7391493749be.yaml | 3 + 5 files changed, 158 insertions(+), 108 deletions(-) create mode 100644 releasenotes/notes/new-agent-api-afbe7391493749be.yaml diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index 0a1799aa6..0ff3aa67e 100644 --- a/ironic_python_agent/agent.py +++ b/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], diff --git a/ironic_python_agent/ironic_api_client.py b/ironic_python_agent/ironic_api_client.py index 8f494f4fa..651a4ede9 100644 --- a/ironic_python_agent/ironic_api_client.py +++ b/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, - data=data) + headers=headers, + 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( - api_version=self.api_version, - uuid=uuid - ) - data = { - 'agent_url': self._get_agent_url(advertise_address) - } + 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): - """The actual call to lookup a node. + def _do_new_lookup(self, hardware_info, node_uuid): + 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`. - """ - path = '/{api_version}/drivers/{driver}/vendor_passthru/lookup'.format( - api_version=self.api_version, - driver=self.driver_name - ) + response = self._request('GET', self.lookup_api, + headers=self.ramdisk_api_headers, + params=params) + if response.status_code == requests.codes.NOT_FOUND: + # 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) - except Exception as e: - LOG.warning('POST failed: %s' % str(e)) + response = (self._do_new_lookup(hardware_info, node_uuid) + if self.use_ramdisk_api + 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,9 +175,14 @@ class APIClient(object): LOG.warning('Got invalid node data from the API: %s' % content) return False - if 'heartbeat_timeout' not in content: - LOG.warning('Got invalid heartbeat from the API: %s' % content) - return False + 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 # Got valid content raise loopingcall.LoopingCallDone(retvalue=content) diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index 96ca0a95b..ec5772b79 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/ironic_python_agent/tests/unit/test_agent.py @@ -186,7 +186,9 @@ class TestBaseAgent(test_base.BaseTestCase): 'node': { 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' }, - 'heartbeat_timeout': 300 + 'config': { + 'heartbeat_timeout': 300 + } } self.agent.run() @@ -224,7 +226,9 @@ class TestBaseAgent(test_base.BaseTestCase): 'node': { 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' }, - 'heartbeat_timeout': 300, + 'config': { + 'heartbeat_timeout': 300, + } } self.agent.run() @@ -285,7 +289,9 @@ class TestBaseAgent(test_base.BaseTestCase): 'node': { 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' }, - 'heartbeat_timeout': 300 + 'config': { + 'heartbeat_timeout': 300 + } } self.agent.run() @@ -392,7 +398,9 @@ class TestAgentStandalone(test_base.BaseTestCase): 'node': { 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' }, - 'heartbeat_timeout': 300 + 'config': { + 'heartbeat_timeout': 300 + } } self.agent.run() diff --git a/ironic_python_agent/tests/unit/test_ironic_api_client.py b/ironic_python_agent/tests/unit/test_ironic_api_client.py index ed7d0ea9e..d43cb1f0f 100644 --- a/ironic_python_agent/tests/unit/test_ironic_api_client.py +++ b/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/' \ - 'vendor_passthru/heartbeat' + heartbeat_path = 'v1/heartbeat/deadbeef-dabb-ad00-b105-f00d00bab10c' + 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,7 +126,9 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase): 'node': { 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' }, - 'heartbeat_timeout': 300 + 'config': { + 'heartbeat_timeout': 300 + } } lookup_mock.side_effect = loopingcall.LoopingCallDone( retvalue=content) @@ -135,7 +154,9 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase): 'node': { 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' }, - 'heartbeat_timeout': 300 + '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( - api_url=API_URL, driver=DRIVER) + url = '{api_url}v1/lookup'.format(api_url=API_URL) 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'] - content = json.loads(data) - self.assertNotIn('node_uuid', content) - 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'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'cpu': { - u'model_name': u'Awesome Jay CPU x10 9001', - u'frequency': u'9001', - u'count': u'10', - u'architecture': u'ARMv9', - u'flags': [], + def test_do_lookup_with_uuid(self): + response = FakeResponse(status_code=200, content={ + 'node': { + 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' }, - 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']) + 'config': { + 'heartbeat_timeout': 300 + } + }) + + self.api_client.session.request = mock.Mock() + self.api_client.session.request.return_value = response + + self.assertRaises(loopingcall.LoopingCallDone, + self.api_client._do_lookup, + hardware_info=self.hardware_info, + node_uuid='someuuid') + + url = '{api_url}v1/lookup'.format(api_url=API_URL) + request_args = self.api_client.session.request.call_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', + 'node_uuid': 'someuuid'}, + params) 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) diff --git a/releasenotes/notes/new-agent-api-afbe7391493749be.yaml b/releasenotes/notes/new-agent-api-afbe7391493749be.yaml new file mode 100644 index 000000000..7e7458631 --- /dev/null +++ b/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.