Clean up Image v2 image set command

Make the Image v2 image set command meet at the intersection of the v1
image set command and the v2 image create command:

* Add visibility to the deadopts list and remove the option
* Put the options in the same order as v1 image set
* Make the help text match
* Add --properties
* Move the additional options that do not appear in either v1 image set or
  v2 image create after --property as they are really pre-defined properties
* Add tests for v2 image set to match v1 and then some
* Put the SetImage class in v2/image.py in alphabetical order

Change-Id: I102b914e8ad09a014f6fdd846c5766b6c2eaadb8

Author: Dean Troyer

doc/source/command-objects/image.rst

@@ -238,6 +238,12 @@ Set image properties
         [--checksum <checksum>]
         [--stdin]
         [--property <key=value> [...] ]
+        [--architecture <architecture>]
+        [--instance-id <instance-id>]
+        [--kernel-id <kernel-id>]
+        [--os-distro <os-distro>]
+        [--os-version <os-version>]
+        [--ramdisk-id <ramdisk-id>]
         <image>
 
 .. option:: --name <name>
@@ -258,14 +264,11 @@ Set image properties
 
 .. option:: --container-format <container-format>
 
-    Container format of image.
-    Acceptable formats: ['ami', 'ari', 'aki', 'bare', 'ovf']
+    Image container format (default: bare)
 
 .. option:: --disk-format <disk-format>
 
-    Disk format of image.
-    Acceptable formats: ['ami', 'ari', 'aki', 'vhd', 'vmdk', 'raw', 'qcow2',
-    'vdi', 'iso']
+    Image disk format (default: raw)
 
 .. option:: --size <size>
 
@@ -339,45 +342,41 @@ Set image properties
 
 .. option:: --property <key=value>
 
-    Set a property on this image (repeat for multiple values)
-
-    *Image version 1 only.*
+    Set a property on this image (repeat option to set multiple properties)
 
 .. option:: --architecture <architecture>
 
-    Operating system Architecture
+    Operating system architecture
 
     .. versionadded:: 2
 
-.. option:: --ramdisk-id <ramdisk-id>
+.. option:: --instance-id <instance-id>
 
-    ID of image stored in Glance that should be used as
-    the ramdisk when booting an AMI-style image
+    ID of server instance used to create this image
 
     .. versionadded:: 2
 
-.. option:: --os-distro <os-distro>
+.. option:: --kernel-id <kernel-id>
 
-    Common name of operating system distribution
+    ID of kernel image used to boot this disk image
 
     .. versionadded:: 2
 
-.. option:: --os-version <os-version>
+.. option:: --os-distro <os-distro>
 
-    Operating system version as specified by the distributor
+    Operating system distribution name
 
     .. versionadded:: 2
 
-.. option:: --kernel-id <kernel-id>
+.. option:: --os-version <os-version>
 
-    ID of image in Glance that should be used as the
-    kernel when booting an AMI-style image
+    Operating system distribution version
 
     .. versionadded:: 2
 
-.. option:: --instance-uuid <instance_uuid>
+.. option:: --ramdisk-id <ramdisk-id>
 
-    ID of instance used to create this image
+    ID of ramdisk image used to boot this disk image
 
     .. versionadded:: 2
 

openstackclient/image/v2/image.py

@@ -210,7 +210,7 @@ def get_parser(self, prog_name):
                 "--%s" % deadopt,
                 metavar="<%s>" % deadopt,
                 dest=deadopt.replace('-', '_'),
-                help=argparse.SUPPRESS
+                help=argparse.SUPPRESS,
             )
         return parser
 
@@ -522,38 +522,11 @@ def take_action(self, parsed_args):
         gc_utils.save_image(data, parsed_args.file)
 
 
