Rebased to trunk rev 1027, and resolved a conflict in nova/virt/libvirt_conn.py

7 files changed, 84 insertions, 49 deletions

diff --git a/nova/api/ec2/admin.py b/nova/api/ec2/admin.py
index 6a5609d4a..ea94d9c1f 100644
--- a/nova/api/ec2/admin.py
+++ b/nova/api/ec2/admin.py
@@ -266,7 +266,7 @@ class AdminController(object):
     def _vpn_for(self, context, project_id):
         """Get the VPN instance for a project ID."""
         for instance in db.instance_get_all_by_project(context, project_id):
-            if (instance['image_id'] == FLAGS.vpn_image_id
+            if (instance['image_id'] == str(FLAGS.vpn_image_id)
                 and not instance['state_description'] in
                     ['shutting_down', 'shutdown']):
                 return instance
diff --git a/nova/api/ec2/apirequest.py b/nova/api/ec2/apirequest.py
index d7ad08d2f..6672e60bb 100644
--- a/nova/api/ec2/apirequest.py
+++ b/nova/api/ec2/apirequest.py
@@ -196,7 +196,7 @@ class APIRequest(object):
         elif isinstance(data, datetime.datetime):
             data_el.appendChild(
                   xml.createTextNode(_database_to_isoformat(data)))
-        elif data != None:
+        elif data is not None:
             data_el.appendChild(xml.createTextNode(str(data)))
 
         return data_el
diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 10b1d0ac5..9f4c0c05e 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -159,7 +159,7 @@ class CloudController(object):
         floating_ip = db.instance_get_floating_address(ctxt,
                                                        instance_ref['id'])
         ec2_id = ec2utils.id_to_ec2_id(instance_ref['id'])
-        image_ec2_id = self._image_ec2_id(instance_ref['image_id'], 'ami')
+        image_ec2_id = self.image_ec2_id(instance_ref['image_id'])
         data = {
             'user-data': base64.b64decode(instance_ref['user_data']),
             'meta-data': {
@@ -187,9 +187,9 @@ class CloudController(object):
                 'mpi': mpi}}
 
         for image_type in ['kernel', 'ramdisk']:
-            if '%s_id' % image_type in instance_ref:
-                ec2_id = self._image_ec2_id(instance_ref['%s_id' % image_type],
-                                            self._image_type(image_type))
+            if instance_ref.get('%s_id' % image_type):
+                ec2_id = self.image_ec2_id(instance_ref['%s_id' % image_type],
+                                           self._image_type(image_type))
                 data['meta-data']['%s-id' % image_type] = ec2_id
 
         if False:  # TODO(vish): store ancestor ids
@@ -442,7 +442,7 @@ class CloudController(object):
                                                        group_name)
 
         criteria = self._revoke_rule_args_to_dict(context, **kwargs)
-        if criteria == None:
+        if criteria is None:
             raise exception.ApiError(_("Not enough parameters to build a "
                                        "valid rule."))
 
@@ -613,7 +613,7 @@ class CloudController(object):
         # TODO(vish): Instance should be None at db layer instead of
         #             trying to lazy load, but for now we turn it into
         #             a dict to avoid an error.
-        return {'volumeSet': [self._format_volume(context, dict(volume))]}
+        return self._format_volume(context, dict(volume))
 
     def delete_volume(self, context, volume_id, **kwargs):
         volume_id = ec2utils.ec2_id_to_id(volume_id)
@@ -664,7 +664,7 @@ class CloudController(object):
                 'volumeId': ec2utils.id_to_ec2_id(volume_id, 'vol-%08x')}
 
     def _convert_to_set(self, lst, label):
-        if lst == None or lst == []:
+        if lst is None or lst == []:
             return None
         if not isinstance(lst, list):
             lst = [lst]
@@ -703,13 +703,13 @@ class CloudController(object):
             instances = self.compute_api.get_all(context, **kwargs)
         for instance in instances:
             if not context.is_admin:
-                if instance['image_id'] == FLAGS.vpn_image_id:
+                if instance['image_id'] == str(FLAGS.vpn_image_id):
                     continue
             i = {}
             instance_id = instance['id']
             ec2_id = ec2utils.id_to_ec2_id(instance_id)
             i['instanceId'] = ec2_id
-            i['imageId'] = self._image_ec2_id(instance['image_id'])
+            i['imageId'] = self.image_ec2_id(instance['image_id'])
             i['instanceState'] = {
                 'code': instance['state'],
                 'name': instance['state_description']}
@@ -726,7 +726,9 @@ class CloudController(object):
                         instance['mac_address'])
 
             i['privateDnsName'] = fixed_addr
+            i['privateIpAddress'] = fixed_addr
             i['publicDnsName'] = floating_addr
+            i['ipAddress'] = floating_addr or fixed_addr
             i['dnsName'] = i['publicDnsName'] or i['privateDnsName']
             i['keyName'] = instance['key_name']
 
@@ -898,7 +900,7 @@ class CloudController(object):
         return image_type
 
     @staticmethod
-    def _image_ec2_id(image_id, image_type='ami'):
+    def image_ec2_id(image_id, image_type='ami'):
         """Returns image ec2_id using id and three letter type."""
         template = image_type + '-%08x'
         return ec2utils.id_to_ec2_id(int(image_id), template=template)
@@ -917,15 +919,15 @@ class CloudController(object):
         """Convert from format defined by BaseImageService to S3 format."""
         i = {}
         image_type = self._image_type(image.get('container_format'))
-        ec2_id = self._image_ec2_id(image.get('id'), image_type)
+        ec2_id = self.image_ec2_id(image.get('id'), image_type)
         name = image.get('name')
         i['imageId'] = ec2_id
         kernel_id = image['properties'].get('kernel_id')
         if kernel_id:
-            i['kernelId'] = self._image_ec2_id(kernel_id, 'aki')
+            i['kernelId'] = self.image_ec2_id(kernel_id, 'aki')
         ramdisk_id = image['properties'].get('ramdisk_id')
         if ramdisk_id:
-            i['ramdiskId'] = self._image_ec2_id(ramdisk_id, 'ari')
+            i['ramdiskId'] = self.image_ec2_id(ramdisk_id, 'ari')
         i['imageOwnerId'] = image['properties'].get('owner_id')
         if name:
             i['imageLocation'] = "%s (%s)" % (image['properties'].
@@ -976,8 +978,8 @@ class CloudController(object):
         metadata = {'properties': {'image_location': image_location}}
         image = self.image_service.create(context, metadata)
         image_type = self._image_type(image.get('container_format'))
-        image_id = self._image_ec2_id(image['id'],
-                                      image_type)
+        image_id = self.image_ec2_id(image['id'],
+                                     image_type)
         msg = _("Registered image %(image_location)s with"
                 " id %(image_id)s") % locals()
         LOG.audit(msg, context=context)
diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py
index 234f921ab..0b6dc944a 100644
--- a/nova/api/openstack/common.py
+++ b/nova/api/openstack/common.py
@@ -25,7 +25,7 @@ from nova import log as logging
 from nova import wsgi
 
 
-LOG = logging.getLogger('common')
+LOG = logging.getLogger('nova.api.openstack.common')
 
 
 FLAGS = flags.FLAGS
@@ -116,8 +116,14 @@ def get_image_id_from_image_hash(image_service, context, image_hash):
         items = image_service.index(context)
     for image in items:
         image_id = image['id']
-        if abs(hash(image_id)) == int(image_hash):
-            return image_id
+        try:
+            if abs(hash(image_id)) == int(image_hash):
+                return image_id
+        except ValueError:
+            msg = _("Requested image_id has wrong format: %s,"
+                    "should have numerical format") % image_id
+            LOG.error(msg)
+            raise Exception(msg)
     raise exception.NotFound(image_hash)
 
 
diff --git a/nova/api/openstack/contrib/volumes.py b/nova/api/openstack/contrib/volumes.py
index 6efacce52..18de2ec71 100644
--- a/nova/api/openstack/contrib/volumes.py
+++ b/nova/api/openstack/contrib/volumes.py
@@ -322,8 +322,7 @@ class Volumes(extensions.ExtensionDescriptor):
         # Does this matter?
         res = extensions.ResourceExtension('volumes',
                                         VolumeController(),
-                                        collection_actions={'detail': 'GET'}
-                                          )
+                                        collection_actions={'detail': 'GET'})
         resources.append(res)
 
         res = extensions.ResourceExtension('volume_attachments',
diff --git a/nova/api/openstack/server_metadata.py b/nova/api/openstack/server_metadata.py
index 5c1390b9c..fd64ee4fb 100644
--- a/nova/api/openstack/server_metadata.py
+++ b/nova/api/openstack/server_metadata.py
@@ -18,6 +18,7 @@
 from webob import exc
 
 from nova import compute
+from nova import quota
 from nova import wsgi
 from nova.api.openstack import common
 from nova.api.openstack import faults
@@ -44,10 +45,14 @@ class Controller(common.OpenstackController):
 
     def create(self, req, server_id):
         context = req.environ['nova.context']
-        body = self._deserialize(req.body, req.get_content_type())
-        self.compute_api.update_or_create_instance_metadata(context,
-                                                            server_id,
-                                                            body['metadata'])
+        data = self._deserialize(req.body, req.get_content_type())
+        metadata = data.get('metadata')
+        try:
+            self.compute_api.update_or_create_instance_metadata(context,
+                                                                server_id,
+                                                                metadata)
+        except quota.QuotaError as error:
+            self._handle_quota_error(error)
         return req.body
 
     def update(self, req, server_id, id):
@@ -59,9 +64,13 @@ class Controller(common.OpenstackController):
         if len(body) > 1:
             expl = _('Request body contains too many items')
             raise exc.HTTPBadRequest(explanation=expl)
-        self.compute_api.update_or_create_instance_metadata(context,
-                                                            server_id,
-                                                            body)
+        try:
+            self.compute_api.update_or_create_instance_metadata(context,
+                                                                server_id,
+                                                                body)
+        except quota.QuotaError as error:
+            self._handle_quota_error(error)
+
         return req.body
 
     def show(self, req, server_id, id):
@@ -77,3 +86,9 @@ class Controller(common.OpenstackController):
         """ Deletes an existing metadata """
         context = req.environ['nova.context']
         self.compute_api.delete_instance_metadata(context, server_id, id)
+
+    def _handle_quota_error(self, error):
+        """Reraise quota errors as api-specific http exceptions."""
+        if error.code == "MetadataLimitExceeded":
+            raise exc.HTTPBadRequest(explanation=error.message)
+        raise error
diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py
index 43e0c7963..415c0995f 100644
--- a/nova/api/openstack/servers.py
+++ b/nova/api/openstack/servers.py
@@ -40,7 +40,7 @@ import nova.api.openstack
 from nova.scheduler import api as scheduler_api
 
 
-LOG = logging.getLogger('server')
+LOG = logging.getLogger('nova.api.openstack.servers')
 FLAGS = flags.FLAGS
 
 
@@ -118,6 +118,8 @@ class Controller(common.OpenstackController):
 
         context = req.environ['nova.context']
 
+        password = self._get_server_admin_password(env['server'])
+
         key_name = None
         key_data = None
         key_pairs = auth_manager.AuthManager.get_key_pairs(context)
@@ -127,21 +129,16 @@ class Controller(common.OpenstackController):
             key_data = key_pair['public_key']
 
         requested_image_id = self._image_id_from_req_data(env)
-        image_id = common.get_image_id_from_image_hash(self._image_service,
-            context, requested_image_id)
+        try:
+            image_id = common.get_image_id_from_image_hash(self._image_service,
+                context, requested_image_id)
+        except:
+            msg = _("Can not find requested image")
+            return faults.Fault(exc.HTTPBadRequest(msg))
+
         kernel_id, ramdisk_id = self._get_kernel_ramdisk_from_image(
             req, image_id)
 
-        # Metadata is a list, not a Dictionary, because we allow duplicate keys
-        # (even though JSON can't encode this)
-        # In future, we may not allow duplicate keys.
-        # However, the CloudServers API is not definitive on this front,
-        #  and we want to be compatible.
-        metadata = []
-        if env['server'].get('metadata'):
-            for k, v in env['server']['metadata'].items():
-                metadata.append({'key': k, 'value': v})
-
         personality = env['server'].get('personality')
         injected_files = []
         if personality:
@@ -170,7 +167,7 @@ class Controller(common.OpenstackController):
                 display_description=name,
                 key_name=key_name,
                 key_data=key_data,
-                metadata=metadata,
+                metadata=env['server'].get('metadata', {}),
                 injected_files=injected_files)
         except quota.QuotaError as error:
             self._handle_quota_error(error)
