Re: [Kimchi-devel] [PATCH 1/7] Granular Task Queries: Backend

On 04/27/2015 07:54 AM, Aline Manera wrote: > > On 24/04/2015 19:49, Christy Perez wrote: >> On 04/24/2015 04:01 PM, Aline Manera wrote: >>> On 17/04/2015 19:35, Christy Perez wrote: >>>> On 04/17/2015 01:39 PM, Aline Manera wrote: >>>>> On 17/04/2015 15:08, Christy Perez wrote: >>>>>> On 04/17/2015 12:51 PM, Aline Manera wrote: >>>>>>> On 17/04/2015 12:24, Christy Perez wrote: >>>>>>>> You can filter tasks by their state (running) and/or a target_uri. >>>>>>>> If you want to see all cloning VM tasks, you can only query for >>>>>>>> running tasks with /vm in the target_uri. This patch adds an action >>>>>>>> keyword to the target_uris for existing tasks. This will make it >>>>>>>> possible to query other types of tasks for the same resource. >>>>>>>> >>>>>>>> Signed-off-by: Christy Perez <christy(a)linux.vnet.ibm.com&gt; >>>>>>>> --- >>>>>>>> docs/API.md | 16 ++++++++++++++++ >>>>>>>> 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 | 12 +++++++----- >>>>>>>> src/kimchi/model/vms.py | 5 ++--- >>>>>>>> src/kimchi/model/vmsnapshots.py | 5 +++-- >>>>>>>> 8 files changed, 37 insertions(+), 18 deletions(-) >>>>>>>> >>>>>>>> diff --git a/docs/API.md b/docs/API.md >>>>>>>> index 3f7925f..1f8e769 100644 >>>>>>>> --- a/docs/API.md >>>>>>>> +++ b/docs/API.md >>>>>>>> @@ -651,6 +651,22 @@ server. >>>>>>>> * failed: The task failed >>>>>>>> * message: Human-readable details about the Task status >>>>>>>> * target_uri: Resource URI related to the Task >>>>>>>> + The target uri may end with an action descriptor: >>>>>>>> + * VM task descriptions: >>>>>>>> + * clone >>>>>>>> + * create >>>>>>>> + * VM Snapshot description >>>>>>>> + * create >>>>>>>> + * Storage descriptions: >>>>>>>> + * create >>>>>>>> + * clone >>>>>>>> + * Pool descriptions: >>>>>>>> + * create >>>>>>>> + * scan >>>>>>> Is there a scan action? Something like /storagepools/<name>/scan ? >>>>>> In StoragePoolsModel, def _do_deep_scan(self, params): >>>>>> task_id = add_task('/storagepools/%s/scan' % ISO_POOL_NAME, >>>>>> self.scanner.start_scan, self.objstore, >>>>>> scan_params) >>>>>>>> + * Debugreport descriptions: >>>>>>>> + * create >>>>>>>> + * Host SW Update descriptions: >>>>>>>> + * update >>>>>>> I don't think we need to change the main POST action. Just those >>>>>>> actions >>>>>>> to a specific resource. >>>>>>> Example, >>>>>>> >>>>>>> # to create the VM >>>>>>> POST /vms >>>>>>> Target URI: /vms/<name> >>>>>>> >>>>>>> # to clone the VM >>>>>>> POST /vms/<name>/clone >>>>>>> Target URI: /vms/<name>/clone >>>>>>> >>>>>>> # to create a snapshot >>>>>>> POST /vms/<name>/snapshot >>>>>>> Target URI: /vms/<name>/snapshot/<snap> >>>>>>> >>>>>>> Does that make sense? >>>>>> That makes sense. I can see how it can work without changing this, >>>>>> but I >>>>>> like adding it for consistency. If you then query all tasks, some >>>>>> will >>>>>> not have the extra /<action> at the end. That will be understood >>>>>> to be >>>>>> the POST (create), but, I like the consistency of having the >>>>>> action for >>>>>> everything better. >>>>>> >>>>>> Also, the way it is now, you'll be able to query all 'create' >>>>>> tasks for >>>>>> all objects across the system, which I think would be nice -- >>>>>> although I >>>>>> don't have a direct use case for that in mind. >>>>> My suggestion is because we want to auto generate the target_uri based >>>>> on request (in future) >>>>> And in that case, we will not have the /create in the end. >>>>> >>>>> See: https://github.com/kimchi-project/kimchi/issues/559 >>>> I see! So if we did keep it as it is, we'd have to add a little extra >>>> logic in for that feature, like: >>>> >>>> def generate_target_uri(request): >>>> target_uri = some_cool_one_liner >>>> if request_type is 'POST': >>>> target_uri = "%s/create" , %target_uri >>>> return target_uri >>>> >>>> or something, which isn't as elegant. >>>> >>>> Another thing I just thought of is DELETE tasks. Right now those are >>>> all >>>> done synchronously, but things should probably also be deleted >>>> asynchronously in the future. Would that target uri also just be >>>> /vms/<vm-name>? >>> When generating the target URI automatically we can use the METHOD value >>> to distinguish the request to a resource. >>> >>> But for now, I'd suggest to only change the target URI to add the action >>> in the end. Just to make things simpler. >> It sounds like you're saying this patchset is fine as-is, then? > No. Do not append "create" when it is not an action - just a POST request > > POST /vms will turn on target_uri=/vms/<name> > POST /debugreports will turn on target_uri=/debugreports/<name> > > POST /vms/<name>/start will turn on target_uri=/vms/<name>/start > POST /vms/<name>/clone will turn on target_uri=/vms/<name>/clone > > Does that make sense? > > Using those target_uri we will only need to append the METHOD on the > beginning when creating it automatically. That makes sense, but how will a user will search for running tasks that aren't delete tasks. That's what I was asking about previously. I'll find you on IRC.

