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

@@ -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()
-            client = _clients.get_ironic_client()
-        else:
-            client = _clients.get_nova_bm_client()
 
         nodes.register_all_nodes(
             args.service_host, nodes_list, client=client, remove=args.remove,

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()

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()]
-        nodes = [n.to_dict() for n in client.node.list()]
+    for node in nodes:
-        for node in nodes:
+        node_details = client.node.get(node['uuid'])
-            node_details = client.node.get(node['uuid'])
+        if node_details.driver == 'pxe_ssh':
-            if node_details.driver == 'pxe_ssh':
+            for port in client.node.list_ports(node['uuid']):
-                for port in client.node.list_ports(node['uuid']):
+                node_map['mac'][port.address] = node['uuid']
-                    node_map['mac'][port.address] = node['uuid']
+        elif 'ipmi' in node_details.driver:
-            elif 'ipmi' in node_details.driver:
+            pm_addr = node_details.driver_info['ipmi_address']
-                pm_addr = node_details.driver_info['ipmi_address']
+            node_map['pm_addr'][pm_addr] = node['uuid']
-                node_map['pm_addr'][pm_addr] = node['uuid']
+        elif node_details.driver == 'pxe_ilo':
-            elif node_details.driver == 'pxe_ilo':
+            pm_addr = node_details.driver_info['ilo_address']
-                pm_addr = node_details.driver_info['ilo_address']
+            node_map['pm_addr'][pm_addr] = node['uuid']
-                node_map['pm_addr'][pm_addr] = node['uuid']
+        elif node_details.driver == 'pxe_drac':
-            elif node_details.driver == 'pxe_drac':
+            pm_addr = node_details.driver_info['drac_host']
-                pm_addr = node_details.driver_info['drac_host']
+            node_map['pm_addr'][pm_addr] = node['uuid']
-                node_map['pm_addr'][pm_addr] = node['uuid']
+        elif node_details.driver == 'pxe_iboot':
-            elif node_details.driver == 'pxe_iboot':
+            iboot_addr = node_details.driver_info['iboot_address']
-                iboot_addr = node_details.driver_info['iboot_address']
+            if "iboot_port" in node_details.driver_info:
-                if "iboot_port" in node_details.driver_info:
+                iboot_addr += (':%s' %
-                    iboot_addr += (':%s' %
+                               node_details.driver_info['iboot_port'])
-                                   node_details.driver_info['iboot_port'])
+            if "iboot_relay_id" in node_details.driver_info:
-                if "iboot_relay_id" in node_details.driver_info:
+                iboot_addr += ('#%s' %
-                    iboot_addr += ('#%s' %
+                               node_details.driver_info['iboot_relay_id'])
-                                   node_details.driver_info['iboot_relay_id'])
+            node_map['pm_addr'][iboot_addr] = node['uuid']
-                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']
     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):
+def _clean_up_extra_nodes(seen, client, remove=False):
-    if ironic_in_use:
+    all_nodes = set([n.uuid for n in client.node.list()])
-        all_nodes = set([n.uuid for n in client.node.list()])
+    remove_func = client.node.delete
-        remove_func = client.node.delete
-    else:
-        all_nodes = set([bmn.id for bmn in client.baremetal.list()])
-        remove_func = client.baremetal.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 client is None:
-    if ironic_in_use:
+        LOG.warn('Creating ironic client inline is deprecated, please '
-        if client is None:
+                 'pass the client as parameter.')
-            LOG.warn('Creating ironic client inline is deprecated, please '
+        client = clients.get_ironic_client()
-                     'pass the client as parameter.')
+    register_func = _update_or_register_ironic_node
-            client = clients.get_ironic_client()
+    node_map = _populate_node_mapping(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)
     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)
+    _clean_up_extra_nodes(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()]

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):
-    def test_register_all_nodes_ironic(self, using_ironic):
         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):
-    def test_register_all_nodes_ironic_kernel_ramdisk(self, using_ironic):
         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))
+        self.assertEqual(expected, nodes._populate_node_mapping(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')
 
     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)

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