From: Aaron Rosen Date: Wed, 5 Jun 2013 00:19:07 +0000 (-0700) Subject: Change lazy='dynamic' to 'joined' in models_v2 X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=6c510bce812900af1b69f346d8f46a2cc5a8fe8c;p=openstack-build%2Fneutron-build.git Change lazy='dynamic' to 'joined' in models_v2 Previously several models were defined with lazy='dynamic'. This would then cause Ports.fixed_ips (or w/e was specified as dynamic) to be defined as a AppenderQuery object and when accessed would query for the result. Therefore in get_ports() for example, when fixed_ips was accessed in _make_port_dict() we would be issuing an addition query to the database to lookup the fixed_ips information. This patch changes update_port/subnet() so that it keeps track of what changed so it does not have to query the db for values it should already know about. This patch also adds a unit test for update_subnet() adding host routes where there was missing code coverage. Implements blueprint improve-db-performance Change-Id: I0b2214604042a1fd362cbbf3fd70e31adf0ce279 --- diff --git a/quantum/db/db_base_plugin_v2.py b/quantum/db/db_base_plugin_v2.py index 39cd5310b..225ad703a 100644 --- a/quantum/db/db_base_plugin_v2.py +++ b/quantum/db/db_base_plugin_v2.py @@ -671,6 +671,8 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2, new_ips): """Add or remove IPs from the port.""" ips = [] + # These ips are still on the port and haven't been removed + prev_ips = [] # the new_ips contain all of the fixed_ips that are to be updated if len(new_ips) > cfg.CONF.max_fixed_ips_per_port: @@ -684,6 +686,7 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2, original_ip['ip_address'] == new_ip['ip_address']): original_ips.remove(original_ip) new_ips.remove(new_ip) + prev_ips.append(original_ip) # Check if the IP's to add are OK to_add = self._test_fixed_ips_for_port(context, network_id, new_ips) @@ -699,7 +702,7 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2, LOG.debug(_("Port update. Adding %s"), to_add) network = self._get_network(context, network_id) ips = self._allocate_fixed_ips(context, network, to_add) - return ips + return ips, prev_ips def _allocate_ips_for_port(self, context, network, port): """Allocate IP addresses for the port. @@ -1186,6 +1189,8 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2, dns lease or we support gratuitous DHCP offers """ s = subnet['subnet'] + changed_host_routes = False + changed_dns = False db_subnet = self._get_subnet(context, id) # Fill 'ip_version' and 'allocation_pools' fields with the current # value since _validate_subnet() expects subnet spec has 'ip_version' @@ -1201,11 +1206,13 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2, with context.session.begin(subtransactions=True): if "dns_nameservers" in s: + changed_dns = True old_dns_list = self._get_dns_by_subnet(context, id) new_dns_addr_set = set(s["dns_nameservers"]) old_dns_addr_set = set([dns['address'] for dns in old_dns_list]) + new_dns = list(new_dns_addr_set) for dns_addr in old_dns_addr_set - new_dns_addr_set: for dns in old_dns_list: if dns['address'] == dns_addr: @@ -1221,6 +1228,7 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2, return ht['destination'] + "_" + ht['nexthop'] if "host_routes" in s: + changed_host_routes = True old_route_list = self._get_route_by_subnet(context, id) new_route_set = set([_combine(route) @@ -1239,11 +1247,24 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2, nexthop=route_str.partition("_")[2], subnet_id=id) context.session.add(route) + + # Gather host routes for result + new_routes = [] + for route_str in new_route_set: + new_routes.append( + {'destination': route_str.partition("_")[0], + 'nexthop': route_str.partition("_")[2]}) del s["host_routes"] subnet = self._get_subnet(context, id) subnet.update(s) - return self._make_subnet_dict(subnet) + result = self._make_subnet_dict(subnet) + # Keep up with fields that changed + if changed_dns: + result['dns_nameservers'] = new_dns + if changed_host_routes: + result['host_routes'] = new_routes + return result def delete_subnet(self, context, id): with context.session.begin(subtransactions=True): @@ -1357,20 +1378,21 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2, def update_port(self, context, id, port): p = port['port'] + changed_ips = False with context.session.begin(subtransactions=True): port = self._get_port(context, id) # Check if the IPs need to be updated if 'fixed_ips' in p: + changed_ips = True self._recycle_expired_ip_allocations(context, port['network_id']) original = self._make_port_dict(port, process_extensions=False) - ips = self._update_ips_for_port(context, - port["network_id"], - id, - original["fixed_ips"], - p['fixed_ips']) + added_ips, prev_ips = self._update_ips_for_port( + context, port["network_id"], id, original["fixed_ips"], + p['fixed_ips']) + # Update ips if necessary - for ip in ips: + for ip in added_ips: allocated = models_v2.IPAllocation( network_id=port['network_id'], port_id=port.id, ip_address=ip['ip_address'], subnet_id=ip['subnet_id'], @@ -1380,7 +1402,11 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2, # and then update the port port.update(self._filter_non_model_columns(p, models_v2.Port)) - return self._make_port_dict(port) + result = self._make_port_dict(port) + # Keep up with fields that changed + if changed_ips: + result['fixed_ips'] = prev_ips + added_ips + return result def delete_port(self, context, id): with context.session.begin(subtransactions=True): diff --git a/quantum/db/l3_db.py b/quantum/db/l3_db.py index ea7625a67..22fcf07f3 100644 --- a/quantum/db/l3_db.py +++ b/quantum/db/l3_db.py @@ -749,7 +749,7 @@ class L3_NAT_db_mixin(l3.RouterPluginBase): DEVICE_OWNER_FLOATINGIP]: # Raise port in use only if the port has IP addresses # Otherwise it's a stale port that can be removed - fixed_ips = port_db['fixed_ips'].all() + fixed_ips = port_db['fixed_ips'] if fixed_ips: raise l3.L3PortInUse(port_id=port_id, device_owner=port_db['device_owner']) diff --git a/quantum/db/models_v2.py b/quantum/db/models_v2.py index 1840fb654..f7e1aa1f4 100644 --- a/quantum/db/models_v2.py +++ b/quantum/db/models_v2.py @@ -71,7 +71,7 @@ class IPAllocationPool(model_base.BASEV2, HasId): last_ip = sa.Column(sa.String(64), nullable=False) available_ranges = orm.relationship(IPAvailabilityRange, backref='ipallocationpool', - lazy="dynamic", + lazy="joined", cascade='delete') def __repr__(self): @@ -116,7 +116,7 @@ class Port(model_base.BASEV2, HasId, HasTenant): name = sa.Column(sa.String(255)) network_id = sa.Column(sa.String(36), sa.ForeignKey("networks.id"), nullable=False) - fixed_ips = orm.relationship(IPAllocation, backref='ports', lazy="dynamic") + fixed_ips = orm.relationship(IPAllocation, backref='ports', lazy='joined') mac_address = sa.Column(sa.String(32), nullable=False) admin_state_up = sa.Column(sa.Boolean(), nullable=False) status = sa.Column(sa.String(16), nullable=False) @@ -148,7 +148,7 @@ class Subnet(model_base.BASEV2, HasId, HasTenant): gateway_ip = sa.Column(sa.String(64)) allocation_pools = orm.relationship(IPAllocationPool, backref='subnet', - lazy="dynamic", + lazy="joined", cascade='delete') enable_dhcp = sa.Column(sa.Boolean()) dns_nameservers = orm.relationship(DNSNameServer, diff --git a/quantum/tests/unit/test_db_plugin.py b/quantum/tests/unit/test_db_plugin.py index d2280a03e..b77835e16 100644 --- a/quantum/tests/unit/test_db_plugin.py +++ b/quantum/tests/unit/test_db_plugin.py @@ -2976,6 +2976,35 @@ class TestSubnetsV2(QuantumDbPluginV2TestCase): self.assertEqual(res['subnet']['gateway_ip'], data['subnet']['gateway_ip']) + def test_update_subnet_adding_additional_host_routes_and_dns(self): + host_routes = [{'destination': '172.16.0.0/24', + 'nexthop': '10.0.2.2'}] + with self.network() as network: + data = {'subnet': {'network_id': network['network']['id'], + 'cidr': '10.0.2.0/24', + 'ip_version': 4, + 'dns_nameservers': ['192.168.0.1'], + 'host_routes': host_routes, + 'tenant_id': network['network']['tenant_id']}} + subnet_req = self.new_create_request('subnets', data) + res = self.deserialize(self.fmt, subnet_req.get_response(self.api)) + + host_routes = [{'destination': '172.16.0.0/24', + 'nexthop': '10.0.2.2'}, + {'destination': '192.168.0.0/24', + 'nexthop': '10.0.2.3'}] + + dns_nameservers = ['192.168.0.1', '192.168.0.2'] + data = {'subnet': {'host_routes': host_routes, + 'dns_nameservers': dns_nameservers}} + req = self.new_update_request('subnets', data, + res['subnet']['id']) + res = self.deserialize(self.fmt, req.get_response(self.api)) + self.assertEquals(sorted(res['subnet']['host_routes']), + sorted(host_routes)) + self.assertEquals(sorted(res['subnet']['dns_nameservers']), + sorted(dns_nameservers)) + def test_update_subnet_shared_returns_400(self): with self.network(shared=True) as network: with self.subnet(network=network) as subnet: