[Kimchi-devel] [PATCHv1 2/5] Storage volume upload: Change storagevolumes to AsyncCollection

[Aline Manera]

Reviewed-by: Aline Manera <alinefm at linux.vnet.ibm.com>

On 09/01/2014 08:50 AM, lvroyce0210 at gmail.com wrote:
> From: Royce Lv <lvroyce at 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 at linux.vnet.ibm.com>
> ---
>   src/kimchi/control/storagevolumes.py |  4 ++--
>   src/kimchi/model/storagevolumes.py   | 13 +++++++++----
>   tests/test_model.py                  |  8 ++++++--
>   tests/test_rest.py                   |  9 ++++++---
>   4 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/src/kimchi/control/storagevolumes.py b/src/kimchi/control/storagevolumes.py
> index 327bf75..436e8ef 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 Collection, Resource, AsyncCollection
>   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/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py
> index fc63a16..203d5e1 100644
> --- a/src/kimchi/model/storagevolumes.py
> +++ b/src/kimchi/model/storagevolumes.py
> @@ -27,7 +27,8 @@ 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.utils import kimchi_log, add_task
> +from kimchi.model.tasks import TaskModel
>   from kimchi.model.vms import VMsModel, VMModel
>   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']
> @@ -55,9 +57,12 @@ class StorageVolumesModel(object):
>                       create_func = getattr(self, "_create_volume_with_" + p)
>                   except AttributeError:
>                       raise InvalidParameter("KCHVOL0019E", {'param': p})
> -                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>
> @@ -102,7 +107,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 59f416c..dc3a4e4 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 2117399..9263e8d 100644
> --- a/tests/test_rest.py
> +++ b/tests/test_rest.py
> @@ -494,7 +494,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()
> @@ -1006,8 +1007,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)
> @@ -1042,7 +1044,8 @@ class RestTests(unittest.TestCase):
>           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')