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

3 files changed, 183 insertions, 56 deletions

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.