>>>>>>> >>>>>>> >>>>>>>> * **POST**: *See Task Actions* >>>>>>>> >>>>>>>> **Actions (POST):** >>>>>>>> diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py >>>>>>>> index 413ac5d..2b1b214 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/create' % name, >>>>>>>> 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/create' % >>>>>>>> (vm_name, >>>>>>>> name), >>>>>>>> self._vmsnapshots_create_task, >>>>>>>> self.objstore, params) >>>>>>>> return self.task_lookup(taskid) >>>>>>>> >>>>>>>> diff --git a/src/kimchi/model/debugreports.py >>>>>>>> b/src/kimchi/model/debugreports.py >>>>>>>> index 5f74da8..19b4de5 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/create' % name, >>>>>>>> gen_cmd, >>>>>>>> + self.objstore, name) >>>>>>>> >>>>>>>> raise OperationFailed("KCHDR0002E") >>>>>>>> >>>>>>>> diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py >>>>>>>> index 4419bb3..f429415 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..094c841 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/scan' % >>>>>>>> ISO_POOL_NAME, >>>>>>>> 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..9f93f10 100644 >>>>>>>> --- a/src/kimchi/model/storagevolumes.py >>>>>>>> +++ b/src/kimchi/model/storagevolumes.py >>>>>>>> @@ -116,8 +116,10 @@ def create(self, pool_name, params): >>>>>>>> raise InvalidParameter('KCHVOL0001E', {'name': >>>>>>>> name}) >>>>>>>> >>>>>>>> params['pool'] = pool_name >>>>>>>> - targeturi = '/storagepools/%s/storagevolumes/%s' % >>>>>>>> (pool_name, name) >>>>>>>> - taskid = add_task(targeturi, create_func, self.objstore, >>>>>>>> params) >>>>>>>> + targeturi = '/storagepools/%s/storagevolumes/%s/create' \ >>>>>>>> + % (pool_name, name) >>>>>>>> + taskid = add_task(targeturi, create_func, self.objstore, >>>>>>>> + params) >>>>>>>> return self.task.lookup(taskid) >>>>>>>> >>>>>>>> def _create_volume_with_file(self, cb, params): >>>>>>>> @@ -413,9 +415,9 @@ def clone(self, pool, name, new_pool=None, >>>>>>>> new_name=None): >>>>>>>> 'name': name, >>>>>>>> '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) >>>>>>>> + taskid = >>>>>>>> add_task(u'/storagepools/%s/storagevolumes/%s/clone' % >>>>>>>> + (pool, new_name), self._clone_task, >>>>>>>> + self.objstore, params) >>>>>>>> return self.task.lookup(taskid) >>>>>>>> >>>>>>>> def _clone_task(self, cb, params): >>>>>>>> diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py >>>>>>>> index 4c5f443..1ae841d 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/clone' % new_name, >>>>>>>> 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..c4cc663 100644 >>>>>>>> --- a/src/kimchi/model/vmsnapshots.py >>>>>>>> +++ b/src/kimchi/model/vmsnapshots.py >>>>>>>> @@ -72,8 +72,9 @@ def create(self, vm_name, params={}): >>>>>>>> name = params.get('name', unicode(int(time.time()))) >>>>>>>> >>>>>>>> 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) >>>>>>>> + taskid = add_task(u'/vms/%s/snapshots/%s/create' % >>>>>>>> (vm_name, >>>>>>>> name), >>>>>>>> + self._create_task, self.objstore, >>>>>>>> + task_params) >>>>>>>> return self.task.lookup(taskid) >>>>>>>> >>>>>>>> def _create_task(self, cb, params):