Support Block Storage API mv 3.66
Block Storage API mv 3.66 enables snapshots of in-use volumes without requiring a 'force' flag. For backward compatibility, the API silently accepts force=true, even though the 'force' flag is considered invalid for that call. That behavior is replicated in the client, where --force with a true value is silently accepted. The --force option is not advertised in the shell and an option value that doesn't evaluate to true raises an UnsupportedAttribute error. Similar behavior from the v3 Snapshot class, except it raises a ValueError under similar circumstances. Change-Id: I7408d0e3a5ed7f4cbcaf65cf3434ad60aaed511d
This commit is contained in:
Brian Rosmaita
parent
45cebe406f
commit
c3c15f6cb2
|
|
@ -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 = {}
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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.')
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
Loading…
Reference in New Issue