From 6a64fc7eb990a88ae40e98c615ecc3f6c97e1470 Mon Sep 17 00:00:00 2001 From: Tim Rozet Date: Fri, 6 Apr 2018 15:40:34 -0400 Subject: Fixes deleting security group in SNAPS for ODL scenarios Sometimes API check was failing for ODL scenarios on security group deletes. This patch backports a fix from upstream to fix the issue: https://review.openstack.org/#/c/538352/4 JIRA: APEX-586 Change-Id: I36a3547228cdcb1883140f17de7b5a713298d366 Signed-off-by: Tim Rozet --- build/overcloud-opendaylight.sh | 2 + build/patches/networking-odl-sg-fix.patch | 233 ++++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+) create mode 100644 build/patches/networking-odl-sg-fix.patch diff --git a/build/overcloud-opendaylight.sh b/build/overcloud-opendaylight.sh index 9e85859d..3d1fe85d 100755 --- a/build/overcloud-opendaylight.sh +++ b/build/overcloud-opendaylight.sh @@ -79,6 +79,8 @@ LIBGUESTFS_BACKEND=direct $VIRT_CUSTOMIZE \ --install capnproto-libs,capnproto \ --upload ${BUILD_ROOT}/patches/neutron-patch-NSDriver.patch:/usr/lib/python2.7/site-packages/ \ --upload ${CACHE_DIR}/opendaylight-7.0.0-0.1.20170531snap665.el7.noarch.rpm:/root/ \ + --upload ${BUILD_ROOT}/patches/networking-odl-sg-fix.patch:/usr/lib/python2.7/site-packages/ \ + --run-command "cd /usr/lib/python2.7/site-packages/ && patch -p1 < networking-odl-sg-fix.patch" \ -a overcloud-full-opendaylight_build.qcow2 # Arch dependent on x86 diff --git a/build/patches/networking-odl-sg-fix.patch b/build/patches/networking-odl-sg-fix.patch new file mode 100644 index 00000000..9ea2e04f --- /dev/null +++ b/build/patches/networking-odl-sg-fix.patch @@ -0,0 +1,233 @@ +From 684c46437dbeb97bb71dde248d39db4dbebcd65e Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Jaime=20Caama=C3=B1o=20Ruiz?= +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 +(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 + -- cgit 1.2.3-korg