9:22 p.m.
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(a)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
--
1.9.3