Register all hardware_interfaces together

Prevent each driver comming online one at a time. So that
/driver returns nothign until all interfaces are registered

Story: #2008423
Task: #41368

Change-Id: I6ef3e6e36b96106faf4581509d9219e5c535a6d8

ironic/common/exception.py

@@ -335,10 +335,9 @@ class ConductorAlreadyRegistered(IronicException):
 
 
 class ConductorHardwareInterfacesAlreadyRegistered(IronicException):
-    _msg_fmt = _("At least one of these (hardware type %(hardware_type)s, "
-                 "interface type %(interface_type)s, interfaces "
-                 "%(interfaces)s) combinations are already registered for "
-                 "this conductor.")
+    _msg_fmt = _("Duplication detected for hardware_type, interface_type "
+                 "and interface combinations for this conductor while "
+                 "registering the row %(row)s")
 
 
 class PowerStateFailure(InvalidState):

ironic/conductor/base_manager.py

@@ -12,6 +12,7 @@
 
 """Base conductor manager functionality."""
 
+import copy
 import inspect
 import threading
 
@@ -372,15 +373,21 @@ class BaseConductorManager(object):
         # first unregister, in case we have cruft laying around
         self.conductor.unregister_all_hardware_interfaces()
 
+        interfaces = []
         for ht_name, ht in hardware_types.items():
             interface_map = driver_factory.enabled_supported_interfaces(ht)
             for interface_type, interface_names in interface_map.items():
                 default_interface = driver_factory.default_interface(
                     ht, interface_type, driver_name=ht_name)
-                self.conductor.register_hardware_interfaces(ht_name,
-                                                            interface_type,
-                                                            interface_names,
-                                                            default_interface)
+                interface = {}
+                interface["hardware_type"] = ht_name
+                interface["interface_type"] = interface_type
+                for interface_name in interface_names:
+                    interface["interface_name"] = interface_name
+                    interface["default"] = \
+                        (interface_name == default_interface)
+                    interfaces.append(copy.copy(interface))
+        self.conductor.register_hardware_interfaces(interfaces)
 
         # TODO(jroll) validate against other conductor, warn if different
         # how do we do this performantly? :|

ironic/db/sqlalchemy/api.py

@@ -1110,26 +1110,20 @@ class Connection(api.Connection):
         return query.all()
 
     @oslo_db_api.retry_on_deadlock
-    def register_conductor_hardware_interfaces(self, conductor_id,
-                                               hardware_type, interface_type,
-                                               interfaces, default_interface):
+    def register_conductor_hardware_interfaces(self, conductor_id, interfaces):
         with _session_for_write() as session:
             try:
                 for iface in interfaces:
                     conductor_hw_iface = models.ConductorHardwareInterfaces()
                     conductor_hw_iface['conductor_id'] = conductor_id
-                    conductor_hw_iface['hardware_type'] = hardware_type
-                    conductor_hw_iface['interface_type'] = interface_type
-                    conductor_hw_iface['interface_name'] = iface
-                    is_default = (iface == default_interface)
-                    conductor_hw_iface['default'] = is_default
+                    for k, v in iface.items():
+                        conductor_hw_iface[k] = v
                     session.add(conductor_hw_iface)
                 session.flush()
-            except db_exc.DBDuplicateEntry:
-                raise exception.ConductorHardwareInterfacesAlreadyRegistered(
-                    hardware_type=hardware_type,
-                    interface_type=interface_type,
-                    interfaces=interfaces)
+            except db_exc.DBDuplicateEntry as e:
+                r = exception.ConductorHardwareInterfacesAlreadyRegistered(
+                    row=str(e.inner_exception.params))
+                raise r
 
     @oslo_db_api.retry_on_deadlock
     def unregister_conductor_hardware_interfaces(self, conductor_id):

ironic/objects/conductor.py

@@ -156,21 +156,16 @@ class Conductor(base.IronicObject, object_base.VersionedObjectDictCompat):
         self.unregister_all_hardware_interfaces()
         self.dbapi.unregister_conductor(self.hostname)
 
