Merge trunk

12 files changed, 114 insertions, 32 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 1f84fdd54..3fe463d69 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -39,6 +39,7 @@ from nova import flags
 from nova import ipv6
 from nova import log as logging
 from nova import network
+from nova import rpc
 from nova import utils
 from nova import volume
 from nova.api.ec2 import ec2utils
@@ -872,8 +873,14 @@ class CloudController(object):
 
     def allocate_address(self, context, **kwargs):
         LOG.audit(_("Allocate address"), context=context)
-        public_ip = self.network_api.allocate_floating_ip(context)
-        return {'publicIp': public_ip}
+        try:
+            public_ip = self.network_api.allocate_floating_ip(context)
+            return {'publicIp': public_ip}
+        except rpc.RemoteError as ex:
+            if ex.exc_type == 'NoMoreAddresses':
+                raise exception.NoMoreFloatingIps()
+            else:
+                raise
 
     def release_address(self, context, public_ip, **kwargs):
         LOG.audit(_("Release address %s"), public_ip, context=context)
diff --git a/nova/api/openstack/server_metadata.py b/nova/api/openstack/server_metadata.py
index b38b84a2a..57666f6b7 100644
--- a/nova/api/openstack/server_metadata.py
+++ b/nova/api/openstack/server_metadata.py
@@ -37,12 +37,18 @@ class Controller(object):
             meta_dict[key] = value
         return dict(metadata=meta_dict)
 
+    def _check_body(self, body):
+        if body == None or body == "":
+            expl = _('No Request Body')
+            raise exc.HTTPBadRequest(explanation=expl)
+
     def index(self, req, server_id):
         """ Returns the list of metadata for a given instance """
         context = req.environ['nova.context']
         return self._get_metadata(context, server_id)
 
     def create(self, req, server_id, body):
+        self._check_body(body)
         context = req.environ['nova.context']
         metadata = body.get('metadata')
         try:
@@ -51,9 +57,10 @@ class Controller(object):
                                                                 metadata)
         except quota.QuotaError as error:
             self._handle_quota_error(error)
-        return req.body
+        return body
 
     def update(self, req, server_id, id, body):
+        self._check_body(body)
         context = req.environ['nova.context']
         if not id in body:
             expl = _('Request body and URI mismatch')
@@ -68,7 +75,7 @@ class Controller(object):
         except quota.QuotaError as error:
             self._handle_quota_error(error)
 
-        return req.body
+        return body
 
     def show(self, req, server_id, id):
         """ Return a single metadata item """
diff --git a/nova/api/openstack/wsgi.py b/nova/api/openstack/wsgi.py
index ddf4e6fa9..6760735c4 100644
--- a/nova/api/openstack/wsgi.py
+++ b/nova/api/openstack/wsgi.py
@@ -225,7 +225,7 @@ class XMLDictSerializer(DictSerializer):
         if not xmlns and self.xmlns:
             node.setAttribute('xmlns', self.xmlns)
 
-        return node.toprettyxml(indent='    ')
+        return node.toprettyxml(indent='    ', encoding='utf-8')
 
     def _to_xml_node(self, doc, metadata, nodename, data):
         """Recursive method to convert data members to XML nodes."""
diff --git a/nova/exception.py b/nova/exception.py
index 69b3e0359..1571dd032 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -376,6 +376,10 @@ class NoFloatingIpsDefinedForInstance(NoFloatingIpsDefined):
     message = _("Zero floating ips defined for instance %(instance_id)s.")
 
 
+class NoMoreFloatingIps(NotFound):
+    message = _("Zero floating ips available.")
+
+
 class KeypairNotFound(NotFound):
     message = _("Keypair %(keypair_name)s not found for user %(user_id)s")
 
