From bd589778c287f1f1b2f7c2dcce7c917e49f956c3 Mon Sep 17 00:00:00 2001
From: Marek Aufart <maufart@redhat.com>
Date: Tue, 23 Jun 2015 15:11:06 +0200
Subject: [PATCH] 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
---
doc/source/backwards-incompatible.rst | 14 ++
doc/source/command-objects/image.rst | 40 ++++++
openstackclient/image/v1/image.py | 138 ++++++++++++++-----
openstackclient/tests/image/v1/test_image.py | 132 +++++++++---------
4 files changed, 221 insertions(+), 103 deletions(-)
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 <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)
diff --git a/openstackclient/image/v1/image.py b/openstackclient/image/v1/image.py
index d4d45fa287..2fa42a0414 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 {}, {}
+ # 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):