volume: Allow filtering volume types by properties

Starting in volume API microversion 3.52, it is possible to filter
volume types by extra specs (a.k.a. properties). Even non-admins can do
this for so-called "user visible" extra specs. Make it possible to do
this.

While we're here, the test file is renamed to match the name of the
module under test.

Change-Id: Ia9acc06c0c615bf24e12b63146ea97ac2505f596
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

Author: stephenfin

openstackclient/tests/unit/volume/v2/test_volume_type.py

@@ -15,6 +15,7 @@
 from unittest import mock
 from unittest.mock import call
 
+from cinderclient import api_versions
 from osc_lib.cli import format_columns
 from osc_lib import exceptions
 from osc_lib import utils
@@ -329,7 +330,9 @@ def test_type_list_without_options(self):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         columns, data = self.cmd.take_action(parsed_args)
-        self.volume_types_mock.list.assert_called_once_with(is_public=None)
+        self.volume_types_mock.list.assert_called_once_with(
+            search_opts={}, is_public=None
+        )
         self.assertEqual(self.columns, columns)
         self.assertCountEqual(self.data, list(data))
 
@@ -346,7 +349,9 @@ def test_type_list_with_options(self):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         columns, data = self.cmd.take_action(parsed_args)
-        self.volume_types_mock.list.assert_called_once_with(is_public=True)
+        self.volume_types_mock.list.assert_called_once_with(
+            search_opts={}, is_public=True
+        )
         self.assertEqual(self.columns_long, columns)
         self.assertCountEqual(self.data_long, list(data))
 
@@ -362,7 +367,9 @@ def test_type_list_with_private_option(self):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         columns, data = self.cmd.take_action(parsed_args)
-        self.volume_types_mock.list.assert_called_once_with(is_public=False)
+        self.volume_types_mock.list.assert_called_once_with(
+            search_opts={}, is_public=False
+        )
         self.assertEqual(self.columns, columns)
         self.assertCountEqual(self.data, list(data))
 
@@ -383,6 +390,60 @@ def test_type_list_with_default_option(self):
         self.assertEqual(self.columns, columns)
         self.assertCountEqual(self.data_with_default_type, list(data))
 
