From 655eb8fbd21376e694f8134e42f10ddbc1aafb0e Mon Sep 17 00:00:00 2001 From: John Tran Date: Wed, 6 Apr 2011 18:22:03 -0700 Subject: ec2 api run_instances checks for image status must be 'available'. Overhauled test_run_instances for working set of test assertions --- nova/api/ec2/cloud.py | 10 +++++++- nova/tests/test_cloud.py | 62 +++++++++++++++++++++++++----------------------- 2 files changed, 41 insertions(+), 31 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 58effd134..0ea0e3603 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -814,10 +814,18 @@ class CloudController(object): if kwargs.get('ramdisk_id'): ramdisk = self._get_image(context, kwargs['ramdisk_id']) kwargs['ramdisk_id'] = ramdisk['id'] + image = self._get_image(context, kwargs['image_id']) + if not image: + raise exception.NotFound(_('Image %s not found') % + kwargs['image_id']) + if not 'properties' in image or \ + (not 'image_state' in image['properties']) or \ + (image['properties']['image_state'] is not 'available'): + raise exception.ApiError(_('Image must be available')) instances = self.compute_api.create(context, instance_type=instance_types.get_by_type( kwargs.get('instance_type', None)), - image_id=self._get_image(context, kwargs['image_id'])['id'], + image_id = image['id'], min_count=int(kwargs.get('min_count', max_count)), max_count=max_count, kernel_id=kwargs.get('kernel_id'), diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 5cb969979..85f3a8e87 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -36,12 +36,12 @@ from nova import rpc from nova import service from nova import test from nova import utils +from nova import exception from nova.auth import manager from nova.compute import power_state from nova.api.ec2 import cloud from nova.api.ec2 import ec2utils from nova.image import local -from nova.exception import NotFound FLAGS = flags.FLAGS @@ -226,7 +226,7 @@ class CloudTestCase(test.TestCase): 'type': 'machine'}}] def fake_show_none(meh, context, id): - raise NotFound + raise exception.NotFound self.stubs.Set(local.LocalImageService, 'detail', fake_detail) # list all @@ -244,7 +244,7 @@ class CloudTestCase(test.TestCase): self.stubs.UnsetAll() self.stubs.Set(local.LocalImageService, 'show', fake_show_none) self.stubs.Set(local.LocalImageService, 'show_by_name', fake_show_none) - self.assertRaises(NotFound, describe_images, + self.assertRaises(exception.NotFound, describe_images, self.context, ['ami-fake']) def test_console_output(self): @@ -307,39 +307,41 @@ class CloudTestCase(test.TestCase): self.cloud.delete_key_pair(self.context, 'test') def test_run_instances(self): - if FLAGS.connection_type == 'fake': - LOG.debug(_("Can't test instances without a real virtual env.")) - return + allinst = db.instance_get_all(context.get_admin_context()) + self.assertEqual(0, len(allinst)) + def fake_show_decrypt(meh, context, id): + return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1, + 'type': 'machine', 'image_state': 'decrypting'}} + + def fake_show_avail(meh, context, id): + return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1, + 'type': 'machine', 'image_state': 'available'}} + image_id = FLAGS.default_image instance_type = FLAGS.default_instance_type max_count = 1 kwargs = {'image_id': image_id, 'instance_type': instance_type, 'max_count': max_count} - rv = self.cloud.run_instances(self.context, **kwargs) - # TODO: check for proper response - instance_id = rv['reservationSet'][0].keys()[0] - instance = rv['reservationSet'][0][instance_id][0] - LOG.debug(_("Need to watch instance %s until it's running..."), - instance['instance_id']) - while True: - greenthread.sleep(1) - info = self.cloud._get_instance(instance['instance_id']) - LOG.debug(info['state']) - if info['state'] == power_state.RUNNING: - break - self.assert_(rv) - - if FLAGS.connection_type != 'fake': - time.sleep(45) # Should use boto for polling here - for reservations in rv['reservationSet']: - # for res_id in reservations.keys(): - # LOG.debug(reservations[res_id]) - # for instance in reservations[res_id]: - for instance in reservations[reservations.keys()[0]]: - instance_id = instance['instance_id'] - LOG.debug(_("Terminating instance %s"), instance_id) - rv = self.compute.terminate_instance(instance_id) + run_instances = self.cloud.run_instances + # when image doesn't have 'image_state' attr at all + self.assertRaises(exception.ApiError, run_instances, + self.context, **kwargs) + # when image has 'image_state' yet not 'available' + self.stubs.UnsetAll() + self.stubs.Set(local.LocalImageService, 'show', fake_show_decrypt) + self.assertRaises(exception.ApiError, run_instances, + self.context, **kwargs) + # when image has valid image_state + self.stubs.UnsetAll() + self.stubs.Set(local.LocalImageService, 'show', fake_show_avail) + result = run_instances(self.context, **kwargs) + instance = result['instancesSet'][0] + self.assertEqual(instance['imageId'], 'ami-00000001') + self.assertEqual(instance['displayName'], 'Server 1') + self.assertEqual(instance['instanceId'], 'i-00000001') + self.assertEqual(instance['instanceState']['name'], 'scheduling') + self.assertEqual(instance['instanceType'], 'm1.small') def test_update_of_instance_display_fields(self): inst = db.instance_create(self.context, {}) -- cgit From 10db492376a8bb8409e3fb3c33707865ac0f3ee7 Mon Sep 17 00:00:00 2001 From: John Tran Date: Mon, 2 May 2011 14:25:21 -0700 Subject: implemented review suggestion EAFP style, and fixed test stub fake_show needs to have image_state = available or other tests will fail --- nova/api/ec2/cloud.py | 14 +++++++++----- nova/tests/test_cloud.py | 35 ++++++++++++++++++----------------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 0ea0e3603..5dc608139 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -815,17 +815,21 @@ class CloudController(object): ramdisk = self._get_image(context, kwargs['ramdisk_id']) kwargs['ramdisk_id'] = ramdisk['id'] image = self._get_image(context, kwargs['image_id']) - if not image: + if not image: raise exception.NotFound(_('Image %s not found') % kwargs['image_id']) - if not 'properties' in image or \ - (not 'image_state' in image['properties']) or \ - (image['properties']['image_state'] is not 'available'): + try: + available = (image['properties']['image_state'] == 'available') + except KeyError: + available = False + + if not available: raise exception.ApiError(_('Image must be available')) + instances = self.compute_api.create(context, instance_type=instance_types.get_by_type( kwargs.get('instance_type', None)), - image_id = image['id'], + image_id=image['id'], min_count=int(kwargs.get('min_count', max_count)), max_count=max_count, kernel_id=kwargs.get('kernel_id'), diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 85f3a8e87..da2fce06b 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -30,13 +30,13 @@ from eventlet import greenthread from nova import context from nova import crypto from nova import db +from nova import exception from nova import flags from nova import log as logging from nova import rpc from nova import service from nova import test from nova import utils -from nova import exception from nova.auth import manager from nova.compute import power_state from nova.api.ec2 import cloud @@ -73,7 +73,7 @@ class CloudTestCase(test.TestCase): def fake_show(meh, context, id): return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1, - 'type': 'machine'}} + 'type': 'machine', 'image_state': 'available'}} self.stubs.Set(local.LocalImageService, 'show', fake_show) self.stubs.Set(local.LocalImageService, 'show_by_name', fake_show) @@ -307,15 +307,16 @@ class CloudTestCase(test.TestCase): self.cloud.delete_key_pair(self.context, 'test') def test_run_instances(self): - allinst = db.instance_get_all(context.get_admin_context()) - self.assertEqual(0, len(allinst)) - def fake_show_decrypt(meh, context, id): + all_instances = db.instance_get_all(context.get_admin_context()) + self.assertEqual(0, len(all_instances)) + + def fake_show_decrypt(self, context, id): return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1, 'type': 'machine', 'image_state': 'decrypting'}} - def fake_show_avail(meh, context, id): + def fake_show_no_state(self, context, id): return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1, - 'type': 'machine', 'image_state': 'available'}} + 'type': 'machine'}} image_id = FLAGS.default_image instance_type = FLAGS.default_instance_type @@ -324,17 +325,7 @@ class CloudTestCase(test.TestCase): 'instance_type': instance_type, 'max_count': max_count} run_instances = self.cloud.run_instances - # when image doesn't have 'image_state' attr at all - self.assertRaises(exception.ApiError, run_instances, - self.context, **kwargs) - # when image has 'image_state' yet not 'available' - self.stubs.UnsetAll() - self.stubs.Set(local.LocalImageService, 'show', fake_show_decrypt) - self.assertRaises(exception.ApiError, run_instances, - self.context, **kwargs) # when image has valid image_state - self.stubs.UnsetAll() - self.stubs.Set(local.LocalImageService, 'show', fake_show_avail) result = run_instances(self.context, **kwargs) instance = result['instancesSet'][0] self.assertEqual(instance['imageId'], 'ami-00000001') @@ -342,6 +333,16 @@ class CloudTestCase(test.TestCase): self.assertEqual(instance['instanceId'], 'i-00000001') self.assertEqual(instance['instanceState']['name'], 'scheduling') self.assertEqual(instance['instanceType'], 'm1.small') + # when image doesn't have 'image_state' attr at all + self.stubs.UnsetAll() + self.stubs.Set(local.LocalImageService, 'show', fake_show_no_state) + self.assertRaises(exception.ApiError, run_instances, + self.context, **kwargs) + # when image has 'image_state' yet not 'available' + self.stubs.UnsetAll() + self.stubs.Set(local.LocalImageService, 'show', fake_show_decrypt) + self.assertRaises(exception.ApiError, run_instances, + self.context, **kwargs) def test_update_of_instance_display_fields(self): inst = db.instance_create(self.context, {}) -- cgit From 0a3da155228228d3f0eeac1efdea1e29eef2f3a0 Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 12 May 2011 12:04:39 -0700 Subject: changed NotFound exception to ImageNotFound --- nova/api/ec2/cloud.py | 3 +-- nova/tests/test_cloud.py | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 8a54d23f2..ad8c3fe90 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -823,8 +823,7 @@ class CloudController(object): kwargs['ramdisk_id'] = ramdisk['id'] image = self._get_image(context, kwargs['image_id']) if not image: - raise exception.NotFound(_('Image %s not found') % - kwargs['image_id']) + raise exception.ImageNotFound(kwargs['image_id']) try: available = (image['properties']['image_state'] == 'available') except KeyError: diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 202dc36bc..ebfb5ee44 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -227,7 +227,7 @@ class CloudTestCase(test.TestCase): 'type': 'machine'}}] def fake_show_none(meh, context, id): - raise exception.NotFound + raise exception.ImageNotFound self.stubs.Set(local.LocalImageService, 'detail', fake_detail) # list all @@ -245,7 +245,7 @@ class CloudTestCase(test.TestCase): self.stubs.UnsetAll() self.stubs.Set(local.LocalImageService, 'show', fake_show_none) self.stubs.Set(local.LocalImageService, 'show_by_name', fake_show_none) - self.assertRaises(exception.NotFound, describe_images, + self.assertRaises(exception.ImageNotFound, describe_images, self.context, ['ami-fake']) def test_describe_image_attribute(self): -- cgit From 7a521f49f6daf0a0a37a9ef98ff1ea8813f04a6f Mon Sep 17 00:00:00 2001 From: John Tran Date: Mon, 23 May 2011 14:54:11 -0700 Subject: merged from trunk --- nova/tests/test_cloud.py | 51 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index ebfb5ee44..f3887b07b 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -37,7 +37,6 @@ from nova import rpc from nova import service from nova import test from nova import utils -from nova import exception from nova.auth import manager from nova.compute import power_state from nova.api.ec2 import cloud @@ -279,6 +278,26 @@ class CloudTestCase(test.TestCase): user_group=['all']) self.assertEqual(True, result['is_public']) + def test_deregister_image(self): + deregister_image = self.cloud.deregister_image + + def fake_delete(self, context, id): + return None + + self.stubs.Set(local.LocalImageService, 'delete', fake_delete) + # valid image + result = deregister_image(self.context, 'ami-00000001') + self.assertEqual(result['imageId'], 'ami-00000001') + # invalid image + self.stubs.UnsetAll() + + def fake_detail_empty(self, context): + return [] + + self.stubs.Set(local.LocalImageService, 'detail', fake_detail_empty) + self.assertRaises(exception.ImageNotFound, deregister_image, + self.context, 'ami-bad001') + def test_console_output(self): instance_type = FLAGS.default_instance_type max_count = 1 @@ -334,6 +353,36 @@ class CloudTestCase(test.TestCase): self.assertTrue(filter(lambda k: k['keyName'] == 'test1', keys)) self.assertTrue(filter(lambda k: k['keyName'] == 'test2', keys)) + def test_import_public_key(self): + # test when user provides all values + result1 = self.cloud.import_public_key(self.context, + 'testimportkey1', + 'mytestpubkey', + 'mytestfprint') + self.assertTrue(result1) + keydata = db.key_pair_get(self.context, + self.context.user.id, + 'testimportkey1') + self.assertEqual('mytestpubkey', keydata['public_key']) + self.assertEqual('mytestfprint', keydata['fingerprint']) + # test when user omits fingerprint + pubkey_path = os.path.join(os.path.dirname(__file__), 'public_key') + f = open(pubkey_path + '/dummy.pub', 'r') + dummypub = f.readline().rstrip() + f.close + f = open(pubkey_path + '/dummy.fingerprint', 'r') + dummyfprint = f.readline().rstrip() + f.close + result2 = self.cloud.import_public_key(self.context, + 'testimportkey2', + dummypub) + self.assertTrue(result2) + keydata = db.key_pair_get(self.context, + self.context.user.id, + 'testimportkey2') + self.assertEqual(dummypub, keydata['public_key']) + self.assertEqual(dummyfprint, keydata['fingerprint']) + def test_delete_key_pair(self): self._create_key('test') self.cloud.delete_key_pair(self.context, 'test') -- cgit From 8e7c3121fab4b5a87c2efe865f3c06b1bd267cbc Mon Sep 17 00:00:00 2001 From: John Tran Date: Tue, 24 May 2011 08:59:02 -0700 Subject: added imageid string to exception, per peer review --- nova/tests/test_cloud.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index f3887b07b..e37aca4d6 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -226,7 +226,7 @@ class CloudTestCase(test.TestCase): 'type': 'machine'}}] def fake_show_none(meh, context, id): - raise exception.ImageNotFound + raise exception.ImageNotFound('bad_image_id') self.stubs.Set(local.LocalImageService, 'detail', fake_detail) # list all -- cgit From a0cffc4de8ba4b15958e320308477d42287858e7 Mon Sep 17 00:00:00 2001 From: John Tran Date: Tue, 24 May 2011 09:43:52 -0700 Subject: specified image_id keyword in exception arg --- nova/api/ec2/cloud.py | 2 +- nova/tests/test_cloud.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 59e00781e..80c61d62b 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -848,7 +848,7 @@ class CloudController(object): kwargs['ramdisk_id'] = ramdisk['id'] image = self._get_image(context, kwargs['image_id']) if not image: - raise exception.ImageNotFound(kwargs['image_id']) + raise exception.ImageNotFound(image_id=kwargs['image_id']) try: available = (image['properties']['image_state'] == 'available') except KeyError: diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index e37aca4d6..1e583377b 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -226,7 +226,7 @@ class CloudTestCase(test.TestCase): 'type': 'machine'}}] def fake_show_none(meh, context, id): - raise exception.ImageNotFound('bad_image_id') + raise exception.ImageNotFound(image_id='bad_image_id') self.stubs.Set(local.LocalImageService, 'detail', fake_detail) # list all -- cgit From 5d89721f5fa3212146749236c666f0e584c8590f Mon Sep 17 00:00:00 2001 From: John Tran Date: Wed, 1 Jun 2011 16:27:51 -0700 Subject: merged, with trunk, fixed the test failure, and split the test into 3 as per peer review. --- nova/tests/test_cloud.py | 45 ++++++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 04fd02ba3..02b7c8a38 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -445,38 +445,41 @@ class CloudTestCase(test.TestCase): self.cloud.delete_key_pair(self.context, 'test') def test_run_instances(self): - all_instances = db.instance_get_all(context.get_admin_context()) - self.assertEqual(0, len(all_instances)) - - def fake_show_decrypt(self, context, id): - return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1, - 'type': 'machine', 'image_state': 'decrypting'}} - - def fake_show_no_state(self, context, id): - return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1, - 'type': 'machine'}} - - image_id = FLAGS.default_image - instance_type = FLAGS.default_instance_type - max_count = 1 - kwargs = {'image_id': image_id, - 'instance_type': instance_type, - 'max_count': max_count} + kwargs = {'image_id': FLAGS.default_image, + 'instance_type': FLAGS.default_instance_type, + 'max_count': 1} run_instances = self.cloud.run_instances - # when image has valid image_state result = run_instances(self.context, **kwargs) instance = result['instancesSet'][0] self.assertEqual(instance['imageId'], 'ami-00000001') self.assertEqual(instance['displayName'], 'Server 1') self.assertEqual(instance['instanceId'], 'i-00000001') - self.assertEqual(instance['instanceState']['name'], 'scheduling') + self.assertEqual(instance['instanceState']['name'], 'networking') self.assertEqual(instance['instanceType'], 'm1.small') - # when image doesn't have 'image_state' attr at all + + def test_run_instances_image_state_none(self): + kwargs = {'image_id': FLAGS.default_image, + 'instance_type': FLAGS.default_instance_type, + 'max_count': 1} + run_instances = self.cloud.run_instances + def fake_show_no_state(self, context, id): + return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1, + 'type': 'machine'}} + self.stubs.UnsetAll() self.stubs.Set(local.LocalImageService, 'show', fake_show_no_state) self.assertRaises(exception.ApiError, run_instances, self.context, **kwargs) - # when image has 'image_state' yet not 'available' + + def test_run_instances_image_state_invalid(self): + kwargs = {'image_id': FLAGS.default_image, + 'instance_type': FLAGS.default_instance_type, + 'max_count': 1} + run_instances = self.cloud.run_instances + def fake_show_decrypt(self, context, id): + return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1, + 'type': 'machine', 'image_state': 'decrypting'}} + self.stubs.UnsetAll() self.stubs.Set(local.LocalImageService, 'show', fake_show_decrypt) self.assertRaises(exception.ApiError, run_instances, -- cgit