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

Christy Perez christy at linux.vnet.ibm.com
Mon Apr 27 13:54:56 UTC 2015



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 at linux.vnet.ibm.com>
>>>>>>>> ---
>>>>>>>>      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):
> 




More information about the Kimchi-devel mailing list