[Kimchi-devel] [PATCH] storagevolume: Use default value for param 'name' when appropriate
Paulo Ricardo Paz Vital
pvital at linux.vnet.ibm.com
Tue Sep 9 21:41:13 UTC 2014
--
Reviewed-by: Paulo Vital <pvital at linux.vnet.ibm.com>
On Mon, 2014-09-08 at 16:22 -0300, Crístian Viana wrote:
> The parameter 'name' is only required when creating storage volumes with
> the parameter 'capacity' (i.e. an empty volume). When creating volumes
> with 'url' or 'file', the volume name will have a default value if 'name'
> is not specified.
>
> Use a default name when creating storage volumes from a file or from a URL.
> The default name will be the file's or the URL's basename.
>
> Signed-off-by: Crístian Viana <vianac at linux.vnet.ibm.com>
> ---
> src/kimchi/API.json | 2 --
> src/kimchi/mockmodel.py | 19 ++++++++++++++-----
> src/kimchi/model/storagevolumes.py | 26 +++++++++++++++++++++-----
> tests/test_model.py | 17 +++++++++++------
> tests/test_rest.py | 16 ++++++++++++----
> 5 files changed, 58 insertions(+), 22 deletions(-)
>
> diff --git a/src/kimchi/API.json b/src/kimchi/API.json
> index 8a95804..1319531 100644
> --- a/src/kimchi/API.json
> +++ b/src/kimchi/API.json
> @@ -156,13 +156,11 @@
> },
> "storagevolumes_create": {
> "type": "object",
> - "error": "KCHVOL0016E",
> "properties": {
> "name": {
> "description": "The name of the Storage Volume",
> "type": "string",
> "minLength": 1,
> - "required": true,
> "error": "KCHVOL0013E"
> },
> "capacity": {
> diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
> index 15a75af..ef09a5b 100644
> --- a/src/kimchi/mockmodel.py
> +++ b/src/kimchi/mockmodel.py
> @@ -482,8 +482,9 @@ class MockModel(object):
>
> def storagevolumes_create(self, pool_name, params):
> vol_source = ['file', 'url', 'capacity']
> + require_name_params = ['capacity']
>
> - name = params['name']
> + name = params.get('name')
>
> index_list = list(i for i in range(len(vol_source))
> if vol_source[i] in params)
> @@ -491,12 +492,20 @@ class MockModel(object):
> raise InvalidParameter("KCHVOL0018E",
> {'param': ",".join(vol_source)})
>
> + create_param = vol_source[index_list[0]]
> +
> + if name is None:
> + if create_param in require_name_params:
> + raise InvalidParameter('KCHVOL0016E')
> +
> + name = os.path.basename(create_param)
> + params['name'] = name
> +
> try:
> - create_func = getattr(self, "_create_volume_with_" +
> - vol_source[index_list[0]])
> + create_func = getattr(self, '_create_volume_with_%s' %
> + create_param)
> except AttributeError:
> - raise InvalidParameter("KCHVOL0019E",
> - {'param': vol_source[index_list[0]]})
> + raise InvalidParameter("KCHVOL0019E", {'param': create_param})
>
> pool = self._get_storagepool(pool_name)
> if pool.info['type'] in READONLY_POOL_TYPE:
> diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py
> index ef8e684..ec610ac 100644
> --- a/src/kimchi/model/storagevolumes.py
> +++ b/src/kimchi/model/storagevolumes.py
> @@ -44,6 +44,9 @@ VOLUME_TYPE_MAP = {0: 'file',
> DOWNLOAD_CHUNK_SIZE = 1048576 # 1 MiB
>
>
> +REQUIRE_NAME_PARAMS = ['capacity']
> +
> +
> class StorageVolumesModel(object):
> def __init__(self, **kargs):
> self.conn = kargs['conn']
> @@ -53,7 +56,7 @@ class StorageVolumesModel(object):
> def create(self, pool_name, params):
> vol_source = ['file', 'url', 'capacity']
>
> - name = params['name']
> + name = params.get('name')
>
> index_list = list(i for i in range(len(vol_source))
> if vol_source[i] in params)
> @@ -61,12 +64,25 @@ class StorageVolumesModel(object):
> raise InvalidParameter("KCHVOL0018E",
> {'param': ",".join(vol_source)})
>
> + create_param = vol_source[index_list[0]]
> +
> + if name is None:
> + # the methods listed in 'REQUIRE_NAME_PARAMS' cannot have
> + # 'name' == None
> + if create_param in REQUIRE_NAME_PARAMS:
> + raise InvalidParameter('KCHVOL0016E')
> +
> + # if 'name' is omitted - except for the methods listed in
> + # 'REQUIRE_NAME_PARAMS' - the default volume name will be the
> + # file/URL basename.
> + name = os.path.basename(create_param)
> + params['name'] = name
> +
> try:
> - create_func = getattr(self, "_create_volume_with_" +
> - vol_source[index_list[0]])
> + create_func = getattr(self, '_create_volume_with_%s' %
> + create_param)
> except AttributeError:
> - raise InvalidParameter("KCHVOL0019E",
> - {'param': vol_source[index_list[0]]})
> + raise InvalidParameter("KCHVOL0019E", {'param': create_param})
>
> pool_info = StoragePoolModel(conn=self.conn,
> objstore=self.objstore).lookup(pool_name)
> diff --git a/tests/test_model.py b/tests/test_model.py
> index 4e9ba97..ecb0a8a 100644
> --- a/tests/test_model.py
> +++ b/tests/test_model.py
> @@ -526,10 +526,13 @@ class ModelTests(unittest.TestCase):
>
> vols = inst.storagevolumes_get_list(pool)
> num = len(vols) + 2
> - params = {'name': vol,
> - 'capacity': 1024,
> + params = {'capacity': 1024,
> 'allocation': 512,
> 'format': 'raw'}
> + # 'name' is required for this type of volume
> + self.assertRaises(InvalidParameter, inst.storagevolumes_create,
> + pool, params)
> + params['name'] = vol
> task_id = inst.storagevolumes_create(pool, params)['id']
> self._wait_task(inst, task_id)
> self.assertEquals('finished', inst.task_lookup(task_id)['status'])
> @@ -560,20 +563,22 @@ class ModelTests(unittest.TestCase):
>
> # download remote volume
> # 1) try an invalid URL
> - params = {'name': 'foo', 'url': 'http://www.invalid.url'}
> + params = {'url': 'http://www.invalid.url'}
> taskid = inst.storagevolumes_create(pool, params)['id']
> self._wait_task(inst, taskid)
> self.assertEquals('failed', inst.task_lookup(taskid)['status'])
> # 2) download Kimchi's "COPYING" from Github and compare its
> # content to the corresponding local file's
> url = 'https://github.com/kimchi-project/kimchi/raw/master/COPYING'
> - params = {'name': 'copying', 'url': url}
> - taskid = inst.storagevolumes_create(pool, params)['id']
> + params = {'url': url}
> + task_response = inst.storagevolumes_create(pool, params)
> + taskid = task_response['id']
> + vol_name = task_response['target_uri'].split('/')[-1]
> self._wait_task(inst, taskid)
> self.assertEquals('finished', inst.task_lookup(taskid)['status'])
> rollback.prependDefer(inst.storagevolume_delete, pool,
> params['name'])
> - vol_path = os.path.join(args['path'], params['name'])
> + vol_path = os.path.join(args['path'], vol_name)
> self.assertTrue(os.path.isfile(vol_path))
> with open(vol_path) as vol_file:
> vol_content = vol_file.read()
> diff --git a/tests/test_rest.py b/tests/test_rest.py
> index ae1c971..a3dcf87 100644
> --- a/tests/test_rest.py
> +++ b/tests/test_rest.py
> @@ -489,6 +489,14 @@ class RestTests(unittest.TestCase):
> resp = self.request('/storagepools/tmp/activate', req, 'POST')
> self.assertEquals(200, resp.status)
>
> + # 'name' is required for this type of volume
> + req = json.dumps({'capacity': 1024,
> + 'allocation': 512,
> + 'type': 'disk',
> + 'format': 'raw'})
> + resp = self.request('/storagepools/tmp/storagevolumes',
> + req, 'POST')
> + self.assertEquals(400, resp.status)
> req = json.dumps({'name': "attach-volume",
> 'capacity': 1024,
> 'allocation': 512,
> @@ -1027,16 +1035,16 @@ class RestTests(unittest.TestCase):
> self.assertEquals('/var/lib/libvirt/images/volume-1',
> storagevolume['path'])
>
> - req = json.dumps({'name': 'downloaded',
> - 'url': 'https://anyurl.wor.kz'})
> + req = json.dumps({'url': 'https://anyurl.wor.kz'})
> resp = self.request('/storagepools/pool-1/storagevolumes', req, 'POST')
> self.assertEquals(202, resp.status)
> task = json.loads(resp.read())
> self._wait_task(task['id'])
> task = json.loads(self.request('/tasks/%s' % task['id']).read())
> self.assertEquals('finished', task['status'])
> - resp = self.request('/storagepools/pool-1/storagevolumes/downloaded',
> - '{}', 'GET')
> + vol_name = task['target_uri'].split('/')[-1]
> + resp = self.request('/storagepools/pool-1/storagevolumes/%s' %
> + vol_name, '{}', 'GET')
> self.assertEquals(200, resp.status)
>
> # Now remove the StoragePool from mock model
More information about the Kimchi-devel
mailing list