Merge lp:~justin-fathomdb/nova/justinsb-metadata into lp:~hudson-openstack/nova/trunk
- justinsb-metadata
- Merge into trunk
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 |
Related bugs: |
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.
Commit message
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.
Dan Prince (dan-prince) wrote : Posted in a previous version of this proposal | # |
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:/
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_
Cheers!
jay
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?
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
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."
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>
Devin Carlen (devcamcar) wrote : Posted in a previous version of this proposal | # |
Agree, make the number of metadata items configurable
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?
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.
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.
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?
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:/
> You are subscribed to branch lp:nova.
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.
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/
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
Todd Willey (xtoddx) wrote : Posted in a previous version of this proposal | # |
lgtm
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
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 |
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