From d5a3fdabca25a63bd3d01c86442ef649e7613aff Mon Sep 17 00:00:00 2001 From: TommyLike Date: Tue, 25 Apr 2017 11:30:23 +0800 Subject: [PATCH] Add volume type filter to API Get-Pools Add feature that administrators can get back-end storage pools filtered by volume-type, Cinder will return the pools filtered by volume-type's extra-spec. This is depended on generalized resource filtering feature. APIImpact Depends-On: ff3d41b15abb2915de87830980147be51e5da971 Implements: blueprint add-volume-type-filter-to-get-pool Change-Id: If2ae4616340d061db833cbbdffc77f3e976d8254 --- cinder/api/common.py | 5 ++- cinder/api/contrib/scheduler_stats.py | 4 ++ cinder/api/openstack/api_version_request.py | 3 +- .../openstack/rest_api_version_history.rst | 4 ++ cinder/scheduler/host_manager.py | 38 ++++++++++++++-- .../unit/api/contrib/test_scheduler_stats.py | 33 ++++++++++++++ cinder/tests/unit/api/test_common.py | 22 ++++++--- .../tests/unit/scheduler/test_host_manager.py | 45 +++++++++++++++++++ doc/source/man/generalized_filters.rst | 2 +- etc/cinder/resource_filters.json | 2 +- ...-filter-to_get-pools-c791132540921398.yaml | 3 ++ 11 files changed, 146 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/add-volume-type-filter-to_get-pools-c791132540921398.yaml diff --git a/cinder/api/common.py b/cinder/api/common.py index 93d7fa705b7..ac7991c8b97 100644 --- a/cinder/api/common.py +++ b/cinder/api/common.py @@ -449,8 +449,9 @@ def get_enabled_resource_filters(resource=None): def reject_invalid_filters(context, filters, resource, enable_like_filter=False): - if context.is_admin: - # Allow all options + if context.is_admin and resource not in ['pool']: + # Allow all options except resource is pool + # pool API is only available for admin return # Check the configured filters against those passed in resource configured_filters = get_enabled_resource_filters(resource) diff --git a/cinder/api/contrib/scheduler_stats.py b/cinder/api/contrib/scheduler_stats.py index c864538477b..875d42ca54e 100644 --- a/cinder/api/contrib/scheduler_stats.py +++ b/cinder/api/contrib/scheduler_stats.py @@ -22,6 +22,7 @@ from cinder.scheduler import rpcapi from cinder import utils GET_POOL_NAME_FILTER_MICRO_VERSION = '3.28' +GET_POOL_VOLUME_TYPE_FILTER_MICRO_VERSION = '3.35' def authorize(context, action_name): @@ -59,6 +60,9 @@ class SchedulerStatsController(wsgi.Controller): filters=filters, req_version=req_version) + if not req_version.matches(GET_POOL_VOLUME_TYPE_FILTER_MICRO_VERSION): + filters.pop('volume_type', None) + pools = self.scheduler_api.get_pools(context, filters=filters) return self._view_builder.pools(req, pools, detail) diff --git a/cinder/api/openstack/api_version_request.py b/cinder/api/openstack/api_version_request.py index 1545e3f6423..5d926051914 100644 --- a/cinder/api/openstack/api_version_request.py +++ b/cinder/api/openstack/api_version_request.py @@ -89,6 +89,7 @@ REST_API_VERSION_HISTORY = """ * 3.34 - Add like filter support in ``volume``, ``backup``, ``snapshot``, ``message``, ``attachment``, ``group`` and ``group-snapshot`` list APIs. + * 3.35 - Add ``volume-type`` filter to Get-Pools API. """ @@ -97,7 +98,7 @@ REST_API_VERSION_HISTORY = """ # minimum version of the API supported. # Explicitly using /v1 or /v2 endpoints will still work _MIN_API_VERSION = "3.0" -_MAX_API_VERSION = "3.34" +_MAX_API_VERSION = "3.35" _LEGACY_API_VERSION1 = "1.0" _LEGACY_API_VERSION2 = "2.0" diff --git a/cinder/api/openstack/rest_api_version_history.rst b/cinder/api/openstack/rest_api_version_history.rst index b47a14546ad..a541c876b87 100644 --- a/cinder/api/openstack/rest_api_version_history.rst +++ b/cinder/api/openstack/rest_api_version_history.rst @@ -313,3 +313,7 @@ user documentation. ---- Add like filter support in ``volume``, ``backup``, ``snapshot``, ``message``, ``attachment``, ``group`` and ``group-snapshot`` list APIs. + +3.35 +---- + Add ``volume-type`` filter to Get-Pools API. diff --git a/cinder/scheduler/host_manager.py b/cinder/scheduler/host_manager.py index 4fbd7465a78..641691676b1 100644 --- a/cinder/scheduler/host_manager.py +++ b/cinder/scheduler/host_manager.py @@ -32,6 +32,7 @@ from cinder import objects from cinder.scheduler import filters from cinder import utils from cinder.volume import utils as vol_utils +from cinder.volume import volume_types # FIXME: This file should be renamed to backend_manager, we should also rename @@ -627,15 +628,33 @@ class HostManager(object): return all_pools.values() + def _filter_pools_by_volume_type(self, context, volume_type, pools): + """Return the pools filtered by volume type specs""" + + # wrap filter properties only with volume_type + filter_properties = { + 'context': context, + 'volume_type': volume_type, + 'resource_type': volume_type, + 'qos_specs': volume_type.get('qos_specs'), + } + + filtered = self.get_filtered_backends(pools.values(), + filter_properties) + + # filter the pools by value + return {k: v for k, v in pools.items() if v in filtered} + def get_pools(self, context, filters=None): """Returns a dict of all pools on all hosts HostManager knows about.""" self._update_backend_state_map(context) - all_pools = [] - name = None + all_pools = {} + name = volume_type = None if filters: name = filters.pop('name', None) + volume_type = filters.pop('volume_type', None) for backend_key, state in self.backend_state_map.items(): for key in state.pools: @@ -658,9 +677,20 @@ class HostManager(object): break if not filtered: - all_pools.append(new_pool) + all_pools[pool_key] = pool - return all_pools + # filter pools by volume type + if volume_type: + volume_type = volume_types.get_by_name_or_id( + context, volume_type) + all_pools = ( + self._filter_pools_by_volume_type(context, + volume_type, + all_pools)) + + # encapsulate pools in format:{name: XXX, capabilities: XXX} + return [dict(name=key, capabilities=value.capabilities) + for key, value in all_pools.items()] def get_usage_and_notify(self, capa_new, updated_pools, host, timestamp): context = cinder_context.get_admin_context() diff --git a/cinder/tests/unit/api/contrib/test_scheduler_stats.py b/cinder/tests/unit/api/contrib/test_scheduler_stats.py index 54a8520f165..3e241e60e90 100644 --- a/cinder/tests/unit/api/contrib/test_scheduler_stats.py +++ b/cinder/tests/unit/api/contrib/test_scheduler_stats.py @@ -14,6 +14,7 @@ # License for the specific language governing permissions and limitations # under the License. +import ddt import mock from cinder.api.contrib import scheduler_stats @@ -45,6 +46,7 @@ def schedule_rpcapi_get_pools(self, context, filters=None): return all_pools +@ddt.data class SchedulerStatsAPITest(test.TestCase): def setUp(self): super(SchedulerStatsAPITest, self).setUp() @@ -171,3 +173,34 @@ class SchedulerStatsAPITest(test.TestCase): self.assertRaises(exception.InvalidParameterValue, self.controller.get_pools, req) + + @ddt.data(('3.34', False), + ('3.35', True)) + @ddt.unpack + @mock.patch('cinder.scheduler.rpcapi.SchedulerAPI.get_pools') + @mock.patch('cinder.api.common.update_general_filters') + def test_get_pools_by_volume_type(self, + version, + support_volume_type, + mock_update_filter, + mock_get_pools + ): + req = fakes.HTTPRequest.blank('/v3/%s/scheduler_stats?' + 'volume_type=lvm' % fake.PROJECT_ID) + mock_get_pools.return_value = [{'name': 'pool1', + 'capabilities': {'foo': 'bar'}}] + req.api_version_request = api_version.APIVersionRequest(version) + req.environ['cinder.context'] = self.ctxt + res = self.controller.get_pools(req) + + expected = { + 'pools': [{'name': 'pool1'}] + } + + filters = None + if support_volume_type: + filters = {'volume_type': 'lvm'} + mock_update_filter.assert_called_once_with(self.ctxt, filters, + 'pool') + self.assertDictEqual(expected, res) + mock_get_pools.assert_called_with(mock.ANY, filters=filters) diff --git a/cinder/tests/unit/api/test_common.py b/cinder/tests/unit/api/test_common.py index cbc5dc60b23..ee40a6049b0 100644 --- a/cinder/tests/unit/api/test_common.py +++ b/cinder/tests/unit/api/test_common.py @@ -379,11 +379,13 @@ class GeneralFiltersTest(test.TestCase): @ddt.data({'filters': {'key1': 'value1'}, 'is_admin': False, 'result': {'fake_resource': ['key1']}, - 'expected': {'key1': 'value1'}}, + 'expected': {'key1': 'value1'}, + 'resource': 'fake_resource'}, {'filters': {'key1': 'value1', 'key2': 'value2'}, 'is_admin': False, 'result': {'fake_resource': ['key1']}, - 'expected': None}, + 'expected': None, + 'resource': 'fake_resource'}, {'filters': {'key1': 'value1', 'all_tenants': 'value2', 'key3': 'value3'}, @@ -391,11 +393,19 @@ class GeneralFiltersTest(test.TestCase): 'result': {'fake_resource': []}, 'expected': {'key1': 'value1', 'all_tenants': 'value2', - 'key3': 'value3'}}) + 'key3': 'value3'}, + 'resource': 'fake_resource'}, + {'filters': {'key1': 'value1', + 'all_tenants': 'value2', + 'key3': 'value3'}, + 'is_admin': True, + 'result': {'pool': []}, + 'expected': None, + 'resource': 'pool'}) @ddt.unpack @mock.patch('cinder.api.common.get_enabled_resource_filters') def test_reject_invalid_filters(self, mock_get, filters, - is_admin, result, expected): + is_admin, result, expected, resource): class FakeContext(object): def __init__(self, admin): self.is_admin = admin @@ -404,13 +414,13 @@ class GeneralFiltersTest(test.TestCase): mock_get.return_value = result if expected: common.reject_invalid_filters(fake_context, - filters, 'fake_resource') + filters, resource) self.assertEqual(expected, filters) else: self.assertRaises( webob.exc.HTTPBadRequest, common.reject_invalid_filters, fake_context, - filters, 'fake_resource') + filters, resource) @ddt.ddt diff --git a/cinder/tests/unit/scheduler/test_host_manager.py b/cinder/tests/unit/scheduler/test_host_manager.py index 593a5f9031d..81b5db813b3 100644 --- a/cinder/tests/unit/scheduler/test_host_manager.py +++ b/cinder/tests/unit/scheduler/test_host_manager.py @@ -46,6 +46,12 @@ class FakeFilterClass2(filters.BaseBackendFilter): pass +class FakeFilterClass3(filters.BaseHostFilter): + def host_passes(self, host_state, filter_properties): + return host_state.get('volume_backend_name') == \ + filter_properties.get('volume_type')['volume_backend_name'] + + @ddt.ddt class HostManagerTestCase(test.TestCase): """Test case for HostManager class.""" @@ -1020,6 +1026,45 @@ class HostManagerTestCase(test.TestCase): self.assertEqual(expected, res) + @mock.patch('cinder.scheduler.host_manager.HostManager.' + '_choose_backend_filters') + def test_get_pools_filtered_by_volume_type(self, + _mock_choose_backend_filters): + context = 'fake_context' + filter_class = FakeFilterClass3 + _mock_choose_backend_filters.return_value = [filter_class] + + hosts = { + 'host1': {'volume_backend_name': 'AAA', + 'total_capacity_gb': 512, + 'free_capacity_gb': 200, + 'timestamp': None, + 'reserved_percentage': 0, + 'provisioned_capacity_gb': 312}, + 'host2@back1': {'volume_backend_name': 'BBB', + 'total_capacity_gb': 256, + 'free_capacity_gb': 100, + 'timestamp': None, + 'reserved_percentage': 0, + 'provisioned_capacity_gb': 156}} + mock_warning = mock.Mock() + host_manager.LOG.warn = mock_warning + mock_volume_type = { + 'volume_backend_name': 'AAA', + 'qos_specs': 'BBB', + } + + res = self.host_manager._filter_pools_by_volume_type(context, + mock_volume_type, + hosts) + expected = {'host1': {'volume_backend_name': 'AAA', + 'total_capacity_gb': 512, + 'free_capacity_gb': 200, + 'timestamp': None, 'reserved_percentage': 0, + 'provisioned_capacity_gb': 312}} + + self.assertEqual(expected, res) + @mock.patch('cinder.db.service_get_all') @mock.patch('cinder.objects.service.Service.is_up', new_callable=mock.PropertyMock) diff --git a/doc/source/man/generalized_filters.rst b/doc/source/man/generalized_filters.rst index b3db1a6ce9a..f9454f7d240 100644 --- a/doc/source/man/generalized_filters.rst +++ b/doc/source/man/generalized_filters.rst @@ -74,5 +74,5 @@ valid for first. The supported APIs are marked with "*" below in the table. | | id, event_id, resource_uuid, resource_type, request_id, message_level, | | list message* | project_id | +-----------------+-------------------------------------------------------------------------+ -| get pools | name | +| get pools | name, volume_type | +-----------------+-------------------------------------------------------------------------+ diff --git a/etc/cinder/resource_filters.json b/etc/cinder/resource_filters.json index 68c428ad1f0..08a6bdd66c5 100644 --- a/etc/cinder/resource_filters.json +++ b/etc/cinder/resource_filters.json @@ -8,5 +8,5 @@ "attachment": ["volume_id"], "message": ["resource_uuid", "resource_type", "event_id", "request_id", "message_level"], - "pool": ["name"] + "pool": ["name", "volume_type"] } diff --git a/releasenotes/notes/add-volume-type-filter-to_get-pools-c791132540921398.yaml b/releasenotes/notes/add-volume-type-filter-to_get-pools-c791132540921398.yaml new file mode 100644 index 00000000000..8b23c44f8c8 --- /dev/null +++ b/releasenotes/notes/add-volume-type-filter-to_get-pools-c791132540921398.yaml @@ -0,0 +1,3 @@ +--- +features: + - Add ``volume-type`` filter to API Get-Pools \ No newline at end of file