Merge "Move update code from image create command"

doc/source/backwards-incompatible.rst

@@ -47,6 +47,20 @@ List of Backwards Incompatible Changes
   * Bug: https://bugs.launchpad.net/python-openstackclient/+bug/1450872
   * Commit: https://review.openstack.org/#/c/179446/
 
+4. Command `openstack image create` does not update already existing image
+
+  Previously, the image create command updated already existing image if it had
+  same name. It disabled possibility to create multiple images with same name
+  and lead to potentially unwanted update of existing images by image create
+  command.
+  Now, update code was moved from create action to set action.
+
+  * In favor of: Create multiple images with same name (as glance does).
+  * As of: 1.5.0
+  * Removed in: NA
+  * Bug: https://bugs.launchpad.net/python-openstackclient/+bug/1461817
+  * Commit: https://review.openstack.org/#/c/194654/
+
 For Developers
 ==============
 

doc/source/command-objects/image.rst

@@ -210,6 +210,14 @@ Set image properties
         [--size <size>]
         [--protected | --unprotected]
         [--public | --private]
+        [--store <store>]
+        [--location <image-url>]
+        [--copy-from <image-url>]
+        [--file <file>]
+        [--volume <volume>]
+        [--force]
+        [--checksum <checksum>]
+        [--stdin]
         [--property <key=value> [...] ]
         <image>
 
@@ -260,6 +268,38 @@ Set image properties
 
     Image is inaccessible to the public (default)
 
+.. option:: --store <store>
+
+    Upload image to this store
+
+.. option:: --location <image-url>
+
+    Download image from an existing URL
+
+.. option:: --copy-from <image-url>
+
+    Copy image from the data store (similar to --location)
+
+.. option:: --file <file>
+
+    Upload image from local file
+
+.. option:: --volume <volume>
+
+    Update image with a volume
+
+.. option:: --force
+
+    Force image update if volume is in use (only meaningful with --volume)
+
+.. option:: --checksum <checksum>
+
+    Image hash used for verification
+
+.. option:: --stdin
+
+    Allow to read image data from standard input
+
 .. option:: --property <key=value>
 
     Set a property on this image (repeat for multiple values)

openstackclient/image/v1/image.py

@@ -33,7 +33,6 @@ from cliff import show
 
 from glanceclient.common import utils as gc_utils
 from openstackclient.api import utils as api_utils
-from openstackclient.common import exceptions
 from openstackclient.common import parseractions
 from openstackclient.common import utils
 
@@ -244,29 +243,7 @@ class CreateImage(show.ShowOne):
 
         # Wrap the call to catch exceptions in order to close files
         try:
-            try:
+            image = image_client.images.create(**kwargs)
-                image = utils.find_resource(
-                    image_client.images,
-                    parsed_args.name,
-                )
-
-                # Preserve previous properties if any are being set now
-                if image.properties:
-                    if parsed_args.properties:
-                        image.properties.update(kwargs['properties'])
-                    kwargs['properties'] = image.properties
-
-            except exceptions.CommandError:
-                if not parsed_args.volume:
-                    # This is normal for a create or reserve (create w/o
-                    # an image), but skip for create from volume
-                    image = image_client.images.create(**kwargs)
-            else:
-                # Update an existing reservation
-
-                # If an image is specified via --file, --location or
-                # --copy-from let the API handle it
-                image = image_client.images.update(image.id, **kwargs)
         finally:
             # Clean up open files - make sure data isn't a string
             if ('data' in kwargs and hasattr(kwargs['data'], 'close') and
@@ -561,6 +538,51 @@ class SetImage(show.ShowOne):
             help="Set a property on this image "
                  "(repeat option to set multiple properties)",
         )
+        parser.add_argument(
+            "--store",
+            metavar="<store>",
+            help="Upload image to this store",
+        )
+        parser.add_argument(
+            "--location",
+            metavar="<image-url>",
+            help="Download image from an existing URL",
+        )
+        parser.add_argument(
+            "--copy-from",
+            metavar="<image-url>",
+            help="Copy image from the data store (similar to --location)",
+        )
+        parser.add_argument(
+            "--file",
+            metavar="<file>",
+            help="Upload image from local file",
+        )
+        parser.add_argument(
+            "--volume",
+            metavar="<volume>",
+            help="Create image from a volume",
+        )
+        parser.add_argument(
+            "--force",
+            dest='force',
+            action='store_true',
+            default=False,
+            help="Force image change if volume is in use "
+            "(only meaningful with --volume)",
+        )
+        parser.add_argument(
+            "--stdin",
+            dest='stdin',
+            action='store_true',
+            default=False,
+            help="Read image data from standard input",
+        )
+        parser.add_argument(
+            "--checksum",
+            metavar="<checksum>",
+            help="Image hash used for verification",
+        )
         return parser
 
     def take_action(self, parsed_args):
