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

7 files changed, 73 insertions, 1 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index aa7e880df..6cbaf2309 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -1196,6 +1196,9 @@ class CloudController(object):
         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"}
 
diff --git a/nova/api/openstack/compute/contrib/floating_ips.py b/nova/api/openstack/compute/contrib/floating_ips.py
index b97f200da..56a6a8fad 100644
--- a/nova/api/openstack/compute/contrib/floating_ips.py
+++ b/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):
diff --git a/nova/exception.py b/nova/exception.py
index 4261ad3ab..8983103dd 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -678,6 +678,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")
 
diff --git a/nova/network/manager.py b/nova/network/manager.py
index 36cc45379..02cdfdd3a 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -580,7 +580,7 @@ class FloatingIP(object):
 
         # handle auto assigned
         if not affect_auto_assigned and floating_ip.get('auto_assigned'):
-            return
+            raise exception.CannotDisassociateAutoAssignedFloatingIP()
 
         # make sure project owns this floating ip (allocated)
         self._floating_ip_owned_by_project(context, floating_ip)
diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py
index 29f68328e..d86897dc1 100644
--- a/nova/tests/api/ec2/test_cloud.py
+++ b/nova/tests/api/ec2/test_cloud.py
@@ -286,6 +286,28 @@ class CloudTestCase(test.TestCase):
         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,
diff --git a/nova/tests/api/openstack/compute/contrib/test_floating_ips.py b/nova/tests/api/openstack/compute/contrib/test_floating_ips.py
index 72927252d..96c56d3be 100644
--- a/nova/tests/api/openstack/compute/contrib/test_floating_ips.py
+++ b/nova/tests/api/openstack/compute/contrib/test_floating_ips.py
@@ -429,6 +429,31 @@ class FloatingIpTest(test.TestCase):
         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):
diff --git a/nova/tests/network/test_manager.py b/nova/tests/network/test_manager.py
index b39d1b5f0..1ade8bacc 100644
--- a/nova/tests/network/test_manager.py
+++ b/nova/tests/network/test_manager.py
@@ -826,6 +826,14 @@ class VlanNetworkTestCase(test.TestCase):
         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
@@ -853,6 +861,13 @@ class VlanNetworkTestCase(test.TestCase):
         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')