openstack/python-openstackclient
Code Issues Proposed changes

Move update code from image create command

Browse Source 
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
This commit is contained in:
Marek Aufart 2015-06-23 15:11:06 +02:00
parent 2eb0f7287f
commit bd589778c2
4 changed files with 221 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
doc/source/backwards-incompatible.rst
View File

@ -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
==============

40
doc/source/command-objects/image.rst
View File

@ -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)

138
openstackclient/image/v1/image.py
View File

@ -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)

132
openstackclient/tests/image/v1/test_image.py
View File

@ -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):