diff --git a/nova/flags.py b/nova/flags.py
index a8f16c6bb..acfcf8d68 100644
--- a/nova/flags.py
+++ b/nova/flags.py
@@ -270,8 +270,10 @@ DEFINE_list('region_list',
 DEFINE_string('connection_type', 'libvirt', 'libvirt, xenapi or fake')
 DEFINE_string('aws_access_key_id', 'admin', 'AWS Access ID')
 DEFINE_string('aws_secret_access_key', 'admin', 'AWS Access Key')
-DEFINE_integer('glance_port', 9292, 'glance port')
-DEFINE_string('glance_host', '$my_ip', 'glance host')
+# NOTE(sirp): my_ip interpolation doesn't work within nested structures
+DEFINE_list('glance_api_servers',
+            ['127.0.0.1:9292'],
+            'list of glance api servers available to nova (host:port)')
 DEFINE_integer('s3_port', 3333, 's3 port')
 DEFINE_string('s3_host', '$my_ip', 's3 host (for infrastructure)')
 DEFINE_string('s3_dmz', '$my_ip', 's3 dmz ip (for instances)')
diff --git a/nova/image/__init__.py b/nova/image/__init__.py
index 93d83df24..a27d649d4 100644
--- a/nova/image/__init__.py
+++ b/nova/image/__init__.py
@@ -22,6 +22,7 @@ import nova
 from nova import exception
 from nova import utils
 from nova import flags
+from nova.image import glance as glance_image_service
 
 FLAGS = flags.FLAGS
 
@@ -48,6 +49,8 @@ def get_default_image_service():
     return ImageService()
 
 
+# FIXME(sirp): perhaps this should be moved to nova/images/glance so that we
+# keep Glance specific code together for the most part
 def get_glance_client(image_href):
     """Get the correct glance client and id for the given image_href.
 
@@ -62,7 +65,9 @@ def get_glance_client(image_href):
     """
     image_href = image_href or 0
     if str(image_href).isdigit():
-        glance_client = GlanceClient(FLAGS.glance_host, FLAGS.glance_port)
+        glance_host, glance_port = \
+            glance_image_service.pick_glance_api_server()
+        glance_client = GlanceClient(glance_host, glance_port)
         return (glance_client, int(image_href))
 
     try:
diff --git a/nova/image/glance.py b/nova/image/glance.py
index 61308431d..6e058ab2f 100644
--- a/nova/image/glance.py
+++ b/nova/image/glance.py
@@ -20,6 +20,7 @@
 from __future__ import absolute_import
 
 import datetime
+import random
 
 from glance.common import exception as glance_exception
 
@@ -39,6 +40,21 @@ FLAGS = flags.FLAGS
 GlanceClient = utils.import_class('glance.client.Client')
 
 
+def pick_glance_api_server():
+    """Return which Glance API server to use for the request
+
+    This method provides a very primitive form of load-balancing suitable for
+    testing and sandbox environments. In production, it would be better to use
+    one IP and route that to a real load-balancer.
+
+        Returns (host, port)
+    """
+    host_port = random.choice(FLAGS.glance_api_servers)
+    host, port_str = host_port.split(':')
+    port = int(port_str)
+    return host, port
+
+
 class GlanceImageService(service.BaseImageService):
     """Provides storage and retrieval of disk image objects within Glance."""
 
@@ -51,12 +67,21 @@ class GlanceImageService(service.BaseImageService):
                           GLANCE_ONLY_ATTRS
 
     def __init__(self, client=None):
-        # FIXME(sirp): can we avoid dependency-injection here by using
-        # stubbing out a fake?
-        if client is None:
-            self.client = GlanceClient(FLAGS.glance_host, FLAGS.glance_port)
-        else:
-            self.client = client
+        self._client = client
+
+    def _get_client(self):
+        # NOTE(sirp): we want to load balance each request across glance
+        # servers. Since GlanceImageService is a long-lived object, `client`
+        # is made to choose a new server each time via this property.
+        if self._client is not None:
+            return self._client
+        glance_host, glance_port = pick_glance_api_server()
+        return GlanceClient(glance_host, glance_port)
+
+    def _set_client(self, client):
+        self._client = client
+
+    client = property(_get_client, _set_client)
 
     def index(self, context, filters=None, marker=None, limit=None):
         """Calls out to Glance for a list of images available."""
diff --git a/nova/tests/api/openstack/test_server_metadata.py b/nova/tests/api/openstack/test_server_metadata.py
index c4d1d4fd8..b583d40fe 100644
--- a/nova/tests/api/openstack/test_server_metadata.py
+++ b/nova/tests/api/openstack/test_server_metadata.py
@@ -89,6 +89,7 @@ class ServerMetaDataTest(unittest.TestCase):
         res = req.get_response(fakes.wsgi_app())
         res_dict = json.loads(res.body)
         self.assertEqual(200, res.status_int)
+        self.assertEqual('application/json', res.headers['Content-Type'])
         self.assertEqual('value1', res_dict['metadata']['key1'])
 
     def test_index_no_data(self):
@@ -99,6 +100,7 @@ class ServerMetaDataTest(unittest.TestCase):
         res = req.get_response(fakes.wsgi_app())
         res_dict = json.loads(res.body)
         self.assertEqual(200, res.status_int)
+        self.assertEqual('application/json', res.headers['Content-Type'])
         self.assertEqual(0, len(res_dict['metadata']))
 
     def test_show(self):
@@ -109,6 +111,7 @@ class ServerMetaDataTest(unittest.TestCase):
         res = req.get_response(fakes.wsgi_app())
         res_dict = json.loads(res.body)
         self.assertEqual(200, res.status_int)
+        self.assertEqual('application/json', res.headers['Content-Type'])
         self.assertEqual('value5', res_dict['key5'])
 
     def test_show_meta_not_found(self):
@@ -140,8 +143,19 @@ class ServerMetaDataTest(unittest.TestCase):
         res = req.get_response(fakes.wsgi_app())
         res_dict = json.loads(res.body)
         self.assertEqual(200, res.status_int)
+        self.assertEqual('application/json', res.headers['Content-Type'])
         self.assertEqual('value1', res_dict['metadata']['key1'])
 
+    def test_create_empty_body(self):
+        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+                       return_create_instance_metadata)
+        req = webob.Request.blank('/v1.1/servers/1/meta')
+        req.environ['api.version'] = '1.1'
+        req.method = 'POST'
+        req.headers["content-type"] = "application/json"
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(400, res.status_int)
+
     def test_update_item(self):
         self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
                        return_create_instance_metadata)
