From f5cdbe8f74e64ce912e875141c2e092655988344 Mon Sep 17 00:00:00 2001 From: Vivek Agrawal Date: Tue, 27 Sep 2016 17:36:41 -0700 Subject: [PATCH] Adds metadata in search option for snapshot With this change one should be able to search snapshots based on snapshot metadata keys too. Adding test cases for querying snapshot based on metadata. Adding support to API V3. Adding a new microversion for this. DocImpact APIImpact Closes-Bug: #1569554 Change-Id: I7f3a8b9eea69e4320ac7c394910278807a0ce100 --- cinder/api/openstack/api_version_request.py | 3 +- .../openstack/rest_api_version_history.rst | 5 + cinder/api/v3/snapshots.py | 76 ++++++++++++ cinder/db/sqlalchemy/api.py | 51 +++++++- cinder/tests/unit/api/v3/test_snapshots.py | 115 ++++++++++++++++++ ...pshot-list-filtering-6e6df68a7ce981f5.yaml | 5 + 6 files changed, 249 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/support-metadata-based-snapshot-list-filtering-6e6df68a7ce981f5.yaml diff --git a/cinder/api/openstack/api_version_request.py b/cinder/api/openstack/api_version_request.py index b81c6aac4..c14e72d0e 100644 --- a/cinder/api/openstack/api_version_request.py +++ b/cinder/api/openstack/api_version_request.py @@ -71,6 +71,7 @@ REST_API_VERSION_HISTORY = """ * 3.20 - Add API reset status actions 'reset_status' to generic volume group. * 3.21 - Show provider_id in detailed view of a volume for admin. + * 3.22 - Add filtering based on metadata for snapshot listing. """ # The minimum and maximum versions of the API supported @@ -78,7 +79,7 @@ REST_API_VERSION_HISTORY = """ # minimum version of the API supported. # Explicitly using /v1 or /v2 enpoints will still work _MIN_API_VERSION = "3.0" -_MAX_API_VERSION = "3.21" +_MAX_API_VERSION = "3.22" _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 dfff8913b..431b5e261 100644 --- a/cinder/api/openstack/rest_api_version_history.rst +++ b/cinder/api/openstack/rest_api_version_history.rst @@ -222,3 +222,8 @@ user documentation. 3.21 ---- Show provider_id in detailed view of a volume for admin. + +3.22 +---- + Added support to filter snapshot list based on metadata of snapshot. + diff --git a/cinder/api/v3/snapshots.py b/cinder/api/v3/snapshots.py index a46fcac41..f5c0be422 100644 --- a/cinder/api/v3/snapshots.py +++ b/cinder/api/v3/snapshots.py @@ -15,9 +15,17 @@ """The volumes snapshots V3 api.""" +import ast + +from oslo_log import log as logging + +from cinder.api import common from cinder.api.openstack import wsgi from cinder.api.v2 import snapshots as snapshots_v2 from cinder.api.v3.views import snapshots as snapshot_views +from cinder import utils + +LOG = logging.getLogger(__name__) class SnapshotsController(snapshots_v2.SnapshotsController): @@ -25,6 +33,74 @@ class SnapshotsController(snapshots_v2.SnapshotsController): _view_builder_class = snapshot_views.ViewBuilder + def _get_snapshot_filter_options(self): + """returns tuple of valid filter options""" + + return 'status', 'volume_id', 'name', 'metadata' + + def _format_snapshot_filter_options(self, search_opts): + """Convert valid filter options to correct expected format""" + + # Get the dict object out of queried metadata + # convert metadata query value from string to dict + if 'metadata' in search_opts.keys(): + try: + search_opts['metadata'] = ast.literal_eval( + search_opts['metadata']) + except (ValueError, SyntaxError): + LOG.debug('Could not evaluate value %s, assuming string', + search_opts['metadata']) + + def _process_filters(self, req, context, search_opts): + """Formats allowed filters""" + + req_version = req.api_version_request + # if the max version is less than or same as 3.21 + # metadata based filtering is not supported + if req_version.matches(None, "3.21"): + search_opts.pop('metadata', None) + + # Filter out invalid options + allowed_search_options = self._get_snapshot_filter_options() + + utils.remove_invalid_filter_options(context, search_opts, + allowed_search_options) + + # process snapshot filters to appropriate formats if required + self._format_snapshot_filter_options(search_opts) + + def _items(self, req, is_detail=True): + """Returns a list of snapshots, transformed through view builder.""" + context = req.environ['cinder.context'] + + # Pop out non search_opts and create local variables + search_opts = req.GET.copy() + sort_keys, sort_dirs = common.get_sort_params(search_opts) + marker, limit, offset = common.get_pagination_params(search_opts) + + # process filters + self._process_filters(req, context, search_opts) + + # NOTE(thingee): v3 API allows name instead of display_name + if 'name' in search_opts: + search_opts['display_name'] = search_opts.pop('name') + + snapshots = self.volume_api.get_all_snapshots(context, + search_opts=search_opts, + marker=marker, + limit=limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + offset=offset) + + req.cache_db_snapshots(snapshots.objects) + + if is_detail: + snapshots = self._view_builder.detail_list(req, snapshots.objects) + else: + snapshots = self._view_builder.summary_list(req, snapshots.objects) + return snapshots + def create_resource(ext_mgr): return wsgi.Resource(SnapshotsController(ext_mgr)) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 1798d7205..48fbdf948 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -2774,11 +2774,38 @@ def _snaps_get_query(context, session=None, project_only=False): def _process_snaps_filters(query, filters): if filters: - # Ensure that filters' keys exist on the model - if not is_valid_model_filters(models.Snapshot, filters, - exclude_list=('host', 'cluster_name')): - return None filters = filters.copy() + + exclude_list = ('host', 'cluster_name') + + # Ensure that filters' keys exist on the model or is metadata + for key in filters.keys(): + # Ensure if filtering based on metadata filter is queried + # then the filters value is a dictionary + if key == 'metadata': + if not isinstance(filters[key], dict): + LOG.debug("Metadata filter value is not valid dictionary") + return None + continue + + if key in exclude_list: + continue + + # for keys in filter other than metadata and exclude_list + # ensure that the keys are in Snapshot modelt + try: + column_attr = getattr(models.Snapshot, key) + prop = getattr(column_attr, 'property') + if isinstance(prop, RelationshipProperty): + LOG.debug( + "'%s' key is not valid, it maps to a relationship.", + key) + return None + except AttributeError: + LOG.debug("'%s' filter key is not valid.", key) + return None + + # filter handling for host and cluster name host = filters.pop('host', None) cluster = filters.pop('cluster_name', None) if host or cluster: @@ -2788,7 +2815,21 @@ def _process_snaps_filters(query, filters): query = query.filter(_filter_host(vol_field.host, host)) if cluster: query = query.filter(_filter_host(vol_field.cluster_name, cluster)) - query = query.filter_by(**filters) + + filters_dict = {} + LOG.debug("Building query based on filter") + for key, value in filters.items(): + if key == 'metadata': + col_attr = getattr(models.Snapshot, 'snapshot_metadata') + for k, v in value.items(): + query = query.filter(col_attr.any(key=k, value=v)) + else: + filters_dict[key] = value + + # Apply exact matches + if filters_dict: + query = query.filter_by(**filters_dict) + return query diff --git a/cinder/tests/unit/api/v3/test_snapshots.py b/cinder/tests/unit/api/v3/test_snapshots.py index 3e035ce76..040dd4e16 100644 --- a/cinder/tests/unit/api/v3/test_snapshots.py +++ b/cinder/tests/unit/api/v3/test_snapshots.py @@ -27,15 +27,44 @@ from cinder.tests.unit.api import fakes from cinder.tests.unit import fake_constants as fake from cinder.tests.unit import fake_snapshot from cinder.tests.unit import fake_volume +from cinder import volume UUID = '00000000-0000-0000-0000-000000000001' INVALID_UUID = '00000000-0000-0000-0000-000000000002' +def stub_get(context, *args, **kwargs): + vol = {'id': fake.VOLUME_ID, + 'size': 100, + 'name': 'fake', + 'host': 'fake-host', + 'status': 'available', + 'encryption_key_id': None, + 'volume_type_id': None, + 'migration_status': None, + 'availability_zone': 'fake-zone', + 'attach_status': 'detached', + 'metadata': {}} + return fake_volume.fake_volume_obj(context, **vol) + + +def create_snapshot_query_with_metadata(metadata_query_string, + api_microversion): + """Helper to create metadata querystring with microversion""" + req = fakes.HTTPRequest.blank('/v3/snapshots?metadata=' + + metadata_query_string) + req.headers["OpenStack-API-Version"] = "volume " + api_microversion + req.api_version_request = api_version.APIVersionRequest( + api_microversion) + + return req + + @ddt.ddt class SnapshotApiTest(test.TestCase): def setUp(self): super(SnapshotApiTest, self).setUp() + self.stubs.Set(volume.api.API, 'get', stub_get) self.controller = snapshots.SnapshotsController() self.ctx = context.RequestContext(fake.USER_ID, fake.PROJECT_ID, True) @@ -77,3 +106,89 @@ class SnapshotApiTest(test.TestCase): req = fakes.HTTPRequest.blank('/v3/snapshots/%s' % snapshot_id) self.assertRaises(exception.SnapshotNotFound, self.controller.show, req, snapshot_id) + + def _create_snapshot_with_metadata(self, metadata): + """Creates test snapshopt with provided metadata""" + req = fakes.HTTPRequest.blank('/v3/snapshots') + snap = {"volume_size": 200, + "volume_id": fake.VOLUME_ID, + "display_name": "Volume Test Name", + "display_description": "Volume Test Desc", + "availability_zone": "zone1:host1", + "host": "fake-host", + "metadata": metadata} + body = {"snapshot": snap} + self.controller.create(req, body) + + def test_snapshot_list_with_one_metadata_in_filter(self): + # Create snapshot with metadata key1: value1 + metadata = {"key1": "val1"} + self._create_snapshot_with_metadata(metadata) + + # Create request with metadata filter key1: value1 + req = create_snapshot_query_with_metadata('{"key1":"val1"}', '3.22') + + # query controller with above request + res_dict = self.controller.detail(req) + + # verify 1 snapshot is returned + self.assertEqual(1, len(res_dict['snapshots'])) + + # verify if the medadata of the returned snapshot is key1: value1 + self.assertDictEqual({"key1": "val1"}, res_dict['snapshots'][0][ + 'metadata']) + + # Create request with metadata filter key2: value2 + req = create_snapshot_query_with_metadata('{"key2":"val2"}', '3.22') + + # query controller with above request + res_dict = self.controller.detail(req) + + # verify no snapshot is returned + self.assertEqual(0, len(res_dict['snapshots'])) + + def test_snapshot_list_with_multiple_metadata_in_filter(self): + # Create snapshot with metadata key1: value1, key11: value11 + metadata = {"key1": "val1", "key11": "val11"} + self._create_snapshot_with_metadata(metadata) + + # Create request with metadata filter key1: value1, key11: value11 + req = create_snapshot_query_with_metadata( + '{"key1":"val1", "key11":"val11"}', '3.22') + + # query controller with above request + res_dict = self.controller.detail(req) + + # verify 1 snapshot is returned + self.assertEqual(1, len(res_dict['snapshots'])) + + # verify if the medadata of the returned snapshot is key1: value1 + self.assertDictEqual({"key1": "val1", "key11": "val11"}, res_dict[ + 'snapshots'][0]['metadata']) + + # Create request with metadata filter key1: value1 + req = create_snapshot_query_with_metadata('{"key1":"val1"}', '3.22') + + # query controller with above request + res_dict = self.controller.detail(req) + + # verify 1 snapshot is returned + self.assertEqual(1, len(res_dict['snapshots'])) + + # verify if the medadata of the returned snapshot is key1: value1 + self.assertDictEqual({"key1": "val1", "key11": "val11"}, res_dict[ + 'snapshots'][0]['metadata']) + + def test_snapshot_list_with_metadata_unsupported_microversion(self): + # Create snapshot with metadata key1: value1 + metadata = {"key1": "val1"} + self._create_snapshot_with_metadata(metadata) + + # Create request with metadata filter key2: value2 + req = create_snapshot_query_with_metadata('{"key2":"val2"}', '3.21') + + # query controller with above request + res_dict = self.controller.detail(req) + + # verify some snapshot is returned + self.assertNotEqual(0, len(res_dict['snapshots'])) diff --git a/releasenotes/notes/support-metadata-based-snapshot-list-filtering-6e6df68a7ce981f5.yaml b/releasenotes/notes/support-metadata-based-snapshot-list-filtering-6e6df68a7ce981f5.yaml new file mode 100644 index 000000000..f593d770f --- /dev/null +++ b/releasenotes/notes/support-metadata-based-snapshot-list-filtering-6e6df68a7ce981f5.yaml @@ -0,0 +1,5 @@ +--- +features: + - Added support to querying snapshots filtered by metadata key/value + using 'metadata' optional URL parameter. + For example, "/v3/snapshots?metadata=={'key1':'value1'}".