Merge lp:~justin-fathomdb/nova/justinsb-metadata3 into lp:~hudson-openstack/nova/trunk

Proposed by justinsb
Status: Superseded
Proposed branch: lp:~justin-fathomdb/nova/justinsb-metadata3
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 410 lines (+195/-11)
9 files modified
nova/api/openstack/servers.py (+22/-5)
nova/compute/api.py (+26/-1)
nova/db/sqlalchemy/api.py (+2/-0)
nova/db/sqlalchemy/migrate_repo/versions/005_add_instance_metadata.py (+78/-0)
nova/db/sqlalchemy/models.py (+17/-1)
nova/quota.py (+13/-1)
nova/tests/api/openstack/test_servers.py (+9/-2)
nova/tests/test_quota.py (+25/-0)
run_tests.sh (+3/-1)
To merge this branch: bzr merge lp:~justin-fathomdb/nova/justinsb-metadata3
Reviewer Review Type Date Requested Status
Todd Willey (community) Needs Fixing
Jay Pipes Pending
Eric Day Pending
Dan Prince Pending
Review via email: mp+50667@code.launchpad.net

This proposal supersedes a proposal from 2011-02-17.

This proposal has been superseded by a proposal from 2011-02-23.

Description of the change

Initial support for per-instance metadata, though the OpenStack API. Key/value pairs can be specified at instance creation time and are returned in the details view. Support limits based on quota system.

To post a comment you must log in.
Revision history for this message
Dan Prince (dan-prince) wrote : Posted in a previous version of this proposal

Justin,

Does this branch work with Glance? I'm pretty sure Glance stores kernel ID as image['properties']['kernel_id']. Additionally ramdisk ID is stored as image['properties']['ramdisk_id'].

Perhaps you were developing/testing against S3ImageStore?

Dan

review: Needs Fixing
Revision history for this message
Jay Pipes (jaypipes) wrote : Posted in a previous version of this proposal

To answer Dan's question, no, this patch won't work if FLAGS.image_service == 'glance'. But, that's not necessarily Justin's fault of course, and the OpenStack servers.py controller shouldn't be relying on the Glance image service being set, either...

Devin and Dan have been working on fixing the inconsistencies in the keys returned from the S3ImageService and the GlanceImageService when it comes to the kernel_id (kernelId) and ramdisk_id (ramdiskId) keys. Please see this bug and patch here for a few more details: https://bugs.launchpad.net/nova/+bug/712782

As for the specifics of this patch, Justin, I think it would be clearer if you renamed the Metadata model to InstanceProperty and the table name from metadata to instance_properties, to make it clear what the metadata properties refer to. Since there is already the concept of Metadata in SQLAlchemy, it would make reading the code a tad easier.

Cheers!
jay

Revision history for this message
justinsb (justin-fathomdb) wrote : Posted in a previous version of this proposal

I think I'll probably revert the lines that touched the kernel_id & ramdisk_id - I had to put them in there to get this to work, but it probably doesn't really belong in this patch.

