1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
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
|