+    def test_type_list_with_property_option(self):
+        self.app.client_manager.volume.api_version = api_versions.APIVersion(
+            '3.52'
+        )
+
+        arglist = [
+            "--property",
+            "multiattach=<is> True",
+        ]
+        verifylist = [
+            ("encryption_type", False),
+            ("long", False),
+            ("is_public", None),
+            ("default", False),
+            ("properties", {"multiattach": "<is> True"}),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+        self.volume_types_mock.list.assert_called_once_with(
+            search_opts={"extra_specs": {"multiattach": "<is> True"}},
+            is_public=None,
+        )
+        self.assertEqual(self.columns, columns)
+        self.assertCountEqual(self.data, list(data))
+
+    def test_type_list_with_property_option_pre_v352(self):
+        self.app.client_manager.volume.api_version = api_versions.APIVersion(
+            '3.51'
+        )
+
+        arglist = [
+            "--property",
+            "multiattach=<is> True",
+        ]
+        verifylist = [
+            ("encryption_type", False),
+            ("long", False),
+            ("is_public", None),
+            ("default", False),
+            ("properties", {"multiattach": "<is> True"}),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        exc = self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args,
+        )
+        self.assertIn(
+            '--os-volume-api-version 3.52 or greater is required',
+            str(exc),
+        )
+
     def test_type_list_with_encryption(self):
         encryption_type = volume_fakes.create_one_encryption_volume_type(
             attrs={'volume_type_id': self.volume_types[0].id},
@@ -428,7 +489,9 @@ def test_type_list_with_encryption(self):
 
         columns, data = self.cmd.take_action(parsed_args)
         self.volume_encryption_types_mock.list.assert_called_once_with()
-        self.volume_types_mock.list.assert_called_once_with(is_public=None)
+        self.volume_types_mock.list.assert_called_once_with(
+            search_opts={}, is_public=None
+        )
         self.assertEqual(encryption_columns, columns)
         self.assertCountEqual(encryption_data, list(data))
 
@@ -461,7 +524,7 @@ def test_type_set(self):
         verifylist = [
             ('name', 'new_name'),
             ('description', 'new_description'),
-            ('property', None),
+            ('properties', None),
             ('volume_type', self.volume_type.id),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -491,7 +554,7 @@ def test_type_set_property(self):
         verifylist = [
             ('name', None),
             ('description', None),
-            ('property', {'myprop': 'myvalue'}),
+            ('properties', {'myprop': 'myvalue'}),
             ('volume_type', self.volume_type.id),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -859,7 +922,7 @@ def test_type_unset(self):
             self.volume_type.id,
         ]
         verifylist = [
-            ('property', ['property', 'multi_property']),
+            ('properties', ['property', 'multi_property']),
             ('volume_type', self.volume_type.id),
         ]
 

openstackclient/volume/v2/volume_type.py

@@ -17,6 +17,7 @@
 import functools
 import logging
 
+from cinderclient import api_versions
 from cliff import columns as cliff_columns
 from osc_lib.cli import format_columns
 from osc_lib.cli import parseractions
@@ -139,6 +140,7 @@ def get_parser(self, prog_name):
             '--property',
             metavar='<key=value>',
             action=parseractions.KeyValueAction,
+            dest='properties',
             help=_(
                 'Set a property on this volume type '
                 '(repeat option to set multiple properties)'
@@ -232,11 +234,13 @@ def take_action(self, parsed_args):
                     "type: %(e)s"
                 )
                 LOG.error(msg % {'project': parsed_args.project, 'e': e})
-        if parsed_args.property:
-            result = volume_type.set_keys(parsed_args.property)
+
+        if parsed_args.properties:
+            result = volume_type.set_keys(parsed_args.properties)
             volume_type._info.update(
                 {'properties': format_columns.DictColumn(result)}
             )
+
         if (
             parsed_args.encryption_provider
             or parsed_args.encryption_cipher
@@ -261,6 +265,7 @@ def take_action(self, parsed_args):
             volume_type._info.update(
                 {'encryption': format_columns.DictColumn(encryption._info)}
             )
+
         volume_type._info.pop("os-volume-type-access:is_public", None)
 
         return zip(*sorted(volume_type._info.items()))
@@ -348,6 +353,18 @@ def get_parser(self, prog_name):
                 "(admin only)"
             ),
         )
+        parser.add_argument(
+            '--property',
+            metavar='<key=value>',
+            action=parseractions.KeyValueAction,
+            dest='properties',
+            help=_(
+                'Filter by a property on the volume types '
+                '(repeat option to filter by multiple properties) '
+                '(admin only except for user-visible extra specs) '
+                '(supported by --os-volume-api-version 3.52 or above)'
+            ),
+        )
         return parser
 
     def take_action(self, parsed_args):
@@ -375,8 +392,22 @@ def take_action(self, parsed_args):
         if parsed_args.default:
             data = [volume_client.volume_types.default()]
         else:
+            search_opts = {}
+
+            if parsed_args.properties:
+                if volume_client.api_version < api_versions.APIVersion('3.52'):
+                    msg = _(
+                        "--os-volume-api-version 3.52 or greater is required "
+                        "to use the '--property' option"
+                    )
+                    raise exceptions.CommandError(msg)
+
+                # we pass this through as-is
+                search_opts['extra_specs'] = parsed_args.properties
+
             data = volume_client.volume_types.list(
-                is_public=parsed_args.is_public
+                search_opts=search_opts,
+                is_public=parsed_args.is_public,
             )
 
         formatters = {'Extra Specs': format_columns.DictColumn}
@@ -445,6 +476,7 @@ def get_parser(self, prog_name):
             '--property',
             metavar='<key=value>',
             action=parseractions.KeyValueAction,
+            dest='properties',
             help=_(
                 'Set a property on this volume type '
                 '(repeat option to set multiple properties)'
@@ -555,9 +587,9 @@ def take_action(self, parsed_args):
                 )
                 result += 1
 
-        if parsed_args.property:
+        if parsed_args.properties:
             try:
-                volume_type.set_keys(parsed_args.property)
+                volume_type.set_keys(parsed_args.properties)
             except Exception as e:
                 LOG.error(_("Failed to set volume type property: %s"), e)
                 result += 1
@@ -689,6 +721,7 @@ def get_parser(self, prog_name):
             '--property',
             metavar='<key>',
             action='append',
+            dest='properties',
             help=_(
                 'Remove a property from this volume type '
                 '(repeat option to remove multiple properties)'
@@ -723,9 +756,9 @@ def take_action(self, parsed_args):
         )
 
         result = 0
-        if parsed_args.property:
+        if parsed_args.properties:
             try:
-                volume_type.unset_keys(parsed_args.property)
+                volume_type.unset_keys(parsed_args.properties)
             except Exception as e:
                 LOG.error(_("Failed to unset volume type property: %s"), e)
                 result += 1

releasenotes/notes/volume-type-list-properties-filter-8532f96d16733915.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    The ``volume type list`` command now accepts a ``--property <key>=<value>``
+    option, allowing users to filter volume types by their extra spec
+    properties.