[PATCH 0/2 V2] Change storage volume creation to async tasks
V1 -> V2: - Fix "make check-local" issues - Update mockmodel to also return a Task while creating a storage volume - Update the jsonschema and API.md Royce Lv (2): Storage volume upload: Dispatch volume create to right handler Storage volume upload: Change storagevolumes to AsyncCollection docs/API.md | 1 + src/kimchi/API.json | 6 ++++++ src/kimchi/control/storagevolumes.py | 4 ++-- src/kimchi/i18n.py | 3 +++ src/kimchi/mockmodel.py | 22 +++++++++++++++++++++- src/kimchi/model/storagevolumes.py | 26 ++++++++++++++++++++++++-- tests/test_model.py | 8 ++++++-- tests/test_rest.py | 28 ++++++++++++++++------------ 8 files changed, 79 insertions(+), 19 deletions(-) -- 1.9.3
From: Royce Lv <lvroyce@linux.vnet.ibm.com> As we are starting to support upload and download to create volume, they need to be distinguished from previous creating through libvirt api. Adding a dispatcher to support this. Also update the jsonschema to add capacity parameter. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/API.json | 6 ++++++ src/kimchi/i18n.py | 3 +++ src/kimchi/mockmodel.py | 17 +++++++++++++++++ src/kimchi/model/storagevolumes.py | 17 +++++++++++++++++ 4 files changed, 43 insertions(+) diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 67612f2..2e2f9b0 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -165,6 +165,12 @@ "required": true, "error": "KCHVOL0013E" }, + "capacity": { + "description": "The total size (MiB) of the storage volume", + "type": "number", + "minimum": 1, + "error": "KCHVOL0020E" + }, "allocation": { "description": "The size(MiB) of allocation when create the storage volume", "type": "number", diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index c276b38..1e8b47a 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -185,6 +185,9 @@ messages = { "KCHVOL0015E": _("Storage volume format not supported"), "KCHVOL0016E": _("Storage volume requires a volume name"), "KCHVOL0017E": _("Unable to update database with storage volume information due error: %(err)s"), + "KCHVOL0018E": _("Only one of %(param)s can be specified"), + "KCHVOL0019E": _("Creating volume from %(param)s is not supported"), + "KCHVOL0020E": _("Storage volume capacity must be an integer number."), "KCHIFACE0001E": _("Interface %(name)s does not exist"), diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index e23c21a..f15e6c9 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -478,6 +478,23 @@ class MockModel(object): raise NotFoundError("KCHPOOL0002E", {'name': name}) def storagevolumes_create(self, pool_name, params): + vol_source = ['file', 'url', 'capacity'] + + index_list = list(i for i in range(len(vol_source)) + if vol_source[i] in params) + if len(index_list) != 1: + raise InvalidParameter("KCHVOL0018E", + {'param': ",".join(vol_source)}) + + try: + create_func = getattr(self, "_create_volume_with_" + + vol_source[index_list[0]]) + except AttributeError: + raise InvalidParameter("KCHVOL0019E", + {'param': vol_source[index_list[0]]}) + return create_func(pool_name, params) + + def _create_volume_with_capacity(self, pool_name, params): pool = self._get_storagepool(pool_name) if pool.info['state'] == 'inactive': raise InvalidOperation("KCHVOL0003E", diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index b60884c..8e44cab 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -44,6 +44,23 @@ class StorageVolumesModel(object): self.objstore = kargs['objstore'] def create(self, pool_name, params): + vol_source = ['file', 'url', 'capacity'] + + index_list = list(i for i in range(len(vol_source)) + if vol_source[i] in params) + if len(index_list) != 1: + raise InvalidParameter("KCHVOL0018E", + {'param': ",".join(vol_source)}) + + try: + create_func = getattr(self, "_create_volume_with_" + + vol_source[index_list[0]]) + except AttributeError: + raise InvalidParameter("KCHVOL0019E", + {'param': vol_source[index_list[0]]}) + return create_func(pool_name, params) + + def _create_volume_with_capacity(self, pool_name, params): vol_xml = """ <volume> <name>%(name)s</name> -- 1.9.3
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Because storage volume create will include upload and download, storagevolumes will not present before async task's finished. so change storage volumes to async collection. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- docs/API.md | 1 + src/kimchi/control/storagevolumes.py | 4 ++-- src/kimchi/mockmodel.py | 9 ++++++--- src/kimchi/model/storagevolumes.py | 13 +++++++++---- tests/test_model.py | 8 ++++++-- tests/test_rest.py | 28 ++++++++++++++++------------ 6 files changed, 40 insertions(+), 23 deletions(-) diff --git a/docs/API.md b/docs/API.md index 40a0c0b..298441f 100644 --- a/docs/API.md +++ b/docs/API.md @@ -412,6 +412,7 @@ A interface represents available network interface on VM. * **GET**: Retrieve a summarized list of all defined Storage Volumes in the defined Storage Pool * **POST**: Create a new Storage Volume in the Storage Pool + The return resource is a task resource * See Resource: Task * * name: The name of the Storage Volume * type: The type of the defined Storage Volume * capacity: The total space which can be used to store volumes diff --git a/src/kimchi/control/storagevolumes.py b/src/kimchi/control/storagevolumes.py index 327bf75..79170ee 100644 --- a/src/kimchi/control/storagevolumes.py +++ b/src/kimchi/control/storagevolumes.py @@ -18,11 +18,11 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA import kimchi.template -from kimchi.control.base import Collection, Resource +from kimchi.control.base import AsyncCollection, Collection, Resource from kimchi.control.utils import get_class_name, model_fn -class StorageVolumes(Collection): +class StorageVolumes(AsyncCollection): def __init__(self, model, pool): super(StorageVolumes, self).__init__(model) self.resource = StorageVolume diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index f15e6c9..05e3fa4 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -492,9 +492,12 @@ class MockModel(object): except AttributeError: raise InvalidParameter("KCHVOL0019E", {'param': vol_source[index_list[0]]}) - return create_func(pool_name, params) + params['pool'] = pool_name + taskid = self.add_task('', create_func, params) + return self.task_lookup(taskid) - def _create_volume_with_capacity(self, pool_name, params): + def _create_volume_with_capacity(self, cb, params): + pool_name = params.pop('pool') pool = self._get_storagepool(pool_name) if pool.info['state'] == 'inactive': raise InvalidOperation("KCHVOL0003E", @@ -519,7 +522,7 @@ class MockModel(object): raise InvalidOperation("KCHVOL0001E", {'name': name}) pool._volumes[name] = volume - return name + cb('OK', True) def storagevolume_lookup(self, pool, name): if self._get_storagepool(pool).info['state'] != 'active': diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 8e44cab..6b001f7 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -27,8 +27,9 @@ from kimchi.exception import InvalidOperation, InvalidParameter, IsoFormatError from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.isoinfo import IsoImage from kimchi.model.storagepools import StoragePoolModel -from kimchi.utils import kimchi_log +from kimchi.model.tasks import TaskModel from kimchi.model.vms import VMsModel, VMModel +from kimchi.utils import add_task, kimchi_log from kimchi.vmdisks import get_vm_disk, get_vm_disk_list @@ -42,6 +43,7 @@ class StorageVolumesModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] self.objstore = kargs['objstore'] + self.task = TaskModel(**kargs) def create(self, pool_name, params): vol_source = ['file', 'url', 'capacity'] @@ -58,9 +60,12 @@ class StorageVolumesModel(object): except AttributeError: raise InvalidParameter("KCHVOL0019E", {'param': vol_source[index_list[0]]}) - return create_func(pool_name, params) + params['pool'] = pool_name + taskid = add_task('', create_func, self.objstore, params) + return self.task.lookup(taskid) - def _create_volume_with_capacity(self, pool_name, params): + def _create_volume_with_capacity(self, cb, params): + pool_name = params.pop('pool') vol_xml = """ <volume> <name>%(name)s</name> @@ -105,7 +110,7 @@ class StorageVolumesModel(object): kimchi_log.warning('Unable to store storage volume id in ' 'objectstore due error: %s', e.message) - return name + cb('', True) def get_list(self, pool_name): pool = StoragePoolModel.get_storagepool(pool_name, self.conn) diff --git a/tests/test_model.py b/tests/test_model.py index 4eed8b3..5ee824d 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -118,7 +118,9 @@ class ModelTests(unittest.TestCase): 'capacity': 1024, 'allocation': 1, 'format': 'qcow2'} - inst.storagevolumes_create('default', params) + task_id = inst.storagevolumes_create('default', params)['id'] + self._wait_task(inst, task_id) + self.assertEquals('finished', inst.task_lookup(task_id)['status']) vol_path = inst.storagevolume_lookup('default', vol)['path'] rollback.prependDefer(inst.storagevolume_delete, 'default', vol) @@ -527,7 +529,9 @@ class ModelTests(unittest.TestCase): 'capacity': 1024, 'allocation': 512, 'format': 'raw'} - inst.storagevolumes_create(pool, params) + task_id = inst.storagevolumes_create(pool, params)['id'] + self._wait_task(inst, task_id) + self.assertEquals('finished', inst.task_lookup(task_id)['status']) rollback.prependDefer(inst.storagevolume_delete, pool, vol) fd, path = tempfile.mkstemp(dir=path) diff --git a/tests/test_rest.py b/tests/test_rest.py index 1bbdc47..9e14b6d 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -495,7 +495,8 @@ class RestTests(unittest.TestCase): 'format': 'raw'}) resp = self.request('/storagepools/tmp/storagevolumes', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + time.sleep(1) # Attach cdrom with both path and volume specified open('/tmp/existent.iso', 'w').close() @@ -1007,8 +1008,9 @@ class RestTests(unittest.TestCase): 'format': 'raw'}) resp = self.request('/storagepools/pool-1/storagevolumes', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + time.sleep(5) nr_vols = json.loads( self.request('/storagepools/pool-1').read())['nr_volumes'] self.assertEquals(5, nr_vols) @@ -1038,12 +1040,13 @@ class RestTests(unittest.TestCase): 'format': 'raw'}) resp = self.request('/storagepools/pool-2/storagevolumes/', req, 'POST') - self.assertEquals(400, resp.status) + self.assertEquals(202, resp.status) resp = self.request('/storagepools/pool-2/activate', '{}', 'POST') self.assertEquals(200, resp.status) resp = self.request('/storagepools/pool-2/storagevolumes/', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + time.sleep(1) # Verify the storage volume resp = self.request('/storagepools/pool-2/storagevolumes/test-volume') @@ -1510,16 +1513,17 @@ class RestTests(unittest.TestCase): time.sleep(1) def test_tasks(self): - model.add_task('', self._async_op) - model.add_task('', self._except_op) - model.add_task('', self._intermid_op) + id1 = model.add_task('', self._async_op) + id2 = model.add_task('', self._except_op) + id3 = model.add_task('', self._intermid_op) tasks = json.loads(self.request('/tasks').read()) - self.assertEquals(3, len(tasks)) - self._wait_task('2') - foo2 = json.loads(self.request('/tasks/%s' % '2').read()) + tasks_ids = [int(t['id']) for t in tasks] + self.assertEquals(set([id1, id2, id3]) - set(tasks_ids), set([])) + self._wait_task(id2) + foo2 = json.loads(self.request('/tasks/%s' % id2).read()) self.assertEquals('failed', foo2['status']) - self._wait_task('3') - foo3 = json.loads(self.request('/tasks/%s' % '3').read()) + self._wait_task(id3) + foo3 = json.loads(self.request('/tasks/%s' % id3).read()) self.assertEquals('in progress', foo3['message']) self.assertEquals('running', foo3['status']) -- 1.9.3
Reviewed-by: Crístian Viana <vianac@linux.vnet.ibm.com> On 02-09-2014 17:26, Aline Manera wrote:
V1 -> V2: - Fix "make check-local" issues - Update mockmodel to also return a Task while creating a storage volume - Update the jsonschema and API.md
Royce Lv (2): Storage volume upload: Dispatch volume create to right handler Storage volume upload: Change storagevolumes to AsyncCollection
docs/API.md | 1 + src/kimchi/API.json | 6 ++++++ src/kimchi/control/storagevolumes.py | 4 ++-- src/kimchi/i18n.py | 3 +++ src/kimchi/mockmodel.py | 22 +++++++++++++++++++++- src/kimchi/model/storagevolumes.py | 26 ++++++++++++++++++++++++-- tests/test_model.py | 8 ++++++-- tests/test_rest.py | 28 ++++++++++++++++------------ 8 files changed, 79 insertions(+), 19 deletions(-)
Applied. Thanks. Regards, Aline Manera
participants (2)
-
Aline Manera -
Crístian Viana