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
This commit is contained in:
Joshua Harlow
parent
2355c79350
commit
415a4ba29b
@ -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})
|
||||
|
||||
|
||||
@ -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(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
|
||||
|
||||
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):
|
||||
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,
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user