openstack
/
python-glanceclient
Code Issues Proposed changes

Require disk and container format on image-create 

Currently glanceclient doesn't enforce disk-format or container-format
presence in the command line on image-create when providing image data
(with --file, --location, --copy-from), which means that the POST
request is made with the whole image and error is reported by Glance
API.

This post enforces presence of those arguments when image data is
provided so we can get the error quicker and avoid sending unnecessary
data over the network.

Change-Id: I5914fa9cfef190a028b374005adbf3a804b1b677
Closes-Bug: #1309272
This commit is contained in:
Gorka Eguileor 2015-03-13 17:47:09 +01:00 committed by Flavio Percoco
parent 13af690396
commit 8d118ccedc
5 changed files with 198 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
glanceclient/common/utils.py
View File

@ -36,11 +36,15 @@ from oslo_utils import encodeutils
from oslo_utils import strutils
import prettytable
from glanceclient import _i18n
from glanceclient import exc
_ = _i18n._
_memoized_property_lock = threading.Lock()
SENSITIVE_HEADERS = ('X-Auth-Token', )
REQUIRED_FIELDS_ON_DATA = ('disk_format', 'container_format')
# Decorator for cli-args
@ -53,6 +57,47 @@ def arg(*args, **kwargs):
return _decorator
def on_data_require_fields(data_fields, required=REQUIRED_FIELDS_ON_DATA):
"""Decorator to check commands' validity
This decorator checks that required fields are present when image
data has been supplied via command line arguments or via stdin
On error throws CommandError exception with meaningful message.
:param data_fields: Which fields' presence imply image data
:type data_fields: iter
:param required: Required fields
:type required: iter
:return: function decorator
"""
def args_decorator(func):
def prepare_fields(fields):
args = ('--' + x.replace('_', '-') for x in fields)
return ', '.join(args)
def func_wrapper(gc, args):
# Set of arguments with data
fields = set(a[0] for a in vars(args).items() if a[1])
# Fields the conditional requirements depend on
present = fields.intersection(data_fields)
# How many conditional requirements are missing
missing = set(required) - fields
# We use get_data_file to check if data is provided in stdin
if (present or get_data_file(args)) and missing:
msg = (_("error: Must provide %(req)s when using %(opt)s.") %
{'req': prepare_fields(missing),
'opt': prepare_fields(present) or 'stdin'})
raise exc.CommandError(msg)
return func(gc, args)
return func_wrapper
return args_decorator
def schema_args(schema_getter, omit=None):
omit = omit or []
typemap = {

44
glanceclient/tests/unit/v1/test_shell.py
View File

@ -232,6 +232,9 @@ class ShellInvalidEndpointandParameterTest(utils.TestCase):
'OS_IMAGE_URL': 'http://is.invalid'}
self.shell = shell.OpenStackImagesShell()
self.patched = mock.patch('glanceclient.common.utils.get_data_file',
autospec=True, return_value=None)
self.mock_get_data_file = self.patched.start()
self.gc = self._mock_glance_client()
@ -254,6 +257,7 @@ class ShellInvalidEndpointandParameterTest(utils.TestCase):
def tearDown(self):
super(ShellInvalidEndpointandParameterTest, self).tearDown()
os.environ = self.old_environment
self.patched.stop()
def run_command(self, cmd):
self.shell.main(cmd.split())
@ -323,6 +327,46 @@ class ShellInvalidEndpointandParameterTest(utils.TestCase):
SystemExit,
self.run_command, 'image-create --min-disk 10gb')
@mock.patch('sys.stderr')
def test_image_create_missing_disk_format(self, __):
# We test for all possible sources
for origin in ('--file', '--location', '--copy-from'):
e = self.assertRaises(exc.CommandError, self.run_command,
'--os-image-api-version 1 image-create ' +
origin + ' fake_src --container-format bare')
self.assertEqual('error: Must provide --disk-format when using '
+ origin + '.', e.message)
@mock.patch('sys.stderr')
def test_image_create_missing_container_format(self, __):
# We test for all possible sources
for origin in ('--file', '--location', '--copy-from'):
e = self.assertRaises(exc.CommandError, self.run_command,
'--os-image-api-version 1 image-create ' +
origin + ' fake_src --disk-format qcow2')
self.assertEqual('error: Must provide --container-format when '
'using ' + origin + '.', e.message)
@mock.patch('sys.stderr')
def test_image_create_missing_container_format_stdin_data(self, __):
# Fake that get_data_file method returns data
self.mock_get_data_file.return_value = six.StringIO()
e = self.assertRaises(exc.CommandError, self.run_command,
'--os-image-api-version 1 image-create'
' --disk-format qcow2')
self.assertEqual('error: Must provide --container-format when '
'using stdin.', e.message)
@mock.patch('sys.stderr')
def test_image_create_missing_disk_format_stdin_data(self, __):
# Fake that get_data_file method returns data
self.mock_get_data_file.return_value = six.StringIO()
e = self.assertRaises(exc.CommandError, self.run_command,
'--os-image-api-version 1 image-create'
' --container-format bare')
self.assertEqual('error: Must provide --disk-format when using stdin.',
e.message)
@mock.patch('sys.stderr')
def test_image_update_invalid_size_parameter(self, __):
self.assertRaises(

107
glanceclient/tests/unit/v2/test_shell_v2.py
View File

@ -16,18 +16,73 @@
import json
import mock
import os
import six
import tempfile
import testtools
from glanceclient.common import utils
from glanceclient import exc
from glanceclient import shell
# NOTE(geguileo): This is very nasty, but I can't find a better way to set
# command line arguments in glanceclient.v2.shell.do_image_create that are
# set by decorator utils.schema_args while preserving the spirits of the test
# Backup original decorator
original_schema_args = utils.schema_args
# Set our own decorator that calls the original but with simulated schema
def schema_args(schema_getter, omit=None):
global original_schema_args
# We only add the 2 arguments that are required by image-create
my_schema_getter = lambda: {
'properties': {
'container_format': {
'enum': [None, 'ami', 'ari', 'aki', 'bare', 'ovf', 'ova'],
'type': 'string',
'description': 'Format of the container'},
'disk_format': {
'enum': [None, 'ami', 'ari', 'aki', 'vhd', 'vmdk', 'raw',
'qcow2', 'vdi', 'iso'],
'type': 'string',
'description': 'Format of the disk'},
'location': {'type': 'string'},
'locations': {'type': 'string'},
'copy_from': {'type': 'string'}}}
return original_schema_args(my_schema_getter, omit)
utils.schema_args = schema_args
from glanceclient.v2 import shell as test_shell
# Return original decorator.
utils.schema_args = original_schema_args
class ShellV2Test(testtools.TestCase):
def setUp(self):
super(ShellV2Test, self).setUp()
self._mock_utils()
self.gc = self._mock_glance_client()
self.shell = shell.OpenStackImagesShell()
os.environ = {
'OS_USERNAME': 'username',
'OS_PASSWORD': 'password',
'OS_TENANT_ID': 'tenant_id',
'OS_TOKEN_ID': 'test',
'OS_AUTH_URL': 'http://127.0.0.1:5000/v2.0/',
'OS_AUTH_TOKEN': 'pass',
'OS_IMAGE_API_VERSION': '1',
'OS_REGION_NAME': 'test',
'OS_IMAGE_URL': 'http://is.invalid'}
self.shell = shell.OpenStackImagesShell()
self.patched = mock.patch('glanceclient.common.utils.get_data_file',
autospec=True, return_value=None)
self.mock_get_data_file = self.patched.start()
def tearDown(self):
super(ShellV2Test, self).tearDown()
self.patched.stop()
def _make_args(self, args):
# NOTE(venkatesh): this conversion from a dict to an object
@ -59,6 +114,49 @@ class ShellV2Test(testtools.TestCase):
mocked_utils_exit.assert_called_once_with(err_msg)
def _run_command(self, cmd):
self.shell.main(cmd.split())
@mock.patch('sys.stderr')
def test_image_create_missing_disk_format(self, __):
# We test for all possible sources
for origin in ('--file', '--location', '--copy-from'):
e = self.assertRaises(exc.CommandError, self._run_command,
'--os-image-api-version 2 image-create ' +
origin + ' fake_src --container-format bare')
self.assertEqual('error: Must provide --disk-format when using '
+ origin + '.', e.message)
@mock.patch('sys.stderr')
def test_image_create_missing_container_format(self, __):
# We test for all possible sources
for origin in ('--file', '--location', '--copy-from'):
e = self.assertRaises(exc.CommandError, self._run_command,
'--os-image-api-version 2 image-create ' +
origin + ' fake_src --disk-format qcow2')
self.assertEqual('error: Must provide --container-format when '
'using ' + origin + '.', e.message)
@mock.patch('sys.stderr')
def test_image_create_missing_container_format_stdin_data(self, __):
# Fake that get_data_file method returns data
self.mock_get_data_file.return_value = six.StringIO()
e = self.assertRaises(exc.CommandError, self._run_command,
'--os-image-api-version 2 image-create'
' --disk-format qcow2')
self.assertEqual('error: Must provide --container-format when '
'using stdin.', e.message)
@mock.patch('sys.stderr')
def test_image_create_missing_disk_format_stdin_data(self, __):
# Fake that get_data_file method returns data
self.mock_get_data_file.return_value = six.StringIO()
e = self.assertRaises(exc.CommandError, self._run_command,
'--os-image-api-version 2 image-create'
' --container-format bare')
self.assertEqual('error: Must provide --disk-format when using stdin.',
e.message)
def test_do_image_list(self):
input = {
'limit': None,
@ -261,6 +359,7 @@ class ShellV2Test(testtools.TestCase):
'container_format': 'bare'})
def test_do_image_create_with_file(self):
self.mock_get_data_file.return_value = six.StringIO()
try:
file_name = None
with open(tempfile.mktemp(), 'w+') as f:
@ -305,7 +404,9 @@ class ShellV2Test(testtools.TestCase):
def test_do_image_create_with_user_props(self, mock_stdin):
args = self._make_args({'name': 'IMG-01',
'property': ['myprop=myval'],
'file': None})
'file': None,
'container_format': 'bare',
'disk_format': 'qcow2'})
with mock.patch.object(self.gc.images, 'create') as mocked_create:
ignore_fields = ['self', 'access', 'file', 'schema']
expect_image = dict([(field, field) for field in ignore_fields])
@ -320,7 +421,9 @@ class ShellV2Test(testtools.TestCase):
test_shell.do_image_create(self.gc, args)
mocked_create.assert_called_once_with(name='IMG-01',
myprop='myval')
myprop='myval',
container_format='bare',
disk_format='qcow2')
utils.print_dict.assert_called_once_with({
'id': 'pass', 'name': 'IMG-01', 'myprop': 'myval'})

