incorporate feedback from brian waldon and brian lamar. Move associate/disassociate to server actions

2 files changed, 69 insertions, 57 deletions

diff --git a/nova/api/openstack/contrib/floating_ips.py b/nova/api/openstack/contrib/floating_ips.py
index 2f5fdd001..b305ebdcb 100644
--- a/nova/api/openstack/contrib/floating_ips.py
+++ b/nova/api/openstack/contrib/floating_ips.py
@@ -15,8 +15,9 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License
-from webob import exc
+import webob
 
+from nova import compute
 from nova import exception
 from nova import log as logging
 from nova import network
@@ -71,7 +72,7 @@ class FloatingIPController(object):
         try:
             floating_ip = self.network_api.get_floating_ip(context, id)
         except exception.NotFound:
-            return faults.Fault(exc.HTTPNotFound())
+            return faults.Fault(webob.exc.HTTPNotFound())
 
         return _translate_floating_ip_view(floating_ip)
 
@@ -110,40 +111,49 @@ class FloatingIPController(object):
 
         self.network_api.release_floating_ip(context,
                                              address=floating_ip['address'])
-        return exc.HTTPAccepted()
+        return webob.exc.HTTPAccepted()
 
-    def associate(self, req, id, body):
-        """PUT /floating_ips/{id}/associate fixed ip in body """
+    def _get_ip_by_id(self, context, value):
+        """Checks that value is id and then returns its address."""
+        return self.network_api.get_floating_ip(context, value)['address']
+
+
+class Floating_ips(extensions.ExtensionDescriptor):
+    def __init__(self):
+        self.compute_api = compute.API()
+        self.network_api = network.API()
+        super(Floating_ips, self).__init__()
+
+    def _add_floating_ip(self, input_dict, req, instance_id):
+        """Associate floating_ip to an instance."""
         context = req.environ['nova.context']
-        floating_ip = self._get_ip_by_id(context, id)
 
-        fixed_ip = body['floating_ip']['fixed_ip']
+        try:
+            address = input_dict['addFloatingIp']['address']
+        except KeyError:
+            msg = _("Address not specified")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
 
-        self.network_api.associate_floating_ip(context,
-                                               floating_ip, fixed_ip)
+        self.compute_api.associate_floating_ip(context, instance_id, address)
 
-        floating_ip = self.network_api.get_floating_ip(context, id)
-        return _translate_floating_ip_view(floating_ip)
+        return webob.Response(status_int=202)
 
-    def disassociate(self, req, id, body=None):
-        """PUT /floating_ips/{id}/disassociate """
+    def _remove_floating_ip(self, input_dict, req, instance_id):
+        """Dissociate floating_ip from an instance."""
         context = req.environ['nova.context']
-        floating_ip = self.network_api.get_floating_ip(context, id)
-        address = floating_ip['address']
 
-        # no-op if this ip is already disassociated
+        try:
+            address = input_dict['removeFloatingIp']['address']
+        except KeyError:
+            msg = _("Address not specified")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+
+        floating_ip = self.network_api.get_floating_ip_by_ip(context, address)
         if 'fixed_ip' in floating_ip:
             self.network_api.disassociate_floating_ip(context, address)
-            floating_ip = self.network_api.get_floating_ip(context, id)
-
-        return _translate_floating_ip_view(floating_ip)
-
-    def _get_ip_by_id(self, context, value):
-        """Checks that value is id and then returns its address."""
-        return self.network_api.get_floating_ip(context, value)['address']
 
+        return webob.Response(status_int=202)
 
-class Floating_ips(extensions.ExtensionDescriptor):
     def get_name(self):
         return "Floating_ips"
 
@@ -170,3 +180,14 @@ class Floating_ips(extensions.ExtensionDescriptor):
         resources.append(res)
 
         return resources
+
+    def get_actions(self):
+        """Return the actions the extension adds, as required by contract."""
+        actions = [
+                extensions.ActionExtension("servers", "addFloatingIp",
+                                            self._add_floating_ip),
+                extensions.ActionExtension("servers", "removeFloatingIp",
+                                            self._remove_floating_ip),
+        ]
+
+        return actions
diff --git a/nova/tests/api/openstack/contrib/test_floating_ips.py b/nova/tests/api/openstack/contrib/test_floating_ips.py
index e506519f4..09234072a 100644
--- a/nova/tests/api/openstack/contrib/test_floating_ips.py
+++ b/nova/tests/api/openstack/contrib/test_floating_ips.py
@@ -17,6 +17,7 @@ import json
 import stubout
 import webob
 
+from nova import compute
 from nova import context
 from nova import db
 from nova import test
