From 0f5a7f3ac31f73c12f627f54e6c41449cce99b98 Mon Sep 17 00:00:00 2001 From: wangxiyuan Date: Tue, 1 Aug 2017 14:39:38 +0800 Subject: [PATCH] Support az filter for snapshot Sometimes if users want to create volumes from snapshots with a specified availability_zone, users may want to know what snapshots are available. In this case, users always want to filter the snapshots with "availability_zone" first. This patch added the availability_zone filter for snapshot list. Change-Id: I8953eca5f535c1399dc882c4a232fbeef9bb2959 --- cinder/db/sqlalchemy/api.py | 16 ++++++++++------ cinder/tests/unit/api/test_common.py | 3 ++- cinder/tests/unit/api/v3/test_snapshots.py | 19 +++++++++++++------ doc/source/man/generalized_filters.rst | 3 ++- etc/cinder/resource_filters.json | 3 ++- ...-filter-for-snapshot-8e1494212276abde.yaml | 3 +++ 6 files changed, 32 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/add-availability_zone-filter-for-snapshot-8e1494212276abde.yaml diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index c47cb00aeae..84321eea162 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -2983,9 +2983,9 @@ def snapshot_get_all(context, filters=None, marker=None, limit=None, paired with corresponding item in sort_keys :returns: list of matching snapshots """ - if filters and not is_valid_model_filters(models.Snapshot, filters, - exclude_list=('host', - 'cluster_name')): + if filters and not is_valid_model_filters( + models.Snapshot, filters, + exclude_list=('host', 'cluster_name', 'availability_zone')): return [] session = get_session() @@ -3011,7 +3011,7 @@ def _process_snaps_filters(query, filters): if filters: filters = filters.copy() - exclude_list = ('host', 'cluster_name') + exclude_list = ('host', 'cluster_name', 'availability_zone') # Ensure that filters' keys exist on the model or is metadata for key in filters.keys(): @@ -3043,13 +3043,16 @@ def _process_snaps_filters(query, filters): # filter handling for host and cluster name host = filters.pop('host', None) cluster = filters.pop('cluster_name', None) - if host or cluster: + az = filters.pop('availability_zone', None) + if host or cluster or az: query = query.join(models.Snapshot.volume) vol_field = models.Volume if host: query = query.filter(_filter_host(vol_field.host, host)) if cluster: query = query.filter(_filter_host(vol_field.cluster_name, cluster)) + if az: + query = query.filter_by(availability_zone=az) filters_dict = {} LOG.debug("Building query based on filter") @@ -3162,7 +3165,8 @@ def snapshot_get_all_by_project(context, project_id, filters=None, marker=None, :returns: list of matching snapshots """ if filters and not is_valid_model_filters( - models.Snapshot, filters, exclude_list=('host', 'cluster_name')): + models.Snapshot, filters, + exclude_list=('host', 'cluster_name', 'availability_zone')): return [] authorize_project_context(context, project_id) diff --git a/cinder/tests/unit/api/test_common.py b/cinder/tests/unit/api/test_common.py index 108cd6560ba..288d68beb3f 100644 --- a/cinder/tests/unit/api/test_common.py +++ b/cinder/tests/unit/api/test_common.py @@ -466,7 +466,8 @@ class GeneralFiltersTest(test.TestCase): {'resource': 'backup', 'expected': ["name", "status", "volume_id"]}, {'resource': 'snapshot', - 'expected': ["name", "status", "volume_id", "metadata"]}, + 'expected': ["name", "status", "volume_id", "metadata", + "availability_zone"]}, {'resource': 'group_snapshot', 'expected': ["status", "group_id"]}, {'resource': 'attachment', diff --git a/cinder/tests/unit/api/v3/test_snapshots.py b/cinder/tests/unit/api/v3/test_snapshots.py index b8788b2e43b..b15bc70c1b0 100644 --- a/cinder/tests/unit/api/v3/test_snapshots.py +++ b/cinder/tests/unit/api/v3/test_snapshots.py @@ -126,18 +126,25 @@ class SnapshotApiTest(test.TestCase): body = {"snapshot": snap} self.controller.create(req, body) - @ddt.data(('host', 'test_host1'), ('cluster_name', 'cluster1')) + @ddt.data(('host', 'test_host1', True), ('cluster_name', 'cluster1', True), + ('availability_zone', 'nova1', False)) @ddt.unpack - def test_snapshot_list_with_filter(self, filter_name, filter_value): + def test_snapshot_list_with_filter(self, filter_name, filter_value, + is_admin_user): volume1 = test_utils.create_volume(self.ctx, host='test_host1', - cluster_name='cluster1') + cluster_name='cluster1', + availability_zone='nova1') volume2 = test_utils.create_volume(self.ctx, host='test_host2', - cluster_name='cluster2') + cluster_name='cluster2', + availability_zone='nova2') snapshot1 = test_utils.create_snapshot(self.ctx, volume1.id) - snapshot2 = test_utils.create_snapshot(self.ctx, volume2.id) # noqa + test_utils.create_snapshot(self.ctx, volume2.id) url = '/v3/snapshots?%s=%s' % (filter_name, filter_value) - req = fakes.HTTPRequest.blank(url, use_admin_context=True) + # Generic filtering is introduced since '3,31' and we add + # 'availability_zone' support by using generic filtering. + req = fakes.HTTPRequest.blank(url, use_admin_context=is_admin_user, + version='3.31') res_dict = self.controller.detail(req) self.assertEqual(1, len(res_dict['snapshots'])) diff --git a/doc/source/man/generalized_filters.rst b/doc/source/man/generalized_filters.rst index 57321c52d93..f178202b072 100644 --- a/doc/source/man/generalized_filters.rst +++ b/doc/source/man/generalized_filters.rst @@ -58,7 +58,8 @@ valid for first. The supported APIs are marked with "*" below in the table. | | size, description, replication_status, multiattach | +-----------------+-------------------------------------------------------------------------+ | | id, volume_id, user_id, project_id, status, volume_size, name, | -| list snapshot* | description, volume_type_id, group_snapshot_id, metadata | +| list snapshot* | description, volume_type_id, group_snapshot_id, metadata, | +| | availability_zone | +-----------------+-------------------------------------------------------------------------+ | | id, name, status, container, availability_zone, description, | | list backup* | volume_id, is_incremental, size, host, parent_id | diff --git a/etc/cinder/resource_filters.json b/etc/cinder/resource_filters.json index 00a4831b148..867c1a70555 100644 --- a/etc/cinder/resource_filters.json +++ b/etc/cinder/resource_filters.json @@ -3,7 +3,8 @@ "bootable", "migration_status", "availability_zone", "group_id"], "backup": ["name", "status", "volume_id"], - "snapshot": ["name", "status", "volume_id", "metadata"], + "snapshot": ["name", "status", "volume_id", "metadata", + "availability_zone"], "group": [], "group_snapshot": ["status", "group_id"], "attachment": ["volume_id", "status", "instance_id", "attach_status"], diff --git a/releasenotes/notes/add-availability_zone-filter-for-snapshot-8e1494212276abde.yaml b/releasenotes/notes/add-availability_zone-filter-for-snapshot-8e1494212276abde.yaml new file mode 100644 index 00000000000..9fa5412f2f9 --- /dev/null +++ b/releasenotes/notes/add-availability_zone-filter-for-snapshot-8e1494212276abde.yaml @@ -0,0 +1,3 @@ +--- +features: + - Added availability_zone filter for snapshots list.