@@ -152,9 +166,20 @@ class ServerMetaDataTest(unittest.TestCase):
         req.headers["content-type"] = "application/json"
         res = req.get_response(fakes.wsgi_app())
         self.assertEqual(200, res.status_int)
+        self.assertEqual('application/json', res.headers['Content-Type'])
         res_dict = json.loads(res.body)
         self.assertEqual('value1', res_dict['key1'])
 
+    def test_update_item_empty_body(self):
+        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+                       return_create_instance_metadata)
+        req = webob.Request.blank('/v1.1/servers/1/meta/key1')
+        req.environ['api.version'] = '1.1'
+        req.method = 'PUT'
+        req.headers["content-type"] = "application/json"
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(400, res.status_int)
+
     def test_update_item_too_many_keys(self):
         self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
                        return_create_instance_metadata)
diff --git a/nova/tests/image/test_glance.py b/nova/tests/image/test_glance.py
index 041da1e13..223e7ae57 100644
--- a/nova/tests/image/test_glance.py
+++ b/nova/tests/image/test_glance.py
@@ -60,10 +60,8 @@ class BaseGlanceTest(unittest.TestCase):
     NOW_DATETIME = datetime.datetime(2010, 10, 11, 10, 30, 22)
 
     def setUp(self):
-        # FIXME(sirp): we can probably use stubs library here rather than
-        # dependency injection
         self.client = StubGlanceClient(None)
-        self.service = glance.GlanceImageService(self.client)
+        self.service = glance.GlanceImageService(client=self.client)
         self.context = context.RequestContext(None, None)
 
     def assertDateTimesFilled(self, image_meta):
diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py
index ba133c860..13046f861 100644
--- a/nova/tests/test_cloud.py
+++ b/nova/tests/test_cloud.py
@@ -115,6 +115,18 @@ class CloudTestCase(test.TestCase):
                                   public_ip=address)
         db.floating_ip_destroy(self.context, address)
 
+    def test_allocate_address(self):
+        address = "10.10.10.10"
+        allocate = self.cloud.allocate_address
+        db.floating_ip_create(self.context,
+                              {'address': address,
+                               'host': self.network.host})
+        self.assertEqual(allocate(self.context)['publicIp'], address)
+        db.floating_ip_destroy(self.context, address)
+        self.assertRaises(exception.NoMoreFloatingIps,
+                          allocate,
+                          self.context)
+
     def test_associate_disassociate_address(self):
         """Verifies associate runs cleanly without raising an exception"""
         address = "10.10.10.10"
diff --git a/nova/virt/images.py b/nova/virt/images.py
index de7ac61df..40bf6107c 100644
--- a/nova/virt/images.py
+++ b/nova/virt/images.py
@@ -23,6 +23,7 @@ Handling of VM disk images.
 
 from nova import context
 from nova import flags
+from nova.image import glance as glance_image_service
 import nova.image
 from nova import log as logging
 from nova import utils
@@ -42,13 +43,3 @@ def fetch(image_href, path, _user, _project):
         elevated = context.get_admin_context()
         metadata = image_service.get(elevated, image_id, image_file)
     return metadata
-
-
-# TODO(vish): xenapi should use the glance client code directly instead
-#             of retrieving the image using this method.
-def image_url(image):
-    if FLAGS.image_service == "nova.image.glance.GlanceImageService":
-        return "http://%s:%s/images/%s" % (FLAGS.glance_host,
-            FLAGS.glance_port, image)
-    return "http://%s:%s/_images/%s/image" % (FLAGS.s3_host, FLAGS.s3_port,
-                                              image)
diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index 98668e6ae..b9d4346e4 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -33,6 +33,7 @@ import glance.client
 from nova import exception
 from nova import flags
 import nova.image
+from nova.image import glance as glance_image_service
 from nova import log as logging
 from nova import utils
 from nova.auth.manager import AuthManager
@@ -358,10 +359,12 @@ class VMHelper(HelperBase):
 
         os_type = instance.os_type or FLAGS.default_os_type
 
+        glance_host, glance_port = \
+            glance_image_service.pick_glance_api_server()
         params = {'vdi_uuids': vdi_uuids,
                   'image_id': image_id,
-                  'glance_host': FLAGS.glance_host,
-                  'glance_port': FLAGS.glance_port,
+                  'glance_host': glance_host,
+                  'glance_port': glance_port,
                   'sr_path': cls.get_sr_path(session),
                   'os_type': os_type}
 
@@ -409,9 +412,11 @@ class VMHelper(HelperBase):
         # here (under Python 2.6+) and pass them as arguments
         uuid_stack = [str(uuid.uuid4()) for i in xrange(2)]
 
+        glance_host, glance_port = \
+            glance_image_service.pick_glance_api_server()
         params = {'image_id': image,
-                  'glance_host': FLAGS.glance_host,
-                  'glance_port': FLAGS.glance_port,
+                  'glance_host': glance_host,
+                  'glance_port': glance_port,
                   'uuid_stack': uuid_stack,
                   'sr_path': cls.get_sr_path(session)}
 
@@ -576,7 +581,8 @@ class VMHelper(HelperBase):
         Returns: A single filename if image_type is KERNEL_RAMDISK
                  A list of dictionaries that describe VDIs, otherwise
         """
-        url = images.image_url(image)
+        url = "http://%s:%s/_images/%s/image" % (FLAGS.s3_host, FLAGS.s3_port,
+                                                 image)
         LOG.debug(_("Asking xapi to fetch %(url)s as %(access)s") % locals())
         if image_type == ImageType.KERNEL_RAMDISK:
             fn = 'get_kernel'