@@ -30,6 +31,11 @@ from nova.api.openstack.contrib.floating_ips import _translate_floating_ip_view
 
 def network_api_get_floating_ip(self, context, id):
     return {'id': 1, 'address': '10.10.10.10',
+            'fixed_ip': None}
+
+
+def network_api_get_floating_ip_by_ip(self, context, address):
+    return {'id': 1, 'address': '10.10.10.10',
             'fixed_ip': {'address': '11.0.0.1'}}
 
 
@@ -50,7 +56,7 @@ def network_api_release(self, context, address):
     pass
 
 
-def network_api_associate(self, context, floating_ip, fixed_ip):
+def compute_api_associate(self, context, instance_id, floating_ip):
     pass
 
 
@@ -78,14 +84,16 @@ class FloatingIpTest(test.TestCase):
         fakes.stub_out_rate_limiting(self.stubs)
         self.stubs.Set(network.api.API, "get_floating_ip",
                        network_api_get_floating_ip)
+        self.stubs.Set(network.api.API, "get_floating_ip_by_ip",
+                       network_api_get_floating_ip)
         self.stubs.Set(network.api.API, "list_floating_ips",
                        network_api_list_floating_ips)
         self.stubs.Set(network.api.API, "allocate_floating_ip",
                        network_api_allocate)
         self.stubs.Set(network.api.API, "release_floating_ip",
                        network_api_release)
-        self.stubs.Set(network.api.API, "associate_floating_ip",
-                       network_api_associate)
+        self.stubs.Set(compute.api.API, "associate_floating_ip",
+                       compute_api_associate)
         self.stubs.Set(network.api.API, "disassociate_floating_ip",
                        network_api_disassociate)
         self.context = context.get_admin_context()
@@ -133,7 +141,6 @@ class FloatingIpTest(test.TestCase):
         res_dict = json.loads(res.body)
         self.assertEqual(res_dict['floating_ip']['id'], 1)
         self.assertEqual(res_dict['floating_ip']['ip'], '10.10.10.10')
-        self.assertEqual(res_dict['floating_ip']['fixed_ip'], '11.0.0.1')
         self.assertEqual(res_dict['floating_ip']['instance_id'], None)
 
     def test_floating_ip_allocate(self):
@@ -141,7 +148,6 @@ class FloatingIpTest(test.TestCase):
         req.method = 'POST'
         req.headers['Content-Type'] = 'application/json'
         res = req.get_response(fakes.wsgi_app())
-        print res
         self.assertEqual(res.status_int, 200)
         ip = json.loads(res.body)['floating_ip']
 
@@ -158,37 +164,22 @@ class FloatingIpTest(test.TestCase):
         res = req.get_response(fakes.wsgi_app())
         self.assertEqual(res.status_int, 202)
 
-    def test_floating_ip_associate(self):
-        body = dict(floating_ip=dict(fixed_ip='11.0.0.1'))
-        req = webob.Request.blank('/v1.1/os-floating-ips/1/associate')
-        req.method = 'PUT'
+    def test_add_floating_ip_to_instance(self):
+        body = dict(addFloatingIp=dict(address='11.0.0.1'))
+        req = webob.Request.blank('/v1.1/servers/test_inst/action')
+        req.method = "POST"
         req.body = json.dumps(body)
         req.headers["content-type"] = "application/json"
 
-        res = req.get_response(fakes.wsgi_app())
-        self.assertEqual(res.status_int, 200)
-        actual = json.loads(res.body)['floating_ip']
-
-        expected = {
-            "id": 1,
-            "instance_id": None,
-            "ip": "10.10.10.10",
-            "fixed_ip": "11.0.0.1"}
-        self.assertEqual(actual, expected)
+        resp = req.get_response(fakes.wsgi_app())
+        self.assertEqual(resp.status_int, 202)
 
-    def test_floating_ip_disassociate(self):
-        body = dict()
-        req = webob.Request.blank('/v1.1/os-floating-ips/1/disassociate')
-        req.method = 'PUT'
+    def test_remove_floating_ip_from_instance(self):
+        body = dict(removeFloatingIp=dict(address='11.0.0.1'))
+        req = webob.Request.blank('/v1.1/servers/test_inst/action')
+        req.method = "POST"
         req.body = json.dumps(body)
-        req.headers['Content-Type'] = 'application/json'
-        res = req.get_response(fakes.wsgi_app())
-        self.assertEqual(res.status_int, 200)
-        ip = json.loads(res.body)['floating_ip']
-        expected = {
-            "id": 1,
-            "instance_id": None,
-            "ip": '10.10.10.10',
-            "fixed_ip": '11.0.0.1'}
+        req.headers["content-type"] = "application/json"
 
-        self.assertEqual(ip, expected)
+        resp = req.get_response(fakes.wsgi_app())
+        self.assertEqual(resp.status_int, 202)