From 9fd2240c97f5ac6b3e7b3d60d4de00e5493d501b Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Mon, 22 Jun 2015 10:36:24 -0400 Subject: [PATCH] register-nodes: drop support for Nova baremetal This patch drops support for using the deprecated (no longer in tree) Nova baremetal service. By removing this code we can also re-enable the C901 complexity pep8 check. Change-Id: I5844604d2d83ea34369023dbe0083cb9ab3c300f --- os_cloud_config/cmd/register_nodes.py | 5 +- .../cmd/tests/test_register_nodes.py | 40 ----- os_cloud_config/nodes.py | 154 ++++-------------- os_cloud_config/tests/test_nodes.py | 132 +-------------- tox.ini | 2 +- 5 files changed, 41 insertions(+), 292 deletions(-) diff --git a/os_cloud_config/cmd/register_nodes.py b/os_cloud_config/cmd/register_nodes.py index 2ff1d03..63bf1ac 100644 --- a/os_cloud_config/cmd/register_nodes.py +++ b/os_cloud_config/cmd/register_nodes.py @@ -71,10 +71,7 @@ def main(): keystone_client = _clients.get_keystone_client() glance_client = _clients.get_glance_client() - if nodes.using_ironic(keystone=keystone_client): - client = _clients.get_ironic_client() - else: - client = _clients.get_nova_bm_client() + client = _clients.get_ironic_client() nodes.register_all_nodes( args.service_host, nodes_list, client=client, remove=args.remove, diff --git a/os_cloud_config/cmd/tests/test_register_nodes.py b/os_cloud_config/cmd/tests/test_register_nodes.py index fb41fc5..a254f08 100644 --- a/os_cloud_config/cmd/tests/test_register_nodes.py +++ b/os_cloud_config/cmd/tests/test_register_nodes.py @@ -24,49 +24,12 @@ from os_cloud_config.tests import base class RegisterNodesTest(base.TestCase): - @mock.patch('os_cloud_config.cmd.utils._clients.get_glance_client', - return_value='glance_client_mock') - @mock.patch('os_cloud_config.cmd.utils._clients.get_nova_bm_client', - return_value='nova_bm_client_mock') - @mock.patch('os_cloud_config.cmd.utils._clients.get_keystone_client', - return_value='keystone_client_mock') - @mock.patch('os_cloud_config.nodes.using_ironic', return_value=False) - @mock.patch('os_cloud_config.nodes.register_all_nodes') - @mock.patch.dict('os.environ', {'OS_USERNAME': 'a', 'OS_PASSWORD': 'a', - 'OS_TENANT_NAME': 'a', 'OS_AUTH_URL': 'a'}) - @mock.patch.object(sys, 'argv', ['register-nodes', '--service-host', - 'seed', '--nodes']) - def test_with_arguments_nova_baremetal(self, register_mock, - using_ironic_mock, - get_keystone_client_mock, - get_nova_bm_client_mock, - get_glance_client_mock): - with tempfile.NamedTemporaryFile() as f: - f.write(u'{}\n'.encode('utf-8')) - f.flush() - sys.argv.append(f.name) - return_code = register_nodes.main() - - register_mock.assert_called_once_with( - "seed", {}, client='nova_bm_client_mock', remove=False, - blocking=True, keystone_client='keystone_client_mock', - glance_client='glance_client_mock', kernel_name=None, - ramdisk_name=None) - using_ironic_mock.assert_called_once_with( - keystone='keystone_client_mock') - get_keystone_client_mock.assert_called_once_with() - get_nova_bm_client_mock.assert_called_once_with() - get_glance_client_mock.assert_called_once_with() - - self.assertEqual(0, return_code) - @mock.patch('os_cloud_config.cmd.utils._clients.get_glance_client', return_value='glance_client_mock') @mock.patch('os_cloud_config.cmd.utils._clients.get_ironic_client', return_value='ironic_client_mock') @mock.patch('os_cloud_config.cmd.utils._clients.get_keystone_client', return_value='keystone_client_mock') - @mock.patch('os_cloud_config.nodes.using_ironic', return_value=True) @mock.patch('os_cloud_config.nodes.register_all_nodes') @mock.patch.dict('os.environ', {'OS_USERNAME': 'a', 'OS_PASSWORD': 'a', 'OS_TENANT_NAME': 'a', 'OS_AUTH_URL': 'a'}) @@ -74,7 +37,6 @@ class RegisterNodesTest(base.TestCase): 'seed', '--ramdisk-name', 'bm-ramdisk', '--kernel-name', 'bm-kernel', '--nodes']) def test_with_arguments_ironic(self, register_mock, - using_ironic_mock, get_keystone_client_mock, get_ironic_client_mock, get_glance_client_mock): @@ -89,8 +51,6 @@ class RegisterNodesTest(base.TestCase): blocking=True, keystone_client='keystone_client_mock', glance_client='glance_client_mock', kernel_name='bm-kernel', ramdisk_name='bm-ramdisk') - using_ironic_mock.assert_called_once_with( - keystone='keystone_client_mock') get_keystone_client_mock.assert_called_once_with() get_ironic_client_mock.assert_called_once_with() get_glance_client_mock.assert_called_once_with() diff --git a/os_cloud_config/nodes.py b/os_cloud_config/nodes.py index ce7e2f5..cc4fe29 100644 --- a/os_cloud_config/nodes.py +++ b/os_cloud_config/nodes.py @@ -17,7 +17,6 @@ import logging import time from ironicclient.openstack.common.apiclient import exceptions as ironicexp -from novaclient.openstack.common.apiclient import exceptions as novaexc import six from os_cloud_config.cmd.utils import _clients as clients @@ -26,46 +25,6 @@ from os_cloud_config import glance LOG = logging.getLogger(__name__) -def register_nova_bm_node(service_host, node, client=None, blocking=True): - if not service_host: - raise ValueError("Nova-baremetal requires a service host.") - kwargs = {'pm_address': node["pm_addr"], 'pm_user': node["pm_user"]} - # Nova now enforces the password to be 255 or less characters, and the - # ssh key/password to use is set in configuration. - if not node.get('pm_password'): - LOG.debug('pm_password not set.') - elif len(node["pm_password"]) <= 255: - LOG.debug('Setting pm_password for nova-bm, it is <=255 characters.') - kwargs["pm_password"] = node["pm_password"] - else: - LOG.info('Ignoring pm_password for nova-bm, it is >255 characters.') - for count in range(60): - LOG.debug('Registering %s node with nova-baremetal, try #%d.' % - (node["pm_addr"], count)) - try: - bm_node = client.baremetal.create(service_host, - six.text_type(node["cpu"]), - six.text_type(node["memory"]), - six.text_type(node["disk"]), - node["mac"][0], **kwargs) - break - except (novaexc.ConnectionRefused, novaexc.ServiceUnavailable): - if blocking: - LOG.debug('Service not available, sleeping for 10 seconds.') - time.sleep(10) - else: - LOG.debug('Service not available.') - else: - if blocking: - LOG.debug('Service unavailable after 10 minutes, giving up.') - else: - LOG.debug('Service unavailable after 60 tries, giving up.') - raise novaexc.ServiceUnavailable() - for mac in node["mac"][1:]: - client.baremetal.add_interface(bm_node, mac) - return bm_node - - def _extract_driver_info(node): if "ipmi" in node["pm_type"]: driver_info = {"ipmi_address": node["pm_addr"], @@ -154,40 +113,33 @@ def register_ironic_node(service_host, node, client=None, blocking=True): return ironic_node -def _populate_node_mapping(ironic_in_use, client): +def _populate_node_mapping(client): LOG.debug('Populating list of registered nodes.') node_map = {'mac': {}, 'pm_addr': {}} - if ironic_in_use: - nodes = [n.to_dict() for n in client.node.list()] - for node in nodes: - node_details = client.node.get(node['uuid']) - if node_details.driver == 'pxe_ssh': - for port in client.node.list_ports(node['uuid']): - node_map['mac'][port.address] = node['uuid'] - elif 'ipmi' in node_details.driver: - pm_addr = node_details.driver_info['ipmi_address'] - node_map['pm_addr'][pm_addr] = node['uuid'] - elif node_details.driver == 'pxe_ilo': - pm_addr = node_details.driver_info['ilo_address'] - node_map['pm_addr'][pm_addr] = node['uuid'] - elif node_details.driver == 'pxe_drac': - pm_addr = node_details.driver_info['drac_host'] - node_map['pm_addr'][pm_addr] = node['uuid'] - elif node_details.driver == 'pxe_iboot': - iboot_addr = node_details.driver_info['iboot_address'] - if "iboot_port" in node_details.driver_info: - iboot_addr += (':%s' % - node_details.driver_info['iboot_port']) - if "iboot_relay_id" in node_details.driver_info: - iboot_addr += ('#%s' % - node_details.driver_info['iboot_relay_id']) - node_map['pm_addr'][iboot_addr] = node['uuid'] - else: - nodes = [bmn.to_dict() for bmn in client.baremetal.list()] - for node in nodes: - node_map['pm_addr'][node['pm_address']] = node['id'] - for addr in node['interfaces']: - node_map['mac'][addr['address']] = node['id'] + nodes = [n.to_dict() for n in client.node.list()] + for node in nodes: + node_details = client.node.get(node['uuid']) + if node_details.driver == 'pxe_ssh': + for port in client.node.list_ports(node['uuid']): + node_map['mac'][port.address] = node['uuid'] + elif 'ipmi' in node_details.driver: + pm_addr = node_details.driver_info['ipmi_address'] + node_map['pm_addr'][pm_addr] = node['uuid'] + elif node_details.driver == 'pxe_ilo': + pm_addr = node_details.driver_info['ilo_address'] + node_map['pm_addr'][pm_addr] = node['uuid'] + elif node_details.driver == 'pxe_drac': + pm_addr = node_details.driver_info['drac_host'] + node_map['pm_addr'][pm_addr] = node['uuid'] + elif node_details.driver == 'pxe_iboot': + iboot_addr = node_details.driver_info['iboot_address'] + if "iboot_port" in node_details.driver_info: + iboot_addr += (':%s' % + node_details.driver_info['iboot_port']) + if "iboot_relay_id" in node_details.driver_info: + iboot_addr += ('#%s' % + node_details.driver_info['iboot_relay_id']) + node_map['pm_addr'][iboot_addr] = node['uuid'] return node_map @@ -209,21 +161,6 @@ def _get_node_id(node, node_map): return node_map['pm_addr'][node['pm_addr']] -def _update_or_register_bm_node(service_host, node, node_map, client=None, - blocking=True): - bm_id = _get_node_id(node, node_map) - if bm_id: - bm_node = client.baremetal.get(bm_id) - else: - bm_node = None - if bm_node is None: - bm_node = register_nova_bm_node(service_host, node, client, - blocking=blocking) - else: - LOG.warning('Node %d already registered, skipping.' % bm_node.id) - return bm_node.id - - def _update_or_register_ironic_node(service_host, node, node_map, client=None, blocking=True): node_uuid = _get_node_id(node, node_map) @@ -282,13 +219,9 @@ def _update_or_register_ironic_node(service_host, node, node_map, client=None, return ironic_node.uuid -def _clean_up_extra_nodes(ironic_in_use, seen, client, remove=False): - if ironic_in_use: - all_nodes = set([n.uuid for n in client.node.list()]) - remove_func = client.node.delete - else: - all_nodes = set([bmn.id for bmn in client.baremetal.list()]) - remove_func = client.baremetal.delete +def _clean_up_extra_nodes(seen, client, remove=False): + all_nodes = set([n.uuid for n in client.node.list()]) + remove_func = client.node.delete extra_nodes = all_nodes - seen for node in extra_nodes: if remove: @@ -322,20 +255,12 @@ def register_all_nodes(service_host, nodes_list, client=None, remove=False, blocking=True, keystone_client=None, glance_client=None, kernel_name=None, ramdisk_name=None): LOG.debug('Registering all nodes.') - ironic_in_use = using_ironic(keystone=keystone_client) - if ironic_in_use: - if client is None: - LOG.warn('Creating ironic client inline is deprecated, please ' - 'pass the client as parameter.') - client = clients.get_ironic_client() - register_func = _update_or_register_ironic_node - else: - if client is None: - LOG.warn('Creating nova-bm client inline is deprecated, please ' - 'pass the client as parameter.') - client = clients.get_nova_bm_client() - register_func = _update_or_register_bm_node - node_map = _populate_node_mapping(ironic_in_use, client) + if client is None: + LOG.warn('Creating ironic client inline is deprecated, please ' + 'pass the client as parameter.') + client = clients.get_ironic_client() + register_func = _update_or_register_ironic_node + node_map = _populate_node_mapping(client) glance_ids = {'kernel': None, 'ramdisk': None} if kernel_name and ramdisk_name: if glance_client is None: @@ -347,13 +272,4 @@ def register_all_nodes(service_host, nodes_list, client=None, remove=False, seen = _register_list_of_nodes(register_func, node_map, client, nodes_list, blocking, service_host, glance_ids['kernel'], glance_ids['ramdisk']) - _clean_up_extra_nodes(ironic_in_use, seen, client, remove=remove) - - -def using_ironic(keystone=None): - LOG.debug('Checking for usage of ironic.') - if keystone is None: - LOG.warn('Creating keystone client inline is deprecated, please pass ' - 'the client as parameter.') - keystone = clients.get_keystone_client() - return 'ironic' in [service.name for service in keystone.services.list()] + _clean_up_extra_nodes(seen, client, remove=remove) diff --git a/os_cloud_config/tests/test_nodes.py b/os_cloud_config/tests/test_nodes.py index e3dda9e..8c4bdf1 100644 --- a/os_cloud_config/tests/test_nodes.py +++ b/os_cloud_config/tests/test_nodes.py @@ -17,7 +17,6 @@ import collections from ironicclient.openstack.common.apiclient import exceptions as ironicexp import mock -from novaclient.openstack.common.apiclient import exceptions as novaexc from testtools import matchers from os_cloud_config import nodes @@ -32,18 +31,6 @@ class NodesTest(base.TestCase): 'pm_password': 'random', 'pm_type': 'pxe_ssh', 'name': 'node1', 'capabilities': 'num_nics:6'} - @mock.patch('os_cloud_config.nodes.using_ironic', return_value=False) - def test_register_all_nodes_nova_bm(self, ironic_mock): - node_list = [self._get_node(), self._get_node()] - node_list[0]["mac"].append("bbb") - client = mock.MagicMock() - nodes.register_all_nodes('servicehost', node_list, client=client) - nova_bm_call = mock.call( - "servicehost", "1", "2048", "30", "aaa", pm_address="foo.bar", - pm_user="test", pm_password="random") - client.baremetal.create.has_calls([nova_bm_call, nova_bm_call]) - client.baremetal.add_interface.assert_called_once_with(mock.ANY, "bbb") - def test_register_list_of_nodes(self): nodes_list = ['aaa', 'bbb'] return_node = nodes_list[0] @@ -54,85 +41,6 @@ class NodesTest(base.TestCase): None, None) self.assertEqual(seen, set(nodes_list)) - @mock.patch('time.sleep') - def test_register_nova_bm_node_retry(self, sleep): - client = mock.MagicMock() - side_effect = (novaexc.ConnectionRefused, - novaexc.ServiceUnavailable, mock.DEFAULT) - client.baremetal.create.side_effect = side_effect - nodes.register_nova_bm_node('servicehost', - self._get_node(), client=client) - sleep.assert_has_calls([mock.call(10), mock.call(10)]) - nova_bm_call = mock.call( - "servicehost", "1", "2048", "30", "aaa", pm_address="foo.bar", - pm_user="test", pm_password="random") - client.has_calls([nova_bm_call]) - - @mock.patch('os_cloud_config.nodes.using_ironic', return_value=False) - @mock.patch('os_cloud_config.nodes.register_nova_bm_node') - def test_register_nova_bm_node_no_update(self, ironic_mock, register_mock): - client = mock.MagicMock() - node_map = {'mac': {'aaa': 1}} - nodes._update_or_register_bm_node('servicehost', self._get_node(), - node_map, client=client) - client.baremetal.get.assert_called_once_with(1) - register_mock.assert_not_called() - - @mock.patch('time.sleep') - def test_register_nova_bm_node_failure(self, sleep): - client = mock.MagicMock() - client.baremetal.create.side_effect = novaexc.ConnectionRefused - self.assertRaises(novaexc.ServiceUnavailable, - nodes.register_nova_bm_node, 'servicehost', - self._get_node(), client=client) - - def test_register_nova_bm_node_ignore_long_pm_password(self): - client = mock.MagicMock() - node = self._get_node() - node["pm_password"] = 'abc' * 100 - nodes.register_nova_bm_node('servicehost', node, client=client) - nova_bm_call = mock.call( - "servicehost", "1", "2048", "30", "aaa", pm_address="foo.bar", - pm_user="test") - client.has_calls([nova_bm_call]) - - def assert_nova_bm_call_with_no_pm_password(self, node): - client = mock.MagicMock() - nodes.register_nova_bm_node('servicehost', node, client=client) - nova_bm_call = mock.call( - "servicehost", "1", "2048", "30", "aaa", pm_address="foo.bar", - pm_user="test") - client.has_calls([nova_bm_call]) - - def test_register_nova_bm_node_no_pm_password(self): - node = self._get_node() - del node["pm_password"] - self.assert_nova_bm_call_with_no_pm_password(node) - - def test_register_nova_bm_node_pm_password_of_none(self): - node = self._get_node() - node["pm_password"] = None - self.assert_nova_bm_call_with_no_pm_password(node) - - def test_register_nova_bm_node_pm_password_of_empty_string(self): - node = self._get_node() - node["pm_password"] = "" - self.assert_nova_bm_call_with_no_pm_password(node) - - def test_register_nova_bm_node_int_values(self): - node = self._get_node() - node['cpu'] = 1 - node['memory'] = 2048 - node['disk'] = 30 - client = mock.MagicMock() - nodes.register_nova_bm_node('service_host', node, client=client) - client.baremetal.create.assert_called_once_with('service_host', '1', - '2048', '30', - node["mac"][0], - pm_password='random', - pm_address='foo.bar', - pm_user='test') - def test_extract_driver_info_ipmi(self): node = self._get_node() node["pm_type"] = "ipmi" @@ -213,8 +121,7 @@ class NodesTest(base.TestCase): node["pm_type"] = "unknown_type" self.assertRaises(ValueError, nodes._extract_driver_info, node) - @mock.patch('os_cloud_config.nodes.using_ironic', return_value=True) - def test_register_all_nodes_ironic(self, using_ironic): + def test_register_all_nodes_ironic(self): node_list = [self._get_node()] node_properties = {"cpus": "1", "memory_mb": "2048", @@ -234,13 +141,11 @@ class NodesTest(base.TestCase): port_call = mock.call(node_uuid=ironic.node.create.return_value.uuid, address='aaa') power_off_call = mock.call(ironic.node.create.return_value.uuid, 'off') - using_ironic.assert_called_once_with(keystone=None) ironic.node.create.assert_has_calls([pxe_node, mock.ANY]) ironic.port.create.assert_has_calls([port_call]) ironic.node.set_power_state.assert_has_calls([power_off_call]) - @mock.patch('os_cloud_config.nodes.using_ironic', return_value=True) - def test_register_all_nodes_ironic_kernel_ramdisk(self, using_ironic): + def test_register_all_nodes_ironic_kernel_ramdisk(self): node_list = [self._get_node()] node_properties = {"cpus": "1", "memory_mb": "2048", @@ -268,7 +173,6 @@ class NodesTest(base.TestCase): port_call = mock.call(node_uuid=ironic.node.create.return_value.uuid, address='aaa') power_off_call = mock.call(ironic.node.create.return_value.uuid, 'off') - using_ironic.assert_called_once_with(keystone=None) ironic.node.create.assert_has_calls([pxe_node, mock.ANY]) ironic.port.create.assert_has_calls([port_call]) ironic.node.set_power_state.assert_has_calls([power_off_call]) @@ -428,20 +332,6 @@ class NodesTest(base.TestCase): nodes._update_or_register_ironic_node(None, node, node_map, client=ironic) - def test_populate_node_mapping_nova_bm(self): - client = mock.MagicMock() - node1 = mock.MagicMock() - node1.to_dict.return_value = {'id': '1', - 'interfaces': [{'address': 'aaa'}], - 'pm_address': '10.0.1.5'} - node4 = mock.MagicMock() - node4.to_dict.return_value = {'id': '4', 'interfaces': [], - 'pm_address': '10.0.1.2'} - client.baremetal.list.return_value = [node1, node4] - expected = {'mac': {'aaa': '1'}, - 'pm_addr': {'10.0.1.2': '4', '10.0.1.5': '1'}} - self.assertEqual(expected, nodes._populate_node_mapping(False, client)) - def test_populate_node_mapping_ironic(self): client = mock.MagicMock() node1 = mock.MagicMock() @@ -459,25 +349,11 @@ class NodesTest(base.TestCase): client.node.list.return_value = [node1, node2] expected = {'mac': {'aaa': 'abcdef'}, 'pm_addr': {'10.0.1.2': 'fedcba'}} - self.assertEqual(expected, nodes._populate_node_mapping(True, client)) - - def test_clean_up_extra_nodes_nova_bm(self): - node = collections.namedtuple('node', ['id']) - client = mock.MagicMock() - client.baremetal.list.return_value = [node('4')] - nodes._clean_up_extra_nodes(False, set((1,)), client, remove=True) - client.baremetal.delete.assert_called_once_with('4') + self.assertEqual(expected, nodes._populate_node_mapping(client)) def test_clean_up_extra_nodes_ironic(self): node = collections.namedtuple('node', ['uuid']) client = mock.MagicMock() client.node.list.return_value = [node('foobar')] - nodes._clean_up_extra_nodes(True, set(('abcd',)), client, remove=True) + nodes._clean_up_extra_nodes(set(('abcd',)), client, remove=True) client.node.delete.assert_called_once_with('foobar') - - def test_using_ironic(self): - keystone = mock.MagicMock() - service = collections.namedtuple('servicelist', ['name']) - keystone.services.list.return_value = [service('compute')] - self.assertFalse(nodes.using_ironic(keystone=keystone)) - self.assertEqual(1, keystone.services.list.call_count) diff --git a/tox.ini b/tox.ini index 8ffb965..6da63ca 100644 --- a/tox.ini +++ b/tox.ini @@ -30,7 +30,7 @@ commands = python setup.py test --coverage --coverage-package-name='os_cloud_con # E123, E125 skipped as they are invalid PEP-8. show-source = True -ignore = E123,E125,H302,H803,C901 +ignore = E123,E125,H302,H803 builtins = _ exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build max-complexity=14