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

Proposed by justinsb
Status: Superseded
Proposed branch: lp:~justin-fathomdb/nova/justinsb-metadata
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 776 lines (+495/-17) (has conflicts)
13 files modified
nova/api/ec2/cloud.py (+5/-1)
nova/api/openstack/__init__.py (+4/-0)
nova/api/openstack/servers.py (+53/-11)
nova/compute/api.py (+27/-2)
nova/db/sqlalchemy/api.py (+2/-0)
nova/db/sqlalchemy/migrate_repo/versions/003_add_label_to_networks.py (+51/-0)
nova/db/sqlalchemy/migrate_repo/versions/004_add_instance_metadata.py (+78/-0)
nova/db/sqlalchemy/models.py (+24/-0)
nova/quota.py (+13/-1)
nova/tests/api/openstack/test_common.py (+161/-0)
nova/tests/api/openstack/test_servers.py (+50/-1)
nova/tests/test_quota.py (+24/-0)
run_tests.sh (+3/-1)
Text conflict in nova/api/openstack/__init__.py
Text conflict in nova/api/openstack/servers.py
Conflict adding file nova/db/sqlalchemy/migrate_repo/versions/003_add_label_to_networks.py.  Moved existing file to nova/db/sqlalchemy/migrate_repo/versions/003_add_label_to_networks.py.moved.
Text conflict in nova/db/sqlalchemy/models.py
Conflict adding file nova/tests/api/openstack/test_common.py.  Moved existing file to nova/tests/api/openstack/test_common.py.moved.
Text conflict in nova/tests/api/openstack/test_servers.py
To merge this branch: bzr merge lp:~justin-fathomdb/nova/justinsb-metadata
Reviewer Review Type Date Requested Status
Eric Day (community) Needs Fixing
Nova Core security contacts Pending
Dan Prince Pending
Review via email: mp+49902@code.launchpad.net

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

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

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 :

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 :

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 :

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 :

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 :

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 :

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 :

Good work.

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

669. By SuperStack <email address hidden>

Merge branch 'metadata' of https://github.com/justinsb/nova into justinsb-metadata

670. By SuperStack <email address hidden>

Merge branch 'metadata' of https://github.com/justinsb/nova into justinsb-metadata

671. By SuperStack <email address hidden>

Merge branch 'metadata' of https://github.com/justinsb/nova into justinsb-metadata

672. By SuperStack <email address hidden>

Fixed up note

673. By SuperStack <email address hidden>

Comment to confirm that dictionary use is intentional

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

lgtm

review: Approve

Unmerged revisions

673. By SuperStack <email address hidden>

Comment to confirm that dictionary use is intentional

672. By SuperStack <email address hidden>

Fixed up note

671. By SuperStack <email address hidden>

Merge branch 'metadata' of https://github.com/justinsb/nova into justinsb-metadata

670. By SuperStack <email address hidden>

Merge branch 'metadata' of https://github.com/justinsb/nova into justinsb-metadata

669. By SuperStack <email address hidden>

Merge branch 'metadata' of https://github.com/justinsb/nova into justinsb-metadata

668. By SuperStack <email address hidden>

Merge branch 'metadata' of https://github.com/justinsb/nova into justinsb-metadata

667. By SuperStack <email address hidden>

Merge branch 'metadata' of https://github.com/justinsb/nova into justinsb-metadata

666. By SuperStack <email address hidden>

