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 c4a860a89bb62ad159dd182648c84480207c8367..e1e082f92c9a316e4caa4b99695104212ce33fdf 100644 (file)
--- 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 8e4201ccbae9a43dd809038549401f9da549ba7f..9cccc6a01528fa26dba2bda5067b05abd9c6165a 100644 (file)
--- 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 545df556ae0590f8ebac21ca7d70c968469a7d8c..d7678dbfb43b5cff8ad489bf2d6f0465d94d2533 100644 (file)
--- 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