From: Salvatore Orlando <salv.orlando@gmail.com>
Date: Fri, 15 Nov 2013 11:06:19 +0000 (-0800)
Subject: NVP plugin:fix delete sec group when backend is out of sync
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=ed83265bb794d38f6f66d9e5182a9abc6b82ed53;p=openstack-build%2Fneutron-build.git

NVP plugin:fix delete sec group when backend is out of sync

If a security group does not exist on the NVP backend, an error
should not be raised on deletion of the security group.

This patch changes the plugin behavior by deleting the record
from the database and just logging that the security group
was not found on the NVP backend.

Closes-Bug: #1251422

Change-Id: Ib8adf7a830ff336655fd83ad4118cde641adf284
---

diff --git a/neutron/plugins/nicira/NeutronPlugin.py b/neutron/plugins/nicira/NeutronPlugin.py
index 26edc9b8c..edef16a79 100644
--- a/neutron/plugins/nicira/NeutronPlugin.py
+++ b/neutron/plugins/nicira/NeutronPlugin.py
@@ -2055,8 +2055,12 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin,
             if super(NvpPluginV2, self)._get_port_security_group_bindings(
                 context, filters):
                 raise ext_sg.SecurityGroupInUse(id=security_group['id'])
-            nvplib.delete_security_profile(self.cluster,
-                                           security_group['id'])
+            try:
+                nvplib.delete_security_profile(
+                    self.cluster, security_group['id'])
+            except q_exc.NotFound:
+                LOG.info(_("Security group: %s was already deleted "
+                           "from backend"), security_group_id)
             return super(NvpPluginV2, self).delete_security_group(
                 context, security_group_id)
 
diff --git a/neutron/plugins/nicira/nvplib.py b/neutron/plugins/nicira/nvplib.py
index e7e7c8ae7..eeec8167e 100644
--- a/neutron/plugins/nicira/nvplib.py
+++ b/neutron/plugins/nicira/nvplib.py
@@ -1142,13 +1142,13 @@ def update_security_group_rules(cluster, spid, rules):
 
 def delete_security_profile(cluster, spid):
     path = "/ws.v1/security-profile/%s" % spid
-
     try:
         do_request(HTTP_DELETE, path, cluster=cluster)
-    except exception.NotFound as e:
-        # FIXME(salv-orlando): should not raise NeutronException
-        LOG.error(format_exception("Unknown", e, locals()))
-        raise exception.NeutronException()
+    except exception.NotFound:
+        # This is not necessarily an error condition
+        LOG.warn(_("Unable to find security profile %s on NSX backend"),
+                 spid)
+        raise
 
 
 def _create_nat_match_obj(**kwargs):
diff --git a/neutron/tests/unit/nicira/test_nicira_plugin.py b/neutron/tests/unit/nicira/test_nicira_plugin.py
index 9cbfe658d..c006d27ff 100644
--- a/neutron/tests/unit/nicira/test_nicira_plugin.py
+++ b/neutron/tests/unit/nicira/test_nicira_plugin.py
@@ -486,6 +486,30 @@ class TestNiciraL3ExtensionManager(object):
         return []
 
 
+class TestNiciraL3SecGrpExtensionManager(TestNiciraL3ExtensionManager):
+    """A fake extension manager for L3 and Security Group extensions.
+
+    Includes also Nicira-specific L3 attributes.
+    """
+
+    def get_resources(self):
+        resources = super(TestNiciraL3SecGrpExtensionManager,
+                          self).get_resources()
+        resources.extend(secgrp.Securitygroup.get_resources())
+        return resources
+
+
+def backup_l3_attribute_map():
+    """Return a backup of the original l3 attribute map."""
+    return dict((res, attrs.copy()) for
+                (res, attrs) in l3.RESOURCE_ATTRIBUTE_MAP.iteritems())
+
+
+def restore_l3_attribute_map(map_to_restore):
+    """Ensure changes made by fake ext mgrs are reverted."""
+    l3.RESOURCE_ATTRIBUTE_MAP = map_to_restore
+
+
 class NiciraL3NatTest(test_l3_plugin.L3BaseForIntTests,
                       NiciraPluginV2TestCase):
 
@@ -498,7 +522,8 @@ class NiciraL3NatTest(test_l3_plugin.L3BaseForIntTests,
             self._l3_attribute_map_bk[item] = (
                 l3.RESOURCE_ATTRIBUTE_MAP[item].copy())
         cfg.CONF.set_override('api_extensions_path', NVPEXT_PATH)
-        self.addCleanup(self._restore_l3_attribute_map)
+        l3_attribute_map_bk = backup_l3_attribute_map()
+        self.addCleanup(restore_l3_attribute_map, l3_attribute_map_bk)
         ext_mgr = ext_mgr or TestNiciraL3ExtensionManager()
         super(NiciraL3NatTest, self).setUp(
             plugin=plugin, ext_mgr=ext_mgr, service_plugins=service_plugins)
@@ -1256,11 +1281,14 @@ class NiciraExtGwModeTestCase(NiciraPluginV2TestCase,
 
 
 class NiciraNeutronNVPOutOfSync(NiciraPluginV2TestCase,
-                                test_l3_plugin.L3NatTestCaseMixin):
+                                test_l3_plugin.L3NatTestCaseMixin,
+                                ext_sg.SecurityGroupsTestCase):
 
     def setUp(self):
-        ext_mgr = test_l3_plugin.L3TestExtensionManager()
-        super(NiciraNeutronNVPOutOfSync, self).setUp(ext_mgr=ext_mgr)
+        l3_attribute_map_bk = backup_l3_attribute_map()
+        self.addCleanup(restore_l3_attribute_map, l3_attribute_map_bk)
+        super(NiciraNeutronNVPOutOfSync, self).setUp(
+            ext_mgr=TestNiciraL3SecGrpExtensionManager())
 
     def test_delete_network_not_in_nvp(self):
         res = self._create_network('json', 'net1', True)
@@ -1414,6 +1442,16 @@ class NiciraNeutronNVPOutOfSync(NiciraPluginV2TestCase,
         self.assertEqual(router['router']['status'],
                          constants.NET_STATUS_ERROR)
 
+    def test_delete_security_group_not_in_nvp(self):
+        res = self._create_security_group('json', 'name', 'desc')
+        sec_group = self.deserialize('json', res)
+        self.fc._fake_securityprofile_dict.clear()
+        req = self.new_delete_request(
+            'security-groups',
+            sec_group['security_group']['id'])
+        res = req.get_response(self.ext_api)
+        self.assertEqual(res.status_int, 204)
+
 
 class TestNiciraNetworkGateway(NiciraPluginV2TestCase,
                                test_l2_gw.NetworkGatewayDbTestCase):