I agree that 'metadata', 'key' and 'value' are potentially problematic names. Two issues though: First, there will be metadata for more than just instances - I can either have one table 'item_properties', or I can go with InstanceProperties, VolumeProperties etc and have multiple tables. And second, "properties" may be a bit overloaded already (e.g. the images already has 'properties' in Dan's code snippet!) Although I'd like the special flags (kernel_id, ramdisk_id, zone, image_type, volume_size etc) to just be treated as well-known metadata, I don't see this happening any time soon, so I don't think we can use 'properties'. Maybe 'Tags'? What do you think Jay?

Revision history for this message
Jay Pipes (jaypipes) wrote : Posted in a previous version of this proposal

> I think I'll probably revert the lines that touched the kernel_id & ramdisk_id
> - I had to put them in there to get this to work, but it probably doesn't
> really belong in this patch.

k, agreed.

> I agree that 'metadata', 'key' and 'value' are potentially problematic names.
> Two issues though: First, there will be metadata for more than just instances
> - I can either have one table 'item_properties', or I can go with
> InstanceProperties, VolumeProperties etc and have multiple tables. And
> second, "properties" may be a bit overloaded already (e.g. the images already
> has 'properties' in Dan's code snippet!)

Heh, yes it does, because Glance's registry database has a table image_properties, with a model, yep, you guessed it, ImageProperty :)

So, I think that InstanceProperties is perfectly fine in this case :)

> Although I'd like the special flags
> (kernel_id, ramdisk_id, zone, image_type, volume_size etc) to just be treated
> as well-known metadata, I don't see this happening any time soon, so I don't
> think we can use 'properties'. Maybe 'Tags'? What do you think Jay?

This should be a mailing list question, I believe. If it's an attribute that *every* instance has, or possibly that *most* instances have, I think it should be in the Instance model directly, but this is just my opinion. Best to ask the ML I think...

-jay

Revision history for this message
Dan Prince (dan-prince) wrote : Posted in a previous version of this proposal

This branch currently doesn't appear to restrict the number of metadata items per instances(server) to 5.

According to the Cloud Servers API docs:

"The maximum size of the metadata key and value is each 255 bytes and the maximum number of key-value pairs that can be supplied per server is 5."

review: Needs Fixing
Revision history for this message
Jay Pipes (jaypipes) wrote : Posted in a previous version of this proposal

<diplomacy-hat>
OpenStack != Cloud Servers, of course, but I would slightly modify what Dan said to:

Could we add a FLAG option that would allow the deployer to set a limit to the number of metadata items per object? ;)
</diplomacy-hat>

Revision history for this message
Devin Carlen (devcamcar) wrote : Posted in a previous version of this proposal

Agree, make the number of metadata items configurable

Revision history for this message
Dan Prince (dan-prince) wrote : Posted in a previous version of this proposal

Absolutely. Making this a configuration flag would be great.

I might suggest we not make the FLAG option default to being 'unlimited'. The ability to specify an unlimited number of items seems like kind of a security concern.

10 items maybe?

Revision history for this message
justinsb (justin-fathomdb) wrote : Posted in a previous version of this proposal

Fixed per suggestions. I'm a bit uneasy about calling the metadata items properties, as I don't think it's particularly clear. Support a limit on the number of items, set through the quota system. Check key/value sizes vs. max length 255.

Revision history for this message
Eric Day (eday) wrote : Posted in a previous version of this proposal

I think we need to decide on using a single term, either 'metadata' or 'properties'. Using one name in the API and another in the DB layer is confusing. Also, it's all metadata (or properties), and this should not be confused with the format (list of key/values pairs or dict of key => value mappings). Using different names for different formats is also going to confuse folks.

Line 8: Nothing in nova/api should be importing db directly, especially implementation specific db modules. We need to encapsulate anything needing this with a proper nova.compute API and then a nova.db API call.

review: Needs Fixing
Revision history for this message
justinsb (justin-fathomdb) wrote : Posted in a previous version of this proposal

Eric: The suggestion from the mailing list was 'properties' because that's what Glance chose. The API uses 'metadata' (which I personally prefer), but was potentially confusing because SQLAlchemy has a Metadata object also (though it didn't cause any code problems). Are you voting that I change the API to 'properties' or that I go back to 'metadata'? If the former, am I allowed to just change the API?

Revision history for this message
Vish Ishaya (vishvananda) wrote : Posted in a previous version of this proposal

Jorge was saying in the api thread that he didn't think metadata should be used for tagging, it is supposed to be client only metadata. We should probably create something new ("tags? properties?"), although it seems fine to use metadata to test it out initially.

Vish

On Feb 16, 2011, at 1:18 PM, justinsb wrote:

> Eric: The suggestion from the mailing list was 'properties' because that's what Glance chose. The API uses 'metadata' (which I personally prefer), but was potentially confusing because SQLAlchemy has a Metadata object also (though it didn't cause any code problems). Are you voting that I change the API to 'properties' or that I go back to 'metadata'? If the former, am I allowed to just change the API?
> --
> https://code.launchpad.net/~justin-fathomdb/nova/justinsb-metadata/+merge/49902
> You are subscribed to branch lp:nova.

Revision history for this message
Eric Day (eday) wrote : Posted in a previous version of this proposal

I have no opinion on what we call it, I just prefer it be the same end-to-end. I defer to others to decide which rename is possible, and if there are still multiple choices, which is most appropriate.

