]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
NVP plugin:fix delete sec group when backend is out of sync
authorSalvatore Orlando <salv.orlando@gmail.com>
Fri, 15 Nov 2013 11:06:19 +0000 (03:06 -0800)
committerThomas Goirand <thomas@goirand.fr>
Thu, 13 Mar 2014 07:20:17 +0000 (15:20 +0800)
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
neutron/plugins/nicira/nvplib.py
neutron/tests/unit/nicira/test_nicira_plugin.py

index 32ee011eb66b23dbf939b96e79914d109e18996d..5fe118f80e52a500eda6cce837c1b765f87ba217 100644 (file)
@@ -2058,8 +2058,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)
 
index e7e7c8ae747b599ddfee0fa489a83b2fa3a47a7c..eeec8167ee39886846b40784680f95f7b055f063 100644 (file)
@@ -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):
index 9cbfe658d16e7affa63d5419541b85fc139090ca..c006d27ffcf9b40943544c2747090e2fcc0dbdcf 100644 (file)
@@ -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):