starlingx/config
Code Issues Proposed changes

Merge "Fixed address interface foreign key inconsistency"

Browse Source
This commit is contained in:
Zuul
2020-03-30 16:44:34 +00:00
committed by Gerrit Code Review
parent 1647793584 6f162c3422
commit c00cc570ef
8 changed files with 219 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sysinv/sysinv/sysinv/sysinv/api/controllers/v1/address.py
+11 -2
View File
@@ -88,7 +88,13 @@ class Address(base.APIBase):
"The UUID of the address pool from which this address was allocated"
def __init__(self, **kwargs):
self.fields = objects.address.fields.keys()
# The interface_uuid in this `Address` type is kept to avoid changes to
# API/CLI. However, `self.field` refers to `objects.address.field` which
# doesn't include 'interface_uuid', and therefore it is added manually.
# Otherwise, controller `Address.as_dict()` will not include `interface_uuid`
# despite the field being present.
self.fields = list(objects.address.fields.keys())
self.fields.append('interface_uuid')
for k in self.fields:
if not hasattr(self, k):
# Skip fields that we choose to hide
@@ -111,6 +117,9 @@ class Address(base.APIBase):
@classmethod
def convert_with_links(cls, rpc_address, expand=True):
address = Address(**rpc_address.as_dict())
if rpc_address.interface_id:
address.interface_uuid = pecan.request.dbapi.iinterface_get(
rpc_address.interface_id).uuid
if not expand:
address.unset_fields_except(['uuid', 'address',
'prefix', 'interface_uuid', 'ifname',
@@ -286,7 +295,7 @@ class AddressController(rest.RestController):
raise exception.AddressInSameSubnetExists(
**{'address': entry['address'],
'prefix': entry['prefix'],
'interface': entry['interface_uuid']})
'interface': entry['interface_id']})
def _check_address_count(self, interface_id, host_id):
interface = pecan.request.dbapi.iinterface_get(interface_id)
sysinv/sysinv/sysinv/sysinv/conductor/manager.py
+4 -5
View File
@@ -1244,11 +1244,11 @@ class ConductorManager(service.PeriodicService):
return
address = self.dbapi.address_get_by_name(address_name)
interface_uuid = address.interface_uuid
interface_id = address.interface_id
ip_address = address.address
if interface_uuid:
interface = self.dbapi.iinterface_get(interface_uuid)
if interface_id:
interface = self.dbapi.iinterface_get(interface_id)
mac_address = interface.imac
elif network_type == constants.NETWORK_TYPE_MGMT:
ihost = self.dbapi.ihost_get_by_hostname(hostname)
@@ -1796,7 +1796,6 @@ class ConductorManager(service.PeriodicService):
:param inic_dict_array: initial values for iport objects
:returns: pass or fail
"""
LOG.debug("Entering iport_update_by_ihost %s %s" %
(ihost_uuid, inic_dict_array))
ihost_uuid.strip()
@@ -2078,7 +2077,7 @@ class ConductorManager(service.PeriodicService):
addr_name = cutils.format_address_name(ihost.hostname,
networktype)
address = self.dbapi.address_get_by_name(addr_name)
if address['interface_uuid'] is None:
if address['interface_id'] is None:
self.dbapi.address_update(address['uuid'], values)
except exception.AddressNotFoundByName:
pass
sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/api.py
+3 -9
View File
@@ -347,17 +347,11 @@ def add_interface_filter(query, value):
:return: Modified query.
"""
if utils.is_valid_mac(value):
return query.filter(or_(models.EthernetInterfaces.imac == value,
models.AeInterfaces.imac == value,
models.VlanInterfaces.imac == value))
return query.filter(models.Interfaces.imac == value)
elif uuidutils.is_uuid_like(value):
return query.filter(or_(models.EthernetInterfaces.uuid == value,
models.AeInterfaces.uuid == value,
models.VlanInterfaces.uuid == value))
return query.filter(models.Interfaces.uuid == value)
elif utils.is_int_like(value):
return query.filter(or_(models.EthernetInterfaces.id == value,
models.AeInterfaces.id == value,
models.VlanInterfaces.id == value))
return query.filter(models.Interfaces.id == value)
else:
return add_identity_filter(query, value, use_ifname=True)
sysinv/sysinv/sysinv/sysinv/helm/nova.py
+1 -1
View File
@@ -388,7 +388,7 @@ class NovaHelm(openstack.OpenstackBaseHelm):
cluster_host_ip = None
ip_family = None
for addr in addresses:
if addr.interface_uuid == cluster_host_iface.uuid:
if addr.interface_id == cluster_host_iface.id:
cluster_host_ip = addr.address
ip_family = addr.family
sysinv/sysinv/sysinv/sysinv/objects/address.py
+2 -2
View File
@@ -22,7 +22,7 @@ class Address(base.SysinvObject):
fields = {'id': int,
'uuid': utils.uuid_or_none,
'forihostid': utils.int_or_none,
'interface_uuid': utils.uuid_or_none,
'interface_id': utils.int_or_none,
'pool_uuid': utils.uuid_or_none,
'ifname': utils.str_or_none,
'family': utils.int_or_none,
@@ -32,7 +32,7 @@ class Address(base.SysinvObject):
'name': utils.str_or_none,
}
_foreign_fields = {'interface_uuid': 'interface:uuid',
_foreign_fields = {'interface_id': 'interface:id',
'pool_uuid': 'address_pool:uuid',
'ifname': 'interface:ifname',
'forihostid': 'interface:forihostid'}
sysinv/sysinv/sysinv/sysinv/tests/api/test_address.py
+114 -5
View File
@@ -8,6 +8,7 @@
Tests for the API / address / methods.
"""
import mock
import netaddr
from six.moves import http_client
@@ -248,6 +249,8 @@ class TestDelete(AddressTestCase):
def setUp(self):
super(TestDelete, self).setUp()
self.worker = self._create_test_host(constants.WORKER,
administrative=constants.ADMIN_LOCKED)
def test_delete(self):
# Delete the API object
@@ -259,11 +262,117 @@ class TestDelete(AddressTestCase):
# Verify the expected API response for the delete
self.assertEqual(response.status_code, http_client.NO_CONTENT)
# TODO: Add unit tests to verify deletion is rejected as expected by
# _check_orphaned_routes, _check_host_state, and _check_from_pool.
#
# Currently blocked by bug in dbapi preventing testcase setup:
# https://bugs.launchpad.net/starlingx/+bug/1861131
def test_delete_address_with_interface(self):
interface = dbutils.create_test_interface(
ifname="test0",
ifclass=constants.INTERFACE_CLASS_PLATFORM,
forihostid=self.worker.id,
ihost_uuid=self.worker.uuid)
address = dbutils.create_test_address(
interface_id=interface.id,
name="enptest01",
family=self.oam_subnet.version,
address=str(self.oam_subnet[25]),
prefix=self.oam_subnet.prefixlen)
self.assertEqual(address["interface_id"], interface.id)
response = self.delete(self.get_single_url(address.uuid),
headers=self.API_HEADERS)
self.assertEqual(response.status_code, http_client.NO_CONTENT)
def test_orphaned_routes(self):
interface = dbutils.create_test_interface(
ifname="test0",
ifclass=constants.INTERFACE_CLASS_PLATFORM,
forihostid=self.worker.id,
ihost_uuid=self.worker.uuid)
address = dbutils.create_test_address(
interface_id=interface.id,
name="enptest01",
family=self.oam_subnet.version,
address=str(self.oam_subnet[25]),
prefix=self.oam_subnet.prefixlen)
self.assertEqual(address["interface_id"], interface.id)
route = dbutils.create_test_route(
interface_id=interface.id,
family=4,
network='10.10.10.0',
prefix=24,
gateway=str(self.oam_subnet[1]),
)
self.assertEqual(route['gateway'], str(self.oam_subnet[1]))
response = self.delete(self.get_single_url(address.uuid),
headers=self.API_HEADERS,
expect_errors=True)
self.assertEqual(response.content_type, 'application/json')
self.assertEqual(response.status_code, http_client.CONFLICT)
self.assertIn(
"Address %s is in use by a route to %s/%d via %s" % (
address["address"], route["network"], route["prefix"],
route["gateway"]
), response.json['error_message'])
def test_bad_host_state(self):
interface = dbutils.create_test_interface(
ifname="test0",
ifclass=constants.INTERFACE_CLASS_PLATFORM,
forihostid=self.worker.id,
ihost_uuid=self.worker.uuid)
address = dbutils.create_test_address(
interface_id=interface.id,
name="enptest01",
family=self.oam_subnet.version,
address=str(self.oam_subnet[25]),
prefix=self.oam_subnet.prefixlen)
self.assertEqual(address["interface_id"], interface.id)
# unlock the worker
dbapi = dbutils.db_api.get_instance()
worker = dbapi.ihost_update(self.worker.uuid, {
"administrative": constants.ADMIN_UNLOCKED
})
self.assertEqual(worker['administrative'],
constants.ADMIN_UNLOCKED)
response = self.delete(self.get_single_url(address.uuid),
headers=self.API_HEADERS,
expect_errors=True)
self.assertEqual(response.content_type, 'application/json')
self.assertEqual(response.status_code,
http_client.INTERNAL_SERVER_ERROR)
self.assertIn("administrative state = unlocked",
response.json['error_message'])
def test_delete_address_from_pool(self):
pool = dbutils.create_test_address_pool(
name='testpool',
network='192.168.204.0',
ranges=[['192.168.204.2', '192.168.204.254']],
prefix=24)
address = dbutils.create_test_address(
name="enptest01",
family=4,
address='192.168.204.4',
prefix=24,
address_pool_id=pool.id)
self.assertEqual(address['pool_uuid'], pool.uuid)
with mock.patch(
'sysinv.common.utils.is_initial_config_complete', lambda: True):
response = self.delete(self.get_single_url(address.uuid),
headers=self.API_HEADERS,
expect_errors=True)
self.assertEqual(response.content_type, 'application/json')
self.assertEqual(response.status_code,
http_client.CONFLICT)
self.assertIn("Address has been allocated from pool; "
"cannot be manually deleted",
response.json['error_message'])
class TestList(AddressTestCase):
sysinv/sysinv/sysinv/sysinv/tests/conductor/test_manager.py
+56
View File
@@ -1426,3 +1426,59 @@ class ManagerTestCase(base.DbTestCase):
updated_port = self.dbapi.ethernet_port_get(port1['uuid'], host_id)
self.assertEqual(updated_port['node_id'], 3)
class ManagerTestCaseInternal(base.BaseHostTestCase):
def setUp(self):
super(ManagerTestCaseInternal, self).setUp()
# Set up objects for testing
self.service = manager.ConductorManager('test-host', 'test-topic')
self.service.dbapi = dbapi.get_instance()
def test_remove_lease_for_address(self):
# create test interface
ihost = self._create_test_host(
personality=constants.WORKER,
administrative=constants.ADMIN_UNLOCKED)
iface = utils.create_test_interface(
ifname="test0",
ifclass=constants.INTERFACE_CLASS_PLATFORM,
forihostid=ihost.id,
ihost_uuid=ihost.uuid)
network = self.dbapi.network_get_by_type(constants.NETWORK_TYPE_MGMT)
utils.create_test_interface_network(
interface_id=iface.id,
network_id=network.id)
# create test address associated with interface
address_name = cutils.format_address_name(ihost.hostname,
network.type)
self.dbapi.address_create({
'name': address_name,
'family': self.oam_subnet.version,
'prefix': self.oam_subnet.prefixlen,
'address': str(self.oam_subnet[24]),
'interface_id': iface.id,
'enable_dad': self.oam_subnet.version == 6
})
# stub the system i/o calls
self.mock_objs = [
mock.patch.object(
manager.ConductorManager, '_find_local_interface_name',
lambda x, y: iface.ifname),
mock.patch('sysinv.common.utils.get_dhcp_cid',
lambda x, y, z: None),
mock.patch.object(
manager.ConductorManager, '_dhcp_release',
lambda a, b, c, d, e: None)
]
for mock_obj in self.mock_objs:
mock_obj.start()
self.addCleanup(mock_obj.stop)
self.service._remove_lease_for_address(ihost.hostname,
constants.NETWORK_TYPE_MGMT)
sysinv/sysinv/sysinv/sysinv/tests/helm/test_nova.py
+28
View File
@@ -0,0 +1,28 @@
from sysinv.helm import nova
from sysinv.helm import helm
from sysinv.common import constants
from sysinv.tests.db import base as dbbase
class NovaGetOverrideTest(dbbase.ControllerHostTestCase):
def setUp(self):
super(NovaGetOverrideTest, self).setUp()
self.operator = helm.HelmOperator(self.dbapi)
self.nova = nova.NovaHelm(self.operator)
self.worker = self._create_test_host(
personality=constants.WORKER,
administrative=constants.ADMIN_LOCKED)
self.ifaces = self._create_test_host_platform_interface(self.worker)
self.dbapi.address_create({
'name': 'test',
'family': self.oam_subnet.version,
'prefix': self.oam_subnet.prefixlen,
'address': str(self.oam_subnet[24]),
'interface_id': self.ifaces[0].id,
'enable_dad': self.oam_subnet.version == 6
})
def test_update_host_addresses(self):
self.nova._update_host_addresses(self.worker, {}, {}, {})