From b033bfd933b1cd52b3493ce3b1962ba262a882b7 Mon Sep 17 00:00:00 2001 From: Pavlo Shchelokovskyy Date: Thu, 13 Oct 2016 18:23:20 +0300 Subject: [PATCH] Remove old lookup/heartbeat from IPA Lookup/Heartbeat via vendor passthru was deprecated in Newton. This patch removes the corresponding functionality from IPA, and also removes handling of 'ipa-driver-name' kernel parameter, as it was only used in code related to old passthru. Change-Id: I2c7989063ab3e4c0bae33f05d6d2ed857a2d9944 Closes-Bug: #1640533 --- ironic_python_agent/agent.py | 6 +- ironic_python_agent/cmd/agent.py | 1 - ironic_python_agent/config.py | 5 - ironic_python_agent/ironic_api_client.py | 75 ++--------- ironic_python_agent/tests/functional/base.py | 1 - ironic_python_agent/tests/unit/test_agent.py | 2 - .../tests/unit/test_ironic_api_client.py | 124 +----------------- ...move-vendor-passthru-eda3519c322eb4e2.yaml | 10 ++ 8 files changed, 26 insertions(+), 198 deletions(-) create mode 100644 releasenotes/notes/remove-vendor-passthru-eda3519c322eb4e2.yaml diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index d8afb8ec0..eb86f16f4 100644 --- a/ironic_python_agent/agent.py +++ b/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.driver_name) + self.api_client = ironic_api_client.APIClient(self.api_url) self.listen_address = listen_address self.advertise_address = advertise_address self.version = pkg_resources.get_distribution('ironic-python-agent')\ diff --git a/ironic_python_agent/cmd/agent.py b/ironic_python_agent/cmd/agent.py index 318c38826..432e9511c 100644 --- a/ironic_python_agent/cmd/agent.py +++ b/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() diff --git a/ironic_python_agent/config.py b/ironic_python_agent/config.py index 5613fc427..098190920 100644 --- a/ironic_python_agent/config.py +++ b/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)), diff --git a/ironic_python_agent/ironic_api_client.py b/ironic_python_agent/ironic_api_client.py index d4acfb25a..d55c6c7cd 100644 --- a/ironic_python_agent/ironic_api_client.py +++ b/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) - if self.use_ramdisk_api - else self._do_passthru_lookup(hardware_info, - node_uuid)) + response = self._request('GET', self.lookup_api, + headers=self.ramdisk_api_headers, + params=params) except Exception: LOG.exception('Lookup failed') return False diff --git a/ironic_python_agent/tests/functional/base.py b/ironic_python_agent/tests/functional/base.py index ab57bb110..066500a5a 100644 --- a/ironic_python_agent/tests/functional/base.py +++ b/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) diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index b008d633e..7c35ec682 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/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): 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 703ae9493..3c345a931 100644 --- a/ironic_python_agent/tests/unit/test_ironic_api_client.py +++ b/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) diff --git a/releasenotes/notes/remove-vendor-passthru-eda3519c322eb4e2.yaml b/releasenotes/notes/remove-vendor-passthru-eda3519c322eb4e2.yaml new file mode 100644 index 000000000..6d6b7e2c3 --- /dev/null +++ b/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).