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>
This commit is contained in:
Stephen Finucane
parent
67bec7785c
commit
e0c7cef434
openstackclient
releasenotes/notes
@ -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()
|
||||
|
||||
@ -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:
|
||||
|
||||
@ -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.
|
||||
Loading…
x
Reference in New Issue
Block a user