Dis-associate an auto-assigned floating IP should return proper warning

Added new exception class CannotDissociateAutoAssignedFloatingIP and raised
exception instead of return.

Fixes bug 1061499

Change-Id: I348573a235c6b81653837267072b5c48fa15b8af
(cherry picked from commit ce04b13)

nova/api/ec2/cloud.py

@@ -1196,6 +1196,9 @@ def disassociate_address(self, context, public_ip, **kwargs):
         except exception.FloatingIpNotAssociated:
             msg = _('Floating ip is not associated.')
             raise exception.EC2APIError(msg)
+        except exception.CannotDisassociateAutoAssignedFloatingIP:
+            msg = _('Cannot disassociate auto assigned floating ip')
+            raise exception.EC2APIError(msg)
 
         return {'return': "true"}
 

nova/api/openstack/compute/contrib/floating_ips.py

@@ -96,6 +96,9 @@ def disassociate_floating_ip(self, context, instance, address):
     except exception.FloatingIpNotAssociated:
         msg = _('Floating ip is not associated')
         raise webob.exc.HTTPBadRequest(explanation=msg)
+    except exception.CannotDisassociateAutoAssignedFloatingIP:
+        msg = _('Cannot disassociate auto assigned floating ip')
+        raise webob.exc.HTTPForbidden(explanation=msg)
 
 
 class FloatingIPController(object):

nova/exception.py

@@ -674,6 +674,10 @@ class NoFloatingIpInterface(NotFound):
     message = _("Interface %(interface)s not found.")
 
 
+class CannotDisassociateAutoAssignedFloatingIP(NovaException):
+    message = _("Cannot disassociate auto assigined floating ip")
+
+
 class KeypairNotFound(NotFound):
     message = _("Keypair %(name)s not found for user %(user_id)s")
 

nova/network/manager.py

@@ -589,7 +589,7 @@ def disassociate_floating_ip(self, context, address,
 
         # handle auto assigned
         if not affect_auto_assigned and floating_ip.get('auto_assigned'):
-            return
+            raise exception.CannotDisassociateAutoAssignedFloatingIP()
 
         # make sure project ownz this floating ip (allocated)
         self._floating_ip_owned_by_project(context, floating_ip)

nova/tests/api/ec2/test_cloud.py

@@ -290,6 +290,28 @@ def test_associate_disassociate_address(self):
         db.instance_destroy(self.context, inst['uuid'])
         db.floating_ip_destroy(self.context, address)
 
+    def test_disassociate_auto_assigned_address(self):
+        """Verifies disassociating auto assigned floating IP
+        raises an exception
+        """
+        address = "10.10.10.10"
+
+        def fake_get(*args, **kwargs):
+            pass
+
+        def fake_disassociate_floating_ip(*args, **kwargs):
+            raise exception.CannotDisassociateAutoAssignedFloatingIP()
+
+        self.stubs.Set(network_api.API, 'get_instance_id_by_floating_address',
+                       lambda *args: 1)
+        self.stubs.Set(self.cloud.compute_api, 'get', fake_get)
+        self.stubs.Set(network_api.API, 'disassociate_floating_ip',
+                                    fake_disassociate_floating_ip)
+
+        self.assertRaises(exception.EC2APIError,
+                          self.cloud.disassociate_address,
+                          self.context, public_ip=address)
+
     def test_describe_security_groups(self):
         """Makes sure describe_security_groups works and filters results."""
         sec = db.security_group_create(self.context,

nova/tests/api/openstack/compute/contrib/test_floating_ips.py

@@ -357,6 +357,31 @@ def get_instance_by_floating_ip_addr(self, context, address):
         rsp = self.manager._remove_floating_ip(req, 'test_inst', body)
         self.assertTrue(rsp.status_int == 404)
 
+    def test_floating_ip_disassociate_auto_assigned(self):
+        def fake_get_floating_ip_addr_auto_assigned(self, context, address):
+            return {'id': 1, 'address': '10.10.10.10', 'pool': 'nova',
+            'fixed_ip_id': 10, 'auto_assigned': 1}
+
+        def get_instance_by_floating_ip_addr(self, context, address):
+            if address == '10.10.10.10':
+                return 'test_inst'
+
+        def network_api_disassociate(self, context, instance,
+                                     floating_address):
+            raise exception.CannotDisassociateAutoAssignedFloatingIP()
+
+        self.stubs.Set(network.api.API, "get_floating_ip_by_address",
+                       fake_get_floating_ip_addr_auto_assigned)
+        self.stubs.Set(network.api.API, "get_instance_id_by_floating_address",
+                       get_instance_by_floating_ip_addr)
+        self.stubs.Set(network.api.API, "disassociate_floating_ip",
+                       network_api_disassociate)
+        body = dict(removeFloatingIp=dict(address='10.10.10.10'))
+        req = fakes.HTTPRequest.blank('/v2/fake/servers/test_inst/action')
+        self.assertRaises(webob.exc.HTTPForbidden,
+                          self.manager._remove_floating_ip,
+                          req, 'test_inst', body)
+
 # these are a few bad param tests
 
     def test_bad_address_param_in_remove_floating_ip(self):

nova/tests/network/test_manager.py

@@ -827,6 +827,14 @@ def fake6(*args, **kwargs):
         def fake7(*args, **kwargs):
             self.local = True
 
+        def fake8(*args, **kwargs):
+            return {'address': '10.0.0.1',
+                    'pool': 'nova',
+                    'interface': 'eth0',
+                    'fixed_ip_id': 1,
+                    'auto_assigned': True,
+                    'project_id': ctxt.project_id}
+
         self.stubs.Set(self.network, '_floating_ip_owned_by_project', fake1)
 
         # raises because floating_ip is not associated to a fixed_ip
@@ -854,6 +862,13 @@ def fake7(*args, **kwargs):
         self.network.disassociate_floating_ip(ctxt, mox.IgnoreArg())
         self.assertTrue(self.local)
 
+        # raises because auto_assigned floating IP cannot be disassociated
+        self.stubs.Set(self.network.db, 'floating_ip_get_by_address', fake8)
+        self.assertRaises(exception.CannotDisassociateAutoAssignedFloatingIP,
+                          self.network.disassociate_floating_ip,
+                          ctxt,
+                          mox.IgnoreArg())
+
     def test_add_fixed_ip_instance_without_vpn_requested_networks(self):
         self.mox.StubOutWithMock(db, 'network_get')
         self.mox.StubOutWithMock(db, 'fixed_ip_associate_pool')