delete sg rule on sg deletion on ODL neutron northbound
When security group is deleted, rules belongs to that security groups are
automatically deleted by cascade=delete.
So rules need to be deleted explicitly when security group is deleted.
Otherwise stale rules remains.
Closes-Bug: #1663470
Change-Id: I44a07d27e44ef5ff17409163c3194cb47b1eede0
(cherry picked from commit c2d7228f1a)
This commit is contained in:
committed by
juraj.linkes
parent
57d9b878c0
commit
58cba3a394
@@ -87,7 +87,10 @@ class OdlSecurityGroupsHandler(object):
|
|||||||
'res_id': res_id, 'odl_res_dict': odl_res_dict,
|
'res_id': res_id, 'odl_res_dict': odl_res_dict,
|
||||||
'kwargs': kwargs})
|
'kwargs': kwargs})
|
||||||
|
|
||||||
callback(context, odl_ops, odl_res_type, res_id, odl_res_dict)
|
copy_kwargs = kwargs.copy()
|
||||||
|
copy_kwargs.pop('context')
|
||||||
|
callback(context, odl_ops, odl_res_type, res_id, odl_res_dict,
|
||||||
|
**copy_kwargs)
|
||||||
|
|
||||||
def sg_callback_precommit(self, resource, event, trigger, **kwargs):
|
def sg_callback_precommit(self, resource, event, trigger, **kwargs):
|
||||||
self._sg_callback(self._precommit, resource, event, trigger, **kwargs)
|
self._sg_callback(self._precommit, resource, event, trigger, **kwargs)
|
||||||
|
|||||||
@@ -379,8 +379,8 @@ class OpenDaylightDriver(object):
|
|||||||
'object_id': obj_id})
|
'object_id': obj_id})
|
||||||
self.out_of_sync = True
|
self.out_of_sync = True
|
||||||
|
|
||||||
def sync_from_callback(self, context,
|
def sync_from_callback(self, context, operation, res_type,
|
||||||
operation, res_type, res_id, resource_dict):
|
res_id, resource_dict, **kwrags):
|
||||||
object_type = odl_utils.neutronify(res_type.plural)
|
object_type = odl_utils.neutronify(res_type.plural)
|
||||||
try:
|
try:
|
||||||
if operation == odl_const.ODL_DELETE:
|
if operation == odl_const.ODL_DELETE:
|
||||||
|
|||||||
@@ -178,7 +178,7 @@ class OpenDaylightMechanismDriver(api.MechanismDriver):
|
|||||||
rule['id'], odl_const.ODL_CREATE, res_rule)
|
rule['id'], odl_const.ODL_CREATE, res_rule)
|
||||||
|
|
||||||
def sync_from_callback_precommit(self, context, operation, res_type,
|
def sync_from_callback_precommit(self, context, operation, res_type,
|
||||||
res_id, resource_dict):
|
res_id, resource_dict, **kwargs):
|
||||||
object_type = res_type.singular
|
object_type = res_type.singular
|
||||||
if resource_dict is not None:
|
if resource_dict is not None:
|
||||||
resource_dict = resource_dict[object_type]
|
resource_dict = resource_dict[object_type]
|
||||||
@@ -237,9 +237,18 @@ class OpenDaylightMechanismDriver(api.MechanismDriver):
|
|||||||
_("unsupporetd bulk creation of security group rule"))
|
_("unsupporetd bulk creation of security group rule"))
|
||||||
journal.record(context, None, object_type, object_uuid,
|
journal.record(context, None, object_type, object_uuid,
|
||||||
operation, resource_dict)
|
operation, resource_dict)
|
||||||
|
# NOTE(yamahata): DB auto deletion
|
||||||
|
# Security Group Rule under this Security Group needs to
|
||||||
|
# be deleted. At NeutronDB layer rules are auto deleted with
|
||||||
|
# cascade='all,delete'.
|
||||||
|
if (object_type == odl_const.ODL_SG and
|
||||||
|
operation == odl_const.ODL_DELETE):
|
||||||
|
for rule in kwargs['security_group'].rules:
|
||||||
|
journal.record(context, None, odl_const.ODL_SG_RULE,
|
||||||
|
rule.id, odl_const.ODL_DELETE, [object_uuid])
|
||||||
|
|
||||||
def sync_from_callback_postcommit(self, context, operation, res_type,
|
def sync_from_callback_postcommit(self, context, operation, res_type,
|
||||||
res_id, resource_dict):
|
res_id, resource_dict, **kwargs):
|
||||||
self._postcommit(context)
|
self._postcommit(context)
|
||||||
|
|
||||||
def _postcommit(self, context):
|
def _postcommit(self, context):
|
||||||
|
|||||||
@@ -50,7 +50,7 @@ class ODLCallbackTestCase(testtools.TestCase):
|
|||||||
self._precommit.assert_called_with(
|
self._precommit.assert_called_with(
|
||||||
plugin_context_mock, op,
|
plugin_context_mock, op,
|
||||||
callback._RESOURCE_MAPPING[resources.SECURITY_GROUP], sg_id,
|
callback._RESOURCE_MAPPING[resources.SECURITY_GROUP], sg_id,
|
||||||
expected_dict)
|
expected_dict, security_group=sg, security_group_id=sg_id)
|
||||||
|
|
||||||
def _test_callback_postcommit_for_sg(self, event, op, sg, sg_id):
|
def _test_callback_postcommit_for_sg(self, event, op, sg, sg_id):
|
||||||
plugin_context_mock = mock.Mock()
|
plugin_context_mock = mock.Mock()
|
||||||
@@ -66,7 +66,7 @@ class ODLCallbackTestCase(testtools.TestCase):
|
|||||||
self._postcommit.assert_called_with(
|
self._postcommit.assert_called_with(
|
||||||
plugin_context_mock, op,
|
plugin_context_mock, op,
|
||||||
callback._RESOURCE_MAPPING[resources.SECURITY_GROUP], sg_id,
|
callback._RESOURCE_MAPPING[resources.SECURITY_GROUP], sg_id,
|
||||||
expected_dict)
|
expected_dict, security_group=sg, security_group_id=sg_id)
|
||||||
|
|
||||||
def test_callback_precommit_sg_create(self):
|
def test_callback_precommit_sg_create(self):
|
||||||
sg = mock.Mock()
|
sg = mock.Mock()
|
||||||
@@ -112,7 +112,8 @@ class ODLCallbackTestCase(testtools.TestCase):
|
|||||||
self._precommit.assert_called_with(
|
self._precommit.assert_called_with(
|
||||||
plugin_context_mock, op,
|
plugin_context_mock, op,
|
||||||
callback._RESOURCE_MAPPING[resources.SECURITY_GROUP_RULE],
|
callback._RESOURCE_MAPPING[resources.SECURITY_GROUP_RULE],
|
||||||
sg_rule_id, expected_dict)
|
sg_rule_id, expected_dict, security_group_rule=sg_rule,
|
||||||
|
security_group_rule_id=sg_rule_id)
|
||||||
|
|
||||||
@mock.patch.object(OpenDaylightDriver, 'sync_from_callback')
|
@mock.patch.object(OpenDaylightDriver, 'sync_from_callback')
|
||||||
def _test_callback_postcommit_for_sg_rules(
|
def _test_callback_postcommit_for_sg_rules(
|
||||||
@@ -130,7 +131,9 @@ class ODLCallbackTestCase(testtools.TestCase):
|
|||||||
self._postcommit.assert_called_with(
|
self._postcommit.assert_called_with(
|
||||||
plugin_context_mock, op,
|
plugin_context_mock, op,
|
||||||
callback._RESOURCE_MAPPING[resources.SECURITY_GROUP_RULE],
|
callback._RESOURCE_MAPPING[resources.SECURITY_GROUP_RULE],
|
||||||
sg_rule_id, expected_dict)
|
sg_rule_id, expected_dict,
|
||||||
|
security_group_rule=sg_rule, security_group_rule_id=sg_rule_id,
|
||||||
|
)
|
||||||
|
|
||||||
def test_callback_precommit_sg_rules_create(self):
|
def test_callback_precommit_sg_rules_create(self):
|
||||||
rule = mock.Mock()
|
rule = mock.Mock()
|
||||||
|
|||||||
@@ -263,6 +263,7 @@ class OpenDaylightMechanismDriverTestCase(OpenDaylightConfigBase):
|
|||||||
'project_id': 'test-tenant',
|
'project_id': 'test-tenant',
|
||||||
'tenant_id': 'test-tenant',
|
'tenant_id': 'test-tenant',
|
||||||
'description': 'test-description',
|
'description': 'test-description',
|
||||||
|
'rules': [],
|
||||||
'id': SG_FAKE_ID}}
|
'id': SG_FAKE_ID}}
|
||||||
return context
|
return context
|
||||||
|
|
||||||
@@ -324,16 +325,24 @@ class OpenDaylightMechanismDriverTestCase(OpenDaylightConfigBase):
|
|||||||
context_ = (copy.deepcopy(context)
|
context_ = (copy.deepcopy(context)
|
||||||
if operation != odl_const.ODL_DELETE else None)
|
if operation != odl_const.ODL_DELETE else None)
|
||||||
if (object_type == odl_const.ODL_SG and
|
if (object_type == odl_const.ODL_SG and
|
||||||
operation == odl_const.ODL_CREATE):
|
operation in [odl_const.ODL_CREATE, odl_const.ODL_DELETE]):
|
||||||
# TODO(yamahata): remove this work around once
|
# TODO(yamahata): remove this work around once
|
||||||
# https://review.openstack.org/#/c/281693/
|
# https://review.openstack.org/#/c/281693/
|
||||||
# is merged.
|
# is merged.
|
||||||
|
if operation == odl_const.ODL_CREATE:
|
||||||
sg = securitygroup.SecurityGroup(
|
sg = securitygroup.SecurityGroup(
|
||||||
id=res_id, name=context_[object_type]['name'],
|
id=res_id, name=context_[object_type]['name'],
|
||||||
tenant_id=context_[object_type]['tenant_id'],
|
tenant_id=context_[object_type]['tenant_id'],
|
||||||
description=context_[object_type]['description'])
|
description=context_[object_type]['description'])
|
||||||
plugin_context_mock.session.add(sg)
|
plugin_context_mock.session.add(sg)
|
||||||
context_[odl_const.ODL_SG].pop('id', None)
|
context_[odl_const.ODL_SG].pop('id', None)
|
||||||
|
if operation == odl_const.ODL_DELETE:
|
||||||
|
sg = mock.Mock()
|
||||||
|
sg.rules = []
|
||||||
|
self.mech.sync_from_callback_precommit(
|
||||||
|
plugin_context_mock, operation, res_type, res_id, context_,
|
||||||
|
security_group=sg)
|
||||||
|
else:
|
||||||
self.mech.sync_from_callback_precommit(
|
self.mech.sync_from_callback_precommit(
|
||||||
plugin_context_mock, operation, res_type, res_id, context_)
|
plugin_context_mock, operation, res_type, res_id, context_)
|
||||||
else:
|
else:
|
||||||
@@ -375,6 +384,10 @@ class OpenDaylightMechanismDriverTestCase(OpenDaylightConfigBase):
|
|||||||
else:
|
else:
|
||||||
url = '%s/%ss' % (cfg.CONF.ml2_odl.url, url_object_type)
|
url = '%s/%ss' % (cfg.CONF.ml2_odl.url, url_object_type)
|
||||||
|
|
||||||
|
if (object_type == odl_const.ODL_SG and
|
||||||
|
operation == odl_const.ODL_CREATE):
|
||||||
|
context = copy.deepcopy(context)
|
||||||
|
context[odl_const.ODL_SG].pop('rules')
|
||||||
if operation in [odl_const.ODL_CREATE, odl_const.ODL_UPDATE]:
|
if operation in [odl_const.ODL_CREATE, odl_const.ODL_UPDATE]:
|
||||||
kwargs = {
|
kwargs = {
|
||||||
'url': url,
|
'url': url,
|
||||||
@@ -594,6 +607,34 @@ class OpenDaylightMechanismDriverTestCase(OpenDaylightConfigBase):
|
|||||||
def test_sg_rule(self):
|
def test_sg_rule(self):
|
||||||
self._test_object_type(odl_const.ODL_SG_RULE)
|
self._test_object_type(odl_const.ODL_SG_RULE)
|
||||||
|
|
||||||
|
def test_sg_delete(self):
|
||||||
|
with mock.patch.object(journal, 'record') as record:
|
||||||
|
context = self._get_mock_operation_context(odl_const.ODL_SG)
|
||||||
|
res_id = context[odl_const.ODL_SG]['id']
|
||||||
|
plugin_context_mock = mock.Mock()
|
||||||
|
plugin_context_mock.session = neutron_db_api.get_session()
|
||||||
|
rule = mock.Mock()
|
||||||
|
rule.id = SG_RULE_FAKE_ID
|
||||||
|
rule.security_group_id = SG_FAKE_ID
|
||||||
|
sg = mock.Mock()
|
||||||
|
sg.id = SG_FAKE_ID
|
||||||
|
sg.rules = [rule]
|
||||||
|
kwargs = {'security_group': sg}
|
||||||
|
self.mech.sync_from_callback_precommit(
|
||||||
|
plugin_context_mock, odl_const.ODL_DELETE,
|
||||||
|
callback._RESOURCE_MAPPING[odl_const.ODL_SG],
|
||||||
|
res_id, context, **kwargs)
|
||||||
|
record.assert_has_calls(
|
||||||
|
[mock.call(mock.ANY, None, 'security_group', 'sg_fake_uuid',
|
||||||
|
'delete',
|
||||||
|
{'description': 'test-description',
|
||||||
|
'project_id': 'test-tenant',
|
||||||
|
'rules': [],
|
||||||
|
'tenant_id': 'test-tenant',
|
||||||
|
'id': 'sg_fake_uuid', 'name': 'test_sg'}),
|
||||||
|
mock.call(mock.ANY, None, 'security_group_rule',
|
||||||
|
'sg_rule_fake_uuid', 'delete', ['sg_fake_uuid'])])
|
||||||
|
|
||||||
def test_sync_multiple_updates(self):
|
def test_sync_multiple_updates(self):
|
||||||
# add 2 updates
|
# add 2 updates
|
||||||
for i in range(2):
|
for i in range(2):
|
||||||
|
|||||||
Reference in New Issue
Block a user