@@ -569,7 +591,8 @@ class SetImage(show.ShowOne):
 
         kwargs = {}
         copy_attrs = ('name', 'owner', 'min_disk', 'min_ram', 'properties',
-                      'container_format', 'disk_format', 'size')
+                      'container_format', 'disk_format', 'size', 'store',
+                      'location', 'copy_from', 'volume', 'force', 'checksum')
         for attr in copy_attrs:
             if attr in parsed_args:
                 val = getattr(parsed_args, attr, None)
@@ -592,20 +615,63 @@ class SetImage(show.ShowOne):
         if parsed_args.private:
             kwargs['is_public'] = False
 
-        if not kwargs:
+        # Wrap the call to catch exceptions in order to close files
-            self.log.warning('no arguments specified')
+        try:
-            return {}, {}
+            image = utils.find_resource(
+                image_client.images,
+                parsed_args.image,
+            )
 
-        image = utils.find_resource(
+            if not parsed_args.location and not parsed_args.copy_from:
-            image_client.images,
+                if parsed_args.volume:
-            parsed_args.image,
+                    volume_client = self.app.client_manager.volume
-        )
+                    source_volume = utils.find_resource(
+                        volume_client.volumes,
+                        parsed_args.volume,
+                    )
+                    response, body = volume_client.volumes.upload_to_image(
+                        source_volume.id,
+                        parsed_args.force,
+                        parsed_args.image,
+                        (parsed_args.container_format
+                         if parsed_args.container_format
+                         else image.container_format),
+                        (parsed_args.disk_format
+                         if parsed_args.disk_format
+                         else image.disk_format),
+                    )
+                    info = body['os-volume_upload_image']
+                elif parsed_args.file:
+                    # Send an open file handle to glanceclient so it will
+                    # do a chunked transfer
+                    kwargs["data"] = io.open(parsed_args.file, "rb")
+                else:
+                    # Read file from stdin
+                    if sys.stdin.isatty() is not True:
+                        if parsed_args.stdin:
+                            if msvcrt:
+                                msvcrt.setmode(sys.stdin.fileno(), os.O_BINARY)
+                            # Send an open file handle to glanceclient so it
+                            # will do a chunked transfer
+                            kwargs["data"] = sys.stdin
+                        else:
+                            self.log.warning('Use --stdin to enable read image'
+                                             ' data from standard input')
 
-        if image.properties and parsed_args.properties:
+            if image.properties and parsed_args.properties:
-            image.properties.update(kwargs['properties'])
+                image.properties.update(kwargs['properties'])
-            kwargs['properties'] = image.properties
+                kwargs['properties'] = image.properties
 
-        image = image_client.images.update(image.id, **kwargs)
+            if not kwargs:
+                self.log.warning('no arguments specified')
+                return {}, {}
+
+            image = image_client.images.update(image.id, **kwargs)
+        finally:
+            # Clean up open files - make sure data isn't a string
+            if ('data' in kwargs and hasattr(kwargs['data'], 'close') and
+               kwargs['data'] != sys.stdin):
+                    kwargs['data'].close()
 
         info = {}
         info.update(image._info)

openstackclient/tests/image/v1/test_image.py

@@ -178,8 +178,8 @@ class TestImageCreate(TestImage):
         # Ensure the input file is closed
         mock_file.close.assert_called_with()
 
