From 78858e6719f621544748adb63a80f87c61ff460b Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Fri, 13 Nov 2020 12:21:22 +0000 Subject: [PATCH] Add "standard_attr_id" to some OVO dictionaries Included standard attributes ID in some OVO dictionaries to improve the OVN revision numbers operation. Having this ID, there is no need to retrieve it from the database. The following OVOs have been updated: - port - network - subnet - security group - security group rule - router Closes-Bug: #1904188 Change-Id: Ia16335a2fc8f9324b9489692c76a73e4ef5bef96 --- neutron/db/db_base_plugin_common.py | 23 +++++++++++++++---- neutron/db/l3_db.py | 4 +++- neutron/db/ovn_revision_numbers_db.py | 22 ++++++++++++------ neutron/db/securitygroups_db.py | 8 +++++-- .../drivers/ovn/mech_driver/mech_driver.py | 21 +++++++++++------ neutron/services/ovn_l3/plugin.py | 6 +++-- .../tests/unit/db/test_securitygroups_db.py | 1 + neutron/tests/unit/extensions/test_l3.py | 3 ++- neutron/tests/unit/fake_resources.py | 12 ++++++++++ neutron/tests/unit/plugins/ml2/test_plugin.py | 2 ++ 10 files changed, 77 insertions(+), 25 deletions(-) diff --git a/neutron/db/db_base_plugin_common.py b/neutron/db/db_base_plugin_common.py index 1194ccd4690..1ae1d26e4be 100644 --- a/neutron/db/db_base_plugin_common.py +++ b/neutron/db/db_base_plugin_common.py @@ -143,6 +143,11 @@ class DbBasePluginCommon(object): allocated.create() def _make_subnet_dict(self, subnet, fields=None, context=None): + if isinstance(subnet, subnet_obj.Subnet): + standard_attr_id = subnet.db_obj.standard_attr.id + else: + standard_attr_id = subnet.standard_attr.id + res = {'id': subnet['id'], 'name': subnet['name'], 'tenant_id': subnet['tenant_id'], @@ -152,6 +157,7 @@ class DbBasePluginCommon(object): 'enable_dhcp': subnet['enable_dhcp'], 'ipv6_ra_mode': subnet['ipv6_ra_mode'], 'ipv6_address_mode': subnet['ipv6_address_mode'], + 'standard_attr_id': standard_attr_id, } res['gateway_ip'] = str( subnet['gateway_ip']) if subnet['gateway_ip'] else None @@ -218,6 +224,13 @@ class DbBasePluginCommon(object): def _make_port_dict(self, port, fields=None, process_extensions=True, with_fixed_ips=True): + if isinstance(port, port_obj.Port): + port_data = port.db_obj + standard_attr_id = port.db_obj.standard_attr.id + else: + port_data = port + standard_attr_id = port.standard_attr.id + mac = port["mac_address"] if isinstance(mac, netaddr.EUI): mac.dialect = netaddr.mac_unix_expanded @@ -229,7 +242,9 @@ class DbBasePluginCommon(object): "admin_state_up": port["admin_state_up"], "status": port["status"], "device_id": port["device_id"], - "device_owner": port["device_owner"]} + "device_owner": port["device_owner"], + 'standard_attr_id': standard_attr_id, + } if with_fixed_ips: res["fixed_ips"] = [ {'subnet_id': ip["subnet_id"], @@ -237,9 +252,6 @@ class DbBasePluginCommon(object): ip["ip_address"])} for ip in port["fixed_ips"]] # Call auxiliary extend functions, if any if process_extensions: - port_data = port - if isinstance(port, port_obj.Port): - port_data = port.db_obj resource_extend.apply_funcs( port_def.COLLECTION_NAME, res, port_data) return db_utils.resource_fields(res, fields) @@ -310,7 +322,8 @@ class DbBasePluginCommon(object): 'mtu': network.get('mtu', constants.DEFAULT_NETWORK_MTU), 'status': network['status'], 'subnets': [subnet['id'] - for subnet in network['subnets']]} + for subnet in network['subnets']], + 'standard_attr_id': network.standard_attr.id} res['shared'] = self._is_network_shared(context, network.rbac_entries) # Call auxiliary extend functions, if any if process_extensions: diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index ceee76d7b06..46a5c0f19ee 100644 --- a/neutron/db/l3_db.py +++ b/neutron/db/l3_db.py @@ -1045,7 +1045,9 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, 'router_id': floatingip.router_id, 'port_id': floatingip.fixed_port_id, 'fixed_ip_address': fixed_ip_address, - 'status': floatingip.status} + 'status': floatingip.status, + 'standard_attr_id': floatingip.db_obj.standard_attr.id, + } # NOTE(mlavalle): The following assumes this mixin is used in a # class inheriting from CommonDbMixin, which is true for all existing # plugins. diff --git a/neutron/db/ovn_revision_numbers_db.py b/neutron/db/ovn_revision_numbers_db.py index ba2ba1b89b6..be07154d0cb 100644 --- a/neutron/db/ovn_revision_numbers_db.py +++ b/neutron/db/ovn_revision_numbers_db.py @@ -100,12 +100,12 @@ def _get_standard_attr_id(context, resource_uuid, resource_type): @db_api.retry_if_session_inactive() def create_initial_revision(context, resource_uuid, resource_type, revision_number=INITIAL_REV_NUM, - may_exist=False): + may_exist=False, std_attr_id=None): LOG.debug('create_initial_revision uuid=%s, type=%s, rev=%s', resource_uuid, resource_type, revision_number) db_func = context.session.merge if may_exist else context.session.add with context.session.begin(subtransactions=True): - std_attr_id = _get_standard_attr_id( + std_attr_id = std_attr_id or _get_standard_attr_id( context, resource_uuid, resource_type) row = ovn_models.OVNRevisionNumbers( resource_uuid=resource_uuid, resource_type=resource_type, @@ -124,7 +124,7 @@ def delete_revision(context, resource_uuid, resource_type): context.session.delete(row) -def _ensure_revision_row_exist(context, resource, resource_type): +def _ensure_revision_row_exist(context, resource, resource_type, std_attr_id): """Ensure the revision row exists. Ensure the revision row exist before we try to bump its revision @@ -145,7 +145,8 @@ def _ensure_revision_row_exist(context, resource, resource_type): '%(res_type)s) when bumping the revision number. ' 'Creating one.', {'res_uuid': resource['id'], 'res_type': resource_type}) - create_initial_revision(context, resource['id'], resource_type) + create_initial_revision(context, resource['id'], resource_type, + std_attr_id=std_attr_id) @db_api.retry_if_session_inactive() @@ -163,9 +164,16 @@ def get_revision_row(context, resource_uuid): def bump_revision(context, resource, resource_type): revision_number = ovn_utils.get_revision_number(resource, resource_type) with context.session.begin(subtransactions=True): - _ensure_revision_row_exist(context, resource, resource_type) - std_attr_id = _get_standard_attr_id( - context, resource['id'], resource_type) + # NOTE(ralonsoh): "resource" could be a dict or an OVO. + try: + std_attr_id = resource.db_obj.standard_attr.id + except AttributeError: + std_attr_id = resource.get('standard_attr_id', None) + _ensure_revision_row_exist(context, resource, resource_type, + std_attr_id) + std_attr_id = (std_attr_id or + _get_standard_attr_id(context, resource['id'], + resource_type)) row = context.session.merge(ovn_models.OVNRevisionNumbers( standard_attr_id=std_attr_id, resource_uuid=resource['id'], resource_type=resource_type)) diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py index a2cc80314fb..3a50379a3c3 100644 --- a/neutron/db/securitygroups_db.py +++ b/neutron/db/securitygroups_db.py @@ -327,7 +327,9 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, 'name': security_group['name'], 'stateful': security_group['stateful'], 'tenant_id': security_group['tenant_id'], - 'description': security_group['description']} + 'description': security_group['description'], + 'standard_attr_id': security_group.db_obj.standard_attr.id, + } if security_group.rules: res['security_group_rules'] = [ self._make_security_group_rule_dict(r.db_obj) @@ -661,7 +663,9 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, 'port_range_min': security_group_rule['port_range_min'], 'port_range_max': security_group_rule['port_range_max'], 'remote_ip_prefix': security_group_rule['remote_ip_prefix'], - 'remote_group_id': security_group_rule['remote_group_id']} + 'remote_group_id': security_group_rule['remote_group_id'], + 'standard_attr_id': security_group_rule.standard_attr.id, + } resource_extend.apply_funcs(ext_sg.SECURITYGROUPRULES, res, security_group_rule) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index 8008a47127d..e9aeb16026e 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -337,7 +337,8 @@ class OVNMechanismDriver(api.MechanismDriver): **kwargs): ovn_revision_numbers_db.create_initial_revision( kwargs['context'], kwargs['security_group']['id'], - ovn_const.TYPE_SECURITY_GROUPS) + ovn_const.TYPE_SECURITY_GROUPS, + std_attr_id=kwargs['security_group']['standard_attr_id']) def _create_security_group(self, resource, event, trigger, security_group, **kwargs): @@ -361,7 +362,8 @@ class OVNMechanismDriver(api.MechanismDriver): sg_rule = kwargs.get('security_group_rule') context = kwargs.get('context') ovn_revision_numbers_db.create_initial_revision( - context, sg_rule['id'], ovn_const.TYPE_SECURITY_GROUP_RULES) + context, sg_rule['id'], ovn_const.TYPE_SECURITY_GROUP_RULES, + std_attr_id=sg_rule['standard_attr_id']) def _process_sg_rule_notification( self, resource, event, trigger, **kwargs): @@ -470,7 +472,8 @@ class OVNMechanismDriver(api.MechanismDriver): self._validate_network_segments(context.network_segments) ovn_revision_numbers_db.create_initial_revision( context._plugin_context, context.current['id'], - ovn_const.TYPE_NETWORKS) + ovn_const.TYPE_NETWORKS, + std_attr_id=context.current['standard_attr_id']) def create_network_postcommit(self, context): """Create a network. @@ -547,7 +550,8 @@ class OVNMechanismDriver(api.MechanismDriver): def create_subnet_precommit(self, context): ovn_revision_numbers_db.create_initial_revision( context._plugin_context, context.current['id'], - ovn_const.TYPE_SUBNETS) + ovn_const.TYPE_SUBNETS, + std_attr_id=context.current['standard_attr_id']) def create_subnet_postcommit(self, context): self._ovn_client.create_subnet(context._plugin_context, @@ -593,14 +597,16 @@ class OVNMechanismDriver(api.MechanismDriver): port['id']) ovn_revision_numbers_db.create_initial_revision( - context._plugin_context, port['id'], ovn_const.TYPE_PORTS) + context._plugin_context, port['id'], ovn_const.TYPE_PORTS, + std_attr_id=context.current['standard_attr_id']) # in the case of router ports we also need to # track the creation and update of the LRP OVN objects if ovn_utils.is_lsp_router_port(port): ovn_revision_numbers_db.create_initial_revision( context._plugin_context, port['id'], - ovn_const.TYPE_ROUTER_PORTS) + ovn_const.TYPE_ROUTER_PORTS, + std_attr_id=context.current['standard_attr_id']) def _is_port_provisioning_required(self, port, host, original_host=None): vnic_type = port.get(portbindings.VNIC_TYPE, portbindings.VNIC_NORMAL) @@ -714,7 +720,8 @@ class OVNMechanismDriver(api.MechanismDriver): if not ovn_utils.is_lsp_router_port(original_port): ovn_revision_numbers_db.create_initial_revision( context._plugin_context, port['id'], - ovn_const.TYPE_ROUTER_PORTS, may_exist=True) + ovn_const.TYPE_ROUTER_PORTS, may_exist=True, + std_attr_id=context.current['standard_attr_id']) def update_port_postcommit(self, context): """Update a port. diff --git a/neutron/services/ovn_l3/plugin.py b/neutron/services/ovn_l3/plugin.py index 942b0e698cb..13e8af208d3 100644 --- a/neutron/services/ovn_l3/plugin.py +++ b/neutron/services/ovn_l3/plugin.py @@ -154,7 +154,8 @@ class OVNL3RouterPlugin(service_base.ServicePluginBase, def create_router_precommit(self, resource, event, trigger, context, router, router_id, router_db): db_rev.create_initial_revision( - context, router_id, ovn_const.TYPE_ROUTERS) + context, router_id, ovn_const.TYPE_ROUTERS, + std_attr_id=router_db.standard_attr.id) def create_router(self, context, router): router = super(OVNL3RouterPlugin, self).create_router(context, router) @@ -249,7 +250,8 @@ class OVNL3RouterPlugin(service_base.ServicePluginBase, def create_floatingip_precommit(self, resource, event, trigger, context, floatingip, floatingip_id, floatingip_db): db_rev.create_initial_revision( - context, floatingip_id, ovn_const.TYPE_FLOATINGIPS) + context, floatingip_id, ovn_const.TYPE_FLOATINGIPS, + std_attr_id=floatingip_db.standard_attr.id) def create_floatingip(self, context, floatingip, initial_status=n_const.FLOATINGIP_STATUS_DOWN): diff --git a/neutron/tests/unit/db/test_securitygroups_db.py b/neutron/tests/unit/db/test_securitygroups_db.py index cbf271db8c4..96a548bfe79 100644 --- a/neutron/tests/unit/db/test_securitygroups_db.py +++ b/neutron/tests/unit/db/test_securitygroups_db.py @@ -286,6 +286,7 @@ class SecurityGroupDbMixinTestCase(testlib_api.SqlTestCase): 'name': 'default', 'description': 'Default security group', 'stateful': mock.ANY, + 'standard_attr_id': mock.ANY, 'security_group_rules': [ # Four rules for egress/ingress and ipv4/ipv6 mock.ANY, mock.ANY, mock.ANY, mock.ANY, diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index a4ade645d3e..f34e3ff4d3c 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -3995,7 +3995,8 @@ class L3AgentDbTestCaseBase(L3NatTestCaseMixin): project_id=f['floatingip']['project_id'], router_id=f['floatingip']['router_id'], status=f['floatingip']['status'], - tenant_id=f['floatingip']['tenant_id']) + tenant_id=f['floatingip']['tenant_id'], + standard_attr_id=mock.ANY) finally: registry.unsubscribe(fake_method, resources.FLOATING_IP, events.AFTER_DELETE) diff --git a/neutron/tests/unit/fake_resources.py b/neutron/tests/unit/fake_resources.py index ff0a288e2cb..2a218a21c70 100644 --- a/neutron/tests/unit/fake_resources.py +++ b/neutron/tests/unit/fake_resources.py @@ -187,6 +187,16 @@ class FakePlugin(object): self._get_port_security_group_bindings = mock.Mock() +class FakeStandardAttribute(object): + + def __init__(self, _id=1, resource_type=mock.ANY, description=mock.ANY, + revision_number=1): + self.id = _id + self.resource_type = resource_type + self.description = description + self.revision_number = revision_number + + class FakeResource(dict): def __init__(self, manager=None, info=None, loaded=False, methods=None): @@ -282,6 +292,7 @@ class FakeNetwork(object): 'availability_zones': [], 'availability_zone_hints': [], 'is_default': False, + 'standard_attr_id': 1, } # Overwrite default attributes. @@ -659,6 +670,7 @@ class FakeFloatingIp(object): 'dns_domain': '', 'dns_name': '', 'project_id': '', + 'standard_attr': FakeStandardAttribute(), } # Overwrite default attributes. diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index 62244748565..1f27e266653 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -1181,6 +1181,7 @@ class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase): with self.port() as port: port_id = port['port']['id'] new_port = plugin.update_port(ctx, port_id, {"port": {}}) + new_port.pop('standard_attr_id') self.assertEqual(port["port"], new_port) def _add_fake_dhcp_agent(self): @@ -2076,6 +2077,7 @@ class TestMl2DvrPortsV2(TestMl2PortsV2): self.assertEqual(1, publish.call_count) # needed for a full match in the assertion below port['port']['extra_dhcp_opts'] = [] + port['port']['standard_attr_id'] = mock.ANY expected = [mock.call(resources.PORT, events.PRECOMMIT_DELETE, mock.ANY, network=mock.ANY, bind=mock.ANY, port=port['port'], port_db=mock.ANY,