Browse Source

Check for security group correctness

This patch is updating networking-ovn to check for correctness when
creating and deleting security groups.

Note that, networking-ovn doesn't care about updates on security groups
so the patch only cares about whether the security group exist or not.

Change-Id: I2add7ba1eb989fd585792c751cba4ec87f3c4974
changes/37/529637/15
Lucas Alvares Gomes 4 years ago
parent
commit
08b95a5d04
  1. 1
      networking_ovn/common/constants.py
  2. 27
      networking_ovn/common/maintenance.py
  3. 2
      networking_ovn/common/ovn_client.py
  4. 3
      networking_ovn/common/utils.py
  5. 19
      networking_ovn/ml2/mech_driver.py
  6. 7
      networking_ovn/ovsdb/impl_idl_ovn.py
  7. 9
      networking_ovn/ovsdb/ovn_api.py
  8. 41
      networking_ovn/tests/unit/common/test_maintenance.py
  9. 5
      networking_ovn/tests/unit/ml2/test_mech_driver.py

1
networking_ovn/common/constants.py

@ -99,3 +99,4 @@ TYPE_NETWORKS = 'networks'
TYPE_PORTS = 'ports'
TYPE_SECURITY_GROUP_RULES = 'security_group_rules'
TYPE_ROUTERS = 'routers'
TYPE_SECURITY_GROUPS = 'security_groups'

27
networking_ovn/common/maintenance.py

@ -213,6 +213,29 @@ class DBInconsistenciesPeriodics(object):
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
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)
@periodics.periodic(spacing=DB_CONSISTENCY_CHECK_INTERVAL,
run_immediately=True)
def check_for_inconsistencies(self):
@ -238,6 +261,8 @@ class DBInconsistenciesPeriodics(object):
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)
except Exception:
LOG.exception('Failed to fix resource %(res_uuid)s '
'(type: %(res_type)s)',
@ -255,6 +280,8 @@ class DBInconsistenciesPeriodics(object):
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)
except Exception:
LOG.exception('Failed to fix deleted resource %(res_uuid)s '
'(type: %(res_type)s)',

2
networking_ovn/common/ovn_client.py

@ -1312,12 +1312,14 @@ class OVNClient(object):
ext_ids = {ovn_const.OVN_SG_EXT_ID_KEY: security_group['id']}
txn.add(self._nb_idl.create_address_set(
name=name, external_ids=ext_ids))
db_rev.bump_revision(security_group, ovn_const.TYPE_SECURITY_GROUPS)
def delete_security_group(self, security_group_id):
with self._nb_idl.transaction(check_error=True) as txn:
for ip_version in ('ip4', 'ip6'):
name = utils.ovn_addrset_name(security_group_id, ip_version)
txn.add(self._nb_idl.delete_address_set(name=name))
db_rev.delete_revision(security_group_id)
def _process_security_group_rule(self, rule, is_add_acl=True):
admin_context = n_context.get_admin_context()

3
networking_ovn/common/utils.py

@ -206,7 +206,8 @@ def get_revision_number(resource, resource_type):
if resource_type in (constants.TYPE_NETWORKS,
constants.TYPE_PORTS,
constants.TYPE_SECURITY_GROUP_RULES,
constants.TYPE_ROUTERS):
constants.TYPE_ROUTERS,
constants.TYPE_SECURITY_GROUPS):
return resource['revision_number']
else:
raise ovn_exc.UnknownResourceType(resource_type=resource_type)

19
networking_ovn/ml2/mech_driver.py

@ -147,6 +147,12 @@ class OVNMechanismDriver(api.MechanismDriver):
# Handle security group/rule notifications
if self.sg_enabled:
registry.subscribe(self._create_security_group_precommit,
resources.SECURITY_GROUP,
events.PRECOMMIT_CREATE)
registry.subscribe(self._update_security_group,
resources.SECURITY_GROUP,
events.AFTER_UPDATE)
registry.subscribe(self._create_security_group,
resources.SECURITY_GROUP,
events.AFTER_CREATE)
@ -198,6 +204,12 @@ class OVNMechanismDriver(api.MechanismDriver):
maintenance.DBInconsistenciesPeriodics(self._ovn_client))
self._maintenance_thread.start()
def _create_security_group_precommit(self, resource, event, trigger,
security_group, context, **kwargs):
db_rev.create_initial_revision(
security_group['id'], ovn_const.TYPE_SECURITY_GROUPS,
context.session)
def _create_security_group(self, resource, event, trigger,
security_group, **kwargs):
self._ovn_client.create_security_group(security_group)
@ -206,6 +218,13 @@ class OVNMechanismDriver(api.MechanismDriver):
security_group_id, **kwargs):
self._ovn_client.delete_security_group(security_group_id)
def _update_security_group(self, resource, event, trigger,
security_group, **kwargs):
# OVN doesn't care about updates to security groups, only if they
# exist or not. We are bumping the revision number here so it
# doesn't show as inconsistent to the maintenance periodic task
db_rev.bump_revision(security_group, ovn_const.TYPE_SECURITY_GROUPS)
def _create_sg_rule_precommit(self, resource, event, trigger, **kwargs):
sg_rule = kwargs.get('security_group_rule')
context = kwargs.get('context')