2
glanceclient/v1/shell.py
View File

@ -32,6 +32,7 @@ import glanceclient.v1.images
CONTAINER_FORMATS = 'Acceptable formats: ami, ari, aki, bare, and ovf.'
DISK_FORMATS = ('Acceptable formats: ami, ari, aki, vhd, vmdk, raw, '
'qcow2, vdi, and iso.')
DATA_FIELDS = ('location', 'copy_from', 'file')
_bool_strict = functools.partial(strutils.bool_from_string, strict=True)
@ -221,6 +222,7 @@ def do_image_download(gc, args):
help='Print image size in a human-friendly format.')
@utils.arg('--progress', action='store_true', default=False,
help='Show upload progress bar.')
@utils.on_data_require_fields(DATA_FIELDS)
def do_image_create(gc, args):
"""Create a new image."""
# Filter out None values

2
glanceclient/v2/shell.py
View File

@ -27,6 +27,7 @@ import os
MEMBER_STATUS_VALUES = image_members.MEMBER_STATUS_VALUES
IMAGE_SCHEMA = None
DATA_FIELDS = ('location', 'copy_from', 'file')
def get_image_schema():
@ -55,6 +56,7 @@ def get_image_schema():
'to the client via stdin.')
@utils.arg('--progress', action='store_true', default=False,
help='Show upload progress bar.')
@utils.on_data_require_fields(DATA_FIELDS)
def do_image_create(gc, args):
"""Create a new image."""
schema = gc.schemas.get("image")