[PATCH 0/5] Create VMs Asynchronously

If a guest has a large disk, and uses a filesystem that requires preallocation, it can take several minutes to create a VM. During that time, kimchi is tied up by the VM creation. This patch changes the VMs Collection to be an AsyncCollection. Another change required for this was to create a more granular way to query tasks. Currently it is not possible (using the API) to query tasks for the same collection or resource type that may have different operations. For example, VM cloning is also an asynchronous operation. For the guests tab, the UI was querying all running tasks and displaying them with the Cloning label. This picked up VMs that were being created as well. For more information about how the tasks can be queried, see the updated API doc and the UI change. Christy Perez (5): Granular Task Queries: Backend Granular task queries test updates More Granular Task Queries: UI Create guests asynchronously: Backend Async vm creation test updates docs/API.md | 14 +++++++ src/kimchi/asynctask.py | 6 ++- src/kimchi/control/vms.py | 4 +- src/kimchi/mockmodel.py | 7 ++-- src/kimchi/model/debugreports.py | 4 +- src/kimchi/model/host.py | 4 +- src/kimchi/model/storagepools.py | 2 +- src/kimchi/model/storagevolumes.py | 7 ++-- src/kimchi/model/tasks.py | 1 - src/kimchi/model/vms.py | 33 ++++++++++++--- src/kimchi/model/vmsnapshots.py | 3 +- src/kimchi/utils.py | 4 +- tests/test_authorization.py | 23 ++++++----- tests/test_mockmodel.py | 15 +++++-- tests/test_model.py | 62 ++++++++++++++++++---------- tests/test_rest.py | 82 ++++++++++++++++++++++++++++---------- ui/js/src/kimchi.guest_main.js | 2 +- 17 files changed, 192 insertions(+), 81 deletions(-) -- 2.1.0

