Move update code from image create command

Openstack image create command updates existing image (with same name) by
default. That might be confusing since glance allows create multiple
images with same names and may lead to unwanted image update by image
create command.

Image update code was moved from image create action to image set action.

BackwardsIncompatibleImpact

Change-Id: I1686c6544c366262efab9e33c066d5f8a667f707
Closes-Bug: #1461817

2 files changed, 169 insertions, 105 deletions

diff --git a/openstackclient/image/v1/image.py b/openstackclient/image/v1/image.py
index d4d45fa2..2fa42a04 100644
--- a/openstackclient/image/v1/image.py
+++ b/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 = 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)
+            image = image_client.images.create(**kwargs)
         finally:
             # Clean up open files - make sure data isn't a string
             if ('data' in kwargs and hasattr(kwargs['data'], 'close') and
@@ -560,6 +537,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):
@@ -568,7 +590,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)
@@ -591,20 +614,63 @@ class SetImage(show.ShowOne):
         if parsed_args.private:
             kwargs['is_public'] = False
 
-        if not kwargs:
-            self.log.warning('no arguments specified')
-            return {}, {}
-
-        image = utils.find_resource(
-            image_client.images,
-            parsed_args.image,
-        )
-
-        if image.properties and parsed_args.properties:
-            image.properties.update(kwargs['properties'])
-            kwargs['properties'] = image.properties
+        # Wrap the call to catch exceptions in order to close files
+        try:
+            image = utils.find_resource(
+                image_client.images,
+                parsed_args.image,
+            )
 
-        image = image_client.images.update(image.id, **kwargs)
+            if not parsed_args.location and not parsed_args.copy_from:
+                if parsed_args.volume:
+                    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:
+                image.properties.update(kwargs['properties'])
+                kwargs['properties'] = image.properties
+
+            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)
diff --git a/openstackclient/tests/image/v1/test_image.py b/openstackclient/tests/image/v1/test_image.py
index eec8cfa5..a79df8b4 100644
--- a/openstackclient/tests/image/v1/test_image.py
+++ b/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)
-        self.images_mock.get.assert_called_with(image_fakes.image_name)
+        # ImageManager.get(name) not to be called since update action exists
+        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):