From 3487ece9cbaa2396263ee75ec42f4e476e576280 Mon Sep 17 00:00:00 2001 From: Unmesh Gurjar Date: Sat, 11 Aug 2012 10:31:51 -0700 Subject: [PATCH] Adds new volume API extensions Adds following extensions: 1. Create volume from image 2. Copy volume to image Added unit tests. Implements: blueprint create-volume-from-image Conflicts: cinder/api/openstack/volume/contrib/volume_actions.py cinder/tests/api/openstack/fakes.py cinder/tests/api/openstack/volume/contrib/test_volume_actions.py cinder/tests/policy.json nova/api/openstack/volume/volumes.py nova/flags.py nova/tests/api/openstack/volume/test_volumes.py nova/tests/test_volume.py nova/utils.py nova/volume/api.py nova/volume/manager.py This is based on a cherry-pick of cinder commit 2f5360753308eb8b10581fc3c026c1b66f42ebdc with bug fixes 8c30edff982042d2533a810709308b586267c0e9 and ffe5036fa0e63ccde2d19aa0f425ec43de338dd7 squashed in. Change-Id: I9c73bd3fa2fa2e0648c01ff3f4fc66f757d7bc3f --- nova/scheduler/chance.py | 4 +- nova/scheduler/driver.py | 2 +- nova/scheduler/filter_scheduler.py | 2 +- nova/scheduler/manager.py | 5 +- nova/scheduler/rpcapi.py | 5 +- nova/scheduler/simple.py | 7 +- nova/tests/policy.json | 1 + nova/tests/scheduler/test_rpcapi.py | 3 +- nova/tests/test_volume.py | 241 +++++++++++++++++++++++++++- nova/utils.py | 12 ++ 10 files changed, 266 insertions(+), 16 deletions(-) diff --git a/nova/scheduler/chance.py b/nova/scheduler/chance.py index e44e9d7d2..b8dd468f0 100644 --- a/nova/scheduler/chance.py +++ b/nova/scheduler/chance.py @@ -93,10 +93,10 @@ class ChanceScheduler(driver.Scheduler): self.compute_rpcapi.prep_resize(context, image, instance, instance_type, host, reservations) - def schedule_create_volume(self, context, volume_id, snapshot_id, + def schedule_create_volume(self, context, volume_id, snapshot_id, image_id, reservations): """Picks a host that is up at random.""" host = self._schedule(context, FLAGS.volume_topic, None, {}) driver.cast_to_host(context, FLAGS.volume_topic, host, 'create_volume', volume_id=volume_id, snapshot_id=snapshot_id, - reservations=reservations) + image_id=image_id, reservations=reservations) diff --git a/nova/scheduler/driver.py b/nova/scheduler/driver.py index ee7db02b9..df49acfae 100644 --- a/nova/scheduler/driver.py +++ b/nova/scheduler/driver.py @@ -207,7 +207,7 @@ class Scheduler(object): msg = _("Driver must implement schedule_run_instance") raise NotImplementedError(msg) - def schedule_create_volume(self, context, volume_id, snapshot_id, + def schedule_create_volume(self, context, volume_id, snapshot_id, image_id, reservations): msg = _("Driver must implement schedule_create_volune") raise NotImplementedError(msg) diff --git a/nova/scheduler/filter_scheduler.py b/nova/scheduler/filter_scheduler.py index a3e3e1354..371aebf53 100644 --- a/nova/scheduler/filter_scheduler.py +++ b/nova/scheduler/filter_scheduler.py @@ -42,7 +42,7 @@ class FilterScheduler(driver.Scheduler): self.cost_function_cache = {} self.options = scheduler_options.SchedulerOptions() - def schedule_create_volume(self, context, volume_id, snapshot_id, + def schedule_create_volume(self, context, volume_id, snapshot_id, image_id, reservations): # NOTE: We're only focused on compute instances right now, # so this method will always raise NoValidHost(). diff --git a/nova/scheduler/manager.py b/nova/scheduler/manager.py index dca8c0cea..647c2c780 100644 --- a/nova/scheduler/manager.py +++ b/nova/scheduler/manager.py @@ -69,10 +69,11 @@ class SchedulerManager(manager.Manager): self.driver.update_service_capabilities(service_name, host, capabilities) - def create_volume(self, context, volume_id, snapshot_id, reservations): + def create_volume(self, context, volume_id, snapshot_id, image_id, + reservations): try: self.driver.schedule_create_volume( - context, volume_id, snapshot_id, reservations) + context, volume_id, snapshot_id, image_id, reservations) except Exception as ex: with excutils.save_and_reraise_exception(): self._set_vm_state_and_notify('create_volume', diff --git a/nova/scheduler/rpcapi.py b/nova/scheduler/rpcapi.py index 940fe315e..894131179 100644 --- a/nova/scheduler/rpcapi.py +++ b/nova/scheduler/rpcapi.py @@ -93,11 +93,12 @@ class SchedulerAPI(nova.openstack.common.rpc.proxy.RpcProxy): disk_over_commit=disk_over_commit, instance=instance_p, dest=dest)) - def create_volume(self, ctxt, volume_id, snapshot_id, reservations): + def create_volume(self, ctxt, volume_id, snapshot_id, image_id, + reservations): self.cast(ctxt, self.make_msg('create_volume', volume_id=volume_id, snapshot_id=snapshot_id, - reservations=reservations)) + image_id=image_id, reservations=reservations)) def update_service_capabilities(self, ctxt, service_name, host, capabilities): diff --git a/nova/scheduler/simple.py b/nova/scheduler/simple.py index 78578dbd0..5eee0b136 100644 --- a/nova/scheduler/simple.py +++ b/nova/scheduler/simple.py @@ -56,7 +56,7 @@ class SimpleScheduler(chance.ChanceScheduler): request_spec, admin_password, injected_files, requested_networks, is_first_time, filter_properties) - def schedule_create_volume(self, context, volume_id, snapshot_id, + def schedule_create_volume(self, context, volume_id, snapshot_id, image_id, reservations): """Picks a host that is up and has the fewest volumes.""" deprecated.warn(_('nova-volume functionality is deprecated in Folsom ' @@ -76,7 +76,7 @@ class SimpleScheduler(chance.ChanceScheduler): raise exception.WillNotSchedule(host=host) driver.cast_to_volume_host(context, host, 'create_volume', volume_id=volume_id, snapshot_id=snapshot_id, - reservations=reservations) + image_id=image_id, reservations=reservations) return None results = db.service_get_all_volume_sorted(elevated) @@ -91,7 +91,8 @@ class SimpleScheduler(chance.ChanceScheduler): if utils.service_is_up(service) and not service['disabled']: driver.cast_to_volume_host(context, service['host'], 'create_volume', volume_id=volume_id, - snapshot_id=snapshot_id, reservations=reservations) + snapshot_id=snapshot_id, image_id=image_id, + reservations=reservations) return None msg = _("Is the appropriate service running?") raise exception.NoValidHost(reason=msg) diff --git a/nova/tests/policy.json b/nova/tests/policy.json index a6330c9e5..b856da58a 100644 --- a/nova/tests/policy.json +++ b/nova/tests/policy.json @@ -154,6 +154,7 @@ "volume_extension:volume_admin_actions:reset_status": [["rule:admin_api"]], "volume_extension:snapshot_admin_actions:reset_status": [["rule:admin_api"]], "volume_extension:volume_admin_actions:force_delete": [["rule:admin_api"]], + "volume_extension:volume_actions:upload_image": [], "volume_extension:types_manage": [], "volume_extension:types_extra_specs": [], diff --git a/nova/tests/scheduler/test_rpcapi.py b/nova/tests/scheduler/test_rpcapi.py index 1ff278a20..62147ce25 100644 --- a/nova/tests/scheduler/test_rpcapi.py +++ b/nova/tests/scheduler/test_rpcapi.py @@ -94,4 +94,5 @@ class SchedulerRpcAPITestCase(test.TestCase): def test_create_volume(self): self._test_scheduler_api('create_volume', rpc_method='cast', volume_id="fake_volume", - snapshot_id="fake_snapshots", reservations=list('fake_res')) + snapshot_id="fake_snapshots", image_id="fake_image", + reservations=list('fake_res')) diff --git a/nova/tests/test_volume.py b/nova/tests/test_volume.py index 6de3d380e..0c5328456 100644 --- a/nova/tests/test_volume.py +++ b/nova/tests/test_volume.py @@ -21,8 +21,10 @@ Tests for Volume Code. """ import cStringIO +import datetime import mox +import os import shutil import tempfile @@ -37,9 +39,13 @@ from nova.openstack.common import rpc import nova.policy from nova import quota from nova import test +from nova.tests.image import fake as fake_image +import nova.volume.api from nova.volume import iscsi QUOTAS = quota.QUOTAS + + FLAGS = flags.FLAGS @@ -60,11 +66,12 @@ class VolumeTestCase(test.TestCase): self.instance_id = instance['id'] self.instance_uuid = instance['uuid'] test_notifier.NOTIFICATIONS = [] + fake_image.stub_out_image_service(self.stubs) def tearDown(self): try: shutil.rmtree(FLAGS.volumes_dir) - except OSError, e: + except OSError: pass db.instance_destroy(self.context, self.instance_uuid) notifier_api._reset_drivers() @@ -74,11 +81,12 @@ class VolumeTestCase(test.TestCase): return 1 @staticmethod - def _create_volume(size=0, snapshot_id=None, metadata=None): + def _create_volume(size=0, snapshot_id=None, image_id=None, metadata=None): """Create a volume object.""" vol = {} vol['size'] = size vol['snapshot_id'] = snapshot_id + vol['image_id'] = image_id vol['user_id'] = 'fake' vol['project_id'] = 'fake' vol['availability_zone'] = FLAGS.storage_availability_zone @@ -137,7 +145,7 @@ class VolumeTestCase(test.TestCase): def test_create_delete_volume_with_metadata(self): """Test volume can be created and deleted.""" test_meta = {'fake_key': 'fake_value'} - volume = self._create_volume('0', None, test_meta) + volume = self._create_volume('0', None, metadata=test_meta) volume_id = volume['id'] self.volume.create_volume(self.context, volume_id) result_meta = { @@ -564,6 +572,231 @@ class VolumeTestCase(test.TestCase): volume = db.volume_get(self.context, volume['id']) self.assertEqual(volume['status'], "in-use") + def _create_volume_from_image(self, expected_status, + fakeout_copy_image_to_volume=False): + """Call copy image to volume, Test the status of volume after calling + copying image to volume.""" + def fake_local_path(volume): + return dst_path + + def fake_copy_image_to_volume(context, volume, image_id): + pass + + dst_fd, dst_path = tempfile.mkstemp() + os.close(dst_fd) + self.stubs.Set(self.volume.driver, 'local_path', fake_local_path) + if fakeout_copy_image_to_volume: + self.stubs.Set(self.volume, '_copy_image_to_volume', + fake_copy_image_to_volume) + + image_id = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77' + volume_id = 1 + # creating volume testdata + db.volume_create(self.context, {'id': volume_id, + 'updated_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'display_description': 'Test Desc', + 'size': 20, + 'status': 'creating', + 'instance_uuid': None, + 'host': 'dummy'}) + try: + self.volume.create_volume(self.context, + volume_id, + image_id=image_id) + + volume = db.volume_get(self.context, volume_id) + self.assertEqual(volume['status'], expected_status) + finally: + # cleanup + db.volume_destroy(self.context, volume_id) + os.unlink(dst_path) + + def test_create_volume_from_image_status_downloading(self): + """Verify that before copying image to volume, it is in downloading + state.""" + self._create_volume_from_image('downloading', True) + + def test_create_volume_from_image_status_available(self): + """Verify that before copying image to volume, it is in available + state.""" + self._create_volume_from_image('available') + + def test_create_volume_from_image_exception(self): + """Verify that create volume from image, the volume status is + 'downloading'.""" + dst_fd, dst_path = tempfile.mkstemp() + os.close(dst_fd) + + self.stubs.Set(self.volume.driver, 'local_path', lambda x: dst_path) + + image_id = 'aaaaaaaa-0000-0000-0000-000000000000' + # creating volume testdata + volume_id = 1 + db.volume_create(self.context, {'id': volume_id, + 'updated_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'display_description': 'Test Desc', + 'size': 20, + 'status': 'creating', + 'host': 'dummy'}) + + self.assertRaises(exception.ImageNotFound, + self.volume.create_volume, + self.context, + volume_id, + None, + image_id) + volume = db.volume_get(self.context, volume_id) + self.assertEqual(volume['status'], "error") + # cleanup + db.volume_destroy(self.context, volume_id) + os.unlink(dst_path) + + def test_copy_volume_to_image_status_available(self): + dst_fd, dst_path = tempfile.mkstemp() + os.close(dst_fd) + + def fake_local_path(volume): + return dst_path + + self.stubs.Set(self.volume.driver, 'local_path', fake_local_path) + + image_id = '70a599e0-31e7-49b7-b260-868f441e862b' + # creating volume testdata + volume_id = 1 + db.volume_create(self.context, {'id': volume_id, + 'updated_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'display_description': 'Test Desc', + 'size': 20, + 'status': 'uploading', + 'instance_uuid': None, + 'host': 'dummy'}) + + try: + # start test + self.volume.copy_volume_to_image(self.context, + volume_id, + image_id) + + volume = db.volume_get(self.context, volume_id) + self.assertEqual(volume['status'], 'available') + finally: + # cleanup + db.volume_destroy(self.context, volume_id) + os.unlink(dst_path) + + def test_copy_volume_to_image_status_use(self): + dst_fd, dst_path = tempfile.mkstemp() + os.close(dst_fd) + + def fake_local_path(volume): + return dst_path + + self.stubs.Set(self.volume.driver, 'local_path', fake_local_path) + + #image_id = '70a599e0-31e7-49b7-b260-868f441e862b' + image_id = 'a440c04b-79fa-479c-bed1-0b816eaec379' + # creating volume testdata + volume_id = 1 + db.volume_create(self.context, + {'id': volume_id, + 'updated_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'display_description': 'Test Desc', + 'size': 20, + 'status': 'uploading', + 'instance_uuid': + 'b21f957d-a72f-4b93-b5a5-45b1161abb02', + 'host': 'dummy'}) + + try: + # start test + self.volume.copy_volume_to_image(self.context, + volume_id, + image_id) + + volume = db.volume_get(self.context, volume_id) + self.assertEqual(volume['status'], 'in-use') + finally: + # cleanup + db.volume_destroy(self.context, volume_id) + os.unlink(dst_path) + + def test_copy_volume_to_image_exception(self): + dst_fd, dst_path = tempfile.mkstemp() + os.close(dst_fd) + + def fake_local_path(volume): + return dst_path + + self.stubs.Set(self.volume.driver, 'local_path', fake_local_path) + + image_id = 'aaaaaaaa-0000-0000-0000-000000000000' + # creating volume testdata + volume_id = 1 + db.volume_create(self.context, {'id': volume_id, + 'updated_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'display_description': 'Test Desc', + 'size': 20, + 'status': 'in-use', + 'host': 'dummy'}) + + try: + # start test + self.assertRaises(exception.ImageNotFound, + self.volume.copy_volume_to_image, + self.context, + volume_id, + image_id) + + volume = db.volume_get(self.context, volume_id) + self.assertEqual(volume['status'], 'available') + finally: + # cleanup + db.volume_destroy(self.context, volume_id) + os.unlink(dst_path) + + def test_create_volume_from_exact_sized_image(self): + """Verify that an image which is exactly the same size as the + volume, will work correctly.""" + class _FakeImageService: + def __init__(self, db_driver=None, image_service=None): + pass + + def show(self, context, image_id): + return {'size': 2 * 1024 * 1024 * 1024} + + image_id = '70a599e0-31e7-49b7-b260-868f441e862b' + + try: + volume_id = None + volume_api = nova.volume.api.API( + image_service=_FakeImageService()) + volume = volume_api.create(self.context, 2, 'name', 'description', + image_id=1) + volume_id = volume['id'] + self.assertEqual(volume['status'], 'creating') + + finally: + # cleanup + db.volume_destroy(self.context, volume_id) + + def test_create_volume_from_oversized_image(self): + """Verify that an image which is too big will fail correctly.""" + class _FakeImageService: + def __init__(self, db_driver=None, image_service=None): + pass + + def show(self, context, image_id): + return {'size': 2 * 1024 * 1024 * 1024 + 1} + + image_id = '70a599e0-31e7-49b7-b260-868f441e862b' + + volume_api = nova.volume.api.API(image_service=_FakeImageService()) + + self.assertRaises(exception.InvalidInput, + volume_api.create, + self.context, 2, + 'name', 'description', image_id=1) + class DriverTestCase(test.TestCase): """Base Test class for Drivers.""" @@ -591,7 +824,7 @@ class DriverTestCase(test.TestCase): def tearDown(self): try: shutil.rmtree(FLAGS.volumes_dir) - except OSError, e: + except OSError: pass super(DriverTestCase, self).tearDown() diff --git a/nova/utils.py b/nova/utils.py index 41b268c2e..f2642f07c 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -1135,6 +1135,18 @@ def read_cached_file(filename, cache_info, reload_func=None): return cache_info['data'] +def file_open(*args, **kwargs): + """Open file + + see built-in file() documentation for more details + + Note: The reason this is kept in a separate module is to easily + be able to provide a stub module that doesn't alter system + state at all (for unit tests) + """ + return file(*args, **kwargs) + + def hash_file(file_like_object): """Generate a hash for the contents of a file.""" checksum = hashlib.sha1()