Always create a "router_extra_attributes" register per router

The table "router_extra_attributes" is a child of "router" table.
Each register contains extra information that completes the router
description. When using ML2/OVS mechanism driver, the methods that
create and populate the "router_extra_attributes" register are always
called from the L3 DVR, L3 HA and availability zones extensions.

When using ML2/OVN, those extensions are not loaded and therefore the
"router_extra_attributes" register is not created.

Despite this register is currently not used in ML2/OVN (it will be in
future features), there are some project expecting the
"router_extra_attributes" register to be always created (for example,
neutron-dynamic-routing [1]).

This patch enforces the child register creating always when a router is
created. This register is populated with the default values. This new
register does not affect any current operation related to ML2/OVN nor
ML2/OVS.

There is a 1:1 relationship between "routers" and
"router_extra_attributes". The child register is deleted by the database
engine when the "routers" register is deleted (ondelete="CASCADE").

[1]https://review.opendev.org/c/openstack/neutron-dynamic-routing/+/863713

Closes-Bug: #1995974
Change-Id: Ic546e40513402fa101c9687acce382cd6b84356c
This commit is contained in:
Rodolfo Alonso Hernandez 2022-11-11 12:57:05 +01:00 committed by Rodolfo Alonso
parent 9e2a0ac058
commit 2081910d6d
10 changed files with 105 additions and 28 deletions

View File

@ -49,5 +49,4 @@ class RouterAvailabilityZoneMixin(l3_attrs_db.ExtraAttributesMixin):
if az_hints:
self.validate_availability_zones(context, 'router',
az_hints)
self.set_extra_attr_value(context, router_db, az_def.AZ_HINTS,
az_hints)
self.set_extra_attr_value(router_db, az_def.AZ_HINTS, az_hints)

View File

@ -45,20 +45,20 @@ class ExtraAttributesMixin(object):
from_db = info.get('transform_from_db', lambda x: x)
router_res[name] = from_db(extra_attrs.get(name, info['default']))
def _ensure_extra_attr_model(self, context, router_db):
if not router_db['extra_attributes']:
kwargs = {k: v['default'] for k, v in get_attr_info().items()}
kwargs['router_id'] = router_db['id']
new = l3_attrs.RouterExtraAttributes(**kwargs)
context.session.add(new)
router_db['extra_attributes'] = new
@staticmethod
def add_extra_attr(context, router_db):
kwargs = {k: v['default'] for k, v in get_attr_info().items()}
kwargs['router_id'] = router_db['id']
new = l3_attrs.RouterExtraAttributes(**kwargs)
context.session.add(new)
router_db['extra_attributes'] = new
def set_extra_attr_value(self, context, router_db, key, value):
@staticmethod
def set_extra_attr_value(router_db, key, value):
# set a single value explicitly
if key in get_attr_info():
info = get_attr_info()[key]
to_db = info.get('transform_to_db', lambda x: x)
self._ensure_extra_attr_model(context, router_db)
router_db['extra_attributes'].update({key: to_db(value)})
return
raise RuntimeError(_("Tried to set a key '%s' that doesn't exist "

View File

@ -49,6 +49,7 @@ from neutron.api.rpc.agentnotifiers import l3_rpc_agent_api
from neutron.common import ipv6_utils
from neutron.common import utils
from neutron.db import _utils as db_utils
from neutron.db import l3_attrs_db
from neutron.db.models import l3 as l3_models
from neutron.db.models import l3_attrs as l3_attrs_models
from neutron.db import models_v2
@ -249,6 +250,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
status=constants.ACTIVE,
description=router.get('description'))
context.session.add(router_db)
l3_attrs_db.ExtraAttributesMixin.add_extra_attr(context, router_db)
registry.publish(resources.ROUTER, events.PRECOMMIT_CREATE, self,
payload=events.DBEventPayload(
@ -284,6 +286,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
def create_router(self, context, router):
r = router['router']
gw_info = r.get(EXTERNAL_GW_INFO, None)
# TODO(ralonsoh): migrate "tenant_id" to "project_id"
# https://blueprints.launchpad.net/neutron/+spec/keystone-v3
create = functools.partial(self._create_router_db, context, r,
r['tenant_id'])
delete = functools.partial(self.delete_router, context)

View File

@ -103,13 +103,11 @@ class DVRResourceOperationHandler(object):
priority_group.PRIORITY_ROUTER_EXTENDED_ATTRIBUTE)
def _set_distributed_flag(self, resource, event, trigger, payload):
"""Event handler to set distributed flag on creation."""
context = payload.context
router = payload.latest_state
router_db = payload.metadata['router_db']
dist = is_distributed_router(router)
router['distributed'] = dist
self.l3plugin.set_extra_attr_value(context, router_db, 'distributed',
dist)
self.l3plugin.set_extra_attr_value(router_db, 'distributed', dist)
def _validate_router_migration(self, context, router_db, router_res,
old_router=None):
@ -203,8 +201,7 @@ class DVRResourceOperationHandler(object):
payload.context, payload.resource_id,
agent['id'])
self.l3plugin.set_extra_attr_value(
payload.context, payload.desired_state,
'distributed', migrating_to_distributed)
payload.desired_state, 'distributed', migrating_to_distributed)
@registry.receives(resources.ROUTER, [events.AFTER_UPDATE],
priority_group.PRIORITY_ROUTER_EXTENDED_ATTRIBUTE)

