From 92f00f80fa3fe9abaf32dea33057de445e5df1f3 Mon Sep 17 00:00:00 2001 From: Steven Webster Date: Thu, 4 Apr 2024 19:52:32 -0400 Subject: [PATCH] Fix usage of address_get_by_name Recent commit https://opendev.org/starlingx/config/commit/634d4916 introducing changes for dual-stack networking made a change to the DB api's address_get_by_name to return a list of IPv4 and IPv6 addresses rather than a singular address. As such, the list can be empty if there are no addresses associated with a particular name, rather than throwing an AddressNotFoundByName exception. Currently, the interface_network code depends on the AddressNotFoundByName exception to determine whether a new address needs to be allocated for a dynamic network. This can cause an issue for worker, storage nodes when one of their interfaces is associated with certain networks (such as the storage network). The symptom of this may be an interface which is 'DOWN' after unlock, as it's interface configuration file is marked for a 'static' address, with no address present (because it wasn't allocated). This commit fixes the issue by simply checking that the list returned by address_get_by_name is empty. Test Plan: - Fresh install of a Standard system. - Ensure named addresses are present in the DB for all nodes (mgmt, cluster-host, oam for controllers) - Create a new address pool and storage network and assign it to a worker node interface. - Unlock the worker node and ensure the address is present on the interface and it is in 'UP' state. Story: 2011027 Task: 49627 Change-Id: I9763f7c71797d9b321e7bf9e1b6db759378af632 Signed-off-by: Steven Webster --- .../api/controllers/v1/interface_network.py | 33 +++++++--------- .../sysinv/sysinv/api/controllers/v1/utils.py | 8 ++-- sysinv/sysinv/sysinv/sysinv/common/utils.py | 8 ++-- .../sysinv/sysinv/sysinv/conductor/manager.py | 30 +++++++------- .../tests/api/test_interface_network.py | 39 +++++++++++++++++++ 5 files changed, 73 insertions(+), 45 deletions(-) diff --git a/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/interface_network.py b/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/interface_network.py index 846d015512..a2641c3809 100644 --- a/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/interface_network.py +++ b/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/interface_network.py @@ -525,12 +525,11 @@ def _update_host_mgmt_address(host, interface): def _update_host_admin_address(host, interface): address_name = cutils.format_address_name(host.hostname, constants.NETWORK_TYPE_ADMIN) - try: - addresses = pecan.request.dbapi.address_get_by_name(address_name) - for addr in addresses: - updates = {'interface_id': interface['id']} - pecan.request.dbapi.address_update(addr.uuid, updates) - except exception.AddressNotFoundByName: + addresses = pecan.request.dbapi.address_get_by_name(address_name) + for addr in addresses: + updates = {'interface_id': interface['id']} + pecan.request.dbapi.address_update(addr.uuid, updates) + if len(addresses) == 0: # For non-controller hosts, allocate address from pool if dynamic admin_network = pecan.request.dbapi.network_get_by_type( constants.NETWORK_TYPE_ADMIN) @@ -570,12 +569,11 @@ def _update_host_cluster_address(host, interface): """ address_name = cutils.format_address_name( host.hostname, constants.NETWORK_TYPE_CLUSTER_HOST) - try: - addresses = pecan.request.dbapi.address_get_by_name(address_name) - for addr in addresses: - updates = {'interface_id': interface['id']} - pecan.request.dbapi.address_update(addr.uuid, updates) - except exception.AddressNotFoundByName: + addresses = pecan.request.dbapi.address_get_by_name(address_name) + for addr in addresses: + updates = {'interface_id': interface['id']} + pecan.request.dbapi.address_update(addr.uuid, updates) + if len(addresses) == 0: cluster_host_network = pecan.request.dbapi.network_get_by_type( constants.NETWORK_TYPE_CLUSTER_HOST) if cluster_host_network.dynamic: @@ -596,12 +594,11 @@ def _update_host_ironic_address(host, interface): def _update_host_storage_address(host, interface): address_name = cutils.format_address_name(host.hostname, constants.NETWORK_TYPE_STORAGE) - try: - addresses = pecan.request.dbapi.address_get_by_name(address_name) - for addr in addresses: - updates = {'interface_id': interface['id']} - pecan.request.dbapi.address_update(addr.uuid, updates) - except exception.AddressNotFoundByName: + addresses = pecan.request.dbapi.address_get_by_name(address_name) + for addr in addresses: + updates = {'interface_id': interface['id']} + pecan.request.dbapi.address_update(addr.uuid, updates) + if len(addresses) == 0: # For non-controller hosts, allocate address from pool if dynamic storage_network = pecan.request.dbapi.network_get_by_type( constants.NETWORK_TYPE_STORAGE) diff --git a/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/utils.py b/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/utils.py index c3c9eab845..1e70da86d2 100644 --- a/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/utils.py +++ b/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/utils.py @@ -1055,7 +1055,7 @@ class PopulateAddresses(object): def get_primary_address_by_name(db_address_name, networktype, raise_exc=False): - """Search address by database name to retrieve the relevant addres from + """Search address by database name to retrieve the relevant address from the primary pool, if multipĺe entries for the same name are found, the query will use the network's pool_uuid to get the address family (IPv4 or IPv6) related to the primary. @@ -1066,11 +1066,9 @@ def get_primary_address_by_name(db_address_name, networktype, raise_exc=False): :return: the address object if found, None otherwise """ - address = None # first search directly by name - try: - address = pecan.request.dbapi.address_get_by_name(db_address_name) - except exception.AddressNotFoundByName(): + address = pecan.request.dbapi.address_get_by_name(db_address_name) + if len(address) == 0: # if there is no match by name return here LOG.info(f"address {db_address_name} not found, returning") if raise_exc: diff --git a/sysinv/sysinv/sysinv/sysinv/common/utils.py b/sysinv/sysinv/sysinv/sysinv/common/utils.py index 2816bda12f..5cc4966167 100644 --- a/sysinv/sysinv/sysinv/sysinv/common/utils.py +++ b/sysinv/sysinv/sysinv/sysinv/common/utils.py @@ -3707,7 +3707,7 @@ def is_bundle_extension_valid(filename): def get_primary_address_by_name(dbapi, db_address_name, networktype, raise_exc=False): - """Search address by database name to retrieve the relevant addres from + """Search address by database name to retrieve the relevant address from the primary pool, if multipĺe entries for the same name are found, the query will use the network's pool_uuid to get the address family (IPv4 or IPv6) related to the primary. @@ -3719,11 +3719,9 @@ def get_primary_address_by_name(dbapi, db_address_name, networktype, raise_exc=F :return: the address object if found, None otherwise """ - address = None # first search directly by name - try: - address = dbapi.address_get_by_name(db_address_name) - except exception.AddressNotFoundByName(): + address = dbapi.address_get_by_name(db_address_name) + if len(address) == 0: # if there is no match by name return here LOG.info(f"address {db_address_name} not found, returning") if raise_exc: diff --git a/sysinv/sysinv/sysinv/sysinv/conductor/manager.py b/sysinv/sysinv/sysinv/sysinv/conductor/manager.py index 3d47d2f968..641b75b869 100644 --- a/sysinv/sysinv/sysinv/sysinv/conductor/manager.py +++ b/sysinv/sysinv/sysinv/sysinv/conductor/manager.py @@ -3676,14 +3676,12 @@ class ConductorManager(service.PeriodicService): if set_address_interface: if new_interface and 'id' in new_interface: values = {'interface_id': new_interface['id']} - try: - addr_name = cutils.format_address_name( - ihost.hostname, new_interface_networktype) - addresses = self.dbapi.address_get_by_name(addr_name) - for address in addresses: - self.dbapi.address_update(address['uuid'], values) - except exception.AddressNotFoundByName: - pass + addr_name = cutils.format_address_name( + ihost.hostname, new_interface_networktype) + addresses = self.dbapi.address_get_by_name(addr_name) + for address in addresses: + self.dbapi.address_update(address['uuid'], values) + # Do any potential distributed cloud config # We do this here where the interface is created. cutils.perform_distributed_cloud_config(self.dbapi, @@ -3691,15 +3689,13 @@ class ConductorManager(service.PeriodicService): ihost) if port: values = {'interface_id': port.interface_id} - try: - addr_name = cutils.format_address_name(ihost.hostname, - networktype) - addresses = self.dbapi.address_get_by_name(addr_name) - for address in addresses: - if address['interface_id'] is None: - self.dbapi.address_update(address['uuid'], values) - except exception.AddressNotFoundByName: - pass + + addr_name = cutils.format_address_name(ihost.hostname, + networktype) + addresses = self.dbapi.address_get_by_name(addr_name) + for address in addresses: + if address['interface_id'] is None: + self.dbapi.address_update(address['uuid'], values) if ihost.invprovision not in [constants.PROVISIONED, constants.PROVISIONING, constants.UPGRADING]: LOG.info("Updating %s host invprovision from %s to %s" % diff --git a/sysinv/sysinv/sysinv/sysinv/tests/api/test_interface_network.py b/sysinv/sysinv/sysinv/sysinv/tests/api/test_interface_network.py index 1421193968..510922d999 100644 --- a/sysinv/sysinv/sysinv/sysinv/tests/api/test_interface_network.py +++ b/sysinv/sysinv/sysinv/sysinv/tests/api/test_interface_network.py @@ -14,11 +14,13 @@ from sysinv.api.controllers.v1 import interface as api_if_v1 from sysinv.common import constants from sysinv.tests.api import base from sysinv.tests.db import utils as dbutils +from sysinv.db import api as dbapi class InterfaceNetworkTestCase(base.FunctionalTest): def setUp(self): super(InterfaceNetworkTestCase, self).setUp() + self.dbapi = dbapi.get_instance() p = mock.patch.object(api_if_v1, '_get_lower_interface_macs') self.mock_lower_macs = p.start() @@ -139,6 +141,16 @@ class InterfaceNetworkTestCase(base.FunctionalTest): link_capacity=10000, vlan_id=8, address_pool_id=self.address_pool_admin.id) + self.address_pool_storage = dbutils.create_test_address_pool( + id=6, + network='192.168.209.0', + name='storage', + ranges=[['192.168.209.2', '192.168.209.254']], + prefix=24) + self.storage_network = dbutils.create_test_network( + id=6, + type=constants.NETWORK_TYPE_STORAGE, + address_pool_id=self.address_pool_storage.id) def _post_and_check(self, ndict, expect_errors=False): response = self.post_json('%s' % self._get_path(), ndict, @@ -298,6 +310,33 @@ class InterfaceNetworkCreateTestCase(InterfaceNetworkTestCase): network_uuid=self.cluster_host_network.uuid) self._post_and_check(worker_interface_network, expect_errors=False) + def test_create_storage_interface_network(self): + controller_interface = dbutils.create_test_interface( + ifname='enp0s8', + forihostid=self.controller.id) + worker_interface = dbutils.create_test_interface( + ifname='enp0s8', + forihostid=self.worker.id) + + controller_interface_network = dbutils.post_get_test_interface_network( + interface_uuid=controller_interface.uuid, + network_uuid=self.storage_network.uuid) + self._post_and_check(controller_interface_network, expect_errors=False) + + # Since the network is (by default) setup for dynamic address + # allocation, the addresses for each node should be created + # automatically when the interface is associated with the network. + addresses = self.dbapi.address_get_by_name('controller-0-storage') + self.assertEqual(len(addresses), 1) + + worker_interface_network = dbutils.post_get_test_interface_network( + interface_uuid=worker_interface.uuid, + network_uuid=self.storage_network.uuid) + self._post_and_check(worker_interface_network, expect_errors=False) + + addresses = self.dbapi.address_get_by_name('worker-0-storage') + self.assertEqual(len(addresses), 1) + # Expected error: # You cannot assign a network of type 'oam' to an interface # which is already assigned with a different network