From ca783f46595a7d90cea0ad8491b65aa5f9370a04 Mon Sep 17 00:00:00 2001
From: Dean Troyer <dtroyer@gmail.com>
Date: Mon, 13 Oct 2014 22:40:11 -0500
Subject: [PATCH] Close files on image create
The file opened for --file was never closed. Close it if it is a
file object.
Change-Id: I7bd120a2413de42339771d01e8fd1894d38c3011
---
openstackclient/image/v1/image.py | 49 +++++++++++---------
openstackclient/tests/image/v1/test_image.py | 17 +++++--
2 files changed, 40 insertions(+), 26 deletions(-)
diff --git a/openstackclient/image/v1/image.py b/openstackclient/image/v1/image.py
index 465e9d7b15..32dd388c0f 100644
--- a/openstackclient/image/v1/image.py
+++ b/openstackclient/image/v1/image.py
@@ -15,6 +15,7 @@
"""Image V1 Action Implementations"""
+import io
import logging
import os
import six
@@ -214,10 +215,9 @@ class CreateImage(show.ShowOne):
elif parsed_args.file:
# Send an open file handle to glanceclient so it will
# do a chunked transfer
- kwargs["data"] = open(parsed_args.file, "rb")
+ kwargs["data"] = io.open(parsed_args.file, "rb")
else:
# Read file from stdin
- kwargs["data"] = None
if sys.stdin.isatty() is not True:
if msvcrt:
msvcrt.setmode(sys.stdin.fileno(), os.O_BINARY)
@@ -225,29 +225,36 @@ class CreateImage(show.ShowOne):
# do a chunked transfer
kwargs["data"] = sys.stdin
+ # Wrap the call to catch exceptions in order to close files
try:
- image = utils.find_resource(
- image_client.images,
- parsed_args.name,
- )
+ 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
+ # 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
+ 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)
+ # 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
+ 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 3f97b151c2..a05669300e 100644
--- a/openstackclient/tests/image/v1/test_image.py
+++ b/openstackclient/tests/image/v1/test_image.py
@@ -139,14 +139,17 @@ class TestImageCreate(TestImage):
self.assertEqual(image_fakes.IMAGE_columns, columns)
self.assertEqual(image_fakes.IMAGE_data, data)
- @mock.patch('six.moves.builtins.open')
- def test_image_create_file(self, open_mock):
+ @mock.patch('openstackclient.image.v1.image.io.open', name='Open')
+ def test_image_create_file(self, mock_open):
+ mock_file = mock.MagicMock(name='File')
+ mock_open.return_value = mock_file
+ mock_open.read.return_value = image_fakes.image_data
mock_exception = {
'find.side_effect': exceptions.CommandError('x'),
'get.side_effect': exceptions.CommandError('x'),
}
self.images_mock.configure_mock(**mock_exception)
- open_mock.return_value = image_fakes.image_data
+
arglist = [
'--file', 'filer',
'--unprotected',
@@ -169,7 +172,11 @@ class TestImageCreate(TestImage):
# DisplayCommandBase.take_action() returns two tuples
columns, data = self.cmd.take_action(parsed_args)
- open_mock.assert_called_with('filer', 'rb')
+ # Ensure input file is opened
+ mock_open.assert_called_with('filer', 'rb')
+
+ # 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)
@@ -185,7 +192,7 @@ class TestImageCreate(TestImage):
'Alpha': '1',
'Beta': '2',
},
- data=image_fakes.image_data,
+ data=mock_file,
)
# Verify update() was not called, if it was show the args