-    def register_hardware_interfaces(self, hardware_type, interface_type,
-                                     interfaces, default_interface):
+    def register_hardware_interfaces(self, interfaces):
         """Register hardware interfaces with the conductor.
 
-        :param hardware_type: Name of hardware type for the interfaces.
-        :param interface_type: Type of interfaces, e.g. 'deploy' or 'boot'.
-        :param interfaces: List of interface names to register.
-        :param default_interface: String, the default interface for this
-                                  hardware type and interface type.
+        :param interfaces: List of interface to register, each entry should
+            be a dictionary conaining "hardware_type", "interface_type",
+            "interface_name" and "default", e.g.
+            {'hardware_type': 'hardware-type', 'interface_type': 'deploy',
+            'interface_name': 'iscsi', 'default': True}
         """
-        self.dbapi.register_conductor_hardware_interfaces(self.id,
-                                                          hardware_type,
-                                                          interface_type,
-                                                          interfaces,
-                                                          default_interface)
+        self.dbapi.register_conductor_hardware_interfaces(self.id, interfaces)
 
     def unregister_all_hardware_interfaces(self):
         """Unregister all hardware interfaces for this conductor."""

ironic/tests/unit/api/controllers/v1/test_driver.py

@@ -44,9 +44,19 @@ class TestListDrivers(base.BaseApiTest):
         })
         for c in (c1, c2):
             self.dbapi.register_conductor_hardware_interfaces(
-                c.id, self.hw1, 'deploy', ['iscsi', 'direct'], 'direct')
+                c.id,
+                [{'hardware_type': self.hw1, 'interface_type': 'deploy',
+                  'interface_name': 'iscsi', 'default': False},
+                 {'hardware_type': self.hw1, 'interface_type': 'deploy',
+                  'interface_name': 'direct', 'default': True}]
+            )
         self.dbapi.register_conductor_hardware_interfaces(
-            c1.id, self.hw2, 'deploy', ['iscsi', 'direct'], 'direct')
+            c1.id,
+            [{'hardware_type': self.hw2, 'interface_type': 'deploy',
+              'interface_name': 'iscsi', 'default': False},
+             {'hardware_type': self.hw2, 'interface_type': 'deploy',
+              'interface_name': 'direct', 'default': True}]
+        )
 
     def _test_drivers(self, use_dynamic, detail=False, latest_if=False):
         self.register_fake_conductors()

ironic/tests/unit/common/test_hash_ring.py

@@ -59,7 +59,12 @@ class HashRingManagerTestCase(db_base.DbTestCase):
         })
         for c in (c1, c2, c3, c4, c5):
             self.dbapi.register_conductor_hardware_interfaces(
-                c.id, 'hardware-type', 'deploy', ['iscsi', 'direct'], 'iscsi')
+                c.id,
+                [{'hardware_type': 'hardware-type', 'interface_type': 'deploy',
+                  'interface_name': 'iscsi', 'default': True},
+                 {'hardware_type': 'hardware-type', 'interface_type': 'deploy',
+                  'interface_name': 'direct', 'default': False}]
+            )
 
     def test_hash_ring_manager_hardware_type_success(self):
         self.register_conductors()
@@ -100,13 +105,23 @@ class HashRingManagerTestCase(db_base.DbTestCase):
             'drivers': ['driver1'],
         })
         self.dbapi.register_conductor_hardware_interfaces(
-            c1.id, 'hardware-type', 'deploy', ['iscsi', 'direct'], 'iscsi')
+            c1.id,
+            [{'hardware_type': 'hardware-type', 'interface_type': 'deploy',
+              'interface_name': 'iscsi', 'default': True},
+             {'hardware_type': 'hardware-type', 'interface_type': 'deploy',
+              'interface_name': 'direct', 'default': False}]
+        )
 
         ring = self.ring_manager.get_ring('hardware-type', '')
         self.assertEqual(1, len(ring))
 
         self.dbapi.register_conductor_hardware_interfaces(
-            c2.id, 'hardware-type', 'deploy', ['iscsi', 'direct'], 'iscsi')
+            c2.id,
+            [{'hardware_type': 'hardware-type', 'interface_type': 'deploy',
+              'interface_name': 'iscsi', 'default': True},
+             {'hardware_type': 'hardware-type', 'interface_type': 'deploy',
+              'interface_name': 'direct', 'default': False}]
+        )
         ring = self.ring_manager.get_ring('hardware-type', '')
         # The new conductor is not known yet. Automatic retry does not kick in,
         # since there is an active conductor for the requested hardware type.

ironic/tests/unit/conductor/test_base_manager.py

