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

Proposed by justinsb
Status: Merged
Approved by: Jay Pipes
Approved revision: 702
Merged at revision: 717
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
Jay Pipes (community) Approve
Todd Willey (community) Approve
Eric Day Pending
Dan Prince Pending
Review via email: mp+50965@code.launchpad.net

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

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 : Posted in a previous version of this proposal

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 : Posted in a previous version of this proposal

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 : Posted in a previous version of this proposal

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.

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

Nice!

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

Todd: The patch you proposed was exactly what it ended up being... I should have just copied and pasted!

Revision history for this message
Jay Pipes (jaypipes) wrote :

lgtm4 :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'nova/api/openstack/servers.py'
2--- nova/api/openstack/servers.py 2011-02-21 07:16:10 +0000
3+++ nova/api/openstack/servers.py 2011-02-23 17:47:10 +0000
4@@ -77,9 +77,14 @@
5 except KeyError:
6 LOG.debug(_("Failed to read public ip(s)"))
7
8- inst_dict['metadata'] = {}
9 inst_dict['hostId'] = ''
10
11+ # Return the metadata as a dictionary
12+ metadata = {}
13+ for item in inst['metadata']:
14+ metadata[item['key']] = item['value']
15+ inst_dict['metadata'] = metadata
16+
17 return dict(server=inst_dict)
18
19
20@@ -161,18 +166,29 @@
21 if not env:
22 return faults.Fault(exc.HTTPUnprocessableEntity())
23
24- key_pairs = auth_manager.AuthManager.get_key_pairs(
25- req.environ['nova.context'])
26+ context = req.environ['nova.context']
27+ key_pairs = auth_manager.AuthManager.get_key_pairs(context)
28 if not key_pairs:
29 raise exception.NotFound(_("No keypairs defined"))
30 key_pair = key_pairs[0]
31
32 image_id = common.get_image_id_from_image_hash(self._image_service,
33- req.environ['nova.context'], env['server']['imageId'])
34+ context, env['server']['imageId'])
35 kernel_id, ramdisk_id = self._get_kernel_ramdisk_from_image(
36 req, image_id)
37+
38+ # Metadata is a list, not a Dictionary, because we allow duplicate keys
39+ # (even though JSON can't encode this)
40+ # In future, we may not allow duplicate keys.
41+ # However, the CloudServers API is not definitive on this front,
42+ # and we want to be compatible.
43+ metadata = []
44+ if env['server']['metadata']:
45+ for k, v in env['server']['metadata'].items():
46+ metadata.append({'key': k, 'value': v})
47+
48 instances = self.compute_api.create(
49- req.environ['nova.context'],
50+ context,
51 instance_types.get_by_flavor_id(env['server']['flavorId']),
52 image_id,
53 kernel_id=kernel_id,
54@@ -181,6 +197,7 @@
55 display_description=env['server']['name'],
56 key_name=key_pair['name'],
57 key_data=key_pair['public_key'],
58+ metadata=metadata,
59 onset_files=env.get('onset_files', []))
60 return _translate_keys(instances[0])
61
62
63=== modified file 'nova/compute/api.py'
64--- nova/compute/api.py 2011-02-23 07:21:01 +0000
65+++ nova/compute/api.py 2011-02-23 17:47:10 +0000
66@@ -85,7 +85,7 @@
67 min_count=1, max_count=1,
68 display_name='', display_description='',
69 key_name=None, key_data=None, security_group='default',
70- availability_zone=None, user_data=None,
71+ availability_zone=None, user_data=None, metadata=[],
72 onset_files=None):
73 """Create the number of instances requested if quota and
74 other arguments check out ok.
75@@ -100,6 +100,30 @@
76 "run %s more instances of this type.") %
77 num_instances, "InstanceLimitExceeded")
78
79+ num_metadata = len(metadata)
80+ quota_metadata = quota.allowed_metadata_items(context, num_metadata)
81+ if quota_metadata < num_metadata:
82+ pid = context.project_id
83+ msg = (_("Quota exceeeded for %(pid)s,"
84+ " tried to set %(num_metadata)s metadata properties")
85+ % locals())
86+ LOG.warn(msg)
87+ raise quota.QuotaError(msg, "MetadataLimitExceeded")
88+
89+ # Because metadata is stored in the DB, we hard-code the size limits
90+ # In future, we may support more variable length strings, so we act
91+ # as if this is quota-controlled for forwards compatibility
92+ for metadata_item in metadata:
93+ k = metadata_item['key']
94+ v = metadata_item['value']
95+ if len(k) > 255 or len(v) > 255:
96+ pid = context.project_id
97+ msg = (_("Quota exceeeded for %(pid)s,"
98+ " metadata property key or value too long")
99+ % locals())
100+ LOG.warn(msg)
101+ raise quota.QuotaError(msg, "MetadataLimitExceeded")
102+
103 image = self.image_service.show(context, image_id)
104 if kernel_id is None:
105 kernel_id = image.get('kernel_id', None)
106@@ -154,6 +178,7 @@
107 'key_name': key_name,
108 'key_data': key_data,
109 'locked': False,
110+ 'metadata': metadata,
111 'availability_zone': availability_zone}
112 elevated = context.elevated()
113 instances = []
114
115=== modified file 'nova/db/sqlalchemy/api.py'
116--- nova/db/sqlalchemy/api.py 2011-02-23 07:21:01 +0000
117+++ nova/db/sqlalchemy/api.py 2011-02-23 17:47:10 +0000
118@@ -715,6 +715,7 @@
119 options(joinedload_all('security_groups.rules')).\
120 options(joinedload('volumes')).\
121 options(joinedload_all('fixed_ip.network')).\
122+ options(joinedload('metadata')).\
123 filter_by(id=instance_id).\
124 filter_by(deleted=can_read_deleted(context)).\
125 first()
126@@ -723,6 +724,7 @@
127 options(joinedload_all('fixed_ip.floating_ips')).\
128 options(joinedload_all('security_groups.rules')).\
129 options(joinedload('volumes')).\
130+ options(joinedload('metadata')).\
131 filter_by(project_id=context.project_id).\
132 filter_by(id=instance_id).\
133 filter_by(deleted=False).\
134
135=== added file 'nova/db/sqlalchemy/migrate_repo/versions/005_add_instance_metadata.py'
136--- nova/db/sqlalchemy/migrate_repo/versions/005_add_instance_metadata.py 1970-01-01 00:00:00 +0000
137+++ nova/db/sqlalchemy/migrate_repo/versions/005_add_instance_metadata.py 2011-02-23 17:47:10 +0000
138@@ -0,0 +1,78 @@
139+# vim: tabstop=4 shiftwidth=4 softtabstop=4
140+
141+# Copyright 2011 Justin Santa Barbara
142+# All Rights Reserved.
143+#
144+# Licensed under the Apache License, Version 2.0 (the "License"); you may
145+# not use this file except in compliance with the License. You may obtain
146+# a copy of the License at
147+#
148+# http://www.apache.org/licenses/LICENSE-2.0
149+#
150+# Unless required by applicable law or agreed to in writing, software
151+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
152+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
153+# License for the specific language governing permissions and limitations
154+# under the License.
155+
156+from sqlalchemy import *
157+from migrate import *
158+
159+from nova import log as logging
160+
161+
162+meta = MetaData()
163+
164+
165+# Just for the ForeignKey and column creation to succeed, these are not the
166+# actual definitions of instances or services.
167+instances = Table('instances', meta,
168+ Column('id', Integer(), primary_key=True, nullable=False),
169+ )
170+
171+quotas = Table('quotas', meta,
172+ Column('id', Integer(), primary_key=True, nullable=False),
173+ )
174+
175+
176+#
177+# New Tables
178+#
179+
180+instance_metadata_table = Table('instance_metadata', meta,
181+ Column('created_at', DateTime(timezone=False)),
182+ Column('updated_at', DateTime(timezone=False)),
183+ Column('deleted_at', DateTime(timezone=False)),
184+ Column('deleted', Boolean(create_constraint=True, name=None)),
185+ Column('id', Integer(), primary_key=True, nullable=False),
186+ Column('instance_id',
187+ Integer(),
188+ ForeignKey('instances.id'),
189+ nullable=False),
190+ Column('key',
191+ String(length=255, convert_unicode=False, assert_unicode=None,
192+ unicode_error=None, _warn_on_bytestring=False)),
193+ Column('value',
194+ String(length=255, convert_unicode=False, assert_unicode=None,
195+ unicode_error=None, _warn_on_bytestring=False)))
196+
197+
198+#
199+# New columns
200+#
201+quota_metadata_items = Column('metadata_items', Integer())
202+
203+
204+def upgrade(migrate_engine):
205+ # Upgrade operations go here. Don't create your own engine;
206+ # bind migrate_engine to your metadata
207+ meta.bind = migrate_engine
208+ for table in (instance_metadata_table, ):
209+ try:
210+ table.create()
211+ except Exception:
212+ logging.info(repr(table))
213+ logging.exception('Exception while creating table')
214+ raise
215+
216+ quotas.create_column(quota_metadata_items)
217
218=== modified file 'nova/db/sqlalchemy/models.py'
219--- nova/db/sqlalchemy/models.py 2011-02-17 21:39:03 +0000
220+++ nova/db/sqlalchemy/models.py 2011-02-23 17:47:10 +0000
221@@ -256,6 +256,7 @@
222 volumes = Column(Integer)
223 gigabytes = Column(Integer)
224 floating_ips = Column(Integer)
225+ metadata_items = Column(Integer)
226
227
228 class ExportDevice(BASE, NovaBase):
229@@ -536,6 +537,20 @@
230 pool = relationship(ConsolePool, backref=backref('consoles'))
231
232
233+class InstanceMetadata(BASE, NovaBase):
234+ """Represents a metadata key/value pair for an instance"""
235+ __tablename__ = 'instance_metadata'
236+ id = Column(Integer, primary_key=True)
237+ key = Column(String(255))
238+ value = Column(String(255))
239+ instance_id = Column(Integer, ForeignKey('instances.id'), nullable=False)
240+ instance = relationship(Instance, backref="metadata",
241+ foreign_keys=instance_id,
242+ primaryjoin='and_('
243+ 'InstanceMetadata.instance_id == Instance.id,'
244+ 'InstanceMetadata.deleted == False)')
245+
246+
247 class Zone(BASE, NovaBase):
248 """Represents a child zone of this zone."""
249 __tablename__ = 'zones'
250@@ -557,7 +572,8 @@
251 Volume, ExportDevice, IscsiTarget, FixedIp, FloatingIp,
252 Network, SecurityGroup, SecurityGroupIngressRule,
253 SecurityGroupInstanceAssociation, AuthToken, User,
254- Project, Certificate, ConsolePool, Console, Zone)
255+ Project, Certificate, ConsolePool, Console, Zone,
256+ InstanceMetadata)
257 engine = create_engine(FLAGS.sql_connection, echo=False)
258 for model in models:
259 model.metadata.create_all(engine)
260
261=== modified file 'nova/quota.py'
262--- nova/quota.py 2011-01-04 21:56:36 +0000
263+++ nova/quota.py 2011-02-23 17:47:10 +0000
264@@ -35,6 +35,8 @@
265 'number of volume gigabytes allowed per project')
266 flags.DEFINE_integer('quota_floating_ips', 10,
267 'number of floating ips allowed per project')
268+flags.DEFINE_integer('quota_metadata_items', 128,
269+ 'number of metadata items allowed per instance')
270
271
272 def get_quota(context, project_id):
273@@ -42,7 +44,8 @@
274 'cores': FLAGS.quota_cores,
275 'volumes': FLAGS.quota_volumes,
276 'gigabytes': FLAGS.quota_gigabytes,
277- 'floating_ips': FLAGS.quota_floating_ips}
278+ 'floating_ips': FLAGS.quota_floating_ips,
279+ 'metadata_items': FLAGS.quota_metadata_items}
280 try:
281 quota = db.quota_get(context, project_id)
282 for key in rval.keys():
283@@ -94,6 +97,15 @@
284 return min(num_floating_ips, allowed_floating_ips)
285
286
287+def allowed_metadata_items(context, num_metadata_items):
288+ """Check quota; return min(num_metadata_items,allowed_metadata_items)"""
289+ project_id = context.project_id
290+ context = context.elevated()
291+ quota = get_quota(context, project_id)
292+ num_allowed_metadata_items = quota['metadata_items']
293+ return min(num_metadata_items, num_allowed_metadata_items)
294+
295+
296 class QuotaError(exception.ApiError):
297 """Quota Exceeeded"""
298 pass
299
300=== modified file 'nova/tests/api/openstack/test_servers.py'
301--- nova/tests/api/openstack/test_servers.py 2011-02-23 07:21:01 +0000
302+++ nova/tests/api/openstack/test_servers.py 2011-02-23 17:47:10 +0000
303@@ -28,6 +28,7 @@
304 from nova.api.openstack import servers
305 import nova.db.api
306 from nova.db.sqlalchemy.models import Instance
307+from nova.db.sqlalchemy.models import InstanceMetadata
308 import nova.rpc
309 from nova.tests.api.openstack import fakes
310
311@@ -64,6 +65,9 @@
312
313
314 def stub_instance(id, user_id=1, private_address=None, public_addresses=None):
315+ metadata = []
316+ metadata.append(InstanceMetadata(key='seq', value=id))
317+
318 if public_addresses == None:
319 public_addresses = list()
320
321@@ -95,7 +99,8 @@
322 "availability_zone": "",
323 "display_name": "server%s" % id,
324 "display_description": "",
325- "locked": False}
326+ "locked": False,
327+ "metadata": metadata}
328
329 instance["fixed_ip"] = {
330 "address": private_address,
331@@ -214,7 +219,8 @@
332 "get_image_id_from_image_hash", image_id_from_hash)
333
334 body = dict(server=dict(
335- name='server_test', imageId=2, flavorId=2, metadata={},
336+ name='server_test', imageId=2, flavorId=2,
337+ metadata={'hello': 'world', 'open': 'stack'},
338 personality={}))
339 req = webob.Request.blank('/v1.0/servers')
340 req.method = 'POST'
341@@ -291,6 +297,7 @@
342 self.assertEqual(s['id'], i)
343 self.assertEqual(s['name'], 'server%d' % i)
344 self.assertEqual(s['imageId'], 10)
345+ self.assertEqual(s['metadata']['seq'], i)
346 i += 1
347
348 def test_server_pause(self):
349
350=== modified file 'nova/tests/test_quota.py'
351--- nova/tests/test_quota.py 2011-01-07 14:46:17 +0000
352+++ nova/tests/test_quota.py 2011-02-23 17:47:10 +0000
353@@ -16,6 +16,7 @@
354 # License for the specific language governing permissions and limitations
355 # under the License.
356
357+from nova import compute
358 from nova import context
359 from nova import db
360 from nova import flags
361@@ -87,6 +88,18 @@
362 num_instances = quota.allowed_instances(self.context, 100,
363 instance_types.INSTANCE_TYPES['m1.small'])
364 self.assertEqual(num_instances, 10)
365+
366+ # metadata_items
367+ too_many_items = FLAGS.quota_metadata_items + 1000
368+ num_metadata_items = quota.allowed_metadata_items(self.context,
369+ too_many_items)
370+ self.assertEqual(num_metadata_items, FLAGS.quota_metadata_items)
371+ db.quota_update(self.context, self.project.id, {'metadata_items': 5})
372+ num_metadata_items = quota.allowed_metadata_items(self.context,
373+ too_many_items)
374+ self.assertEqual(num_metadata_items, 5)
375+
376+ # Cleanup
377 db.quota_destroy(self.context, self.project.id)
378
379 def test_too_many_instances(self):
380@@ -151,3 +164,15 @@
381 self.assertRaises(quota.QuotaError, self.cloud.allocate_address,
382 self.context)
383 db.floating_ip_destroy(context.get_admin_context(), address)
384+
385+ def test_too_many_metadata_items(self):
386+ metadata = {}
387+ for i in range(FLAGS.quota_metadata_items + 1):
388+ metadata['key%s' % i] = 'value%s' % i
389+ self.assertRaises(quota.QuotaError, compute.API().create,
390+ self.context,
391+ min_count=1,
392+ max_count=1,
393+ instance_type='m1.small',
394+ image_id='fake',
395+ metadata=metadata)
396
397=== modified file 'run_tests.sh'
398--- run_tests.sh 2011-02-22 00:05:28 +0000
399+++ run_tests.sh 2011-02-23 17:47:10 +0000
400@@ -73,7 +73,9 @@
401
402 if [ -z "$noseargs" ];
403 then
404- run_tests && pep8 --repeat --show-pep8 --show-source --exclude=vcsversion.py bin/* nova setup.py || exit 1
405+ srcfiles=`find bin -type f ! -name "nova.conf*"`
406+ srcfiles+=" nova setup.py"
407+ run_tests && pep8 --repeat --show-pep8 --show-source --exclude=vcsversion.py ${srcfiles} || exit 1
408 else
409 run_tests
410 fi