[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 | 32 ++++++++++++--- 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, 191 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 379c850..078e63e 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -330,9 +330,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
On 25-02-2015 13:53, Christy Perez wrote:
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*
Shouldn't the snapshot creation be listed here as well? That operation is also task-based.
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 | 27 ++++++++++++++++++++++++--- 2 files changed, 26 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 078e63e..c8089e4 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -181,7 +181,6 @@ def _get_disk_io_rate(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() @@ -203,6 +202,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: @@ -217,6 +236,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) @@ -229,6 +249,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: @@ -239,10 +260,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 VMsModel.get_vms(self.conn) -- 2.1.0
On 25-02-2015 13:53, Christy Perez wrote:
+ return self.task.lookup(taskid)
When I tried to create a VM with this patch applied, I got the following error: [26/Feb/2015:15:56:44] HTTP Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py", line 656, in respond response.body = self.handler() File "/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py", line 188, in __call__ self.body = self.oldhandler(*args, **kwargs) File "/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py", line 34, in __call__ return self.callable(*self.args, **self.kwargs) File "/home/vianac/LTC/kimchi/src/kimchi/control/base.py", line 330, in index return self.create(parse_request(), *args) File "/home/vianac/LTC/kimchi/src/kimchi/control/base.py", line 362, in create task = create(*args) File "/home/vianac/LTC/kimchi/src/kimchi/model/vms.py", line 209, in create return self.task.lookup(taskid) AttributeError: 'VMsModel' object has no attribute 'task' The class "VMsModel" doesn't have the field "self.task" (as opposed to "VMModel", which has it).
On 02/26/2015 01:05 PM, Crístian Viana wrote:
On 25-02-2015 13:53, Christy Perez wrote:
+ return self.task.lookup(taskid)
When I tried to create a VM with this patch applied, I got the following error:
[26/Feb/2015:15:56:44] HTTP Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py", line 656, in respond response.body = self.handler() File "/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py", line 188, in __call__ self.body = self.oldhandler(*args, **kwargs) File "/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py", line 34, in __call__ return self.callable(*self.args, **self.kwargs) File "/home/vianac/LTC/kimchi/src/kimchi/control/base.py", line 330, in index return self.create(parse_request(), *args) File "/home/vianac/LTC/kimchi/src/kimchi/control/base.py", line 362, in create task = create(*args) File "/home/vianac/LTC/kimchi/src/kimchi/model/vms.py", line 209, in create return self.task.lookup(taskid) AttributeError: 'VMsModel' object has no attribute 'task'
The class "VMsModel" doesn't have the field "self.task" (as opposed to "VMModel", which has it).
Thanks for the tests! I had to rebase multiple times due to the amount of time it took me to get this done and other projects popping up, so I'm sure I missed something somewhere. All the tests passed, but I'll definitely look into this one.
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
Nice patch! Once I noticed that when a snapshot is being created, the UI displays the snapshot's task as a VM being created because it thinks is a VM being cloned, due to the task's URI. This patch should fix that :-) I have a few minor comments though. Check the other e-mail replies. On 25-02-2015 13:53, 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 | 32 ++++++++++++--- 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, 191 insertions(+), 81 deletions(-)
participants (2)
-
Christy Perez -
Crístian Viana