From 220ddb2bd5413ea5fa2bff450f4fb3aba136e909 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 16 Jun 2011 20:29:10 +0900 Subject: api/ec2: check user permission for start/stop instances This patch adds precise permission check for start/stop instances. --- nova/api/ec2/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py index 890d57fe7..857fa96c3 100644 --- a/nova/api/ec2/__init__.py +++ b/nova/api/ec2/__init__.py @@ -262,6 +262,8 @@ class Authorizer(wsgi.Middleware): 'TerminateInstances': ['projectmanager', 'sysadmin'], 'RebootInstances': ['projectmanager', 'sysadmin'], 'UpdateInstance': ['projectmanager', 'sysadmin'], + 'StartInstances': ['projectmanager', 'sysadmin'], + 'StopInstances': ['projectmanager', 'sysadmin'], 'DeleteVolume': ['projectmanager', 'sysadmin'], 'DescribeImages': ['all'], 'DeregisterImage': ['projectmanager', 'sysadmin'], -- cgit From 009874210e818752d5f206df01313242c728e5f8 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 16 Jun 2011 20:29:10 +0900 Subject: api/ec2: check user permission for start/stop instances This patch adds precise permission check for start/stop instances. --- nova/api/ec2/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py index 1915d007d..b8f0dd576 100644 --- a/nova/api/ec2/__init__.py +++ b/nova/api/ec2/__init__.py @@ -261,6 +261,8 @@ class Authorizer(wsgi.Middleware): 'TerminateInstances': ['projectmanager', 'sysadmin'], 'RebootInstances': ['projectmanager', 'sysadmin'], 'UpdateInstance': ['projectmanager', 'sysadmin'], + 'StartInstances': ['projectmanager', 'sysadmin'], + 'StopInstances': ['projectmanager', 'sysadmin'], 'DeleteVolume': ['projectmanager', 'sysadmin'], 'DescribeImages': ['all'], 'DeregisterImage': ['projectmanager', 'sysadmin'], -- cgit From df63c8e14da8c93453d4d7485829e38e0db30711 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 16 Jun 2011 20:35:49 +0900 Subject: ec2utils: consolidate 'vol-%08x' and 'snap-%08x' By introducing helper functions, consolidate scattered '{vol, snap}-%08x' --- nova/api/ec2/__init__.py | 4 ++-- nova/api/ec2/cloud.py | 14 ++++++-------- nova/api/ec2/ec2utils.py | 11 +++++++++++ nova/tests/test_cloud.py | 16 ++++++++-------- 4 files changed, 27 insertions(+), 18 deletions(-) diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py index 857fa96c3..35f67d39b 100644 --- a/nova/api/ec2/__init__.py +++ b/nova/api/ec2/__init__.py @@ -327,13 +327,13 @@ class Executor(wsgi.Application): except exception.VolumeNotFound as ex: LOG.info(_('VolumeNotFound raised: %s'), unicode(ex), context=context) - ec2_id = ec2utils.id_to_ec2_id(ex.volume_id, 'vol-%08x') + ec2_id = ec2utils.id_to_ec2_vol_id(ex.volume_id) message = _('Volume %s not found') % ec2_id return self._error(req, context, type(ex).__name__, message) except exception.SnapshotNotFound as ex: LOG.info(_('SnapshotNotFound raised: %s'), unicode(ex), context=context) - ec2_id = ec2utils.id_to_ec2_id(ex.snapshot_id, 'snap-%08x') + ec2_id = ec2utils.id_to_ec2_snap_id(ex.snapshot_id) message = _('Snapshot %s not found') % ec2_id return self._error(req, context, type(ex).__name__, message) except exception.NotFound as ex: diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index a59173103..d36eb7a04 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -305,9 +305,8 @@ class CloudController(object): def _format_snapshot(self, context, snapshot): s = {} - s['snapshotId'] = ec2utils.id_to_ec2_id(snapshot['id'], 'snap-%08x') - s['volumeId'] = ec2utils.id_to_ec2_id(snapshot['volume_id'], - 'vol-%08x') + s['snapshotId'] = ec2utils.id_to_ec2_snap_id(snapshot['id']) + s['volumeId'] = ec2utils.id_to_ec2_vol_id(snapshot['volume_id']) s['status'] = snapshot['status'] s['startTime'] = snapshot['created_at'] s['progress'] = snapshot['progress'] @@ -641,7 +640,7 @@ class CloudController(object): instance_data = '%s[%s]' % (instance_ec2_id, volume['instance']['host']) v = {} - v['volumeId'] = ec2utils.id_to_ec2_id(volume['id'], 'vol-%08x') + v['volumeId'] = ec2utils.id_to_ec2_vol_id(volume['id']) v['status'] = volume['status'] v['size'] = volume['size'] v['availabilityZone'] = volume['availability_zone'] @@ -663,8 +662,7 @@ class CloudController(object): else: v['attachmentSet'] = [{}] if volume.get('snapshot_id') != None: - v['snapshotId'] = ec2utils.id_to_ec2_id(volume['snapshot_id'], - 'snap-%08x') + v['snapshotId'] = ec2utils.id_to_ec2_snap_id(volume['snapshot_id']) else: v['snapshotId'] = None @@ -727,7 +725,7 @@ class CloudController(object): 'instanceId': ec2utils.id_to_ec2_id(instance_id), 'requestId': context.request_id, 'status': volume['attach_status'], - 'volumeId': ec2utils.id_to_ec2_id(volume_id, 'vol-%08x')} + 'volumeId': ec2utils.id_to_ec2_vol_id(volume_id)} def detach_volume(self, context, volume_id, **kwargs): volume_id = ec2utils.ec2_id_to_id(volume_id) @@ -739,7 +737,7 @@ class CloudController(object): 'instanceId': ec2utils.id_to_ec2_id(instance['id']), 'requestId': context.request_id, 'status': volume['attach_status'], - 'volumeId': ec2utils.id_to_ec2_id(volume_id, 'vol-%08x')} + 'volumeId': ec2utils.id_to_ec2_vol_id(volume_id)} def _convert_to_set(self, lst, label): if lst is None or lst == []: diff --git a/nova/api/ec2/ec2utils.py b/nova/api/ec2/ec2utils.py index 222e1de1e..bcdf2ba78 100644 --- a/nova/api/ec2/ec2utils.py +++ b/nova/api/ec2/ec2utils.py @@ -34,6 +34,17 @@ def id_to_ec2_id(instance_id, template='i-%08x'): return template % instance_id +def id_to_ec2_snap_id(instance_id): + """Convert an snapshot ID (int) to an ec2 snapshot ID + (snap-[base 16 number])""" + return id_to_ec2_id(instance_id, 'snap-%08x') + + +def id_to_ec2_vol_id(instance_id): + """Convert an volume ID (int) to an ec2 volume ID (vol-[base 16 number])""" + return id_to_ec2_id(instance_id, 'vol-%08x') + + _c2u = re.compile('(((?<=[a-z])[A-Z])|([A-Z](?![A-Z]|$)))') diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 6a6256c20..2efcd304d 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -171,7 +171,7 @@ class CloudTestCase(test.TestCase): vol2 = db.volume_create(self.context, {}) result = self.cloud.describe_volumes(self.context) self.assertEqual(len(result['volumeSet']), 2) - volume_id = ec2utils.id_to_ec2_id(vol2['id'], 'vol-%08x') + volume_id = ec2utils.id_to_ec2_vol_id(vol2['id']) result = self.cloud.describe_volumes(self.context, volume_id=[volume_id]) self.assertEqual(len(result['volumeSet']), 1) @@ -187,7 +187,7 @@ class CloudTestCase(test.TestCase): snap = db.snapshot_create(self.context, {'volume_id': vol['id'], 'volume_size': vol['size'], 'status': "available"}) - snapshot_id = ec2utils.id_to_ec2_id(snap['id'], 'snap-%08x') + snapshot_id = ec2utils.id_to_ec2_snap_id(snap['id']) result = self.cloud.create_volume(self.context, snapshot_id=snapshot_id) @@ -224,7 +224,7 @@ class CloudTestCase(test.TestCase): snap2 = db.snapshot_create(self.context, {'volume_id': vol['id']}) result = self.cloud.describe_snapshots(self.context) self.assertEqual(len(result['snapshotSet']), 2) - snapshot_id = ec2utils.id_to_ec2_id(snap2['id'], 'snap-%08x') + snapshot_id = ec2utils.id_to_ec2_snap_id(snap2['id']) result = self.cloud.describe_snapshots(self.context, snapshot_id=[snapshot_id]) self.assertEqual(len(result['snapshotSet']), 1) @@ -238,7 +238,7 @@ class CloudTestCase(test.TestCase): def test_create_snapshot(self): """Makes sure create_snapshot works.""" vol = db.volume_create(self.context, {'status': "available"}) - volume_id = ec2utils.id_to_ec2_id(vol['id'], 'vol-%08x') + volume_id = ec2utils.id_to_ec2_vol_id(vol['id']) result = self.cloud.create_snapshot(self.context, volume_id=volume_id) @@ -255,7 +255,7 @@ class CloudTestCase(test.TestCase): vol = db.volume_create(self.context, {'status': "available"}) snap = db.snapshot_create(self.context, {'volume_id': vol['id'], 'status': "available"}) - snapshot_id = ec2utils.id_to_ec2_id(snap['id'], 'snap-%08x') + snapshot_id = ec2utils.id_to_ec2_snap_id(snap['id']) result = self.cloud.delete_snapshot(self.context, snapshot_id=snapshot_id) @@ -553,7 +553,7 @@ class CloudTestCase(test.TestCase): def test_update_of_volume_display_fields(self): vol = db.volume_create(self.context, {}) self.cloud.update_volume(self.context, - ec2utils.id_to_ec2_id(vol['id'], 'vol-%08x'), + ec2utils.id_to_ec2_vol_id(vol['id']), display_name='c00l v0lum3') vol = db.volume_get(self.context, vol['id']) self.assertEqual('c00l v0lum3', vol['display_name']) @@ -562,7 +562,7 @@ class CloudTestCase(test.TestCase): def test_update_of_volume_wont_update_private_fields(self): vol = db.volume_create(self.context, {}) self.cloud.update_volume(self.context, - ec2utils.id_to_ec2_id(vol['id'], 'vol-%08x'), + ec2utils.id_to_ec2_vol_id(vol['id']), mountpoint='/not/here') vol = db.volume_get(self.context, vol['id']) self.assertEqual(None, vol['mountpoint']) @@ -804,7 +804,7 @@ class CloudTestCase(test.TestCase): def test_run_with_snapshot(self): """Makes sure run/stop/start instance with snapshot works.""" vol = self._volume_create() - ec2_volume_id = ec2utils.id_to_ec2_id(vol['id'], 'vol-%08x') + ec2_volume_id = ec2utils.id_to_ec2_vol_id(vol['id']) ec2_snapshot1_id = self._create_snapshot(ec2_volume_id) snapshot1_id = ec2utils.ec2_id_to_id(ec2_snapshot1_id) -- cgit From 72730a3ba122a86f736513b9dab886bc4087bdc5 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 16 Jun 2011 21:25:02 +0900 Subject: db/model: add root_device_name column to instances table The root_device_name column is necessary to support ec2 RootDeviceName. --- .../versions/022_add_root_device_name.py | 47 ++++++++++++++++++++++ nova/db/sqlalchemy/models.py | 2 + 2 files changed, 49 insertions(+) create mode 100644 nova/db/sqlalchemy/migrate_repo/versions/022_add_root_device_name.py diff --git a/nova/db/sqlalchemy/migrate_repo/versions/022_add_root_device_name.py b/nova/db/sqlalchemy/migrate_repo/versions/022_add_root_device_name.py new file mode 100644 index 000000000..6b98b9890 --- /dev/null +++ b/nova/db/sqlalchemy/migrate_repo/versions/022_add_root_device_name.py @@ -0,0 +1,47 @@ +# Copyright 2011 OpenStack LLC. +# Copyright 2011 Isaku Yamahata +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from sqlalchemy import Column, Integer, MetaData, Table, String + +meta = MetaData() + + +# Just for the ForeignKey and column creation to succeed, these are not the +# actual definitions of instances or services. +instances = Table('instances', meta, + Column('id', Integer(), primary_key=True, nullable=False), + ) + +# +# New Column +# +root_device_name = Column( + 'root_device_name', + String(length=255, convert_unicode=False, assert_unicode=None, + unicode_error=None, _warn_on_bytestring=False), + nullable=True) + + +def upgrade(migrate_engine): + # Upgrade operations go here. Don't create your own engine; + # bind migrate_engine to your metadata + meta.bind = migrate_engine + instances.create_column(root_device_name) + + +def downgrade(migrate_engine): + # Operations to reverse the above upgrade go here. + meta.bind = migrate_engine + instances.drop_column('root_device_name') diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 23e1bd112..62293daba 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -234,6 +234,8 @@ class Instance(BASE, NovaBase): os_type = Column(String(255)) vm_mode = Column(String(255)) + root_device_name = Column(String(255)) + # TODO(vish): see Ewan's email about state improvements, probably # should be in a driver base class or some such # vmstate_state = running, halted, suspended, paused -- cgit From 1a54498af5bc2b81bf8bf6e3b9a4ad4cc2db79e2 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 16 Jun 2011 21:25:35 +0900 Subject: api/ec2/image: support block device mapping This patch adds --block-device-mapping support for register image and instance creation. --- nova/api/ec2/cloud.py | 189 ++++++++++++++++++++++++++++++++++++++++------- nova/api/ec2/ec2utils.py | 19 +++++ nova/compute/api.py | 82 +++++++++++++++----- nova/compute/manager.py | 20 ++--- nova/image/s3.py | 25 +++++++ 5 files changed, 282 insertions(+), 53 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index d36eb7a04..1e594cd73 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -27,6 +27,7 @@ import IPy import os import urllib import tempfile +import time import shutil from nova import compute @@ -75,6 +76,73 @@ def _gen_key(context, user_id, key_name): return {'private_key': private_key, 'fingerprint': fingerprint} +# TODO(yamahata): hypervisor dependent default device name +_DEFAULT_ROOT_DEVICE_NAME = '/dev/sda1' + + +def _parse_block_device_mapping(bdm): + """Parse BlockDeviceMappingItemType into flat hash + BlockDevicedMapping..DeviceName + BlockDevicedMapping..Ebs.SnapshotId + BlockDevicedMapping..Ebs.VolumeSize + BlockDevicedMapping..Ebs.DeleteOnTermination + BlockDevicedMapping..Ebs.NoDevice + BlockDevicedMapping..VirtualName + => remove .Ebs and allow volume id in SnapshotId + """ + ebs = bdm.pop('ebs', None) + if ebs: + ec2_id = ebs.pop('snapshot_id') + id = ec2utils.ec2_id_to_id(ec2_id) + if ec2_id.startswith('snap-'): + bdm['snapshot_id'] = id + elif ec2_id.startswith('vol-'): + bdm['volume_id'] = id + ebs.setdefault('delete_on_termination', True) + bdm.update(ebs) + return bdm + + +def _format_block_device_mapping(bdm): + """Contruct BlockDeviceMappingItemType + {'device_name': '...', 'Snapshot_Id': , ...} + => BlockDeviceMappingItemType + """ + keys = (('deviceName', 'device_name'), + ('virtualName', 'virtual_name')) + item = {} + for name, k in keys: + if k in bdm: + item[name] = bdm[k] + if bdm.get('no_device'): + item['noDevice'] = True + if ('snapshot_id' in bdm) or ('volume_id' in bdm): + ebs_keys = (('snapshotId', 'snapshot_id'), + ('snapshotId', 'volume_id'), # snapshotId is abused + ('volumeSize', 'volume_size'), + ('deleteOnTermination', 'delete_on_termination')) + ebs = {} + for name, k in ebs_keys: + if k in bdm: + if k == 'snapshot_id': + ebs[name] = ec2utils.id_to_ec2_snap_id(bdm[k]) + elif k == 'volume_id': + ebs[name] = ec2utils.id_to_ec2_vol_id(bdm[k]) + else: + ebs[name] = bdm[k] + assert 'snapshotId' in ebs + item['ebs'] = ebs + return item + + +def _format_mappings(mappings, result): + """Format multiple BlockDeviceMappingItemType""" + block_device_mapping = [_format_block_device_mapping(bdm) for bdm in + mappings] + if block_device_mapping: + result['blockDeviceMapping'] = block_device_mapping + + class CloudController(object): """ CloudController provides the critical dispatch between inbound API calls through the endpoint and messages @@ -177,7 +245,7 @@ class CloudController(object): # TODO(vish): replace with real data 'ami': 'sda1', 'ephemeral0': 'sda2', - 'root': '/dev/sda1', + 'root': _DEFAULT_ROOT_DEVICE_NAME, 'swap': 'sda3'}, 'hostname': hostname, 'instance-action': 'none', @@ -761,6 +829,37 @@ class CloudController(object): assert len(i) == 1 return i[0] + def _format_instance_bdm(self, context, instance_id, root_device_name, + result): + """Format InstanceBlockDeviceMappingResponseItemType""" + root_device_type = 'instance-store' + mapping = [] + for bdm in db.block_device_mapping_get_all_by_instance(context, + instance_id): + volume_id = bdm['volume_id'] + if (volume_id is None or bdm['no_device']): + continue + + if (bdm['device_name'] == root_device_name and + (bdm['snapshot_id'] or bdm['volume_id'])): + assert not bdm['virtual_name'] + root_device_type = 'ebs' + + vol = self.volume_api.get(context, volume_id=volume_id) + LOG.debug(_("vol = %s\n"), vol) + # TODO(yamahata): volume attach time + ebs = {'volumeId': volume_id, + 'deleteOnTermination': bdm['delete_on_termination'], + 'attachTime': vol['attach_time'] or '-', + 'status': vol['status'], } + res = {'deviceName': bdm['device_name'], + 'ebs': ebs, } + mapping.append(res) + + if mapping: + result['blockDeviceMapping'] = mapping + result['rootDeviceType'] = root_device_type + def _format_instances(self, context, instance_id=None, **kwargs): # TODO(termie): this method is poorly named as its name does not imply # that it will be making a variety of database calls @@ -822,6 +921,10 @@ class CloudController(object): i['amiLaunchIndex'] = instance['launch_index'] i['displayName'] = instance['display_name'] i['displayDescription'] = instance['display_description'] + i['rootDeviceName'] = (instance['root_device_name'] or + _DEFAULT_ROOT_DEVICE_NAME) + self._format_instance_bdm(context, instance_id, + i['rootDeviceName'], i) host = instance['host'] zone = self._get_availability_zone_by_host(context, host) i['placement'] = {'availabilityZone': zone} @@ -907,24 +1010,7 @@ class CloudController(object): if kwargs.get('ramdisk_id'): ramdisk = self._get_image(context, kwargs['ramdisk_id']) kwargs['ramdisk_id'] = ramdisk['id'] - for bdm in kwargs.get('block_device_mapping', []): - # NOTE(yamahata) - # BlockDevicedMapping..DeviceName - # BlockDevicedMapping..Ebs.SnapshotId - # BlockDevicedMapping..Ebs.VolumeSize - # BlockDevicedMapping..Ebs.DeleteOnTermination - # BlockDevicedMapping..VirtualName - # => remove .Ebs and allow volume id in SnapshotId - ebs = bdm.pop('ebs', None) - if ebs: - ec2_id = ebs.pop('snapshot_id') - id = ec2utils.ec2_id_to_id(ec2_id) - if ec2_id.startswith('snap-'): - bdm['snapshot_id'] = id - elif ec2_id.startswith('vol-'): - bdm['volume_id'] = id - ebs.setdefault('delete_on_termination', True) - bdm.update(ebs) + _parse_block_device_mapping(bdm) image = self._get_image(context, kwargs['image_id']) @@ -1078,6 +1164,20 @@ class CloudController(object): i['imageType'] = display_mapping.get(image_type) i['isPublic'] = image.get('is_public') == True i['architecture'] = image['properties'].get('architecture') + + properties = image['properties'] + root_device_name = ec2utils.properties_root_device_name(properties) + root_device_type = 'instance-store' + for bdm in properties.get('block_device_mapping', []): + if (bdm.get('device_name') == root_device_name and + ('snapshot_id' in bdm or 'volume_id' in bdm) and + not bdm.get('no_device')): + root_device_type = 'ebs' + i['rootDeviceName'] = (root_device_name or _DEFAULT_ROOT_DEVICE_NAME) + i['rootDeviceType'] = root_device_type + + _format_mappings(properties.get('block_device_mapping', []), i) + return i def describe_images(self, context, image_id=None, **kwargs): @@ -1102,30 +1202,65 @@ class CloudController(object): self.image_service.delete(context, internal_id) return {'imageId': image_id} + def _register_image(self, context, metadata): + image = self.image_service.create(context, metadata) + image_type = self._image_type(image.get('container_format')) + image_id = self.image_ec2_id(image['id'], image_type) + return image_id + def register_image(self, context, image_location=None, **kwargs): if image_location is None and 'name' in kwargs: image_location = kwargs['name'] metadata = {'properties': {'image_location': image_location}} - image = self.image_service.create(context, metadata) - image_type = self._image_type(image.get('container_format')) - image_id = self.image_ec2_id(image['id'], - image_type) + + if 'root_device_name' in kwargs: + metadata['properties']['root_device_name'] = \ + kwargs.get('root_device_name') + + mappings = [_parse_block_device_mapping(bdm) for bdm in + kwargs.get('block_device_mapping', [])] + if mappings: + metadata['properties']['block_device_mapping'] = mappings + + image_id = self._register_image(context, metadata) msg = _("Registered image %(image_location)s with" " id %(image_id)s") % locals() LOG.audit(msg, context=context) return {'imageId': image_id} def describe_image_attribute(self, context, image_id, attribute, **kwargs): - if attribute != 'launchPermission': + def _block_device_mapping_attribute(image, result): + _format_mappings( + image['properties'].get('block_device_mapping', []), result) + + def _launch_permission_attribute(image, result): + result['launchPermission'] = [] + if image['is_public']: + result['launchPermission'].append({'group': 'all'}) + + def _root_device_name_attribute(image, result): + result['rootDeviceName'] = \ + ec2utils.properties_root_device_name(image['properties']) + if result['rootDeviceName'] is None: + result['rootDeviceName'] = _DEFAULT_ROOT_DEVICE_NAME + + supported_attributes = { + 'blockDeviceMapping': _block_device_mapping_attribute, + 'launchPermission': _launch_permission_attribute, + 'rootDeviceName': _root_device_name_attribute, + } + + fn = supported_attributes.get(attribute) + if fn is None: raise exception.ApiError(_('attribute not supported: %s') % attribute) try: image = self._get_image(context, image_id) except exception.NotFound: raise exception.ImageNotFound(image_id=image_id) - result = {'imageId': image_id, 'launchPermission': []} - if image['is_public']: - result['launchPermission'].append({'group': 'all'}) + + result = {'imageId': image_id} + fn(image, result) return result def modify_image_attribute(self, context, image_id, attribute, diff --git a/nova/api/ec2/ec2utils.py b/nova/api/ec2/ec2utils.py index bcdf2ba78..9839f8604 100644 --- a/nova/api/ec2/ec2utils.py +++ b/nova/api/ec2/ec2utils.py @@ -135,3 +135,22 @@ def dict_from_dotted_str(items): args[key] = value return args + + +def properties_root_device_name(properties): + """get root device name from image meta data. + If it isn't specified, return None. + """ + root_device_name = None + + # NOTE(yamahata): see image_service.s3.s3create() + for bdm in properties.get('mappings', []): + if bdm['virtual'] == 'root': + root_device_name = bdm['device'] + + # NOTE(yamahata): register_image's command line can override + # .manifest.xml + if 'root_device_name' in properties: + root_device_name = properties['root_device_name'] + + return root_device_name diff --git a/nova/compute/api.py b/nova/compute/api.py index 18363ace0..d89dfb746 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -32,6 +32,7 @@ from nova import quota from nova import rpc from nova import utils from nova import volume +from nova.api.ec2 import ec2utils from nova.compute import instance_types from nova.compute import power_state from nova.compute.utils import terminate_volumes @@ -220,6 +221,9 @@ class API(base.Base): if reservation_id is None: reservation_id = utils.generate_uid('r') + root_device_name = ec2utils.properties_root_device_name( + image['properties']) + base_options = { 'reservation_id': reservation_id, 'image_ref': image_href, @@ -243,10 +247,62 @@ class API(base.Base): 'metadata': metadata, 'availability_zone': availability_zone, 'os_type': os_type, - 'vm_mode': vm_mode} + 'vm_mode': vm_mode, + 'root_device_name': root_device_name} return (num_instances, base_options, security_groups) + def _update_image_block_device_mapping(self, elevated_context, instance_id, + mappings): + """tell vm driver to create ephemeral/swap device at boot time by + updating BlockDeviceMapping + """ + for bdm in mappings: + LOG.debug(_("bdm %s"), bdm) + + virtual_name = bdm['virtualName'] + if virtual_name == 'ami' or virtual_name == 'root': + continue + + assert (virtual_name == 'swap' or + virtual_name.startswith('ephemeral')) + values = { + 'instance_id': instance_id, + 'device_name': bdm['deviceName'], + 'virtual_name': virtual_name, } + self.db.block_device_mapping_create(elevated_context, values) + + def _update_block_device_mapping(self, elevated_context, instance_id, + block_device_mapping): + """tell vm driver to attach volume at boot time by updating + BlockDeviceMapping + """ + for bdm in block_device_mapping: + LOG.debug(_('bdm %s'), bdm) + assert 'device_name' in bdm + + values = { + 'instance_id': instance_id, + 'device_name': bdm['device_name'], + 'delete_on_termination': bdm.get('delete_on_termination'), + 'virtual_name': bdm.get('virtual_name'), + 'snapshot_id': bdm.get('snapshot_id'), + 'volume_id': bdm.get('volume_id'), + 'volume_size': bdm.get('volume_size'), + 'no_device': bdm.get('no_device')} + + # NOTE(yamahata): NoDevice eliminates devices defined in image + # files by command line option. + # (--block-device-mapping) + if bdm.get('virtual_name') == 'NoDevice': + values['no_device'] = True + for k in ('delete_on_termination', 'volume_id', + 'snapshot_id', 'volume_id', 'volume_size', + 'virtual_name'): + values[k] = None + + self.db.block_device_mapping_create(elevated_context, values) + def create_db_entry_for_new_instance(self, context, base_options, security_groups, block_device_mapping, num=1): """Create an entry in the DB for this new instance, @@ -268,22 +324,14 @@ class API(base.Base): instance_id, security_group_id) - # NOTE(yamahata) - # tell vm driver to attach volume at boot time by updating - # BlockDeviceMapping - for bdm in block_device_mapping: - LOG.debug(_('bdm %s'), bdm) - assert 'device_name' in bdm - values = { - 'instance_id': instance_id, - 'device_name': bdm['device_name'], - 'delete_on_termination': bdm.get('delete_on_termination'), - 'virtual_name': bdm.get('virtual_name'), - 'snapshot_id': bdm.get('snapshot_id'), - 'volume_id': bdm.get('volume_id'), - 'volume_size': bdm.get('volume_size'), - 'no_device': bdm.get('no_device')} - self.db.block_device_mapping_create(elevated, values) + # BlockDeviceMapping table + self._update_image_block_device_mapping(elevated, instance_id, + image['properties'].get('mappings', [])) + self._update_block_device_mapping(elevated, instance_id, + image['properties'].get('block_device_mapping', [])) + # override via command line option + self._update_block_device_mapping(elevated, instance_id, + block_device_mapping) # Set sane defaults if not specified updates = dict(hostname=self.hostname_factory(instance_id)) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 8ab744855..146cdbb9f 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -227,6 +227,17 @@ class ComputeManager(manager.SchedulerDependentManager): for bdm in self.db.block_device_mapping_get_all_by_instance( context, instance_id): LOG.debug(_("setting up bdm %s"), bdm) + + if bdm['no_device']: + continue + if bdm['virtual_name']: + # TODO(yamahata): + # block devices for swap and ephemeralN will be + # created by virt driver locally in compute node. + assert (bdm['virtual_name'] == 'swap' or + bdm['virtual_name'].startswith('ephemeral')) + continue + if ((bdm['snapshot_id'] is not None) and (bdm['volume_id'] is None)): # TODO(yamahata): default name and description @@ -259,15 +270,6 @@ class ComputeManager(manager.SchedulerDependentManager): block_device_mapping.append({'device_path': dev_path, 'mount_device': bdm['device_name']}) - elif bdm['virtual_name'] is not None: - # TODO(yamahata): ephemeral/swap device support - LOG.debug(_('block_device_mapping: ' - 'ephemeral device is not supported yet')) - else: - # TODO(yamahata): NoDevice support - assert bdm['no_device'] - LOG.debug(_('block_device_mapping: ' - 'no device is not supported yet')) return block_device_mapping diff --git a/nova/image/s3.py b/nova/image/s3.py index 9e95bd698..b1afc6a81 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -141,6 +141,28 @@ class S3ImageService(service.BaseImageService): except Exception: arch = 'x86_64' + # NOTE(yamahata): + # EC2 ec2-budlne-image --block-device-mapping accepts + # = where + # virtual name = {ami, root, swap, ephemeral} + # where N is no negative integer + # device name = the device name seen by guest kernel. + # They are converted into + # block_device_mapping/mapping/{virtual, device} + # + # Do NOT confuse this with ec2-register's block device mapping + # argument. + mappings = [] + try: + block_device_mapping = manifest.findall('machine_configuration/' + 'block_device_mapping/' + 'mapping') + for bdm in block_device_mapping: + mappings.append({'virtual': bdm.find('virtual').text, + 'device': bdm.find('device').text}) + except Exception: + mappings = [] + properties = metadata['properties'] properties['project_id'] = context.project_id properties['architecture'] = arch @@ -151,6 +173,9 @@ class S3ImageService(service.BaseImageService): if ramdisk_id: properties['ramdisk_id'] = ec2utils.ec2_id_to_id(ramdisk_id) + if mappings: + properties['mappings'] = mappings + metadata.update({'disk_format': image_format, 'container_format': image_format, 'status': 'queued', -- cgit From 46c7e018ee3aed0f0e27ae3544193adb5fa62958 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 16 Jun 2011 21:25:57 +0900 Subject: api/ec2: support CreateImage --- nova/api/ec2/__init__.py | 1 + nova/api/ec2/cloud.py | 101 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py index 35f67d39b..cf1734281 100644 --- a/nova/api/ec2/__init__.py +++ b/nova/api/ec2/__init__.py @@ -271,6 +271,7 @@ class Authorizer(wsgi.Middleware): 'DescribeImageAttribute': ['all'], 'ModifyImageAttribute': ['projectmanager', 'sysadmin'], 'UpdateImage': ['projectmanager', 'sysadmin'], + 'CreateImage': ['projectmanager', 'sysadmin'], }, 'AdminController': { # All actions have the same permission: ['none'] (the default) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 1e594cd73..b38061eaf 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -1291,3 +1291,104 @@ class CloudController(object): internal_id = ec2utils.ec2_id_to_id(image_id) result = self.image_service.update(context, internal_id, dict(kwargs)) return result + + # TODO(yamahata): race condition + # At the moment there is no way to prevent others from + # manipulating instances/volumes/snapshots. + # As other code doesn't take it into consideration, here we don't + # care of it for now. Ostrich algorithm + def create_image(self, context, instance_id, **kwargs): + # NOTE(yamahata): name/description are ignored by register_image(), + # do so here + #description = kwargs.get('name') + #description = kwargs.get('description') + no_reboot = kwargs.get('no_reboot', False) + + ec2_instance_id = instance_id + instance_id = ec2utils.ec2_id_to_id(ec2_instance_id) + instance = self.compute_api.get(context, instance_id) + + # stop the instance if necessary + restart_instance = False + if not no_reboot: + state_description = instance['state_description'] + + # if the instance is in subtle state, refuse to proceed. + if state_description not in ('running', 'stopping', 'stopped'): + raise exception.InstanceNotRunning(instance_id=ec2_instance_id) + + if state_description == 'running': + restart_instance = True + self.compute_api.stop(context, instance_id=instance_id) + + # wait instance for really stopped + while state_description != 'stopped': + time.sleep(1) + instance = self.compute_api.get(context, instance_id) + state_description = instance['state_description'] + # NOTE(yamahata): timeout and error? + + src_image = self.image_service.show(context, instance['image_id']) + properties = src_image['properties'] + if instance['root_device_name']: + properties['root_device_name'] = instance['root_device_name'] + + mapping = [] + bdms = db.block_device_mapping_get_all_by_instance(context, + instance_id) + for bdm in bdms: + if bdm.no_device: + continue + m = {} + for attr in ('device_name', 'snapshot_id', 'volume_id', + 'volume_size', 'delete_on_termination', 'no_device', + 'virtual_name'): + val = getattr(bdm, attr) + if val is not None: + m[attr] = val + + volume_id = m.get('volume_id') + if m.get('snapshot_id') and volume_id: + # create snapshot based on volume_id + vol = self.volume_api.get(context, volume_id=volume_id) + # NOTE(yamahata): Should we wait for snapshot creation? + # Linux LVM snapshot creation completes in + # short time, it doesn't matter for now. + snapshot = self.volume_api.create_snapshot( + context, volume_id=volume_id, name=vol['display_name'], + description=vol['display_description']) + m['snapshot_id'] = snapshot['id'] + del m['volume_id'] + + if m: + mapping.append(m) + + for m in properties.get('mappings', []): + virtual_name = m['virtual'] + if virtual_name in ('ami', 'root'): + continue + + assert (virtual_name == 'swap' or + virtual_name.startswith('ephemeral')) + device_name = m['device_name'] + if device_name in [b.device_name for b in mapping + if not b.no_device]: + continue + + # NOTE(yamahata): swap and ephemeral devices are specified in + # AMI, but disabled for this instance by user. + # So disable those device by no_device. + mapping.append({'device_name': device_name, 'no_device': True}) + + if mapping: + properties['block_device_mapping'] = mapping + + for attr in ('status', 'location', 'id'): + del src_image[attr] + + image_id = self._register_image(context, src_image) + + if restart_instance: + self.compute_api.start(context, instance_id=instance_id) + + return {'imageId': image_id} -- cgit From 8da93c0352996ec733f3678b9ca02b72a96498c2 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 16 Jun 2011 21:44:50 +0900 Subject: fix conflict with rebasing. --- .../versions/022_add_root_device_name.py | 47 ---------------------- .../versions/025_add_root_device_name.py | 47 ++++++++++++++++++++++ 2 files changed, 47 insertions(+), 47 deletions(-) delete mode 100644 nova/db/sqlalchemy/migrate_repo/versions/022_add_root_device_name.py create mode 100644 nova/db/sqlalchemy/migrate_repo/versions/025_add_root_device_name.py diff --git a/nova/db/sqlalchemy/migrate_repo/versions/022_add_root_device_name.py b/nova/db/sqlalchemy/migrate_repo/versions/022_add_root_device_name.py deleted file mode 100644 index 6b98b9890..000000000 --- a/nova/db/sqlalchemy/migrate_repo/versions/022_add_root_device_name.py +++ /dev/null @@ -1,47 +0,0 @@ -# Copyright 2011 OpenStack LLC. -# Copyright 2011 Isaku Yamahata -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -from sqlalchemy import Column, Integer, MetaData, Table, String - -meta = MetaData() - - -# Just for the ForeignKey and column creation to succeed, these are not the -# actual definitions of instances or services. -instances = Table('instances', meta, - Column('id', Integer(), primary_key=True, nullable=False), - ) - -# -# New Column -# -root_device_name = Column( - 'root_device_name', - String(length=255, convert_unicode=False, assert_unicode=None, - unicode_error=None, _warn_on_bytestring=False), - nullable=True) - - -def upgrade(migrate_engine): - # Upgrade operations go here. Don't create your own engine; - # bind migrate_engine to your metadata - meta.bind = migrate_engine - instances.create_column(root_device_name) - - -def downgrade(migrate_engine): - # Operations to reverse the above upgrade go here. - meta.bind = migrate_engine - instances.drop_column('root_device_name') diff --git a/nova/db/sqlalchemy/migrate_repo/versions/025_add_root_device_name.py b/nova/db/sqlalchemy/migrate_repo/versions/025_add_root_device_name.py new file mode 100644 index 000000000..6b98b9890 --- /dev/null +++ b/nova/db/sqlalchemy/migrate_repo/versions/025_add_root_device_name.py @@ -0,0 +1,47 @@ +# Copyright 2011 OpenStack LLC. +# Copyright 2011 Isaku Yamahata +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from sqlalchemy import Column, Integer, MetaData, Table, String + +meta = MetaData() + + +# Just for the ForeignKey and column creation to succeed, these are not the +# actual definitions of instances or services. +instances = Table('instances', meta, + Column('id', Integer(), primary_key=True, nullable=False), + ) + +# +# New Column +# +root_device_name = Column( + 'root_device_name', + String(length=255, convert_unicode=False, assert_unicode=None, + unicode_error=None, _warn_on_bytestring=False), + nullable=True) + + +def upgrade(migrate_engine): + # Upgrade operations go here. Don't create your own engine; + # bind migrate_engine to your metadata + meta.bind = migrate_engine + instances.create_column(root_device_name) + + +def downgrade(migrate_engine): + # Operations to reverse the above upgrade go here. + meta.bind = migrate_engine + instances.drop_column('root_device_name') -- cgit From 7bc76ef403507f6762f782ff4d305cf2718346d5 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 16 Jun 2011 22:17:54 +0900 Subject: ec2/cloud.py: fix mismerge --- nova/api/ec2/cloud.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index b38061eaf..1bf8bbd35 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -1010,7 +1010,8 @@ class CloudController(object): if kwargs.get('ramdisk_id'): ramdisk = self._get_image(context, kwargs['ramdisk_id']) kwargs['ramdisk_id'] = ramdisk['id'] - _parse_block_device_mapping(bdm) + for bdm in kwargs.get('block_device_mapping', []): + _parse_block_device_mapping(bdm) image = self._get_image(context, kwargs['image_id']) -- cgit From 774e201a04addf95fab2253998967b212588cb0a Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 16 Jun 2011 22:18:06 +0900 Subject: compute/api: fix mismerge due to instance creation change --- nova/compute/api.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index d89dfb746..5b56d7865 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -250,7 +250,7 @@ class API(base.Base): 'vm_mode': vm_mode, 'root_device_name': root_device_name} - return (num_instances, base_options, security_groups) + return (num_instances, base_options, security_groups, image) def _update_image_block_device_mapping(self, elevated_context, instance_id, mappings): @@ -303,7 +303,7 @@ class API(base.Base): self.db.block_device_mapping_create(elevated_context, values) - def create_db_entry_for_new_instance(self, context, base_options, + def create_db_entry_for_new_instance(self, context, image, base_options, security_groups, block_device_mapping, num=1): """Create an entry in the DB for this new instance, including any related table updates (such as security @@ -393,7 +393,7 @@ class API(base.Base): """Provision the instances by passing the whole request to the Scheduler for execution. Returns a Reservation ID related to the creation of all of these instances.""" - num_instances, base_options, security_groups = \ + num_instances, base_options, security_groups, image = \ self._check_create_parameters( context, instance_type, image_href, kernel_id, ramdisk_id, @@ -429,7 +429,7 @@ class API(base.Base): Returns a list of instance dicts. """ - num_instances, base_options, security_groups = \ + num_instances, base_options, security_groups, image = \ self._check_create_parameters( context, instance_type, image_href, kernel_id, ramdisk_id, @@ -444,7 +444,7 @@ class API(base.Base): instances = [] LOG.debug(_("Going to run %s instances..."), num_instances) for num in range(num_instances): - instance = self.create_db_entry_for_new_instance(context, + instance = self.create_db_entry_for_new_instance(context, image, base_options, security_groups, block_device_mapping, num=num) instances.append(instance) -- cgit From 96fc985878cd52813aa07a4843e5928031b1501a Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:40:06 +0900 Subject: db/migration: resolve version conflict --- .../versions/025_add_root_device_name.py | 47 ---------------------- .../versions/027_add_root_device_name.py | 47 ++++++++++++++++++++++ 2 files changed, 47 insertions(+), 47 deletions(-) delete mode 100644 nova/db/sqlalchemy/migrate_repo/versions/025_add_root_device_name.py create mode 100644 nova/db/sqlalchemy/migrate_repo/versions/027_add_root_device_name.py diff --git a/nova/db/sqlalchemy/migrate_repo/versions/025_add_root_device_name.py b/nova/db/sqlalchemy/migrate_repo/versions/025_add_root_device_name.py deleted file mode 100644 index 6b98b9890..000000000 --- a/nova/db/sqlalchemy/migrate_repo/versions/025_add_root_device_name.py +++ /dev/null @@ -1,47 +0,0 @@ -# Copyright 2011 OpenStack LLC. -# Copyright 2011 Isaku Yamahata -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -from sqlalchemy import Column, Integer, MetaData, Table, String - -meta = MetaData() - - -# Just for the ForeignKey and column creation to succeed, these are not the -# actual definitions of instances or services. -instances = Table('instances', meta, - Column('id', Integer(), primary_key=True, nullable=False), - ) - -# -# New Column -# -root_device_name = Column( - 'root_device_name', - String(length=255, convert_unicode=False, assert_unicode=None, - unicode_error=None, _warn_on_bytestring=False), - nullable=True) - - -def upgrade(migrate_engine): - # Upgrade operations go here. Don't create your own engine; - # bind migrate_engine to your metadata - meta.bind = migrate_engine - instances.create_column(root_device_name) - - -def downgrade(migrate_engine): - # Operations to reverse the above upgrade go here. - meta.bind = migrate_engine - instances.drop_column('root_device_name') diff --git a/nova/db/sqlalchemy/migrate_repo/versions/027_add_root_device_name.py b/nova/db/sqlalchemy/migrate_repo/versions/027_add_root_device_name.py new file mode 100644 index 000000000..6b98b9890 --- /dev/null +++ b/nova/db/sqlalchemy/migrate_repo/versions/027_add_root_device_name.py @@ -0,0 +1,47 @@ +# Copyright 2011 OpenStack LLC. +# Copyright 2011 Isaku Yamahata +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from sqlalchemy import Column, Integer, MetaData, Table, String + +meta = MetaData() + + +# Just for the ForeignKey and column creation to succeed, these are not the +# actual definitions of instances or services. +instances = Table('instances', meta, + Column('id', Integer(), primary_key=True, nullable=False), + ) + +# +# New Column +# +root_device_name = Column( + 'root_device_name', + String(length=255, convert_unicode=False, assert_unicode=None, + unicode_error=None, _warn_on_bytestring=False), + nullable=True) + + +def upgrade(migrate_engine): + # Upgrade operations go here. Don't create your own engine; + # bind migrate_engine to your metadata + meta.bind = migrate_engine + instances.create_column(root_device_name) + + +def downgrade(migrate_engine): + # Operations to reverse the above upgrade go here. + meta.bind = migrate_engine + instances.drop_column('root_device_name') -- cgit From 4020e0dab41caf22de629c94cf94f5ea2101faee Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:48:30 +0900 Subject: db/block_device_mapping/api: introduce update_or_create introduce db.block_device_mapping_udpate_or_create() which update the colume if exists. Create new column if not existed. This api will be used later for block device mapping tracking. --- nova/db/api.py | 8 +++++++- nova/db/sqlalchemy/api.py | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/nova/db/api.py b/nova/db/api.py index 8f8e856b8..d77d0c352 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -936,10 +936,16 @@ def block_device_mapping_create(context, values): def block_device_mapping_update(context, bdm_id, values): - """Create an entry of block device mapping""" + """Update an entry of block device mapping""" return IMPL.block_device_mapping_update(context, bdm_id, values) +def block_device_mapping_update_or_create(context, values): + """Update an entry of block device mapping. + If not existed, create a new entry""" + return IMPL.block_device_mapping_update_or_create(context, values) + + def block_device_mapping_get_all_by_instance(context, instance_id): """Get all block device mapping belonging to a instance""" return IMPL.block_device_mapping_get_all_by_instance(context, instance_id) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index a2500a38d..7108c7a7a 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1932,6 +1932,23 @@ def block_device_mapping_update(context, bdm_id, values): update(values) +@require_context +def block_device_mapping_update_or_create(context, values): + session = get_session() + with session.begin(): + result = session.query(models.BlockDeviceMapping).\ + filter_by(instance_id=values['instance_id']).\ + filter_by(device_name=values['device_name']).\ + filter_by(deleted=False).\ + first() + if not result: + bdm_ref = models.BlockDeviceMapping() + bdm_ref.update(values) + bdm_ref.save(session=session) + else: + result.update(values) + + @require_context def block_device_mapping_get_all_by_instance(context, instance_id): session = get_session() -- cgit From 3a83471ec002127a84d319e397ce54e49bd696a1 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:53:47 +0900 Subject: api/ec2/image: make block device mapping pass unit tests. This patch makes pass unit tests which will follow later. --- nova/api/ec2/cloud.py | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 8138567d0..703dd82b4 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -135,12 +135,29 @@ def _format_block_device_mapping(bdm): return item -def _format_mappings(mappings, result): +def _format_mappings(properties, result): """Format multiple BlockDeviceMappingItemType""" + mappings = [{'virtualName': m['virtual'], 'deviceName': m['device']} + for m in properties.get('mappings', []) + if (m['virtual'] == 'swap' or + m['virtual'].startswith('ephemeral'))] + block_device_mapping = [_format_block_device_mapping(bdm) for bdm in - mappings] - if block_device_mapping: - result['blockDeviceMapping'] = block_device_mapping + properties.get('block_device_mapping', [])] + + # NOTE(yamahata): overwrite mappings with block_device_mapping + for bdm in block_device_mapping: + for i in range(len(mappings)): + if bdm['deviceName'] == mappings[i]['deviceName']: + del mappings[i] + break + mappings.append(bdm) + + # NOTE(yamahata): trim ebs.no_device == true. Is this necessary? + mappings = [bdm for bdm in mappings if not (bdm.get('noDevice', False))] + + if mappings: + result['blockDeviceMapping'] = mappings class CloudController(object): @@ -1178,7 +1195,7 @@ class CloudController(object): i['rootDeviceName'] = (root_device_name or _DEFAULT_ROOT_DEVICE_NAME) i['rootDeviceType'] = root_device_type - _format_mappings(properties.get('block_device_mapping', []), i) + _format_mappings(properties, i) return i @@ -1232,8 +1249,7 @@ class CloudController(object): def describe_image_attribute(self, context, image_id, attribute, **kwargs): def _block_device_mapping_attribute(image, result): - _format_mappings( - image['properties'].get('block_device_mapping', []), result) + _format_mappings(image['properties'], result) def _launch_permission_attribute(image, result): result['launchPermission'] = [] -- cgit From 91cc2d5f974d67d91e1e783aaec105c489a47cce Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:54:22 +0900 Subject: volume/api: introduce create_snapshot_force() Introduce create_snapshot_force() which create snapshot even when the volume is in in-use. This is needed for CreateImage with no_reboot=true. --- nova/compute/api.py | 24 +++++++++++------------- nova/volume/api.py | 13 +++++++++++-- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index e2692a42f..884ec9198 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -264,7 +264,7 @@ class API(base.Base): for bdm in mappings: LOG.debug(_("bdm %s"), bdm) - virtual_name = bdm['virtualName'] + virtual_name = bdm['virtual'] if virtual_name == 'ami' or virtual_name == 'root': continue @@ -272,9 +272,10 @@ class API(base.Base): virtual_name.startswith('ephemeral')) values = { 'instance_id': instance_id, - 'device_name': bdm['deviceName'], + 'device_name': bdm['device'], 'virtual_name': virtual_name, } - self.db.block_device_mapping_create(elevated_context, values) + self.db.block_device_mapping_update_or_create(elevated_context, + values) def _update_block_device_mapping(self, elevated_context, instance_id, block_device_mapping): @@ -285,15 +286,11 @@ class API(base.Base): LOG.debug(_('bdm %s'), bdm) assert 'device_name' in bdm - values = { - 'instance_id': instance_id, - 'device_name': bdm['device_name'], - 'delete_on_termination': bdm.get('delete_on_termination'), - 'virtual_name': bdm.get('virtual_name'), - 'snapshot_id': bdm.get('snapshot_id'), - 'volume_id': bdm.get('volume_id'), - 'volume_size': bdm.get('volume_size'), - 'no_device': bdm.get('no_device')} + values = {'instance_id': instance_id} + for key in ('device_name', 'delete_on_termination', 'virtual_name', + 'snapshot_id', 'volume_id', 'volume_size', + 'no_device'): + values[key] = bdm.get(key) # NOTE(yamahata): NoDevice eliminates devices defined in image # files by command line option. @@ -305,7 +302,8 @@ class API(base.Base): 'virtual_name'): values[k] = None - self.db.block_device_mapping_create(elevated_context, values) + self.db.block_device_mapping_update_or_create(elevated_context, + values) def create_db_entry_for_new_instance(self, context, image, base_options, security_groups, block_device_mapping, num=1): diff --git a/nova/volume/api.py b/nova/volume/api.py index 7d27abff9..cfc274c77 100644 --- a/nova/volume/api.py +++ b/nova/volume/api.py @@ -140,9 +140,10 @@ class API(base.Base): {"method": "remove_volume", "args": {'volume_id': volume_id}}) - def create_snapshot(self, context, volume_id, name, description): + def _create_snapshot(self, context, volume_id, name, description, + force=False): volume = self.get(context, volume_id) - if volume['status'] != "available": + if ((not force) and (volume['status'] != "available")): raise exception.ApiError(_("Volume status must be available")) options = { @@ -164,6 +165,14 @@ class API(base.Base): "snapshot_id": snapshot['id']}}) return snapshot + def create_snapshot(self, context, volume_id, name, description): + return self._create_snapshot(context, volume_id, name, description, + False) + + def create_snapshot_force(self, context, volume_id, name, description): + return self._create_snapshot(context, volume_id, name, description, + True) + def delete_snapshot(self, context, snapshot_id): snapshot = self.get_snapshot(context, snapshot_id) if snapshot['status'] != "available": -- cgit From d81d75bec04fe19492544e5bf7548dce5a2366ad Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:54:22 +0900 Subject: api/ec2: make CreateImage pass unit tests --- nova/api/ec2/cloud.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 703dd82b4..c25db9014 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -1346,7 +1346,7 @@ class CloudController(object): state_description = instance['state_description'] # NOTE(yamahata): timeout and error? - src_image = self.image_service.show(context, instance['image_id']) + src_image = self._get_image(context, instance['image_ref']) properties = src_image['properties'] if instance['root_device_name']: properties['root_device_name'] = instance['root_device_name'] @@ -1372,7 +1372,7 @@ class CloudController(object): # NOTE(yamahata): Should we wait for snapshot creation? # Linux LVM snapshot creation completes in # short time, it doesn't matter for now. - snapshot = self.volume_api.create_snapshot( + snapshot = self.volume_api.create_snapshot_force( context, volume_id=volume_id, name=vol['display_name'], description=vol['display_description']) m['snapshot_id'] = snapshot['id'] @@ -1388,9 +1388,9 @@ class CloudController(object): assert (virtual_name == 'swap' or virtual_name.startswith('ephemeral')) - device_name = m['device_name'] - if device_name in [b.device_name for b in mapping - if not b.no_device]: + device_name = m['device'] + if device_name in [b['device_name'] for b in mapping + if not b.get('no_device', False)]: continue # NOTE(yamahata): swap and ephemeral devices are specified in @@ -1402,7 +1402,7 @@ class CloudController(object): properties['block_device_mapping'] = mapping for attr in ('status', 'location', 'id'): - del src_image[attr] + src_image.pop(attr, None) image_id = self._register_image(context, src_image) -- cgit From 181ae36fe34edd206c33e3a0b7e10800ced93e97 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:54:32 +0900 Subject: test_api: unit tests for ec2utils.id_to_ec2_{snap, vol}_id() --- nova/tests/test_api.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/nova/tests/test_api.py b/nova/tests/test_api.py index 20b20fcbf..e74ae839e 100644 --- a/nova/tests/test_api.py +++ b/nova/tests/test_api.py @@ -107,6 +107,8 @@ class Ec2utilsTestCase(test.TestCase): def test_ec2_id_to_id(self): self.assertEqual(ec2utils.ec2_id_to_id('i-0000001e'), 30) self.assertEqual(ec2utils.ec2_id_to_id('ami-1d'), 29) + self.assertEqual(ec2utils.ec2_id_to_id('snap-0000001c'), 28) + self.assertEqual(ec2utils.ec2_id_to_id('vol-0000001b'), 27) def test_bad_ec2_id(self): self.assertRaises(exception.InvalidEc2Id, @@ -116,7 +118,8 @@ class Ec2utilsTestCase(test.TestCase): def test_id_to_ec2_id(self): self.assertEqual(ec2utils.id_to_ec2_id(30), 'i-0000001e') self.assertEqual(ec2utils.id_to_ec2_id(29, 'ami-%08x'), 'ami-0000001d') - + self.assertEqual(ec2utils.id_to_ec2_snap_id(28), 'snap-0000001c') + self.assertEqual(ec2utils.id_to_ec2_vol_id(27), 'vol-0000001b') class ApiEc2TestCase(test.TestCase): """Unit test for the cloud controller on an EC2 API""" -- cgit From 79d97f7232c119496dde1dd2f0534520ab383962 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:54:39 +0900 Subject: ec2utils: add an unit test for dict_from_dotted_str() --- nova/tests/test_api.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/nova/tests/test_api.py b/nova/tests/test_api.py index e74ae839e..20d78687e 100644 --- a/nova/tests/test_api.py +++ b/nova/tests/test_api.py @@ -121,6 +121,26 @@ class Ec2utilsTestCase(test.TestCase): self.assertEqual(ec2utils.id_to_ec2_snap_id(28), 'snap-0000001c') self.assertEqual(ec2utils.id_to_ec2_vol_id(27), 'vol-0000001b') + def test_dict_from_dotted_str(self): + in_str = [('BlockDeviceMapping.1.DeviceName', '/dev/sda1'), + ('BlockDeviceMapping.1.Ebs.SnapshotId', 'snap-0000001c'), + ('BlockDeviceMapping.1.Ebs.VolumeSize', '80'), + ('BlockDeviceMapping.1.Ebs.DeleteOnTermination', 'false'), + ('BlockDeviceMapping.2.DeviceName', '/dev/sdc'), + ('BlockDeviceMapping.2.VirtualName', 'ephemeral0')] + expected_dict = { + 'block_device_mapping': { + '1': {'device_name': '/dev/sda1', + 'ebs': {'snapshot_id': 'snap-0000001c', + 'volume_size': 80, + 'delete_on_termination': False}}, + '2': {'device_name': '/dev/sdc', + 'virtual_name': 'ephemeral0'}}} + out_dict = ec2utils.dict_from_dotted_str(in_str) + + self.assertDictMatch(out_dict, expected_dict) + + class ApiEc2TestCase(test.TestCase): """Unit test for the cloud controller on an EC2 API""" def setUp(self): -- cgit From 776571b38fb898a4dafa80e8f3da34b214c948b8 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:54:43 +0900 Subject: ec2utils: unit tests for case insensitive true/false conversion --- nova/tests/test_api.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nova/tests/test_api.py b/nova/tests/test_api.py index 20d78687e..bea4b4e9d 100644 --- a/nova/tests/test_api.py +++ b/nova/tests/test_api.py @@ -92,7 +92,9 @@ class XmlConversionTestCase(test.TestCase): conv = ec2utils._try_convert self.assertEqual(conv('None'), None) self.assertEqual(conv('True'), True) + self.assertEqual(conv('true'), True) self.assertEqual(conv('False'), False) + self.assertEqual(conv('false'), False) self.assertEqual(conv('0'), 0) self.assertEqual(conv('42'), 42) self.assertEqual(conv('3.14'), 3.14) -- cgit From ec515fa667e2954aa93a6954a541739e6e3aa221 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:54:49 +0900 Subject: image/s3: factor out _s3_create() for testability The unittest will come with later changeset. --- nova/image/s3.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/nova/image/s3.py b/nova/image/s3.py index b1afc6a81..86dd2e309 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -102,18 +102,7 @@ class S3ImageService(service.BaseImageService): key.get_contents_to_filename(local_filename) return local_filename - def _s3_create(self, context, metadata): - """Gets a manifext from s3 and makes an image.""" - - image_path = tempfile.mkdtemp(dir=FLAGS.image_decryption_dir) - - image_location = metadata['properties']['image_location'] - bucket_name = image_location.split('/')[0] - manifest_path = image_location[len(bucket_name) + 1:] - bucket = self._conn(context).get_bucket(bucket_name) - key = bucket.get_key(manifest_path) - manifest = key.get_contents_as_string() - + def _s3_parse_manifest(self, context, metadata, manifest): manifest = ElementTree.fromstring(manifest) image_format = 'ami' image_type = 'machine' @@ -183,6 +172,21 @@ class S3ImageService(service.BaseImageService): 'properties': properties}) metadata['properties']['image_state'] = 'pending' image = self.service.create(context, metadata) + return manifest, image + + def _s3_create(self, context, metadata): + """Gets a manifext from s3 and makes an image.""" + + image_path = tempfile.mkdtemp(dir=FLAGS.image_decryption_dir) + + image_location = metadata['properties']['image_location'] + bucket_name = image_location.split('/')[0] + manifest_path = image_location[len(bucket_name) + 1:] + bucket = self._conn(context).get_bucket(bucket_name) + key = bucket.get_key(manifest_path) + manifest = key.get_contents_as_string() + + manifest, image = self._s3_parse_manifest(metadata, context, manifest) image_id = image['id'] def delayed_create(): -- cgit From ac9ed64077eaad6b4df91fbf90af7933a6bddd5a Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:54:49 +0900 Subject: unittest, image/s3: unit tests for s3 image handler --- nova/tests/image/test_s3.py | 122 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 nova/tests/image/test_s3.py diff --git a/nova/tests/image/test_s3.py b/nova/tests/image/test_s3.py new file mode 100644 index 000000000..231e109f8 --- /dev/null +++ b/nova/tests/image/test_s3.py @@ -0,0 +1,122 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2011 Isaku Yamahata +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from nova import context +from nova import flags +from nova import test +from nova.image import s3 + +FLAGS = flags.FLAGS + + +ami_manifest_xml = """ + + 2011-06-17 + + test-s3 + 0 + 0 + + + x86_64 + + + ami + sda1 + + + root + /dev/sda1 + + + ephemeral0 + sda2 + + + swap + sda3 + + + + +""" + + +class TestS3ImageService(test.TestCase): + def setUp(self): + super(TestS3ImageService, self).setUp() + self.orig_image_service = FLAGS.image_service + FLAGS.image_service = 'nova.image.fake.FakeImageService' + self.image_service = s3.S3ImageService() + self.context = context.RequestContext(None, None) + + def tearDown(self): + super(TestS3ImageService, self).tearDown() + FLAGS.image_service = self.orig_image_service + + def _assertEqualList(self, list0, list1, keys): + self.assertEqual(len(list0), len(list1)) + key = keys[0] + for x in list0: + self.assertEqual(len(x), len(keys)) + self.assertTrue(key in x) + for y in list1: + self.assertTrue(key in y) + if x[key] == y[key]: + for k in keys: + self.assertEqual(x[k], y[k]) + + def test_s3_create(self): + metadata = {'properties': { + 'root_device_name': '/dev/sda1', + 'block_device_mapping': [ + {'device_name': '/dev/sda1', + 'snapshot_id': 'snap-12345678', + 'delete_on_termination': True}, + {'device_name': '/dev/sda2', + 'virutal_name': 'ephemeral0'}, + {'device_name': '/dev/sdb0', + 'no_device': True}]}} + _manifest, image = self.image_service._s3_parse_manifest( + self.context, metadata, ami_manifest_xml) + image_id = image['id'] + + ret_image = self.image_service.show(self.context, image_id) + self.assertTrue('properties' in ret_image) + properties = ret_image['properties'] + + self.assertTrue('mappings' in properties) + mappings = properties['mappings'] + expected_mappings = [ + {"device": "sda1", "virtual": "ami"}, + {"device": "/dev/sda1", "virtual": "root"}, + {"device": "sda2", "virtual": "ephemeral0"}, + {"device": "sda3", "virtual": "swap"}] + self._assertEqualList(mappings, expected_mappings, + ['device', 'virtual']) + + self.assertTrue('block_device_mapping', properties) + block_device_mapping = properties['block_device_mapping'] + expected_bdm = [ + {'device_name': '/dev/sda1', + 'snapshot_id': 'snap-12345678', + 'delete_on_termination': True}, + {'device_name': '/dev/sda2', + 'virutal_name': 'ephemeral0'}, + {'device_name': '/dev/sdb0', + 'no_device': True}] + self.assertEqual(block_device_mapping, expected_bdm) -- cgit From aadb9a0a8a9a0b947643c04f24b623412db7d48d Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:54:56 +0900 Subject: ec2utils: an unit test for ec2utils.properties_root_defice_name. --- nova/tests/test_api.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/nova/tests/test_api.py b/nova/tests/test_api.py index bea4b4e9d..ebc5508cc 100644 --- a/nova/tests/test_api.py +++ b/nova/tests/test_api.py @@ -142,6 +142,17 @@ class Ec2utilsTestCase(test.TestCase): self.assertDictMatch(out_dict, expected_dict) + def test_properties_root_defice_name(self): + mappings = [{"device": "/dev/sda1", "virtual": "root"}] + properties0 = {'mappings': mappings} + properties1 = {'root_device_name': '/dev/sdb', 'mappings': mappings} + + root_device_name = ec2utils.properties_root_device_name(properties0) + self.assertEqual(root_device_name, '/dev/sda1') + + root_device_name = ec2utils.properties_root_device_name(properties1) + self.assertEqual(root_device_name, '/dev/sdb') + class ApiEc2TestCase(test.TestCase): """Unit test for the cloud controller on an EC2 API""" -- cgit From 1df8275883930c71ea4324b0d43b6508440e1d65 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:55:03 +0900 Subject: test_cloud: an unit test for describe image with block device mapping --- nova/tests/test_cloud.py | 133 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 131 insertions(+), 2 deletions(-) diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index fd1c21386..35660d4bb 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -323,6 +323,135 @@ class CloudTestCase(test.TestCase): self.assertRaises(exception.ImageNotFound, describe_images, self.context, ['ami-fake']) + def assertDictListUnorderedMatch(self, L1, L2, key): + self.assertEqual(len(L1), len(L2)) + for d1 in L1: + self.assertTrue(key in d1) + for d2 in L2: + self.assertTrue(key in d2) + if d1[key] == d2[key]: + self.assertDictMatch(d1, d2) + + def _assertImageSet(self, result, root_device_type, root_device_name): + self.assertEqual(1, len(result['imagesSet'])) + result = result['imagesSet'][0] + self.assertTrue('rootDeviceType' in result) + self.assertEqual(result['rootDeviceType'], root_device_type) + self.assertTrue('rootDeviceName' in result) + self.assertEqual(result['rootDeviceName'], root_device_name) + self.assertTrue('blockDeviceMapping' in result) + + return result + + # NOTE(yamahata): + # InstanceBlockDeviceMappingItemType + # rootDeviceType + # rootDeviceName + # blockDeviceMapping + # deviceName + # virtualName + # ebs + # snapshotId + # volumeSize + # deleteOnTermination + # noDevice + def test_describe_image_mapping(self): + """test for rootDeviceName and blockDeiceMapping""" + describe_images = self.cloud.describe_images + mappings1 = [ + {'device': '/dev/sda1', 'virtual': 'root'}, + + {'device': '/dev/sdb0', 'virtual': 'ephemeral0'}, + {'device': '/dev/sdb1', 'virtual': 'ephemeral1'}, + {'device': '/dev/sdb2', 'virtual': 'ephemeral2'}, + {'device': '/dev/sdb3', 'virtual': 'ephemeral3'}, + {'device': '/dev/sdb4', 'virtual': 'ephemeral4'}, + + {'device': '/dev/sdc0', 'virtual': 'swap'}, + {'device': '/dev/sdc1', 'virtual': 'swap'}, + {'device': '/dev/sdc2', 'virtual': 'swap'}, + {'device': '/dev/sdc3', 'virtual': 'swap'}, + {'device': '/dev/sdc4', 'virtual': 'swap'}] + block_device_mapping1 = [ + {'device_name': '/dev/sdb1', 'snapshot_id': 01234567}, + {'device_name': '/dev/sdb2', 'volume_id': 01234567}, + {'device_name': '/dev/sdb3', 'virtual_name': 'ephemeral5'}, + {'device_name': '/dev/sdb4', 'no_device': True}, + + {'device_name': '/dev/sdc1', 'snapshot_id': 12345678}, + {'device_name': '/dev/sdc2', 'volume_id': 12345678}, + {'device_name': '/dev/sdc3', 'virtual_name': 'ephemeral6'}, + {'device_name': '/dev/sdc4', 'no_device': True}] + image1 = { + 'id': 1, + 'properties': { + 'kernel_id': 1, + 'type': 'machine', + 'mappings': mappings1, + 'block_device_mapping': block_device_mapping1, + } + } + + mappings2 = [{'device': '/dev/sda1', 'virtual': 'root'}] + block_device_mapping2 = [{'device_name': '/dev/sdb1', + 'snapshot_id': 01234567}] + image2 = { + 'id': 2, + 'properties': { + 'kernel_id': 2, + 'type': 'machine', + 'root_device_name': '/dev/sdb1', + 'mappings': mappings2, + 'block_device_mapping': block_device_mapping2}} + + def fake_show(meh, context, image_id): + for i in [image1, image2]: + if i['id'] == image_id: + return i + raise exception.ImageNotFound(image_id=image_id) + + def fake_detail(meh, context): + return [image1, image2] + + self.stubs.Set(fake._FakeImageService, 'show', fake_show) + self.stubs.Set(fake._FakeImageService, 'detail', fake_detail) + + result = describe_images(self.context, ['ami-00000001']) + result = self._assertImageSet(result, 'instance-store', '/dev/sda1') + + # NOTE(yamahata): noDevice doesn't make sense when returning mapping + # It makes sense only when user overriding existing + # mapping. + expected_bdms = [ + {'deviceName': '/dev/sdb0', 'virtualName': 'ephemeral0'}, + {'deviceName': '/dev/sdb1', 'ebs': {'snapshotId': + 'snap-00053977'}}, + {'deviceName': '/dev/sdb2', 'ebs': {'snapshotId': + 'vol-00053977'}}, + {'deviceName': '/dev/sdb3', 'virtualName': 'ephemeral5'}, + # {'deviceName': '/dev/sdb4', 'noDevice': True}, + + {'deviceName': '/dev/sdc0', 'virtualName': 'swap'}, + {'deviceName': '/dev/sdc1', 'ebs': {'snapshotId': + 'snap-00bc614e'}}, + {'deviceName': '/dev/sdc2', 'ebs': {'snapshotId': + 'vol-00bc614e'}}, + {'deviceName': '/dev/sdc3', 'virtualName': 'ephemeral6'}, + # {'deviceName': '/dev/sdc4', 'noDevice': True} + ] + self.assertDictListUnorderedMatch(result['blockDeviceMapping'], + expected_bdms, 'deviceName') + + result = describe_images(self.context, ['ami-00000002']) + result = self._assertImageSet(result, 'ebs', '/dev/sdb1') + + expected_bdms = [{'deviceName': '/dev/sdb1', + 'ebs': {'snapshotId': 'snap-00053977'}}] + self.assertDictListUnorderedMatch(result['blockDeviceMapping'], + expected_bdms, 'deviceName') + + self.stubs.UnsetAll() + def test_describe_image_attribute(self): describe_image_attribute = self.cloud.describe_image_attribute @@ -679,10 +808,10 @@ class CloudTestCase(test.TestCase): 'max_count': 1, 'block_device_mapping': [{'device_name': '/dev/vdb', 'volume_id': vol1['id'], - 'delete_on_termination': False, }, + 'delete_on_termination': False}, {'device_name': '/dev/vdc', 'volume_id': vol2['id'], - 'delete_on_termination': True, }, + 'delete_on_termination': True}, ]} ec2_instance_id = self._run_instance_wait(**kwargs) instance_id = ec2utils.ec2_id_to_id(ec2_instance_id) -- cgit From c6792450aa745ef003b80999eae3283533a15521 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:55:08 +0900 Subject: unittest: an unit test for ec2 describe image attribute --- nova/tests/test_cloud.py | 141 +++++++++++++++++++++++++++++------------------ 1 file changed, 88 insertions(+), 53 deletions(-) diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 35660d4bb..43bcdf703 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -332,32 +332,7 @@ class CloudTestCase(test.TestCase): if d1[key] == d2[key]: self.assertDictMatch(d1, d2) - def _assertImageSet(self, result, root_device_type, root_device_name): - self.assertEqual(1, len(result['imagesSet'])) - result = result['imagesSet'][0] - self.assertTrue('rootDeviceType' in result) - self.assertEqual(result['rootDeviceType'], root_device_type) - self.assertTrue('rootDeviceName' in result) - self.assertEqual(result['rootDeviceName'], root_device_name) - self.assertTrue('blockDeviceMapping' in result) - - return result - - # NOTE(yamahata): - # InstanceBlockDeviceMappingItemType - # rootDeviceType - # rootDeviceName - # blockDeviceMapping - # deviceName - # virtualName - # ebs - # snapshotId - # volumeSize - # deleteOnTermination - # noDevice - def test_describe_image_mapping(self): - """test for rootDeviceName and blockDeiceMapping""" - describe_images = self.cloud.describe_images + def _setUpImageSet(self): mappings1 = [ {'device': '/dev/sda1', 'virtual': 'root'}, @@ -416,39 +391,73 @@ class CloudTestCase(test.TestCase): self.stubs.Set(fake._FakeImageService, 'show', fake_show) self.stubs.Set(fake._FakeImageService, 'detail', fake_detail) + def _assertImageSet(self, result, root_device_type, root_device_name): + self.assertEqual(1, len(result['imagesSet'])) + result = result['imagesSet'][0] + self.assertTrue('rootDeviceType' in result) + self.assertEqual(result['rootDeviceType'], root_device_type) + self.assertTrue('rootDeviceName' in result) + self.assertEqual(result['rootDeviceName'], root_device_name) + self.assertTrue('blockDeviceMapping' in result) + + return result + + _expected_root_device_name1 = '/dev/sda1' + # NOTE(yamahata): noDevice doesn't make sense when returning mapping + # It makes sense only when user overriding existing + # mapping. + _expected_bdms1 = [ + {'deviceName': '/dev/sdb0', 'virtualName': 'ephemeral0'}, + {'deviceName': '/dev/sdb1', 'ebs': {'snapshotId': + 'snap-00053977'}}, + {'deviceName': '/dev/sdb2', 'ebs': {'snapshotId': + 'vol-00053977'}}, + {'deviceName': '/dev/sdb3', 'virtualName': 'ephemeral5'}, + # {'deviceName': '/dev/sdb4', 'noDevice': True}, + + {'deviceName': '/dev/sdc0', 'virtualName': 'swap'}, + {'deviceName': '/dev/sdc1', 'ebs': {'snapshotId': + 'snap-00bc614e'}}, + {'deviceName': '/dev/sdc2', 'ebs': {'snapshotId': + 'vol-00bc614e'}}, + {'deviceName': '/dev/sdc3', 'virtualName': 'ephemeral6'}, + # {'deviceName': '/dev/sdc4', 'noDevice': True} + ] + + _expected_root_device_name2 = '/dev/sdb1' + _expected_bdms2 = [{'deviceName': '/dev/sdb1', + 'ebs': {'snapshotId': 'snap-00053977'}}] + + # NOTE(yamahata): + # InstanceBlockDeviceMappingItemType + # rootDeviceType + # rootDeviceName + # blockDeviceMapping + # deviceName + # virtualName + # ebs + # snapshotId + # volumeSize + # deleteOnTermination + # noDevice + def test_describe_image_mapping(self): + """test for rootDeviceName and blockDeiceMapping""" + describe_images = self.cloud.describe_images + self._setUpImageSet() + result = describe_images(self.context, ['ami-00000001']) - result = self._assertImageSet(result, 'instance-store', '/dev/sda1') - - # NOTE(yamahata): noDevice doesn't make sense when returning mapping - # It makes sense only when user overriding existing - # mapping. - expected_bdms = [ - {'deviceName': '/dev/sdb0', 'virtualName': 'ephemeral0'}, - {'deviceName': '/dev/sdb1', 'ebs': {'snapshotId': - 'snap-00053977'}}, - {'deviceName': '/dev/sdb2', 'ebs': {'snapshotId': - 'vol-00053977'}}, - {'deviceName': '/dev/sdb3', 'virtualName': 'ephemeral5'}, - # {'deviceName': '/dev/sdb4', 'noDevice': True}, - - {'deviceName': '/dev/sdc0', 'virtualName': 'swap'}, - {'deviceName': '/dev/sdc1', 'ebs': {'snapshotId': - 'snap-00bc614e'}}, - {'deviceName': '/dev/sdc2', 'ebs': {'snapshotId': - 'vol-00bc614e'}}, - {'deviceName': '/dev/sdc3', 'virtualName': 'ephemeral6'}, - # {'deviceName': '/dev/sdc4', 'noDevice': True} - ] + result = self._assertImageSet(result, 'instance-store', + self._expected_root_device_name1) + self.assertDictListUnorderedMatch(result['blockDeviceMapping'], - expected_bdms, 'deviceName') + self._expected_bdms1, 'deviceName') result = describe_images(self.context, ['ami-00000002']) - result = self._assertImageSet(result, 'ebs', '/dev/sdb1') + result = self._assertImageSet(result, 'ebs', + self._expected_root_device_name2) - expected_bdms = [{'deviceName': '/dev/sdb1', - 'ebs': {'snapshotId': 'snap-00053977'}}] self.assertDictListUnorderedMatch(result['blockDeviceMapping'], - expected_bdms, 'deviceName') + self._expected_bdms2, 'deviceName') self.stubs.UnsetAll() @@ -465,6 +474,32 @@ class CloudTestCase(test.TestCase): 'launchPermission') self.assertEqual([{'group': 'all'}], result['launchPermission']) + def test_describe_image_attribute_root_device_name(self): + describe_image_attribute = self.cloud.describe_image_attribute + self._setUpImageSet() + + result = describe_image_attribute(self.context, 'ami-00000001', + 'rootDeviceName') + self.assertEqual(result['rootDeviceName'], + self._expected_root_device_name1) + result = describe_image_attribute(self.context, 'ami-00000002', + 'rootDeviceName') + self.assertEqual(result['rootDeviceName'], + self._expected_root_device_name2) + + def test_describe_image_attribute_block_device_mapping(self): + describe_image_attribute = self.cloud.describe_image_attribute + self._setUpImageSet() + + result = describe_image_attribute(self.context, 'ami-00000001', + 'blockDeviceMapping') + self.assertDictListUnorderedMatch(result['blockDeviceMapping'], + self._expected_bdms1, 'deviceName') + result = describe_image_attribute(self.context, 'ami-00000002', + 'blockDeviceMapping') + self.assertDictListUnorderedMatch(result['blockDeviceMapping'], + self._expected_bdms2, 'deviceName') + def test_modify_image_attribute(self): modify_image_attribute = self.cloud.modify_image_attribute -- cgit From 5276e80c403a2ae87d3c93979289331e286fd2a1 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:55:11 +0900 Subject: api/ec2, boot-from-volume: an unit test for describe instances --- nova/test.py | 12 ++++ nova/tests/test_cloud.py | 140 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+) diff --git a/nova/test.py b/nova/test.py index 4a0a18fe7..99d4cec4f 100644 --- a/nova/test.py +++ b/nova/test.py @@ -252,3 +252,15 @@ class TestCase(unittest.TestCase): for d1, d2 in zip(L1, L2): self.assertDictMatch(d1, d2, approx_equal=approx_equal, tolerance=tolerance) + + def assertSubDictMatch(self, sub_dict, super_dict): + """Assert a sub_dict is subset of super_dict.""" + self.assertTrue(set(sub_dict.keys()).issubset(set(super_dict.keys()))) + for k, sub_value in sub_dict.items(): + super_value = super_dict[k] + if isinstance(sub_value, dict): + self.assertSubDictMatch(sub_value, super_value) + elif 'DONTCARE' in (sub_value, super_value): + continue + else: + self.assertEqual(sub_value, super_value) diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 43bcdf703..f7326e66f 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -294,6 +294,146 @@ class CloudTestCase(test.TestCase): db.service_destroy(self.context, comp1['id']) db.service_destroy(self.context, comp2['id']) + def _assertInstance(self, instance_id): + ec2_instance_id = ec2utils.id_to_ec2_id(instance_id) + result = self.cloud.describe_instances(self.context, + instance_id=[ec2_instance_id]) + result = result['reservationSet'][0] + self.assertEqual(len(result['instancesSet']), 1) + result = result['instancesSet'][0] + self.assertEqual(result['instanceId'], ec2_instance_id) + return (ec2_instance_id, result) + + def _assertEqualBlockDeviceMapping(self, expected, result): + self.assertEqual(len(expected), len(result)) + for x in expected: + found = False + for y in result: + if x['deviceName'] == y['deviceName']: + self.assertSubDictMatch(x, y) + found = True + break + self.assertTrue(found) + + def test_describe_instances_bdm(self): + """Make sure describe_instances works with root_device_name and + block device mappings + """ + inst1 = db.instance_create(self.context, + {'image_ref': 1, + 'root_device_name': '/dev/sdb1'}) + inst2 = db.instance_create(self.context, + {'image_ref': 2, + 'root_device_name': '/dev/sdc1'}) + + instance_id = inst1['id'] + mappings = [ + {'instance_id': instance_id, + 'device_name': '/dev/sdb1', + 'snapshot_id': '1', + 'volume_id': '2'}, + {'instance_id': instance_id, + 'device_name': '/dev/sdb2', + 'volume_id': '3'}, + {'instance_id': instance_id, + 'device_name': '/dev/sdb3', + 'delete_on_termination': True, + 'snapshot_id': '4', + 'volume_id': '5'}, + {'instance_id': instance_id, + 'device_name': '/dev/sdb4', + 'delete_on_termination': False, + 'snapshot_id': '6', + 'volume_id': '7'}, + {'instance_id': instance_id, + 'device_name': '/dev/sdb5', + 'snapshot_id': '8', + 'volume_id': '9', + 'volume_size': 0}, + {'instance_id': instance_id, + 'device_name': '/dev/sdb6', + 'snapshot_id': '10', + 'volume_id': '11', + 'volume_size': 1}, + {'instance_id': instance_id, + 'device_name': '/dev/sdb7', + 'no_device': True}, + {'instance_id': instance_id, + 'device_name': '/dev/sdb8', + 'virtual_name': 'swap'}, + {'instance_id': instance_id, + 'device_name': '/dev/sdb9', + 'virtual_name': 'ephemeral3'}] + + volumes = [] + for bdm in mappings: + db.block_device_mapping_create(self.context, bdm) + if bdm.get('volume_id'): + values = {'volume_id': bdm['volume_id']} + for bdm_key, vol_key in [('snapshot_id', 'snapshot_id'), + ('snapshot_size', 'volume_size'), + ('delete_on_termination', + 'delete_on_termination')]: + if bdm.get(bdm_key): + values[vol_key] = bdm[bdm_key] + vol = db.volume_create(self.context, values) + db.volume_attached(self.context, vol['id'], + instance_id, bdm['device_name']) + volumes.append(vol) + + ec2_instance_id, result = self._assertInstance(instance_id) + expected_result = {'instanceId': ec2_instance_id, + 'rootDeviceName': '/dev/sdb1', + 'rootDeviceType': 'ebs'} + expected_block_device_mapping = [ + {'deviceName': '/dev/sdb1', + 'ebs': {'status': 'in-use', + 'deleteOnTermination': False, + 'volumeId': 2, + }}, + {'deviceName': '/dev/sdb2', + 'ebs': {'status': 'in-use', + 'deleteOnTermination': False, + 'volumeId': 3, + }}, + {'deviceName': '/dev/sdb3', + 'ebs': {'status': 'in-use', + 'deleteOnTermination': True, + 'volumeId': 5, + }}, + {'deviceName': '/dev/sdb4', + 'ebs': {'status': 'in-use', + 'deleteOnTermination': False, + 'volumeId': 7, + }}, + {'deviceName': '/dev/sdb5', + 'ebs': {'status': 'in-use', + 'deleteOnTermination': False, + 'volumeId': 9, + }}, + {'deviceName': '/dev/sdb6', + 'ebs': {'status': 'in-use', + 'deleteOnTermination': False, + 'volumeId': 11, }}] + # NOTE(yamahata): swap/ephemeral device case isn't supported yet. + self.assertSubDictMatch(expected_result, result) + self._assertEqualBlockDeviceMapping(expected_block_device_mapping, + result['blockDeviceMapping']) + + ec2_instance_id, result = self._assertInstance(inst2['id']) + expected_result = {'instanceId': ec2_instance_id, + 'rootDeviceName': '/dev/sdc1', + 'rootDeviceType': 'instance-store'} + self.assertSubDictMatch(expected_result, result) + + for vol in volumes: + db.volume_destroy(self.context, vol['id']) + for bdm in db.block_device_mapping_get_all_by_instance(self.context, + instance_id): + db.block_device_mapping_destroy(self.context, bdm['id']) + db.instance_destroy(self.context, inst2['id']) + db.instance_destroy(self.context, inst1['id']) + def test_describe_images(self): describe_images = self.cloud.describe_images -- cgit From 97a710fa191b0abd94fef25d7110448c41c4e259 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 22 Jun 2011 12:55:14 +0900 Subject: api/ec2: an unit test for create image unit test for ec2 create image. This is incomplete as there is no unit test for register image. --- nova/tests/test_cloud.py | 90 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 72 insertions(+), 18 deletions(-) diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index f7326e66f..aed5aff6a 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -294,6 +294,24 @@ class CloudTestCase(test.TestCase): db.service_destroy(self.context, comp1['id']) db.service_destroy(self.context, comp2['id']) + def _block_device_mapping_create(self, instance_id, mappings): + volumes = [] + for bdm in mappings: + db.block_device_mapping_create(self.context, bdm) + if 'volume_id' in bdm: + values = {'id': bdm['volume_id']} + for bdm_key, vol_key in [('snapshot_id', 'snapshot_id'), + ('snapshot_size', 'volume_size'), + ('delete_on_termination', + 'delete_on_termination')]: + if bdm_key in bdm: + values[vol_key] = bdm[bdm_key] + vol = db.volume_create(self.context, values) + db.volume_attached(self.context, vol['id'], + instance_id, bdm['device_name']) + volumes.append(vol) + return volumes + def _assertInstance(self, instance_id): ec2_instance_id = ec2utils.id_to_ec2_id(instance_id) result = self.cloud.describe_instances(self.context, @@ -334,7 +352,8 @@ class CloudTestCase(test.TestCase): 'volume_id': '2'}, {'instance_id': instance_id, 'device_name': '/dev/sdb2', - 'volume_id': '3'}, + 'volume_id': '3', + 'volume_size': 1}, {'instance_id': instance_id, 'device_name': '/dev/sdb3', 'delete_on_termination': True, @@ -365,21 +384,7 @@ class CloudTestCase(test.TestCase): 'device_name': '/dev/sdb9', 'virtual_name': 'ephemeral3'}] - volumes = [] - for bdm in mappings: - db.block_device_mapping_create(self.context, bdm) - if bdm.get('volume_id'): - values = {'volume_id': bdm['volume_id']} - for bdm_key, vol_key in [('snapshot_id', 'snapshot_id'), - ('snapshot_size', 'volume_size'), - ('delete_on_termination', - 'delete_on_termination')]: - if bdm.get(bdm_key): - values[vol_key] = bdm[bdm_key] - vol = db.volume_create(self.context, values) - db.volume_attached(self.context, vol['id'], - instance_id, bdm['device_name']) - volumes.append(vol) + volumes = self._block_device_mapping_create(instance_id, mappings) ec2_instance_id, result = self._assertInstance(instance_id) expected_result = {'instanceId': ec2_instance_id, @@ -472,7 +477,7 @@ class CloudTestCase(test.TestCase): if d1[key] == d2[key]: self.assertDictMatch(d1, d2) - def _setUpImageSet(self): + def _setUpImageSet(self, create_volumes_and_snapshots=False): mappings1 = [ {'device': '/dev/sda1', 'virtual': 'root'}, @@ -502,6 +507,7 @@ class CloudTestCase(test.TestCase): 'properties': { 'kernel_id': 1, 'type': 'machine', + 'image_state': 'available', 'mappings': mappings1, 'block_device_mapping': block_device_mapping1, } @@ -531,6 +537,22 @@ class CloudTestCase(test.TestCase): self.stubs.Set(fake._FakeImageService, 'show', fake_show) self.stubs.Set(fake._FakeImageService, 'detail', fake_detail) + volumes = [] + snapshots = [] + if create_volumes_and_snapshots: + for bdm in block_device_mapping1: + if 'volume_id' in bdm: + vol = self._volume_create(bdm['volume_id']) + volumes.append(vol['id']) + if 'snapshot_id' in bdm: + snap = db.snapshot_create(self.context, + {'id': bdm['snapshot_id'], + 'volume_id': 76543210, + 'status': "available", + 'volume_size': 1}) + snapshots.append(snap['id']) + return (volumes, snapshots) + def _assertImageSet(self, result, root_device_type, root_device_name): self.assertEqual(1, len(result['imagesSet'])) result = result['imagesSet'][0] @@ -951,11 +973,13 @@ class CloudTestCase(test.TestCase): self._restart_compute_service() - def _volume_create(self): + def _volume_create(self, volume_id=None): kwargs = {'status': 'available', 'host': self.volume.host, 'size': 1, 'attach_status': 'detached', } + if volume_id: + kwargs['id'] = volume_id return db.volume_create(self.context, kwargs) def _assert_volume_attached(self, vol, instance_id, mountpoint): @@ -1175,3 +1199,33 @@ class CloudTestCase(test.TestCase): self.cloud.delete_snapshot(self.context, snapshot_id) greenthread.sleep(0.3) db.volume_destroy(self.context, vol['id']) + + def test_create_image(self): + """Make sure that CreateImage works""" + # enforce periodic tasks run in short time to avoid wait for 60s. + self._restart_compute_service(periodic_interval=0.3) + + (volumes, snapshots) = self._setUpImageSet( + create_volumes_and_snapshots=True) + + kwargs = {'image_id': 'ami-1', + 'instance_type': FLAGS.default_instance_type, + 'max_count': 1} + ec2_instance_id = self._run_instance_wait(**kwargs) + + # TODO(yamahata): s3._s3_create() can't be tested easily by unit test + # as there is no unit test for s3.create() + ## result = self.cloud.create_image(self.context, ec2_instance_id, + ## no_reboot=True) + ## ec2_image_id = result['imageId'] + ## created_image = self.cloud.describe_images(self.context, + ## [ec2_image_id]) + + self.cloud.terminate_instances(self.context, [ec2_instance_id]) + for vol in volumes: + db.volume_destroy(self.context, vol) + for snap in snapshots: + db.snapshot_destroy(self.context, snap) + # TODO(yamahata): clean up snapshot created by CreateImage. + + self._restart_compute_service() -- cgit From c24b30c9a060e50c7bd953a7d68c409416f4f752 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 23 Jun 2011 19:51:00 +0900 Subject: volume/api: an unit test for create_snapshot_force() --- nova/tests/test_volume.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/nova/tests/test_volume.py b/nova/tests/test_volume.py index 4f10ee6af..5230cef0e 100644 --- a/nova/tests/test_volume.py +++ b/nova/tests/test_volume.py @@ -27,8 +27,10 @@ from nova import exception from nova import db from nova import flags from nova import log as logging +from nova import rpc from nova import test from nova import utils +from nova import volume FLAGS = flags.FLAGS LOG = logging.getLogger('nova.tests.volume') @@ -43,6 +45,11 @@ class VolumeTestCase(test.TestCase): self.flags(connection_type='fake') self.volume = utils.import_object(FLAGS.volume_manager) self.context = context.get_admin_context() + self.instance_id = db.instance_create(self.context, {})['id'] + + def tearDown(self): + db.instance_destroy(self.context, self.instance_id) + super(VolumeTestCase, self).tearDown() @staticmethod def _create_volume(size='0', snapshot_id=None): @@ -224,6 +231,30 @@ class VolumeTestCase(test.TestCase): snapshot_id) self.volume.delete_volume(self.context, volume_id) + def test_create_snapshot_force(self): + """Test snapshot in use can be created forcibly.""" + + def fake_cast(ctxt, topic, msg): + pass + self.stubs.Set(rpc, 'cast', fake_cast) + + volume_id = self._create_volume() + self.volume.create_volume(self.context, volume_id) + db.volume_attached(self.context, volume_id, self.instance_id, + '/dev/sda1') + + volume_api = volume.api.API() + self.assertRaises(exception.ApiError, + volume_api.create_snapshot, + self.context, volume_id, + 'fake_name', 'fake_description') + snapshot_ref = volume_api.create_snapshot_force(self.context, + volume_id, + 'fake_name', + 'fake_description') + db.snapshot_destroy(self.context, snapshot_ref['id']) + db.volume_destroy(self.context, volume_id) + class DriverTestCase(test.TestCase): """Base Test class for Drivers.""" -- cgit From 1c4a3e14a0cef6938c477908d5c3bfe5ddf0e07b Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 23 Jun 2011 19:51:00 +0900 Subject: ec2utils: introduce helper function to prepend '/dev/' in mappings Introduce a helper function to prepend /dev/ to device name in block device mapping of bundle --- nova/api/ec2/ec2utils.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/nova/api/ec2/ec2utils.py b/nova/api/ec2/ec2utils.py index 9839f8604..bae1e0ee5 100644 --- a/nova/api/ec2/ec2utils.py +++ b/nova/api/ec2/ec2utils.py @@ -154,3 +154,13 @@ def properties_root_device_name(properties): root_device_name = properties['root_device_name'] return root_device_name + + +def mappings_prepend_dev(mappings): + """Prepend '/dev/' to 'device' entry of swap/ephemeral virtual type""" + for m in mappings: + virtual = m['virtual'] + if ((virtual == 'swap' or virtual.startswith('ephemeral')) and + (not m['device'].startswith('/'))): + m['device'] = '/dev/' + m['device'] + return mappings -- cgit From 4b5fdb2ee109960be6b3ff1fa8068ab3ec428283 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 23 Jun 2011 19:51:00 +0900 Subject: ec2: bundle block device mapping device name in block device mapping of bundle doesn't necessary carry "/dev/". So prepend it before processing. --- nova/api/ec2/cloud.py | 25 +++++++++++++++---------- nova/compute/api.py | 2 +- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index c25db9014..8bf0950b2 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -92,20 +92,25 @@ def _parse_block_device_mapping(bdm): """ ebs = bdm.pop('ebs', None) if ebs: - ec2_id = ebs.pop('snapshot_id') - id = ec2utils.ec2_id_to_id(ec2_id) - if ec2_id.startswith('snap-'): - bdm['snapshot_id'] = id - elif ec2_id.startswith('vol-'): - bdm['volume_id'] = id - ebs.setdefault('delete_on_termination', True) + ec2_id = ebs.pop('snapshot_id', None) + if ec2_id: + id = ec2utils.ec2_id_to_id(ec2_id) + if ec2_id.startswith('snap-'): + bdm['snapshot_id'] = id + elif ec2_id.startswith('vol-'): + bdm['volume_id'] = id + ebs.setdefault('delete_on_termination', True) bdm.update(ebs) return bdm +def _properties_get_mappings(properties): + return ec2utils.mappings_prepend_dev(properties.get('mappings', [])) + + def _format_block_device_mapping(bdm): """Contruct BlockDeviceMappingItemType - {'device_name': '...', 'Snapshot_Id': , ...} + {'device_name': '...', 'snapshot_id': , ...} => BlockDeviceMappingItemType """ keys = (('deviceName', 'device_name'), @@ -138,7 +143,7 @@ def _format_block_device_mapping(bdm): def _format_mappings(properties, result): """Format multiple BlockDeviceMappingItemType""" mappings = [{'virtualName': m['virtual'], 'deviceName': m['device']} - for m in properties.get('mappings', []) + for m in _properties_get_mappings(properties) if (m['virtual'] == 'swap' or m['virtual'].startswith('ephemeral'))] @@ -1381,7 +1386,7 @@ class CloudController(object): if m: mapping.append(m) - for m in properties.get('mappings', []): + for m in _properties_get_mappings(properties): virtual_name = m['virtual'] if virtual_name in ('ami', 'root'): continue diff --git a/nova/compute/api.py b/nova/compute/api.py index 884ec9198..b3635d71f 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -261,7 +261,7 @@ class API(base.Base): """tell vm driver to create ephemeral/swap device at boot time by updating BlockDeviceMapping """ - for bdm in mappings: + for bdm in ec2utils.mappings_prepend_dev(mappings): LOG.debug(_("bdm %s"), bdm) virtual_name = bdm['virtual'] -- cgit From 8e3da07f2af1fb4c0d5fcb58cb6747afaa6b76d8 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 23 Jun 2011 19:51:00 +0900 Subject: ec2utils: an unit test for mapping_prepend_dev() --- nova/tests/test_api.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/nova/tests/test_api.py b/nova/tests/test_api.py index ebc5508cc..26ac5ff24 100644 --- a/nova/tests/test_api.py +++ b/nova/tests/test_api.py @@ -153,6 +153,40 @@ class Ec2utilsTestCase(test.TestCase): root_device_name = ec2utils.properties_root_device_name(properties1) self.assertEqual(root_device_name, '/dev/sdb') + def test_mapping_prepend_dev(self): + mappings = [ + {'virtual': 'ami', + 'device': 'sda1'}, + {'virtual': 'root', + 'device': '/dev/sda1'}, + + {'virtual': 'swap', + 'device': 'sdb1'}, + {'virtual': 'swap', + 'device': '/dev/sdb2'}, + + {'virtual': 'ephemeral0', + 'device': 'sdc1'}, + {'virtual': 'ephemeral1', + 'device': '/dev/sdc1'}] + expected_result = [ + {'virtual': 'ami', + 'device': 'sda1'}, + {'virtual': 'root', + 'device': '/dev/sda1'}, + + {'virtual': 'swap', + 'device': '/dev/sdb1'}, + {'virtual': 'swap', + 'device': '/dev/sdb2'}, + + {'virtual': 'ephemeral0', + 'device': '/dev/sdc1'}, + {'virtual': 'ephemeral1', + 'device': '/dev/sdc1'}] + self.assertDictListMatch(ec2utils.mappings_prepend_dev(mappings), + expected_result) + class ApiEc2TestCase(test.TestCase): """Unit test for the cloud controller on an EC2 API""" -- cgit From c5761c6e983e539e5bb24ae6c0f3ea88faea676f Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 23 Jun 2011 19:51:01 +0900 Subject: ec2/cloud: an unit test for _format_instance_bdm() --- nova/tests/test_cloud.py | 206 +++++++++++++++++++++++++++-------------------- 1 file changed, 120 insertions(+), 86 deletions(-) diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index aed5aff6a..a179899ca 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -312,31 +312,7 @@ class CloudTestCase(test.TestCase): volumes.append(vol) return volumes - def _assertInstance(self, instance_id): - ec2_instance_id = ec2utils.id_to_ec2_id(instance_id) - result = self.cloud.describe_instances(self.context, - instance_id=[ec2_instance_id]) - result = result['reservationSet'][0] - self.assertEqual(len(result['instancesSet']), 1) - result = result['instancesSet'][0] - self.assertEqual(result['instanceId'], ec2_instance_id) - return (ec2_instance_id, result) - - def _assertEqualBlockDeviceMapping(self, expected, result): - self.assertEqual(len(expected), len(result)) - for x in expected: - found = False - for y in result: - if x['deviceName'] == y['deviceName']: - self.assertSubDictMatch(x, y) - found = True - break - self.assertTrue(found) - - def test_describe_instances_bdm(self): - """Make sure describe_instances works with root_device_name and - block device mappings - """ + def _setUpBlockDeviceMapping(self): inst1 = db.instance_create(self.context, {'image_ref': 1, 'root_device_name': '/dev/sdb1'}) @@ -345,7 +321,7 @@ class CloudTestCase(test.TestCase): 'root_device_name': '/dev/sdc1'}) instance_id = inst1['id'] - mappings = [ + mappings0 = [ {'instance_id': instance_id, 'device_name': '/dev/sdb1', 'snapshot_id': '1', @@ -384,61 +360,119 @@ class CloudTestCase(test.TestCase): 'device_name': '/dev/sdb9', 'virtual_name': 'ephemeral3'}] - volumes = self._block_device_mapping_create(instance_id, mappings) - - ec2_instance_id, result = self._assertInstance(instance_id) - expected_result = {'instanceId': ec2_instance_id, - 'rootDeviceName': '/dev/sdb1', - 'rootDeviceType': 'ebs'} - expected_block_device_mapping = [ - {'deviceName': '/dev/sdb1', - 'ebs': {'status': 'in-use', - 'deleteOnTermination': False, - 'volumeId': 2, - }}, - {'deviceName': '/dev/sdb2', - 'ebs': {'status': 'in-use', - 'deleteOnTermination': False, - 'volumeId': 3, - }}, - {'deviceName': '/dev/sdb3', - 'ebs': {'status': 'in-use', - 'deleteOnTermination': True, - 'volumeId': 5, - }}, - {'deviceName': '/dev/sdb4', - 'ebs': {'status': 'in-use', - 'deleteOnTermination': False, - 'volumeId': 7, - }}, - {'deviceName': '/dev/sdb5', - 'ebs': {'status': 'in-use', - 'deleteOnTermination': False, - 'volumeId': 9, - }}, - {'deviceName': '/dev/sdb6', - 'ebs': {'status': 'in-use', - 'deleteOnTermination': False, - 'volumeId': 11, }}] - # NOTE(yamahata): swap/ephemeral device case isn't supported yet. - self.assertSubDictMatch(expected_result, result) - self._assertEqualBlockDeviceMapping(expected_block_device_mapping, - result['blockDeviceMapping']) - - ec2_instance_id, result = self._assertInstance(inst2['id']) - expected_result = {'instanceId': ec2_instance_id, - 'rootDeviceName': '/dev/sdc1', - 'rootDeviceType': 'instance-store'} - self.assertSubDictMatch(expected_result, result) + volumes = self._block_device_mapping_create(instance_id, mappings0) + return (inst1, inst2, volumes) + def _tearDownBlockDeviceMapping(self, inst1, inst2, volumes): for vol in volumes: db.volume_destroy(self.context, vol['id']) - for bdm in db.block_device_mapping_get_all_by_instance(self.context, - instance_id): - db.block_device_mapping_destroy(self.context, bdm['id']) + for id in (inst1['id'], inst2['id']): + for bdm in db.block_device_mapping_get_all_by_instance( + self.context, id): + db.block_device_mapping_destroy(self.context, bdm['id']) db.instance_destroy(self.context, inst2['id']) db.instance_destroy(self.context, inst1['id']) + _expected_instance_bdm1 = { + 'instanceId': 'i-00000001', + 'rootDeviceName': '/dev/sdb1', + 'rootDeviceType': 'ebs'} + + _expected_block_device_mapping0 = [ + {'deviceName': '/dev/sdb1', + 'ebs': {'status': 'in-use', + 'deleteOnTermination': False, + 'volumeId': 2, + }}, + {'deviceName': '/dev/sdb2', + 'ebs': {'status': 'in-use', + 'deleteOnTermination': False, + 'volumeId': 3, + }}, + {'deviceName': '/dev/sdb3', + 'ebs': {'status': 'in-use', + 'deleteOnTermination': True, + 'volumeId': 5, + }}, + {'deviceName': '/dev/sdb4', + 'ebs': {'status': 'in-use', + 'deleteOnTermination': False, + 'volumeId': 7, + }}, + {'deviceName': '/dev/sdb5', + 'ebs': {'status': 'in-use', + 'deleteOnTermination': False, + 'volumeId': 9, + }}, + {'deviceName': '/dev/sdb6', + 'ebs': {'status': 'in-use', + 'deleteOnTermination': False, + 'volumeId': 11, }}] + # NOTE(yamahata): swap/ephemeral device case isn't supported yet. + + _expected_instance_bdm2 = { + 'instanceId': 'i-00000002', + 'rootDeviceName': '/dev/sdc1', + 'rootDeviceType': 'instance-store'} + + def test_format_instance_bdm(self): + (inst1, inst2, volumes) = self._setUpBlockDeviceMapping() + + result = {} + self.cloud._format_instance_bdm(self.context, inst1['id'], '/dev/sdb1', + result) + self.assertSubDictMatch( + {'rootDeviceType': self._expected_instance_bdm1['rootDeviceType']}, + result) + self._assertEqualBlockDeviceMapping( + self._expected_block_device_mapping0, result['blockDeviceMapping']) + + result = {} + self.cloud._format_instance_bdm(self.context, inst2['id'], '/dev/sdc1', + result) + self.assertSubDictMatch( + {'rootDeviceType': self._expected_instance_bdm2['rootDeviceType']}, + result) + + self._tearDownBlockDeviceMapping(inst1, inst2, volumes) + + def _assertInstance(self, instance_id): + ec2_instance_id = ec2utils.id_to_ec2_id(instance_id) + result = self.cloud.describe_instances(self.context, + instance_id=[ec2_instance_id]) + result = result['reservationSet'][0] + self.assertEqual(len(result['instancesSet']), 1) + result = result['instancesSet'][0] + self.assertEqual(result['instanceId'], ec2_instance_id) + return result + + def _assertEqualBlockDeviceMapping(self, expected, result): + self.assertEqual(len(expected), len(result)) + for x in expected: + found = False + for y in result: + if x['deviceName'] == y['deviceName']: + self.assertSubDictMatch(x, y) + found = True + break + self.assertTrue(found) + + def test_describe_instances_bdm(self): + """Make sure describe_instances works with root_device_name and + block device mappings + """ + (inst1, inst2, volumes) = self._setUpBlockDeviceMapping() + + result = self._assertInstance(inst1['id']) + self.assertSubDictMatch(self._expected_instance_bdm1, result) + self._assertEqualBlockDeviceMapping( + self._expected_block_device_mapping0, result['blockDeviceMapping']) + + result = self._assertInstance(inst2['id']) + self.assertSubDictMatch(self._expected_instance_bdm2, result) + + self._tearDownBlockDeviceMapping(inst1, inst2, volumes) + def test_describe_images(self): describe_images = self.cloud.describe_images @@ -481,17 +515,17 @@ class CloudTestCase(test.TestCase): mappings1 = [ {'device': '/dev/sda1', 'virtual': 'root'}, - {'device': '/dev/sdb0', 'virtual': 'ephemeral0'}, - {'device': '/dev/sdb1', 'virtual': 'ephemeral1'}, - {'device': '/dev/sdb2', 'virtual': 'ephemeral2'}, - {'device': '/dev/sdb3', 'virtual': 'ephemeral3'}, - {'device': '/dev/sdb4', 'virtual': 'ephemeral4'}, - - {'device': '/dev/sdc0', 'virtual': 'swap'}, - {'device': '/dev/sdc1', 'virtual': 'swap'}, - {'device': '/dev/sdc2', 'virtual': 'swap'}, - {'device': '/dev/sdc3', 'virtual': 'swap'}, - {'device': '/dev/sdc4', 'virtual': 'swap'}] + {'device': 'sdb0', 'virtual': 'ephemeral0'}, + {'device': 'sdb1', 'virtual': 'ephemeral1'}, + {'device': 'sdb2', 'virtual': 'ephemeral2'}, + {'device': 'sdb3', 'virtual': 'ephemeral3'}, + {'device': 'sdb4', 'virtual': 'ephemeral4'}, + + {'device': 'sdc0', 'virtual': 'swap'}, + {'device': 'sdc1', 'virtual': 'swap'}, + {'device': 'sdc2', 'virtual': 'swap'}, + {'device': 'sdc3', 'virtual': 'swap'}, + {'device': 'sdc4', 'virtual': 'swap'}] block_device_mapping1 = [ {'device_name': '/dev/sdb1', 'snapshot_id': 01234567}, {'device_name': '/dev/sdb2', 'volume_id': 01234567}, -- cgit From c13bb7c3bf2400c45d1b93141e67916c81296e38 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 23 Jun 2011 19:51:01 +0900 Subject: ec2/cloud: unit tests for parser/formatter of block device mapping This patch adds several unit tests for private functions in ec2/cloud.py. Which are used to parse/format block device mapping. _parse_block_device_mapping(), _format_block_device_mapping() and _format_mappings() --- nova/tests/test_bdm.py | 233 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 233 insertions(+) create mode 100644 nova/tests/test_bdm.py diff --git a/nova/tests/test_bdm.py b/nova/tests/test_bdm.py new file mode 100644 index 000000000..b258f6a75 --- /dev/null +++ b/nova/tests/test_bdm.py @@ -0,0 +1,233 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2011 Isaku Yamahata +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +Tests for Block Device Mapping Code. +""" + +from nova.api.ec2 import cloud +from nova import test + + +class BlockDeviceMappingEc2CloudTestCase(test.TestCase): + """Test Case for Block Device Mapping""" + + def setUp(self): + super(BlockDeviceMappingEc2CloudTestCase, self).setUp() + + def tearDown(self): + super(BlockDeviceMappingEc2CloudTestCase, self).tearDown() + + def _assertApply(self, action, bdm_list): + for bdm, expected_result in bdm_list: + self.assertDictMatch(action(bdm), expected_result) + + def test_parse_block_device_mapping(self): + bdm_list = [ + ({'device_name': '/dev/fake0', + 'ebs': {'snapshot_id': 'snap-12345678', + 'volume_size': 1}}, + {'device_name': '/dev/fake0', + 'snapshot_id': 0x12345678, + 'volume_size': 1, + 'delete_on_termination': True}), + + ({'device_name': '/dev/fake1', + 'ebs': {'snapshot_id': 'snap-23456789', + 'delete_on_termination': False}}, + {'device_name': '/dev/fake1', + 'snapshot_id': 0x23456789, + 'delete_on_termination': False}), + + ({'device_name': '/dev/fake2', + 'ebs': {'snapshot_id': 'vol-87654321', + 'volume_size': 2}}, + {'device_name': '/dev/fake2', + 'volume_id': 0x87654321, + 'volume_size': 2, + 'delete_on_termination': True}), + + ({'device_name': '/dev/fake3', + 'ebs': {'snapshot_id': 'vol-98765432', + 'delete_on_termination': False}}, + {'device_name': '/dev/fake3', + 'volume_id': 0x98765432, + 'delete_on_termination': False}), + + ({'device_name': '/dev/fake4', + 'ebs': {'no_device': True}}, + {'device_name': '/dev/fake4', + 'no_device': True}), + + ({'device_name': '/dev/fake5', + 'virtual_name': 'ephemeral0'}, + {'device_name': '/dev/fake5', + 'virtual_name': 'ephemeral0'}), + + ({'device_name': '/dev/fake6', + 'virtual_name': 'swap'}, + {'device_name': '/dev/fake6', + 'virtual_name': 'swap'}), + ] + self._assertApply(cloud._parse_block_device_mapping, bdm_list) + + def test_format_block_device_mapping(self): + bdm_list = [ + ({'device_name': '/dev/fake0', + 'snapshot_id': 0x12345678, + 'volume_size': 1, + 'delete_on_termination': True}, + {'deviceName': '/dev/fake0', + 'ebs': {'snapshotId': 'snap-12345678', + 'volumeSize': 1, + 'deleteOnTermination': True}}), + + ({'device_name': '/dev/fake1', + 'snapshot_id': 0x23456789}, + {'deviceName': '/dev/fake1', + 'ebs': {'snapshotId': 'snap-23456789'}}), + + ({'device_name': '/dev/fake2', + 'snapshot_id': 0x23456789, + 'delete_on_termination': False}, + {'deviceName': '/dev/fake2', + 'ebs': {'snapshotId': 'snap-23456789', + 'deleteOnTermination': False}}), + + ({'device_name': '/dev/fake3', + 'volume_id': 0x12345678, + 'volume_size': 1, + 'delete_on_termination': True}, + {'deviceName': '/dev/fake3', + 'ebs': {'snapshotId': 'vol-12345678', + 'volumeSize': 1, + 'deleteOnTermination': True}}), + + ({'device_name': '/dev/fake4', + 'volume_id': 0x23456789}, + {'deviceName': '/dev/fake4', + 'ebs': {'snapshotId': 'vol-23456789'}}), + + ({'device_name': '/dev/fake5', + 'volume_id': 0x23456789, + 'delete_on_termination': False}, + {'deviceName': '/dev/fake5', + 'ebs': {'snapshotId': 'vol-23456789', + 'deleteOnTermination': False}}), + ] + self._assertApply(cloud._format_block_device_mapping, bdm_list) + + def test_format_mapping(self): + properties = { + 'mappings': [ + {'virtual': 'ami', + 'device': 'sda1'}, + {'virtual': 'root', + 'device': '/dev/sda1'}, + + {'virtual': 'swap', + 'device': 'sdb1'}, + {'virtual': 'swap', + 'device': 'sdb2'}, + {'virtual': 'swap', + 'device': 'sdb3'}, + {'virtual': 'swap', + 'device': 'sdb4'}, + + {'virtual': 'ephemeral0', + 'device': 'sdc1'}, + {'virtual': 'ephemeral1', + 'device': 'sdc2'}, + {'virtual': 'ephemeral2', + 'device': 'sdc3'}, + ], + + 'block_device_mapping': [ + # root + {'device_name': '/dev/sda1', + 'snapshot_id': 0x12345678, + 'delete_on_termination': False}, + + + # overwrite swap + {'device_name': '/dev/sdb2', + 'snapshot_id': 0x23456789, + 'delete_on_termination': False}, + {'device_name': '/dev/sdb3', + 'snapshot_id': 0x3456789A}, + {'device_name': '/dev/sdb4', + 'no_device': True}, + + # overwrite ephemeral + {'device_name': '/dev/sdc2', + 'snapshot_id': 0x3456789A, + 'delete_on_termination': False}, + {'device_name': '/dev/sdc3', + 'snapshot_id': 0x456789AB}, + {'device_name': '/dev/sdc4', + 'no_device': True}, + + # volume + {'device_name': '/dev/sdd1', + 'snapshot_id': 0x87654321, + 'delete_on_termination': False}, + {'device_name': '/dev/sdd2', + 'snapshot_id': 0x98765432}, + {'device_name': '/dev/sdd3', + 'snapshot_id': 0xA9875463}, + {'device_name': '/dev/sdd4', + 'no_device': True}]} + + expected_result = { + 'blockDeviceMapping': [ + # root + {'deviceName': '/dev/sda1', + 'ebs': {'snapshotId': 'snap-12345678', + 'deleteOnTermination': False}}, + + # swap + {'deviceName': '/dev/sdb1', + 'virtualName': 'swap'}, + {'deviceName': '/dev/sdb2', + 'ebs': {'snapshotId': 'snap-23456789', + 'deleteOnTermination': False}}, + {'deviceName': '/dev/sdb3', + 'ebs': {'snapshotId': 'snap-3456789a'}}, + + # ephemeral + {'deviceName': '/dev/sdc1', + 'virtualName': 'ephemeral0'}, + {'deviceName': '/dev/sdc2', + 'ebs': {'snapshotId': 'snap-3456789a', + 'deleteOnTermination': False}}, + {'deviceName': '/dev/sdc3', + 'ebs': {'snapshotId': 'snap-456789ab'}}, + + # volume + {'deviceName': '/dev/sdd1', + 'ebs': {'snapshotId': 'snap-87654321', + 'deleteOnTermination': False}}, + {'deviceName': '/dev/sdd2', + 'ebs': {'snapshotId': 'snap-98765432'}}, + {'deviceName': '/dev/sdd3', + 'ebs': {'snapshotId': 'snap-a9875463'}}]} + + result = {} + cloud._format_mappings(properties, result) + print result + self.assertEqual(result['blockDeviceMapping'].sort(), + expected_result['blockDeviceMapping'].sort()) -- cgit From fd577b786c3a929300ae744858b57ccfed4fb2fc Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 23 Jun 2011 19:51:01 +0900 Subject: compute/api: an unit test for _update_{image_}bdm an unit test for _update_image_block_device_mapping() and _update_block_device_mapping() --- nova/tests/test_compute.py | 111 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index 439508b27..783261127 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -721,3 +721,114 @@ class ComputeTestCase(test.TestCase): LOG.info(_("After force-killing instances: %s"), instances) self.assertEqual(len(instances), 1) self.assertEqual(power_state.SHUTOFF, instances[0]['state']) + + @staticmethod + def _parse_db_block_device_mapping(bdm_ref): + attr_list = ('delete_on_termination', 'device_name', 'no_device', + 'virtual_name', 'volume_id', 'volume_size', 'snapshot_id') + bdm = {} + for attr in attr_list: + val = bdm_ref.get(attr, None) + if val: + bdm[attr] = val + + return bdm + + def test_update_block_device_mapping(self): + instance_id = self._create_instance() + mappings = [ + {'virtual': 'ami', 'device': 'sda1'}, + {'virtual': 'root', 'device': '/dev/sda1'}, + + {'virtual': 'swap', 'device': 'sdb1'}, + {'virtual': 'swap', 'device': 'sdb2'}, + {'virtual': 'swap', 'device': 'sdb3'}, + {'virtual': 'swap', 'device': 'sdb4'}, + + {'virtual': 'ephemeral0', 'device': 'sdc1'}, + {'virtual': 'ephemeral1', 'device': 'sdc2'}, + {'virtual': 'ephemeral2', 'device': 'sdc3'}] + block_device_mapping = [ + # root + {'device_name': '/dev/sda1', + 'snapshot_id': 0x12345678, + 'delete_on_termination': False}, + + + # overwrite swap + {'device_name': '/dev/sdb2', + 'snapshot_id': 0x23456789, + 'delete_on_termination': False}, + {'device_name': '/dev/sdb3', + 'snapshot_id': 0x3456789A}, + {'device_name': '/dev/sdb4', + 'no_device': True}, + + # overwrite ephemeral + {'device_name': '/dev/sdc2', + 'snapshot_id': 0x456789AB, + 'delete_on_termination': False}, + {'device_name': '/dev/sdc3', + 'snapshot_id': 0x56789ABC}, + {'device_name': '/dev/sdc4', + 'no_device': True}, + + # volume + {'device_name': '/dev/sdd1', + 'snapshot_id': 0x87654321, + 'delete_on_termination': False}, + {'device_name': '/dev/sdd2', + 'snapshot_id': 0x98765432}, + {'device_name': '/dev/sdd3', + 'snapshot_id': 0xA9875463}, + {'device_name': '/dev/sdd4', + 'no_device': True}] + + self.compute_api._update_image_block_device_mapping( + self.context, instance_id, mappings) + + bdms = [self._parse_db_block_device_mapping(bdm_ref) + for bdm_ref in db.block_device_mapping_get_all_by_instance( + self.context, instance_id)] + expected_result = [ + {'virtual_name': 'swap', 'device_name': '/dev/sdb1'}, + {'virtual_name': 'swap', 'device_name': '/dev/sdb2'}, + {'virtual_name': 'swap', 'device_name': '/dev/sdb3'}, + {'virtual_name': 'swap', 'device_name': '/dev/sdb4'}, + {'virtual_name': 'ephemeral0', 'device_name': '/dev/sdc1'}, + {'virtual_name': 'ephemeral1', 'device_name': '/dev/sdc2'}, + {'virtual_name': 'ephemeral2', 'device_name': '/dev/sdc3'}] + bdms.sort() + expected_result.sort() + self.assertDictListMatch(bdms, expected_result) + + self.compute_api._update_block_device_mapping( + self.context, instance_id, block_device_mapping) + bdms = [self._parse_db_block_device_mapping(bdm_ref) + for bdm_ref in db.block_device_mapping_get_all_by_instance( + self.context, instance_id)] + expected_result = [ + {'snapshot_id': 0x12345678, 'device_name': '/dev/sda1'}, + + {'virtual_name': 'swap', 'device_name': '/dev/sdb1'}, + {'snapshot_id': 0x23456789, 'device_name': '/dev/sdb2'}, + {'snapshot_id': 0x3456789A, 'device_name': '/dev/sdb3'}, + {'no_device': True, 'device_name': '/dev/sdb4'}, + + {'virtual_name': 'ephemeral0', 'device_name': '/dev/sdc1'}, + {'snapshot_id': 0x456789AB, 'device_name': '/dev/sdc2'}, + {'snapshot_id': 0x56789ABC, 'device_name': '/dev/sdc3'}, + {'no_device': True, 'device_name': '/dev/sdc4'}, + + {'snapshot_id': 0x87654321, 'device_name': '/dev/sdd1'}, + {'snapshot_id': 0x98765432, 'device_name': '/dev/sdd2'}, + {'snapshot_id': 0xA9875463, 'device_name': '/dev/sdd3'}, + {'no_device': True, 'device_name': '/dev/sdd4'}] + bdms.sort() + expected_result.sort() + self.assertDictListMatch(bdms, expected_result) + + for bdm in db.block_device_mapping_get_all_by_instance( + self.context, instance_id): + db.block_device_mapping_destroy(self.context, bdm['id']) + self.compute.terminate_instance(self.context, instance_id) -- cgit From 8a884121e6a7c5f03f51266632bb671603c9c9a0 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 23 Jun 2011 19:51:01 +0900 Subject: ec2/cloud: address review. - eliminated commented out lines in create_image() - added time out to create_image() , --- nova/api/ec2/cloud.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 8bf0950b2..b6274df0d 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -1323,8 +1323,6 @@ class CloudController(object): def create_image(self, context, instance_id, **kwargs): # NOTE(yamahata): name/description are ignored by register_image(), # do so here - #description = kwargs.get('name') - #description = kwargs.get('description') no_reboot = kwargs.get('no_reboot', False) ec2_instance_id = instance_id @@ -1345,11 +1343,18 @@ class CloudController(object): self.compute_api.stop(context, instance_id=instance_id) # wait instance for really stopped + start_time = time.time() while state_description != 'stopped': time.sleep(1) instance = self.compute_api.get(context, instance_id) state_description = instance['state_description'] - # NOTE(yamahata): timeout and error? + # NOTE(yamahata): timeout and error. 1 hour for now for safety. + # Is it too short/long? + # Or is there any better way? + timeout = 1 * 60 * 60 * 60 + if time.time() > start_time + timeout: + raise exception.ApiError( + _('Couldn\'t stop instance with in %d sec') % timeout) src_image = self._get_image(context, instance['image_ref']) properties = src_image['properties'] -- cgit From c0a750757b2b6121bb04c6355a01cb31967c15e2 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 24 Jun 2011 19:08:26 +0900 Subject: image/s3: typo --- nova/image/s3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/image/s3.py b/nova/image/s3.py index 86dd2e309..4a3df98ba 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -186,7 +186,7 @@ class S3ImageService(service.BaseImageService): key = bucket.get_key(manifest_path) manifest = key.get_contents_as_string() - manifest, image = self._s3_parse_manifest(metadata, context, manifest) + manifest, image = self._s3_parse_manifest(context, metadata, manifest) image_id = image['id'] def delayed_create(): -- cgit From ce3b1ffec9bab02e2988b69e7e361d76e56ec002 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 24 Jun 2011 19:08:26 +0900 Subject: ec2/cloud: typo --- nova/api/ec2/cloud.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index b6274df0d..a0a7db1f2 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -943,7 +943,7 @@ class CloudController(object): i['amiLaunchIndex'] = instance['launch_index'] i['displayName'] = instance['display_name'] i['displayDescription'] = instance['display_description'] - i['rootDeviceName'] = (instance['root_device_name'] or + i['rootDeviceName'] = (instance.get('root_device_name') or _DEFAULT_ROOT_DEVICE_NAME) self._format_instance_bdm(context, instance_id, i['rootDeviceName'], i) -- cgit From 02c0bf3b242395e63baf582b1f9c279eef4282d6 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Mon, 27 Jun 2011 20:21:40 +0900 Subject: sqlalchmey/migration: resolved version conflict --- .../versions/027_add_root_device_name.py | 47 ---------------------- .../versions/028_add_root_device_name.py | 47 ++++++++++++++++++++++ 2 files changed, 47 insertions(+), 47 deletions(-) delete mode 100644 nova/db/sqlalchemy/migrate_repo/versions/027_add_root_device_name.py create mode 100644 nova/db/sqlalchemy/migrate_repo/versions/028_add_root_device_name.py diff --git a/nova/db/sqlalchemy/migrate_repo/versions/027_add_root_device_name.py b/nova/db/sqlalchemy/migrate_repo/versions/027_add_root_device_name.py deleted file mode 100644 index 6b98b9890..000000000 --- a/nova/db/sqlalchemy/migrate_repo/versions/027_add_root_device_name.py +++ /dev/null @@ -1,47 +0,0 @@ -# Copyright 2011 OpenStack LLC. -# Copyright 2011 Isaku Yamahata -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -from sqlalchemy import Column, Integer, MetaData, Table, String - -meta = MetaData() - - -# Just for the ForeignKey and column creation to succeed, these are not the -# actual definitions of instances or services. -instances = Table('instances', meta, - Column('id', Integer(), primary_key=True, nullable=False), - ) - -# -# New Column -# -root_device_name = Column( - 'root_device_name', - String(length=255, convert_unicode=False, assert_unicode=None, - unicode_error=None, _warn_on_bytestring=False), - nullable=True) - - -def upgrade(migrate_engine): - # Upgrade operations go here. Don't create your own engine; - # bind migrate_engine to your metadata - meta.bind = migrate_engine - instances.create_column(root_device_name) - - -def downgrade(migrate_engine): - # Operations to reverse the above upgrade go here. - meta.bind = migrate_engine - instances.drop_column('root_device_name') diff --git a/nova/db/sqlalchemy/migrate_repo/versions/028_add_root_device_name.py b/nova/db/sqlalchemy/migrate_repo/versions/028_add_root_device_name.py new file mode 100644 index 000000000..6b98b9890 --- /dev/null +++ b/nova/db/sqlalchemy/migrate_repo/versions/028_add_root_device_name.py @@ -0,0 +1,47 @@ +# Copyright 2011 OpenStack LLC. +# Copyright 2011 Isaku Yamahata +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from sqlalchemy import Column, Integer, MetaData, Table, String + +meta = MetaData() + + +# Just for the ForeignKey and column creation to succeed, these are not the +# actual definitions of instances or services. +instances = Table('instances', meta, + Column('id', Integer(), primary_key=True, nullable=False), + ) + +# +# New Column +# +root_device_name = Column( + 'root_device_name', + String(length=255, convert_unicode=False, assert_unicode=None, + unicode_error=None, _warn_on_bytestring=False), + nullable=True) + + +def upgrade(migrate_engine): + # Upgrade operations go here. Don't create your own engine; + # bind migrate_engine to your metadata + meta.bind = migrate_engine + instances.create_column(root_device_name) + + +def downgrade(migrate_engine): + # Operations to reverse the above upgrade go here. + meta.bind = migrate_engine + instances.drop_column('root_device_name') -- cgit From 8c8e92b2662a3ab9aa2fd71bef48d95408ebb89b Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 8 Jul 2011 12:09:50 +0900 Subject: sqlalchemy/migrate: resolved version conflict --- .../versions/028_add_root_device_name.py | 47 ---------------------- .../versions/032_add_root_device_name.py | 47 ++++++++++++++++++++++ 2 files changed, 47 insertions(+), 47 deletions(-) delete mode 100644 nova/db/sqlalchemy/migrate_repo/versions/028_add_root_device_name.py create mode 100644 nova/db/sqlalchemy/migrate_repo/versions/032_add_root_device_name.py diff --git a/nova/db/sqlalchemy/migrate_repo/versions/028_add_root_device_name.py b/nova/db/sqlalchemy/migrate_repo/versions/028_add_root_device_name.py deleted file mode 100644 index 6b98b9890..000000000 --- a/nova/db/sqlalchemy/migrate_repo/versions/028_add_root_device_name.py +++ /dev/null @@ -1,47 +0,0 @@ -# Copyright 2011 OpenStack LLC. -# Copyright 2011 Isaku Yamahata -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -from sqlalchemy import Column, Integer, MetaData, Table, String - -meta = MetaData() - - -# Just for the ForeignKey and column creation to succeed, these are not the -# actual definitions of instances or services. -instances = Table('instances', meta, - Column('id', Integer(), primary_key=True, nullable=False), - ) - -# -# New Column -# -root_device_name = Column( - 'root_device_name', - String(length=255, convert_unicode=False, assert_unicode=None, - unicode_error=None, _warn_on_bytestring=False), - nullable=True) - - -def upgrade(migrate_engine): - # Upgrade operations go here. Don't create your own engine; - # bind migrate_engine to your metadata - meta.bind = migrate_engine - instances.create_column(root_device_name) - - -def downgrade(migrate_engine): - # Operations to reverse the above upgrade go here. - meta.bind = migrate_engine - instances.drop_column('root_device_name') diff --git a/nova/db/sqlalchemy/migrate_repo/versions/032_add_root_device_name.py b/nova/db/sqlalchemy/migrate_repo/versions/032_add_root_device_name.py new file mode 100644 index 000000000..6b98b9890 --- /dev/null +++ b/nova/db/sqlalchemy/migrate_repo/versions/032_add_root_device_name.py @@ -0,0 +1,47 @@ +# Copyright 2011 OpenStack LLC. +# Copyright 2011 Isaku Yamahata +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from sqlalchemy import Column, Integer, MetaData, Table, String + +meta = MetaData() + + +# Just for the ForeignKey and column creation to succeed, these are not the +# actual definitions of instances or services. +instances = Table('instances', meta, + Column('id', Integer(), primary_key=True, nullable=False), + ) + +# +# New Column +# +root_device_name = Column( + 'root_device_name', + String(length=255, convert_unicode=False, assert_unicode=None, + unicode_error=None, _warn_on_bytestring=False), + nullable=True) + + +def upgrade(migrate_engine): + # Upgrade operations go here. Don't create your own engine; + # bind migrate_engine to your metadata + meta.bind = migrate_engine + instances.create_column(root_device_name) + + +def downgrade(migrate_engine): + # Operations to reverse the above upgrade go here. + meta.bind = migrate_engine + instances.drop_column('root_device_name') -- cgit From 4f973269e84adb10ac3959ef255ecc60cc90620e Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 8 Jul 2011 15:36:18 +0900 Subject: nova/compute/api.py: fixed mismerge --- nova/compute/api.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 5350b8f28..8530deb2d 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -248,7 +248,7 @@ class API(base.Base): 'vm_mode': vm_mode, 'root_device_name': root_device_name} - return (num_instances, base_options, security_groups, image) + return (num_instances, base_options, image) def _update_image_block_device_mapping(self, elevated_context, instance_id, mappings): @@ -300,7 +300,7 @@ class API(base.Base): values) def create_db_entry_for_new_instance(self, context, image, base_options, - security_groups, block_device_mapping, num=1): + security_group, block_device_mapping, num=1): """Create an entry in the DB for this new instance, including any related table updates (such as security group, etc). @@ -401,8 +401,7 @@ class API(base.Base): """Provision the instances by passing the whole request to the Scheduler for execution. Returns a Reservation ID related to the creation of all of these instances.""" - num_instances, base_options, security_groups, image = \ - self._check_create_parameters( + num_instances, base_options, image = self._check_create_parameters( context, instance_type, image_href, kernel_id, ramdisk_id, min_count, max_count, @@ -440,8 +439,7 @@ class API(base.Base): Returns a list of instance dicts. """ - num_instances, base_options, security_groups, image = \ - self._check_create_parameters( + num_instances, base_options, image = self._check_create_parameters( context, instance_type, image_href, kernel_id, ramdisk_id, min_count, max_count, @@ -451,11 +449,12 @@ class API(base.Base): injected_files, admin_password, zone_blob, reservation_id) + block_device_mapping = block_device_mapping or [] instances = [] LOG.debug(_("Going to run %s instances..."), num_instances) for num in range(num_instances): instance = self.create_db_entry_for_new_instance(context, image, - base_options, security_groups, + base_options, security_group, block_device_mapping, num=num) instances.append(instance) instance_id = instance['id'] -- cgit From 58d7fa8bf8610ac2fa65e974061bf8ae78ca321f Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 8 Jul 2011 15:36:40 +0900 Subject: tests/test_cloud: make an unit test, test_create_image, happy --- nova/tests/test_cloud.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index bd308f865..77b0f0a2e 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -45,7 +45,8 @@ LOG = logging.getLogger('nova.tests.cloud') class CloudTestCase(test.TestCase): def setUp(self): super(CloudTestCase, self).setUp() - self.flags(connection_type='fake') + self.flags(connection_type='fake', + stub_network=True) self.conn = rpc.Connection.instance() -- cgit From d5307a2e1575778fcbfcf3d8ad65733be7544a54 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 8 Jul 2011 18:35:01 +0900 Subject: image/fake: added teardown method Unit tests may alter images in FakeImageService which has pre-defined images. Since some unit tests depend on those images, so it needs to be cleaned up after image alternation. Otherwise running many unit tests may fail. --- nova/image/fake.py | 11 ++++++++++- nova/test.py | 4 ++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/nova/image/fake.py b/nova/image/fake.py index c4b3d5fd6..28e912534 100644 --- a/nova/image/fake.py +++ b/nova/image/fake.py @@ -137,7 +137,11 @@ class _FakeImageService(service.BaseImageService): try: image_id = metadata['id'] except KeyError: - image_id = random.randint(0, 2 ** 31 - 1) + while True: + image_id = random.randint(0, 2 ** 31 - 1) + if not self.images.get(str(image_id)): + break + image_id = str(image_id) if self.images.get(image_id): @@ -176,3 +180,8 @@ _fakeImageService = _FakeImageService() def FakeImageService(): return _fakeImageService + + +def FakeImageService_reset(): + global _fakeImageService + _fakeImageService = _FakeImageService() diff --git a/nova/test.py b/nova/test.py index b2599d4be..9790b0aa1 100644 --- a/nova/test.py +++ b/nova/test.py @@ -31,6 +31,7 @@ import unittest import mox import nose.plugins.skip +import nova.image.fake import shutil import stubout from eventlet import greenthread @@ -119,6 +120,9 @@ class TestCase(unittest.TestCase): if hasattr(fake.FakeConnection, '_instance'): del fake.FakeConnection._instance + if FLAGS.image_service == 'nova.image.fake.FakeImageService': + nova.image.fake.FakeImageService_reset() + # Reset any overriden flags self.reset_flags() -- cgit From 3a89f16ea07ebfc3d2c4e08ff9072b5020a5d348 Mon Sep 17 00:00:00 2001 From: Anne Gentle Date: Wed, 13 Jul 2011 09:04:20 -0500 Subject: Add multinic doc and distributed scheduler doc to developer guide front page --- doc/source/devref/index.rst | 7 +++++-- doc/source/devref/multinic.rst | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/doc/source/devref/index.rst b/doc/source/devref/index.rst index 0a5a7a4d6..859d4e331 100644 --- a/doc/source/devref/index.rst +++ b/doc/source/devref/index.rst @@ -30,13 +30,16 @@ Programming HowTos and Tutorials addmethod.openstackapi -Programming Concepts --------------------- +Background Concepts for Nova +---------------------------- .. toctree:: :maxdepth: 3 + distributed_scheduler + multinic zone rabbit + API Reference ------------- diff --git a/doc/source/devref/multinic.rst b/doc/source/devref/multinic.rst index b3a82d341..43830258f 100644 --- a/doc/source/devref/multinic.rst +++ b/doc/source/devref/multinic.rst @@ -29,11 +29,11 @@ FlatDHCP Manager .. image:: /images/multinic_dhcp.png -FlatDHCP manager builds on the the Flat manager adding dnsmask (DNS and DHCP) and radvd (Router Advertisement) servers on the bridge for that network. The services run on the host that is assigned to that nework. The FlatDHCP manager will create its bridge as specified when the network was created on the network-host when the network host starts up or when a new network gets allocated to that host. Compute nodes will also create the bridges as necessary and connect instance VIFs to them. +FlatDHCP manager builds on the the Flat manager adding dnsmask (DNS and DHCP) and radvd (Router Advertisement) servers on the bridge for that network. The services run on the host that is assigned to that network. The FlatDHCP manager will create its bridge as specified when the network was created on the network-host when the network host starts up or when a new network gets allocated to that host. Compute nodes will also create the bridges as necessary and connect instance VIFs to them. VLAN Manager ------------ .. image:: /images/multinic_vlan.png -The VLAN manager sets up forwarding to/from a cloudpipe instance in addition to providing dnsmask (DNS and DHCP) and radvd (Router Advertisement) services for each network. The manager will create its bridge as specified when the network was created on the network-host when the network host starts up or when a new network gets allocated to that host. Compute nodes will also create the bridges as necessary and conenct instance VIFs to them. +The VLAN manager sets up forwarding to/from a cloudpipe instance in addition to providing dnsmask (DNS and DHCP) and radvd (Router Advertisement) services for each network. The manager will create its bridge as specified when the network was created on the network-host when the network host starts up or when a new network gets allocated to that host. Compute nodes will also create the bridges as necessary and connect instance VIFs to them. -- cgit From 6daf6d30dfeab459a0b672d909b115b1a5ce86c3 Mon Sep 17 00:00:00 2001 From: "Kevin L. Mitchell" Date: Wed, 13 Jul 2011 14:41:13 -0500 Subject: Augment rate limiting to allow greater flexibility through the api-paste.ini configuration --- nova/api/openstack/limits.py | 96 +++++++++++++++++++++++++++++++-- nova/tests/api/openstack/test_limits.py | 30 +++++++++-- 2 files changed, 116 insertions(+), 10 deletions(-) diff --git a/nova/api/openstack/limits.py b/nova/api/openstack/limits.py index d08287f6b..8642a28f9 100644 --- a/nova/api/openstack/limits.py +++ b/nova/api/openstack/limits.py @@ -31,8 +31,8 @@ from collections import defaultdict from webob.dec import wsgify from nova import quota +from nova import utils from nova import wsgi as base_wsgi -from nova import wsgi from nova.api.openstack import common from nova.api.openstack import faults from nova.api.openstack.views import limits as limits_views @@ -119,6 +119,8 @@ class Limit(object): 60 * 60 * 24: "DAY", } + UNIT_MAP = dict([(v, k) for k, v in UNITS.items()]) + def __init__(self, verb, uri, regex, value, unit): """ Initialize a new `Limit`. @@ -224,16 +226,30 @@ class RateLimitingMiddleware(base_wsgi.Middleware): is stored in memory for this implementation. """ - def __init__(self, application, limits=None): + def __init__(self, application, limits=None, limiter=None, **kwargs): """ Initialize new `RateLimitingMiddleware`, which wraps the given WSGI application and sets up the given limits. @param application: WSGI application to wrap - @param limits: List of dictionaries describing limits + @param limits: String describing limits + @param limiter: String identifying class for representing limits + + Other parameters are passed to the constructor for the limiter. """ base_wsgi.Middleware.__init__(self, application) - self._limiter = Limiter(limits or DEFAULT_LIMITS) + + # Select the limiter class + if limiter is None: + limiter = Limiter + else: + limiter = utils.import_class(limiter) + + # Parse the limits, if any are provided + if limits is not None: + limits = limiter.parse_limits(limits) + + self._limiter = limiter(limits or DEFAULT_LIMITS, **kwargs) @wsgify(RequestClass=wsgi.Request) def __call__(self, req): @@ -271,7 +287,7 @@ class Limiter(object): Rate-limit checking class which handles limits in memory. """ - def __init__(self, limits): + def __init__(self, limits, **kwargs): """ Initialize the new `Limiter`. @@ -280,6 +296,12 @@ class Limiter(object): self.limits = copy.deepcopy(limits) self.levels = defaultdict(lambda: copy.deepcopy(limits)) + # Pick up any per-user limit information + for key, value in kwargs.items(): + if key.startswith('user:'): + username = key[5:] + self.levels[username] = self.parse_limits(value) + def get_limits(self, username=None): """ Return the limits for a given user. @@ -305,6 +327,59 @@ class Limiter(object): return None, None + @staticmethod + def parse_limits(limits): + """ + Convert a string into a list of Limit instances. This + implementation expects a semicolon-separated sequence of + parenthesized groups, where each group contains a + comma-separated sequence consisting of HTTP method, + user-readable URI, a URI reg-exp, an integer number of + requests which can be made, and a unit of measure. Valid + values for the latter are "SECOND", "MINUTE", "HOUR", and + "DAY". + + @return: List of Limit instances. + """ + + # Handle empty limit strings + limits = limits.strip() + if not limits: + return [] + + # Split up the limits by semicolon + result = [] + for group in limits.split(';'): + group = group.strip() + if group[0] != '(' or group[-1] != ')': + raise ValueError("Groups must be surrounded by parentheses") + group = group[1:-1] + + # Extract the Limit arguments + args = [a.strip() for a in group.split(',')] + if len(args) != 5: + raise ValueError("Groups must contain exactly 5 elements") + + # Pull out the arguments + verb, uri, regex, value, unit = args + + # Upper-case the verb + verb = verb.upper() + + # Convert value--raises ValueError if it's not integer + value = int(value) + + # Convert unit + unit = unit.upper() + if unit not in Limit.UNIT_MAP: + raise ValueError("Invalid units specified") + unit = Limit.UNIT_MAP[unit] + + # Build a limit + result.append(Limit(verb, uri, regex, value, unit)) + + return result + class WsgiLimiter(object): """ @@ -388,3 +463,14 @@ class WsgiLimiterProxy(object): return None, None return resp.getheader("X-Wait-Seconds"), resp.read() or None + + @staticmethod + def parse_limits(limits): + """ + Ignore a limits string--simply doesn't apply for the limit + proxy. + + @return: Empty list. + """ + + return [] diff --git a/nova/tests/api/openstack/test_limits.py b/nova/tests/api/openstack/test_limits.py index 38c959fae..eeacd2fca 100644 --- a/nova/tests/api/openstack/test_limits.py +++ b/nova/tests/api/openstack/test_limits.py @@ -400,6 +400,10 @@ class LimitsControllerV11Test(BaseLimitTestSuite): self._test_index_absolute_limits_json(expected) +class TestLimiter(limits.Limiter): + pass + + class LimitMiddlewareTest(BaseLimitTestSuite): """ Tests for the `limits.RateLimitingMiddleware` class. @@ -413,10 +417,14 @@ class LimitMiddlewareTest(BaseLimitTestSuite): def setUp(self): """Prepare middleware for use through fake WSGI app.""" BaseLimitTestSuite.setUp(self) - _limits = [ - limits.Limit("GET", "*", ".*", 1, 60), - ] - self.app = limits.RateLimitingMiddleware(self._empty_app, _limits) + _limits = '(GET, *, .*, 1, MINUTE)' + self.app = limits.RateLimitingMiddleware(self._empty_app, _limits, + "%s.TestLimiter" % + self.__class__.__module__) + + def test_limit_class(self): + """Test that middleware selected correct limiter class.""" + assert isinstance(self.app._limiter, TestLimiter) def test_good_request(self): """Test successful GET request through middleware.""" @@ -500,7 +508,8 @@ class LimiterTest(BaseLimitTestSuite): def setUp(self): """Run before each test.""" BaseLimitTestSuite.setUp(self) - self.limiter = limits.Limiter(TEST_LIMITS) + userlimits = {'user:user3': ''} + self.limiter = limits.Limiter(TEST_LIMITS, **userlimits) def _check(self, num, verb, url, username=None): """Check and yield results from checks.""" @@ -605,6 +614,12 @@ class LimiterTest(BaseLimitTestSuite): results = list(self._check(10, "PUT", "/anything")) self.assertEqual(expected, results) + def test_user_limit(self): + """ + Test user-specific limits. + """ + self.assertEqual(self.limiter.levels['user3'], []) + def test_multiple_users(self): """ Tests involving multiple users. @@ -619,6 +634,11 @@ class LimiterTest(BaseLimitTestSuite): results = list(self._check(15, "PUT", "/anything", "user2")) self.assertEqual(expected, results) + # User3 + expected = [None] * 20 + results = list(self._check(20, "PUT", "/anything", "user3")) + self.assertEqual(expected, results) + self.time += 1.0 # User1 again -- cgit From 1539866314393e8565eef05f1f63dba9ffa69de3 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Thu, 14 Jul 2011 12:03:06 -0400 Subject: adding bookmark to images index --- nova/api/openstack/views/images.py | 18 +++++++++++------- nova/tests/api/openstack/test_images.py | 14 ++++++++++---- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/nova/api/openstack/views/images.py b/nova/api/openstack/views/images.py index 5c0510377..873ce212a 100644 --- a/nova/api/openstack/views/images.py +++ b/nova/api/openstack/views/images.py @@ -121,16 +121,20 @@ class ViewBuilderV11(ViewBuilder): href = self.generate_href(image_obj["id"]) bookmark = self.generate_bookmark(image_obj["id"]) - image["links"] = [{ - "rel": "self", - "href": href, - }] + image["links"] = [ + { + "rel": "self", + "href": href, + }, + { + "rel": "bookmark", + "href": bookmark, + }, + + ] if detail: image["metadata"] = image_obj.get("properties", {}) - image["links"].append({"rel": "bookmark", - "href": bookmark, - }) return image diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index c1bdd6906..534460d46 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -568,10 +568,16 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): test_image = { "id": image["id"], "name": image["name"], - "links": [{ - "rel": "self", - "href": href, - }], + "links": [ + { + "rel": "self", + "href": href, + }, + { + "rel": "bookmark", + "href": bookmark, + }, + ], } self.assertTrue(test_image in response_list) -- cgit From cbf05e0b6351c9577e7e992da072d190c8c9a592 Mon Sep 17 00:00:00 2001 From: "Kevin L. Mitchell" Date: Thu, 14 Jul 2011 11:37:32 -0500 Subject: Comment on parse_limits(); expand an exception message; add unit tests; fix a minor discovered bug --- nova/api/openstack/limits.py | 18 ++++++-- nova/tests/api/openstack/test_limits.py | 81 +++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 3 deletions(-) diff --git a/nova/api/openstack/limits.py b/nova/api/openstack/limits.py index 8642a28f9..bc76547d8 100644 --- a/nova/api/openstack/limits.py +++ b/nova/api/openstack/limits.py @@ -327,6 +327,11 @@ class Limiter(object): return None, None + # Note: This method gets called before the class is instantiated, + # so this must be either a static method or a class method. It is + # used to develop a list of limits to feed to the constructor. We + # put this in the class so that subclasses can override the + # default limit parsing. @staticmethod def parse_limits(limits): """ @@ -351,14 +356,16 @@ class Limiter(object): result = [] for group in limits.split(';'): group = group.strip() - if group[0] != '(' or group[-1] != ')': - raise ValueError("Groups must be surrounded by parentheses") + if group[:1] != '(' or group[-1:] != ')': + raise ValueError("Limit rules must be surrounded by " + "parentheses") group = group[1:-1] # Extract the Limit arguments args = [a.strip() for a in group.split(',')] if len(args) != 5: - raise ValueError("Groups must contain exactly 5 elements") + raise ValueError("Limit rules must contain the following " + "arguments: verb, uri, regex, value, unit") # Pull out the arguments verb, uri, regex, value, unit = args @@ -464,6 +471,11 @@ class WsgiLimiterProxy(object): return resp.getheader("X-Wait-Seconds"), resp.read() or None + # Note: This method gets called before the class is instantiated, + # so this must be either a static method or a class method. It is + # used to develop a list of limits to feed to the constructor. + # This implementation returns an empty list, since all limit + # decisions are made by a remote server. @staticmethod def parse_limits(limits): """ diff --git a/nova/tests/api/openstack/test_limits.py b/nova/tests/api/openstack/test_limits.py index eeacd2fca..e0368d5f5 100644 --- a/nova/tests/api/openstack/test_limits.py +++ b/nova/tests/api/openstack/test_limits.py @@ -500,6 +500,87 @@ class LimitTest(BaseLimitTestSuite): self.assertEqual(4, limit.last_request) +class ParseLimitsTest(BaseLimitTestSuite): + """ + Tests for the default limits parser in the in-memory + `limits.Limiter` class. + """ + + def test_invalid(self): + """Test that parse_limits() handles invalid input correctly.""" + try: + limits.Limiter.parse_limits(';;;;;') + except ValueError: + return + assert False, "Failure to reject invalid input" + + def test_bad_rule(self): + """Test that parse_limits() handles bad rules correctly.""" + try: + limits.Limiter.parse_limits('GET, *, .*, 20, minute') + except ValueError: + return + assert False, "Failure to reject bad rule" + + def test_missing_arg(self): + """Test that parse_limits() handles missing args correctly.""" + try: + limits.Limiter.parse_limits('(GET, *, .*, 20)') + except ValueError: + return + assert False, "Failure to reject missing rule argument" + + def test_bad_value(self): + """Test that parse_limits() handles bad values correctly.""" + try: + limits.Limiter.parse_limits('(GET, *, .*, foo, minute)') + except ValueError: + return + assert False, "Failure to reject invalid value" + + def test_bad_unit(self): + """Test that parse_limits() handles bad units correctly.""" + try: + limits.Limiter.parse_limits('(GET, *, .*, 20, lightyears)') + except ValueError: + return + assert False, "Failure to reject invalid unit" + + def test_multiple_rules(self): + """Test that parse_limits() handles multiple rules correctly.""" + try: + l = limits.Limiter.parse_limits('(get, *, .*, 20, minute);' + '(PUT, /foo*, /foo.*, 10, hour);' + '(POST, /bar*, /bar.*, 5, second);' + '(Say, /derp*, /derp.*, 1, day)') + except ValueError, e: + assert False, str(e) + + # Make sure the number of returned limits are correct + self.assertEqual(len(l), 4) + + # Check all the verbs... + expected = ['GET', 'PUT', 'POST', 'SAY'] + self.assertEqual([t.verb for t in l], expected) + + # ...the URIs... + expected = ['*', '/foo*', '/bar*', '/derp*'] + self.assertEqual([t.uri for t in l], expected) + + # ...the regexes... + expected = ['.*', '/foo.*', '/bar.*', '/derp.*'] + self.assertEqual([t.regex for t in l], expected) + + # ...the values... + expected = [20, 10, 5, 1] + self.assertEqual([t.value for t in l], expected) + + # ...and the units... + expected = [limits.PER_MINUTE, limits.PER_HOUR, + limits.PER_SECOND, limits.PER_DAY] + self.assertEqual([t.unit for t in l], expected) + + class LimiterTest(BaseLimitTestSuite): """ Tests for the in-memory `limits.Limiter` class. -- cgit From ac6dcb5a8802d53390584fcde8cba8ca74c1d0d0 Mon Sep 17 00:00:00 2001 From: "Kevin L. Mitchell" Date: Thu, 14 Jul 2011 12:40:38 -0500 Subject: Use assertRaises instead of try/except--stupid brain-o --- nova/tests/api/openstack/test_limits.py | 35 ++++++++++----------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/nova/tests/api/openstack/test_limits.py b/nova/tests/api/openstack/test_limits.py index e0368d5f5..76363450d 100644 --- a/nova/tests/api/openstack/test_limits.py +++ b/nova/tests/api/openstack/test_limits.py @@ -508,43 +508,28 @@ class ParseLimitsTest(BaseLimitTestSuite): def test_invalid(self): """Test that parse_limits() handles invalid input correctly.""" - try: - limits.Limiter.parse_limits(';;;;;') - except ValueError: - return - assert False, "Failure to reject invalid input" + self.assertRaises(ValueError, limits.Limiter.parse_limits, + ';;;;;') def test_bad_rule(self): """Test that parse_limits() handles bad rules correctly.""" - try: - limits.Limiter.parse_limits('GET, *, .*, 20, minute') - except ValueError: - return - assert False, "Failure to reject bad rule" + self.assertRaises(ValueError, limits.Limiter.parse_limits, + 'GET, *, .*, 20, minute') def test_missing_arg(self): """Test that parse_limits() handles missing args correctly.""" - try: - limits.Limiter.parse_limits('(GET, *, .*, 20)') - except ValueError: - return - assert False, "Failure to reject missing rule argument" + self.assertRaises(ValueError, limits.Limiter.parse_limits, + '(GET, *, .*, 20)') def test_bad_value(self): """Test that parse_limits() handles bad values correctly.""" - try: - limits.Limiter.parse_limits('(GET, *, .*, foo, minute)') - except ValueError: - return - assert False, "Failure to reject invalid value" + self.assertRaises(ValueError, limits.Limiter.parse_limits, + '(GET, *, .*, foo, minute)') def test_bad_unit(self): """Test that parse_limits() handles bad units correctly.""" - try: - limits.Limiter.parse_limits('(GET, *, .*, 20, lightyears)') - except ValueError: - return - assert False, "Failure to reject invalid unit" + self.assertRaises(ValueError, limits.Limiter.parse_limits, + '(GET, *, .*, 20, lightyears)') def test_multiple_rules(self): """Test that parse_limits() handles multiple rules correctly.""" -- cgit From a48041f7b587b91413a138264e0ec31ba4dcc78a Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Fri, 15 Jul 2011 04:33:20 -0400 Subject: Adding unit and integration tests for updating the server name via the 1.1 api. --- nova/tests/api/openstack/test_servers.py | 18 +++++++++++++++++- nova/tests/integrated/api/client.py | 14 ++++++++++++++ nova/tests/integrated/test_servers.py | 19 +++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 775f66ad0..235b0a2ca 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -901,7 +901,7 @@ class ServersTest(test.TestCase): res = req.get_response(fakes.wsgi_app()) self.assertEqual(res.status_int, 400) - def test_update_no_body(self): + def test_update_server_no_body(self): req = webob.Request.blank('/v1.0/servers/1') req.method = 'PUT' res = req.get_response(fakes.wsgi_app()) @@ -967,6 +967,21 @@ class ServersTest(test.TestCase): self.assertEqual(mock_method.instance_id, '1') self.assertEqual(mock_method.password, 'bacon') + def test_update_server_no_body_v1_1(self): + req = webob.Request.blank('/v1.0/servers/1') + req.method = 'PUT' + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 400) + + def test_update_server_name_v1_1(self): + req = webob.Request.blank('/v1.1/servers/1') + req.method = 'PUT' + req.content_type = 'application/json' + req.body = json.dumps({'server': {'name': 'new-name'}}) + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 204) + self.assertEqual(res.body, '') + def test_update_server_adminPass_ignored_v1_1(self): inst_dict = dict(name='server_test', adminPass='bacon') self.body = json.dumps(dict(server=inst_dict)) @@ -985,6 +1000,7 @@ class ServersTest(test.TestCase): req.body = self.body res = req.get_response(fakes.wsgi_app()) self.assertEqual(res.status_int, 204) + self.assertEqual(res.body, '') def test_create_backup_schedules(self): req = webob.Request.blank('/v1.0/servers/1/backup_schedule') diff --git a/nova/tests/integrated/api/client.py b/nova/tests/integrated/api/client.py index 59cc3b564..035a35aab 100644 --- a/nova/tests/integrated/api/client.py +++ b/nova/tests/integrated/api/client.py @@ -172,6 +172,17 @@ class TestOpenStackClient(object): response = self.api_request(relative_uri, **kwargs) return self._decode_json(response) + def api_put(self, relative_uri, body, **kwargs): + kwargs['method'] = 'PUT' + if body: + headers = kwargs.setdefault('headers', {}) + headers['Content-Type'] = 'application/json' + kwargs['body'] = json.dumps(body) + + kwargs.setdefault('check_response_status', [200, 202, 204]) + response = self.api_request(relative_uri, **kwargs) + return self._decode_json(response) + def api_delete(self, relative_uri, **kwargs): kwargs['method'] = 'DELETE' kwargs.setdefault('check_response_status', [200, 202, 204]) @@ -187,6 +198,9 @@ class TestOpenStackClient(object): def post_server(self, server): return self.api_post('/servers', server)['server'] + def put_server(self, server_id, server): + return self.api_put('/servers/%s' % server_id, server) + def post_server_action(self, server_id, data): return self.api_post('/servers/%s/action' % server_id, data) diff --git a/nova/tests/integrated/test_servers.py b/nova/tests/integrated/test_servers.py index fcb517cf5..4e8e85c7b 100644 --- a/nova/tests/integrated/test_servers.py +++ b/nova/tests/integrated/test_servers.py @@ -285,6 +285,25 @@ class ServersTest(integrated_helpers._IntegratedTestBase): # Cleanup self._delete_server(created_server_id) + def test_rename_server(self): + """Test building and renaming a server.""" + + # Create a server + server = self._build_minimal_create_server_request() + created_server = self.api.post_server({'server': server}) + LOG.debug("created_server: %s" % created_server) + server_id = created_server['id'] + self.assertTrue(server_id) + + # Rename the server to 'new-name' + self.api.put_server(server_id, {'server': {'name': 'new-name'}}) + + # Check the name of the server + created_server = self.api.get_server(server_id) + self.assertEqual(created_server['name'], 'new-name') + + # Cleanup + self._delete_server(server_id) if __name__ == "__main__": unittest.main() -- cgit From 58eb29ebe4376a276a54f4fd984802a0e50fb8e3 Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Fri, 15 Jul 2011 15:05:46 -0400 Subject: Updates to the XenServer agent plugin to fix file injection: -Update _agent_has_method so that it parses the features 'message' from nova-agent correctly. (it was trying to call .split on a dict). -Rip out the agent_has_method caching functionality which just plain isn't working with XenServer 5.6 SP2. -Pass the arg_dict to _agent_has_method. This fixes an issue where a subsequent call to xenstore.write_record didn't get the 'dom_id' (KeyError). -Fix a string formatting issue in inject_file in creating the b64 data. --- plugins/xenserver/xenapi/etc/xapi.d/plugins/agent | 44 ++++++++++------------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent b/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent index 68d7e7bff..b169a74bf 100755 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent @@ -37,7 +37,7 @@ import time import XenAPIPlugin from pluginlib_nova import * -configure_logging("xenstore") +configure_logging("agent") import xenstore AGENT_TIMEOUT = 30 @@ -129,18 +129,18 @@ def inject_file(self, arg_dict): b64_path = arg_dict["b64_path"] b64_file = arg_dict["b64_contents"] request_id = arg_dict["id"] - if self._agent_has_method("file_inject"): + if _agent_has_method(self, "file_inject", arg_dict): # New version of the agent. Agent should receive a 'value' # key whose value is a dictionary containing 'b64_path' and # 'b64_file'. See old version below. arg_dict["value"] = json.dumps({"name": "file_inject", "value": {"b64_path": b64_path, "b64_file": b64_file}}) - elif self._agent_has_method("injectfile"): + elif _agent_has_method(self, "injectfile", arg_dict): # Old agent requires file path and file contents to be # combined into one base64 value. raw_path = base64.b64decode(b64_path) raw_file = base64.b64decode(b64_file) - new_b64 = base64.b64encode("%s,%s") % (raw_path, raw_file) + new_b64 = base64.b64encode("%s,%s" % (raw_path, raw_file)) arg_dict["value"] = json.dumps({"name": "injectfile", "value": new_b64}) else: @@ -174,30 +174,24 @@ def agent_update(self, arg_dict): return resp -def _agent_has_method(self, method): +def _agent_has_method(self, method, arg_dict): """Check that the agent has a particular method by checking its - features. Cache the features so we don't have to query the agent - every time we need to check. + features. """ + tmp_id = commands.getoutput("uuidgen") + dct = {} + dct.update(arg_dict) + dct["value"] = json.dumps({"name": "features", "value": ""}) + dct["path"] = "data/host/%s" % tmp_id + xenstore.write_record(self, dct) try: - self._agent_methods - except AttributeError: - self._agent_methods = [] - if not self._agent_methods: - # Haven't been defined - tmp_id = commands.getoutput("uuidgen") - dct = {} - dct["value"] = json.dumps({"name": "features", "value": ""}) - dct["path"] = "data/host/%s" % tmp_id - xenstore.write_record(self, dct) - try: - resp = _wait_for_agent(self, tmp_id, dct) - except TimeoutError, e: - raise PluginError(e) - response = json.loads(resp) - # The agent returns a comma-separated list of methods. - self._agent_methods = response.split(",") - return method in self._agent_methods + resp = _wait_for_agent(self, tmp_id, dct) + except TimeoutError, e: + raise PluginError(e) + response = json.loads(resp) + method_arr = response["message"].split(",") + # The agent returns a comma-separated list of methods in the message + return method in method_arr def _wait_for_agent(self, request_id, arg_dict): -- cgit From 3233d6cafb22305f09c1384ba30e677751cace6a Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Sat, 16 Jul 2011 23:30:47 -0400 Subject: Change _agent_has_method to _get_agent_features. Update the inject files function so that it calls _get_agent_features only once per injected file. --- plugins/xenserver/xenapi/etc/xapi.d/plugins/agent | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent b/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent index b169a74bf..1321c820e 100755 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent @@ -129,13 +129,14 @@ def inject_file(self, arg_dict): b64_path = arg_dict["b64_path"] b64_file = arg_dict["b64_contents"] request_id = arg_dict["id"] - if _agent_has_method(self, "file_inject", arg_dict): + agent_features = _get_agent_features(self, arg_dict) + if "file_inject" in agent_features: # New version of the agent. Agent should receive a 'value' # key whose value is a dictionary containing 'b64_path' and # 'b64_file'. See old version below. arg_dict["value"] = json.dumps({"name": "file_inject", "value": {"b64_path": b64_path, "b64_file": b64_file}}) - elif _agent_has_method(self, "injectfile", arg_dict): + elif "injectfile" in agent_features: # Old agent requires file path and file contents to be # combined into one base64 value. raw_path = base64.b64decode(b64_path) @@ -174,10 +175,8 @@ def agent_update(self, arg_dict): return resp -def _agent_has_method(self, method, arg_dict): - """Check that the agent has a particular method by checking its - features. - """ +def _get_agent_features(self, arg_dict): + """Return an array of features that an agent supports.""" tmp_id = commands.getoutput("uuidgen") dct = {} dct.update(arg_dict) @@ -189,9 +188,7 @@ def _agent_has_method(self, method, arg_dict): except TimeoutError, e: raise PluginError(e) response = json.loads(resp) - method_arr = response["message"].split(",") - # The agent returns a comma-separated list of methods in the message - return method in method_arr + return response["message"].split(",") def _wait_for_agent(self, request_id, arg_dict): -- cgit From b927674112849e7b4ebbd59c188d8f7a1eb47e2a Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Mon, 18 Jul 2011 09:38:50 -0400 Subject: Check returncode in get_agent_features. --- plugins/xenserver/xenapi/etc/xapi.d/plugins/agent | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent b/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent index 1321c820e..41225e6f3 100755 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent @@ -188,7 +188,10 @@ def _get_agent_features(self, arg_dict): except TimeoutError, e: raise PluginError(e) response = json.loads(resp) - return response["message"].split(",") + if response['returncode'] != '0': + return response["message"].split(",") + else: + return {} def _wait_for_agent(self, request_id, arg_dict): -- cgit From cf14d867673e944cc0c0d5fc160d2fbcfe56e98e Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Mon, 18 Jul 2011 10:29:49 -0400 Subject: returncode is an integer. --- plugins/xenserver/xenapi/etc/xapi.d/plugins/agent | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent b/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent index 41225e6f3..8ec4fb061 100755 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent @@ -188,7 +188,7 @@ def _get_agent_features(self, arg_dict): except TimeoutError, e: raise PluginError(e) response = json.loads(resp) - if response['returncode'] != '0': + if response['returncode'] != 0: return response["message"].split(",") else: return {} -- cgit From 813253e8bea9a8db9c1df45f9aa5e094503a015a Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Mon, 18 Jul 2011 13:18:16 -0400 Subject: Don't jsonify the inject_file response. It is already json. --- plugins/xenserver/xenapi/etc/xapi.d/plugins/agent | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent b/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent index 8ec4fb061..288ccc78a 100755 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/agent @@ -114,7 +114,6 @@ def resetnetwork(self, arg_dict): xenstore.write_record(self, arg_dict) -@jsonify def inject_file(self, arg_dict): """Expects a file path and the contents of the file to be written. Both should be base64-encoded in order to eliminate errors as they are passed -- cgit