openstack/python-manilaclient
Code Issues Proposed changes

Handle empty metadata properly in filter/list

Browse Source 
Introduced "None" tag for metadata command.
"list --metadata None" command will filter out shares that has empty metadata, i.e., metadata={}
Closes-Bug: #1782847
Change-Id: Iac33cd1295b1e7c2ff11e6d7bd003fde28785c0e
This commit is contained in:
tspyderboy 2024-03-27 18:02:49 +05:30
parent e12db73f85
commit d24527a7f4
5 changed files with 127 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
manilaclient/tests/unit/v2/fakes.py
View File

@ -224,12 +224,15 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
def get_shares_detail(self, **kw):
endpoint = "http://127.0.0.1:8786/v2"
share_id = '1234'
share_id1234 = '1234'
share_id1111 = '1111'
share_id2222 = '2222'
share_id3333 = '3333'
shares = {
'shares': [
{
'id': share_id,
'id': share_id1234,
'name': 'sharename',
'status': 'fake_status',
'size': 1,
@ -237,9 +240,52 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
'export_location': 'fake_export_location',
'snapshot_id': 'fake_snapshot_id',
'links': [
{"href": endpoint + "/fake_project/shares/" + share_id,
"rel": "self"},
{"href": endpoint + "/fake_project/shares/"
+ share_id1234, "rel": "self"},
],
'metadata': {}
},
{
'id': share_id1111,
'name': 'sharename1',
'status': 'fake_status1',
'size': 1,
'host': 'fake_host',
'export_location': 'fake_export_location',
'snapshot_id': 'fake_snapshot_id',
'links': [
{"href": endpoint + "/fake_project/shares/"
+ share_id1111, "rel": "self"},
],
'metadata': {}
},
{
'id': share_id2222,
'name': 'sharename2',
'status': 'fake_status2',
'size': 1,
'host': 'fake_host',
'export_location': 'fake_export_location',
'snapshot_id': 'fake_snapshot_id',
'links': [
{"href": endpoint + "/fake_project/shares/"
+ share_id2222, "rel": "self"},
],
'metadata': {'key1': 'value1'}
},
{
'id': share_id3333,
'name': 'sharename3',
'status': 'fake_status3',
'size': 1,
'host': 'fake_host',
'export_location': 'fake_export_location',
'snapshot_id': 'fake_snapshot_id',
'links': [
{"href": endpoint + "/fake_project/shares/"
+ share_id3333, "rel": "self"},
],
'metadata': {'key1': 'value1', 'key2': 'value2'}
},
],
}

2
manilaclient/tests/unit/v2/test_shares.py
View File

@ -421,7 +421,7 @@ class SharesTest(utils.TestCase):
cs.assert_called(
'GET', '/shares/detail?is_public=True&with_count=True')
self.assertEqual(2, count)
self.assertEqual(1, len(shares))
self.assertEqual(4, len(shares))
def test_list_shares_detailed_with_count(self):
cs.shares.list(detailed=True)

59
manilaclient/tests/unit/v2/test_shell.py
View File