Merge branch 'metadata' of https://github.com/justinsb/nova into justinsb-metadata

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'nova/api/ec2/cloud.py'
2--- nova/api/ec2/cloud.py 2011-02-15 00:51:51 +0000
3+++ nova/api/ec2/cloud.py 2011-02-17 22:32:15 +0000
4@@ -783,6 +783,9 @@
5
6 def run_instances(self, context, **kwargs):
7 max_count = int(kwargs.get('max_count', 1))
8+ # NOTE(justinsb): the EC2 API doesn't support metadata here, but this
9+ # is needed for the unit tests. Maybe the unit tests shouldn't be
10+ # calling the EC2 code
11 instances = self.compute_api.create(context,
12 instance_type=instance_types.get_by_type(
13 kwargs.get('instance_type', None)),
14@@ -797,7 +800,8 @@
15 user_data=kwargs.get('user_data'),
16 security_group=kwargs.get('security_group'),
17 availability_zone=kwargs.get('placement', {}).get(
18- 'AvailabilityZone'))
19+ 'AvailabilityZone'),
20+ metadata=kwargs.get('metadata', []))
21 return self._format_run_instances(context,
22 instances[0]['reservation_id'])
23
24
25=== modified file 'nova/api/openstack/__init__.py'
26--- nova/api/openstack/__init__.py 2011-02-17 21:39:03 +0000
27+++ nova/api/openstack/__init__.py 2011-02-17 22:32:15 +0000
28@@ -80,10 +80,14 @@
29 server_members["actions"] = "GET"
30 server_members['suspend'] = 'POST'
31 server_members['resume'] = 'POST'
32+<<<<<<< TREE
33 server_members['reset_network'] = 'POST'
34
35 mapper.resource("zone", "zones", controller=zones.Controller(),
36 collection={'detail': 'GET'})
37+=======
38+ server_members['reset_network'] = 'POST'
39+>>>>>>> MERGE-SOURCE
40
41 mapper.resource("server", "servers", controller=servers.Controller(),
42 collection={'detail': 'GET'},
43
44=== modified file 'nova/api/openstack/servers.py'
45--- nova/api/openstack/servers.py 2011-02-17 21:51:24 +0000
46+++ nova/api/openstack/servers.py 2011-02-17 22:32:15 +0000
47@@ -62,6 +62,7 @@
48
49 inst_dict['status'] = power_mapping[inst_dict['status']]
50 inst_dict['addresses'] = dict(public=[], private=[])
51+<<<<<<< TREE
52
53 # grab single private fixed ip
54 try:
55@@ -79,8 +80,32 @@
56 LOG.debug(_("Failed to read public ip(s)"))
57
58 inst_dict['metadata'] = {}
59+=======
60+
61+ # grab single private fixed ip
62+ try:
63+ private_ip = inst['fixed_ip']['address']
64+ if private_ip:
65+ inst_dict['addresses']['private'].append(private_ip)
66+ except KeyError:
67+ LOG.debug(_("Failed to read private ip"))
68+
69+ # grab all public floating ips
70+ try:
71+ for floating in inst['fixed_ip']['floating_ips']:
72+ inst_dict['addresses']['public'].append(floating['address'])
73+ except KeyError:
74+ LOG.debug(_("Failed to read public ip(s)"))
75+
76+>>>>>>> MERGE-SOURCE
77 inst_dict['hostId'] = ''
78
79+ # Return the metadata as a dictionary
80+ metadata = {}
81+ for item in inst['metadata']:
82+ metadata[item['key']] = item['value']
83+ inst_dict['metadata'] = metadata
84+
85 return dict(server=inst_dict)
86
87
88@@ -144,13 +169,17 @@
89 metadata stored in Glance as 'image_properties'
90 """
91 def lookup(param):
92+ value = None
93+
94 _image_id = image_id
95- try:
96- return image['properties'][param]
97- except KeyError:
98- raise exception.NotFound(
99- _("%(param)s property not found for image %(_image_id)s") %
100- locals())
101+ properties = image.get('properties')
102+ if properties:
103+ value = properties.get(param)
104+
105+ if not value:
106+ value = image.get(param)
107+
108+ return value
109
110 image_id = str(image_id)
111 image = self._image_service.show(req.environ['nova.context'], image_id)
112@@ -162,14 +191,26 @@
113 if not env:
114 return faults.Fault(exc.HTTPUnprocessableEntity())
115
116- key_pair = auth_manager.AuthManager.get_key_pairs(
117- req.environ['nova.context'])[0]
118+ context = req.environ['nova.context']
119+
120+ key_pair = auth_manager.AuthManager.get_key_pairs(context)[0]
121 image_id = common.get_image_id_from_image_hash(self._image_service,
122- req.environ['nova.context'], env['server']['imageId'])
123+ context, env['server']['imageId'])
124 kernel_id, ramdisk_id = self._get_kernel_ramdisk_from_image(
125 req, image_id)
126+
127+ # Metadata is a list, not a Dictionary, because we allow duplicate keys
128+ # (even though JSON can't encode this)
129+ # In future, we may not allow duplicate keys.
130+ # However, the CloudServers API is not definitive on this front,
131+ # and we want to be compatible.
132+ metadata = []
133+ if env['server']['metadata']:
134+ for k, v in env['server']['metadata'].items():
135+ metadata.append({'key': k, 'value': v})
136+
137 instances = self.compute_api.create(
138- req.environ['nova.context'],
139+ context,
140 instance_types.get_by_flavor_id(env['server']['flavorId']),
141 image_id,
142 kernel_id=kernel_id,
143@@ -177,7 +218,8 @@
144 display_name=env['server']['name'],
145 display_description=env['server']['name'],
146 key_name=key_pair['name'],
147- key_data=key_pair['public_key'])
148+ key_data=key_pair['public_key'],
149+ metadata=metadata)
150 return _translate_keys(instances[0])
151
152 def update(self, req, id):
153
154=== modified file 'nova/compute/api.py'
155--- nova/compute/api.py 2011-02-17 19:10:37 +0000
156+++ nova/compute/api.py 2011-02-17 22:32:15 +0000
157@@ -85,7 +85,7 @@
158 min_count=1, max_count=1,
159 display_name='', display_description='',
160 key_name=None, key_data=None, security_group='default',
161- availability_zone=None, user_data=None):
162+ availability_zone=None, user_data=None, metadata=[]):
163 """Create the number of instances requested if quota and
164 other arguments check out ok."""
165
166@@ -99,6 +99,30 @@
167 "run %s more instances of this type.") %
168 num_instances, "InstanceLimitExceeded")
169
170+ num_metadata = len(metadata)
171+ quota_metadata = quota.allowed_metadata_items(context, num_metadata)
172+ if quota_metadata < num_metadata:
173+ pid = context.project_id
174+ msg = (_("Quota exceeeded for %(pid)s,"
175+ " tried to set %(num_metadata)s metadata properties")
176+ % locals())
177+ LOG.warn(msg)
178+ raise quota.QuotaError(msg, "MetadataLimitExceeded")
179+
180+ # Because metadata is stored in the DB, we hard-code the size limits
181+ # In future, we may support more variable length strings, so we act
182+ # as if this is quota-controlled for forwards compatibility
183+ for metadata_item in metadata:
184+ k = metadata_item['key']
185+ v = metadata_item['value']
186+ if len(k) > 255 or len(v) > 255:
187+ pid = context.project_id
188+ msg = (_("Quota exceeeded for %(pid)s,"
189+ " metadata property key or value too long")
190+ % locals())
191+ LOG.warn(msg)
192+ raise quota.QuotaError(msg, "MetadataLimitExceeded")
193+
194 is_vpn = image_id == FLAGS.vpn_image_id
195 if not is_vpn:
196 image = self.image_service.show(context, image_id)
197@@ -155,7 +179,8 @@
198 'key_name': key_name,
199 'key_data': key_data,
200 'locked': False,
201- 'availability_zone': availability_zone}
202+ 'availability_zone': availability_zone,
203+ 'metadata': metadata}
204
205 elevated = context.elevated()
206 instances = []
207
208=== modified file 'nova/db/api.py'
209=== modified file 'nova/db/sqlalchemy/api.py'
210--- nova/db/sqlalchemy/api.py 2011-02-17 21:39:03 +0000
211+++ nova/db/sqlalchemy/api.py 2011-02-17 22:32:15 +0000
212@@ -715,6 +715,7 @@
213 options(joinedload_all('security_groups.rules')).\
214 options(joinedload('volumes')).\
215 options(joinedload_all('fixed_ip.network')).\
216+ options(joinedload('metadata')).\
217 filter_by(id=instance_id).\
218 filter_by(deleted=can_read_deleted(context)).\
219 first()
220@@ -723,6 +724,7 @@
221 options(joinedload_all('fixed_ip.floating_ips')).\
222 options(joinedload_all('security_groups.rules')).\
223 options(joinedload('volumes')).\
224+ options(joinedload('metadata')).\
225 filter_by(project_id=context.project_id).\
226 filter_by(id=instance_id).\
227 filter_by(deleted=False).\
228
229=== added file 'nova/db/sqlalchemy/migrate_repo/versions/003_add_label_to_networks.py'
230--- nova/db/sqlalchemy/migrate_repo/versions/003_add_label_to_networks.py 1970-01-01 00:00:00 +0000
231+++ nova/db/sqlalchemy/migrate_repo/versions/003_add_label_to_networks.py 2011-02-17 22:32:15 +0000
232@@ -0,0 +1,51 @@
233+# vim: tabstop=4 shiftwidth=4 softtabstop=4
234+
235+# Copyright 2011 OpenStack LLC
236+# All Rights Reserved.
237+#
238+# Licensed under the Apache License, Version 2.0 (the "License"); you may
239+# not use this file except in compliance with the License. You may obtain
240+# a copy of the License at
241+#
242+# http://www.apache.org/licenses/LICENSE-2.0
243+#
244+# Unless required by applicable law or agreed to in writing, software
245+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
246+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
247+# License for the specific language governing permissions and limitations
248+# under the License.
249+
250+from sqlalchemy import *
251+from migrate import *
252+
253+from nova import log as logging
254+
255+
256+meta = MetaData()
257+
258+
259+networks = Table('networks', meta,
260+ Column('id', Integer(), primary_key=True, nullable=False),
261+ )
262+
263+
264+#
265+# New Tables
266+#
267+
268+
269+#
270+# Tables to alter
271+#
272+
273+networks_label = Column(
274+ 'label',
275+ String(length=255, convert_unicode=False, assert_unicode=None,
276+ unicode_error=None, _warn_on_bytestring=False))
277+
278+
279+def upgrade(migrate_engine):
280+ # Upgrade operations go here. Don't create your own engine;
281+ # bind migrate_engine to your metadata
282+ meta.bind = migrate_engine
283+ networks.create_column(networks_label)
284
285=== renamed file 'nova/db/sqlalchemy/migrate_repo/versions/003_add_label_to_networks.py' => 'nova/db/sqlalchemy/migrate_repo/versions/003_add_label_to_networks.py.moved'
286=== added file 'nova/db/sqlalchemy/migrate_repo/versions/004_add_instance_metadata.py'
287--- nova/db/sqlalchemy/migrate_repo/versions/004_add_instance_metadata.py 1970-01-01 00:00:00 +0000
288+++ nova/db/sqlalchemy/migrate_repo/versions/004_add_instance_metadata.py 2011-02-17 22:32:15 +0000
289@@ -0,0 +1,78 @@
290+# vim: tabstop=4 shiftwidth=4 softtabstop=4
291+
292+# Copyright 2011 Justin Santa Barbara
293+# All Rights Reserved.
294+#
295+# Licensed under the Apache License, Version 2.0 (the "License"); you may
296+# not use this file except in compliance with the License. You may obtain
297+# a copy of the License at
298+#
299+# http://www.apache.org/licenses/LICENSE-2.0
300+#
301+# Unless required by applicable law or agreed to in writing, software
302+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
303+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
304+# License for the specific language governing permissions and limitations
305+# under the License.
306+
307+from sqlalchemy import *
308+from migrate import *
309+
310+from nova import log as logging
311+
312+
313+meta = MetaData()
314+
315+
316+# Just for the ForeignKey and column creation to succeed, these are not the
317+# actual definitions of instances or services.
318+instances = Table('instances', meta,
319+ Column('id', Integer(), primary_key=True, nullable=False),
320+ )
321+
322+quotas = Table('quotas', meta,
323+ Column('id', Integer(), primary_key=True, nullable=False),
324+ )
325+
326+
327+#
328+# New Tables
329+#
330+
331+instance_metadata_table = Table('instance_metadata', meta,
332+ Column('created_at', DateTime(timezone=False)),
333+ Column('updated_at', DateTime(timezone=False)),
334+ Column('deleted_at', DateTime(timezone=False)),
335+ Column('deleted', Boolean(create_constraint=True, name=None)),
336+ Column('id', Integer(), primary_key=True, nullable=False),
337+ Column('instance_id',
338+ Integer(),
339+ ForeignKey('instances.id'),
340+ nullable=False),
341+ Column('key',
342+ String(length=255, convert_unicode=False, assert_unicode=None,
343+ unicode_error=None, _warn_on_bytestring=False)),
344+ Column('value',
345+ String(length=255, convert_unicode=False, assert_unicode=None,
346+ unicode_error=None, _warn_on_bytestring=False)))
347+
348+
349+#
350+# New columns
351+#
352+quota_metadata_items = Column('metadata_items', Integer())
353+
354+
355+def upgrade(migrate_engine):
356+ # Upgrade operations go here. Don't create your own engine;
357+ # bind migrate_engine to your metadata
358+ meta.bind = migrate_engine
359+ for table in (instance_metadata_table, ):
360+ try:
361+ table.create()
362+ except Exception:
363+ logging.info(repr(table))
364+ logging.exception('Exception while creating table')
365+ raise
366+
367+ quotas.create_column(quota_metadata_items)
368
369=== modified file 'nova/db/sqlalchemy/migration.py'
370=== modified file 'nova/db/sqlalchemy/models.py'
371--- nova/db/sqlalchemy/models.py 2011-02-17 21:39:03 +0000
372+++ nova/db/sqlalchemy/models.py 2011-02-17 22:32:15 +0000
373@@ -256,6 +256,7 @@
374 volumes = Column(Integer)
375 gigabytes = Column(Integer)
376 floating_ips = Column(Integer)
377+ metadata_items = Column(Integer)
378
379
380 class ExportDevice(BASE, NovaBase):
381@@ -536,6 +537,7 @@
382 pool = relationship(ConsolePool, backref=backref('consoles'))
383
384
385+<<<<<<< TREE
386 class Zone(BASE, NovaBase):
387 """Represents a child zone of this zone."""
388 __tablename__ = 'zones'
389@@ -545,6 +547,22 @@
390 password = Column(String(255))
391
392
393+=======
394+class InstanceMetadata(BASE, NovaBase):
395+ """Represents a metadata key/value pair for an instance"""
396+ __tablename__ = 'instance_metadata'
397+ id = Column(Integer, primary_key=True)
398+ key = Column(String(255))
399+ value = Column(String(255))
400+ instance_id = Column(Integer, ForeignKey('instances.id'), nullable=False)
401+ instance = relationship(Instance, backref="metadata",
402+ foreign_keys=instance_id,
403+ primaryjoin='and_('
404+ 'InstanceMetadata.instance_id == Instance.id,'
405+ 'InstanceMetadata.deleted == False)')
406+
407+
408+>>>>>>> MERGE-SOURCE
409 def register_models():
410 """Register Models and create metadata.
411
412@@ -557,7 +575,13 @@
413 Volume, ExportDevice, IscsiTarget, FixedIp, FloatingIp,
414 Network, SecurityGroup, SecurityGroupIngressRule,
415 SecurityGroupInstanceAssociation, AuthToken, User,
416+<<<<<<< TREE
417 Project, Certificate, ConsolePool, Console, Zone)
418+=======
419+ Project, Certificate, ConsolePool, Console,
420+ InstanceMetadata)
421+ # , Image, Host
422+>>>>>>> MERGE-SOURCE
423 engine = create_engine(FLAGS.sql_connection, echo=False)
424 for model in models:
425 model.metadata.create_all(engine)
426
427=== modified file 'nova/network/manager.py'
428=== modified file 'nova/quota.py'
429--- nova/quota.py 2011-01-04 21:56:36 +0000
430+++ nova/quota.py 2011-02-17 22:32:15 +0000
431@@ -35,6 +35,8 @@
432 'number of volume gigabytes allowed per project')
433 flags.DEFINE_integer('quota_floating_ips', 10,
434 'number of floating ips allowed per project')
435+flags.DEFINE_integer('quota_metadata_items', 128,
436+ 'number of metadata items allowed per instance')
437
438
439 def get_quota(context, project_id):
440@@ -42,7 +44,8 @@
441 'cores': FLAGS.quota_cores,
442 'volumes': FLAGS.quota_volumes,
443 'gigabytes': FLAGS.quota_gigabytes,
444- 'floating_ips': FLAGS.quota_floating_ips}
445+ 'floating_ips': FLAGS.quota_floating_ips,
446+ 'metadata_items': FLAGS.quota_metadata_items}
447 try:
448 quota = db.quota_get(context, project_id)
449 for key in rval.keys():
450@@ -94,6 +97,15 @@
451 return min(num_floating_ips, allowed_floating_ips)
452
453
454+def allowed_metadata_items(context, num_metadata_items):
455+ """Check quota; return min(num_metadata_items,allowed_metadata_items)"""
456+ project_id = context.project_id
457+ context = context.elevated()
458+ quota = get_quota(context, project_id)
459+ num_allowed_metadata_items = quota['metadata_items']
460+ return min(num_metadata_items, num_allowed_metadata_items)
461+
462+
463 class QuotaError(exception.ApiError):
464 """Quota Exceeeded"""
465 pass
466
467=== added file 'nova/tests/api/openstack/test_common.py'
468--- nova/tests/api/openstack/test_common.py 1970-01-01 00:00:00 +0000
469+++ nova/tests/api/openstack/test_common.py 2011-02-17 22:32:15 +0000
470@@ -0,0 +1,161 @@
471+# vim: tabstop=4 shiftwidth=4 softtabstop=4
472+
473+# Copyright 2010 OpenStack LLC.
474+# All Rights Reserved.
475+#
476+# Licensed under the Apache License, Version 2.0 (the "License"); you may
477+# not use this file except in compliance with the License. You may obtain
478+# a copy of the License at
479+#
480+# http://www.apache.org/licenses/LICENSE-2.0
481+#
482+# Unless required by applicable law or agreed to in writing, software
483+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
484+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
485+# License for the specific language governing permissions and limitations
486+# under the License.
487+
488+"""
489+Test suites for 'common' code used throughout the OpenStack HTTP API.
490+"""
491+
492+import unittest
493+
494+from webob import Request
495+
496+from nova.api.openstack.common import limited
497+
498+
499+class LimiterTest(unittest.TestCase):
500+ """
501+ Unit tests for the `nova.api.openstack.common.limited` method which takes
502+ in a list of items and, depending on the 'offset' and 'limit' GET params,
503+ returns a subset or complete set of the given items.
504+ """
505+
506+ def setUp(self):
507+ """
508+ Run before each test.
509+ """
510+ self.tiny = range(1)
511+ self.small = range(10)
512+ self.medium = range(1000)
513+ self.large = range(10000)
514+
515+ def test_limiter_offset_zero(self):
516+ """
517+ Test offset key works with 0.
518+ """
519+ req = Request.blank('/?offset=0')
520+ self.assertEqual(limited(self.tiny, req), self.tiny)
521+ self.assertEqual(limited(self.small, req), self.small)
522+ self.assertEqual(limited(self.medium, req), self.medium)
523+ self.assertEqual(limited(self.large, req), self.large[:1000])
524+
525+ def test_limiter_offset_medium(self):
526+ """
527+ Test offset key works with a medium sized number.
528+ """
529+ req = Request.blank('/?offset=10')
530+ self.assertEqual(limited(self.tiny, req), [])
531+ self.assertEqual(limited(self.small, req), self.small[10:])
532+ self.assertEqual(limited(self.medium, req), self.medium[10:])
533+ self.assertEqual(limited(self.large, req), self.large[10:1010])
534+
535+ def test_limiter_offset_over_max(self):
536+ """
537+ Test offset key works with a number over 1000 (max_limit).
538+ """
539+ req = Request.blank('/?offset=1001')
540+ self.assertEqual(limited(self.tiny, req), [])
541+ self.assertEqual(limited(self.small, req), [])
542+ self.assertEqual(limited(self.medium, req), [])
543+ self.assertEqual(limited(self.large, req), self.large[1001:2001])
544+
545+ def test_limiter_offset_blank(self):
546+ """
547+ Test offset key works with a blank offset.
548+ """
549+ req = Request.blank('/?offset=')
550+ self.assertEqual(limited(self.tiny, req), self.tiny)
551+ self.assertEqual(limited(self.small, req), self.small)
552+ self.assertEqual(limited(self.medium, req), self.medium)
553+ self.assertEqual(limited(self.large, req), self.large[:1000])
554+
555+ def test_limiter_offset_bad(self):
556+ """
557+ Test offset key works with a BAD offset.
558+ """
559+ req = Request.blank(u'/?offset=\u0020aa')
560+ self.assertEqual(limited(self.tiny, req), self.tiny)
561+ self.assertEqual(limited(self.small, req), self.small)
562+ self.assertEqual(limited(self.medium, req), self.medium)
563+ self.assertEqual(limited(self.large, req), self.large[:1000])
564+
565+ def test_limiter_nothing(self):
566+ """
567+ Test request with no offset or limit
568+ """
569+ req = Request.blank('/')
570+ self.assertEqual(limited(self.tiny, req), self.tiny)
571+ self.assertEqual(limited(self.small, req), self.small)
572+ self.assertEqual(limited(self.medium, req), self.medium)
573+ self.assertEqual(limited(self.large, req), self.large[:1000])
574+
575+ def test_limiter_limit_zero(self):
576+ """
577+ Test limit of zero.
578+ """
579+ req = Request.blank('/?limit=0')
580+ self.assertEqual(limited(self.tiny, req), self.tiny)
581+ self.assertEqual(limited(self.small, req), self.small)
582+ self.assertEqual(limited(self.medium, req), self.medium)
583+ self.assertEqual(limited(self.large, req), self.large[:1000])
584+
585+ def test_limiter_limit_medium(self):
586+ """
587+ Test limit of 10.
588+ """
589+ req = Request.blank('/?limit=10')
590+ self.assertEqual(limited(self.tiny, req), self.tiny)
591+ self.assertEqual(limited(self.small, req), self.small)
592+ self.assertEqual(limited(self.medium, req), self.medium[:10])
593+ self.assertEqual(limited(self.large, req), self.large[:10])
594+
595+ def test_limiter_limit_over_max(self):
596+ """
597+ Test limit of 3000.
598+ """
599+ req = Request.blank('/?limit=3000')
600+ self.assertEqual(limited(self.tiny, req), self.tiny)
601+ self.assertEqual(limited(self.small, req), self.small)
602+ self.assertEqual(limited(self.medium, req), self.medium)
603+ self.assertEqual(limited(self.large, req), self.large[:1000])
604+
605+ def test_limiter_limit_and_offset(self):
606+ """
607+ Test request with both limit and offset.
608+ """
609+ items = range(2000)
610+ req = Request.blank('/?offset=1&limit=3')
611+ self.assertEqual(limited(items, req), items[1:4])
612+ req = Request.blank('/?offset=3&limit=0')
613+ self.assertEqual(limited(items, req), items[3:1003])
614+ req = Request.blank('/?offset=3&limit=1500')
615+ self.assertEqual(limited(items, req), items[3:1003])
616+ req = Request.blank('/?offset=3000&limit=10')
617+ self.assertEqual(limited(items, req), [])
618+
619+ def test_limiter_custom_max_limit(self):
620+ """
621+ Test a max_limit other than 1000.
622+ """
623+ items = range(2000)
624+ req = Request.blank('/?offset=1&limit=3')
625+ self.assertEqual(limited(items, req, max_limit=2000), items[1:4])
626+ req = Request.blank('/?offset=3&limit=0')
627+ self.assertEqual(limited(items, req, max_limit=2000), items[3:])
628+ req = Request.blank('/?offset=3&limit=2500')
629+ self.assertEqual(limited(items, req, max_limit=2000), items[3:])
630+ req = Request.blank('/?offset=3000&limit=10')
631+ self.assertEqual(limited(items, req, max_limit=2000), [])
632
633=== renamed file 'nova/tests/api/openstack/test_common.py' => 'nova/tests/api/openstack/test_common.py.moved'
634=== modified file 'nova/tests/api/openstack/test_servers.py'
635--- nova/tests/api/openstack/test_servers.py 2011-02-17 19:38:11 +0000
636+++ nova/tests/api/openstack/test_servers.py 2011-02-17 22:32:15 +0000
637@@ -28,6 +28,7 @@
638 from nova.api.openstack import servers
639 import nova.db.api
640 from nova.db.sqlalchemy.models import Instance
641+from nova.db.sqlalchemy.models import InstanceMetadata
642 import nova.rpc
643 from nova.tests.api.openstack import fakes
644
645@@ -63,6 +64,7 @@
646 return None
647
648
649+<<<<<<< TREE
650 def stub_instance(id, user_id=1, private_address=None, public_addresses=None):
651 if public_addresses == None:
652 public_addresses = list()
653@@ -102,6 +104,51 @@
654 "floating_ips": [{"address":ip} for ip in public_addresses]}
655
656 return instance
657+=======
658+def stub_instance(id, user_id=1, private_address=None, public_addresses=None):
659+ metadata = []
660+ metadata.append(InstanceMetadata(key='seq', value=id))
661+
662+ if public_addresses == None:
663+ public_addresses = list()
664+
665+ instance = {
666+ "id": id,
667+ "admin_pass": "",
668+ "user_id": user_id,
669+ "project_id": "",
670+ "image_id": 10,
671+ "kernel_id": "",
672+ "ramdisk_id": "",
673+ "launch_index": 0,
674+ "key_name": "",
675+ "key_data": "",
676+ "state": 0,
677+ "state_description": "",
678+ "memory_mb": 0,
679+ "vcpus": 0,
680+ "local_gb": 0,
681+ "hostname": "",
682+ "host": "",
683+ "instance_type": "",
684+ "user_data": "",
685+ "reservation_id": "",
686+ "mac_address": "",
687+ "scheduled_at": datetime.datetime.now(),
688+ "launched_at": datetime.datetime.now(),
689+ "terminated_at": datetime.datetime.now(),
690+ "availability_zone": "",
691+ "display_name": "server%s" % id,
692+ "display_description": "",
693+ "locked": False,
694+ "metadata": metadata}
695+
696+ instance["fixed_ip"] = {
697+ "address": private_address,
698+ "floating_ips": [{"address":ip} for ip in public_addresses]}
699+
700+ return instance
701+>>>>>>> MERGE-SOURCE
702
703
704 def fake_compute_api(cls, req, id):
705@@ -214,7 +261,8 @@
706 "get_image_id_from_image_hash", image_id_from_hash)
707
708 body = dict(server=dict(
709- name='server_test', imageId=2, flavorId=2, metadata={},
710+ name='server_test', imageId=2, flavorId=2,
711+ metadata={'hello': 'world', 'open': 'stack'},
712 personality={}))
713 req = webob.Request.blank('/v1.0/servers')
714 req.method = 'POST'
715@@ -291,6 +339,7 @@
716 self.assertEqual(s['id'], i)
717 self.assertEqual(s['name'], 'server%d' % i)
718 self.assertEqual(s['imageId'], 10)
719+ self.assertEqual(s['metadata']['seq'], i)
720 i += 1
721
722 def test_server_pause(self):
723
724=== modified file 'nova/tests/test_quota.py'
725--- nova/tests/test_quota.py 2011-01-07 14:46:17 +0000
726+++ nova/tests/test_quota.py 2011-02-17 22:32:15 +0000
727@@ -87,6 +87,18 @@
728 num_instances = quota.allowed_instances(self.context, 100,
729 instance_types.INSTANCE_TYPES['m1.small'])
730 self.assertEqual(num_instances, 10)
731+
732+ # metadata_items
733+ too_many_items = FLAGS.quota_metadata_items + 1000
734+ num_metadata_items = quota.allowed_metadata_items(self.context,
735+ too_many_items)
736+ self.assertEqual(num_metadata_items, FLAGS.quota_metadata_items)
737+ db.quota_update(self.context, self.project.id, {'metadata_items': 5})
738+ num_metadata_items = quota.allowed_metadata_items(self.context,
739+ too_many_items)
740+ self.assertEqual(num_metadata_items, 5)
741+
742+ # Cleanup
743 db.quota_destroy(self.context, self.project.id)
744
745 def test_too_many_instances(self):
746@@ -151,3 +163,15 @@
747 self.assertRaises(quota.QuotaError, self.cloud.allocate_address,
748 self.context)
749 db.floating_ip_destroy(context.get_admin_context(), address)
750+
751+ def test_too_many_metadata_items(self):
752+ metadata = {}
753+ for i in range(FLAGS.quota_metadata_items + 1):
754+ metadata['key%s' % i] = 'value%s' % i
755+ self.assertRaises(quota.QuotaError, self.cloud.run_instances,
756+ self.context,
757+ min_count=1,
758+ max_count=1,
759+ instance_type='m1.small',
760+ image_id='fake',
761+ metadata=metadata)
762
763=== modified file 'run_tests.sh'
764--- run_tests.sh 2011-01-31 05:55:32 +0000
765+++ run_tests.sh 2011-02-17 22:32:15 +0000
766@@ -73,7 +73,9 @@
767
768 if [ -z "$noseargs" ];
769 then
770- run_tests && pep8 --repeat --show-pep8 --show-source --exclude=vcsversion.py bin/* nova setup.py || exit 1
771+ srcfiles=`find bin -type f ! -name "nova.conf*"`
772+ srcfiles+=" nova setup.py"
773+ run_tests && pep8 --repeat --show-pep8 --show-source --exclude=vcsversion.py ${srcfiles} || exit 1
774 else
775 run_tests
776 fi