diff --git a/doc/source/backwards-incompatible.rst b/doc/source/backwards-incompatible.rst index 437f9324bb..ae77164b65 100644 --- a/doc/source/backwards-incompatible.rst +++ b/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 ============== diff --git a/doc/source/command-objects/image.rst b/doc/source/command-objects/image.rst index 18658651c3..824d4930e5 100644 --- a/doc/source/command-objects/image.rst +++ b/doc/source/command-objects/image.rst @@ -210,6 +210,14 @@ Set image properties [--size ] [--protected | --unprotected] [--public | --private] + [--store ] + [--location ] + [--copy-from ] + [--file ] + [--volume ] + [--force] + [--checksum ] + [--stdin] [--property [...] ] @@ -260,6 +268,38 @@ Set image properties Image is inaccessible to the public (default) +.. option:: --store + + Upload image to this store + +.. option:: --location + + Download image from an existing URL + +.. option:: --copy-from + + Copy image from the data store (similar to --location) + +.. option:: --file + + Upload image from local file + +.. option:: --volume + + Update image with a volume + +.. option:: --force + + Force image update if volume is in use (only meaningful with --volume) + +.. option:: --checksum + + Image hash used for verification + +.. option:: --stdin + + Allow to read image data from standard input + .. option:: --property Set a property on this image (repeat for multiple values) diff --git a/openstackclient/image/v1/image.py b/openstackclient/image/v1/image.py index 85a9e07683..68c81cd58d 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 @@ -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="", + help="Upload image to this store", + ) + parser.add_argument( + "--location", + metavar="", + help="Download image from an existing URL", + ) + parser.add_argument( + "--copy-from", + metavar="", + help="Copy image from the data store (similar to --location)", + ) + parser.add_argument( + "--file", + metavar="", + help="Upload image from local file", + ) + parser.add_argument( + "--volume", + metavar="", + 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="", + 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: - self.log.warning('no arguments specified') - return {}, {} + # Wrap the call to catch exceptions in order to close files + try: + image = utils.find_resource( + image_client.images, + parsed_args.image, + ) - image = utils.find_resource( - image_client.images, - parsed_args.image, - ) + 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 image.properties and parsed_args.properties: + image.properties.update(kwargs['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) diff --git a/openstackclient/tests/image/v1/test_image.py b/openstackclient/tests/image/v1/test_image.py index eec8cfa5c3..a79df8b4a1 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):