--- /dev/null
+From 684c46437dbeb97bb71dde248d39db4dbebcd65e Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= <jcaamano@suse.com>
+Date: Tue, 23 Jan 2018 19:44:12 +0100
+Subject: [PATCH] Fix missing parentid on rule delete journal record
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+This fixes an error raised when deleting a security group if there are
+pending rules to be deleted of that group as records in the journal
+that were not triggered by the group deletion operation itself so that
+they did not have the parent security group id as data of the record.
+
+This happens when the rule and the group are deleted independently but
+in rapid succession. It does not happen when the rules are deleted as a
+consequence of deleting the group since the parentid is correctly set
+in that case, neither when the group is deleted some time after the
+rules when their records might have already been processed and removed.
+
+This problem was encountered in OPNFV functest environment but may also
+fix [1] and complement [2].
+
+[1] https://jira.opendaylight.org/browse/NETVIRT-896
+[2] https://review.openstack.org/#/c/536935
+
+Change-Id: I5ccdbdea9d803334c8ae9416b40e3c4b48a8144b
+Closes-Bug: 1744966
+Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>
+(cherry picked from commit 2c15a7926dfd24ee363a15c53ac44dedfae7e0ce)
+---
+ networking_odl/ml2/mech_driver_v2.py | 29 +++++---
+ .../unit/journal/test_dependency_validations.py | 85 ++++++++++++++++++++++
+ .../tests/unit/ml2/test_mechanism_odl_v2.py | 24 ++++++
+ 3 files changed, 127 insertions(+), 11 deletions(-)
+
+diff --git a/networking_odl/ml2/mech_driver_v2.py b/networking_odl/ml2/mech_driver_v2.py
+index 7843cb77f..54f3d472f 100644
+--- a/networking_odl/ml2/mech_driver_v2.py
++++ b/networking_odl/ml2/mech_driver_v2.py
+@@ -191,20 +191,27 @@ class OpenDaylightMechanismDriver(api.MechanismDriver):
+
+ object_uuid = (resource_dict.get('id')
+ if operation == 'create' else res_id)
+-
+- # 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_id in kwargs['security_group_rule_ids']:
+- journal.record(context, odl_const.ODL_SG_RULE,
+- rule_id, odl_const.ODL_DELETE, [object_uuid])
++ data = resource_dict
++
++ if (operation == odl_const.ODL_DELETE):
++ # 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):
++ for rule_id in kwargs['security_group_rule_ids']:
++ journal.record(context, odl_const.ODL_SG_RULE,
++ rule_id, odl_const.ODL_DELETE,
++ [object_uuid])
++ elif (object_type == odl_const.ODL_SG_RULE):
++ # Set the parent security group id so that dependencies
++ # to this security rule deletion can be properly found
++ # in the journal.
++ data = [kwargs['security_group_id']]
+
+ assert object_uuid is not None
+ journal.record(context, object_type, object_uuid,
+- operation, resource_dict)
++ operation, data)
+
+ def sync_from_callback_postcommit(self, context, operation, res_type,
+ res_id, resource_dict, **kwargs):
+diff --git a/networking_odl/tests/unit/journal/test_dependency_validations.py b/networking_odl/tests/unit/journal/test_dependency_validations.py
+index 11ac3fa4a..65babb72a 100644
+--- a/networking_odl/tests/unit/journal/test_dependency_validations.py
++++ b/networking_odl/tests/unit/journal/test_dependency_validations.py
+@@ -53,6 +53,11 @@ _TRUNK_DATA = {'trunk_id': _TRUNK_ID,
+ 'port_id': _PORT_ID,
+ 'sub_ports': [{'port_id': _SUBPORT_ID}]}
+ _BGPVPN_ID = 'BGPVPN_ID'
++_SG_ID = 'SG_ID'
++_SG_DATA = {'id': _SG_ID}
++_SG_RULE_ID = 'SG_RULE_ID'
++_SG_RULE_DATA = {'id': _SG_RULE_ID,
++ 'security_group_id': _SG_ID}
+
+
+ def get_data(res_type, operation):
+@@ -89,6 +94,12 @@ def get_data(res_type, operation):
+ return [{'id': _BGPVPN_ID, 'networks': networks,
+ 'routers': routers,
+ 'route_distinguishers': ['100:1']}]
++ elif res_type == const.ODL_SG:
++ return [_SG_DATA]
++ elif res_type == const.ODL_SG_RULE:
++ if operation == const.ODL_DELETE:
++ return [[_SG_RULE_ID]]
++ return [_SG_RULE_DATA]
+ return [[]]
+
+
+@@ -164,6 +175,80 @@ class SubnetDependencyValidationsTestCase(
+ )
+
+
++def security_rule_fail_security_group_dep(sg_op, sgr_op):
++ return {'expected': 1,
++ 'first_type': const.ODL_SG,
++ 'first_operation': sg_op,
++ 'first_id': _SG_ID,
++ 'second_type': const.ODL_SG_RULE,
++ 'second_operation': sgr_op,
++ 'second_id': _SG_RULE_ID}
++
++
++def security_rule_succeed_security_group_dep(sg_op, sgr_op):
++ return {'expected': 0,
++ 'first_type': const.ODL_SG_RULE,
++ 'first_operation': sgr_op,
++ 'first_id': _SG_RULE_ID,
++ 'second_type': const.ODL_SG,
++ 'second_operation': sg_op,
++ 'second_id': _SG_ID}
++
++
++class SecurityRuleDependencyValidationsTestCase(
++ test_base_db.ODLBaseDbTestCase, BaseDependencyValidationsTestCase):
++ scenarios = (
++ ("security_rule_create_depends_on_older_security_group_create",
++ security_rule_fail_security_group_dep(const.ODL_CREATE,
++ const.ODL_CREATE)),
++ ("security_rule_create_depends_on_older_security_group_update",
++ security_rule_fail_security_group_dep(const.ODL_UPDATE,
++ const.ODL_CREATE)),
++ ("security_rule_create_depends_on_older_security_group_delete",
++ security_rule_fail_security_group_dep(const.ODL_DELETE,
++ const.ODL_CREATE)),
++ ("security_rule_create_doesnt_depend_on_newer_security_group_create",
++ security_rule_succeed_security_group_dep(const.ODL_CREATE,
++ const.ODL_CREATE)),
++ ("security_rule_create_doesnt_depend_on_newer_security_group_update",
++ security_rule_succeed_security_group_dep(const.ODL_UPDATE,
++ const.ODL_CREATE)),
++ ("security_rule_create_doesnt_depend_on_newer_security_group_delete",
++ security_rule_succeed_security_group_dep(const.ODL_DELETE,
++ const.ODL_CREATE)),
++ ("security_rule_update_depends_on_older_security_group_create",
++ security_rule_fail_security_group_dep(const.ODL_CREATE,
++ const.ODL_UPDATE)),
++ ("security_rule_update_depends_on_older_security_group_update",
++ security_rule_fail_security_group_dep(const.ODL_UPDATE,
++ const.ODL_UPDATE)),
++ ("security_rule_update_depends_on_older_security_group_delete",
++ security_rule_fail_security_group_dep(const.ODL_DELETE,
++ const.ODL_UPDATE)),
++ ("security_rule_update_doesnt_depend_on_newer_security_group_create",
++ security_rule_succeed_security_group_dep(const.ODL_CREATE,
++ const.ODL_UPDATE)),
++ ("security_rule_update_doesnt_depend_on_newer_security_group_update",
++ security_rule_succeed_security_group_dep(const.ODL_UPDATE,
++ const.ODL_UPDATE)),
++ ("security_rule_update_doesnt_depend_on_newer_security_group_delete",
++ security_rule_succeed_security_group_dep(const.ODL_DELETE,
++ const.ODL_UPDATE)),
++ ("security_rule_delete_doesnt_depend_on_older_security_group_create",
++ security_rule_succeed_security_group_dep(const.ODL_CREATE,
++ const.ODL_DELETE)),
++ ("security_rule_delete_doesnt_depend_on_older_security_group_update",
++ security_rule_succeed_security_group_dep(const.ODL_UPDATE,
++ const.ODL_DELETE)),
++ ("security_rule_delete_doesnt_depend_on_newer_security_group_create",
++ security_rule_succeed_security_group_dep(const.ODL_CREATE,
++ const.ODL_DELETE)),
++ ("security_rule_delete_doesnt_depend_on_newer_security_group_update",
++ security_rule_succeed_security_group_dep(const.ODL_UPDATE,
++ const.ODL_DELETE)),
++ )
++
++
+ def port_fail_network_dep(net_op, port_op):
+ return {'expected': 1,
+ 'first_type': const.ODL_NETWORK,
+diff --git a/networking_odl/tests/unit/ml2/test_mechanism_odl_v2.py b/networking_odl/tests/unit/ml2/test_mechanism_odl_v2.py
+index 1efe2efd0..5ff51aaee 100644
+--- a/networking_odl/tests/unit/ml2/test_mechanism_odl_v2.py
++++ b/networking_odl/tests/unit/ml2/test_mechanism_odl_v2.py
+@@ -322,6 +322,12 @@ class OpenDaylightMechanismDriverTestCase(base_v2.OpenDaylightConfigBase):
+ security_group={'security_group_rules':
+ {'id': SG_RULE_FAKE_ID}},
+ security_group_rule_ids=[SG_RULE_FAKE_ID])
++ elif (object_type == odl_const.ODL_SG_RULE and
++ operation == odl_const.ODL_DELETE):
++ with self.db_session.begin(subtransactions=True):
++ self.mech.sync_from_callback_precommit(
++ plugin_context, operation, res_type, res_id,
++ context_, security_group_id=SG_FAKE_ID)
+ else:
+ with self.db_session.begin(subtransactions=True):
+ self.mech.sync_from_callback_precommit(
+@@ -624,6 +630,24 @@ class OpenDaylightMechanismDriverTestCase(base_v2.OpenDaylightConfigBase):
+ 'tenant_id': 'test-tenant',
+ 'id': SG_FAKE_ID, 'name': 'test_sg'})])
+
++ def test_sg_rule_delete(self):
++ with mock.patch.object(journal, 'record') as record:
++ context = self._get_mock_operation_context(odl_const.ODL_SG_RULE)
++ res_id = context[odl_const.ODL_SG_RULE]['id']
++ rule = mock.Mock()
++ rule.id = SG_RULE_FAKE_ID
++ rule.security_group_id = SG_FAKE_ID
++ kwargs = {'security_group_rule_id': SG_RULE_FAKE_ID,
++ 'security_group_id': SG_FAKE_ID}
++ with self.db_session.begin(subtransactions=True):
++ self.mech.sync_from_callback_precommit(
++ self.db_context, odl_const.ODL_DELETE,
++ callback._RESOURCE_MAPPING[odl_const.ODL_SG_RULE],
++ res_id, context, **kwargs)
++ record.assert_has_calls(
++ [mock.call(mock.ANY, 'security_group_rule',
++ SG_RULE_FAKE_ID, 'delete', [SG_FAKE_ID])])
++
+ def test_sync_multiple_updates(self):
+ # add 2 updates
+ for i in range(2):
+--
+2.14.3
+
Code Review / apex.git / commitdiff