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

@@ -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"
 

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.
+

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))

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
 
 

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']))

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'}".