Merge "Glance `image set` Resolve Fracturing"

3 files changed, 118 insertions, 9 deletions

diff --git a/doc/source/command-objects/image.rst b/doc/source/command-objects/image.rst
index 8e44f517..d2698b0b 100644
--- a/doc/source/command-objects/image.rst
+++ b/doc/source/command-objects/image.rst
@@ -212,7 +212,7 @@ Save an image locally
 image set
 ---------
 
-*Only supported for Image v1*
+*Image v1, v2*
 
 Set image properties
 
@@ -271,6 +271,8 @@ Set image properties
 
     Size of image data (in bytes)
 
+    *Image version 1 only.*
+
 .. option:: --protected
 
     Prevent image from being deleted
@@ -291,38 +293,94 @@ Set image properties
 
     Upload image to this store
 
+    *Image version 1 only.*
+
 .. option:: --location <image-url>
 
     Download image from an existing URL
 
+    *Image version 1 only.*
+
 .. option:: --copy-from <image-url>
 
     Copy image from the data store (similar to --location)
 
+    *Image version 1 only.*
+
 .. option:: --file <file>
 
     Upload image from local file
 
+    *Image version 1 only.*
+
 .. option:: --volume <volume>
 
     Update image with a volume
 
+    *Image version 1 only.*
+
 .. option:: --force
 
     Force image update if volume is in use (only meaningful with --volume)
 
+    *Image version 1 only.*
+
 .. option:: --checksum <checksum>
 
     Image hash used for verification
 
+    *Image version 1 only.*
+
 .. option:: --stdin
 
     Allow to read image data from standard input
 
+    *Image version 1 only.*
+
 .. option:: --property <key=value>
 
     Set a property on this image (repeat for multiple values)
 
+    *Image version 1 only.*
+
+.. option:: --architecture <architecture>
+
+    Operating system Architecture
+
+    .. versionadded:: 2
+
+.. option:: --ramdisk-id <ramdisk-id>
+
+    ID of image stored in Glance that should be used as
+    the ramdisk when booting an AMI-style image
+
+    .. versionadded:: 2
+
+.. option:: --os-distro <os-distro>
+
+    Common name of operating system distribution
+
+    .. versionadded:: 2
+
+.. option:: --os-version <os-version>
+
+    Operating system version as specified by the distributor
+
+    .. versionadded:: 2
+
+.. option:: --kernel-id <kernel-id>
+
+    ID of image in Glance that should be used as the
+    kernel when booting an AMI-style image
+
+    .. versionadded:: 2
+
+.. option:: --instance-uuid <instance_uuid>
+
+    ID of instance used to create this image
+
+    .. versionadded:: 2
+
 .. describe:: <image>
 
     Image to modify (name or ID)
diff --git a/openstackclient/image/v2/image.py b/openstackclient/image/v2/image.py
index 3e1c824f..fff26c02 100644
--- a/openstackclient/image/v2/image.py
+++ b/openstackclient/image/v2/image.py
@@ -553,9 +553,22 @@ class SetImage(show.ShowOne):
     """Set image properties"""
 
     log = logging.getLogger(__name__ + ".SetImage")
+    deadopts = ('size', 'store', 'location', 'copy-from', 'checksum')
 
     def get_parser(self, prog_name):
         parser = super(SetImage, self).get_parser(prog_name)
+        # TODO(bunting): There are additional arguments that v1 supported
+        # --size - does not exist in v2
+        # --store - does not exist in v2
+        # --location - maybe location add?
+        # --copy-from - does not exist in v2
+        # --file - should be able to upload file
+        # --volume - needs adding
+        # --force - needs adding
+        # --checksum - maybe could be done client side
+        # --stdin - could be implemented
+        # --property - needs adding
+        # --tags - needs adding
         parser.add_argument(
             "image",
             metavar="<image>",
@@ -571,12 +584,28 @@ class SetImage(show.ShowOne):
             metavar="<architecture>",
             help="Operating system Architecture"
         )
-        parser.add_argument(
+        protected_group = parser.add_mutually_exclusive_group()
+        protected_group.add_argument(
             "--protected",
-            dest="protected",
             action="store_true",
             help="Prevent image from being deleted"
         )
+        protected_group.add_argument(
+            "--unprotected",
+            action="store_true",
+            help="Allow image to be deleted (default)"
+        )
+        public_group = parser.add_mutually_exclusive_group()
+        public_group.add_argument(
+            "--public",
+            action="store_true",
+            help="Image is accessible to the public",
+        )
+        public_group.add_argument(
+            "--private",
+            action="store_true",
+            help="Image is inaccessible to the public (default)",
+        )
         parser.add_argument(
             "--instance-uuid",
             metavar="<instance_uuid>",
@@ -589,12 +618,11 @@ class SetImage(show.ShowOne):
             help="Minimum disk size needed to boot image, in gigabytes"
         )
         visibility_choices = ["public", "private"]
-        parser.add_argument(
+        public_group.add_argument(
             "--visibility",
             metavar="<visibility>",
             choices=visibility_choices,
-            help="Scope of image accessibility. Valid values: %s"
-                 % visibility_choices
+            help=argparse.SUPPRESS
         )
         help_msg = ("ID of image in Glance that should be used as the kernel"
                     " when booting an AMI-style image")
@@ -649,12 +677,25 @@ class SetImage(show.ShowOne):
             choices=container_choices,
             help=help_msg
         )
+        for deadopt in self.deadopts:
+            parser.add_argument(
+                "--%s" % deadopt,
+                metavar="<%s>" % deadopt,
+                dest=deadopt.replace('-', '_'),
+                help=argparse.SUPPRESS
+            )
         return parser
 
     def take_action(self, parsed_args):
         self.log.debug("take_action(%s)", parsed_args)
         image_client = self.app.client_manager.image
 
+        for deadopt in self.deadopts:
+            if getattr(parsed_args, deadopt.replace('-', '_'), None):
+                raise exceptions.CommandError(
+                    "ERROR: --%s was given, which is an Image v1 option"
+                    " that is no longer supported in Image v2" % deadopt)
+
         kwargs = {}
         copy_attrs = ('architecture', 'container_format', 'disk_format',
                       'file', 'kernel_id', 'locations', 'name',
@@ -668,10 +709,21 @@ class SetImage(show.ShowOne):
                     # Only include a value in kwargs for attributes that are
                     # actually present on the command line
                     kwargs[attr] = val
+
+        # 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
+        # a single value for the pair of options because the default must be
+        # to do nothing when no options are present as opposed to always
+        # setting a default.
         if parsed_args.protected:
             kwargs['protected'] = True
-        else:
+        if parsed_args.unprotected:
             kwargs['protected'] = False
+        if parsed_args.public:
+            kwargs['visibility'] = 'public'
+        if parsed_args.private:
+            kwargs['visibility'] = 'private'
 
         if not kwargs:
             self.log.warning("No arguments specified")
diff --git a/openstackclient/tests/image/v2/test_image.py b/openstackclient/tests/image/v2/test_image.py
index bb720d79..65d5e555 100644
--- a/openstackclient/tests/image/v2/test_image.py
+++ b/openstackclient/tests/image/v2/test_image.py
@@ -713,8 +713,7 @@ class TestImageSet(TestImage):
             'name': 'new-name',
             'owner': 'new-owner',
             'min_disk': 2,
-            'min_ram': 4,
-            'protected': False
+            'min_ram': 4
         }
         # ImageManager.update(image, **kwargs)
         self.images_mock.update.assert_called_with(