-class ShowImage(show.ShowOne):
-    """Display image details"""
-
-    log = logging.getLogger(__name__ + ".ShowImage")
-
-    def get_parser(self, prog_name):
-        parser = super(ShowImage, self).get_parser(prog_name)
-        parser.add_argument(
-            "image",
-            metavar="<image>",
-            help="Image to display (name or ID)",
-        )
-        return parser
-
-    def take_action(self, parsed_args):
-        self.log.debug("take_action(%s)", parsed_args)
-
-        image_client = self.app.client_manager.image
-        image = utils.find_resource(
-            image_client.images,
-            parsed_args.image,
-        )
-
-        info = _format_image(image)
-        return zip(*sorted(six.iteritems(info)))
-
-
 class SetImage(show.ShowOne):
     """Set image properties"""
 
     log = logging.getLogger(__name__ + ".SetImage")
-    deadopts = ('size', 'store', 'location', 'copy-from', 'checksum')
+    deadopts = ('visibility',)
 
     def get_parser(self, prog_name):
         parser = super(SetImage, self).get_parser(prog_name)
@@ -567,7 +540,6 @@ def get_parser(self, prog_name):
         # --force - needs adding
         # --checksum - maybe could be done client side
         # --stdin - could be implemented
-        # --property - needs adding
         # --tags - needs adding
         parser.add_argument(
             "image",
@@ -580,20 +552,44 @@ def get_parser(self, prog_name):
             help="New image name"
         )
         parser.add_argument(
-            "--architecture",
-            metavar="<architecture>",
-            help="Operating system Architecture"
+            "--owner",
+            metavar="<project>",
+            help="New image owner project (name or ID)",
+        )
+        parser.add_argument(
+            "--min-disk",
+            type=int,
+            metavar="<disk-gb>",
+            help="Minimum disk size needed to boot image, in gigabytes"
+        )
+        parser.add_argument(
+            "--min-ram",
+            type=int,
+            metavar="<ram-mb>",
+            help="Minimum RAM size needed to boot image, in megabytes",
+        )
+        parser.add_argument(
+            "--container-format",
+            metavar="<container-format>",
+            help="Image container format "
+                 "(default: %s)" % DEFAULT_CONTAINER_FORMAT,
+        )
+        parser.add_argument(
+            "--disk-format",
+            metavar="<disk-format>",
+            help="Image disk format "
+                 "(default: %s)" % DEFAULT_DISK_FORMAT,
         )
         protected_group = parser.add_mutually_exclusive_group()
         protected_group.add_argument(
             "--protected",
             action="store_true",
-            help="Prevent image from being deleted"
+            help="Prevent image from being deleted",
         )
         protected_group.add_argument(
             "--unprotected",
             action="store_true",
-            help="Allow image to be deleted (default)"
+            help="Allow image to be deleted (default)",
         )
         public_group = parser.add_mutually_exclusive_group()
         public_group.add_argument(
@@ -607,82 +603,55 @@ def get_parser(self, prog_name):
             help="Image is inaccessible to the public (default)",
         )
         parser.add_argument(
-            "--instance-uuid",
-            metavar="<instance_uuid>",
-            help="ID of instance used to create this image"
+            "--property",
+            dest="properties",
+            metavar="<key=value>",
+            action=parseractions.KeyValueAction,
+            help="Set a property on this image "
+                 "(repeat option to set multiple properties)",
         )
         parser.add_argument(
-            "--min-disk",
-            type=int,
-            metavar="<disk-gb>",
-            help="Minimum disk size needed to boot image, in gigabytes"
-        )
-        visibility_choices = ["public", "private"]
-        public_group.add_argument(
-            "--visibility",
-            metavar="<visibility>",
-            choices=visibility_choices,
-            help=argparse.SUPPRESS
+            "--architecture",
+            metavar="<architecture>",
+            help="Operating system architecture",
         )
-        help_msg = ("ID of image in Glance that should be used as the kernel"
-                    " when booting an AMI-style image")
         parser.add_argument(
-            "--kernel-id",
-            metavar="<kernel-id>",
-            help=help_msg
+            "--instance-id",
+            metavar="<instance-id>",
+            help="ID of server instance used to create this image",
         )
         parser.add_argument(
-            "--os-version",
-            metavar="<os-version>",
-            help="Operating system version as specified by the distributor"
+            "--instance-uuid",
+            metavar="<instance-id>",
+            dest="instance_id",
+            help=argparse.SUPPRESS,
         )
