From 9b12a6c5ec11fd6ef3e110e6f0574762060ac809 Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Tue, 6 Sep 2011 15:19:37 -0400 Subject: Fixes an issue where 'invalid literal for int' would occur when listing images after making a v1.1 server snapshot (with a UUID). v1.1 image id's are now treated as strings (not integer ID's). The v1.0 API still tries to treat image id's as integers but doesn't fail miserably if they are uuid's either. This should pave the way for image ID's as uuids and more closely matches the v1.1 spec with regards to images and the server refs they contain. --- nova/api/openstack/common.py | 24 ++------- nova/api/openstack/views/images.py | 10 ++++ nova/tests/api/openstack/test_common.py | 40 ++++++++++---- nova/tests/api/openstack/test_images.py | 95 +++++++++++++++++++++++++-------- 4 files changed, 118 insertions(+), 51 deletions(-) diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index d743a66ef..dba3ec8e9 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -185,30 +185,16 @@ def limited_by_marker(items, request, max_limit=FLAGS.osapi_max_limit): def get_id_from_href(href): - """Return the id portion of a url as an int. + """Return the id or uuid portion of a url. Given: 'http://www.foo.com/bar/123?q=4' - Returns: 123 + Returns: '123' - In order to support local hrefs, the href argument can be just an id: - Given: '123' - Returns: 123 + Given: 'http://www.foo.com/bar/abc123?q=4' + Returns: 'abc123' """ - LOG.debug(_("Attempting to treat %(href)s as an integer ID.") % locals()) - - try: - return int(href) - except ValueError: - pass - - LOG.debug(_("Attempting to treat %(href)s as a URL.") % locals()) - - try: - return int(urlparse.urlsplit(href).path.split('/')[-1]) - except ValueError as error: - LOG.debug(_("Failed to parse ID from %(href)s: %(error)s") % locals()) - raise + return urlparse.urlsplit("%s" % href).path.split('/')[-1] def remove_version_from_href(href): diff --git a/nova/api/openstack/views/images.py b/nova/api/openstack/views/images.py index 21f1b2d3e..20c99124b 100644 --- a/nova/api/openstack/views/images.py +++ b/nova/api/openstack/views/images.py @@ -70,6 +70,7 @@ class ViewBuilder(object): } self._build_server(image, image_obj) + self._build_image_id(image, image_obj) if detail: image.update({ @@ -95,6 +96,12 @@ class ViewBuilderV10(ViewBuilder): except (KeyError, ValueError): pass + def _build_image_id(self, image, image_obj): + try: + image['id'] = int(image_obj['id']) + except ValueError: + pass + class ViewBuilderV11(ViewBuilder): """OpenStack API v1.1 Image Builder""" @@ -118,6 +125,9 @@ class ViewBuilderV11(ViewBuilder): except KeyError: return + def _build_image_id(self, image, image_obj): + image['id'] = "%s" % image_obj['id'] + def generate_href(self, image_id): """Return an href string pointing to this object.""" return os.path.join(self.base_url, self.project_id, diff --git a/nova/tests/api/openstack/test_common.py b/nova/tests/api/openstack/test_common.py index b422bc4d1..f519ea72b 100644 --- a/nova/tests/api/openstack/test_common.py +++ b/nova/tests/api/openstack/test_common.py @@ -237,21 +237,41 @@ class MiscFunctionsTest(test.TestCase): common.remove_version_from_href, fixture) - def test_get_id_from_href(self): + def test_get_id_from_href_with_int_url(self): fixture = 'http://www.testsite.com/dir/45' actual = common.get_id_from_href(fixture) - expected = 45 + expected = '45' self.assertEqual(actual, expected) - def test_get_id_from_href_bad_request(self): - fixture = 'http://45' - self.assertRaises(ValueError, - common.get_id_from_href, - fixture) + def test_get_id_from_href_with_int(self): + fixture = '45' + actual = common.get_id_from_href(fixture) + expected = '45' + self.assertEqual(actual, expected) - def test_get_id_from_href_int(self): - fixture = 1 - self.assertEqual(fixture, common.get_id_from_href(fixture)) + def test_get_id_from_href_with_int_url_query(self): + fixture = 'http://www.testsite.com/dir/45?asdf=jkl' + actual = common.get_id_from_href(fixture) + expected = '45' + self.assertEqual(actual, expected) + + def test_get_id_from_href_with_uuid_url(self): + fixture = 'http://www.testsite.com/dir/abc123' + actual = common.get_id_from_href(fixture) + expected = "abc123" + self.assertEqual(actual, expected) + + def test_get_id_from_href_with_uuid_url_query(self): + fixture = 'http://www.testsite.com/dir/abc123?asdf=jkl' + actual = common.get_id_from_href(fixture) + expected = "abc123" + self.assertEqual(actual, expected) + + def test_get_id_from_href_with_uuid(self): + fixture = 'abc123' + actual = common.get_id_from_href(fixture) + expected = 'abc123' + self.assertEqual(actual, expected) def test_get_version_from_href(self): fixture = 'http://www.testsite.com/v1.1/images' diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index 2a7cfc382..6448e9986 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -365,7 +365,8 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): {'id': 125, 'name': 'saving snapshot'}, {'id': 126, 'name': 'active snapshot'}, {'id': 127, 'name': 'killed snapshot'}, - {'id': 129, 'name': None}] + {'id': 128, 'name': 'active UUID snapshot'}, + {'id': 130, 'name': None}] self.assertDictListMatch(response_list, expected) @@ -403,14 +404,14 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): expected_image = { "image": { - "id": 124, + "id": '124', "name": "queued snapshot", "updated": self.NOW_API_FORMAT, "created": self.NOW_API_FORMAT, "status": "QUEUED", "progress": 0, 'server': { - 'id': 42, + 'id': '42', "links": [{ "rel": "self", "href": server_href, @@ -458,7 +459,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): self.assertEqual(expected_image.toxml(), actual_image.toxml()) def test_get_image_xml_no_name(self): - request = webob.Request.blank('/v1.0/images/129') + request = webob.Request.blank('/v1.0/images/130') request.accept = "application/xml" response = request.get_response(fakes.wsgi_app()) @@ -466,7 +467,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): expected_now = self.NOW_API_FORMAT expected_image = minidom.parseString(""" - Date: Tue, 13 Sep 2011 15:48:10 -0400 Subject: Update test_libvirt so that flags and fakes are used instead of mocks for utils.import_class and utils.import_object. Fixes #lp849329. --- nova/tests/fake_network.py | 30 ++++++++++++++++ nova/tests/test_libvirt.py | 87 +++++++++++++++++----------------------------- 2 files changed, 62 insertions(+), 55 deletions(-) diff --git a/nova/tests/fake_network.py b/nova/tests/fake_network.py index 1ecb99b31..142206755 100644 --- a/nova/tests/fake_network.py +++ b/nova/tests/fake_network.py @@ -25,6 +25,36 @@ HOST = "testhost" FLAGS = flags.FLAGS +class FakeIptablesFirewallDriver(object): + def __init__(self, **kwargs): + pass + + def setattr(self, key, val): + self.__setattr__(key, val) + + def apply_instance_filter(self, instance, network_info): + pass + + +class FakeVIFDriver(object): + + def __init__(self, **kwargs): + pass + + def setattr(self, key, val): + self.__setattr__(key, val) + + def plug(self, instance, network, mapping): + return { + 'id': 'fake', + 'bridge_name': 'fake', + 'mac_address': 'fake', + 'ip_address': 'fake', + 'dhcp_server': 'fake', + 'extra_params': 'fake', + } + + class FakeModel(dict): """Represent a model from the db""" def __init__(self, *args, **kwargs): diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py index 233ee14de..8193d6ec2 100644 --- a/nova/tests/test_libvirt.py +++ b/nova/tests/test_libvirt.py @@ -51,6 +51,32 @@ def _concurrency(wait, done, target): done.send() +class FakeVirtDomain(object): + + def __init__(self, fake_xml=None): + if fake_xml: + self._fake_dom_xml = fake_xml + else: + self._fake_dom_xml=""" + + + + + + + + """ + + def snapshotCreateXML(self, *args): + return None + + def createWithFlags(self, launch_flags): + pass + + def XMLDesc(self, *args): + return self._fake_dom_xml + + class CacheConcurrencyTestCase(test.TestCase): def setUp(self): super(CacheConcurrencyTestCase, self).setUp() @@ -152,70 +178,23 @@ class LibvirtConnTestCase(test.TestCase): # A fake libvirt.virConnect class FakeLibvirtConnection(object): - pass - - # A fake connection.IptablesFirewallDriver - class FakeIptablesFirewallDriver(object): - - def __init__(self, **kwargs): - pass - - def setattr(self, key, val): - self.__setattr__(key, val) - - # A fake VIF driver - class FakeVIFDriver(object): - - def __init__(self, **kwargs): - pass - - def setattr(self, key, val): - self.__setattr__(key, val) - - def plug(self, instance, network, mapping): - return { - 'id': 'fake', - 'bridge_name': 'fake', - 'mac_address': 'fake', - 'ip_address': 'fake', - 'dhcp_server': 'fake', - 'extra_params': 'fake', - } + def defineXML(self, xml): + return FakeVirtDomain() # Creating mocks fake = FakeLibvirtConnection() - fakeip = FakeIptablesFirewallDriver - fakevif = FakeVIFDriver() # Customizing above fake if necessary for key, val in kwargs.items(): fake.__setattr__(key, val) - # Inevitable mocks for connection.LibvirtConnection - self.mox.StubOutWithMock(connection.utils, 'import_class') - connection.utils.import_class(mox.IgnoreArg()).AndReturn(fakeip) - self.mox.StubOutWithMock(connection.utils, 'import_object') - connection.utils.import_object(mox.IgnoreArg()).AndReturn(fakevif) + self.flags(image_service='nova.image.fake.FakeImageService') + self.flags(firewall_driver="nova.tests.fake_network.FakeIptablesFirewallDriver") + self.flags(libvirt_vif_driver="nova.tests.fake_network.FakeVIFDriver") + self.mox.StubOutWithMock(connection.LibvirtConnection, '_conn') connection.LibvirtConnection._conn = fake def fake_lookup(self, instance_name): - - class FakeVirtDomain(object): - - def snapshotCreateXML(self, *args): - return None - - def XMLDesc(self, *args): - return """ - - - - - - - - """ - return FakeVirtDomain() def fake_execute(self, *args): @@ -797,8 +776,6 @@ class LibvirtConnTestCase(test.TestCase): shutil.rmtree(os.path.join(FLAGS.instances_path, instance.name)) shutil.rmtree(os.path.join(FLAGS.instances_path, '_base')) - self.assertTrue(count) - def test_get_host_ip_addr(self): conn = connection.LibvirtConnection(False) ip = conn.get_host_ip_addr() -- cgit From a85a2c2e82fa8820b04f669c92a3500c7c6cebe2 Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Tue, 13 Sep 2011 20:38:26 -0400 Subject: pep8 fixes. --- nova/tests/test_libvirt.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py index 8193d6ec2..a30b00dbe 100644 --- a/nova/tests/test_libvirt.py +++ b/nova/tests/test_libvirt.py @@ -57,7 +57,7 @@ class FakeVirtDomain(object): if fake_xml: self._fake_dom_xml = fake_xml else: - self._fake_dom_xml=""" + self._fake_dom_xml = """ @@ -188,7 +188,8 @@ class LibvirtConnTestCase(test.TestCase): fake.__setattr__(key, val) self.flags(image_service='nova.image.fake.FakeImageService') - self.flags(firewall_driver="nova.tests.fake_network.FakeIptablesFirewallDriver") + fw_driver = "nova.tests.fake_network.FakeIptablesFirewallDriver" + self.flags(firewall_driver=fw_driver) self.flags(libvirt_vif_driver="nova.tests.fake_network.FakeVIFDriver") self.mox.StubOutWithMock(connection.LibvirtConnection, '_conn') -- cgit From 4eb704e9024111c80f6f7c83810a08d7eec5c4af Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Tue, 13 Sep 2011 21:13:58 -0400 Subject: last of the api.openstack.test_images merge fixes. --- nova/tests/api/openstack/test_images.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index 6890e0e9e..e054ffd03 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -821,7 +821,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): }], }, { - 'id': 128, + 'id': '128', 'name': 'deleted snapshot', 'metadata': { u'instance_ref': u'http://localhost/v1.1/servers/42', @@ -832,7 +832,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): 'status': 'DELETED', 'progress': 0, 'server': { - 'id': 42, + 'id': '42', "links": [{ "rel": "self", "href": server_href, @@ -852,7 +852,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): }], }, { - 'id': 129, + 'id': '129', 'name': 'pending_delete snapshot', 'metadata': { u'instance_ref': u'http://localhost/v1.1/servers/42', @@ -863,7 +863,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): 'status': 'DELETED', 'progress': 0, 'server': { - 'id': 42, + 'id': '42', "links": [{ "rel": "self", "href": server_href, @@ -914,7 +914,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): }], }, { - 'id': 132, + 'id': '132', 'name': None, 'metadata': {}, 'updated': self.NOW_API_FORMAT, @@ -1224,7 +1224,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): # Snapshots for User 1 server_ref = 'http://localhost/v1.1/servers/42' snapshot_properties = {'instance_ref': server_ref, 'user_id': 'fake'} - statuses = ('queued', 'saving', 'active','killed', + statuses = ('queued', 'saving', 'active', 'killed', 'deleted', 'pending_delete') for status in statuses: add_fixture(id=image_id, name='%s snapshot' % status, -- cgit From 7a02394009aae85f722430682f8360371121504b Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Wed, 14 Sep 2011 11:33:36 -0400 Subject: Make tests pass. --- nova/tests/api/openstack/test_images.py | 38 ++++++++++++++++----------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index 45ca73073..27ae8f2bd 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -77,14 +77,14 @@ class ImagesTest(test.TestCase): response_dict = json.loads(response.body) response_list = response_dict["images"] - expected = [{'id': '123', 'name': 'public image'}, - {'id': '124', 'name': 'queued snapshot'}, - {'id': '125', 'name': 'saving snapshot'}, - {'id': '126', 'name': 'active snapshot'}, - {'id': '127', 'name': 'killed snapshot'}, - {'id': '128', 'name': 'deleted snapshot'}, - {'id': '129', 'name': 'pending_delete snapshot'}, - {'id': '130', 'name': None}] + expected = [{'id': 123, 'name': 'public image'}, + {'id': 124, 'name': 'queued snapshot'}, + {'id': 125, 'name': 'saving snapshot'}, + {'id': 126, 'name': 'active snapshot'}, + {'id': 127, 'name': 'killed snapshot'}, + {'id': 128, 'name': 'deleted snapshot'}, + {'id': 129, 'name': 'pending_delete snapshot'}, + {'id': 130, 'name': None}] self.assertDictListMatch(response_list, expected) @@ -99,7 +99,7 @@ class ImagesTest(test.TestCase): expected_image = { "image": { - "id": "123", + "id": 123, "name": "public image", "updated": NOW_API_FORMAT, "created": NOW_API_FORMAT, @@ -406,7 +406,7 @@ class ImagesTest(test.TestCase): response_list = response_dict["images"] expected = [{ - 'id': '123', + 'id': 123, 'name': 'public image', 'updated': NOW_API_FORMAT, 'created': NOW_API_FORMAT, @@ -414,7 +414,7 @@ class ImagesTest(test.TestCase): 'progress': 100, }, { - 'id': '124', + 'id': 124, 'name': 'queued snapshot', 'updated': NOW_API_FORMAT, 'created': NOW_API_FORMAT, @@ -422,7 +422,7 @@ class ImagesTest(test.TestCase): 'progress': 0, }, { - 'id': '125', + 'id': 125, 'name': 'saving snapshot', 'updated': NOW_API_FORMAT, 'created': NOW_API_FORMAT, @@ -430,7 +430,7 @@ class ImagesTest(test.TestCase): 'progress': 0, }, { - 'id': '126', + 'id': 126, 'name': 'active snapshot', 'updated': NOW_API_FORMAT, 'created': NOW_API_FORMAT, @@ -438,7 +438,7 @@ class ImagesTest(test.TestCase): 'progress': 100, }, { - 'id': '127', + 'id': 127, 'name': 'killed snapshot', 'updated': NOW_API_FORMAT, 'created': NOW_API_FORMAT, @@ -446,7 +446,7 @@ class ImagesTest(test.TestCase): 'progress': 0, }, { - 'id': '128', + 'id': 128, 'name': 'deleted snapshot', 'updated': NOW_API_FORMAT, 'created': NOW_API_FORMAT, @@ -454,7 +454,7 @@ class ImagesTest(test.TestCase): 'progress': 0, }, { - 'id': '129', + 'id': 129, 'name': 'pending_delete snapshot', 'updated': NOW_API_FORMAT, 'created': NOW_API_FORMAT, @@ -462,7 +462,7 @@ class ImagesTest(test.TestCase): 'progress': 0, }, { - 'id': '130', + 'id': 130, 'name': None, 'updated': NOW_API_FORMAT, 'created': NOW_API_FORMAT, @@ -914,7 +914,7 @@ class ImagesTest(test.TestCase): app = fakes.wsgi_app(fake_auth_context=self._get_fake_context()) res = req.get_response(app) image_meta = json.loads(res.body)['image'] - expected = {'id': '123', 'name': 'public image', + expected = {'id': 123, 'name': 'public image', 'updated': NOW_API_FORMAT, 'created': NOW_API_FORMAT, 'status': 'ACTIVE', 'progress': 100} @@ -934,7 +934,7 @@ class ImagesTest(test.TestCase): response = req.get_response(fakes.wsgi_app()) self.assertEqual(200, response.status_int) image_meta = json.loads(response.body)['image'] - self.assertEqual('123', image_meta['serverId']) + self.assertEqual(123, image_meta['serverId']) self.assertEqual('Snapshot 1', image_meta['name']) def test_create_snapshot_no_name(self): -- cgit