From: Joshua Harlow Date: Mon, 9 Jun 2014 22:32:17 +0000 (-0700) Subject: Handle the case where az is disabled/removed X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=415a4ba29b12e51b646c79e19bb0a63c80c1157f;p=openstack-build%2Fcinder-build.git Handle the case where az is disabled/removed Instead of retaining a local in-memory list of availability zones that is never purged and therefore never updates itself if availability zones are removed we should instead cache those zones for a given period of time (defaulting to one hour) and on cache expiry refetch the enabled availability zones. DocImpact: adds a new configuration that defines the default availability zone cache expiry time in seconds. This will enable the engine that runs the create volume request to in a future change remove the need to have a functor which proxies the validation logic around availability zones. Change-Id: I8e45845b66ba62313248ee9cbeb301a5fe54a08b --- diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index c4a860a89..e1e082f92 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -43,6 +43,7 @@ from cinder import keymgr from cinder.openstack.common import fileutils from cinder.openstack.common import importutils from cinder.openstack.common import jsonutils +from cinder.openstack.common import timeutils import cinder.policy from cinder import quota from cinder import test @@ -136,6 +137,89 @@ class BaseVolumeTestCase(test.TestCase): 'uuid': 'vR1JU3-FAKE-C4A9-PQFh-Mctm-9FwA-Xwzc1m'}] +class AvailabilityZoneTestCase(BaseVolumeTestCase): + def test_list_availability_zones_cached(self): + volume_api = cinder.volume.api.API() + with mock.patch.object(volume_api.db, + 'service_get_all_by_topic') as get_all: + get_all.return_value = [ + { + 'availability_zone': 'a', + 'disabled': False, + }, + ] + azs = volume_api.list_availability_zones(enable_cache=True) + self.assertEqual([{"name": 'a', 'available': True}], list(azs)) + self.assertIsNotNone(volume_api.availability_zones_last_fetched) + self.assertTrue(get_all.called) + volume_api.list_availability_zones(enable_cache=True) + self.assertEqual(1, get_all.call_count) + + def test_list_availability_zones_no_cached(self): + volume_api = cinder.volume.api.API() + with mock.patch.object(volume_api.db, + 'service_get_all_by_topic') as get_all: + get_all.return_value = [ + { + 'availability_zone': 'a', + 'disabled': False, + }, + ] + azs = volume_api.list_availability_zones(enable_cache=False) + self.assertEqual([{"name": 'a', 'available': True}], list(azs)) + self.assertIsNone(volume_api.availability_zones_last_fetched) + + with mock.patch.object(volume_api.db, + 'service_get_all_by_topic') as get_all: + get_all.return_value = [ + { + 'availability_zone': 'a', + 'disabled': True, + }, + ] + azs = volume_api.list_availability_zones(enable_cache=False) + self.assertEqual([{"name": 'a', 'available': False}], list(azs)) + self.assertIsNone(volume_api.availability_zones_last_fetched) + + def test_list_availability_zones_refetched(self): + timeutils.set_time_override() + volume_api = cinder.volume.api.API() + with mock.patch.object(volume_api.db, + 'service_get_all_by_topic') as get_all: + get_all.return_value = [ + { + 'availability_zone': 'a', + 'disabled': False, + }, + ] + azs = volume_api.list_availability_zones(enable_cache=True) + self.assertEqual([{"name": 'a', 'available': True}], list(azs)) + self.assertIsNotNone(volume_api.availability_zones_last_fetched) + last_fetched = volume_api.availability_zones_last_fetched + self.assertTrue(get_all.called) + volume_api.list_availability_zones(enable_cache=True) + self.assertEqual(1, get_all.call_count) + + # The default cache time is 3600, push past that... + timeutils.advance_time_seconds(3800) + get_all.return_value = [ + { + 'availability_zone': 'a', + 'disabled': False, + }, + { + 'availability_zone': 'b', + 'disabled': False, + }, + ] + azs = volume_api.list_availability_zones(enable_cache=True) + azs = sorted([n['name'] for n in azs]) + self.assertEqual(['a', 'b'], azs) + self.assertEqual(2, get_all.call_count) + self.assertGreater(volume_api.availability_zones_last_fetched, + last_fetched) + + class VolumeTestCase(BaseVolumeTestCase): def setUp(self): @@ -332,7 +416,7 @@ class VolumeTestCase(BaseVolumeTestCase): """Test setting availability_zone correctly during volume create.""" volume_api = cinder.volume.api.API() - def fake_list_availability_zones(): + def fake_list_availability_zones(enable_cache=False): return ({'name': 'az1', 'available': True}, {'name': 'az2', 'available': True}, {'name': 'default-az', 'available': True}) @@ -1041,7 +1125,7 @@ class VolumeTestCase(BaseVolumeTestCase): """Test volume can't be created from snapshot in a different az.""" volume_api = cinder.volume.api.API() - def fake_list_availability_zones(): + def fake_list_availability_zones(enable_cache=False): return ({'name': 'nova', 'available': True}, {'name': 'az2', 'available': True}) @@ -2282,7 +2366,7 @@ class VolumeTestCase(BaseVolumeTestCase): """Test volume can't be cloned from an other volume in different az.""" volume_api = cinder.volume.api.API() - def fake_list_availability_zones(): + def fake_list_availability_zones(enable_cache=False): return ({'name': 'nova', 'available': True}, {'name': 'az2', 'available': True}) diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 8e4201ccb..9cccc6a01 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -20,6 +20,7 @@ Handles all requests relating to volumes. import collections +import datetime import functools from oslo.config import cfg @@ -53,10 +54,16 @@ volume_same_az_opt = cfg.BoolOpt('cloned_volume_same_az', default=True, help='Ensure that the new volumes are the ' 'same AZ as snapshot or source volume') +az_cache_time_opt = cfg.IntOpt('az_cache_duration', + default=3600, + help='Cache volume availability zones in ' + 'memory for the provided duration in ' + 'seconds') CONF = cfg.CONF CONF.register_opt(volume_host_opt) CONF.register_opt(volume_same_az_opt) +CONF.register_opt(az_cache_time_opt) CONF.import_opt('glance_core_properties', 'cinder.image.glance') CONF.import_opt('storage_availability_zone', 'cinder.volume.manager') @@ -97,40 +104,54 @@ class API(base.Base): glance.get_default_image_service()) self.scheduler_rpcapi = scheduler_rpcapi.SchedulerAPI() self.volume_rpcapi = volume_rpcapi.VolumeAPI() - self.availability_zone_names = () + self.availability_zones = [] + self.availability_zones_last_fetched = None self.key_manager = keymgr.API() super(API, self).__init__(db_driver) def _valid_availability_zone(self, availability_zone): - #NOTE(bcwaldon): This approach to caching fails to handle the case - # that an availability zone is disabled/removed. - if availability_zone in self.availability_zone_names: - return True - if CONF.storage_availability_zone == availability_zone: - return True - - azs = self.list_availability_zones() - self.availability_zone_names = [az['name'] for az in azs] - return availability_zone in self.availability_zone_names - - def list_availability_zones(self): + azs = self.list_availability_zones(enable_cache=True) + names = set([az['name'] for az in azs]) + if CONF.storage_availability_zone: + names.add(CONF.storage_availability_zone) + return availability_zone in names + + def list_availability_zones(self, enable_cache=False): """Describe the known availability zones :retval list of dicts, each with a 'name' and 'available' key """ - topic = CONF.volume_topic - ctxt = context.get_admin_context() - services = self.db.service_get_all_by_topic(ctxt, topic) - az_data = [(s['availability_zone'], s['disabled']) for s in services] - - disabled_map = {} - for (az_name, disabled) in az_data: - tracked_disabled = disabled_map.get(az_name, True) - disabled_map[az_name] = tracked_disabled and disabled - - azs = [{'name': name, 'available': not disabled} - for (name, disabled) in disabled_map.items()] - + refresh_cache = False + if enable_cache: + if self.availability_zones_last_fetched is None: + refresh_cache = True + else: + cache_age = timeutils.delta_seconds( + self.availability_zones_last_fetched, + timeutils.utcnow()) + if cache_age >= CONF.az_cache_duration: + refresh_cache = True + if refresh_cache or not enable_cache: + topic = CONF.volume_topic + ctxt = context.get_admin_context() + services = self.db.service_get_all_by_topic(ctxt, topic) + az_data = [(s['availability_zone'], s['disabled']) + for s in services] + disabled_map = {} + for (az_name, disabled) in az_data: + tracked_disabled = disabled_map.get(az_name, True) + disabled_map[az_name] = tracked_disabled and disabled + azs = [{'name': name, 'available': not disabled} + for (name, disabled) in disabled_map.items()] + if refresh_cache: + now = timeutils.utcnow() + self.availability_zones = azs + self.availability_zones_last_fetched = now + LOG.debug("Availability zone cache updated, next update will" + " occur around %s", now + datetime.timedelta( + seconds=CONF.az_cache_duration)) + else: + azs = self.availability_zones return tuple(azs) def create(self, context, size, name, description, snapshot=None, diff --git a/etc/cinder/cinder.conf.sample b/etc/cinder/cinder.conf.sample index 545df556a..d7678dbfb 100644 --- a/etc/cinder/cinder.conf.sample +++ b/etc/cinder/cinder.conf.sample @@ -918,6 +918,10 @@ # Options defined in cinder.volume.api # +# Cache volume availability zones in memory for the provided +# duration in seconds (integer value) +#az_cache_duration=3600 + # Create volume from snapshot at the host where snapshot # resides (boolean value) #snapshot_same_host=true