@@ -420,14 +420,37 @@ class RegisterInterfacesTestCase(mgr_utils.ServiceSetUpMixin,
         ]
         default_mock.side_effect = ('fake', 'agent', 'fake', 'agent')
         expected_calls = [
-            mock.call(mock.ANY, 'fake-hardware', 'management',
-                      ['fake', 'noop'], 'fake'),
-            mock.call(mock.ANY, 'fake-hardware', 'deploy', ['agent', 'iscsi'],
-                      'agent'),
-            mock.call(mock.ANY, 'manual-management', 'management', ['fake'],
-                      'fake'),
-            mock.call(mock.ANY, 'manual-management', 'deploy',
-                      ['agent', 'fake'], 'agent'),
+            mock.call(
+                mock.ANY,
+                [{'hardware_type': 'fake-hardware',
+                  'interface_type': 'management',
+                  'interface_name': 'fake',
+                  'default': True},
+                 {'hardware_type': 'fake-hardware',
+                  'interface_type': 'management',
+                  'interface_name': 'noop',
+                  'default': False},
+                 {'hardware_type': 'fake-hardware',
+                  'interface_type': 'deploy',
+                  'interface_name': 'agent',
+                  'default': True},
+                 {'hardware_type': 'fake-hardware',
+                  'interface_type': 'deploy',
+                  'interface_name': 'iscsi',
+                  'default': False},
+                 {'hardware_type': 'manual-management',
+                  'interface_type': 'management',
+                  'interface_name': 'fake',
+                  'default': True},
+                 {'hardware_type': 'manual-management',
+                  'interface_type': 'deploy',
+                  'interface_name': 'agent',
+                  'default': True},
+                 {'hardware_type': 'manual-management',
+                  'interface_type': 'deploy',
+                  'interface_name': 'fake',
+                  'default': False}]
+            )
         ]
 
         self.service._register_and_validate_hardware_interfaces(hardware_types)

ironic/tests/unit/conductor/test_rpcapi.py

@@ -82,7 +82,12 @@ class RPCAPITestCase(db_base.DbTestCase):
         c = self.dbapi.register_conductor({'hostname': 'fake-host',
                                            'drivers': []})
         self.dbapi.register_conductor_hardware_interfaces(
-            c.id, 'fake-driver', 'deploy', ['iscsi', 'direct'], 'iscsi')
+            c.id,
+            [{'hardware_type': 'fake-driver', 'interface_type': 'deploy',
+              'interface_name': 'iscsi', 'default': True},
+             {'hardware_type': 'fake-driver', 'interface_type': 'deploy',
+              'interface_name': 'direct', 'default': False}]
+        )
 
         rpcapi = conductor_rpcapi.ConductorAPI(topic='fake-topic')
         expected_topic = 'fake-topic.fake-host'
@@ -94,7 +99,12 @@ class RPCAPITestCase(db_base.DbTestCase):
         c = self.dbapi.register_conductor({'hostname': 'fake-host',
                                            'drivers': []})
         self.dbapi.register_conductor_hardware_interfaces(
-            c.id, 'other-driver', 'deploy', ['iscsi', 'direct'], 'iscsi')
+            c.id,
+            [{'hardware_type': 'other-driver', 'interface_type': 'deploy',
+              'interface_name': 'iscsi', 'default': True},
+             {'hardware_type': 'other-driver', 'interface_type': 'deploy',
+              'interface_name': 'direct', 'default': False}]
+        )
 
         rpcapi = conductor_rpcapi.ConductorAPI(topic='fake-topic')
         self.assertRaises(exception.NoValidHost,
@@ -112,7 +122,12 @@ class RPCAPITestCase(db_base.DbTestCase):
         c = self.dbapi.register_conductor({'hostname': 'fake-host',
                                            'drivers': []})
         self.dbapi.register_conductor_hardware_interfaces(
-            c.id, 'fake-driver', 'deploy', ['iscsi', 'direct'], 'iscsi')
+            c.id,
+            [{'hardware_type': 'fake-driver', 'interface_type': 'deploy',
+              'interface_name': 'iscsi', 'default': True},
+             {'hardware_type': 'fake-driver', 'interface_type': 'deploy',
+              'interface_name': 'direct', 'default': False}]
+        )
 
         rpcapi = conductor_rpcapi.ConductorAPI(topic='fake-topic')
         expected_topic = 'fake-topic.fake-host'