@ -231,8 +231,65 @@ class ShellTest(test_utils.TestCase):
def test_list_filter_by_metadata(self):
self.run_command('list --metadata key=value')
# /shares/detail?metadata={'key': 'value'}
self.assert_called(
'GET', '/shares/detail?metadata=%7B%27key%27%3A+%27value%27%7D')
'GET', '/shares/detail?metadata='
'%7B%27key%27%3A+%27value%27%7D')
def test_list_filter_by_metadata_with_multiple_key_values(self):
self.run_command('list --metadata key1=value1 '
'key2=value2 key3=value3')
# /shares/detail?metadata={'key1': 'value1',
# 'key2': 'value2', 'key3': 'value3'}
self.assert_called(
'GET', '/shares/detail?metadata='
'%7B%27key1%27%3A+%27value1%27%2C+%27key2%27%3A+'
'%27value2%27%2C+%27key3%27%3A+%27value3%27%7D')
@mock.patch.object(cliutils, 'print_list', mock.Mock())
def test_list_filter_by_metadata_with_empty_metadata(self):
self.run_command('list --metadata \'\'')
self.assert_called(
'GET', '/shares/detail')
cliutils.print_list.assert_called()
args, _ = cliutils.print_list.call_args
shares = args[0]
# All 4 shares irrespective of metadata values printed
self.assertEqual(len(shares), 4)
@mock.patch.object(cliutils, 'print_list', mock.Mock())
def test_list_filter_by_metadata_set_to_None(self):
self.run_command('list --metadata None')
self.assert_called(
'GET', '/shares/detail')
cliutils.print_list.assert_called()
args, _ = cliutils.print_list.call_args
shares = args[0]
# Check that the size of shares is 2(shares with metadata={})
self.assertEqual(len(shares), 2)
for share in shares:
self.assertEqual(share.metadata, {})
def test_list_filter_by_metadata_with_one_empty_of_many_metadata(self):
self.run_command('list --metadata key=value \'\'')
# /shares/detail?metadata={'key': 'value'}
self.assert_called(
'GET', '/shares/detail?metadata='
'%7B%27key%27%3A+%27value%27%7D')
@mock.patch.object(cliutils, 'print_list', mock.Mock())
def test_list_filter_by_metadata_with_no_metadata(self):
self.run_command('list --metadata')
# /shares/detail
self.assert_called(
'GET', '/shares/detail')
cliutils.print_list.assert_called()
args, _ = cliutils.print_list.call_args
shares = args[0]
# All 4 shares printed
self.assertEqual(len(shares), 4)
def test_list_filter_by_extra_specs_and_its_aliases(self):
aliases = ['--extra-specs', '--extra_specs', ]

18
manilaclient/v2/shell.py
View File

@ -396,8 +396,8 @@ def _translate_keys(collection, convert):
setattr(item, to_key, item._info[from_key])
def _extract_metadata(args):
return _extract_key_value_options(args, 'metadata')
def _extract_metadata(args, allow_empty_key=True):
return _extract_key_value_options(args, 'metadata', allow_empty_key)
def _extract_extra_specs(args):
@ -408,7 +408,7 @@ def _extract_group_specs(args):
return _extract_key_value_options(args, 'group_specs')
def _extract_key_value_options(args, option_name):
def _extract_key_value_options(args, option_name, allow_empty_key=True):
result_dict = {}
duplicate_options = []
@ -419,10 +419,12 @@ def _extract_key_value_options(args, option_name):
# unset doesn't require a val, so we have the if/else
if '=' in option:
(key, value) = option.split('=', 1)
else:
elif allow_empty_key:
key = option
value = None
else:
# disallow empty key
continue
if key not in result_dict:
result_dict[key] = value
else:
@ -2502,7 +2504,7 @@ def do_list(cs, args):
'share_network_id': share_network.id,
'snapshot_id': snapshot.id,
'share_type_id': share_type.id,
'metadata': _extract_metadata(args),
'metadata': _extract_metadata(args, allow_empty_key=False),
'extra_specs': _extract_extra_specs(args),
'share_server_id': args.share_server_id,
'project_id': args.project_id,
@ -2558,6 +2560,10 @@ def do_list(cs, args):
search_opts=search_opts, sort_key=args.sort_key,
sort_dir=args.sort_dir
)
# When shell input is "--metadata None", filter metadata={}
if shares and args.metadata:
if "None" in args.metadata:
shares = [share for share in shares if share.metadata == {}]
# NOTE(vponomaryov): usage of 'export_location' and
# 'export_locations' columns may cause scaling issue using API 2.9+ and
# when lots of shares are returned.

6
releasenotes/notes/bug-1782847-filter-list-resources-with-empty-metadata-key-value-cc6f858994f6e9ee.yaml Normal file
View File

@ -0,0 +1,6 @@
---
fixes:
- |
The --metadata command should handle empty metadata in proper way.
Introduced "None" metadata tag to filter list which have empty.
Example: "list --metadata None" will return only elements with metadata={}