Browse Source

Maintenance: Avoid code duplication

This patch is refactoring the common/maintenance.py module to avoid code
duplication when fixing resources.

Related-Bug: #1605089
Change-Id: Iacf73e4c8a5b5effad890888244745c98527991d
changes/07/536407/3
Lucas Alvares Gomes 4 years ago
parent
commit
dc84c23a86
  1. 278
      networking_ovn/common/maintenance.py
  2. 19
      networking_ovn/ovsdb/impl_idl_ovn.py
  3. 6
      networking_ovn/ovsdb/ovn_api.py
  4. 8
      networking_ovn/tests/unit/common/test_maintenance.py

278
networking_ovn/common/maintenance.py

@ -23,7 +23,6 @@ from neutron_lib import worker
from oslo_log import log
from networking_ovn.common import constants as ovn_const
from networking_ovn.common import utils
from networking_ovn.db import maintenance as db_maint
from networking_ovn.db import revision as db_rev
@ -92,179 +91,94 @@ class DBInconsistenciesPeriodics(object):
self._idl = self._nb_idl.idl
self._idl.set_lock('ovn_db_inconsistencies_periodics')
self._resources_func_map = {
ovn_const.TYPE_NETWORKS: {
'neutron_get': self._ovn_client._plugin.get_network,
'ovn_get': self._nb_idl.get_lswitch,
'ovn_create': self._ovn_client.create_network,
'ovn_update': self._ovn_client.update_network,
'ovn_delete': self._ovn_client.delete_network,
},
ovn_const.TYPE_PORTS: {
'neutron_get': self._ovn_client._plugin.get_port,
'ovn_get': self._nb_idl.get_lswitch_port,
'ovn_create': self._ovn_client.create_port,
'ovn_update': self._ovn_client.update_port,
'ovn_delete': self._ovn_client.delete_port,
},
ovn_const.TYPE_FLOATINGIPS: {
'neutron_get': self._ovn_client._l3_plugin.get_floatingip,
'ovn_get': self._nb_idl.get_floatingip,
'ovn_create': self._ovn_client.create_floatingip,
'ovn_update': self._ovn_client.update_floatingip,
'ovn_delete': self._ovn_client.delete_floatingip,
},
ovn_const.TYPE_ROUTERS: {
'neutron_get': self._ovn_client._l3_plugin.get_router,
'ovn_get': self._nb_idl.get_lrouter,
'ovn_create': self._ovn_client.create_router,
'ovn_update': self._ovn_client.update_router,
'ovn_delete': self._ovn_client.delete_router,
},
ovn_const.TYPE_SECURITY_GROUPS: {
'neutron_get': self._ovn_client._plugin.get_security_group,
'ovn_get': self._nb_idl.get_address_set,
'ovn_create': self._ovn_client.create_security_group,
'ovn_delete': self._ovn_client.delete_security_group,
},
ovn_const.TYPE_SECURITY_GROUP_RULES: {
'neutron_get':
self._ovn_client._plugin.get_security_group_rule,
'ovn_get': self._nb_idl.get_acl_by_id,
'ovn_create': self._ovn_client.create_security_group_rule,
'ovn_delete': self._ovn_client.delete_security_group_rule,
},
}
@property
def has_lock(self):
return not self._idl.is_lock_contended
def _fix_create_update_network(self, row):
# Get the latest version of the resource in Neutron DB
admin_context = n_context.get_admin_context()
n_db_obj = self._ovn_client._plugin.get_network(
admin_context, row.resource_uuid)
ovn_net = self._nb_idl.get_lswitch(utils.ovn_name(row.resource_uuid))
if not ovn_net:
# If the resource doesn't exist in the OVN DB, create it.
self._ovn_client.create_network(n_db_obj)
else:
ext_ids = getattr(ovn_net, 'external_ids', {})
ovn_revision = int(ext_ids.get(
ovn_const.OVN_REV_NUM_EXT_ID_KEY, -1))
# If the resource exist in the OVN DB but the revision
# number is different from Neutron DB, updated it.
if ovn_revision != n_db_obj['revision_number']:
self._ovn_client.update_network(n_db_obj)
else:
# If the resource exist and the revision number
# is equal on both databases just bump the revision on
# the cache table.
db_rev.bump_revision(n_db_obj, ovn_const.TYPE_NETWORKS)
def _fix_delete_network(self, row):
ovn_net = self._nb_idl.get_lswitch(utils.ovn_name(row.resource_uuid))
if not ovn_net:
db_rev.delete_revision(row.resource_uuid)
else:
self._ovn_client.delete_network(row.resource_uuid)
def _fix_create_update_port(self, row):
# Get the latest version of the resource in Neutron DB
admin_context = n_context.get_admin_context()
p_db_obj = self._ovn_client._plugin.get_port(
admin_context, row.resource_uuid)
ovn_port = self._nb_idl.get_lswitch_port(
utils.ovn_name(row.resource_uuid))
if not ovn_port:
# If the resource doesn't exist in the OVN DB, create it.
self._ovn_client.create_port(p_db_obj)
else:
ext_ids = getattr(ovn_port, 'external_ids', {})
ovn_revision = int(ext_ids.get(
ovn_const.OVN_REV_NUM_EXT_ID_KEY, -1))
# If the resource exist in the OVN DB but the revision
# number is different from Neutron DB, updated it.
if ovn_revision != p_db_obj['revision_number']:
self._ovn_client.update_port(p_db_obj)
else:
# If the resource exist and the revision number
# is equal on both databases just bump the revision on
# the cache table.
db_rev.bump_revision(p_db_obj, ovn_const.TYPE_PORTS)
def _fix_delete_port(self, row):
ovn_port = self._nb_idl.get_lswitch_port(
utils.ovn_name(row.resource_uuid))
if not ovn_port:
db_rev.delete_revision(row.resource_uuid)
else:
self._ovn_client.delete_port(row.resource_uuid)
def _fix_delete_sg_rule(self, row):
acl = self._nb_idl.get_acl_by_id(row.resource_uuid)
if not acl:
db_rev.delete_revision(row.resource_uuid)
else:
self._ovn_client.delete_security_group_rule(
row.resource_uuid)
def _fix_create_sg_rule(self, row):
# Get the latest version of the sg rule in Neutron DB
admin_context = n_context.get_admin_context()
sgr_db_obj = self._ovn_client._plugin.get_security_group_rule(
admin_context, row.resource_uuid)
if row.revision_number == ovn_const.INITIAL_REV_NUM:
self._ovn_client.create_security_group_rule(sgr_db_obj)
else:
LOG.error("SG rule %s found with a revision number while this "
"resource doesn't support updates.", row.resource_uuid)
def _fix_create_update_routers(self, row):
# Get the latest version of the resource in Neutron DB
admin_context = n_context.get_admin_context()
r_db_obj = self._ovn_client._l3_plugin.get_router(
admin_context, row.resource_uuid)
ovn_router = self._nb_idl.get_lrouter(
utils.ovn_name(row.resource_uuid))
if not ovn_router:
# If the resource doesn't exist in the OVN DB, create it.
self._ovn_client.create_router(r_db_obj)
else:
ext_ids = getattr(ovn_router, 'external_ids', {})
ovn_revision = int(ext_ids.get(
ovn_const.OVN_REV_NUM_EXT_ID_KEY, -1))
# If the resource exist in the OVN DB but the revision
# number is different from Neutron DB, updated it.
if ovn_revision != r_db_obj['revision_number']:
self._ovn_client.update_router(r_db_obj)
else:
# If the resource exist and the revision number
# is equal on both databases just bump the revision on
# the cache table.
db_rev.bump_revision(r_db_obj, ovn_const.TYPE_ROUTERS)
def _fix_delete_router(self, row):
ovn_router = self._nb_idl.get_lrouter(
utils.ovn_name(row.resource_uuid))
if not ovn_router:
db_rev.delete_revision(row.resource_uuid)
else:
self._ovn_client.delete_router(row.resource_uuid)
def _fix_create_security_group(self, row):
# Get the latest version of the resource in Neutron DB
def _fix_create_update(self, row):
res_map = self._resources_func_map[row.resource_type]
admin_context = n_context.get_admin_context()
sg_db_obj = self._ovn_client._plugin.get_security_group(
admin_context, row.resource_uuid)
ovn_sg = self._nb_idl.get_address_set(
utils.ovn_addrset_name(row.resource_uuid, 'ip4'))
# Since we don't have updates for Security Groups, we only need to
# check whether its been created or not.
if not ovn_sg:
self._ovn_client.create_security_group(sg_db_obj)
else:
db_rev.bump_revision(sg_db_obj, ovn_const.TYPE_SECURITY_GROUPS)
def _fix_delete_security_group(self, row):
ovn_sg = self._nb_idl.get_address_set(
utils.ovn_addrset_name(row.resource_uuid, 'ip4'))
if not ovn_sg:
db_rev.delete_revision(row.resource_uuid)
else:
self._ovn_client.delete_security_group(row.resource_uuid)
def _fix_create_update_floatingip(self, row):
# Get the latest version of the resource in Neutron DB
admin_context = n_context.get_admin_context()
fip_db_obj = self._ovn_client._l3_plugin.get_floatingip(
admin_context, row.resource_uuid)
ovn_fip = self._nb_idl.get_floatingip(row.resource_uuid)
n_obj = res_map['neutron_get'](admin_context, row.resource_uuid)
ovn_obj = res_map['ovn_get'](row.resource_uuid)
if not ovn_fip:
# If the resource doesn't exist in the OVN DB, create it.
self._ovn_client.create_floatingip(fip_db_obj)
if not ovn_obj:
res_map['ovn_create'](n_obj)
else:
ovn_revision = int(ovn_fip['external_ids'].get(
ovn_const.OVN_REV_NUM_EXT_ID_KEY, -1))
# If the resource exist in the OVN DB but the revision
# number is different from Neutron DB, updated it.
if ovn_revision != fip_db_obj['revision_number']:
self._ovn_client.update_floatingip(fip_db_obj)
if row.resource_type == ovn_const.TYPE_SECURITY_GROUP_RULES:
LOG.error("SG rule %s found with a revision number while "
"this resource doesn't support updates",
row.resource_uuid)
elif row.resource_type == ovn_const.TYPE_SECURITY_GROUPS:
# In OVN, we don't care about updates to security groups,
# so just bump the revision number to whatever it's
# supposed to be.
db_rev.bump_revision(n_obj, row.resource_type)
else:
# If the resource exist and the revision number
# is equal on both databases just bump the revision on
# the cache table.
db_rev.bump_revision(fip_db_obj, ovn_const.TYPE_FLOATINGIPS)
def _fix_delete_floatingip(self, row):
ovn_fip = self._nb_idl.get_floatingip(row.resource_uuid)
if not ovn_fip:
ext_ids = getattr(ovn_obj, 'external_ids', {})
ovn_revision = int(ext_ids.get(
ovn_const.OVN_REV_NUM_EXT_ID_KEY, -1))
# If the resource exist in the OVN DB but the revision
# number is different from Neutron DB, updated it.
if ovn_revision != n_obj['revision_number']:
res_map['ovn_update'](n_obj)
else:
# If the resource exist and the revision number
# is equal on both databases just bump the revision on
# the cache table.
db_rev.bump_revision(n_obj, row.resource_type)
def _fix_delete(self, row):
res_map = self._resources_func_map[row.resource_type]
ovn_obj = res_map['ovn_get'](row.resource_uuid)
if not ovn_obj:
db_rev.delete_revision(row.resource_uuid)
else:
self._ovn_client.delete_floatingip(row.resource_uuid)
res_map['ovn_delete'](row.resource_uuid)
def _fix_create_update_subnet(self, row):
# Get the lasted version of the port in Neutron DB
@ -296,20 +210,16 @@ class DBInconsistenciesPeriodics(object):
# Fix the create/update resources inconsistencies
for row in create_update_inconsistencies:
try:
if row.resource_type == ovn_const.TYPE_NETWORKS:
self._fix_create_update_network(row)
elif row.resource_type == ovn_const.TYPE_PORTS:
self._fix_create_update_port(row)
elif row.resource_type == ovn_const.TYPE_SECURITY_GROUP_RULES:
self._fix_create_sg_rule(row)
elif row.resource_type == ovn_const.TYPE_ROUTERS:
self._fix_create_update_routers(row)
elif row.resource_type == ovn_const.TYPE_SECURITY_GROUPS:
self._fix_create_security_group(row)
elif row.resource_type == ovn_const.TYPE_FLOATINGIPS:
self._fix_create_update_floatingip(row)
elif row.resource_type == ovn_const.TYPE_SUBNETS:
# NOTE(lucasagomes): The way to fix subnets is bit
# different than other resources. A subnet in OVN language
# is just a DHCP rule but, this rule only exist if the
# subnet in Neutron has the "enable_dhcp" attribute set
# to True. So, it's possible to have a consistent subnet
# resource even when it does not exist in the OVN database.
if row.resource_type == ovn_const.TYPE_SUBNETS:
self._fix_create_update_subnet(row)
else:
self._fix_create_update(row)
except Exception:
LOG.exception('Failed to fix resource %(res_uuid)s '
'(type: %(res_type)s)',
@ -319,20 +229,10 @@ class DBInconsistenciesPeriodics(object):
# Fix the deleted resources inconsistencies
for row in delete_inconsistencies:
try:
if row.resource_type == ovn_const.TYPE_NETWORKS:
self._fix_delete_network(row)
elif row.resource_type == ovn_const.TYPE_PORTS:
self._fix_delete_port(row)
elif row.resource_type == ovn_const.TYPE_SECURITY_GROUP_RULES:
self._fix_delete_sg_rule(row)
elif row.resource_type == ovn_const.TYPE_ROUTERS:
self._fix_delete_router(row)
elif row.resource_type == ovn_const.TYPE_SECURITY_GROUPS:
self._fix_delete_security_group(row)
elif row.resource_type == ovn_const.TYPE_FLOATINGIPS:
self._fix_delete_floatingip(row)
elif row.resource_type == ovn_const.TYPE_SUBNETS:
if row.resource_type == ovn_const.TYPE_SUBNETS:
self._ovn_client.delete_subnet(row.resource_uuid)
else:
self._fix_delete(row)
except Exception:
LOG.exception('Failed to fix deleted resource %(res_uuid)s '
'(type: %(res_type)s)',

19
networking_ovn/ovsdb/impl_idl_ovn.py

@ -18,6 +18,7 @@ from oslo_log import log
import tenacity
from neutron_lib.utils import helpers
from oslo_utils import uuidutils
from ovsdbapp.backend.ovs_idl import connection
from ovsdbapp.backend.ovs_idl import idlutils
from ovsdbapp.backend.ovs_idl import transaction as idl_trans
@ -537,8 +538,7 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
def get_lswitch_port(self, lsp_name):
try:
return idlutils.row_by_value(self.idl, 'Logical_Switch_Port',
'name', lsp_name)
return self.lookup('Logical_Switch_Port', lsp_name)
except idlutils.RowNotFound:
return None
@ -549,9 +549,14 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
return lsp.parent_name
def get_lswitch(self, lswitch_name):
# FIXME(lucasagomes): We should refactor those get_*()
# methods. Some of 'em require the name, others IDs etc... It can
# be confusing.
if uuidutils.is_uuid_like(lswitch_name):
lswitch_name = utils.ovn_name(lswitch_name)
try:
return idlutils.row_by_value(self.idl, 'Logical_Switch',
'name', lswitch_name)
return self.lookup('Logical_Switch', lswitch_name)
except idlutils.RowNotFound:
return None
@ -608,7 +613,8 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
nat['external_ip'] == external_ip):
return nat
def get_address_set(self, addr_name):
def get_address_set(self, addrset_id, ip_version='ip4'):
addr_name = utils.ovn_addrset_name(addrset_id, ip_version)
try:
return idlutils.row_by_value(self.idl, 'Address_Set',
'name', addr_name)
@ -621,6 +627,9 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
self, name, resource, resource_type, if_exists)
def get_lrouter(self, lrouter_name):
if uuidutils.is_uuid_like(lrouter_name):
lrouter_name = utils.ovn_name(lrouter_name)
# TODO(lucasagomes): Use lr_get() once we start refactoring this
# API to use methods from ovsdbapp.
lr = self.db_find_rows('Logical_Router', ('name', '=', lrouter_name))