@@ -126,7 +141,12 @@ class RPCAPITestCase(db_base.DbTestCase):
             'drivers': [],
         })
         self.dbapi.register_conductor_hardware_interfaces(
-            c.id, 'fake-driver', 'deploy', ['iscsi', 'direct'], 'iscsi')
+            c.id,
+            [{'hardware_type': 'fake-driver', 'interface_type': 'deploy',
+              'interface_name': 'iscsi', 'default': True},
+             {'hardware_type': 'fake-driver', 'interface_type': 'deploy',
+              'interface_name': 'direct', 'default': False}]
+        )
         rpcapi = conductor_rpcapi.ConductorAPI(topic='fake-topic')
         self.assertEqual('fake-topic.fake-host',
                          rpcapi.get_topic_for_driver('fake-driver'))
@@ -138,7 +158,12 @@ class RPCAPITestCase(db_base.DbTestCase):
             'drivers': [],
         })
         self.dbapi.register_conductor_hardware_interfaces(
-            c.id, 'fake-driver', 'deploy', ['iscsi', 'direct'], 'iscsi')
+            c.id,
+            [{'hardware_type': 'fake-driver', 'interface_type': 'deploy',
+              'interface_name': 'iscsi', 'default': True},
+             {'hardware_type': 'fake-driver', 'interface_type': 'deploy',
+              'interface_name': 'direct', 'default': False}]
+        )
         rpcapi = conductor_rpcapi.ConductorAPI(topic='fake-topic')
         self.assertRaises(exception.DriverNotFound,
                           rpcapi.get_topic_for_driver,
@@ -156,7 +181,12 @@ class RPCAPITestCase(db_base.DbTestCase):
             'drivers': [],
         })
         self.dbapi.register_conductor_hardware_interfaces(
-            c.id, 'fake-driver', 'deploy', ['iscsi', 'direct'], 'iscsi')
+            c.id,
+            [{'hardware_type': 'fake-driver', 'interface_type': 'deploy',
+              'interface_name': 'iscsi', 'default': True},
+             {'hardware_type': 'fake-driver', 'interface_type': 'deploy',
+              'interface_name': 'direct', 'default': False}]
+        )
         rpcapi = conductor_rpcapi.ConductorAPI(topic='fake-topic')
         self.assertEqual('fake-topic.fake-host',
                          rpcapi.get_topic_for_driver('fake-driver'))
@@ -166,7 +196,12 @@ class RPCAPITestCase(db_base.DbTestCase):
         c = self.dbapi.register_conductor({'hostname': 'fake-host',
                                            'drivers': []})
         self.dbapi.register_conductor_hardware_interfaces(
-            c.id, 'fake-driver', 'deploy', ['iscsi', 'direct'], 'iscsi')
+            c.id,
+            [{'hardware_type': 'fake-driver', 'interface_type': 'deploy',
+              'interface_name': 'iscsi', 'default': True},
+             {'hardware_type': 'fake-driver', 'interface_type': 'deploy',
+              'interface_name': 'direct', 'default': False}]
+        )
         rpcapi = conductor_rpcapi.ConductorAPI()
         self.assertEqual(rpcapi.get_conductor_for(self.fake_node_obj),
                          'fake-host')

ironic/tests/unit/db/test_conductor.py

@@ -48,18 +48,25 @@ class DbConductorTestCase(base.DbTestCase):
         c = utils.get_test_conductor(**kwargs)
         cdr = self.dbapi.register_conductor(c)
         for ht in hardware_types:
-            self.dbapi.register_conductor_hardware_interfaces(cdr.id, ht,
-                                                              'power',
-                                                              ['ipmi', 'fake'],
-                                                              'ipmi')
+            self.dbapi.register_conductor_hardware_interfaces(
+                cdr.id,
+                [{'hardware_type': ht, 'interface_type': 'power',
+                  'interface_name': 'ipmi', 'default': True},
+                 {'hardware_type': ht, 'interface_type': 'power',
+                  'interface_name': 'fake', 'default': False}]
+            )
         return cdr
 
     def test_register_conductor_hardware_interfaces(self):
         c = self._create_test_cdr()
         interfaces = ['direct', 'iscsi']