Currently, you can only filter tasks by their state (running) or their URL. If you want to see cloning tasks, you query all running vm-related tasks. It also adds descriptions for non-vm-related tasks. Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- docs/API.md | 14 ++++++++++++++ src/kimchi/asynctask.py | 6 ++++-- src/kimchi/model/debugreports.py | 4 ++-- src/kimchi/model/host.py | 4 ++-- src/kimchi/model/storagepools.py | 2 +- src/kimchi/model/storagevolumes.py | 7 ++++--- src/kimchi/model/tasks.py | 1 - src/kimchi/model/vms.py | 5 ++--- src/kimchi/model/vmsnapshots.py | 3 ++- src/kimchi/utils.py | 4 ++-- 10 files changed, 33 insertions(+), 17 deletions(-) diff --git a/docs/API.md b/docs/API.md index 3f7925f..bc68f06 100644 --- a/docs/API.md +++ b/docs/API.md @@ -651,6 +651,20 @@ server. * failed: The task failed * message: Human-readable details about the Task status * target_uri: Resource URI related to the Task + * description: A more-detailed identifier for a task. + * VM task descriptions: + * clone + * create + * Storage descriptions: + * create + * clone + * Pool descriptions: + * create + * scan + * Debugreport descriptions: + * create + * Host SW Update descriptions: + * update * **POST**: *See Task Actions* **Actions (POST):** diff --git a/src/kimchi/asynctask.py b/src/kimchi/asynctask.py index b5673b2..4944d7c 100644 --- a/src/kimchi/asynctask.py +++ b/src/kimchi/asynctask.py @@ -26,12 +26,14 @@ class AsyncTask(object): - def __init__(self, id, target_uri, fn, objstore, opaque=None): + def __init__(self, id, target_uri, descr, fn, objstore, + opaque=None): if objstore is None: raise OperationFailed("KCHASYNC0001E") self.id = str(id) self.target_uri = target_uri + self.description = descr self.fn = fn self.objstore = objstore self.status = 'running' @@ -56,7 +58,7 @@ def _status_cb(self, message, success=None): def _save_helper(self): obj = {} - for attr in ('id', 'target_uri', 'message', 'status'): + for attr in ('id', 'target_uri', 'message', 'status', 'description'): obj[attr] = getattr(self, attr) try: with self.objstore as session: diff --git a/src/kimchi/model/debugreports.py b/src/kimchi/model/debugreports.py index 5f74da8..c657181 100644 --- a/src/kimchi/model/debugreports.py +++ b/src/kimchi/model/debugreports.py @@ -62,8 +62,8 @@ def _gen_debugreport_file(self, name): gen_cmd = self.get_system_report_tool() if gen_cmd is not None: - return add_task('/debugreports/%s' % name, gen_cmd, self.objstore, - name) + return add_task('/debugreports/%s' % name, 'create', gen_cmd, + self.objstore, name) raise OperationFailed("KCHDR0002E") diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py index 4419bb3..26211d6 100644 --- a/src/kimchi/model/host.py +++ b/src/kimchi/model/host.py @@ -115,8 +115,8 @@ def swupdate(self, *name): raise OperationFailed('KCHPKGUPD0001E') kimchi_log.debug('Host is going to be updated.') - taskid = add_task('/host/swupdate', swupdate.doUpdate, self.objstore, - None) + taskid = add_task('/host/swupdate', 'update', swupdate.doUpdate, + self.objstore, None) return self.task.lookup(taskid) def shutdown(self, args=None): diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py index b85f3b4..e7a005b 100644 --- a/src/kimchi/model/storagepools.py +++ b/src/kimchi/model/storagepools.py @@ -167,7 +167,7 @@ def _do_deep_scan(self, params): params['path'] = self.scanner.scan_dir_prepare(params['name']) scan_params['pool_path'] = params['path'] - task_id = add_task('/storagepools/%s' % ISO_POOL_NAME, + task_id = add_task('/storagepools/%s' % ISO_POOL_NAME, 'scan', self.scanner.start_scan, self.objstore, scan_params) # Record scanning-task/storagepool mapping for future querying try: diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 0480496..e44f7db 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -117,7 +117,8 @@ def create(self, pool_name, params): params['pool'] = pool_name targeturi = '/storagepools/%s/storagevolumes/%s' % (pool_name, name) - taskid = add_task(targeturi, create_func, self.objstore, params) + taskid = add_task(targeturi, 'create', create_func, self.objstore, + params) return self.task.lookup(taskid) def _create_volume_with_file(self, cb, params): @@ -414,8 +415,8 @@ def clone(self, pool, name, new_pool=None, new_name=None): 'new_pool': new_pool, 'new_name': new_name} taskid = add_task(u'/storagepools/%s/storagevolumes/%s' % - (pool, new_name), self._clone_task, self.objstore, - params) + (pool, new_name), 'clone', self._clone_task, + self.objstore, params) return self.task.lookup(taskid) def _clone_task(self, cb, params): diff --git a/src/kimchi/model/tasks.py b/src/kimchi/model/tasks.py index 61bc2f3..5520cfd 100644 --- a/src/kimchi/model/tasks.py +++ b/src/kimchi/model/tasks.py @@ -17,7 +17,6 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - import time from kimchi.exception import TimeoutExpired diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index b5fdb12..f49bc88 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -332,9 +332,8 @@ def clone(self, name): new_name = get_next_clone_name(current_vm_names, name) # create a task with the actual clone function - taskid = add_task(u'/vms/%s' % new_name, self._clone_task, - self.objstore, - {'name': name, 'new_name': new_name}) + taskid = add_task(u'/vms/%s' % new_name, 'clone', self._clone_task, + self.objstore, {'name': name, 'new_name': new_name}) return self.task.lookup(taskid) diff --git a/src/kimchi/model/vmsnapshots.py b/src/kimchi/model/vmsnapshots.py index 3a92cdc..c899d2e 100644 --- a/src/kimchi/model/vmsnapshots.py +++ b/src/kimchi/model/vmsnapshots.py @@ -73,7 +73,8 @@ def create(self, vm_name, params={}): task_params = {'vm_name': vm_name, 'name': name} taskid = add_task(u'/vms/%s/snapshots/%s' % (vm_name, name), - self._create_task, self.objstore, task_params) + 'create', self._create_task, self.objstore, + task_params) return self.task.lookup(taskid) def _create_task(self, cb, params): diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index fc5245f..32ec6b1 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -66,9 +66,9 @@ def get_next_task_id(): return task_id -def add_task(target_uri, fn, objstore, opaque=None): +def add_task(target_uri, description, fn, objstore, opaque=None): id = get_next_task_id() - AsyncTask(id, target_uri, fn, objstore, opaque) + AsyncTask(id, target_uri, description, fn, objstore, opaque) return id -- 2.1.0

Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 7 ++++--- tests/test_mockmodel.py | 3 ++- tests/test_model.py | 8 ++++---- tests/test_rest.py | 11 ++++++----- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 413ac5d..d49fd0a 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -244,7 +244,7 @@ def _get_volume_path(self, pool, vol): return MockModel._libvirt_get_vol_path(pool, vol) def _gen_debugreport_file(self, name): - return add_task('/debugreports/%s' % name, self._create_log, + return add_task('/debugreports/%s' % name, 'create', self._create_log, self.objstore, name) def _create_log(self, cb, name): @@ -333,7 +333,8 @@ def _mock_packageupdate_lookup(self, pkg_name): return self._mock_swupdate.pkgs[pkg_name] def _mock_host_swupdate(self, args=None): - task_id = add_task('/host/swupdate', self._mock_swupdate.doUpdate, + task_id = add_task('/host/swupdate', 'update', + self._mock_swupdate.doUpdate, self.objstore) return self.task_lookup(task_id) @@ -385,7 +386,7 @@ def _mock_vm_clone(self, name): def _mock_vmsnapshots_create(self, vm_name, params): name = params.get('name', unicode(int(time.time()))) params = {'vm_name': vm_name, 'name': name} - taskid = add_task(u'/vms/%s/snapshots/%s' % (vm_name, name), + taskid = add_task(u'/vms/%s/snapshots/%s' % (vm_name, name), 'create', self._vmsnapshots_create_task, self.objstore, params) return self.task_lookup(taskid) diff --git a/tests/test_mockmodel.py b/tests/test_mockmodel.py index 29354aa..9fca012 100644 --- a/tests/test_mockmodel.py +++ b/tests/test_mockmodel.py @@ -245,6 +245,7 @@ def test_packages_update(self): self.assertIn('version', pkgupdate.keys()) task = model.host_swupdate() - task_params = [u'id', u'message', u'status', u'target_uri'] + task_params = [u'description', u'id', u'message', u'status', + u'target_uri'] self.assertEquals(sorted(task_params), sorted(task.keys())) wait_task(model.task_lookup, task['id']) diff --git a/tests/test_model.py b/tests/test_model.py index f80f1c9..1045f2d 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -1053,13 +1053,13 @@ def continuous_ops(cb, params): inst = model.Model('test:///default', objstore_loc=self.tmp_store) - taskid = add_task('', quick_op, inst.objstore, 'Hello') + taskid = add_task('', 'test', quick_op, inst.objstore, 'Hello') inst.task_wait(taskid) self.assertEquals(1, taskid) self.assertEquals('finished', inst.task_lookup(taskid)['status']) self.assertEquals('Hello', inst.task_lookup(taskid)['message']) - taskid = add_task('', long_op, inst.objstore, + taskid = add_task('', 'test', long_op, inst.objstore, {'delay': 3, 'result': False, 'message': 'It was not meant to be'}) self.assertEquals(2, taskid) @@ -1069,13 +1069,13 @@ def continuous_ops(cb, params): self.assertEquals('failed', inst.task_lookup(taskid)['status']) self.assertEquals('It was not meant to be', inst.task_lookup(taskid)['message']) - taskid = add_task('', abnormal_op, inst.objstore, {}) + taskid = add_task('', 'test', abnormal_op, inst.objstore, {}) inst.task_wait(taskid) self.assertEquals('Exception raised', inst.task_lookup(taskid)['message']) self.assertEquals('failed', inst.task_lookup(taskid)['status']) - taskid = add_task('', continuous_ops, inst.objstore, + taskid = add_task('', 'test', continuous_ops, inst.objstore, {'result': True}) self.assertEquals('running', inst.task_lookup(taskid)['status']) inst.task_wait(taskid, timeout=10) diff --git a/tests/test_rest.py b/tests/test_rest.py index 812afb7..cb1ea7c 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1370,9 +1370,9 @@ def _task_lookup(self, taskid): return json.loads(self.request('/tasks/%s' % taskid).read()) def test_tasks(self): - id1 = add_task('/tasks/1', self._async_op, model.objstore) - id2 = add_task('/tasks/2', self._except_op, model.objstore) - id3 = add_task('/tasks/3', self._intermid_op, model.objstore) + id1 = add_task('/tasks/1', 'test', self._async_op, model.objstore) + id2 = add_task('/tasks/2', 'test', self._except_op, model.objstore) + id3 = add_task('/tasks/3', 'test', self._intermid_op, model.objstore) target_uri = urllib2.quote('^/tasks/*', safe="") filter_data = 'status=running&target_uri=%s' % target_uri @@ -1384,7 +1384,7 @@ def test_tasks(self): self.assertEquals(set([id1, id2, id3]) - set(tasks_ids), set([])) wait_task(self._task_lookup, id2) foo2 = json.loads(self.request('/tasks/%s' % id2).read()) - keys = ['id', 'status', 'message', 'target_uri'] + keys = ['id', 'status', 'message', 'target_uri', 'description'] self.assertEquals(sorted(keys), sorted(foo2.keys())) self.assertEquals('failed', foo2['status']) wait_task(self._task_lookup, id3) @@ -1628,7 +1628,8 @@ def test_packages_update(self): resp = self.request('/host/swupdate', '{}', 'POST') task = json.loads(resp.read()) - task_params = [u'id', u'message', u'status', u'target_uri'] + task_params = [u'id', u'message', u'status', u'target_uri', + u'description'] self.assertEquals(sorted(task_params), sorted(task.keys())) resp = self.request('/tasks/' + task[u'id'], None, 'GET') -- 2.1.0

Add the 'clone' filter to the Cloning tasks API URL so that when vm-creation is also a running task, only Cloning VMs will be returned. Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/js/src/kimchi.guest_main.js b/ui/js/src/kimchi.guest_main.js index 21caf1b..e31b332 100644 --- a/ui/js/src/kimchi.guest_main.js +++ b/ui/js/src/kimchi.guest_main.js @@ -203,7 +203,7 @@ kimchi.listVmsAuto = function() { } var getCloningGuests = function(){ var guests = []; - kimchi.getTasksByFilter('status=running&target_uri='+encodeURIComponent('^/vms/*'), function(tasks) { + kimchi.getTasksByFilter('status=running&description=clone&target_uri='+encodeURIComponent('^/vms/*'), function(tasks) { for(var i=0;i<tasks.length;i++){ var guestUri = tasks[i].target_uri; var guestName = guestUri.substring(guestUri.lastIndexOf('/')+1, guestUri.length); -- 2.1.0

Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- src/kimchi/control/vms.py | 4 ++-- src/kimchi/model/vms.py | 28 +++++++++++++++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index 5068b7c..8a31dc0 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -17,13 +17,13 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA -from kimchi.control.base import Collection, Resource +from kimchi.control.base import AsyncCollection, Resource from kimchi.control.utils import internal_redirect, UrlSubNode from kimchi.control.vm import sub_nodes @UrlSubNode('vms', True) -class VMs(Collection): +class VMs(AsyncCollection): def __init__(self, model): super(VMs, self).__init__(model) self.resource = VM diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index f49bc88..0ac3a44 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -84,6 +84,7 @@ def __init__(self, **kargs): self.guests_stats_thread = BackgroundTask(GUESTS_STATS_INTERVAL, self._update_guests_stats) self.guests_stats_thread.start() + self.task = TaskModel(**kargs) def _update_guests_stats(self): vm_list = self.get_list() @@ -185,7 +186,6 @@ def _get_disk_io_rate(self, vm_uuid, dom, seconds): 'diskWrKB': diskWrKB}) def create(self, params): - conn = self.conn.get() t_name = template_name_from_uri(params['template']) vm_uuid = str(uuid.uuid4()) vm_list = self.get_list() @@ -207,6 +207,26 @@ def create(self, params): t.validate() + taskid = add_task(u'/vms/%s' % name, 'create', + self._create_task, self.objstore, + {'vm_uuid': vm_uuid, 'template': t, 'name': name}) + + return self.task.lookup(taskid) + + def _create_task(self, cb, params): + """ + params: A dict with the following values: + - vm_uuid: The UUID of the VM being created + - template: The template being used to create the VM + - name: The name for the new VM + """ + + vm_uuid = params['vm_uuid'] + t = params['template'] + name = params['name'] + conn = self.conn.get() + + cb('Storing VM icon') # Store the icon for displaying later icon = t.info.get('icon') if icon: @@ -221,6 +241,7 @@ def create(self, params): # If storagepool is SCSI, volumes will be LUNs and must be passed by # the user from UI or manually. + cb('Provisioning storage for new VM') vol_list = [] if t._get_storage_type() not in ["iscsi", "scsi"]: vol_list = t.fork_vm_storage(vm_uuid) @@ -233,6 +254,7 @@ def create(self, params): graphics=graphics, volumes=vol_list) + cb('Defining new VM') try: conn.defineXML(xml.encode('utf-8')) except libvirt.libvirtError as e: @@ -243,10 +265,10 @@ def create(self, params): raise OperationFailed("KCHVM0007E", {'name': name, 'err': e.get_error_message()}) + cb('Updating VM metadata') VMModel.vm_update_os_metadata(VMModel.get_vm(name, self.conn), t.info, self.caps.metadata_support) - - return name + cb('OK', True) def get_list(self): return self.get_vms(self.conn) -- 2.1.0

Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- tests/test_authorization.py | 23 +++++++++------ tests/test_mockmodel.py | 12 ++++++-- tests/test_model.py | 54 ++++++++++++++++++++++++---------- tests/test_rest.py | 71 ++++++++++++++++++++++++++++++++++----------- 4 files changed, 115 insertions(+), 45 deletions(-) diff --git a/tests/test_authorization.py b/tests/test_authorization.py index 4fcc496..eae837c 100644 --- a/tests/test_authorization.py +++ b/tests/test_authorization.py @@ -26,7 +26,7 @@ import kimchi.mockmodel from iso_gen import construct_fake_iso from utils import get_free_port, patch_auth, request -from utils import run_server +from utils import run_server, wait_task test_server = None @@ -118,19 +118,24 @@ def test_nonroot_access(self): # Non-root users can only get vms authorized to them model.templates_create({'name': u'test', 'cdrom': fake_iso}) - model.vms_create({'name': u'test-me', 'template': '/templates/test'}) + task_info = model.vms_create({'name': u'test-me', + 'template': '/templates/test'}) + wait_task(model.task_lookup, task_info['id']) + model.vm_update(u'test-me', {'users': [kimchi.mockmodel.fake_user.keys()[0]], 'groups': []}) - model.vms_create({'name': u'test-usera', - 'template': '/templates/test'}) + task_info = model.vms_create({'name': u'test-usera', + 'template': '/templates/test'}) + wait_task(model.task_lookup, task_info['id']) non_root = list(set(model.users_get_list()) - set(['root']))[0] model.vm_update(u'test-usera', {'users': [non_root], 'groups': []}) - model.vms_create({'name': u'test-groupa', - 'template': '/templates/test'}) + task_info = model.vms_create({'name': u'test-groupa', + 'template': '/templates/test'}) + wait_task(model.task_lookup, task_info['id']) a_group = model.groups_get_list()[0] model.vm_update(u'test-groupa', {'groups': [a_group]}) @@ -143,9 +148,9 @@ def test_nonroot_access(self): self.assertEquals(403, resp.status) # Create a vm using mockmodel directly to test Resource access - model.vms_create({'name': 'kimchi-test', - 'template': '/templates/test'}) - + task_info = model.vms_create({'name': 'kimchi-test', + 'template': '/templates/test'}) + wait_task(model.task_lookup, task_info['id']) resp = self.request('/vms/kimchi-test', '{}', 'PUT') self.assertEquals(403, resp.status) resp = self.request('/vms/kimchi-test', '{}', 'DELETE') diff --git a/tests/test_mockmodel.py b/tests/test_mockmodel.py index 9fca012..e4378c4 100644 --- a/tests/test_mockmodel.py +++ b/tests/test_mockmodel.py @@ -169,7 +169,9 @@ def test_screenshot_refresh(self): req = json.dumps({'name': 'test', 'cdrom': fake_iso}) request(host, ssl_port, '/templates', req, 'POST') req = json.dumps({'name': 'test-vm', 'template': '/templates/test'}) - request(host, ssl_port, '/vms', req, 'POST') + resp = request(host, ssl_port, '/vms', req, 'POST') + task = json.loads(resp.read()) + wait_task(model.task_lookup, task['id']) # Test screenshot refresh for running vm request(host, ssl_port, '/vms/test-vm/start', '{}', 'POST') @@ -197,7 +199,9 @@ def test_vm_list_sorted(self): def add_vm(name): # Create a VM req = json.dumps({'name': name, 'template': '/templates/test'}) - request(host, ssl_port, '/vms', req, 'POST') + task = json.loads(request(host, ssl_port, '/vms', req, + 'POST').read()) + wait_task(model.task_lookup, task['id']) vms = [u'abc', u'bca', u'cab', u'xba'] for vm in vms: @@ -209,7 +213,9 @@ def add_vm(name): def test_vm_info(self): model.templates_create({'name': u'test', 'cdrom': fake_iso}) - model.vms_create({'name': u'test-vm', 'template': '/templates/test'}) + task = model.vms_create({'name': u'test-vm', + 'template': '/templates/test'}) + wait_task(model.task_lookup, task['id']) vms = model.vms_get_list() self.assertEquals(2, len(vms)) self.assertIn(u'test-vm', vms) diff --git a/tests/test_model.py b/tests/test_model.py index 1045f2d..74fc4a7 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -117,8 +117,11 @@ def test_vm_lifecycle(self): rollback.prependDefer(inst.template_delete, 'test') params = {'name': 'kimchi-vm', 'template': '/templates/test'} - inst.vms_create(params) + task = inst.vms_create(params) rollback.prependDefer(inst.vm_delete, 'kimchi-vm') + inst.task_wait(task['id'], 10) + task = inst.task_lookup(task['id']) + self.assertEquals('finished', task['status']) vms = inst.vms_get_list() self.assertTrue('kimchi-vm' in vms) @@ -235,7 +238,8 @@ def test_image_based_template(self): session.store('template', tmpl_name, tmpl_info) params = {'name': 'kimchi-vm', 'template': '/templates/img-tmpl'} - inst.vms_create(params) + task = inst.vms_create(params) + inst.task_wait(task['id']) rollback.prependDefer(inst.vm_delete, 'kimchi-vm') vms = inst.vms_get_list() @@ -254,7 +258,8 @@ def test_vm_graphics(self): inst.templates_create(params) with RollbackContext() as rollback: params = {'name': 'kimchi-vnc', 'template': '/templates/test'} - inst.vms_create(params) + task1 = inst.vms_create(params) + inst.task_wait(task1['id']) rollback.prependDefer(inst.vm_delete, 'kimchi-vnc') info = inst.vm_lookup('kimchi-vnc') @@ -264,7 +269,8 @@ def test_vm_graphics(self): graphics = {'type': 'spice', 'listen': '127.0.0.1'} params = {'name': 'kimchi-spice', 'template': '/templates/test', 'graphics': graphics} - inst.vms_create(params) + task2 = inst.vms_create(params) + inst.task_wait(task2['id']) rollback.prependDefer(inst.vm_delete, 'kimchi-spice') info = inst.vm_lookup('kimchi-spice') @@ -281,7 +287,8 @@ def test_vm_ifaces(self): inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') params = {'name': 'kimchi-ifaces', 'template': '/templates/test'} - inst.vms_create(params) + task = inst.vms_create(params) + inst.task_wait(task['id']) rollback.prependDefer(inst.vm_delete, 'kimchi-ifaces') # Create a network @@ -387,7 +394,8 @@ def _attach_disk(expect_bus='virtio'): inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') params = {'name': vm_name, 'template': '/templates/test'} - inst.vms_create(params) + task1 = inst.vms_create(params) + inst.task_wait(task1['id']) rollback.prependDefer(inst.vm_delete, vm_name) prev_count = len(inst.vmstorages_get_list(vm_name)) @@ -430,7 +438,8 @@ def _attach_disk(expect_bus='virtio'): rollback.prependDefer(inst.template_delete, 'old_distro_template') params = {'name': vm_name, 'template': '/templates/old_distro_template'} - inst.vms_create(params) + task2 = inst.vms_create(params) + inst.task_wait(task2['id']) rollback.prependDefer(inst.vm_delete, vm_name) # Attach will choose IDE bus for old distro @@ -451,7 +460,8 @@ def test_vm_cdrom(self): inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') params = {'name': vm_name, 'template': '/templates/test'} - inst.vms_create(params) + task = inst.vms_create(params) + inst.task_wait(task['id']) rollback.prependDefer(inst.vm_delete, vm_name) prev_count = len(inst.vmstorages_get_list(vm_name)) @@ -549,7 +559,8 @@ def test_vm_storage_provisioning(self): rollback.prependDefer(inst.template_delete, 'test') params = {'name': 'test-vm-1', 'template': '/templates/test'} - inst.vms_create(params) + task = inst.vms_create(params) + inst.task_wait(task['id']) rollback.prependDefer(inst.vm_delete, 'test-vm-1') vm_info = inst.vm_lookup(params['name']) @@ -601,12 +612,14 @@ def test_template_storage_customise(self): inst.template_update('test', params) params = {'name': 'test-vm-1', 'template': '/templates/test'} + # @TODO: What is invalid here? Rewrite test for that. self.assertRaises(InvalidParameter, inst.vms_create, params) inst.storagepool_activate(pool) rollback.prependDefer(inst.storagepool_deactivate, pool) - inst.vms_create(params) + task = inst.vms_create(params) + inst.task_wait(task['id']) rollback.prependDefer(inst.vm_delete, 'test-vm-1') vm_info = inst.vm_lookup(params['name']) disk_path = '/tmp/kimchi-images/%s-0.img' % vm_info['uuid'] @@ -789,7 +802,10 @@ def test_template_update(self): 'new-test', params) params = {'name': 'some-vm', 'template': '/templates/new-test'} - self.assertEquals('some-vm', inst.vms_create(params)) + task = inst.vms_create(params) + inst.task_wait(task['id']) + vm_name = task['target_uri'].split('/')[-1] + self.assertEquals('some-vm', vm_name) rollback.prependDefer(inst.vm_delete, 'some-vm') iface_args = {'type': 'network', 'network': u'kīмсhī-пet'} @@ -808,10 +824,12 @@ def test_vm_edit(self): with RollbackContext() as rollback: params_1 = {'name': 'kimchi-vm1', 'template': '/templates/test'} params_2 = {'name': 'kimchi-vm2', 'template': '/templates/test'} - inst.vms_create(params_1) + task1 = inst.vms_create(params_1) + inst.task_wait(task1['id']) rollback.prependDefer(utils.rollback_wrapper, inst.vm_delete, 'kimchi-vm1') - inst.vms_create(params_2) + task2 = inst.vms_create(params_2) + inst.task_wait(task2['id']) rollback.prependDefer(utils.rollback_wrapper, inst.vm_delete, 'kimchi-vm2') @@ -1091,11 +1109,13 @@ def test_delete_running_vm(self): rollback.prependDefer(inst.template_delete, 'test') params = {'name': u'kīмсhī-∨м', 'template': u'/templates/test'} - inst.vms_create(params) + task = inst.vms_create(params) + inst.task_wait(task['id']) rollback.prependDefer(utils.rollback_wrapper, inst.vm_delete, u'kīмсhī-∨м') inst.vm_start(u'kīмсhī-∨м') + self.assertEquals(inst.vm_lookup(u'kīмсhī-∨м')['state'], 'running') rollback.prependDefer(utils.rollback_wrapper, inst.vm_poweroff, u'kīмсhī-∨м') @@ -1114,7 +1134,8 @@ def test_vm_list_sorted(self): rollback.prependDefer(inst.template_delete, 'test') params = {'name': 'kimchi-vm', 'template': '/templates/test'} - inst.vms_create(params) + task = inst.vms_create(params) + inst.task_wait(task['id']) rollback.prependDefer(inst.vm_delete, 'kimchi-vm') vms = inst.vms_get_list() @@ -1186,7 +1207,8 @@ def test_use_test_host(self): params = {'name': 'kimchi-vm', 'template': '/templates/test'} - inst.vms_create(params) + task = inst.vms_create(params) + inst.task_wait(task['id']) rollback.prependDefer(inst.vm_delete, 'kimchi-vm') vms = inst.vms_get_list() diff --git a/tests/test_rest.py b/tests/test_rest.py index cb1ea7c..c497791 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -190,7 +190,9 @@ def test_get_vms(self): req = json.dumps({'name': name, 'template': '/templates/test', 'users': test_users, 'groups': test_groups}) resp = self.request('/vms', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) vms = json.loads(self.request('/vms').read()) self.assertEquals(11, len(vms)) @@ -208,7 +210,9 @@ def test_edit_vm(self): req = json.dumps({'name': 'vm-1', 'template': '/templates/test'}) resp = self.request('/vms', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) vm = json.loads(self.request('/vms/vm-1').read()) self.assertEquals('vm-1', vm['name']) @@ -325,7 +329,9 @@ def test_vm_lifecycle(self): # Create a VM req = json.dumps({'name': 'test-vm', 'template': '/templates/test'}) resp = self.request('/vms', req, 'POST') - self.assertEquals(201, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) + self.assertEquals(202, resp.status) # Verify the VM vm = json.loads(self.request('/vms/test-vm').read()) @@ -479,7 +485,9 @@ def test_vm_graphics(self): # Create a VM with default args req = json.dumps({'name': 'test-vm', 'template': '/templates/test'}) resp = self.request('/vms', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) # Verify the VM vm = json.loads(self.request('/vms/test-vm').read()) self.assertEquals('127.0.0.1', vm['graphics']['listen']) @@ -493,7 +501,9 @@ def test_vm_graphics(self): req = json.dumps({'name': 'test-vm', 'template': '/templates/test', 'graphics': graphics}) resp = self.request('/vms', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) # Verify the VM vm = json.loads(self.request('/vms/test-vm').read()) self.assertEquals('127.0.0.1', vm['graphics']['listen']) @@ -507,7 +517,9 @@ def test_vm_graphics(self): req = json.dumps({'name': 'test-vm', 'template': '/templates/test', 'graphics': graphics}) resp = self.request('/vms', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) # Verify the VM vm = json.loads(self.request('/vms/test-vm').read()) self.assertEquals('fe00::0', vm['graphics']['listen']) @@ -521,7 +533,9 @@ def test_vm_graphics(self): req = json.dumps({'name': 'test-vm', 'template': '/templates/test', 'graphics': graphics}) resp = self.request('/vms', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) # Verify the VM vm = json.loads(self.request('/vms/test-vm').read()) self.assertEquals('127.0.0.1', vm['graphics']['listen']) @@ -563,7 +577,9 @@ def test_vm_storage_devices(self): req = json.dumps({'name': 'test-vm', 'template': '/templates/test'}) resp = self.request('/vms', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) # Delete the VM rollback.prependDefer(self.request, '/vms/test-vm', '{}', 'DELETE') @@ -708,7 +724,9 @@ def test_vm_iface(self): req = json.dumps({'name': 'test-vm', 'template': '/templates/test'}) resp = self.request('/vms', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) # Delete the VM rollback.prependDefer(self.request, '/vms/test-vm', '{}', 'DELETE') @@ -782,7 +800,10 @@ def test_vm_customise_storage(self): req = json.dumps({'name': 'test-vm', 'template': '/templates/test', 'storagepool': '/storagepools/alt'}) resp = self.request('/vms', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) + resp = self.request('/vms/test-vm', {}, 'GET') vm_info = json.loads(resp.read()) # Test template not changed after vm customise its pool @@ -835,7 +856,9 @@ def test_scsi_fc_storage(self): req = json.dumps({'name': 'test-vm', 'template': '/templates/test_fc_pool'}) resp = self.request('/vms', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) # Start the VM resp = self.request('/vms/test-vm/start', '{}', 'POST') @@ -886,8 +909,11 @@ def test_template_customise_storage(self): # Create a VM req = json.dumps({'name': 'test-vm', 'template': '/templates/test'}) resp = self.request('/vms', req, 'POST') + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) + resp = self.request('/vms/test-vm', {}, 'GET') vm = json.loads(resp.read()) - self.assertEquals(201, resp.status) + self.assertEquals(200, resp.status) # Verify the volume was created vol_uri = '/storagepools/alt/storagevolumes/%s-0.img' % vm['uuid'] @@ -969,8 +995,10 @@ def test_unnamed_vms(self): # Create 5 unnamed vms from this template for i in xrange(1, 6): req = json.dumps({'template': '/templates/test'}) - vm = json.loads(self.request('/vms', req, 'POST').read()) - self.assertEquals('test-vm-%i' % i, vm['name']) + task = json.loads(self.request('/vms', req, 'POST').read()) + wait_task(self._task_lookup, task['id']) + resp = self.request('/vms/test-vm-%i' % i, {}, 'GET') + self.assertEquals(resp.status, 200) count = len(json.loads(self.request('/vms').read())) self.assertEquals(6, count) @@ -1002,7 +1030,10 @@ def test_create_vm_with_img_based_template(self): self.assertEquals(201, resp.status) req = json.dumps({'template': '/templates/test'}) - json.loads(self.request('/vms', req, 'POST').read()) + resp = self.request('/vms', req, 'POST') + self.assertEquals(202, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) # Test storage volume created with backing store of base file resp = json.loads( @@ -1318,6 +1349,8 @@ def test_screenshot_refresh(self): resp = self.request('/templates', req, 'POST') req = json.dumps({'name': 'test-vm', 'template': '/templates/test'}) resp = self.request('/vms', req, 'POST') + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) # Test screenshot for shut-off state vm resp = self.request('/vms/test-vm/screenshot') @@ -1648,10 +1681,14 @@ def test_get_param(self): # Create a VM req = json.dumps({'name': 'test-vm1', 'template': '/templates/test'}) resp = self.request('/vms', req, 'POST') - self.assertEquals(201, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) + self.assertEquals(202, resp.status) req = json.dumps({'name': 'test-vm2', 'template': '/templates/test'}) resp = self.request('/vms', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + task = json.loads(resp.read()) + wait_task(self._task_lookup, task['id']) resp = request(host, ssl_port, '/vms') self.assertEquals(200, resp.status) -- 2.1.0

This series needs a rebase. Please see the most recent series. Thanks, - Christy On 02/25/2015 10:20 AM, Christy Perez wrote:
If a guest has a large disk, and uses a filesystem that requires preallocation, it can take several minutes to create a VM. During that time, kimchi is tied up by the VM creation.
This patch changes the VMs Collection to be an AsyncCollection.
Another change required for this was to create a more granular way to query tasks. Currently it is not possible (using the API) to query tasks for the same collection or resource type that may have different operations. For example, VM cloning is also an asynchronous operation. For the guests tab, the UI was querying all running tasks and displaying them with the Cloning label. This picked up VMs that were being created as well. For more information about how the tasks can be queried, see the updated API doc and the UI change.
Christy Perez (5): Granular Task Queries: Backend Granular task queries test updates More Granular Task Queries: UI Create guests asynchronously: Backend Async vm creation test updates
docs/API.md | 14 +++++++ src/kimchi/asynctask.py | 6 ++- src/kimchi/control/vms.py | 4 +- src/kimchi/mockmodel.py | 7 ++-- src/kimchi/model/debugreports.py | 4 +- src/kimchi/model/host.py | 4 +- src/kimchi/model/storagepools.py | 2 +- src/kimchi/model/storagevolumes.py | 7 ++-- src/kimchi/model/tasks.py | 1 - src/kimchi/model/vms.py | 33 ++++++++++++--- src/kimchi/model/vmsnapshots.py | 3 +- src/kimchi/utils.py | 4 +- tests/test_authorization.py | 23 ++++++----- tests/test_mockmodel.py | 15 +++++-- tests/test_model.py | 62 ++++++++++++++++++---------- tests/test_rest.py | 82 ++++++++++++++++++++++++++++---------- ui/js/src/kimchi.guest_main.js | 2 +- 17 files changed, 192 insertions(+), 81 deletions(-)
participants (1)
-
Christy Perez