From 1dd3b9de02a58cd678b69b91e5317942845c9582 Mon Sep 17 00:00:00 2001 From: Erik Olof Gunnar Andersson Date: Mon, 9 Sep 2019 13:01:27 -0700 Subject: Cleanup scheduler This patch is not changing any functionality, but instead it is aimed at cleaning up the scheduler code. It also removes the use of reserved keywords in the scheduler code. Change-Id: I93cede3371f1ec650adf3b00bf8250457a38f96c (cherry picked from commit 44383504519d94730fe712e7d8e300bbc5e21892) --- designate/scheduler/base.py | 52 +++++++++++++------------ designate/scheduler/filters/attribute_filter.py | 20 ++++------ designate/scheduler/filters/fallback_filter.py | 9 ++--- designate/scheduler/filters/random_filter.py | 14 +++---- designate/tests/unit/scheduler/test_basic.py | 10 ++--- designate/tests/unit/scheduler/test_filters.py | 4 +- 6 files changed, 54 insertions(+), 55 deletions(-) diff --git a/designate/scheduler/base.py b/designate/scheduler/base.py index b288548d..934f646a 100644 --- a/designate/scheduler/base.py +++ b/designate/scheduler/base.py @@ -27,28 +27,29 @@ class Scheduler(object): :raises: NoFiltersConfigured """ - filters = [] """The list of filters enabled on this scheduler""" def __init__(self, storage): - enabled_filters = cfg.CONF['service:central'].scheduler_filters - # Get a storage connection + self.filters = list() self.storage = storage - if len(enabled_filters) > 0: - filters = named.NamedExtensionManager( - namespace='designate.scheduler.filters', - names=enabled_filters, - name_order=True) - self.filters = [x.plugin(storage=self.storage) for x in filters] - for filter in self.filters: - LOG.info("Loaded Scheduler Filter: %s", filter.name) + if not enabled_filters: + raise exceptions.NoFiltersConfigured( + 'There are no scheduling filters configured' + ) - else: - raise exceptions.NoFiltersConfigured('There are no scheduling ' - 'filters configured') + extensions = named.NamedExtensionManager( + namespace='designate.scheduler.filters', + names=enabled_filters, + name_order=True, + ) + + for extension in extensions: + plugin = extension.plugin(storage=self.storage) + LOG.info('Loaded Scheduler Filter: %s', plugin.name) + self.filters.append(plugin) def schedule_zone(self, context, zone): """Get a pool to create the new zone in. @@ -61,21 +62,24 @@ class Scheduler(object): """ pools = self.storage.find_pools(context) - if len(self.filters) == 0: + if not self.filters: raise exceptions.NoFiltersConfigured('There are no scheduling ' 'filters configured') - for f in self.filters: - LOG.debug("Running %s filter with %d pools", f.name, len(pools)) - pools = f.filter(context, pools, zone) + for plugin in self.filters: + LOG.debug( + 'Running %s filter with %d pools', plugin.name, len(pools) + ) + pools = plugin.filter(context, pools, zone) LOG.debug( - "%d candidate pools remaining after %s filter", - len(pools), - f.name) + '%d candidate pools remaining after %s filter', + len(pools), plugin.name + ) if len(pools) > 1: raise exceptions.MultiplePoolsFound() - if len(pools) == 0: - raise exceptions.NoValidPoolFound('There are no pools that ' - 'matched your request') + if not pools: + raise exceptions.NoValidPoolFound( + 'There are no pools that matched your request' + ) return pools[0].id diff --git a/designate/scheduler/filters/attribute_filter.py b/designate/scheduler/filters/attribute_filter.py index 8751fc24..a53333e6 100644 --- a/designate/scheduler/filters/attribute_filter.py +++ b/designate/scheduler/filters/attribute_filter.py @@ -74,7 +74,7 @@ class AttributeFilter(base.Filter): # fine, we should just continue. pool_attributes.pop('pool_id', None) - if pool_attributes == {}: + if not pool_attributes: # If we did not send any attribute to filter on, we should # not filter the pools based on an empty set, as this will # return no pools. @@ -83,12 +83,10 @@ class AttributeFilter(base.Filter): # Check if the keys requested exist in this pool if not {key for key in six.iterkeys(pool_attributes)}.issuperset( zone_attributes): - msg = "%(pool)s did not match list of requested attribute "\ - "keys - removing from list. Requested: %(r_key)s. Pool:"\ - "%(p_key)s" - LOG.debug( - msg, + '%(pool)s did not match list of requested attribute ' + 'keys - removing from list. Requested: %(r_key)s. Pool:' + '%(p_key)s', { 'pool': pool, 'r_key': zone_attributes, @@ -98,8 +96,8 @@ class AttributeFilter(base.Filter): # Missing required keys - remove from the list return False - for key in six.iterkeys(zone_attributes): - LOG.debug("Checking value of %s for %s", key, pool) + for key in zone_attributes.keys(): + LOG.debug('Checking value of %s for %s', key, pool) pool_attr = bool_from_string(pool_attributes[key], default=pool_attributes[key]) @@ -108,8 +106,8 @@ class AttributeFilter(base.Filter): if not pool_attr == zone_attr: LOG.debug( - "%(pool)s did not match requested value of %(key)s. " - "Requested: %(r_val)s. Pool: %(p_val)s", + '%(pool)s did not match requested value of %(key)s. ' + 'Requested: %(r_val)s. Pool: %(p_val)s', { 'pool': pool, 'key': key, @@ -123,7 +121,5 @@ class AttributeFilter(base.Filter): return True pool_list = [pool for pool in pools if evaluate_pool(pool)] - pools = PoolList(objects=pool_list) - return pools diff --git a/designate/scheduler/filters/fallback_filter.py b/designate/scheduler/filters/fallback_filter.py index cbb31ac3..bacb0d33 100644 --- a/designate/scheduler/filters/fallback_filter.py +++ b/designate/scheduler/filters/fallback_filter.py @@ -40,10 +40,9 @@ class FallbackFilter(base.Filter): """ def filter(self, context, pools, zone): - if len(pools) == 0: + if not pools: pools = objects.PoolList() pools.append( - objects.Pool(id=cfg.CONF['service:central'].default_pool_id)) - return pools - else: - return pools + objects.Pool(id=cfg.CONF['service:central'].default_pool_id) + ) + return pools diff --git a/designate/scheduler/filters/random_filter.py b/designate/scheduler/filters/random_filter.py index 578695db..494a1fad 100644 --- a/designate/scheduler/filters/random_filter.py +++ b/designate/scheduler/filters/random_filter.py @@ -22,7 +22,8 @@ LOG = logging.getLogger(__name__) class RandomFilter(base.Filter): - """Randomly chooses one of the input pools if there is multiple supplied. + """Randomly chooses one of the input pools if there are multiple + ones supplied. .. note:: @@ -35,9 +36,8 @@ class RandomFilter(base.Filter): """ def filter(self, context, pools, zone): - new_list = PoolList() - if len(pools): - new_list.append(random.choice(pools)) - return new_list - else: - return pools + if pools: + new_pool_list = PoolList() + new_pool_list.append(random.choice(pools)) + return new_pool_list + return pools diff --git a/designate/tests/unit/scheduler/test_basic.py b/designate/tests/unit/scheduler/test_basic.py index 49531e81..156a29ad 100644 --- a/designate/tests/unit/scheduler/test_basic.py +++ b/designate/tests/unit/scheduler/test_basic.py @@ -11,7 +11,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -from unittest.mock import Mock +from unittest import mock from designate import exceptions from designate import objects @@ -35,7 +35,7 @@ class SchedulerTest(tests.TestCase): 'find_pools.return_value': objects.PoolList.from_list( [{"id": "794ccc2c-d751-44fe-b57f-8894c9f5c842"}]) } - mock_storage = Mock(**attrs) + mock_storage = mock.Mock(**attrs) test_scheduler = scheduler.get_scheduler(storage=mock_storage) @@ -55,7 +55,7 @@ class SchedulerTest(tests.TestCase): ) } - mock_storage = Mock(**attrs) + mock_storage = mock.Mock(**attrs) test_scheduler = scheduler.get_scheduler(storage=mock_storage) @@ -74,7 +74,7 @@ class SchedulerTest(tests.TestCase): attrs = { 'find_pools.return_value': objects.PoolList() } - mock_storage = Mock(**attrs) + mock_storage = mock.Mock(**attrs) self.CONF.set_override( 'scheduler_filters', ['random'], 'service:central' @@ -96,7 +96,7 @@ class SchedulerTest(tests.TestCase): attrs = { 'find_pools.return_value': objects.PoolList() } - mock_storage = Mock(**attrs) + mock_storage = mock.Mock(**attrs) self.assertRaisesRegex( exceptions.NoFiltersConfigured, diff --git a/designate/tests/unit/scheduler/test_filters.py b/designate/tests/unit/scheduler/test_filters.py index da718914..78570e80 100644 --- a/designate/tests/unit/scheduler/test_filters.py +++ b/designate/tests/unit/scheduler/test_filters.py @@ -11,7 +11,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -from unittest.mock import Mock +from unittest import mock import fixtures @@ -41,7 +41,7 @@ class SchedulerFilterTest(tests.TestCase): id="6c346011-e581-429b-a7a2-6cdf0aba91c3") } - mock_storage = Mock(**attrs) + mock_storage = mock.Mock(**attrs) self.test_filter = self.FILTER(storage=mock_storage) -- cgit v1.2.1