Fix up snapshot command

Several issues with the current snapshot command were resolved:

  * --long for list was added to include volume id/name, and properties
  * changed output from metadata to properties
  * added new option to set properties with 'snapshot set'
  * added new command to unset properties with 'snapshot unset'

Change-Id: I5902cfe876cefada701d4d658a50a4282ff300d6

openstackclient/volume/v1/snapshot.py

@@ -15,14 +15,15 @@
 
 """Volume v1 Snapshot action implementations"""
 
+import copy
 import logging
 import six
-import sys
 
 from cliff import command
 from cliff import lister
 from cliff import show
 
+from openstackclient.common import parseractions
 from openstackclient.common import utils
 
 
@@ -70,6 +71,10 @@ class CreateSnapshot(show.ShowOne):
             parsed_args.description
         )
 
+        snapshot._info.update(
+            {'properties': utils.format_dict(snapshot._info.pop('metadata'))}
+        )
+
         return zip(*sorted(six.iteritems(snapshot._info)))
 
 
@@ -103,20 +108,61 @@ class ListSnapshot(lister.Lister):
 
     log = logging.getLogger(__name__ + '.ListSnapshot')
 
+    def get_parser(self, prog_name):
+        parser = super(ListSnapshot, self).get_parser(prog_name)
+        parser.add_argument(
+            '--long',
+            action='store_true',
+            default=False,
+            help='List additional fields in output',
+        )
+        return parser
+
     def take_action(self, parsed_args):
         self.log.debug('take_action(%s)', parsed_args)
-        columns = (
+
-            'ID',
+        def _format_volume_id(volume_id):
-            'Display Name',
+            """Return a volume name if available
-            'Display Description',
+
-            'Status',
+            :param volume_id: a volume ID
-            'Size'
+            :rtype: either the volume ID or name
-        )
+            """
+
+            volume = volume_id
+            if volume_id in volume_cache.keys():
+                volume = volume_cache[volume_id].display_name
+            return volume
+
+        if parsed_args.long:
+            columns = ['ID', 'Display Name', 'Display Description', 'Status',
+                       'Size', 'Created At', 'Volume ID', 'Metadata']
+            column_headers = copy.deepcopy(columns)
+            column_headers[6] = 'Volume'
+            column_headers[7] = 'Properties'
+        else:
+            columns = ['ID', 'Display Name', 'Display Description', 'Status',
+                       'Size']
+            column_headers = copy.deepcopy(columns)
+
+        # Always update Name and Description
+        column_headers[1] = 'Name'
+        column_headers[2] = 'Description'
+
+        # Cache the volume list
+        volume_cache = {}
+        try:
+            for s in self.app.client_manager.volume.volumes.list():
+                volume_cache[s.id] = s
+        except Exception:
+            # Just forget it if there's any trouble
+            pass
+
         data = self.app.client_manager.volume.volume_snapshots.list()
-        return (columns,
+        return (column_headers,
                 (utils.get_item_properties(
                     s, columns,
-                    formatters={},
+                    formatters={'Metadata': utils.format_dict,
+                                'Volume ID': _format_volume_id},
                 ) for s in data))
 
 
@@ -133,12 +179,19 @@ class SetSnapshot(command.Command):
             help='Name or ID of snapshot to change')
         parser.add_argument(
             '--name',
-            metavar='<new-snapshot-name>',
+            metavar='<snapshot-name>',
             help='New snapshot name')
         parser.add_argument(
             '--description',
-            metavar='<new-snapshot-description>',
+            metavar='<snapshot-description>',
             help='New snapshot description')
+        parser.add_argument(
+            '--property',
+            metavar='<key=value>',
+            action=parseractions.KeyValueAction,
+            help='Property to add/change for this snapshot '
+                 '(repeat option to set multiple properties)',
+        )
         return parser
 
     def take_action(self, parsed_args):
@@ -146,15 +199,21 @@ class SetSnapshot(command.Command):
         volume_client = self.app.client_manager.volume
         snapshot = utils.find_resource(volume_client.volume_snapshots,
                                        parsed_args.snapshot)
+
+        if parsed_args.property:
+            volume_client.volume_snapshots.set_metadata(snapshot.id,
+                                                        parsed_args.property)
+
         kwargs = {}
         if parsed_args.name:
             kwargs['display_name'] = parsed_args.name
         if parsed_args.description:
             kwargs['display_description'] = parsed_args.description
 
-        if not kwargs:
+        if not kwargs and not parsed_args.property:
-            sys.stdout.write("Snapshot not updated, no arguments present")
+            self.app.log.error("No changes requested\n")
             return
+
         snapshot.update(**kwargs)
         return
 
@@ -178,4 +237,46 @@ class ShowSnapshot(show.ShowOne):
         snapshot = utils.find_resource(volume_client.volume_snapshots,
                                        parsed_args.snapshot)
 
+        snapshot._info.update(
+            {'properties': utils.format_dict(snapshot._info.pop('metadata'))}
+        )
+
         return zip(*sorted(six.iteritems(snapshot._info)))
+
+
+class UnsetSnapshot(command.Command):
+    """Unset snapshot properties"""
+
+    log = logging.getLogger(__name__ + '.UnsetSnapshot')
+
+    def get_parser(self, prog_name):
+        parser = super(UnsetSnapshot, self).get_parser(prog_name)
+        parser.add_argument(
+            'snapshot',
+            metavar='<snapshot>',
+            help='snapshot to change (name or ID)',
+        )
+        parser.add_argument(
+            '--property',
+            metavar='<key>',
+            action='append',
+            default=[],
+            help='Property key to remove from snapshot '
+                 '(repeat to remove multiple values)',
+        )
+        return parser
+
+    def take_action(self, parsed_args):
+        self.log.debug('take_action(%s)', parsed_args)
+        volume_client = self.app.client_manager.volume
+        snapshot = utils.find_resource(
+            volume_client.volume_snapshots, parsed_args.snapshot)
+
+        if parsed_args.property:
+            volume_client.volume_snapshots.delete_metadata(
+                snapshot.id,
+                parsed_args.property,
+            )
+        else:
+            self.app.log.error("No changes requested\n")
+        return

setup.cfg

@@ -326,6 +326,7 @@ openstack.volume.v1 =
     snapshot_list = openstackclient.volume.v1.snapshot:ListSnapshot
     snapshot_set = openstackclient.volume.v1.snapshot:SetSnapshot
     snapshot_show = openstackclient.volume.v1.snapshot:ShowSnapshot
+    snapshot_unset = openstackclient.volume.v1.snapshot:UnsetSnapshot
 
     volume_create = openstackclient.volume.v1.volume:CreateVolume
     volume_delete = openstackclient.volume.v1.volume:DeleteVolume