From 364501f574c075ed97810e7dff4478d66bc0c9a9 Mon Sep 17 00:00:00 2001
From: Miguel Angel Ajo <mangelajo@redhat.com>
Date: Thu, 1 Dec 2016 16:18:36 +0100
Subject: [PATCH] QoS: update the database before notifying the backend on
 delete

Previously, the QoS plugin was executing the DB delete operation
after notifying the backend about the operation. This leads
to situations where the backend is notified for deletion, and
then the policy can't be deleted because it's in use.

Correct order is, DB delete first, then notify the backend.

Change-Id: I357543832b9359bf169d05d079bd153f0ee591c4
Closes-Bug: #1646370
(cherry picked from commit 8fcba9dec801a0fb57ff29f97838958f083c8131)
---
 neutron/services/qos/qos_plugin.py            |  2 +-
 .../unit/services/qos/test_qos_plugin.py      | 73 +++++++++++++++++++
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/neutron/services/qos/qos_plugin.py b/neutron/services/qos/qos_plugin.py
index 3565ae96dcb..dc2ddc3b65f 100644
--- a/neutron/services/qos/qos_plugin.py
+++ b/neutron/services/qos/qos_plugin.py
@@ -89,8 +89,8 @@ class QoSPlugin(qos.QoSPluginBase):
         """
         policy = policy_object.QosPolicy(context)
         policy.id = policy_id
-        self.notification_driver_manager.delete_policy(context, policy)
         policy.delete()
+        self.notification_driver_manager.delete_policy(context, policy)
 
     def _get_policy_obj(self, context, policy_id):
         """Fetch a QoS policy.
diff --git a/neutron/tests/unit/services/qos/test_qos_plugin.py b/neutron/tests/unit/services/qos/test_qos_plugin.py
index 2aa996b4fd0..74299db1ee6 100644
--- a/neutron/tests/unit/services/qos/test_qos_plugin.py
+++ b/neutron/tests/unit/services/qos/test_qos_plugin.py
@@ -385,3 +385,76 @@ class TestQosPlugin(base.BaseQosTestCase):
             types = self.qos_plugin.get_rule_types(self.ctxt, filters=filters)
             self.assertEqual(sorted(qos_consts.VALID_RULE_TYPES),
                              sorted(type_['type'] for type_ in types))
+
+    @mock.patch('neutron.objects.qos.policy.QosPolicy')
+    def test_policy_notification_ordering(self, qos_policy_mock):
+
+        policy_actions = {'create': [self.ctxt, {'policy': {}}],
+                          'update': [self.ctxt, self.policy.id,
+                                     {'policy': {}}],
+                          'delete': [self.ctxt, self.policy.id]}
+
+        self.qos_plugin.notification_driver_manager = mock.Mock()
+
+        mock_manager = mock.Mock()
+        mock_manager.attach_mock(qos_policy_mock, 'QosPolicy')
+        mock_manager.attach_mock(self.qos_plugin.notification_driver_manager,
+                                 'notification_driver')
+
+        for action, arguments in policy_actions.items():
+            mock_manager.reset_mock()
+
+            method = getattr(self.qos_plugin, "%s_policy" % action)
+            method(*arguments)
+
+            policy_mock_call = getattr(mock.call.QosPolicy(), action)()
+            notify_mock_call = getattr(mock.call.notification_driver,
+                                       '%s_policy' % action)(self.ctxt,
+                                                             mock.ANY)
+
+            self.assertTrue(mock_manager.mock_calls.index(policy_mock_call) <
+                            mock_manager.mock_calls.index(notify_mock_call))
+
+    @mock.patch('neutron.objects.qos.policy.QosPolicy')
+    def test_rule_notification_ordering(self, qos_policy_mock):
+        rule_cls_mock = mock.Mock()
+        rule_cls_mock.rule_type = 'fake'
+
+        rule_actions = {'create': [self.ctxt, rule_cls_mock,
+                                   self.policy.id, {'fake_rule': {}}],
+                        'update': [self.ctxt, rule_cls_mock,
+                                   self.rule.id,
+                                   self.policy.id, {'fake_rule': {}}],
+                        'delete': [self.ctxt, rule_cls_mock,
+                                   self.rule.id, self.policy.id]}
+
+        self.qos_plugin.notification_driver_manager = mock.Mock()
+
+        mock_manager = mock.Mock()
+        mock_manager.attach_mock(qos_policy_mock, 'QosPolicy')
+        mock_manager.attach_mock(rule_cls_mock, 'RuleCls')
+        mock_manager.attach_mock(self.qos_plugin.notification_driver_manager,
+                                 'notification_driver')
+
+        for action, arguments in rule_actions.items():
+            mock_manager.reset_mock()
+            method = getattr(self.qos_plugin, "%s_policy_rule" % action)
+            method(*arguments)
+
+            # some actions get rule from policy
+            get_rule_mock_call = getattr(
+                mock.call.QosPolicy.get_object().get_rule_by_id(), action)()
+            # some actions construct rule from class reference
+            rule_mock_call = getattr(mock.call.RuleCls(), action)()
+
+            notify_mock_call = mock.call.notification_driver.update_policy(
+                    self.ctxt, mock.ANY)
+
+            if rule_mock_call in mock_manager.mock_calls:
+                action_index = mock_manager.mock_calls.index(rule_mock_call)
+            else:
+                action_index = mock_manager.mock_calls.index(
+                    get_rule_mock_call)
+
+            self.assertTrue(
+                action_index < mock_manager.mock_calls.index(notify_mock_call))