Merge "volume: Add aliases for common volume type props"

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

@@ -46,18 +46,19 @@ class TestType(volume_fakes.TestVolume):
 
 
 class TestTypeCreate(TestType):
-    project = identity_fakes.FakeProject.create_one_project()
-    columns = (
-        'description',
-        'id',
-        'is_public',
-        'name',
-    )
-
     def setUp(self):
         super().setUp()
 
-        self.new_volume_type = volume_fakes.create_one_volume_type()
+        self.new_volume_type = volume_fakes.create_one_volume_type(
+            methods={'set_keys': None},
+        )
+        self.project = identity_fakes.FakeProject.create_one_project()
+        self.columns = (
+            'description',
+            'id',
+            'is_public',
+            'name',
+        )
         self.data = (
             self.new_volume_type.description,
             self.new_volume_type.id,
@@ -121,7 +122,45 @@ class TestTypeCreate(TestType):
         self.assertEqual(self.columns, columns)
         self.assertCountEqual(self.data, data)
 
-    def test_public_type_create_with_project(self):
+    def test_type_create_with_properties(self):
+        arglist = [
+            '--property',
+            'myprop=myvalue',
+            # this combination isn't viable server-side but is okay for testing
+            '--multiattach',
+            '--cacheable',
+            '--replicated',
+            self.new_volume_type.name,
+        ]
+        verifylist = [
+            ('properties', {'myprop': 'myvalue'}),
+            ('multiattach', True),
+            ('cacheable', True),
+            ('replicated', True),
+            ('name', self.new_volume_type.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+        self.volume_types_mock.create.assert_called_with(
+            self.new_volume_type.name, description=None
+        )
+        self.new_volume_type.set_keys.assert_called_once_with(
+            {
+                'myprop': 'myvalue',
+                'multiattach': '<is> True',
+                'cacheable': '<is> True',
+                'replication_enabled': '<is> True',
+            }
+        )
+
+        self.columns += ('properties',)
+        self.data += (format_columns.DictColumn(None),)
+
+        self.assertEqual(self.columns, columns)
+        self.assertCountEqual(self.data, data)
+
+    def test_public_type_create_with_project_public(self):
         arglist = [
             '--project',
             self.project.id,
@@ -134,7 +173,9 @@ class TestTypeCreate(TestType):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         self.assertRaises(
-            exceptions.CommandError, self.cmd.take_action, parsed_args
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args,
         )
 
     def test_type_create_with_encryption(self):
@@ -388,47 +429,60 @@ class TestTypeList(TestType):
         self.assertEqual(self.columns, columns)
         self.assertCountEqual(self.data_with_default_type, list(data))
 
-    def test_type_list_with_property_option(self):
+    def test_type_list_with_properties(self):
         self.app.client_manager.volume.api_version = api_versions.APIVersion(
             '3.52'
         )
 
         arglist = [
             "--property",
-            "multiattach=<is> True",
+            "foo=bar",
+            "--multiattach",
+            "--cacheable",
+            "--replicated",
         ]
         verifylist = [
             ("encryption_type", False),
             ("long", False),
             ("is_public", None),
             ("default", False),
-            ("properties", {"multiattach": "<is> True"}),
+            ("properties", {"foo": "bar"}),
+            ("multiattach", True),
+            ("cacheable", True),
+            ("replicated", 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"}},
+            search_opts={
+                "extra_specs": {
+                    "foo": "bar",
+                    "multiattach": "<is> True",
+                    "cacheable": "<is> True",
+                    "replication_enabled": "<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):
+    def test_type_list_with_properties_pre_v352(self):
         self.app.client_manager.volume.api_version = api_versions.APIVersion(
             '3.51'
         )
 
         arglist = [
             "--property",
-            "multiattach=<is> True",
+            "foo=bar",
         ]
         verifylist = [
             ("encryption_type", False),
             ("long", False),
             ("is_public", None),
             ("default", False),
-            ("properties", {"multiattach": "<is> True"}),
+            ("properties", {"foo": "bar"}),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
@@ -547,12 +601,19 @@ class TestTypeSet(TestType):
         arglist = [
             '--property',
             'myprop=myvalue',
+            # this combination isn't viable server-side but is okay for testing
+            '--multiattach',
+            '--cacheable',
+            '--replicated',
             self.volume_type.id,
         ]
         verifylist = [
             ('name', None),
             ('description', None),
             ('properties', {'myprop': 'myvalue'}),
+            ('multiattach', True),
+            ('cacheable', True),
+            ('replicated', True),
             ('volume_type', self.volume_type.id),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -561,7 +622,12 @@ class TestTypeSet(TestType):
         self.assertIsNone(result)
 
         self.volume_type.set_keys.assert_called_once_with(
-            {'myprop': 'myvalue'}
+            {
+                'myprop': 'myvalue',
+                'multiattach': '<is> True',
+                'cacheable': '<is> True',
+                'replication_enabled': '<is> True',
+            }
         )
         self.volume_type_access_mock.add_project_access.assert_not_called()
         self.volume_encryption_types_mock.update.assert_not_called()

openstackclient/volume/v2/volume_type.py

@@ -146,14 +146,45 @@ class CreateVolumeType(command.ShowOne):
                 '(repeat option to set multiple properties)'
             ),
         )
+        parser.add_argument(
+            '--multiattach',
+            action='store_true',
+            default=False,
+            help=_(
+                "Enable multi-attach for this volume type "
+                "(this is an alias for '--property multiattach=<is> True') "
+                "(requires driver support)"
+            ),
+        )
+        parser.add_argument(
+            '--cacheable',
+            action='store_true',
+            default=False,
+            help=_(
+                "Enable caching for this volume type "
+                "(this is an alias for '--property cacheable=<is> True') "
+                "(requires driver support)"
+            ),
+        )
+        parser.add_argument(
+            '--replicated',
+            action='store_true',
+            default=False,
+            help=_(
+                "Enabled replication for this volume type "
+                "(this is an alias for '--property replication_enabled=<is> True') "  # noqa: E501
+                "(requires driver support)"
+            ),
+        )
         parser.add_argument(
             '--project',
             metavar='<project>',
             help=_(
                 "Allow <project> to access private type (name or ID) "
-                "(Must be used with --private option)"
+                "(must be used with --private option)"
             ),
         )
+        identity_common.add_project_domain_option_to_parser(parser)
         # TODO(Huanxuan Ao): Add choices for each "--encryption-*" option.
         parser.add_argument(
             '--encryption-provider',
@@ -161,8 +192,8 @@ class CreateVolumeType(command.ShowOne):
             help=_(
                 'Set the encryption provider format for '
                 'this volume type (e.g "luks" or "plain") (admin only) '
-                '(This option is required when setting encryption type '
+                '(this option is required when setting encryption type '
-                'of a volume. Consider using other encryption options '
+                'of a volume; consider using other encryption options '
                 'such as: "--encryption-cipher", "--encryption-key-size" '
                 'and "--encryption-control-location")'
             ),
@@ -198,7 +229,6 @@ class CreateVolumeType(command.ShowOne):
                 '"--encryption-provider")'
             ),
         )
-        identity_common.add_project_domain_option_to_parser(parser)
         return parser
 
     def take_action(self, parsed_args):
@@ -235,8 +265,17 @@ class CreateVolumeType(command.ShowOne):
                 )
                 LOG.error(msg % {'project': parsed_args.project, 'e': e})
 
+        properties = {}
         if parsed_args.properties:
-            result = volume_type.set_keys(parsed_args.properties)
+            properties.update(parsed_args.properties)
+        if parsed_args.multiattach:
+            properties['multiattach'] = '<is> True'
+        if parsed_args.cacheable:
+            properties['cacheable'] = '<is> True'
+        if parsed_args.replicated:
+            properties['replication_enabled'] = '<is> True'
+        if properties:
+            result = volume_type.set_keys(properties)
             volume_type._info.update(
                 {'properties': format_columns.DictColumn(result)}
             )
@@ -365,6 +404,37 @@ class ListVolumeType(command.Lister):
                 '(supported by --os-volume-api-version 3.52 or above)'
             ),
         )
+        parser.add_argument(
+            '--multiattach',
+            action='store_true',
+            default=False,
+            help=_(
+                "List only volume types with multi-attach enabled "
+                "(this is an alias for '--property multiattach=<is> True') "
+                "(supported by --os-volume-api-version 3.52 or above)"
+            ),
+        )
+        parser.add_argument(
+            '--cacheable',
+            action='store_true',
+            default=False,
+            help=_(
+                "List only volume types with caching enabled "
+                "(this is an alias for '--property cacheable=<is> True') "
+                "(admin only) "
+                "(supported by --os-volume-api-version 3.52 or above)"
+            ),
+        )
+        parser.add_argument(
+            '--replicated',
+            action='store_true',
+            default=False,
+            help=_(
+                "List only volume types with replication enabled "
+                "(this is an alias for '--property replication_enabled=<is> True') "  # noqa: E501
+                "(supported by --os-volume-api-version 3.52 or above)"
+            ),
+        )
         return parser
 
     def take_action(self, parsed_args):
@@ -393,17 +463,25 @@ class ListVolumeType(command.Lister):
             data = [volume_client.volume_types.default()]
         else:
             search_opts = {}
-
+            properties = {}
             if parsed_args.properties:
+                properties.update(parsed_args.properties)
+            if parsed_args.multiattach:
+                properties['multiattach'] = '<is> True'
+            if parsed_args.cacheable:
+                properties['cacheable'] = '<is> True'
+            if parsed_args.replicated:
+                properties['replication_enabled'] = '<is> True'
+            if 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"
+                        "to use the '--property' option or any of the alias "
+                        "options"
                     )
                     raise exceptions.CommandError(msg)
 
-                # we pass this through as-is
+                search_opts['extra_specs'] = properties
-                search_opts['extra_specs'] = parsed_args.properties
 
             data = volume_client.volume_types.list(
                 search_opts=search_opts,
@@ -482,6 +560,36 @@ class SetVolumeType(command.Command):
                 '(repeat option to set multiple properties)'
             ),
         )
+        parser.add_argument(
+            '--multiattach',
+            action='store_true',
+            default=False,
+            help=_(
+                "Enable multi-attach for this volume type "
+                "(this is an alias for '--property multiattach=<is> True') "
+                "(requires driver support)"
+            ),
+        )
+        parser.add_argument(
+            '--cacheable',
+            action='store_true',
+            default=False,
+            help=_(
+                "Enable caching for this volume type "
+                "(this is an alias for '--property cacheable=<is> True') "
+                "(requires driver support)"
+            ),
+        )
+        parser.add_argument(
+            '--replicated',
+            action='store_true',
+            default=False,
+            help=_(
+                "Enabled replication for this volume type "
+                "(this is an alias for '--property replication_enabled=<is> True') "  # noqa: E501
+                "(requires driver support)"
+            ),
+        )
         parser.add_argument(
             '--project',
             metavar='<project>',
@@ -587,11 +695,22 @@ class SetVolumeType(command.Command):
                 )
                 result += 1
 
+        properties = {}
+
+        properties = {}
         if parsed_args.properties:
+            properties.update(parsed_args.properties)
+        if parsed_args.multiattach:
+            properties['multiattach'] = '<is> True'
+        if parsed_args.cacheable:
+            properties['cacheable'] = '<is> True'
+        if parsed_args.replicated:
+            properties['replication_enabled'] = '<is> True'
+        if properties:
             try:
-                volume_type.set_keys(parsed_args.properties)
+                volume_type.set_keys(properties)
             except Exception as e:
-                LOG.error(_("Failed to set volume type property: %s"), e)
+                LOG.error(_("Failed to set volume type properties: %s"), e)
                 result += 1
 
         if parsed_args.project:
@@ -760,7 +879,7 @@ class UnsetVolumeType(command.Command):
             try:
                 volume_type.unset_keys(parsed_args.properties)
             except Exception as e:
-                LOG.error(_("Failed to unset volume type property: %s"), e)
+                LOG.error(_("Failed to unset volume type properties: %s"), e)
                 result += 1
 
         if parsed_args.project:

releasenotes/notes/volume-type-extra-specs-22a22fcb6e269832.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    The ``volume type create``, ``volume type set``, ``volume type list``
+    commands now accept three new options - ``--multiattach``, ``--cacheable``,
+    and ``--replicated`` - which are short cuts for setting or filtering on
+    the relevant properties on the volume type.