View File

@ -378,7 +378,7 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin,
router_db = payload.metadata['router_db']
is_ha = self._is_ha(router)
router['ha'] = is_ha
self.set_extra_attr_value(context, router_db, 'ha', is_ha)
self.set_extra_attr_value(router_db, 'ha', is_ha)
if not is_ha:
return
# This will throw an exception if there aren't enough agents to
@ -452,14 +452,14 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin,
payload.desired_state.extra_attributes.ha_vr_id = None
if (payload.request_body.get('distributed') or
payload.states[0]['distributed']):
self.set_extra_attr_value(payload.context, payload.desired_state,
self.set_extra_attr_value(payload.desired_state,
'ha', requested_ha_state)
return
self._migrate_router_ports(
payload.context, payload.desired_state,
old_owner=old_owner, new_owner=new_owner)
self.set_extra_attr_value(
payload.context, payload.desired_state, 'ha', requested_ha_state)
payload.desired_state, 'ha', requested_ha_state)
@registry.receives(resources.ROUTER, [events.AFTER_UPDATE],
priority_group.PRIORITY_ROUTER_EXTENDED_ATTRIBUTE)

View File

@ -23,6 +23,7 @@ from neutron_lib.utils import net as net_utils
from oslo_utils import versionutils
from oslo_versionedobjects import fields as obj_fields
from sqlalchemy import func
from sqlalchemy import sql
from neutron.db.models import dvr as dvr_models
from neutron.db.models import l3
@ -278,6 +279,16 @@ class Router(base.NeutronDbObject):
if _target_version < (1, 1):
primitive.pop('qos_policy_id', None)
@staticmethod
@db_api.CONTEXT_READER
def get_router_ids_without_router_std_attrs(context):
r_attrs = l3_attrs.RouterExtraAttributes
query = context.session.query(l3.Router)
query = query.join(r_attrs, r_attrs.router_id == l3.Router.id,
isouter=True)
query = query.filter(r_attrs.router_id == sql.null())
return [r.id for r in query.all()]
@base.NeutronObjectRegistry.register
class FloatingIP(base.NeutronDbObject):

View File

@ -24,6 +24,7 @@ from neutron_lib.api.definitions import portbindings
from neutron_lib.api.definitions import provider_net as pnet
from neutron_lib import constants as n_const
from neutron_lib import context as n_context
from neutron_lib.db import api as db_api
from neutron_lib import exceptions as n_exc
from oslo_config import cfg
from oslo_log import log
@ -33,9 +34,11 @@ from ovsdbapp.backend.ovs_idl import event as row_event
from neutron.common.ovn import constants as ovn_const
from neutron.common.ovn import utils
from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
from neutron.db import l3_attrs_db
from neutron.db import ovn_hash_ring_db as hash_ring_db
from neutron.db import ovn_revision_numbers_db as revision_numbers_db
from neutron.objects import ports as ports_obj
from neutron.objects import router as router_obj
from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_db_sync
@ -818,6 +821,30 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
txn.add(cmd)
raise periodics.NeverAgain()
# TODO(ralonsoh): Remove this in the Antelope+4 cycle
@periodics.periodic(spacing=600, run_immediately=True)
def create_router_extra_attributes_registers(self):
"""Create missing ``RouterExtraAttributes`` registers.
ML2/OVN L3 plugin does not inherit the ``ExtraAttributesMixin`` class.
Before LP#1995974, the L3 plugin was not creating a
``RouterExtraAttributes`` register per ``Routers`` register. This one
only execution method finds those ``Routers`` registers without the
child one and creates one with the default values.
"""
if not self.has_lock:
return
context = n_context.get_admin_context()
for router_id in router_obj.Router.\
get_router_ids_without_router_std_attrs(context):
with db_api.CONTEXT_WRITER.using(context):
router_db = {'id': router_id}
l3_attrs_db.ExtraAttributesMixin.add_extra_attr(context,
router_db)
raise periodics.NeverAgain()
class HashRingHealthCheckPeriodics(object):

View File

