From e0c7cef434e90bb8b971040b0c1a47964c6d89fb Mon Sep 17 00:00:00 2001
From: Stephen Finucane <stephenfin@redhat.com>
Date: Mon, 29 May 2023 16:31:21 +0100
Subject: [PATCH] volume: Add aliases for common volume type props
Add aliases for the enabling multiattach, caching and replication via
extra spec properties. These are useful since the syntax for setting
each of them is a bit weird and frankly not very discoverable.
Change-Id: I08b51224c66a34a9dcfcffc95847e61d9aac0e7e
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
---
.../tests/unit/volume/v2/test_volume_type.py | 104 ++++++++++---
openstackclient/volume/v2/volume_type.py | 143 ++++++++++++++++--
...ume-type-extra-specs-22a22fcb6e269832.yaml | 7 +
3 files changed, 223 insertions(+), 31 deletions(-)
create mode 100644 releasenotes/notes/volume-type-extra-specs-22a22fcb6e269832.yaml
diff --git a/openstackclient/tests/unit/volume/v2/test_volume_type.py b/openstackclient/tests/unit/volume/v2/test_volume_type.py
index 7f235e17a0..e2b503b9b9 100644
--- a/openstackclient/tests/unit/volume/v2/test_volume_type.py
+++ b/openstackclient/tests/unit/volume/v2/test_volume_type.py
@@ -48,18 +48,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,
@@ -123,7 +124,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,
@@ -136,7 +175,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):
@@ -390,47 +431,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)
@@ -549,12 +603,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)
@@ -563,7 +624,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()
diff --git a/openstackclient/volume/v2/volume_type.py b/openstackclient/volume/v2/volume_type.py
index bf10a2e545..c8e5627431 100644
--- a/openstackclient/volume/v2/volume_type.py
+++ b/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 '
- 'of a volume. Consider using other encryption options '
+ '(this option is required when setting encryption type '
+ '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'] = parsed_args.properties
+ search_opts['extra_specs'] = 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:
diff --git a/releasenotes/notes/volume-type-extra-specs-22a22fcb6e269832.yaml b/releasenotes/notes/volume-type-extra-specs-22a22fcb6e269832.yaml
new file mode 100644
index 0000000000..fb30f98cf6
--- /dev/null
+++ b/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.