[Kimchi-devel] [PATCH v2] storagevolume: Use default value for param 'name' when appropriate
Crístian Viana
vianac at linux.vnet.ibm.com
Wed Sep 10 20:33:38 UTC 2014
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 should 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. If an unknown
volume creation method is used (i.e. if another method is added in the
future and this function is not updated accordingly), the default name will
be based on the current time.
Signed-off-by: Crístian Viana <vianac at linux.vnet.ibm.com>
---
src/kimchi/API.json | 2 --
src/kimchi/mockmodel.py | 24 ++++++++++++++++++-----
src/kimchi/model/storagevolumes.py | 32 ++++++++++++++++++++++++++-----
tests/test_model.py | 18 ++++++++++++------
tests/test_rest.py | 39 +++++++++++++++++++++-----------------
5 files changed, 80 insertions(+), 35 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..8b58c36 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,25 @@ 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')
+
+ if create_param == 'file':
+ name = os.path.basename(params['file'].filename)
+ elif create_param == 'url':
+ name = os.path.basename(params['url'])
+ else:
+ name = 'upload-%s' % int(time.time())
+ 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 4d0c231..8d5d697 100644
--- a/src/kimchi/model/storagevolumes.py
+++ b/src/kimchi/model/storagevolumes.py
@@ -19,6 +19,7 @@
import contextlib
import os
+import time
import urllib2
import libvirt
@@ -44,6 +45,9 @@ VOLUME_TYPE_MAP = {0: 'file',
READ_CHUNK_SIZE = 1048576 # 1 MiB
+REQUIRE_NAME_PARAMS = ['capacity']
+
+
class StorageVolumesModel(object):
def __init__(self, **kargs):
self.conn = kargs['conn']
@@ -53,7 +57,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 +65,30 @@ 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.
+ if create_param == 'file':
+ name = os.path.basename(params['file'].filename)
+ elif create_param == 'url':
+ name = os.path.basename(params['url'])
+ else:
+ name = 'upload-%s' % int(time.time())
+ 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..5774c82 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,23 @@ 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.assertEquals('COPYING', vol_name)
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 b63e990..c34b4d5 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -490,6 +490,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,
@@ -1028,16 +1036,17 @@ 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())
+ vol_name = task['target_uri'].split('/')[-1]
+ self.assertEquals('anyurl.wor.kz', vol_name)
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')
+ resp = self.request('/storagepools/pool-1/storagevolumes/%s' %
+ vol_name, '{}', 'GET')
self.assertEquals(200, resp.status)
# Now remove the StoragePool from mock model
@@ -1908,27 +1917,25 @@ class RestTests(unittest.TestCase):
return headers
with RollbackContext() as rollback:
- vol_name = 'COPYING'
- vol_path = os.path.join(paths.get_prefix(), vol_name)
+ vol_path = os.path.join(paths.get_prefix(), 'COPYING')
url = "https://%s:%s/storagepools/default/storagevolumes" % \
(host, ssl_port)
with open(vol_path, 'rb') as fd:
r = requests.post(url,
files={'file': fd},
- data={'name': vol_name},
verify=False,
headers=fake_auth_header())
self.assertEquals(r.status_code, 202)
- self._wait_task(r.json()['id'])
+ task = r.json()
+ self._wait_task(task['id'])
resp = self.request('/storagepools/default/storagevolumes/%s' %
- vol_name)
+ task['target_uri'].split('/')[-1])
self.assertEquals(200, resp.status)
# Create a file with 3M to upload
- vol_name = '3m-file'
- vol_path = os.path.join('/tmp', vol_name)
+ vol_path = '/tmp/3m-file'
with open(vol_path, 'wb') as fd:
fd.seek(3*1024*1024-1)
fd.write("\0")
@@ -1937,20 +1944,19 @@ class RestTests(unittest.TestCase):
with open(vol_path, 'rb') as fd:
r = requests.post(url,
files={'file': fd},
- data={'name': vol_name},
verify=False,
headers=fake_auth_header())
self.assertEquals(r.status_code, 202)
- self._wait_task(r.json()['id'])
+ task = r.json()
+ self._wait_task(task['id'])
resp = self.request('/storagepools/default/storagevolumes/%s' %
- vol_name)
+ task['target_uri'].split('/')[-1])
self.assertEquals(200, resp.status)
# Create a file with 5M to upload
# Max body size is set to 4M so the upload will fail with 413
- vol_name = '5m-file'
- vol_path = os.path.join('/tmp', vol_name)
+ vol_path = '/tmp/5m-file'
with open(vol_path, 'wb') as fd:
fd.seek(5*1024*1024-1)
fd.write("\0")
@@ -1959,7 +1965,6 @@ class RestTests(unittest.TestCase):
with open(vol_path, 'rb') as fd:
r = requests.post(url,
files={'file': fd},
- data={'name': vol_name},
verify=False,
headers=fake_auth_header())
--
1.9.3
More information about the Kimchi-devel
mailing list