Revision history for this message
justinsb (justin-fathomdb) wrote : Posted in a previous version of this proposal

Vish: I don't necessarily agree with Jorge, but this patch is just about implementing Metadata as defined in the blueprint/CloudServers API. Whether we create a second metadata collection or share this one is a matter for another day/another patch :-)

Revision history for this message
Dan Prince (dan-prince) wrote : Posted in a previous version of this proposal

Good work.

How hard would it be to add a test case that tested for the upper limit of the metadata quota?

Revision history for this message
justinsb (justin-fathomdb) wrote : Posted in a previous version of this proposal

Went with InstanceMetadata and 'metadata', for consistency with the API, as agreed on IRC.
Dan: Added the unit test for the max metadata item count.
Eric: Removed DB model class reference from API. Using a simple dictionary instead

Ready for merge, I hope!

Revision history for this message
Jay Pipes (jaypipes) wrote : Posted in a previous version of this proposal

Thanks for your patience and updates, Justin. Looks good to me now.

review: Approve
Revision history for this message
Dan Prince (dan-prince) wrote : Posted in a previous version of this proposal

Looks good to me. Thanks for adding the tests.

review: Approve
Revision history for this message
justinsb (justin-fathomdb) wrote : Posted in a previous version of this proposal

Jay, Dan: Thanks + no worries about the debate on the naming. I think the patch is much better for it :-)

Revision history for this message
Eric Day (eday) wrote : Posted in a previous version of this proposal

lgtm

review: Approve
Revision history for this message
justinsb (justin-fathomdb) wrote :

Resubmitting after merge with trunk; there was a conflict that had to be resolved.

Revision history for this message
Todd Willey (xtoddx) wrote : Posted in a previous version of this proposal

lgtm

review: Approve
Revision history for this message
Todd Willey (xtoddx) wrote :

I like this:

=== modified file 'nova/api/ec2/cloud.py'
--- nova/api/ec2/cloud.py 2011-02-21 22:47:33 +0000
+++ nova/api/ec2/cloud.py 2011-02-23 06:02:39 +0000
@@ -783,9 +783,6 @@

     def run_instances(self, context, **kwargs):
         max_count = int(kwargs.get('max_count', 1))
- # NOTE(justinsb): the EC2 API doesn't support metadata here, but this
- # is needed for the unit tests. Maybe the unit tests shouldn't be
- # calling the EC2 code
         instances = self.compute_api.create(context,
             instance_type=instance_types.get_by_type(
                 kwargs.get('instance_type', None)),
@@ -800,8 +797,7 @@
             user_data=kwargs.get('user_data'),
             security_group=kwargs.get('security_group'),
             availability_zone=kwargs.get('placement', {}).get(
- 'AvailabilityZone'),
- metadata=kwargs.get('metadata', []))
+ 'AvailabilityZone'))
         return self._format_run_instances(context,
                                           instances[0]['reservation_id'])

=== modified file 'nova/tests/test_quota.py'
--- nova/tests/test_quota.py 2011-02-17 23:00:18 +0000
+++ nova/tests/test_quota.py 2011-02-23 06:18:24 +0000
@@ -16,6 +16,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.

+from nova import compute
 from nova import context
 from nova import db
 from nova import flags
@@ -168,7 +169,7 @@
         metadata = {}
         for i in range(FLAGS.quota_metadata_items + 1):
             metadata['key%s' % i] = 'value%s' % i
