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

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,
+            try:
-                                           security_group['id'])
+                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)
 

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:
+    except exception.NotFound:
-        # FIXME(salv-orlando): should not raise NeutronException
+        # This is not necessarily an error condition
-        LOG.error(format_exception("Unknown", e, locals()))
+        LOG.warn(_("Unable to find security profile %s on NSX backend"),
-        raise exception.NeutronException()
+                 spid)
+        raise
 
 
 def _create_nat_match_obj(**kwargs):

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()
+        l3_attribute_map_bk = backup_l3_attribute_map()
-        super(NiciraNeutronNVPOutOfSync, self).setUp(ext_mgr=ext_mgr)
+        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):