From 5e6c69bc7a7e5ddaa1bf0fa83f64da116343dba8 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Tue, 29 Mar 2011 16:35:39 -0700 Subject: Narrowly focused bugfix - don't lose libvirt instances on host reboot or if they crash --- nova/compute/manager.py | 14 ++++++----- nova/virt/libvirt_conn.py | 59 +++++++++++++++++++++++++++++++++++++---------- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index e0a5e2b3f..72f04ecb1 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1088,12 +1088,14 @@ class ComputeManager(manager.SchedulerDependentManager): db_instance['id'], vm_state) - if vm_state == power_state.SHUTOFF: - # TODO(soren): This is what the compute manager does when you - # terminate an instance. At some point I figure we'll have a - # "terminated" state and some sort of cleanup job that runs - # occasionally, cleaning them out. - self.db.instance_destroy(context, db_instance['id']) + # NOTE(justinsb): We no longer auto-remove SHUTOFF instances + # It's quite hard to get them back when we do. + #if vm_state == power_state.SHUTOFF: + # # TODO(soren): This is what the compute manager does when you + # # terminate an instance. At some point I figure we'll have a + # # "terminated" state and some sort of cleanup job that runs + # # occasionally, cleaning them out. + # self.db.instance_destroy(context, db_instance['id']) # Are there VMs not in the DB? for vm_not_found_in_db in vms_not_found_in_db: diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index c144e827e..533ff9394 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -116,6 +116,8 @@ flags.DEFINE_integer('live_migration_bandwidth', 0, 'Define live migration behavior') flags.DEFINE_string('qemu_img', 'qemu-img', 'binary to use for qemu-img commands') +flags.DEFINE_bool('start_guests_on_host_boot', False, + 'Whether to restart guests when the host reboots') def get_connection(read_only): @@ -230,12 +232,14 @@ class LibvirtConnection(driver.ComputeDriver): {'name': instance['name'], 'state': state}) db.instance_set_state(ctxt, instance['id'], state) - if state == power_state.SHUTOFF: - # TODO(soren): This is what the compute manager does when you - # terminate # an instance. At some point I figure we'll have a - # "terminated" state and some sort of cleanup job that runs - # occasionally, cleaning them out. - db.instance_destroy(ctxt, instance['id']) + # NOTE(justinsb): We no longer delete these instances, + # the user may want to power them back on + #if state == power_state.SHUTOFF: + # # TODO(soren): This is what the compute manager does when you + # # terminate # an instance. At some point I figure we'll have a + # # "terminated" state and some sort of cleanup job that runs + # # occasionally, cleaning them out. + # db.instance_destroy(ctxt, instance['id']) if state != power_state.RUNNING: continue @@ -474,7 +478,7 @@ class LibvirtConnection(driver.ComputeDriver): xml = self.to_xml(instance) self.firewall_driver.setup_basic_filtering(instance) self.firewall_driver.prepare_instance_filter(instance) - self._conn.createXML(xml, 0) + self._create_new_domain(xml) self.firewall_driver.apply_instance_filter(instance) timer = utils.LoopingCall(f=None) @@ -522,7 +526,7 @@ class LibvirtConnection(driver.ComputeDriver): 'kernel_id': FLAGS.rescue_kernel_id, 'ramdisk_id': FLAGS.rescue_ramdisk_id} self._create_image(instance, xml, '.rescue', rescue_images) - self._conn.createXML(xml, 0) + self._create_new_domain(xml) timer = utils.LoopingCall(f=None) @@ -565,10 +569,15 @@ class LibvirtConnection(driver.ComputeDriver): self.firewall_driver.setup_basic_filtering(instance, network_info) self.firewall_driver.prepare_instance_filter(instance, network_info) self._create_image(instance, xml, network_info) - self._conn.createXML(xml, 0) + domain = self._create_new_domain(xml) LOG.debug(_("instance %s: is running"), instance['name']) self.firewall_driver.apply_instance_filter(instance) + if FLAGS.start_guests_on_host_boot: + LOG.debug(_("instance %s: setting autostart ON") % + instance['name']) + domain.setAutostart(1) + timer = utils.LoopingCall(f=None) def _wait_for_boot(): @@ -964,11 +973,19 @@ class LibvirtConnection(driver.ComputeDriver): return xml def get_info(self, instance_name): + # NOTE(justinsb): When libvirt isn't running / can't connect, we get: + # libvir: Remote error : unable to connect to + # '/var/run/libvirt/libvirt-sock', libvirtd may need to be started: + # No such file or directory try: virt_dom = self._conn.lookupByName(instance_name) - except: - raise exception.NotFound(_("Instance %s not found") - % instance_name) + except libvirt.libvirtError as e: + if e.get_error_code() == libvirt.VIR_ERR_UNKNOWN_HOST: + raise exception.NotFound(_("Instance %s not found") + % instance_name) + LOG.warning(_("Error from libvirt during lookup: %s") % e) + raise + (state, max_mem, mem, num_cpu, cpu_time) = virt_dom.info() return {'state': state, 'max_mem': max_mem, @@ -976,6 +993,24 @@ class LibvirtConnection(driver.ComputeDriver): 'num_cpu': num_cpu, 'cpu_time': cpu_time} + def _create_new_domain(self, xml, persistent=True, launch_flags=0): + # NOTE(justinsb): libvirt has two types of domain: + # * a transient domain disappears when the guest is shutdown + # or the host is rebooted. + # * a permanent domain is not automatically deleted + # NOTE(justinsb): Even for ephemeral instances, transient seems risky + + if persistent: + # To create a persistent domain, first define it, then launch it. + domain = self._conn.defineXML(xml) + + domain.createWithFlags(launch_flags) + else: + # createXML call creates a transient domain + domain = self._conn.createXML(xml, launch_flags) + + return domain + def get_diagnostics(self, instance_name): raise exception.ApiError(_("diagnostics are not supported " "for libvirt")) -- cgit From 399056300a0be228ec4c56587ec0d9c0d09d927c Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Tue, 29 Mar 2011 16:54:37 -0700 Subject: Fix unit test to reflect fact that instance is no longer deleted, just marked SHUTOFF --- nova/tests/test_compute.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index d1ef68de4..b7f08dfbe 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -654,4 +654,5 @@ class ComputeTestCase(test.TestCase): instances = db.instance_get_all(context.get_admin_context()) LOG.info(_("After force-killing instances: %s"), instances) - self.assertEqual(len(instances), 0) + self.assertEqual(len(instances), 1) + self.assertEqual(power_state.SHUTOFF, instances[0]['state']) -- cgit From 323eb60884cf8736448d997068d32f252d22e7f3 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 30 Mar 2011 12:05:25 -0700 Subject: Key type values in ec2_api off of container format --- nova/api/ec2/cloud.py | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 7ba8dfbea..1e6c4d1db 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -146,7 +146,7 @@ class CloudController(object): floating_ip = db.instance_get_floating_address(ctxt, instance_ref['id']) ec2_id = ec2utils.id_to_ec2_id(instance_ref['id']) - image_ec2_id = self._image_ec2_id(instance_ref['image_id'], 'machine') + image_ec2_id = self._image_ec2_id(instance_ref['image_id'], 'ami') data = { 'user-data': base64.b64decode(instance_ref['user_data']), 'meta-data': { @@ -176,7 +176,7 @@ class CloudController(object): for image_type in ['kernel', 'ramdisk']: if '%s_id' % image_type in instance_ref: ec2_id = self._image_ec2_id(instance_ref['%s_id' % image_type], - image_type) + self._image_type(image_type)) data['meta-data']['%s-id' % image_type] = ec2_id if False: # TODO(vish): store ancestor ids @@ -862,13 +862,27 @@ class CloudController(object): self.compute_api.update(context, instance_id=instance_id, **kwargs) return True - _type_prefix_map = {'machine': 'ami', - 'kernel': 'aki', - 'ramdisk': 'ari'} + @staticmethod + def _image_type(image_type): + """Converts to a three letter image type. - def _image_ec2_id(self, image_id, image_type='machine'): - prefix = self._type_prefix_map[image_type] - template = prefix + '-%08x' + aki, kernel => aki + ari, ramdisk => ari + anything else => ami + + """ + if image_type == 'kernel': + return 'aki' + if image_type == 'ramdisk': + return 'ari' + if image_type not in ['aki', 'ari']: + return 'ami' + return image_type + + @staticmethod + def _image_ec2_id(image_id, image_type='ami'): + """Returns image ec2_id using id and three letter type""" + template = image_type + '-%08x' return ec2utils.id_to_ec2_id(int(image_id), template=template) def _get_image(self, context, ec2_id): @@ -881,7 +895,7 @@ class CloudController(object): def _format_image(self, image): """Convert from format defined by BaseImageService to S3 format.""" i = {} - image_type = image['properties'].get('type') + image_type = self._image_type(image.get('container_format')) ec2_id = self._image_ec2_id(image.get('id'), image_type) name = image.get('name') if name: @@ -890,16 +904,19 @@ class CloudController(object): i['imageId'] = ec2_id kernel_id = image['properties'].get('kernel_id') if kernel_id: - i['kernelId'] = self._image_ec2_id(kernel_id, 'kernel') + i['kernelId'] = self._image_ec2_id(kernel_id, 'aki') ramdisk_id = image['properties'].get('ramdisk_id') if ramdisk_id: - i['ramdiskId'] = self._image_ec2_id(ramdisk_id, 'ramdisk') + i['ramdiskId'] = self._image_ec2_id(ramdisk_id, 'ari') i['imageOwnerId'] = image['properties'].get('owner_id') i['imageLocation'] = image['properties'].get('image_location') i['imageState'] = image['properties'].get('image_state') i['displayName'] = image.get('name') i['description'] = image.get('description') - i['type'] = image_type + display_mapping = {'aki': 'kernel', + 'ari': 'ramdisk', + 'ami': 'machine'} + i['type'] = display_mapping.get(image_type) i['isPublic'] = str(image['properties'].get('is_public', '')) == 'True' i['architecture'] = image['properties'].get('architecture') return i @@ -932,8 +949,9 @@ class CloudController(object): 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['properties']['type']) + image_type) msg = _("Registered image %(image_location)s with" " id %(image_id)s") % locals() LOG.audit(msg, context=context) -- cgit From 1703592992ebdd5bbf19952f79f05022a4cdc849 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 30 Mar 2011 12:34:10 -0700 Subject: remove all references to image_type and change nova-manage upload to set container format more intelligently --- bin/nova-manage | 27 +++++++++++++------------ nova/api/openstack/servers.py | 2 +- nova/image/fake.py | 6 +++--- nova/image/s3.py | 1 - nova/tests/api/openstack/test_image_metadata.py | 2 -- nova/virt/libvirt_conn.py | 1 - 6 files changed, 18 insertions(+), 21 deletions(-) diff --git a/bin/nova-manage b/bin/nova-manage index 6789efba8..4e14b6cab 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -894,20 +894,18 @@ class ImageCommands(object): def __init__(self, *args, **kwargs): self.image_service = utils.import_object(FLAGS.image_service) - def _register(self, image_type, disk_format, container_format, + def _register(self, container_format, disk_format, path, owner, name=None, is_public='T', architecture='x86_64', kernel_id=None, ramdisk_id=None): meta = {'is_public': True, 'name': name, - 'disk_format': disk_format, 'container_format': container_format, + 'disk_format': disk_format, 'properties': {'image_state': 'available', 'owner_id': owner, - 'type': image_type, 'architecture': architecture, 'image_location': 'local', 'is_public': (is_public == 'T')}} - print image_type, meta if kernel_id: meta['properties']['kernel_id'] = int(kernel_id) if ramdisk_id: @@ -932,16 +930,17 @@ class ImageCommands(object): ramdisk_id = self.ramdisk_register(ramdisk, owner, None, is_public, architecture) self.image_register(image, owner, name, is_public, - architecture, kernel_id, ramdisk_id) + architecture, 'ami', 'ami', + kernel_id, ramdisk_id) def image_register(self, path, owner, name=None, is_public='T', - architecture='x86_64', kernel_id=None, ramdisk_id=None, - disk_format='ami', container_format='ami'): + architecture='x86_64', container_format='bare', + disk_format='raw', kernel_id=None, ramdisk_id=None): """Uploads an image into the image_service arguments: path owner [name] [is_public='T'] [architecture='x86_64'] - [kernel_id=None] [ramdisk_id=None] - [disk_format='ami'] [container_format='ami']""" - return self._register('machine', disk_format, container_format, path, + [container_format='bare'] [disk_format='raw'] + [kernel_id=None] [ramdisk_id=None]""" + return self._register(container_format, disk_format, path, owner, name, is_public, architecture, kernel_id, ramdisk_id) @@ -950,7 +949,7 @@ class ImageCommands(object): """Uploads a kernel into the image_service arguments: path owner [name] [is_public='T'] [architecture='x86_64'] """ - return self._register('kernel', 'aki', 'aki', path, owner, name, + return self._register('aki', 'aki', path, owner, name, is_public, architecture) def ramdisk_register(self, path, owner, name=None, is_public='T', @@ -958,7 +957,7 @@ class ImageCommands(object): """Uploads a ramdisk into the image_service arguments: path owner [name] [is_public='T'] [architecture='x86_64'] """ - return self._register('ramdisk', 'ari', 'ari', path, owner, name, + return self._register('ari', 'ari', path, owner, name, is_public, architecture) def _lookup(self, old_image_id): @@ -975,6 +974,9 @@ class ImageCommands(object): 'ramdisk': 'ari'} container_format = mapping[old['type']] disk_format = container_format + if container_format == 'ami' and not old.get('kernelId'): + container_format = 'bare' + disk_format = 'raw' new = {'disk_format': disk_format, 'container_format': container_format, 'is_public': True, @@ -982,7 +984,6 @@ class ImageCommands(object): 'properties': {'image_state': old['imageState'], 'owner_id': old['imageOwnerId'], 'architecture': old['architecture'], - 'type': old['type'], 'image_location': old['imageLocation'], 'is_public': old['isPublic']}} if old.get('kernelId'): diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index f7696d918..d640e8684 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -562,7 +562,7 @@ class Controller(wsgi.Controller): _("Cannot build from image %(image_id)s, status not active") % locals()) - if image_meta['properties']['disk_format'] != 'ami': + if image_meta.get('container_format') != 'ami': return None, None try: diff --git a/nova/image/fake.py b/nova/image/fake.py index 08302d6eb..d1c62757f 100644 --- a/nova/image/fake.py +++ b/nova/image/fake.py @@ -44,10 +44,10 @@ class FakeImageService(service.BaseImageService): 'created_at': timestamp, 'updated_at': timestamp, 'status': 'active', - 'type': 'machine', + 'container_format': 'ami', + 'disk_format': 'raw', 'properties': {'kernel_id': FLAGS.null_kernel, - 'ramdisk_id': FLAGS.null_kernel, - 'disk_format': 'ami'} + 'ramdisk_id': FLAGS.null_kernel} } self.create(None, image) super(FakeImageService, self).__init__() diff --git a/nova/image/s3.py b/nova/image/s3.py index ddec5f3aa..203bedc49 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -177,7 +177,6 @@ class S3ImageService(service.BaseImageService): properties['ramdisk_id'] = ec2utils.ec2_id_to_id(ramdisk_id) properties['is_public'] = False - properties['type'] = image_type metadata.update({'disk_format': image_format, 'container_format': image_format, 'status': 'queued', diff --git a/nova/tests/api/openstack/test_image_metadata.py b/nova/tests/api/openstack/test_image_metadata.py index 9be753f84..7c3287006 100644 --- a/nova/tests/api/openstack/test_image_metadata.py +++ b/nova/tests/api/openstack/test_image_metadata.py @@ -45,7 +45,6 @@ class ImageMetaDataTest(unittest.TestCase): 'is_public': True, 'deleted_at': None, 'properties': { - 'type': 'ramdisk', 'key1': 'value1', 'key2': 'value2' }, @@ -62,7 +61,6 @@ class ImageMetaDataTest(unittest.TestCase): 'is_public': True, 'deleted_at': None, 'properties': { - 'type': 'ramdisk', 'key1': 'value1', 'key2': 'value2' }, diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index f34ea7225..adcb2ffa3 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -424,7 +424,6 @@ class LibvirtConnection(driver.ComputeDriver): 'container_format': base['container_format'], 'is_public': False, 'properties': {'architecture': base['architecture'], - 'type': base['type'], 'name': '%s.%s' % (base['name'], image_id), 'kernel_id': instance['kernel_id'], 'image_location': 'snapshot', -- cgit From b8173ba9fe80045665064208e66f46cca75fd3ba Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 30 Mar 2011 13:48:07 -0700 Subject: review cleanup --- bin/nova-manage | 3 ++- nova/api/ec2/cloud.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/bin/nova-manage b/bin/nova-manage index 4e14b6cab..fbf16f570 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -939,7 +939,8 @@ class ImageCommands(object): """Uploads an image into the image_service arguments: path owner [name] [is_public='T'] [architecture='x86_64'] [container_format='bare'] [disk_format='raw'] - [kernel_id=None] [ramdisk_id=None]""" + [kernel_id=None] [ramdisk_id=None] + """ return self._register(container_format, disk_format, path, owner, name, is_public, architecture, kernel_id, ramdisk_id) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 1e6c4d1db..3541e49ca 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -881,7 +881,7 @@ class CloudController(object): @staticmethod def _image_ec2_id(image_id, image_type='ami'): - """Returns image ec2_id using id and three letter type""" + """Returns image ec2_id using id and three letter type.""" template = image_type + '-%08x' return ec2utils.id_to_ec2_id(int(image_id), template=template) -- cgit From 1654afaabba498ab690c07ccc6de858783dc1742 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Thu, 31 Mar 2011 12:38:05 -0700 Subject: makes sure s3 filtering works even without metadata set properly --- nova/image/s3.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nova/image/s3.py b/nova/image/s3.py index 203bedc49..def49f682 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -80,9 +80,10 @@ class S3ImageService(service.BaseImageService): @classmethod def _is_visible(cls, context, image): + return (context.is_admin - or context.project_id == image['properties']['owner_id'] - or image['properties']['is_public'] == 'True') + or context.project_id == image['properties'].get('owner_id') + or str(image['properties'].get('is_public')) == 'True') @classmethod def _filter(cls, context, images): -- cgit From e6505fd78cc28a5e91ec6fbf55e2e8de07679c7c Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Tue, 5 Apr 2011 10:56:24 -0400 Subject: Fixed the addresses and metadata collections in xml responses. Added corresponding tests. --- nova/api/openstack/servers.py | 7 +++++++ nova/tests/api/openstack/test_servers.py | 36 ++++++++++++++++++++++++++++++++ nova/wsgi.py | 18 ++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 6704a68ae..cada92813 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -55,6 +55,13 @@ class Controller(wsgi.Controller): "imageRef"], "link": ["rel", "type", "href"], }, + "dict_collections": { + "metadata": {"item_name": "meta", "item_key": "key"}, + }, + "list_collections": { + "public": {"item_name": "ip", "item_key": "addr"}, + "private": {"item_name": "ip", "item_key": "addr"}, + }, }, } diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 313676e72..ce9821587 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -192,6 +192,26 @@ class ServersTest(test.TestCase): print res_dict['server'] self.assertEqual(res_dict['server']['links'], expected_links) + def test_get_server_by_id_with_addresses_xml(self): + private = "192.168.0.3" + public = ["1.2.3.4"] + new_return_server = return_server_with_addresses(private, public) + self.stubs.Set(nova.db.api, 'instance_get', new_return_server) + req = webob.Request.blank('/v1.0/servers/1') + req.headers['Accept'] = 'application/xml' + res = req.get_response(fakes.wsgi_app()) + dom = minidom.parseString(res.body) + server = dom.childNodes[0] + self.assertEquals(server.nodeName, 'server') + self.assertEquals(server.getAttribute('id'), '1') + self.assertEquals(server.getAttribute('name'), 'server1') + (public,) = server.getElementsByTagName('public'); + (ip,) = public.getElementsByTagName('ip') + self.assertEquals(ip.getAttribute('addr'), '1.2.3.4') + (private,) = server.getElementsByTagName('private') + (ip,) = private.getElementsByTagName('ip') + self.assertEquals(ip.getAttribute('addr'), '192.168.0.3') + def test_get_server_by_id_with_addresses(self): private = "192.168.0.3" public = ["1.2.3.4"] @@ -618,6 +638,22 @@ class ServersTest(test.TestCase): res = req.get_response(fakes.wsgi_app()) self.assertEqual(res.status_int, 404) + def test_get_all_server_details_xml_v1_0(self): + req = webob.Request.blank('/v1.0/servers/detail') + req.headers['Accept'] = 'application/xml' + res = req.get_response(fakes.wsgi_app()) + print res.body + dom = minidom.parseString(res.body) + for i, server in enumerate(dom.getElementsByTagName('server')): + self.assertEqual(server.getAttribute('id'), str(i)) + self.assertEqual(server.getAttribute('hostId'), '') + self.assertEqual(server.getAttribute('name'), 'server%d' % i) + self.assertEqual(server.getAttribute('imageId'), '10') + self.assertEqual(server.getAttribute('status'), 'BUILD') + (meta,) = server.getElementsByTagName('meta') + self.assertEqual(meta.getAttribute('key'), 'seq') + self.assertEqual(meta.firstChild.data.strip(), str(i)) + def test_get_all_server_details_v1_0(self): req = webob.Request.blank('/v1.0/servers/detail') res = req.get_response(fakes.wsgi_app()) diff --git a/nova/wsgi.py b/nova/wsgi.py index ba0819466..a550a33b0 100644 --- a/nova/wsgi.py +++ b/nova/wsgi.py @@ -484,6 +484,14 @@ class Serializer(object): """Recursive method to convert data members to XML nodes.""" result = doc.createElement(nodename) if type(data) is list: + collections = metadata.get('list_collections', {}) + if nodename in collections: + metadata = collections[nodename] + for item in data: + node = doc.createElement(metadata['item_name']) + node.setAttribute(metadata['item_key'], str(item)) + result.appendChild(node) + return result singular = metadata.get('plurals', {}).get(nodename, None) if singular is None: if nodename.endswith('s'): @@ -494,6 +502,16 @@ class Serializer(object): node = self._to_xml_node(doc, metadata, singular, item) result.appendChild(node) elif type(data) is dict: + collections = metadata.get('dict_collections', {}) + if nodename in collections: + metadata = collections[nodename] + for k, v in data.items(): + node = doc.createElement(metadata['item_name']) + node.setAttribute(metadata['item_key'], str(k)) + text = doc.createTextNode(str(v)) + node.appendChild(text) + result.appendChild(node) + return result attrs = metadata.get('attributes', {}).get(nodename, {}) for k, v in data.items(): if k in attrs: -- cgit From 3c487493de2a0f827780d54080797bfe70183ba6 Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Tue, 5 Apr 2011 17:49:38 -0400 Subject: Removing naughty semicolon. --- nova/tests/api/openstack/test_servers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index ce9821587..a424a8105 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -205,7 +205,7 @@ class ServersTest(test.TestCase): self.assertEquals(server.nodeName, 'server') self.assertEquals(server.getAttribute('id'), '1') self.assertEquals(server.getAttribute('name'), 'server1') - (public,) = server.getElementsByTagName('public'); + (public,) = server.getElementsByTagName('public') (ip,) = public.getElementsByTagName('ip') self.assertEquals(ip.getAttribute('addr'), '1.2.3.4') (private,) = server.getElementsByTagName('private') -- cgit From 56eb682a169c20a9a8275b3b9bfd423f0d562cbc Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 5 Apr 2011 15:01:33 -0700 Subject: unite the filtering done by glance client and s3 --- bin/nova-manage | 14 ++++++-------- nova/api/ec2/cloud.py | 2 +- nova/image/glance.py | 28 ---------------------------- nova/image/local.py | 4 +++- nova/image/s3.py | 45 +++------------------------------------------ nova/image/service.py | 27 +++++++++++++++++++++++++++ 6 files changed, 40 insertions(+), 80 deletions(-) diff --git a/bin/nova-manage b/bin/nova-manage index fbf16f570..bfff227c9 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -897,15 +897,14 @@ class ImageCommands(object): def _register(self, container_format, disk_format, path, owner, name=None, is_public='T', architecture='x86_64', kernel_id=None, ramdisk_id=None): - meta = {'is_public': True, + meta = {'is_public': (is_public == 'T'), 'name': name, 'container_format': container_format, 'disk_format': disk_format, 'properties': {'image_state': 'available', - 'owner_id': owner, + 'project_id': owner, 'architecture': architecture, - 'image_location': 'local', - 'is_public': (is_public == 'T')}} + 'image_location': 'local'}} if kernel_id: meta['properties']['kernel_id'] = int(kernel_id) if ramdisk_id: @@ -980,13 +979,12 @@ class ImageCommands(object): disk_format = 'raw' new = {'disk_format': disk_format, 'container_format': container_format, - 'is_public': True, + 'is_public': old['isPublic'], 'name': old['imageId'], 'properties': {'image_state': old['imageState'], - 'owner_id': old['imageOwnerId'], + 'project_id': old['imageOwnerId'], 'architecture': old['architecture'], - 'image_location': old['imageLocation'], - 'is_public': old['isPublic']}} + 'image_location': old['imageLocation']}} if old.get('kernelId'): new['properties']['kernel_id'] = self._lookup(old['kernelId']) if old.get('ramdiskId'): diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 02a30220a..b4705820e 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -927,7 +927,7 @@ class CloudController(object): 'ari': 'ramdisk', 'ami': 'machine'} i['imageType'] = display_mapping.get(image_type) - i['isPublic'] = str(image['properties'].get('is_public', '')) == 'True' + i['isPublic'] = image.get('is_public') == True i['architecture'] = image['properties'].get('architecture') return i diff --git a/nova/image/glance.py b/nova/image/glance.py index fdf468594..e583c504d 100644 --- a/nova/image/glance.py +++ b/nova/image/glance.py @@ -186,34 +186,6 @@ class GlanceImageService(service.BaseImageService): image_meta = _convert_timestamps_to_datetimes(image_meta) return image_meta - @staticmethod - def _is_image_available(context, image_meta): - """ - Images are always available if they are public or if the user is an - admin. - - Otherwise, we filter by project_id (if present) and then fall-back to - images owned by user. - """ - # FIXME(sirp): We should be filtering by user_id on the Glance side - # for security; however, we can't do that until we get authn/authz - # sorted out. Until then, filtering in Nova. - if image_meta['is_public'] or context.is_admin: - return True - - properties = image_meta['properties'] - - if context.project_id and ('project_id' in properties): - return str(properties['project_id']) == str(project_id) - - try: - user_id = properties['user_id'] - except KeyError: - return False - - return str(user_id) == str(context.user_id) - - # utility functions def _convert_timestamps_to_datetimes(image_meta): """ diff --git a/nova/image/local.py b/nova/image/local.py index 1fb6e1f13..92d3dabd4 100644 --- a/nova/image/local.py +++ b/nova/image/local.py @@ -84,7 +84,9 @@ class LocalImageService(service.BaseImageService): def show(self, context, image_id): try: with open(self._path_to(image_id)) as metadata_file: - return json.load(metadata_file) + image_meta = json.load(metadata_file) + if not self._is_image_available(context, image_meta): + raise exception.NotFound except (IOError, ValueError): raise exception.NotFound diff --git a/nova/image/s3.py b/nova/image/s3.py index def49f682..93af21022 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -58,54 +58,16 @@ class S3ImageService(service.BaseImageService): return image def delete(self, context, image_id): - # FIXME(vish): call to show is to check filter + # FIXME(vish): call to show is to check visibility self.show(context, image_id) self.service.delete(context, image_id) def update(self, context, image_id, metadata, data=None): - # FIXME(vish): call to show is to check filter + # FIXME(vish): call to show is to check visibility self.show(context, image_id) image = self.service.update(context, image_id, metadata, data) return image - def index(self, context): - images = self.service.index(context) - # FIXME(vish): index doesn't filter so we do it manually - return self._filter(context, images) - - def detail(self, context): - images = self.service.detail(context) - # FIXME(vish): detail doesn't filter so we do it manually - return self._filter(context, images) - - @classmethod - def _is_visible(cls, context, image): - - return (context.is_admin - or context.project_id == image['properties'].get('owner_id') - or str(image['properties'].get('is_public')) == 'True') - - @classmethod - def _filter(cls, context, images): - filtered = [] - for image in images: - if not cls._is_visible(context, image): - continue - filtered.append(image) - return filtered - - def show(self, context, image_id): - image = self.service.show(context, image_id) - if not self._is_visible(context, image): - raise exception.NotFound - return image - - def show_by_name(self, context, name): - image = self.service.show_by_name(context, name) - if not self._is_visible(context, image): - raise exception.NotFound - return image - @staticmethod def _conn(context): # TODO(vish): is there a better way to get creds to sign @@ -168,7 +130,7 @@ class S3ImageService(service.BaseImageService): arch = 'x86_64' properties = metadata['properties'] - properties['owner_id'] = context.project_id + properties['project_id'] = context.project_id properties['architecture'] = arch if kernel_id: @@ -177,7 +139,6 @@ class S3ImageService(service.BaseImageService): if ramdisk_id: properties['ramdisk_id'] = ec2utils.ec2_id_to_id(ramdisk_id) - properties['is_public'] = False metadata.update({'disk_format': image_format, 'container_format': image_format, 'status': 'queued', diff --git a/nova/image/service.py b/nova/image/service.py index b9897ecae..fddc72409 100644 --- a/nova/image/service.py +++ b/nova/image/service.py @@ -136,6 +136,33 @@ class BaseImageService(object): """ raise NotImplementedError + @staticmethod + def _is_image_available(context, image_meta): + """ + Images are always available if they are public or if the user is an + admin. + + Otherwise, we filter by project_id (if present) and then fall-back to + images owned by user. + """ + # FIXME(sirp): We should be filtering by user_id on the Glance side + # for security; however, we can't do that until we get authn/authz + # sorted out. Until then, filtering in Nova. + if image_meta['is_public'] or context.is_admin: + return True + + properties = image_meta['properties'] + + if context.project_id and ('project_id' in properties): + return str(properties['project_id']) == str(context.project_id) + + try: + user_id = properties['user_id'] + except KeyError: + return False + + return str(user_id) == str(context.user_id) + @classmethod def _translate_to_base(cls, metadata): """Return a metadata dictionary that is BaseImageService compliant. -- cgit From 8eff0a91bbc475d8559b39fb81e1a62beda841f3 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 5 Apr 2011 15:35:17 -0700 Subject: update and fix tests --- nova/image/local.py | 1 + nova/image/s3.py | 13 +++++++++++++ nova/tests/api/openstack/test_servers.py | 16 +++++++--------- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/nova/image/local.py b/nova/image/local.py index 92d3dabd4..bad4c2d85 100644 --- a/nova/image/local.py +++ b/nova/image/local.py @@ -87,6 +87,7 @@ class LocalImageService(service.BaseImageService): image_meta = json.load(metadata_file) if not self._is_image_available(context, image_meta): raise exception.NotFound + return image_meta except (IOError, ValueError): raise exception.NotFound diff --git a/nova/image/s3.py b/nova/image/s3.py index 93af21022..a7301eefd 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -46,6 +46,7 @@ flags.DEFINE_string('image_decryption_dir', '/tmp', class S3ImageService(service.BaseImageService): + """Wraps an existing image service to support s3 based register""" def __init__(self, service=None, *args, **kwargs): if service == None: service = utils.import_object(FLAGS.image_service) @@ -68,6 +69,18 @@ class S3ImageService(service.BaseImageService): image = self.service.update(context, image_id, metadata, data) return image + def index(self, context): + return self.service.index(context) + + def detail(self, context): + return self.service.detail(context) + + def show(self, context, image_id): + return self.service.show(context, image_id) + + def show_by_name(self, context, name): + return self.service.show(context, name) + @staticmethod def _conn(context): # TODO(vish): is there a better way to get creds to sign diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 313676e72..1ec01bb4c 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1525,29 +1525,27 @@ class TestGetKernelRamdiskFromImage(test.TestCase): def test_not_ami(self): """Anything other than ami should return no kernel and no ramdisk""" - image_meta = {'id': 1, 'status': 'active', - 'properties': {'disk_format': 'vhd'}} + image_meta = {'id': 1, 'status': 'active', 'container_format': 'vhd'} kernel_id, ramdisk_id = self._get_k_r(image_meta) self.assertEqual(kernel_id, None) self.assertEqual(ramdisk_id, None) def test_ami_no_kernel(self): """If an ami is missing a kernel it should raise NotFound""" - image_meta = {'id': 1, 'status': 'active', - 'properties': {'disk_format': 'ami', 'ramdisk_id': 1}} + image_meta = {'id': 1, 'status': 'active', 'container_format': 'ami', + 'properties': {'ramdisk_id': 1}} self.assertRaises(exception.NotFound, self._get_k_r, image_meta) def test_ami_no_ramdisk(self): """If an ami is missing a ramdisk it should raise NotFound""" - image_meta = {'id': 1, 'status': 'active', - 'properties': {'disk_format': 'ami', 'kernel_id': 1}} + image_meta = {'id': 1, 'status': 'active', 'container_format': 'ami', + 'properties': {'kernel_id': 1}} self.assertRaises(exception.NotFound, self._get_k_r, image_meta) def test_ami_kernel_ramdisk_present(self): """Return IDs if both kernel and ramdisk are present""" - image_meta = {'id': 1, 'status': 'active', - 'properties': {'disk_format': 'ami', 'kernel_id': 1, - 'ramdisk_id': 2}} + image_meta = {'id': 1, 'status': 'active', 'container_format': 'ami', + 'properties': {'kernel_id': 1, 'ramdisk_id': 2}} kernel_id, ramdisk_id = self._get_k_r(image_meta) self.assertEqual(kernel_id, 1) self.assertEqual(ramdisk_id, 2) -- cgit From 5f414311fb581ba612cf152f3ec9983d5f39c2e4 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 5 Apr 2011 15:45:01 -0700 Subject: fallback to status if image_state is not set --- nova/api/ec2/cloud.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index b4705820e..4e5b8bfc6 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -920,7 +920,11 @@ class CloudController(object): get('image_location'), name) else: i['imageLocation'] = image['properties'].get('image_location') - i['imageState'] = image['properties'].get('image_state') + # NOTE(vish): fallback status if image_state isn't set + state = image.get('status') + if state == 'active': + state = 'available' + i['imageState'] = image['properties'].get('image_state', state) i['displayName'] = name i['description'] = image.get('description') display_mapping = {'aki': 'kernel', -- cgit From 75082c44cdc0319c56805b5558927760636eaf4b Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 5 Apr 2011 15:49:43 -0700 Subject: pep8 --- nova/image/glance.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nova/image/glance.py b/nova/image/glance.py index e583c504d..f848f40d8 100644 --- a/nova/image/glance.py +++ b/nova/image/glance.py @@ -186,6 +186,7 @@ class GlanceImageService(service.BaseImageService): image_meta = _convert_timestamps_to_datetimes(image_meta) return image_meta + # utility functions def _convert_timestamps_to_datetimes(image_meta): """ -- cgit From cd279bec6391ef40d278f377f0039b507b14904b Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 6 Apr 2011 12:58:57 -0700 Subject: check visibility on delete and update --- nova/image/glance.py | 4 ++++ nova/image/local.py | 6 +++++- nova/image/s3.py | 4 ---- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/nova/image/glance.py b/nova/image/glance.py index f848f40d8..bf49ca96c 100644 --- a/nova/image/glance.py +++ b/nova/image/glance.py @@ -151,6 +151,8 @@ class GlanceImageService(service.BaseImageService): :raises NotFound if the image does not exist. """ + # NOTE(vish): show is to check if image is available + self.show(context, image_id) try: image_meta = self.client.update_image(image_id, image_meta, data) except glance_exception.NotFound: @@ -165,6 +167,8 @@ class GlanceImageService(service.BaseImageService): :raises NotFound if the image does not exist. """ + # NOTE(vish): show is to check if image is available + self.show(context, image_id) try: result = self.client.delete_image(image_id) except glance_exception.NotFound: diff --git a/nova/image/local.py b/nova/image/local.py index bad4c2d85..8bf78b4c9 100644 --- a/nova/image/local.py +++ b/nova/image/local.py @@ -126,6 +126,8 @@ class LocalImageService(service.BaseImageService): def update(self, context, image_id, metadata, data=None): """Replace the contents of the given image with the new data.""" + # NOTE(vish): show is to check if image is available + self.show(context, image_id) metadata['id'] = image_id try: if data: @@ -143,9 +145,11 @@ class LocalImageService(service.BaseImageService): def delete(self, context, image_id): """Delete the given image. - Raises OSError if the image does not exist. + Raises NotFound if the image does not exist. """ + # NOTE(vish): show is to check if image is available + self.show(context, image_id) try: shutil.rmtree(self._path_to(image_id, None)) except (IOError, ValueError): diff --git a/nova/image/s3.py b/nova/image/s3.py index a7301eefd..554760d53 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -59,13 +59,9 @@ class S3ImageService(service.BaseImageService): return image def delete(self, context, image_id): - # FIXME(vish): call to show is to check visibility - self.show(context, image_id) self.service.delete(context, image_id) def update(self, context, image_id, metadata, data=None): - # FIXME(vish): call to show is to check visibility - self.show(context, image_id) image = self.service.update(context, image_id, metadata, data) return image -- cgit From 5534113f65cc578dcdf93432981b836bf4e3dfaf Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Wed, 6 Apr 2011 16:12:32 -0400 Subject: Added support for listing addresses of a server in the openstack api. Now you can GET * /servers/1/ips * /servers/1/ips/public * /servers/1/ips/private Supports v1.0 json and xml. Added corresponding tests. --- nova/api/openstack/__init__.py | 6 +++ nova/api/openstack/ips.py | 71 +++++++++++++++++++++++++++++ nova/api/openstack/servers.py | 9 ---- nova/api/openstack/views/addresses.py | 10 +++- nova/tests/api/openstack/test_servers.py | 78 ++++++++++++++++++++++++++++++++ 5 files changed, 163 insertions(+), 11 deletions(-) create mode 100644 nova/api/openstack/ips.py diff --git a/nova/api/openstack/__init__.py b/nova/api/openstack/__init__.py index 7545eb0c9..5e76a06f7 100644 --- a/nova/api/openstack/__init__.py +++ b/nova/api/openstack/__init__.py @@ -34,6 +34,7 @@ from nova.api.openstack import consoles from nova.api.openstack import flavors from nova.api.openstack import images from nova.api.openstack import image_metadata +from nova.api.openstack import ips from nova.api.openstack import limits from nova.api.openstack import servers from nova.api.openstack import server_metadata @@ -144,6 +145,11 @@ class APIRouterV10(APIRouter): parent_resource=dict(member_name='server', collection_name='servers')) + mapper.resource("ip", "ips", controller=ips.Controller(), + collection=dict(public='GET', private='GET'), + parent_resource=dict(member_name='server', + collection_name='servers')) + class APIRouterV11(APIRouter): """Define routes specific to OpenStack API V1.1.""" diff --git a/nova/api/openstack/ips.py b/nova/api/openstack/ips.py new file mode 100644 index 000000000..89b0936d5 --- /dev/null +++ b/nova/api/openstack/ips.py @@ -0,0 +1,71 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2011 OpenStack LLC. +# 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. + +import time + +from webob import exc + +import nova +import nova.api.openstack.views.addresses +from nova.api.openstack import faults + + +class Controller(nova.wsgi.Controller): + """The servers addresses API controller for the Openstack API.""" + + _serialization_metadata = { + 'application/xml': { + 'list_collections': { + 'public': {'item_name': 'ip', 'item_key': 'addr'}, + 'private': {'item_name': 'ip', 'item_key': 'addr'}, + }, + }, + } + + def __init__(self): + self.compute_api = nova.compute.API() + self.builder = nova.api.openstack.views.addresses.ViewBuilderV10() + + def index(self, req, server_id): + try: + instance = self.compute_api.get(req.environ['nova.context'], id) + except nova.exception.NotFound: + return faults.Fault(exc.HTTPNotFound()) + return {'addresses': self.builder.build(instance)} + + def public(self, req, server_id): + try: + instance = self.compute_api.get(req.environ['nova.context'], id) + except nova.exception.NotFound: + return faults.Fault(exc.HTTPNotFound()) + return {'public': self.builder.build_public_parts(instance)} + + def private(self, req, server_id): + try: + instance = self.compute_api.get(req.environ['nova.context'], id) + except nova.exception.NotFound: + return faults.Fault(exc.HTTPNotFound()) + return {'private': self.builder.build_private_parts(instance)} + + def show(self, req, server_id, id): + return faults.Fault(exc.HTTPNotImplemented()) + + def create(self, req, server_id): + return faults.Fault(exc.HTTPNotImplemented()) + + def delete(self, req, server_id, id): + return faults.Fault(exc.HTTPNotImplemented()) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index cada92813..67e6eefd8 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -70,15 +70,6 @@ class Controller(wsgi.Controller): self._image_service = utils.import_object(FLAGS.image_service) super(Controller, self).__init__() - def ips(self, req, id): - try: - instance = self.compute_api.get(req.environ['nova.context'], id) - except exception.NotFound: - return faults.Fault(exc.HTTPNotFound()) - - builder = self._get_addresses_view_builder(req) - return builder.build(instance) - def index(self, req): """ Returns a list of server names and ids for a given user """ return self._items(req, is_detail=False) diff --git a/nova/api/openstack/views/addresses.py b/nova/api/openstack/views/addresses.py index 90c77855b..2810cce39 100644 --- a/nova/api/openstack/views/addresses.py +++ b/nova/api/openstack/views/addresses.py @@ -28,10 +28,16 @@ class ViewBuilder(object): class ViewBuilderV10(ViewBuilder): def build(self, inst): - private_ips = utils.get_from_path(inst, 'fixed_ip/address') - public_ips = utils.get_from_path(inst, 'fixed_ip/floating_ips/address') + private_ips = self.build_private_parts(inst) + public_ips = self.build_public_parts(inst) return dict(public=public_ips, private=private_ips) + def build_public_parts(self, inst): + return utils.get_from_path(inst, 'fixed_ip/floating_ips/address') + + def build_private_parts(self, inst): + return utils.get_from_path(inst, 'fixed_ip/address') + class ViewBuilderV11(ViewBuilder): def build(self, inst): diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index a424a8105..640418cfa 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -228,6 +228,84 @@ class ServersTest(test.TestCase): self.assertEqual(len(addresses["private"]), 1) self.assertEqual(addresses["private"][0], private) + def test_get_server_addresses_V10(self): + private = '192.168.0.3' + public = ['1.2.3.4'] + new_return_server = return_server_with_addresses(private, public) + self.stubs.Set(nova.db.api, 'instance_get', new_return_server) + req = webob.Request.blank('/v1.0/servers/1/ips') + res = req.get_response(fakes.wsgi_app()) + res_dict = json.loads(res.body) + self.assertEqual(res_dict, { + 'addresses': {'public': public, 'private': [private]}}) + + def test_get_server_addresses_xml_V10(self): + private_expected = "192.168.0.3" + public_expected = ["1.2.3.4"] + new_return_server = return_server_with_addresses(private_expected, + public_expected) + self.stubs.Set(nova.db.api, 'instance_get', new_return_server) + req = webob.Request.blank('/v1.0/servers/1/ips') + req.headers['Accept'] = 'application/xml' + res = req.get_response(fakes.wsgi_app()) + dom = minidom.parseString(res.body) + (addresses,) = dom.childNodes + self.assertEquals(addresses.nodeName, 'addresses') + (public,) = addresses.getElementsByTagName('public') + (ip,) = public.getElementsByTagName('ip') + self.assertEquals(ip.getAttribute('addr'), public_expected[0]) + (private,) = addresses.getElementsByTagName('private') + (ip,) = private.getElementsByTagName('ip') + self.assertEquals(ip.getAttribute('addr'), private_expected) + + def test_get_server_addresses_public_V10(self): + private = "192.168.0.3" + public = ["1.2.3.4"] + new_return_server = return_server_with_addresses(private, public) + self.stubs.Set(nova.db.api, 'instance_get', new_return_server) + req = webob.Request.blank('/v1.0/servers/1/ips/public') + res = req.get_response(fakes.wsgi_app()) + res_dict = json.loads(res.body) + self.assertEqual(res_dict, {'public': public}) + + def test_get_server_addresses_private_V10(self): + private = "192.168.0.3" + public = ["1.2.3.4"] + new_return_server = return_server_with_addresses(private, public) + self.stubs.Set(nova.db.api, 'instance_get', new_return_server) + req = webob.Request.blank('/v1.0/servers/1/ips/private') + res = req.get_response(fakes.wsgi_app()) + res_dict = json.loads(res.body) + self.assertEqual(res_dict, {'private': [private]}) + + def test_get_server_addresses_public_xml_V10(self): + private = "192.168.0.3" + public = ["1.2.3.4"] + new_return_server = return_server_with_addresses(private, public) + self.stubs.Set(nova.db.api, 'instance_get', new_return_server) + req = webob.Request.blank('/v1.0/servers/1/ips/public') + req.headers['Accept'] = 'application/xml' + res = req.get_response(fakes.wsgi_app()) + dom = minidom.parseString(res.body) + (public_node,) = dom.childNodes + self.assertEquals(public_node.nodeName, 'public') + (ip,) = public_node.getElementsByTagName('ip') + self.assertEquals(ip.getAttribute('addr'), public[0]) + + def test_get_server_addresses_private_xml_V10(self): + private = "192.168.0.3" + public = ["1.2.3.4"] + new_return_server = return_server_with_addresses(private, public) + self.stubs.Set(nova.db.api, 'instance_get', new_return_server) + req = webob.Request.blank('/v1.0/servers/1/ips/private') + req.headers['Accept'] = 'application/xml' + res = req.get_response(fakes.wsgi_app()) + dom = minidom.parseString(res.body) + (private_node,) = dom.childNodes + self.assertEquals(private_node.nodeName, 'private') + (ip,) = private_node.getElementsByTagName('ip') + self.assertEquals(ip.getAttribute('addr'), private) + def test_get_server_by_id_with_addresses_v11(self): private = "192.168.0.3" public = ["1.2.3.4"] -- cgit From e16571671e4baab50521870ec64e4ab954c7d165 Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Wed, 6 Apr 2011 17:50:11 -0400 Subject: Controllers now inherit from nova.api.openstack.common.OpenstackController. --- nova/api/openstack/ips.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nova/api/openstack/ips.py b/nova/api/openstack/ips.py index 89b0936d5..778e9ba1a 100644 --- a/nova/api/openstack/ips.py +++ b/nova/api/openstack/ips.py @@ -21,10 +21,11 @@ from webob import exc import nova import nova.api.openstack.views.addresses +from nova.api.openstack import common from nova.api.openstack import faults -class Controller(nova.wsgi.Controller): +class Controller(common.OpenstackController): """The servers addresses API controller for the Openstack API.""" _serialization_metadata = { -- cgit From 040428cc35aa046de4bcf744c95b7c507df94550 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 6 Apr 2011 14:53:35 -0700 Subject: Add automatic metadata ip to network host on start. Also fix race where gw is readded twice --- nova/network/linux_net.py | 13 ++++++++++++- nova/network/manager.py | 3 +++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index d11d21dad..6a7051d10 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -391,6 +391,13 @@ def unbind_floating_ip(floating_ip): 'dev', FLAGS.public_interface) +def ensure_metadata_ip(interface): + """Sets up local metadata ip""" + _execute('sudo', 'ip', 'addr', 'add', '169.254.169.254/32', + 'scope', 'link', 'dev', interface, + check_exit_code=False) + + def ensure_vlan_forward(public_ip, port, private_ip): """Sets up forwarding rules for vlan""" iptables_manager.ipv4['filter'].add_rule("FORWARD", @@ -504,7 +511,11 @@ def ensure_bridge(bridge, interface, net_attrs=None): _execute(*_ip_bridge_cmd('del', params, fields[-1])) _execute(*_ip_bridge_cmd('add', params, bridge)) if gateway: - _execute('sudo', 'route', 'add', '0.0.0.0', 'gw', gateway) + # NOTE(vish): If the gateway already exists we are fine + out, err = _execute('sudo', 'route', 'add', '0.0.0.0', 'gw', + gateway, check_exit_code=False) + if err and err != "SIOCADDRT: File exists\n": + raise exception.Error("Failed to reset gateway: %s" % err) out, err = _execute('sudo', 'brctl', 'addif', bridge, interface, check_exit_code=False) diff --git a/nova/network/manager.py b/nova/network/manager.py index 86ee4fc00..a3c345b69 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -100,6 +100,8 @@ flags.DEFINE_string('network_host', socket.gethostname(), 'Network host to use for ip allocation in flat modes') flags.DEFINE_bool('fake_call', False, 'If True, skip using the queue and make local calls') +flags.DEFINE_string('metadata_interface', 'eth0', + 'interface to add the metadata ip to') class AddressAlreadyAllocated(exception.Error): @@ -128,6 +130,7 @@ class NetworkManager(manager.SchedulerDependentManager): self.driver.init_host() # Set up networking for the projects for which we're already # the designated network host. + self.driver.ensure_metadata_ip(FLAGS.metadata_interface) ctxt = context.get_admin_context() for network in self.db.host_get_networks(ctxt, self.host): self._on_set_network_host(ctxt, network['id']) -- cgit From 3831008b4e8aeec08b58afb49e40428ad5ece5b1 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 6 Apr 2011 16:10:08 -0700 Subject: if we delete the old route when we move it we don't need to check for exists --- nova/network/linux_net.py | 9 ++++----- nova/network/manager.py | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index 6a7051d10..a4d312f02 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -449,6 +449,7 @@ def ensure_vlan(vlan_num): return interface +@utils.synchronized('ensure_bridge', external=True) def ensure_bridge(bridge, interface, net_attrs=None): """Create a bridge unless it already exists. @@ -502,6 +503,8 @@ def ensure_bridge(bridge, interface, net_attrs=None): fields = line.split() if fields and fields[0] == "0.0.0.0" and fields[-1] == interface: gateway = fields[1] + _execute('sudo', 'route', 'del', 'default', 'gw', gateway, + 'dev', interface) out, err = _execute('sudo', 'ip', 'addr', 'show', 'dev', interface, 'scope', 'global') for line in out.split("\n"): @@ -511,11 +514,7 @@ def ensure_bridge(bridge, interface, net_attrs=None): _execute(*_ip_bridge_cmd('del', params, fields[-1])) _execute(*_ip_bridge_cmd('add', params, bridge)) if gateway: - # NOTE(vish): If the gateway already exists we are fine - out, err = _execute('sudo', 'route', 'add', '0.0.0.0', 'gw', - gateway, check_exit_code=False) - if err and err != "SIOCADDRT: File exists\n": - raise exception.Error("Failed to reset gateway: %s" % err) + _execute('sudo', 'route', 'add', 'default', 'gw', gateway) out, err = _execute('sudo', 'brctl', 'addif', bridge, interface, check_exit_code=False) diff --git a/nova/network/manager.py b/nova/network/manager.py index a3c345b69..b80560bdf 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -128,9 +128,9 @@ class NetworkManager(manager.SchedulerDependentManager): standalone service. """ self.driver.init_host() + self.driver.ensure_metadata_ip(FLAGS.metadata_interface) # Set up networking for the projects for which we're already # the designated network host. - self.driver.ensure_metadata_ip(FLAGS.metadata_interface) ctxt = context.get_admin_context() for network in self.db.host_get_networks(ctxt, self.host): self._on_set_network_host(ctxt, network['id']) -- cgit From 8da3e6c19b97ab7cd08e69fb0df114653c0b90db Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Thu, 7 Apr 2011 10:37:40 -0700 Subject: Simplify by always adding to loopback --- nova/network/linux_net.py | 5 ++--- nova/network/manager.py | 4 +--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index a4d312f02..ed6c943c7 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -391,11 +391,10 @@ def unbind_floating_ip(floating_ip): 'dev', FLAGS.public_interface) -def ensure_metadata_ip(interface): +def ensure_metadata_ip(): """Sets up local metadata ip""" _execute('sudo', 'ip', 'addr', 'add', '169.254.169.254/32', - 'scope', 'link', 'dev', interface, - check_exit_code=False) + 'scope', 'link', 'dev', 'lo', check_exit_code=False) def ensure_vlan_forward(public_ip, port, private_ip): diff --git a/nova/network/manager.py b/nova/network/manager.py index b80560bdf..0dd7f2360 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -100,8 +100,6 @@ flags.DEFINE_string('network_host', socket.gethostname(), 'Network host to use for ip allocation in flat modes') flags.DEFINE_bool('fake_call', False, 'If True, skip using the queue and make local calls') -flags.DEFINE_string('metadata_interface', 'eth0', - 'interface to add the metadata ip to') class AddressAlreadyAllocated(exception.Error): @@ -128,7 +126,7 @@ class NetworkManager(manager.SchedulerDependentManager): standalone service. """ self.driver.init_host() - self.driver.ensure_metadata_ip(FLAGS.metadata_interface) + self.driver.ensure_metadata_ip() # Set up networking for the projects for which we're already # the designated network host. ctxt = context.get_admin_context() -- cgit From b54be0e29cdcd91e3d106fb587b89c39ca3a0bff Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Thu, 7 Apr 2011 15:52:27 -0700 Subject: Removed commented-out old 'delete instance on SHUTOFF' code --- nova/compute/manager.py | 6 ------ nova/virt/libvirt_conn.py | 8 +------- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 72f04ecb1..d3cd2e51a 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1090,12 +1090,6 @@ class ComputeManager(manager.SchedulerDependentManager): # NOTE(justinsb): We no longer auto-remove SHUTOFF instances # It's quite hard to get them back when we do. - #if vm_state == power_state.SHUTOFF: - # # TODO(soren): This is what the compute manager does when you - # # terminate an instance. At some point I figure we'll have a - # # "terminated" state and some sort of cleanup job that runs - # # occasionally, cleaning them out. - # self.db.instance_destroy(context, db_instance['id']) # Are there VMs not in the DB? for vm_not_found_in_db in vms_not_found_in_db: diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 533ff9394..4523cdd2f 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -232,14 +232,8 @@ class LibvirtConnection(driver.ComputeDriver): {'name': instance['name'], 'state': state}) db.instance_set_state(ctxt, instance['id'], state) - # NOTE(justinsb): We no longer delete these instances, + # NOTE(justinsb): We no longer delete SHUTOFF instances, # the user may want to power them back on - #if state == power_state.SHUTOFF: - # # TODO(soren): This is what the compute manager does when you - # # terminate # an instance. At some point I figure we'll have a - # # "terminated" state and some sort of cleanup job that runs - # # occasionally, cleaning them out. - # db.instance_destroy(ctxt, instance['id']) if state != power_state.RUNNING: continue -- cgit From 52478e039b094861e7d783b7995b9cafa68e32b9 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Thu, 7 Apr 2011 15:56:16 -0700 Subject: Fix to correct libvirt error code when the domain is not found --- nova/virt/libvirt_conn.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 4523cdd2f..a7a8a14b1 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -974,7 +974,7 @@ class LibvirtConnection(driver.ComputeDriver): try: virt_dom = self._conn.lookupByName(instance_name) except libvirt.libvirtError as e: - if e.get_error_code() == libvirt.VIR_ERR_UNKNOWN_HOST: + if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: raise exception.NotFound(_("Instance %s not found") % instance_name) LOG.warning(_("Error from libvirt during lookup: %s") % e) -- cgit From 0c5f70c0bcf9395fb25a231057d997b075d04fda Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Thu, 7 Apr 2011 16:00:55 -0700 Subject: Log libvirt errcode on exception --- nova/virt/libvirt_conn.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index a7a8a14b1..5f1c12ab3 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -974,10 +974,13 @@ class LibvirtConnection(driver.ComputeDriver): try: virt_dom = self._conn.lookupByName(instance_name) except libvirt.libvirtError as e: - if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: + errcode = e.get_error_code() + if errcode == libvirt.VIR_ERR_NO_DOMAIN: raise exception.NotFound(_("Instance %s not found") % instance_name) - LOG.warning(_("Error from libvirt during lookup: %s") % e) + LOG.warning(_("Error from libvirt during lookup. " + "Code=%(errcode)s Error=%(e)s") % + locals()) raise (state, max_mem, mem, num_cpu, cpu_time) = virt_dom.info() -- cgit From b0ad20c796bd25dad0538ab85b1a56f421e16039 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Thu, 7 Apr 2011 16:42:33 -0700 Subject: fix tests from moving access check into update and delete --- nova/image/local.py | 5 ++++- nova/tests/image/test_glance.py | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/nova/image/local.py b/nova/image/local.py index 8bf78b4c9..d4fd62156 100644 --- a/nova/image/local.py +++ b/nova/image/local.py @@ -122,12 +122,15 @@ class LocalImageService(service.BaseImageService): image_path = self._path_to(image_id, None) if not os.path.exists(image_path): os.mkdir(image_path) - return self.update(context, image_id, metadata, data) + return self._store(context, image_id, metadata, data) def update(self, context, image_id, metadata, data=None): """Replace the contents of the given image with the new data.""" # NOTE(vish): show is to check if image is available self.show(context, image_id) + return self._store(context, image_id, metadata, data) + + def _store(self, context, image_id, metadata, data=None): metadata['id'] = image_id try: if data: diff --git a/nova/tests/image/test_glance.py b/nova/tests/image/test_glance.py index 9d0b14613..109905ded 100644 --- a/nova/tests/image/test_glance.py +++ b/nova/tests/image/test_glance.py @@ -209,17 +209,17 @@ class TestMutatorDateTimeTests(BaseGlanceTest): self.assertDateTimesEmpty(image_meta) def test_update_handles_datetimes(self): + self.client.images = {'image1': self._make_datetime_fixture()} self.client.update_response = self._make_datetime_fixture() - dummy_id = 'dummy_id' dummy_meta = {} - image_meta = self.service.update(self.context, 'dummy_id', dummy_meta) + image_meta = self.service.update(self.context, 'image1', dummy_meta) self.assertDateTimesFilled(image_meta) def test_update_handles_none_datetimes(self): + self.client.images = {'image1': self._make_datetime_fixture()} self.client.update_response = self._make_none_datetime_fixture() - dummy_id = 'dummy_id' dummy_meta = {} - image_meta = self.service.update(self.context, 'dummy_id', dummy_meta) + image_meta = self.service.update(self.context, 'image1', dummy_meta) self.assertDateTimesEmpty(image_meta) def _make_datetime_fixture(self): -- cgit