6
networking_ovn/ovsdb/ovn_api.py

@ -592,10 +592,12 @@ class API(api.API):
"""
@abc.abstractmethod
def get_address_set(self, addr_name):
def get_address_set(self, addrset_id, ip_version='ip4'):
"""Get a Address Set by its ID.
:param addr_name: The Address Set name
:param addrset_id: The Address Set ID
:type addrset_id: string
:param ip_version: Either "ip4" or "ip6". Defaults to "ip4"
:type addr_name: string
:returns: The Address Set row or None
"""

8
networking_ovn/tests/unit/common/test_maintenance.py

@ -43,7 +43,7 @@ class TestDBInconsistenciesPeriodics(db_base.DBTestCase,
self.session = db_api.get_writer_session()
@mock.patch.object(maintenance.DBInconsistenciesPeriodics,
'_fix_create_update_network')
'_fix_create_update')
@mock.patch.object(db_maint, 'get_inconsistent_resources')
def test_check_for_inconsistencies(self, mock_get_incon_res, mock_fix_net):
fake_row = mock.Mock(resource_type=constants.TYPE_NETWORKS)
@ -70,7 +70,7 @@ class TestDBInconsistenciesPeriodics(db_base.DBTestCase,
self.fake_ovn_client._nb_idl.get_lswitch.return_value = fake_ls
self.fake_ovn_client._plugin.get_network.return_value = self.net
self.periodic._fix_create_update_network(row)
self.periodic._fix_create_update(row)
# Since the revision number was < 0, make sure create_network()
# is invoked with the latest version of the object in the neutron
@ -111,7 +111,7 @@ class TestDBInconsistenciesPeriodics(db_base.DBTestCase,
fake_lsp)
self.fake_ovn_client._plugin.get_port.return_value = self.port
self.periodic._fix_create_update_port(row)
self.periodic._fix_create_update(row)
# Since the revision number was < 0, make sure create_port()
# is invoked with the latest version of the object in the neutron
@ -150,7 +150,7 @@ class TestDBInconsistenciesPeriodics(db_base.DBTestCase,
mock.sentinel.AddressSet)
self.fake_ovn_client._plugin.get_security_group.return_value = sg
self.periodic._fix_create_security_group(row)
self.periodic._fix_create_update(row)
if revision_number < 0:
self.fake_ovn_client.create_security_group.assert_called_once_with(

Loading…
Cancel
Save