-        disk_choices = ["None", "ami", "ari", "aki", "vhd", "vmdk", "raw",
-                        "qcow2", "vdi", "iso"]
-        help_msg = ("Format of the disk. Valid values: %s" % disk_choices)
         parser.add_argument(
-            "--disk-format",
-            metavar="<disk-format>",
-            choices=disk_choices,
-            help=help_msg
+            "--kernel-id",
+            metavar="<kernel-id>",
+            help="ID of kernel image used to boot this disk image",
         )
         parser.add_argument(
             "--os-distro",
             metavar="<os-distro>",
-            help="Common name of operating system distribution"
+            help="Operating system distribution name",
         )
         parser.add_argument(
-            "--owner",
-            metavar="<owner>",
-            help="New Owner of the image"
+            "--os-version",
+            metavar="<os-version>",
+            help="Operating system distribution version",
         )
-        msg = ("ID of image stored in Glance that should be used as the "
-               "ramdisk when booting an AMI-style image")
         parser.add_argument(
             "--ramdisk-id",
             metavar="<ramdisk-id>",
-            help=msg
-        )
-        parser.add_argument(
-            "--min-ram",
-            type=int,
-            metavar="<ram-mb>",
-            help="Amount of RAM (in MB) required to boot image"
-        )
-        container_choices = ["None", "ami", "ari", "aki", "bare", "ovf", "ova"]
-        help_msg = ("Format of the container. Valid values: %s"
-                    % container_choices)
-        parser.add_argument(
-            "--container-format",
-            metavar="<container-format>",
-            choices=container_choices,
-            help=help_msg
+            help="ID of ramdisk image used to boot this disk image",
         )
         for deadopt in self.deadopts:
             parser.add_argument(
                 "--%s" % deadopt,
                 metavar="<%s>" % deadopt,
                 dest=deadopt.replace('-', '_'),
-                help=argparse.SUPPRESS
+                help=argparse.SUPPRESS,
             )
         return parser
 
@@ -698,10 +667,9 @@ def take_action(self, parsed_args):
 
         kwargs = {}
         copy_attrs = ('architecture', 'container_format', 'disk_format',
-                      'file', 'kernel_id', 'locations', 'name',
+                      'file', 'instance_id', 'kernel_id', 'locations',
                       'min_disk', 'min_ram', 'name', 'os_distro', 'os_version',
-                      'owner', 'prefix', 'progress', 'ramdisk_id',
-                      'visibility')
+                      'owner', 'prefix', 'progress', 'ramdisk_id')
         for attr in copy_attrs:
             if attr in parsed_args:
                 val = getattr(parsed_args, attr, None)
@@ -710,6 +678,11 @@ def take_action(self, parsed_args):
                     # actually present on the command line
                     kwargs[attr] = val
 
+        # Properties should get flattened into the general kwargs
+        if getattr(parsed_args, 'properties', None):
+            for k, v in six.iteritems(parsed_args.properties):
+                kwargs[k] = str(v)
+
         # Handle exclusive booleans with care
         # Avoid including attributes in kwargs if an option is not
         # present on the command line.  These exclusive booleans are not
@@ -736,3 +709,30 @@ def take_action(self, parsed_args):
         info = {}
         info.update(image)
         return zip(*sorted(six.iteritems(info)))
+
+
+class ShowImage(show.ShowOne):
+    """Display image details"""
+
+    log = logging.getLogger(__name__ + ".ShowImage")
+
+    def get_parser(self, prog_name):
+        parser = super(ShowImage, self).get_parser(prog_name)
+        parser.add_argument(
+            "image",
+            metavar="<image>",
+            help="Image to display (name or ID)",
+        )
+        return parser
+
+    def take_action(self, parsed_args):
+        self.log.debug("take_action(%s)", parsed_args)
+
+        image_client = self.app.client_manager.image
+        image = utils.find_resource(
+            image_client.images,
+            parsed_args.image,
+        )
+
+        info = _format_image(image)
+        return zip(*sorted(six.iteritems(info)))