7
networking_ovn/ovsdb/impl_idl_ovn.py

@ -608,6 +608,13 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
nat['external_ip'] == external_ip):
return nat
def get_address_set(self, addr_name):
try:
return idlutils.row_by_value(self.idl, 'Address_Set',
'name', addr_name)
except idlutils.RowNotFound:
return None
def check_revision_number(self, name, resource, resource_type,
if_exists=True):
return cmd.CheckRevisionNumberCommand(

9
networking_ovn/ovsdb/ovn_api.py

@ -591,6 +591,15 @@ class API(api.API):
:returns: :class:`Command` with no result
"""
@abc.abstractmethod
def get_address_set(self, addr_name):
"""Get a Address Set by its ID.
:param addr_name: The Address Set name
:type addr_name: string
:returns: The Address Set row or None
"""
@six.add_metaclass(abc.ABCMeta)
class SbAPI(api.API):

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

@ -15,11 +15,12 @@
import mock
from neutron.tests.unit.plugins.ml2 import test_plugin
from neutron.tests.unit.plugins.ml2 import test_security_group as test_sg
from neutron_lib.db import api as db_api
from networking_ovn.common import constants
from networking_ovn.common import maintenance
from networking_ovn.common import utils
from networking_ovn.db import maintenance as db_maint
from networking_ovn.db import revision as db_rev
from networking_ovn.tests.unit.db import base as db_base
@ -28,7 +29,7 @@ from networking_ovn.tests.unit.db import base as db_base
@mock.patch.object(maintenance.DBInconsistenciesPeriodics,
'has_lock', lambda _: True)
class TestDBInconsistenciesPeriodics(db_base.DBTestCase,
test_plugin.Ml2PluginV2TestCase):
test_sg.Ml2SecurityGroupsTestCase):
def setUp(self):
super(TestDBInconsistenciesPeriodics, self).setUp()
@ -130,3 +131,39 @@ class TestDBInconsistenciesPeriodics(db_base.DBTestCase,
def test_fix_port_update(self):
self._test_fix_create_update_port(ovn_rev=5, neutron_rev=7)
@mock.patch.object(db_rev, 'bump_revision')
def _test_fix_security_group_create(self, mock_bump, revision_number):
sg_name = utils.ovn_addrset_name('fake_id', 'ip4')
sg = self._make_security_group(self.fmt, sg_name, '')['security_group']
db_rev.create_initial_revision(
sg['id'], constants.TYPE_SECURITY_GROUPS, self.session,
revision_number=revision_number)
row = self.get_revision_row(sg['id'])
self.assertEqual(revision_number, row.revision_number)
if revision_number < 0:
self.fake_ovn_client._nb_idl.get_address_set.return_value = None
else:
self.fake_ovn_client._nb_idl.get_address_set.return_value = (
mock.sentinel.AddressSet)
self.fake_ovn_client._plugin.get_security_group.return_value = sg
self.periodic._fix_create_security_group(row)
if revision_number < 0:
self.fake_ovn_client.create_security_group.assert_called_once_with(
sg)
else:
# If the object already exist let's make sure we just bump
# the revision number in the ovn_revision_numbers table
self.assertFalse(self.fake_ovn_client.create_security_group.called)
mock_bump.assert_called_once_with(
sg, constants.TYPE_SECURITY_GROUPS)
def test_fix_security_group_create_doesnt_exist(self):
self._test_fix_security_group_create(revision_number=-1)
def test_fix_security_group_create_version_mismatch(self):
self._test_fix_security_group_create(revision_number=2)

5
networking_ovn/tests/unit/ml2/test_mech_driver.py

@ -91,7 +91,8 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
).start()
revision_plugin.RevisionPlugin()
def test__create_security_group(self):
@mock.patch.object(db_rev, 'bump_revision')
def test__create_security_group(self, mock_bump):
self.mech_driver._create_security_group(
resources.SECURITY_GROUP, events.AFTER_CREATE, {},
security_group=self.fake_sg)
@ -104,6 +105,8 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
self.nb_ovn.create_address_set.assert_has_calls(
create_address_set_calls, any_order=True)
mock_bump.assert_called_once_with(
self.fake_sg, ovn_const.TYPE_SECURITY_GROUPS)
def test__delete_security_group(self):
self.mech_driver._delete_security_group(

Loading…
Cancel
Save