diff --git a/neutron/db/availability_zone/router.py b/neutron/db/availability_zone/router.py index 3e0650af76e..c346ce0cd29 100644 --- a/neutron/db/availability_zone/router.py +++ b/neutron/db/availability_zone/router.py @@ -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) diff --git a/neutron/db/l3_attrs_db.py b/neutron/db/l3_attrs_db.py index 1e71f745bb6..9c0af8660c0 100644 --- a/neutron/db/l3_attrs_db.py +++ b/neutron/db/l3_attrs_db.py @@ -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 " diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index 886af4af115..c291a45344b 100644 --- a/neutron/db/l3_db.py +++ b/neutron/db/l3_db.py @@ -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) diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index 6b3e357d458..38b79612abc 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -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) diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py index 4c26e5e5d7f..0ec256c4a54 100644 --- a/neutron/db/l3_hamode_db.py +++ b/neutron/db/l3_hamode_db.py @@ -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) diff --git a/neutron/objects/router.py b/neutron/objects/router.py index 42dc3024fbe..60cf54e2972 100644 --- a/neutron/objects/router.py +++ b/neutron/objects/router.py @@ -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): diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index de19ebf57f5..5471c3b6397 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -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): diff --git a/neutron/tests/unit/db/test_l3_db.py b/neutron/tests/unit/db/test_l3_db.py index d50179237df..7f1c5976f02 100644 --- a/neutron/tests/unit/db/test_l3_db.py +++ b/neutron/tests/unit/db/test_l3_db.py @@ -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'], diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index 14039edf56a..81d60ae800e 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -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'] diff --git a/neutron/tests/unit/objects/test_router.py b/neutron/tests/unit/objects/test_router.py index 806cba8d786..88714e70113 100644 --- a/neutron/tests/unit/objects/test_router.py +++ b/neutron/tests/unit/objects/test_router.py @@ -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):