-        self.dbapi.register_conductor_hardware_interfaces(c.id, 'generic',
-                                                          'deploy', interfaces,
-                                                          'iscsi')
+        self.dbapi.register_conductor_hardware_interfaces(
+            c.id,
+            [{'hardware_type': 'generic', 'interface_type': 'deploy',
+              'interface_name': interfaces[0], 'default': False},
+             {'hardware_type': 'generic', 'interface_type': 'deploy',
+              'interface_name': interfaces[1], 'default': True}]
+        )
         ifaces = self.dbapi.list_conductor_hardware_interfaces(c.id)
         ci1, ci2 = ifaces
         self.assertEqual(2, len(ifaces))
@@ -74,10 +81,13 @@ class DbConductorTestCase(base.DbTestCase):
 
     def test_register_conductor_hardware_interfaces_duplicate(self):
         c = self._create_test_cdr()
-        interfaces = ['direct', 'iscsi']
-        self.dbapi.register_conductor_hardware_interfaces(c.id, 'generic',
-                                                          'deploy', interfaces,
-                                                          'iscsi')
+        interfaces = [
+            {'hardware_type': 'generic', 'interface_type': 'deploy',
+             'interface_name': 'direct', 'default': False},
+            {'hardware_type': 'generic', 'interface_type': 'deploy',
+             'interface_name': 'iscsi', 'default': True}
+        ]
+        self.dbapi.register_conductor_hardware_interfaces(c.id, interfaces)
         ifaces = self.dbapi.list_conductor_hardware_interfaces(c.id)
         ci1, ci2 = ifaces
         self.assertEqual(2, len(ifaces))
@@ -86,14 +96,18 @@ class DbConductorTestCase(base.DbTestCase):
         self.assertRaises(
             exception.ConductorHardwareInterfacesAlreadyRegistered,
             self.dbapi.register_conductor_hardware_interfaces,
-            c.id, 'generic', 'deploy', interfaces, 'iscsi')
+            c.id, interfaces)
 
     def test_unregister_conductor_hardware_interfaces(self):
         c = self._create_test_cdr()
         interfaces = ['direct', 'iscsi']
-        self.dbapi.register_conductor_hardware_interfaces(c.id, 'generic',
-                                                          'deploy', interfaces,
-                                                          'iscsi')
+        self.dbapi.register_conductor_hardware_interfaces(
+            c.id,
+            [{'hardware_type': 'generic', 'interface_type': 'deploy',
+              'interface_name': interfaces[0], 'default': False},
+             {'hardware_type': 'generic', 'interface_type': 'deploy',
+              'interface_name': interfaces[1], 'default': True}]
+        )
         self.dbapi.unregister_conductor_hardware_interfaces(c.id)
 
         ifaces = self.dbapi.list_conductor_hardware_interfaces(c.id)

ironic/tests/unit/objects/test_conductor.py

@@ -167,6 +167,10 @@ class TestConductorObject(db_base.DbTestCase):
     def test_register_hardware_interfaces(self):
         host = self.fake_conductor['hostname']
         self.config(default_deploy_interface='iscsi')
+        arg = [{"hardware_type": "hardware-type", "interface_type": "deploy",
+                "interface_name": "iscsi", "default": True},
+               {"hardware_type": "hardware-type", "interface_type": "deploy",
+                "interface_name": "direct", "default": False}]
         with mock.patch.object(self.dbapi, 'get_conductor',
                                autospec=True) as mock_get_cdr:
             with mock.patch.object(self.dbapi,
@@ -174,10 +178,8 @@ class TestConductorObject(db_base.DbTestCase):
                                    autospec=True) as mock_register:
                 mock_get_cdr.return_value = self.fake_conductor
                 c = objects.Conductor.get_by_hostname(self.context, host)
-                args = ('hardware-type', 'deploy', ['iscsi', 'direct'],
-                        'iscsi')
-                c.register_hardware_interfaces(*args)
-                mock_register.assert_called_once_with(c.id, *args)
+                c.register_hardware_interfaces(arg)
+                mock_register.assert_called_once_with(c.id, arg)
 
     def test_unregister_all_hardware_interfaces(self):
         host = self.fake_conductor['hostname']

releasenotes/notes/register_hardware_interfaces_together-7b458a59f5e8f41f.yaml

@@ -0,0 +1,10 @@
+---
+other:
+  - |
+    Register all conductor hardware interfaces together. Adds
+    all conductor hardware interfaces in to the database in a
+    single transaction and to allow this update the
+    ``register_hardware_interfaces`` API. This allows Restful API
+    consumers to understand if the conductor is fully on-line via
+    the presence of driver entries. Previously this was done one
+    driver at a time.