@@ -180,7 +177,6 @@ class Controller(common.OpenstackController):
 
         builder = self._get_view_builder(req)
         server = builder.build(inst, is_detail=True)
-        password = utils.generate_password(16)
         server['server']['adminPass'] = password
         self.compute_api.set_admin_password(context, server['server']['id'],
                                             password)
@@ -242,6 +238,10 @@ class Controller(common.OpenstackController):
         # if the original error is okay, just reraise it
         raise error
 
+    def _get_server_admin_password(self, server):
+        """ Determine the admin password for a server on creation """
+        return utils.generate_password(16)
+
     @scheduler_api.redirect_handler
     def update(self, req, id):
         """ Updates the server name or password """
@@ -321,6 +321,7 @@ class Controller(common.OpenstackController):
         return exc.HTTPAccepted()
 
     def _action_rebuild(self, input_dict, req, id):
+        LOG.debug(_("Rebuild server action is not implemented"))
         return faults.Fault(exc.HTTPNotImplemented())
 
     def _action_resize(self, input_dict, req, id):
@@ -336,18 +337,20 @@ class Controller(common.OpenstackController):
         except Exception, e:
             LOG.exception(_("Error in resize %s"), e)
             return faults.Fault(exc.HTTPBadRequest())
-        return faults.Fault(exc.HTTPAccepted())
+        return exc.HTTPAccepted()
 
     def _action_reboot(self, input_dict, req, id):
-        try:
+        if 'reboot' in input_dict and 'type' in input_dict['reboot']:
             reboot_type = input_dict['reboot']['type']
-        except Exception:
-            raise faults.Fault(exc.HTTPNotImplemented())
+        else:
+            LOG.exception(_("Missing argument 'type' for reboot"))
+            return faults.Fault(exc.HTTPUnprocessableEntity())
         try:
             # TODO(gundlach): pass reboot_type, support soft reboot in
             # virt driver
             self.compute_api.reboot(req.environ['nova.context'], id)
-        except:
+        except Exception, e:
+            LOG.exception(_("Error in reboot %s"), e)
             return faults.Fault(exc.HTTPUnprocessableEntity())
         return exc.HTTPAccepted()
 
@@ -648,6 +651,16 @@ class ControllerV11(Controller):
     def _limit_items(self, items, req):
         return common.limited_by_marker(items, req)
 
+    def _get_server_admin_password(self, server):
+        """ Determine the admin password for a server on creation """
+        password = server.get('adminPass')
+        if password is None:
+            return utils.generate_password(16)
+        if not isinstance(password, basestring) or password == '':
+            msg = _("Invalid adminPass")
+            raise exc.HTTPBadRequest(msg)
+        return password
+
     def get_default_xmlns(self, req):
         return common.XML_NS_V11