diff options
author | Derek Higgins <derekh@redhat.com> | 2020-12-01 09:11:41 +0000 |
---|---|---|
committer | Julia Kreger <juliaashleykreger@gmail.com> | 2021-01-08 15:16:53 +0000 |
commit | 7d85b35c8425f10a2fff64dfc82008a93c3a7b98 (patch) | |
tree | bbd5de7ae661f9028b2c7faa6a29f3c4e76c7ed1 | |
parent | fcf029a0ad078d96ef0da8859d7f361b2d2cac88 (diff) | |
download | ironic-7d85b35c8425f10a2fff64dfc82008a93c3a7b98.tar.gz |
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
-rw-r--r-- | ironic/common/exception.py | 7 | ||||
-rw-r--r-- | ironic/conductor/base_manager.py | 15 | ||||
-rw-r--r-- | ironic/db/sqlalchemy/api.py | 20 | ||||
-rw-r--r-- | ironic/objects/conductor.py | 19 | ||||
-rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_driver.py | 14 | ||||
-rw-r--r-- | ironic/tests/unit/common/test_hash_ring.py | 21 | ||||
-rw-r--r-- | ironic/tests/unit/conductor/test_base_manager.py | 39 | ||||
-rw-r--r-- | ironic/tests/unit/conductor/test_rpcapi.py | 49 | ||||
-rw-r--r-- | ironic/tests/unit/db/test_conductor.py | 44 | ||||
-rw-r--r-- | ironic/tests/unit/objects/test_conductor.py | 10 | ||||
-rw-r--r-- | releasenotes/notes/register_hardware_interfaces_together-7b458a59f5e8f41f.yaml | 10 |
11 files changed, 176 insertions, 72 deletions
diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 6f9a7d331..77ba9f5f4 100644 --- a/ironic/common/exception.py +++ b/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): diff --git a/ironic/conductor/base_manager.py b/ironic/conductor/base_manager.py index d4361dfa9..53b2b48ae 100644 --- a/ironic/conductor/base_manager.py +++ b/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? :| diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index 7891122ce..716fd601f 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/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): diff --git a/ironic/objects/conductor.py b/ironic/objects/conductor.py index 0c0e991a6..ad381359b 100644 --- a/ironic/objects/conductor.py +++ b/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.""" diff --git a/ironic/tests/unit/api/controllers/v1/test_driver.py b/ironic/tests/unit/api/controllers/v1/test_driver.py index 7e8a0f471..220d4aea6 100644 --- a/ironic/tests/unit/api/controllers/v1/test_driver.py +++ b/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() diff --git a/ironic/tests/unit/common/test_hash_ring.py b/ironic/tests/unit/common/test_hash_ring.py index 882216a8b..7e05aabff 100644 --- a/ironic/tests/unit/common/test_hash_ring.py +++ b/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. diff --git a/ironic/tests/unit/conductor/test_base_manager.py b/ironic/tests/unit/conductor/test_base_manager.py index 7680af153..0ee6e9160 100644 --- a/ironic/tests/unit/conductor/test_base_manager.py +++ b/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) diff --git a/ironic/tests/unit/conductor/test_rpcapi.py b/ironic/tests/unit/conductor/test_rpcapi.py index 73d08008b..1cb0b7d9b 100644 --- a/ironic/tests/unit/conductor/test_rpcapi.py +++ b/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') diff --git a/ironic/tests/unit/db/test_conductor.py b/ironic/tests/unit/db/test_conductor.py index bb7832b99..1b390af23 100644 --- a/ironic/tests/unit/db/test_conductor.py +++ b/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) diff --git a/ironic/tests/unit/objects/test_conductor.py b/ironic/tests/unit/objects/test_conductor.py index 42010da0c..6df22c838 100644 --- a/ironic/tests/unit/objects/test_conductor.py +++ b/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'] diff --git a/releasenotes/notes/register_hardware_interfaces_together-7b458a59f5e8f41f.yaml b/releasenotes/notes/register_hardware_interfaces_together-7b458a59f5e8f41f.yaml new file mode 100644 index 000000000..c327cada6 --- /dev/null +++ b/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.
\ No newline at end of file |