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 <trozet@redhat.com>

2 files changed, 235 insertions, 0 deletions

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?= <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
+