- self.assertRaises(quota.QuotaError, self.cloud.run_instances,
+ self.assertRaises(quota.QuotaError, compute.API().create,
                                             self.context,
                                             min_count=1,
                                             max_count=1,

review: Needs Fixing
Revision history for this message
justinsb (justin-fathomdb) wrote :

Todd - good suggestion; it's also saying that quotas should not be enforced at the 'front end' API level, but should instead be supported in the common driver level underneath. I'll make the change.

702. By justinsb

Changed unit test to refer to compute API, per Todd's suggestion.
Avoids needing to extend our implementation of the EC2 API.

703. By justinsb

Merged with trunk

704. By justinsb

Merged with trunk (a criss-cross merge...)

Unmerged revisions

704. By justinsb

Merged with trunk (a criss-cross merge...)

703. By justinsb

Merged with trunk

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'nova/api/openstack/servers.py'
--- nova/api/openstack/servers.py 2011-02-21 07:16:10 +0000
+++ nova/api/openstack/servers.py 2011-02-23 17:43:05 +0000
@@ -77,9 +77,14 @@
77 except KeyError:77 except KeyError:
78 LOG.debug(_("Failed to read public ip(s)"))78 LOG.debug(_("Failed to read public ip(s)"))
7979
80 inst_dict['metadata'] = {}
81 inst_dict['hostId'] = ''80 inst_dict['hostId'] = ''
8281
82 # Return the metadata as a dictionary
83 metadata = {}
84 for item in inst['metadata']:
85 metadata[item['key']] = item['value']
86 inst_dict['metadata'] = metadata
87
83 return dict(server=inst_dict)88 return dict(server=inst_dict)
8489
8590
@@ -161,18 +166,29 @@
161 if not env:166 if not env:
162 return faults.Fault(exc.HTTPUnprocessableEntity())167 return faults.Fault(exc.HTTPUnprocessableEntity())
163168
164 key_pairs = auth_manager.AuthManager.get_key_pairs(169 context = req.environ['nova.context']
165 req.environ['nova.context'])170 key_pairs = auth_manager.AuthManager.get_key_pairs(context)
166 if not key_pairs:171 if not key_pairs:
167 raise exception.NotFound(_("No keypairs defined"))172 raise exception.NotFound(_("No keypairs defined"))
168 key_pair = key_pairs[0]173 key_pair = key_pairs[0]
169174
170 image_id = common.get_image_id_from_image_hash(self._image_service,175 image_id = common.get_image_id_from_image_hash(self._image_service,
171 req.environ['nova.context'], env['server']['imageId'])176 context, env['server']['imageId'])
172 kernel_id, ramdisk_id = self._get_kernel_ramdisk_from_image(177 kernel_id, ramdisk_id = self._get_kernel_ramdisk_from_image(
173 req, image_id)178 req, image_id)
179
180 # Metadata is a list, not a Dictionary, because we allow duplicate keys
181 # (even though JSON can't encode this)
182 # In future, we may not allow duplicate keys.
183 # However, the CloudServers API is not definitive on this front,
184 # and we want to be compatible.
185 metadata = []
186 if env['server']['metadata']:
187 for k, v in env['server']['metadata'].items():
188 metadata.append({'key': k, 'value': v})
189
174 instances = self.compute_api.create(190 instances = self.compute_api.create(
175 req.environ['nova.context'],191 context,
176 instance_types.get_by_flavor_id(env['server']['flavorId']),192 instance_types.get_by_flavor_id(env['server']['flavorId']),
177 image_id,193 image_id,
178 kernel_id=kernel_id,194 kernel_id=kernel_id,
@@ -181,6 +197,7 @@
181 display_description=env['server']['name'],197 display_description=env['server']['name'],
182 key_name=key_pair['name'],198 key_name=key_pair['name'],
183 key_data=key_pair['public_key'],199 key_data=key_pair['public_key'],
200 metadata=metadata,
184 onset_files=env.get('onset_files', []))201 onset_files=env.get('onset_files', []))
185 return _translate_keys(instances[0])202 return _translate_keys(instances[0])
186203
187204
=== modified file 'nova/compute/api.py'
--- nova/compute/api.py 2011-02-23 07:21:01 +0000
+++ nova/compute/api.py 2011-02-23 17:43:05 +0000
@@ -85,7 +85,7 @@
85 min_count=1, max_count=1,85 min_count=1, max_count=1,
86 display_name='', display_description='',86 display_name='', display_description='',
87 key_name=None, key_data=None, security_group='default',87 key_name=None, key_data=None, security_group='default',
88 availability_zone=None, user_data=None,88 availability_zone=None, user_data=None, metadata=[],
89 onset_files=None):89 onset_files=None):
90 """Create the number of instances requested if quota and90 """Create the number of instances requested if quota and
91 other arguments check out ok.91 other arguments check out ok.
@@ -100,6 +100,30 @@
100 "run %s more instances of this type.") %100 "run %s more instances of this type.") %
101 num_instances, "InstanceLimitExceeded")101 num_instances, "InstanceLimitExceeded")
102102
103 num_metadata = len(metadata)
104 quota_metadata = quota.allowed_metadata_items(context, num_metadata)
105 if quota_metadata < num_metadata:
106 pid = context.project_id
107 msg = (_("Quota exceeeded for %(pid)s,"
108 " tried to set %(num_metadata)s metadata properties")
109 % locals())
110 LOG.warn(msg)
111 raise quota.QuotaError(msg, "MetadataLimitExceeded")
112
113 # Because metadata is stored in the DB, we hard-code the size limits
114 # In future, we may support more variable length strings, so we act
115 # as if this is quota-controlled for forwards compatibility
116 for metadata_item in metadata:
117 k = metadata_item['key']
118 v = metadata_item['value']
119 if len(k) > 255 or len(v) > 255:
120 pid = context.project_id
121 msg = (_("Quota exceeeded for %(pid)s,"
122 " metadata property key or value too long")
123 % locals())
124 LOG.warn(msg)
125 raise quota.QuotaError(msg, "MetadataLimitExceeded")
126
103 image = self.image_service.show(context, image_id)127 image = self.image_service.show(context, image_id)
104 if kernel_id is None:128 if kernel_id is None:
105 kernel_id = image.get('kernel_id', None)129 kernel_id = image.get('kernel_id', None)
@@ -154,6 +178,7 @@
154 'key_name': key_name,178 'key_name': key_name,
155 'key_data': key_data,179 'key_data': key_data,
156 'locked': False,180 'locked': False,
181 'metadata': metadata,
157 'availability_zone': availability_zone}182 'availability_zone': availability_zone}
158 elevated = context.elevated()183 elevated = context.elevated()
159 instances = []184 instances = []
160185
=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py 2011-02-23 07:21:01 +0000
+++ nova/db/sqlalchemy/api.py 2011-02-23 17:43:05 +0000
@@ -715,6 +715,7 @@
715 options(joinedload_all('security_groups.rules')).\715 options(joinedload_all('security_groups.rules')).\
716 options(joinedload('volumes')).\716 options(joinedload('volumes')).\
717 options(joinedload_all('fixed_ip.network')).\717 options(joinedload_all('fixed_ip.network')).\
718 options(joinedload('metadata')).\
718 filter_by(id=instance_id).\719 filter_by(id=instance_id).\
719 filter_by(deleted=can_read_deleted(context)).\720 filter_by(deleted=can_read_deleted(context)).\
720 first()721 first()
@@ -723,6 +724,7 @@
723 options(joinedload_all('fixed_ip.floating_ips')).\724 options(joinedload_all('fixed_ip.floating_ips')).\
724 options(joinedload_all('security_groups.rules')).\725 options(joinedload_all('security_groups.rules')).\
725 options(joinedload('volumes')).\726 options(joinedload('volumes')).\
727 options(joinedload('metadata')).\
726 filter_by(project_id=context.project_id).\728 filter_by(project_id=context.project_id).\
727 filter_by(id=instance_id).\729 filter_by(id=instance_id).\
728 filter_by(deleted=False).\730 filter_by(deleted=False).\
729731
=== added file 'nova/db/sqlalchemy/migrate_repo/versions/005_add_instance_metadata.py'
--- nova/db/sqlalchemy/migrate_repo/versions/005_add_instance_metadata.py 1970-01-01 00:00:00 +0000
+++ nova/db/sqlalchemy/migrate_repo/versions/005_add_instance_metadata.py 2011-02-23 17:43:05 +0000
@@ -0,0 +1,78 @@
1# vim: tabstop=4 shiftwidth=4 softtabstop=4
2
3# Copyright 2011 Justin Santa Barbara
4# All Rights Reserved.
5#
6# Licensed under the Apache License, Version 2.0 (the "License"); you may
7# not use this file except in compliance with the License. You may obtain
8# a copy of the License at
9#
10# http://www.apache.org/licenses/LICENSE-2.0
11#
12# Unless required by applicable law or agreed to in writing, software
13# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
14# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
15# License for the specific language governing permissions and limitations
16# under the License.
17
18from sqlalchemy import *
19from migrate import *
20
21from nova import log as logging
22
23
24meta = MetaData()
25
26
27# Just for the ForeignKey and column creation to succeed, these are not the
28# actual definitions of instances or services.
29instances = Table('instances', meta,
30 Column('id', Integer(), primary_key=True, nullable=False),
31 )
32
33quotas = Table('quotas', meta,
34 Column('id', Integer(), primary_key=True, nullable=False),
35 )
36
37
38#
39# New Tables
40#
41
42instance_metadata_table = Table('instance_metadata', meta,
43 Column('created_at', DateTime(timezone=False)),
44 Column('updated_at', DateTime(timezone=False)),
45 Column('deleted_at', DateTime(timezone=False)),
46 Column('deleted', Boolean(create_constraint=True, name=None)),
47 Column('id', Integer(), primary_key=True, nullable=False),
48 Column('instance_id',
49 Integer(),
50 ForeignKey('instances.id'),
51 nullable=False),
52 Column('key',
53 String(length=255, convert_unicode=False, assert_unicode=None,
54 unicode_error=None, _warn_on_bytestring=False)),
55 Column('value',
56 String(length=255, convert_unicode=False, assert_unicode=None,
57 unicode_error=None, _warn_on_bytestring=False)))
58
59
60#
61# New columns
62#
63quota_metadata_items = Column('metadata_items', Integer())
64
65
66def upgrade(migrate_engine):
67 # Upgrade operations go here. Don't create your own engine;
68 # bind migrate_engine to your metadata
69 meta.bind = migrate_engine
70 for table in (instance_metadata_table, ):
71 try:
72 table.create()
73 except Exception:
74 logging.info(repr(table))
75 logging.exception('Exception while creating table')
76 raise
77
78 quotas.create_column(quota_metadata_items)
079
=== modified file 'nova/db/sqlalchemy/models.py'
--- nova/db/sqlalchemy/models.py 2011-02-17 21:39:03 +0000
+++ nova/db/sqlalchemy/models.py 2011-02-23 17:43:05 +0000
@@ -256,6 +256,7 @@
256 volumes = Column(Integer)256 volumes = Column(Integer)
257 gigabytes = Column(Integer)257 gigabytes = Column(Integer)
258 floating_ips = Column(Integer)258 floating_ips = Column(Integer)
259 metadata_items = Column(Integer)
259260
260261
261class ExportDevice(BASE, NovaBase):262class ExportDevice(BASE, NovaBase):
@@ -536,6 +537,20 @@
536 pool = relationship(ConsolePool, backref=backref('consoles'))537 pool = relationship(ConsolePool, backref=backref('consoles'))
537538
538539
540class InstanceMetadata(BASE, NovaBase):
541 """Represents a metadata key/value pair for an instance"""
542 __tablename__ = 'instance_metadata'
543 id = Column(Integer, primary_key=True)
544 key = Column(String(255))
545 value = Column(String(255))
546 instance_id = Column(Integer, ForeignKey('instances.id'), nullable=False)
547 instance = relationship(Instance, backref="metadata",
548 foreign_keys=instance_id,
549 primaryjoin='and_('
550 'InstanceMetadata.instance_id == Instance.id,'
551 'InstanceMetadata.deleted == False)')
552
553
539class Zone(BASE, NovaBase):554class Zone(BASE, NovaBase):
540 """Represents a child zone of this zone."""555 """Represents a child zone of this zone."""
541 __tablename__ = 'zones'556 __tablename__ = 'zones'
@@ -557,7 +572,8 @@
557 Volume, ExportDevice, IscsiTarget, FixedIp, FloatingIp,572 Volume, ExportDevice, IscsiTarget, FixedIp, FloatingIp,
558 Network, SecurityGroup, SecurityGroupIngressRule,573 Network, SecurityGroup, SecurityGroupIngressRule,
559 SecurityGroupInstanceAssociation, AuthToken, User,574 SecurityGroupInstanceAssociation, AuthToken, User,
560 Project, Certificate, ConsolePool, Console, Zone)575 Project, Certificate, ConsolePool, Console, Zone,
576 InstanceMetadata)
561 engine = create_engine(FLAGS.sql_connection, echo=False)577 engine = create_engine(FLAGS.sql_connection, echo=False)
562 for model in models:578 for model in models:
563 model.metadata.create_all(engine)579 model.metadata.create_all(engine)
564580
=== modified file 'nova/quota.py'
--- nova/quota.py 2011-01-04 21:56:36 +0000
+++ nova/quota.py 2011-02-23 17:43:05 +0000
@@ -35,6 +35,8 @@
35 'number of volume gigabytes allowed per project')35 'number of volume gigabytes allowed per project')
36flags.DEFINE_integer('quota_floating_ips', 10,36flags.DEFINE_integer('quota_floating_ips', 10,
37 'number of floating ips allowed per project')37 'number of floating ips allowed per project')
38flags.DEFINE_integer('quota_metadata_items', 128,
39 'number of metadata items allowed per instance')
3840
3941
40def get_quota(context, project_id):42def get_quota(context, project_id):
@@ -42,7 +44,8 @@
42 'cores': FLAGS.quota_cores,44 'cores': FLAGS.quota_cores,
43 'volumes': FLAGS.quota_volumes,45 'volumes': FLAGS.quota_volumes,
44 'gigabytes': FLAGS.quota_gigabytes,46 'gigabytes': FLAGS.quota_gigabytes,
45 'floating_ips': FLAGS.quota_floating_ips}47 'floating_ips': FLAGS.quota_floating_ips,
48 'metadata_items': FLAGS.quota_metadata_items}
46 try:49 try:
47 quota = db.quota_get(context, project_id)50 quota = db.quota_get(context, project_id)
48 for key in rval.keys():51 for key in rval.keys():
@@ -94,6 +97,15 @@
94 return min(num_floating_ips, allowed_floating_ips)97 return min(num_floating_ips, allowed_floating_ips)
9598
9699
100def allowed_metadata_items(context, num_metadata_items):
101 """Check quota; return min(num_metadata_items,allowed_metadata_items)"""
102 project_id = context.project_id
103 context = context.elevated()
104 quota = get_quota(context, project_id)
105 num_allowed_metadata_items = quota['metadata_items']
106 return min(num_metadata_items, num_allowed_metadata_items)
107
108
97class QuotaError(exception.ApiError):109class QuotaError(exception.ApiError):
98 """Quota Exceeeded"""110 """Quota Exceeeded"""
99 pass111 pass
100112
=== modified file 'nova/tests/api/openstack/test_servers.py'
--- nova/tests/api/openstack/test_servers.py 2011-02-23 07:21:01 +0000
+++ nova/tests/api/openstack/test_servers.py 2011-02-23 17:43:05 +0000
@@ -28,6 +28,7 @@
28from nova.api.openstack import servers28from nova.api.openstack import servers
29import nova.db.api29import nova.db.api
30from nova.db.sqlalchemy.models import Instance30from nova.db.sqlalchemy.models import Instance
31from nova.db.sqlalchemy.models import InstanceMetadata
31import nova.rpc32import nova.rpc
32from nova.tests.api.openstack import fakes33from nova.tests.api.openstack import fakes
3334
@@ -64,6 +65,9 @@
6465
6566
66def stub_instance(id, user_id=1, private_address=None, public_addresses=None):67def stub_instance(id, user_id=1, private_address=None, public_addresses=None):
68 metadata = []
69 metadata.append(InstanceMetadata(key='seq', value=id))
70
67 if public_addresses == None:71 if public_addresses == None:
68 public_addresses = list()72 public_addresses = list()
6973
@@ -95,7 +99,8 @@
95 "availability_zone": "",99 "availability_zone": "",
96 "display_name": "server%s" % id,100 "display_name": "server%s" % id,
97 "display_description": "",101 "display_description": "",
98 "locked": False}102 "locked": False,
103 "metadata": metadata}
99104
100 instance["fixed_ip"] = {105 instance["fixed_ip"] = {
101 "address": private_address,106 "address": private_address,
@@ -214,7 +219,8 @@
214 "get_image_id_from_image_hash", image_id_from_hash)219 "get_image_id_from_image_hash", image_id_from_hash)
215220
216 body = dict(server=dict(221 body = dict(server=dict(
217 name='server_test', imageId=2, flavorId=2, metadata={},222 name='server_test', imageId=2, flavorId=2,
223 metadata={'hello': 'world', 'open': 'stack'},
218 personality={}))224 personality={}))
219 req = webob.Request.blank('/v1.0/servers')225 req = webob.Request.blank('/v1.0/servers')
220 req.method = 'POST'226 req.method = 'POST'
@@ -291,6 +297,7 @@
291 self.assertEqual(s['id'], i)297 self.assertEqual(s['id'], i)
292 self.assertEqual(s['name'], 'server%d' % i)298 self.assertEqual(s['name'], 'server%d' % i)
293 self.assertEqual(s['imageId'], 10)299 self.assertEqual(s['imageId'], 10)
300 self.assertEqual(s['metadata']['seq'], i)
294 i += 1301 i += 1
295302
296 def test_server_pause(self):303 def test_server_pause(self):
297304
=== modified file 'nova/tests/test_quota.py'
--- nova/tests/test_quota.py 2011-01-07 14:46:17 +0000
+++ nova/tests/test_quota.py 2011-02-23 17:43:05 +0000
@@ -16,6 +16,7 @@
16# License for the specific language governing permissions and limitations16# License for the specific language governing permissions and limitations
17# under the License.17# under the License.
1818
19from nova import compute
19from nova import context20from nova import context
20from nova import db21from nova import db
21from nova import flags22from nova import flags
@@ -87,6 +88,18 @@
87 num_instances = quota.allowed_instances(self.context, 100,88 num_instances = quota.allowed_instances(self.context, 100,
88 instance_types.INSTANCE_TYPES['m1.small'])89 instance_types.INSTANCE_TYPES['m1.small'])
89 self.assertEqual(num_instances, 10)90 self.assertEqual(num_instances, 10)
91
92 # metadata_items
93 too_many_items = FLAGS.quota_metadata_items + 1000
94 num_metadata_items = quota.allowed_metadata_items(self.context,
95 too_many_items)
96 self.assertEqual(num_metadata_items, FLAGS.quota_metadata_items)
97 db.quota_update(self.context, self.project.id, {'metadata_items': 5})
98 num_metadata_items = quota.allowed_metadata_items(self.context,
99 too_many_items)
100 self.assertEqual(num_metadata_items, 5)
101
102 # Cleanup
90 db.quota_destroy(self.context, self.project.id)103 db.quota_destroy(self.context, self.project.id)
91104
92 def test_too_many_instances(self):105 def test_too_many_instances(self):
@@ -151,3 +164,15 @@
151 self.assertRaises(quota.QuotaError, self.cloud.allocate_address,164 self.assertRaises(quota.QuotaError, self.cloud.allocate_address,
152 self.context)165 self.context)
153 db.floating_ip_destroy(context.get_admin_context(), address)166 db.floating_ip_destroy(context.get_admin_context(), address)
167
168 def test_too_many_metadata_items(self):
169 metadata = {}
170 for i in range(FLAGS.quota_metadata_items + 1):
171 metadata['key%s' % i] = 'value%s' % i
172 self.assertRaises(quota.QuotaError, compute.API().create,
173 self.context,
174 min_count=1,
175 max_count=1,
176 instance_type='m1.small',
177 image_id='fake',
178 metadata=metadata)
154179
=== modified file 'run_tests.sh'
--- run_tests.sh 2011-02-22 00:05:28 +0000
+++ run_tests.sh 2011-02-23 17:43:05 +0000
@@ -73,7 +73,9 @@
7373
74if [ -z "$noseargs" ];74if [ -z "$noseargs" ];
75then75then
76 run_tests && pep8 --repeat --show-pep8 --show-source --exclude=vcsversion.py bin/* nova setup.py || exit 176 srcfiles=`find bin -type f ! -name "nova.conf*"`
77 srcfiles+=" nova setup.py"
78 run_tests && pep8 --repeat --show-pep8 --show-source --exclude=vcsversion.py ${srcfiles} || exit 1
77else79else
78 run_tests80 run_tests
79fi81fi