diff options
-rw-r--r-- | cinderclient/api_versions.py | 2 | ||||
-rw-r--r-- | cinderclient/tests/unit/v3/test_shell.py | 61 | ||||
-rw-r--r-- | cinderclient/v3/shell.py | 138 | ||||
-rw-r--r-- | cinderclient/v3/shell_base.py | 56 | ||||
-rw-r--r-- | cinderclient/v3/volume_snapshots.py | 45 | ||||
-rw-r--r-- | releasenotes/notes/support-bs-mv-3.66-5214deb20d164056.yaml | 9 |
6 files changed, 254 insertions, 57 deletions
diff --git a/cinderclient/api_versions.py b/cinderclient/api_versions.py index 78bb8a0..48e00b7 100644 --- a/cinderclient/api_versions.py +++ b/cinderclient/api_versions.py @@ -26,7 +26,7 @@ LOG = logging.getLogger(__name__) # key is unsupported version, value is appropriate supported alternative REPLACEMENT_VERSIONS = {"1": "3", "2": "3"} -MAX_VERSION = "3.65" +MAX_VERSION = "3.66" MIN_VERSION = "3.0" _SUBSTITUTIONS = {} diff --git a/cinderclient/tests/unit/v3/test_shell.py b/cinderclient/tests/unit/v3/test_shell.py index 9a6852f..8f00525 100644 --- a/cinderclient/tests/unit/v3/test_shell.py +++ b/cinderclient/tests/unit/v3/test_shell.py @@ -900,6 +900,67 @@ class ShellTest(utils.TestCase): 'manageable-list --cluster dest') self.assert_called('GET', '/manageable_volumes/detail?cluster=dest') + @ddt.data(True, False, 'Nonboolean') + @mock.patch('cinderclient.utils.find_resource') + def test_snapshot_create_pre_3_66(self, force_value, mock_find_vol): + mock_find_vol.return_value = volumes.Volume( + self, {'id': '123456'}, loaded=True) + snap_body_3_65 = { + 'snapshot': { + 'volume_id': '123456', + 'force': f'{force_value}', + 'name': None, + 'description': None, + 'metadata': {} + } + } + self.run_command('--os-volume-api-version 3.65 ' + f'snapshot-create --force {force_value} 123456') + self.assert_called_anytime('POST', '/snapshots', body=snap_body_3_65) + + SNAP_BODY_3_66 = { + 'snapshot': { + 'volume_id': '123456', + 'name': None, + 'description': None, + 'metadata': {} + } + } + + @ddt.data(True, 'true', 'on', '1') + @mock.patch('cinderclient.utils.find_resource') + def test_snapshot_create_3_66_with_force_true(self, f_val, mock_find_vol): + mock_find_vol.return_value = volumes.Volume( + self, {'id': '123456'}, loaded=True) + mock_find_vol.return_value = volumes.Volume(self, + {'id': '123456'}, + loaded=True) + self.run_command('--os-volume-api-version 3.66 ' + f'snapshot-create --force {f_val} 123456') + self.assert_called_anytime('POST', '/snapshots', + body=self.SNAP_BODY_3_66) + + @ddt.data(False, 'false', 'no', '0', 'whatever') + @mock.patch('cinderclient.utils.find_resource') + def test_snapshot_create_3_66_with_force_not_true( + self, f_val, mock_find_vol): + mock_find_vol.return_value = volumes.Volume( + self, {'id': '123456'}, loaded=True) + uae = self.assertRaises(exceptions.UnsupportedAttribute, + self.run_command, + '--os-volume-api-version 3.66 ' + f'snapshot-create --force {f_val} 123456') + self.assertIn('not allowed after microversion 3.65', str(uae)) + + @mock.patch('cinderclient.utils.find_resource') + def test_snapshot_create_3_66(self, mock_find_vol): + mock_find_vol.return_value = volumes.Volume( + self, {'id': '123456'}, loaded=True) + self.run_command('--os-volume-api-version 3.66 ' + 'snapshot-create 123456') + self.assert_called_anytime('POST', '/snapshots', + body=self.SNAP_BODY_3_66) + def test_snapshot_manageable_list(self): self.run_command('--os-volume-api-version 3.8 ' 'snapshot-manageable-list fakehost') diff --git a/cinderclient/v3/shell.py b/cinderclient/v3/shell.py index 195d859..18f94d3 100644 --- a/cinderclient/v3/shell.py +++ b/cinderclient/v3/shell.py @@ -2193,6 +2193,144 @@ def do_snapshot_list(cs, args): AppendFilters.filters = [] +@api_versions.wraps("3.0", "3.65") +@utils.arg('volume', + metavar='<volume>', + help='Name or ID of volume to snapshot.') +@utils.arg('--force', + metavar='<True|False>', + const=True, + nargs='?', + default=False, + end_version='3.65', + help='Allows or disallows snapshot of ' + 'a volume when the volume is attached to an instance. ' + 'If set to True, ignores the current status of the ' + 'volume when attempting to snapshot it rather ' + 'than forcing it to be available. From microversion 3.66, ' + 'all snapshots are "forced" and this option is invalid. ' + 'Default=False.') +@utils.arg('--force', + metavar='<True>', + nargs='?', + default=None, + start_version='3.66', + help=argparse.SUPPRESS) +@utils.arg('--name', + metavar='<name>', + default=None, + help='Snapshot name. Default=None.') +@utils.arg('--display-name', + help=argparse.SUPPRESS) +@utils.arg('--display_name', + help=argparse.SUPPRESS) +@utils.arg('--description', + metavar='<description>', + default=None, + help='Snapshot description. Default=None.') +@utils.arg('--display-description', + help=argparse.SUPPRESS) +@utils.arg('--display_description', + help=argparse.SUPPRESS) +@utils.arg('--metadata', + nargs='*', + metavar='<key=value>', + default=None, + help='Snapshot metadata key and value pairs. Default=None.') +def do_snapshot_create(cs, args): + """Creates a snapshot.""" + if args.display_name is not None: + args.name = args.display_name + + if args.display_description is not None: + args.description = args.display_description + + snapshot_metadata = None + if args.metadata is not None: + snapshot_metadata = shell_utils.extract_metadata(args) + + volume = utils.find_volume(cs, args.volume) + snapshot = cs.volume_snapshots.create(volume.id, + args.force, + args.name, + args.description, + metadata=snapshot_metadata) + shell_utils.print_volume_snapshot(snapshot) + + +@api_versions.wraps("3.66") +@utils.arg('volume', + metavar='<volume>', + help='Name or ID of volume to snapshot.') +@utils.arg('--force', + nargs='?', + help=argparse.SUPPRESS) +@utils.arg('--name', + metavar='<name>', + default=None, + help='Snapshot name. Default=None.') +@utils.arg('--display-name', + help=argparse.SUPPRESS) +@utils.arg('--display_name', + help=argparse.SUPPRESS) +@utils.arg('--description', + metavar='<description>', + default=None, + help='Snapshot description. Default=None.') +@utils.arg('--display-description', + help=argparse.SUPPRESS) +@utils.arg('--display_description', + help=argparse.SUPPRESS) +@utils.arg('--metadata', + nargs='*', + metavar='<key=value>', + default=None, + help='Snapshot metadata key and value pairs. Default=None.') +def do_snapshot_create(cs, args): # noqa: F811 + """Creates a snapshot.""" + + # TODO(rosmaita): we really need to look into removing this v1 + # compatibility code and the v1 options entirely. Note that if you + # include the --name and also --display_name, the latter will be used. + # Not sure that's desirable, but it is consistent with all the other + # functions in this file, so we'll do it here too. + if args.display_name is not None: + args.name = args.display_name + if args.display_description is not None: + args.description = args.display_description + + snapshot_metadata = None + if args.metadata is not None: + snapshot_metadata = shell_utils.extract_metadata(args) + + force = getattr(args, 'force', None) + + volume = utils.find_volume(cs, args.volume) + + # this is a little weird, but for consistency with the API we + # will silently ignore the --force option when it's passed with + # a value that evaluates to True; otherwise, we report that the + # --force option is illegal for this call + try: + snapshot = cs.volume_snapshots.create(volume.id, + force=force, + name=args.name, + description=args.description, + metadata=snapshot_metadata) + except ValueError as ve: + # make sure it's the exception we expect + em = cinderclient.v3.volume_snapshots.MV_3_66_FORCE_FLAG_ERROR + if em == str(ve): + raise exceptions.UnsupportedAttribute( + 'force', + start_version=None, + end_version=api_versions.APIVersion('3.65')) + else: + raise + + shell_utils.print_volume_snapshot(snapshot) + + @api_versions.wraps('3.27') @utils.arg('--all-tenants', dest='all_tenants', diff --git a/cinderclient/v3/shell_base.py b/cinderclient/v3/shell_base.py index 4dc6da9..0fc1b5d 100644 --- a/cinderclient/v3/shell_base.py +++ b/cinderclient/v3/shell_base.py @@ -595,62 +595,6 @@ def do_snapshot_show(cs, args): shell_utils.print_volume_snapshot(snapshot) -@utils.arg('volume', - metavar='<volume>', - help='Name or ID of volume to snapshot.') -@utils.arg('--force', - metavar='<True|False>', - const=True, - nargs='?', - default=False, - help='Allows or disallows snapshot of ' - 'a volume when the volume is attached to an instance. ' - 'If set to True, ignores the current status of the ' - 'volume when attempting to snapshot it rather ' - 'than forcing it to be available. ' - 'Default=False.') -@utils.arg('--name', - metavar='<name>', - default=None, - help='Snapshot name. Default=None.') -@utils.arg('--display-name', - help=argparse.SUPPRESS) -@utils.arg('--display_name', - help=argparse.SUPPRESS) -@utils.arg('--description', - metavar='<description>', - default=None, - help='Snapshot description. Default=None.') -@utils.arg('--display-description', - help=argparse.SUPPRESS) -@utils.arg('--display_description', - help=argparse.SUPPRESS) -@utils.arg('--metadata', - nargs='*', - metavar='<key=value>', - default=None, - help='Snapshot metadata key and value pairs. Default=None.') -def do_snapshot_create(cs, args): - """Creates a snapshot.""" - if args.display_name is not None: - args.name = args.display_name - - if args.display_description is not None: - args.description = args.display_description - - snapshot_metadata = None - if args.metadata is not None: - snapshot_metadata = shell_utils.extract_metadata(args) - - volume = utils.find_volume(cs, args.volume) - snapshot = cs.volume_snapshots.create(volume.id, - args.force, - args.name, - args.description, - metadata=snapshot_metadata) - shell_utils.print_volume_snapshot(snapshot) - - @utils.arg('snapshot', metavar='<snapshot>', nargs='+', help='Name or ID of the snapshot(s) to delete.') diff --git a/cinderclient/v3/volume_snapshots.py b/cinderclient/v3/volume_snapshots.py index ce7f4e0..9a94422 100644 --- a/cinderclient/v3/volume_snapshots.py +++ b/cinderclient/v3/volume_snapshots.py @@ -15,10 +15,18 @@ """Volume snapshot interface (v3 extension).""" +from oslo_utils import strutils + from cinderclient import api_versions from cinderclient.apiclient import base as common_base from cinderclient import base +MV_3_66_FORCE_FLAG_ERROR = ( + "Since microversion 3.66 of the Block Storage API, the 'force' option is " + "invalid for this request. For backward compatibility, however, when the " + "'force' flag is passed with a value evaluating to True, it is silently " + "ignored.") + class Snapshot(base.Resource): """A Snapshot is a point-in-time snapshot of an openstack volume.""" @@ -80,6 +88,7 @@ class SnapshotManager(base.ManagerWithFind): """Manage :class:`Snapshot` resources.""" resource_class = Snapshot + @api_versions.wraps("3.0", "3.65") def create(self, volume_id, force=False, name=None, description=None, metadata=None): @@ -106,6 +115,42 @@ class SnapshotManager(base.ManagerWithFind): 'metadata': snapshot_metadata}} return self._create('/snapshots', body, 'snapshot') + @api_versions.wraps("3.66") + def create(self, volume_id, force=None, # noqa: F811 + name=None, description=None, metadata=None): + + """Creates a snapshot of the given volume. + + :param volume_id: The ID of the volume to snapshot. + :param force: This is technically not valid after mv 3.66, but the + API silently accepts force=True for backward compatibility, so this + function will, too + :param name: Name of the snapshot + :param description: Description of the snapshot + :param metadata: Metadata of the snapshot + :raises: ValueError if 'force' is not passed with a value that + evaluates to true + :rtype: :class:`Snapshot` + """ + + if metadata is None: + snapshot_metadata = {} + else: + snapshot_metadata = metadata + + body = {'snapshot': {'volume_id': volume_id, + 'name': name, + 'description': description, + 'metadata': snapshot_metadata}} + if force is not None: + try: + force = strutils.bool_from_string(force, strict=True) + if not force: + raise ValueError() + except ValueError: + raise ValueError(MV_3_66_FORCE_FLAG_ERROR) + return self._create('/snapshots', body, 'snapshot') + def get(self, snapshot_id): """Shows snapshot details. diff --git a/releasenotes/notes/support-bs-mv-3.66-5214deb20d164056.yaml b/releasenotes/notes/support-bs-mv-3.66-5214deb20d164056.yaml new file mode 100644 index 0000000..c4028b0 --- /dev/null +++ b/releasenotes/notes/support-bs-mv-3.66-5214deb20d164056.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + Adds support for Block Storage API version 3.66, which drops the + requirement of a 'force' flag to create a snapshot of an in-use + volume. Although the 'force' flag is invalid for the ``snapshot-create`` + call for API versions 3.66 and higher, for backward compatibility the + cinderclient follows the Block Storage API in silently ignoring the + flag when it is passed with a value that evaluates to True. |