@ -37,6 +37,7 @@ import webob.exc
from neutron.db import extraroute_db
from neutron.db import l3_db
from neutron.db.models import l3 as l3_models
from neutron.db.models import l3_attrs
from neutron.db import models_v2
from neutron.extensions import segment as segment_ext
from neutron.objects import base as base_obj
@ -869,6 +870,20 @@ class L3TestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
]
mock_publish.assert_has_calls(expected_calls)
def test_create_router_extra_attr(self):
router_args = {'router': {'name': 'foo_router',
'admin_state_up': True,
'tenant_id': 'foo_tenant'}
}
router_dict = self.create_router(router_args)
with db_api.CONTEXT_READER.using(self.ctx) as session:
r_extra_attrs = session.query(
l3_attrs.RouterExtraAttributes).filter(
l3_attrs.RouterExtraAttributes.router_id ==
router_dict['id']).all()
self.assertEqual(1, len(r_extra_attrs))
self.assertEqual(router_dict['id'], r_extra_attrs[0].router_id)
def test_update_router_notify(self):
with mock.patch.object(l3_db.registry, 'publish') as mock_publish:
self.mixin.update_router(self.ctx, self.router['id'],

View File

@ -625,9 +625,15 @@ class ExtraAttributesMixinTestCase(testlib_api.SqlTestCase):
self.mixin = l3_attrs_db.ExtraAttributesMixin()
directory.add_plugin(plugin_constants.L3, self.mixin)
self.ctx = context.get_admin_context()
self.router = l3_models.Router()
with db_api.CONTEXT_WRITER.using(self.ctx):
self.ctx.session.add(self.router)
self.router = self._new_router(self.ctx)
@staticmethod
@db_api.CONTEXT_WRITER
def _new_router(ctx):
router = l3_models.Router()
ctx.session.add(router)
l3_attrs_db.ExtraAttributesMixin.add_extra_attr(ctx, router)
return router
def _get_default_api_values(self):
return {k: v.get('transform_from_db', lambda x: x)(v['default'])
@ -636,8 +642,7 @@ class ExtraAttributesMixinTestCase(testlib_api.SqlTestCase):
def test_set_extra_attr_key_bad(self):
with testtools.ExpectedException(RuntimeError):
with db_api.CONTEXT_WRITER.using(self.ctx):
self.mixin.set_extra_attr_value(self.ctx, self.router,
'bad', 'value')
self.mixin.set_extra_attr_value(self.router, 'bad', 'value')
def test__extend_extra_router_dict_defaults(self):
rdict = {}
@ -646,9 +651,8 @@ class ExtraAttributesMixinTestCase(testlib_api.SqlTestCase):
def test_set_attrs_and_extend(self):
with db_api.CONTEXT_WRITER.using(self.ctx):
self.mixin.set_extra_attr_value(self.ctx, self.router,
'ha_vr_id', 99)
self.mixin.set_extra_attr_value(self.ctx, self.router,
self.mixin.set_extra_attr_value(self.router, 'ha_vr_id', 99)
self.mixin.set_extra_attr_value(self.router,
'availability_zone_hints',
['x', 'y', 'z'])
expected = self._get_default_api_values()
@ -658,7 +662,7 @@ class ExtraAttributesMixinTestCase(testlib_api.SqlTestCase):
self.mixin._extend_extra_router_dict(rdict, self.router)
self.assertEqual(expected, rdict)
self.mixin.set_extra_attr_value(self.ctx, self.router,
self.mixin.set_extra_attr_value(self.router,
'availability_zone_hints',
['z', 'y', 'z'])
expected['availability_zone_hints'] = ['z', 'y', 'z']

View File

@ -14,8 +14,10 @@
from unittest import mock
from neutron_lib.db import api as db_api
from oslo_utils import uuidutils
from neutron.db import l3_attrs_db
from neutron.objects.qos import binding as qos_binding
from neutron.objects.qos import policy
from neutron.objects import router
@ -166,6 +168,24 @@ class RouterDbObjectTestCase(obj_test_base.BaseDbObjectTestCase,
router_dict = router_obj.obj_to_primitive('1.0')
self.assertNotIn('qos_policy_id', router_dict['versioned_object.data'])
def test_get_router_ids_without_router_std_attrs(self):
def create_r_attr_reg(idx):
with db_api.CONTEXT_WRITER.using(self.context):
router_db = {'id': self.objs[idx].id}
l3_attrs_db.ExtraAttributesMixin.add_extra_attr(self.context,
router_db)
for idx in range(3):
self.objs[idx].create()
expected_router_ids = [r.id for r in self.objs]
for idx in range(3):
router_ids = router.Router.\
get_router_ids_without_router_std_attrs(self.context)
self.assertEqual(expected_router_ids, router_ids)
create_r_attr_reg(idx)
expected_router_ids = expected_router_ids[1:]
class RouterPortIfaceObjectTestCase(obj_test_base.BaseObjectIfaceTestCase):