-        # ImageManager.get(name)
+        # ImageManager.get(name) not to be called since update action exists
-        self.images_mock.get.assert_called_with(image_fakes.image_name)
+        self.images_mock.get.assert_not_called()
 
         # ImageManager.create(name=, **)
         self.images_mock.create.assert_called_with(
@@ -201,71 +201,6 @@ class TestImageCreate(TestImage):
         self.assertEqual(image_fakes.IMAGE_columns, columns)
         self.assertEqual(image_fakes.IMAGE_data, data)
 
-    def test_image_create_volume(self):
-        # Set up VolumeManager Mock
-        volumes_mock = self.app.client_manager.volume.volumes
-        volumes_mock.reset_mock()
-        volumes_mock.get.return_value = fakes.FakeResource(
-            None,
-            copy.deepcopy({'id': 'vol1', 'name': 'volly'}),
-            loaded=True,
-        )
-        response = {
-            "id": 'volume_id',
-            "updated_at": 'updated_at',
-            "status": 'uploading',
-            "display_description": 'desc',
-            "size": 'size',
-            "volume_type": 'volume_type',
-            "image_id": 'image1',
-            "container_format": image.DEFAULT_CONTAINER_FORMAT,
-            "disk_format": image.DEFAULT_DISK_FORMAT,
-            "image_name": image_fakes.image_name,
-        }
-        full_response = {"os-volume_upload_image": response}
-        volumes_mock.upload_to_image.return_value = (201, full_response)
-
-        arglist = [
-            '--volume', 'volly',
-            image_fakes.image_name,
-        ]
-        verifylist = [
-            ('private', False),
-            ('protected', False),
-            ('public', False),
-            ('unprotected', False),
-            ('volume', 'volly'),
-            ('force', False),
-            ('name', image_fakes.image_name),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        # DisplayCommandBase.take_action() returns two tuples
-        columns, data = self.cmd.take_action(parsed_args)
-
-        # VolumeManager.upload_to_image(volume, force, image_name,
-        #     container_format, disk_format)
-        volumes_mock.upload_to_image.assert_called_with(
-            'vol1',
-            False,
-            image_fakes.image_name,
-            'bare',
-            'raw',
-        )
-
-        # ImageManager.update(image_id, remove_props=, **)
-        self.images_mock.update.assert_called_with(
-            image_fakes.image_id,
-            name=image_fakes.image_name,
-            container_format=image.DEFAULT_CONTAINER_FORMAT,
-            disk_format=image.DEFAULT_DISK_FORMAT,
-            properties=image_fakes.image_properties,
-            volume='volly',
-        )
-
-        self.assertEqual(image_fakes.IMAGE_columns, columns)
-        self.assertEqual(image_fakes.IMAGE_data, data)
-
 
 class TestImageDelete(TestImage):
 
@@ -669,6 +604,69 @@ class TestImageSet(TestImage):
             **kwargs
         )
 
+    def test_image_update_volume(self):
+        # Set up VolumeManager Mock
+        volumes_mock = self.app.client_manager.volume.volumes
+        volumes_mock.reset_mock()
+        volumes_mock.get.return_value = fakes.FakeResource(
+            None,
+            copy.deepcopy({'id': 'vol1', 'name': 'volly'}),
+            loaded=True,
+        )
+        response = {
+            "id": 'volume_id',
+            "updated_at": 'updated_at',
+            "status": 'uploading',
+            "display_description": 'desc',
+            "size": 'size',
+            "volume_type": 'volume_type',
+            "container_format": image.DEFAULT_CONTAINER_FORMAT,
+            "disk_format": image.DEFAULT_DISK_FORMAT,
+            "image": image_fakes.image_name,
+        }
+        full_response = {"os-volume_upload_image": response}
+        volumes_mock.upload_to_image.return_value = (201, full_response)
+
+        arglist = [
+            '--volume', 'volly',
+            '--name', 'updated_image',
+            image_fakes.image_name,
+        ]
+        verifylist = [
+            ('private', False),
+            ('protected', False),
+            ('public', False),
+            ('unprotected', False),
+            ('volume', 'volly'),
+            ('force', False),
+            ('name', 'updated_image'),
+            ('image', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # VolumeManager.upload_to_image(volume, force, image_name,
+        #     container_format, disk_format)
+        volumes_mock.upload_to_image.assert_called_with(
+            'vol1',
+            False,
+            image_fakes.image_name,
+            '',
+            '',
+        )
+
+        # ImageManager.update(image_id, remove_props=, **)
+        self.images_mock.update.assert_called_with(
+            image_fakes.image_id,
+            name='updated_image',
+            volume='volly',
+        )
+
+        self.assertEqual(image_fakes.IMAGE_columns, columns)
+        self.assertEqual(image_fakes.IMAGE_data, data)
+
 
 class TestImageShow(TestImage):