[PATCHv7 0/8] storage server and targets support
From: Royce Lv <lvroyce@linux.vnet.ibm.com> v6>v7, adopt lxml to parse xml, move parse params to get() to avoid duplicate code, fix bugs when one server support multiple target type. v5>v6, change GET param support to cover more scenario of filter collected results. v4>v5, remove storage server list reload function, merge storage server and targets v3>v4, fix inconsistency between doc and json schema v1>v3, fix racing problem, fix style. Royce Lv (8): Support params for GET method Add testcase for GET param Storage server: Update API.md storage server: update controller.py storage server: Update model and mockmodel storage target: Update API.md storage target: Update controller and json schema storage target: Add model support docs/API.md | 22 ++++++++++ src/kimchi/API.json | 20 +++++++++ src/kimchi/control/base.py | 28 +++++++++---- src/kimchi/control/storagepools.py | 4 +- src/kimchi/control/storageserver.py | 62 ++++++++++++++++++++++++++++ src/kimchi/control/storagevolumes.py | 2 +- src/kimchi/control/utils.py | 2 + src/kimchi/mockmodel.py | 30 ++++++++++++++ src/kimchi/model.py | 78 ++++++++++++++++++++++++++++++++++++ src/kimchi/root.py | 2 + tests/test_rest.py | 34 ++++++++++++++++ 11 files changed, 273 insertions(+), 11 deletions(-) create mode 100644 src/kimchi/control/storageserver.py -- 1.8.1.2
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add support for passing params for GET method, This include add param to get_list method, and filter the result with given params. we will call it like: GET /collection?filter_field=value With current implementation with a wrapper, we can easily support any collection query with param. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 28 ++++++++++++++++++++-------- src/kimchi/control/storagepools.py | 4 ++-- src/kimchi/control/storagevolumes.py | 2 +- src/kimchi/control/utils.py | 2 ++ 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 185c8d8..a477abb 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -28,7 +28,7 @@ import urllib2 import kimchi.template from kimchi.control.utils import get_class_name, internal_redirect, model_fn -from kimchi.control.utils import parse_request, validate_method +from kimchi.control.utils import parse_request, parse_param, validate_method from kimchi.control.utils import validate_params from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed @@ -212,10 +212,10 @@ class Collection(object): return res.get() - def _get_resources(self): + def _get_resources(self, filter_params): try: get_list = getattr(self.model, model_fn(self, 'get_list')) - idents = get_list(*self.model_args) + idents = get_list(*self.model_args, **filter_params) res_list = [] for ident in idents: # internal text, get_list changes ident to unicode for sorted @@ -234,19 +234,31 @@ class Collection(object): args = self.resource_args + [ident.decode("utf-8")] return self.resource(self.model, *args) - def get(self): - resources = self._get_resources() + def filter_data(self, resources, filter_params): data = [] for res in resources: - data.append(res.data) + valid = True + for key, val in filter_params.items(): + if key in res.data and res.data[key] != val: + valid = False + break + if valid: + data.append(res.data) + return data + + def get(self, filter_params): + resources = self._get_resources(filter_params) + data = self.filter_data(resources, filter_params) return kimchi.template.render(get_class_name(self), data) @cherrypy.expose - def index(self, *args): + def index(self, *args, **kwargs): method = validate_method(('GET', 'POST')) if method == 'GET': try: - return self.get() + filter_params = parse_param() + validate_params(filter_params, self, 'get_list') + return self.get(filter_params) except InvalidOperation, param: error = "Invalid operation: '%s'" % param raise cherrypy.HTTPError(400, error) diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index 782f5a6..e3236a7 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -63,9 +63,9 @@ class StoragePools(Collection): return resp - def _get_resources(self): + def _get_resources(self, filter_params): try: - res_list = super(StoragePools, self)._get_resources() + res_list = super(StoragePools, self)._get_resources(filter_params) # Append reserved pools isos = getattr(self, ISO_POOL_NAME) isos.lookup() diff --git a/src/kimchi/control/storagevolumes.py b/src/kimchi/control/storagevolumes.py index d541807..cd15bcc 100644 --- a/src/kimchi/control/storagevolumes.py +++ b/src/kimchi/control/storagevolumes.py @@ -70,7 +70,7 @@ class IsoVolumes(Collection): super(IsoVolumes, self).__init__(model) self.pool = pool - def get(self): + def get(self, filter_params): res_list = [] try: get_list = getattr(self.model, model_fn(self, 'get_list')) diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py index c3c5f8e..4b3c4b0 100644 --- a/src/kimchi/control/utils.py +++ b/src/kimchi/control/utils.py @@ -81,6 +81,8 @@ def parse_request(): raise cherrypy.HTTPError(415, "This API only supports" " 'application/json'") +def parse_param(): + return cherrypy.request.params def internal_redirect(url): raise cherrypy.InternalRedirect(url.encode("utf-8")) -- 1.8.1.2
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add support for passing params for GET method, This include add param to get_list method, and filter the result with given params. we will call it like: GET /collection?filter_field=value With current implementation with a wrapper, we can easily support any collection query with param.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 28 ++++++++++++++++++++-------- src/kimchi/control/storagepools.py | 4 ++-- src/kimchi/control/storagevolumes.py | 2 +- src/kimchi/control/utils.py | 2 ++ 4 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 185c8d8..a477abb 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -28,7 +28,7 @@ import urllib2
import kimchi.template from kimchi.control.utils import get_class_name, internal_redirect, model_fn -from kimchi.control.utils import parse_request, validate_method +from kimchi.control.utils import parse_request, parse_param, validate_method from kimchi.control.utils import validate_params from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed @@ -212,10 +212,10 @@ class Collection(object):
return res.get()
- def _get_resources(self): + def _get_resources(self, filter_params): try: get_list = getattr(self.model, model_fn(self, 'get_list')) - idents = get_list(*self.model_args) + idents = get_list(*self.model_args, **filter_params) For the methods doesn't support filter params, it will break if add a
On 01/14/2014 11:47 PM, lvroyce0210@gmail.com wrote: param in url.
res_list = [] for ident in idents: # internal text, get_list changes ident to unicode for sorted @@ -234,19 +234,31 @@ class Collection(object): args = self.resource_args + [ident.decode("utf-8")] return self.resource(self.model, *args)
- def get(self): - resources = self._get_resources() + def filter_data(self, resources, filter_params): data = [] for res in resources: - data.append(res.data) + valid = True + for key, val in filter_params.items(): + if key in res.data and res.data[key] != val: + valid = False + break + if valid:
if all(key in res.data and res.data[key] == val for key, val in filter_params.iteritems()): data.append(res.data)
+ data.append(res.data) + return data
or return [res.data for res in resources if all(key in res.data and res.data[key] == val for key, val in filter_params.iteritems())] :)
+ + def get(self, filter_params): + resources = self._get_resources(filter_params) + data = self.filter_data(resources, filter_params) I guess there're two kinds of filter parameters in your mind: one takes effect during model's get_list method while the other works based the return value from model. So please make those two params explicit and ensure one param only can take effect in one place. return kimchi.template.render(get_class_name(self), data)
@cherrypy.expose - def index(self, *args): + def index(self, *args, **kwargs): method = validate_method(('GET', 'POST')) if method == 'GET': try: - return self.get() + filter_params = parse_param() + validate_params(filter_params, self, 'get_list') + return self.get(filter_params) except InvalidOperation, param: error = "Invalid operation: '%s'" % param raise cherrypy.HTTPError(400, error) diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index 782f5a6..e3236a7 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -63,9 +63,9 @@ class StoragePools(Collection):
return resp
- def _get_resources(self): + def _get_resources(self, filter_params): try: - res_list = super(StoragePools, self)._get_resources() + res_list = super(StoragePools, self)._get_resources(filter_params) # Append reserved pools isos = getattr(self, ISO_POOL_NAME) isos.lookup() diff --git a/src/kimchi/control/storagevolumes.py b/src/kimchi/control/storagevolumes.py index d541807..cd15bcc 100644 --- a/src/kimchi/control/storagevolumes.py +++ b/src/kimchi/control/storagevolumes.py @@ -70,7 +70,7 @@ class IsoVolumes(Collection): super(IsoVolumes, self).__init__(model) self.pool = pool
- def get(self): + def get(self, filter_params): res_list = [] try: get_list = getattr(self.model, model_fn(self, 'get_list')) diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py index c3c5f8e..4b3c4b0 100644 --- a/src/kimchi/control/utils.py +++ b/src/kimchi/control/utils.py @@ -81,6 +81,8 @@ def parse_request(): raise cherrypy.HTTPError(415, "This API only supports" " 'application/json'")
+def parse_param(): + return cherrypy.request.params It's not a good function name for me, because you didn't do any parsing in it. get_query_params or just use cherrypy.request.params?
def internal_redirect(url): raise cherrypy.InternalRedirect(url.encode("utf-8"))
On 01/14/2014 01:47 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add support for passing params for GET method, This include add param to get_list method, and filter the result with given params. we will call it like: GET /collection?filter_field=value With current implementation with a wrapper, we can easily support any collection query with param.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 28 ++++++++++++++++++++-------- src/kimchi/control/storagepools.py | 4 ++-- src/kimchi/control/storagevolumes.py | 2 +- src/kimchi/control/utils.py | 2 ++ 4 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 185c8d8..a477abb 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -28,7 +28,7 @@ import urllib2
import kimchi.template from kimchi.control.utils import get_class_name, internal_redirect, model_fn -from kimchi.control.utils import parse_request, validate_method +from kimchi.control.utils import parse_request, parse_param, validate_method from kimchi.control.utils import validate_params from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed @@ -212,10 +212,10 @@ class Collection(object):
return res.get()
- def _get_resources(self): + def _get_resources(self, filter_params): try: get_list = getattr(self.model, model_fn(self, 'get_list')) - idents = get_list(*self.model_args) + idents = get_list(*self.model_args, **filter_params)
If the filter will be done in filter_data() below we don't need to pass filter_params to get_list
res_list = [] for ident in idents: # internal text, get_list changes ident to unicode for sorted @@ -234,19 +234,31 @@ class Collection(object): args = self.resource_args + [ident.decode("utf-8")] return self.resource(self.model, *args)
- def get(self): - resources = self._get_resources() + def filter_data(self, resources, filter_params): data = [] for res in resources: - data.append(res.data) + valid = True + for key, val in filter_params.items(): + if key in res.data and res.data[key] != val: + valid = False + break
You should continue the loop even when one resource doesn't have the filter parameter. For example, some storagepools have "task_id" parameter and other ones don't have.
+ if valid: + data.append(res.data) + return data + + def get(self, filter_params): + resources = self._get_resources(filter_params) + data = self.filter_data(resources, filter_params) return kimchi.template.render(get_class_name(self), data)
@cherrypy.expose - def index(self, *args): + def index(self, *args, **kwargs): method = validate_method(('GET', 'POST')) if method == 'GET': try: - return self.get() + filter_params = parse_param() + validate_params(filter_params, self, 'get_list') + return self.get(filter_params) except InvalidOperation, param: error = "Invalid operation: '%s'" % param raise cherrypy.HTTPError(400, error) diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index 782f5a6..e3236a7 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -63,9 +63,9 @@ class StoragePools(Collection):
return resp
- def _get_resources(self): + def _get_resources(self, filter_params): try: - res_list = super(StoragePools, self)._get_resources() + res_list = super(StoragePools, self)._get_resources(filter_params) # Append reserved pools isos = getattr(self, ISO_POOL_NAME) isos.lookup() diff --git a/src/kimchi/control/storagevolumes.py b/src/kimchi/control/storagevolumes.py index d541807..cd15bcc 100644 --- a/src/kimchi/control/storagevolumes.py +++ b/src/kimchi/control/storagevolumes.py @@ -70,7 +70,7 @@ class IsoVolumes(Collection): super(IsoVolumes, self).__init__(model) self.pool = pool
- def get(self): + def get(self, filter_params): res_list = [] try: get_list = getattr(self.model, model_fn(self, 'get_list')) diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py index c3c5f8e..4b3c4b0 100644 --- a/src/kimchi/control/utils.py +++ b/src/kimchi/control/utils.py @@ -81,6 +81,8 @@ def parse_request(): raise cherrypy.HTTPError(415, "This API only supports" " 'application/json'")
+def parse_param(): + return cherrypy.request.params
You don't need a function for it. It is just one line. Use cherrypy.request.params directly
def internal_redirect(url): raise cherrypy.InternalRedirect(url.encode("utf-8"))
On 2014年01月18日 02:01, Aline Manera wrote:
> On 01/14/2014 01:47 PM, lvroyce0210@gmail.com wrote:
>> From: Royce Lv <lvroyce@linux.vnet.ibm.com>
>>
>> Add support for passing params for GET method,
>> This include add param to get_list method,
>> and filter the result with given params.
>> we will call it like:
>> GET /collection?filter_field=value
>> With current implementation with a wrapper,
>> we can easily support any collection query with param.
>>
>> Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com>
>> ---
>> src/kimchi/control/base.py | 28 ++++++++++++++++++++--------
>> src/kimchi/control/storagepools.py | 4 ++--
>> src/kimchi/control/storagevolumes.py | 2 +-
>> src/kimchi/control/utils.py | 2 ++
>> 4 files changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py
>> index 185c8d8..a477abb 100644
>> --- a/src/kimchi/control/base.py
>> +++ b/src/kimchi/control/base.py
>> @@ -28,7 +28,7 @@ import urllib2
>>
>> import kimchi.template
>> from kimchi.control.utils import get_class_name, internal_redirect,
>> model_fn
>> -from kimchi.control.utils import parse_request, validate_method
>> +from kimchi.control.utils import parse_request, parse_param,
>> validate_method
>> from kimchi.control.utils import validate_params
>> from kimchi.exception import InvalidOperation, InvalidParameter
>> from kimchi.exception import MissingParameter, NotFoundError,
>> OperationFailed
>> @@ -212,10 +212,10 @@ class Collection(object):
>>
>> return res.get()
>>
>> - def _get_resources(self):
>> + def _get_resources(self, filter_params):
>> try:
>> get_list = getattr(self.model, model_fn(self, 'get_list'))
>> - idents = get_list(*self.model_args)
>> + idents = get_list(*self.model_args, **filter_params)
>
> If the filter will be done in filter_data() below we don't need to
> pass filter_params to get_list
My plan is to make it effective in either of these two places:
reasons are according to http://libvirt.org/html/libvirt-libvirt.html
1. Many list API takes flags to indicate what kinds of resource they
will query, this act as a filter, is supported by libvirt itself, we
don't need to filter on our own.
2. Some full resource query is quite time consuming and not needed:
such as: user want to query 'nfs' targets on a server, if without
get_list filter, we will do:
1> query all type of targets: nfs+ iscsi+ gluster, etc
2> filter out iscsi + gluster
which is quite waste of effort.
>
>> res_list = []
>> for ident in idents:
>> # internal text, get_list changes ident to unicode for sorted
>> @@ -234,19 +234,31 @@ class Collection(object):
>> args = self.resource_args + [ident.decode("utf-8")]
>> return self.resource(self.model, *args)
>>
>> - def get(self):
>> - resources = self._get_resources()
>> + def filter_data(self, resources, filter_params):
>> data = []
>> for res in resources:
>> - data.append(res.data)
>> + valid = True
>> + for key, val in filter_params.items():
>> + if key in res.data and res.data[key] != val:
>> + valid = False
>> + break
>
> You should continue the loop even when one resource doesn't have the
> filter parameter.
yeah, my condition is "if key in res.data and res.data[key] != val" it
means if resource doesn't have filter parameter, it will ignore this key
and iterate next filter key.
> For example, some storagepools have "task_id" parameter and other ones
> don't have.
>
>> + if valid:
>> + data.append(res.data)
>> + return data
>> +
>> + def get(self, filter_params):
>> + resources = self._get_resources(filter_params)
>> + data = self.filter_data(resources, filter_params)
>> return kimchi.template.render(get_class_name(self), data)
>>
>> @cherrypy.expose
>> - def index(self, *args):
>> + def index(self, *args, **kwargs):
>> method = validate_method(('GET', 'POST'))
>> if method == 'GET':
>> try:
>> - return self.get()
>> + filter_params = parse_param()
>> + validate_params(filter_params, self, 'get_list')
>> + return self.get(filter_params)
>> except InvalidOperation, param:
>> error = "Invalid operation: '%s'" % param
>> raise cherrypy.HTTPError(400, error)
>> diff --git a/src/kimchi/control/storagepools.py
>> b/src/kimchi/control/storagepools.py
>> index 782f5a6..e3236a7 100644
>> --- a/src/kimchi/control/storagepools.py
>> +++ b/src/kimchi/control/storagepools.py
>> @@ -63,9 +63,9 @@ class StoragePools(Collection):
>>
>> return resp
>>
>> - def _get_resources(self):
>> + def _get_resources(self, filter_params):
>> try:
>> - res_list = super(StoragePools, self)._get_resources()
>> + res_list = super(StoragePools, self)._get_resources(filter_params)
>> # Append reserved pools
>> isos = getattr(self, ISO_POOL_NAME)
>> isos.lookup()
>> diff --git a/src/kimchi/control/storagevolumes.py
>> b/src/kimchi/control/storagevolumes.py
>> index d541807..cd15bcc 100644
>> --- a/src/kimchi/control/storagevolumes.py
>> +++ b/src/kimchi/control/storagevolumes.py
>> @@ -70,7 +70,7 @@ class IsoVolumes(Collection):
>> super(IsoVolumes, self).__init__(model)
>> self.pool = pool
>>
>> - def get(self):
>> + def get(self, filter_params):
>> res_list = []
>> try:
>> get_list = getattr(self.model, model_fn(self, 'get_list'))
>> diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py
>> index c3c5f8e..4b3c4b0 100644
>> --- a/src/kimchi/control/utils.py
>> +++ b/src/kimchi/control/utils.py
>> @@ -81,6 +81,8 @@ def parse_request():
>> raise cherrypy.HTTPError(415, "This API only supports"
>> " 'application/json'")
>
>>
>> +def parse_param():
>> + return cherrypy.request.params
>
> You don't need a function for it.
> It is just one line.
> Use cherrypy.request.params directly
ACK
>
>>
>> def internal_redirect(url):
>> raise cherrypy.InternalRedirect(url.encode("utf-8"))
>
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add a testcase to test GET param passing to demo how GET param work with current model implementation. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- tests/test_rest.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_rest.py b/tests/test_rest.py index 8732781..4d161d9 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1132,6 +1132,40 @@ class RestTests(unittest.TestCase): self.assertIn('net_recv_rate', stats) self.assertIn('net_sent_rate', stats) + def test_get_param(self): + def hack_model(func): + def _get_param_func(*args, **kwargs): + res = func() + return res + return _get_param_func + + global model + old_handler = model.vms_get_list + model.vms_get_list = hack_model(model.vms_get_list) + + req = json.dumps({'name': 'test', 'cdrom': '/nonexistent.iso'}) + self.request('/templates', req, 'POST') + + # Create a VM + req = json.dumps({'name': 'test-vm1', 'template': '/templates/test'}) + resp = self.request('/vms', req, 'POST') + self.assertEquals(201, resp.status) + req = json.dumps({'name': 'test-vm2', 'template': '/templates/test'}) + resp = self.request('/vms', req, 'POST') + self.assertEquals(201, resp.status) + + resp = request(host, port, '/vms') + self.assertEquals(200, resp.status) + res = json.loads(resp.read()) + self.assertEquals(2, len(res)) + + resp = request(host, port, '/vms?name=test-vm1') + self.assertEquals(200, resp.status) + res = json.loads(resp.read()) + self.assertEquals(1, len(res)) + self.assertEquals('test-vm1', res[0]['name']) + + model.vms_get_list = old_handler class HttpsRestTests(RestTests): """ -- 1.8.1.2
On 01/14/2014 01:47 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add a testcase to test GET param passing to demo how GET param work with current model implementation.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- tests/test_rest.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/tests/test_rest.py b/tests/test_rest.py index 8732781..4d161d9 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1132,6 +1132,40 @@ class RestTests(unittest.TestCase): self.assertIn('net_recv_rate', stats) self.assertIn('net_sent_rate', stats)
+ def test_get_param(self):
+ def hack_model(func): + def _get_param_func(*args, **kwargs): + res = func() + return res + return _get_param_func + + global model + old_handler = model.vms_get_list + model.vms_get_list = hack_model(model.vms_get_list)
Why is it needed? GET /vms?name=test-vm1 will run vms_get_list() which will return all vm names then filter_data() will return only the element with name 'test-vm1'
+ + req = json.dumps({'name': 'test', 'cdrom': '/nonexistent.iso'}) + self.request('/templates', req, 'POST') + + # Create a VM + req = json.dumps({'name': 'test-vm1', 'template': '/templates/test'}) + resp = self.request('/vms', req, 'POST') + self.assertEquals(201, resp.status) + req = json.dumps({'name': 'test-vm2', 'template': '/templates/test'}) + resp = self.request('/vms', req, 'POST') + self.assertEquals(201, resp.status) + + resp = request(host, port, '/vms') + self.assertEquals(200, resp.status) + res = json.loads(resp.read()) + self.assertEquals(2, len(res)) + + resp = request(host, port, '/vms?name=test-vm1') + self.assertEquals(200, resp.status) + res = json.loads(resp.read()) + self.assertEquals(1, len(res)) + self.assertEquals('test-vm1', res[0]['name']) + + model.vms_get_list = old_handler
class HttpsRestTests(RestTests): """
On 2014年01月18日 02:07, Aline Manera wrote:
On 01/14/2014 01:47 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add a testcase to test GET param passing to demo how GET param work with current model implementation.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- tests/test_rest.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/tests/test_rest.py b/tests/test_rest.py index 8732781..4d161d9 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1132,6 +1132,40 @@ class RestTests(unittest.TestCase): self.assertIn('net_recv_rate', stats) self.assertIn('net_sent_rate', stats)
+ def test_get_param(self):
+ def hack_model(func): + def _get_param_func(*args, **kwargs): + res = func() + return res + return _get_param_func + + global model + old_handler = model.vms_get_list + model.vms_get_list = hack_model(model.vms_get_list)
Why is it needed?
GET /vms?name=test-vm1 will run vms_get_list() which will return all vm names
then filter_data() will return only the element with name 'test-vm1' This wrapper is for I pass filter param to get_list(). I will change it in next version and dump this wrapper.
+ + req = json.dumps({'name': 'test', 'cdrom': '/nonexistent.iso'}) + self.request('/templates', req, 'POST') + + # Create a VM + req = json.dumps({'name': 'test-vm1', 'template': '/templates/test'}) + resp = self.request('/vms', req, 'POST') + self.assertEquals(201, resp.status) + req = json.dumps({'name': 'test-vm2', 'template': '/templates/test'}) + resp = self.request('/vms', req, 'POST') + self.assertEquals(201, resp.status) + + resp = request(host, port, '/vms') + self.assertEquals(200, resp.status) + res = json.loads(resp.read()) + self.assertEquals(2, len(res)) + + resp = request(host, port, '/vms?name=test-vm1') + self.assertEquals(200, resp.status) + res = json.loads(resp.read()) + self.assertEquals(1, len(res)) + self.assertEquals('test-vm1', res[0]['name']) + + model.vms_get_list = old_handler
class HttpsRestTests(RestTests): """
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Update API.md to specify storage server api. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/API.md b/docs/API.md index 7bed7a0..7142117 100644 --- a/docs/API.md +++ b/docs/API.md @@ -458,6 +458,19 @@ creation. not tested yet * **POST**: *See Configuration Actions* +**Actions (POST):** + +*No actions defined* + +### Collection: Storage Servers + +**URI:** /storageservers + +**Methods:** + +* **GET**: Retrieve a summarized list of used storage servers. + * target_type: Filter server list with given type, currently support 'netfs'. + ### Collection: Distros **URI:** /config/distros -- 1.8.1.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/14/2014 01:47 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Update API.md to specify storage server api.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 7bed7a0..7142117 100644 --- a/docs/API.md +++ b/docs/API.md @@ -458,6 +458,19 @@ creation. not tested yet * **POST**: *See Configuration Actions*
+**Actions (POST):** + +*No actions defined* + +### Collection: Storage Servers + +**URI:** /storageservers + +**Methods:** + +* **GET**: Retrieve a summarized list of used storage servers. + * target_type: Filter server list with given type, currently support 'netfs'. + ### Collection: Distros
**URI:** /config/distros
On 01/17/2014 04:07 PM, Aline Manera wrote:
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com>
On 01/14/2014 01:47 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Update API.md to specify storage server api.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 7bed7a0..7142117 100644 --- a/docs/API.md +++ b/docs/API.md @@ -458,6 +458,19 @@ creation. not tested yet * **POST**: *See Configuration Actions*
+**Actions (POST):** + +*No actions defined* + +### Collection: Storage Servers + +**URI:** /storageservers + +**Methods:** + +* **GET**: Retrieve a summarized list of used storage servers. + * target_type: Filter server list with given type, currently support 'netfs'.
You also need to add "Storage Server (Resource)" and which values GET returns
+ ### Collection: Distros
**URI:** /config/distros
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
On 2014年01月18日 02:45, Aline Manera wrote:
On 01/17/2014 04:07 PM, Aline Manera wrote:
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com>
On 01/14/2014 01:47 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Update API.md to specify storage server api.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 7bed7a0..7142117 100644 --- a/docs/API.md +++ b/docs/API.md @@ -458,6 +458,19 @@ creation. not tested yet * **POST**: *See Configuration Actions*
+**Actions (POST):** + +*No actions defined* + +### Collection: Storage Servers + +**URI:** /storageservers + +**Methods:** + +* **GET**: Retrieve a summarized list of used storage servers. + * target_type: Filter server list with given type, currently support 'netfs'.
You also need to add "Storage Server (Resource)" and which values GET returns I added this in previous version but deleted at last. Because for storage server, we only care about address, which can be get from /storageservers?query_param This resource yeilds no other information and should not expose to users.
+ ### Collection: Distros
**URI:** /config/distros
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add storage server collection and resource to report used storage server. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/API.json | 10 ++++++++++ src/kimchi/control/storageserver.py | 38 +++++++++++++++++++++++++++++++++++++ src/kimchi/root.py | 2 ++ 3 files changed, 50 insertions(+) create mode 100644 src/kimchi/control/storageserver.py diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 19b1c51..d951696 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -214,6 +214,16 @@ }, "additionalProperties": false }, + "storageservers_get_list": { + "type": "object", + "properties": { + "target_type": { + "description": "List storage servers of given type", + "type": "string", + "pattern": "^netfs$" + } + } + }, "template_update": { "type": "object", "properties": { diff --git a/src/kimchi/control/storageserver.py b/src/kimchi/control/storageserver.py new file mode 100644 index 0000000..0d4cb05 --- /dev/null +++ b/src/kimchi/control/storageserver.py @@ -0,0 +1,38 @@ +# +# Project Kimchi +# +# Copyright IBM, Corp. 2014 +# +# Authors: +# Royce Lv <lvroyce@linux.vnet.ibm.com> +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# 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 + + +class StorageServers(Collection): + def __init__(self, model): + super(StorageServers, self).__init__(model) + self.resource = StorageServer + + +class StorageServer(Resource): + def __init__(self, model, ident): + super(StorageServer, self).__init__(model, ident) + + @property + def data(self): + return self.info diff --git a/src/kimchi/root.py b/src/kimchi/root.py index 3cc6321..ec531c0 100644 --- a/src/kimchi/root.py +++ b/src/kimchi/root.py @@ -36,6 +36,7 @@ from kimchi.control.interfaces import Interfaces from kimchi.control.networks import Networks from kimchi.control.plugins import Plugins from kimchi.control.storagepools import StoragePools +from kimchi.control.storageserver import StorageServers from kimchi.control.tasks import Tasks from kimchi.control.templates import Templates from kimchi.control.utils import parse_request @@ -60,6 +61,7 @@ class Root(Resource): self.vms = VMs(model) self.templates = Templates(model) self.storagepools = StoragePools(model) + self.storageservers = StorageServers(model) self.interfaces = Interfaces(model) self.networks = Networks(model) self.tasks = Tasks(model) -- 1.8.1.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/14/2014 01:47 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add storage server collection and resource to report used storage server.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/API.json | 10 ++++++++++ src/kimchi/control/storageserver.py | 38 +++++++++++++++++++++++++++++++++++++ src/kimchi/root.py | 2 ++ 3 files changed, 50 insertions(+) create mode 100644 src/kimchi/control/storageserver.py
diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 19b1c51..d951696 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -214,6 +214,16 @@ }, "additionalProperties": false }, + "storageservers_get_list": { + "type": "object", + "properties": { + "target_type": { + "description": "List storage servers of given type", + "type": "string", + "pattern": "^netfs$" + } + } + }, "template_update": { "type": "object", "properties": { diff --git a/src/kimchi/control/storageserver.py b/src/kimchi/control/storageserver.py new file mode 100644 index 0000000..0d4cb05 --- /dev/null +++ b/src/kimchi/control/storageserver.py @@ -0,0 +1,38 @@ +# +# Project Kimchi +# +# Copyright IBM, Corp. 2014 +# +# Authors: +# Royce Lv <lvroyce@linux.vnet.ibm.com> +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# 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 + + +class StorageServers(Collection): + def __init__(self, model): + super(StorageServers, self).__init__(model) + self.resource = StorageServer + + +class StorageServer(Resource): + def __init__(self, model, ident): + super(StorageServer, self).__init__(model, ident) + + @property + def data(self): + return self.info diff --git a/src/kimchi/root.py b/src/kimchi/root.py index 3cc6321..ec531c0 100644 --- a/src/kimchi/root.py +++ b/src/kimchi/root.py @@ -36,6 +36,7 @@ from kimchi.control.interfaces import Interfaces from kimchi.control.networks import Networks from kimchi.control.plugins import Plugins from kimchi.control.storagepools import StoragePools +from kimchi.control.storageserver import StorageServers from kimchi.control.tasks import Tasks from kimchi.control.templates import Templates from kimchi.control.utils import parse_request @@ -60,6 +61,7 @@ class Root(Resource): self.vms = VMs(model) self.templates = Templates(model) self.storagepools = StoragePools(model) + self.storageservers = StorageServers(model) self.interfaces = Interfaces(model) self.networks = Networks(model) self.tasks = Tasks(model)
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Query all storage pool to retrieve storage server we used. If no query param is given, all supported type will be listed. With param given, only specified type of server is listed. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 30 ++++++++++++++++++++++++++++++ src/kimchi/model.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 1f5178f..04a55b3 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -403,6 +403,36 @@ class MockModel(object): iso_volumes.append(res) return iso_volumes + def storageservers_get_list(self, target_type=None): + # FIXME: When added new storage server support, this needs to be updated + target_type = kimchi.model.STORAGE_SOURCES.keys() \ + if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if (pool_info['type'] in target_type and + pool_info['source']['addr'] not in server_list): + server_list.append(pool_info['source']['addr']) + except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server: + return dict(addr=server) + except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup + pass + + raise NotFoundError + def dummy_interfaces(self): interfaces = {} ifaces = {"eth1": "nic", "bond0": "bonding", diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 671af02..3207e72 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -1292,6 +1292,36 @@ class Model(object): else: raise + def storageservers_get_list(self, target_type=None): + target_type = STORAGE_SOURCES.keys() if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if (pool_info['type'] in target_type and + pool_info['source']['addr'] not in server_list): + # Avoid to add same server for multiple times + # if it hosts more than one storage type + server_list.append(pool_info['source']['addr']) + except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server: + return dict(addr=server) + except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup + pass + + raise NotFoundError + def _get_screenshot(self, vm_uuid): with self.objstore as session: try: -- 1.8.1.2
On 01/14/2014 11:48 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Query all storage pool to retrieve storage server we used. If no query param is given, all supported type will be listed. With param given, only specified type of server is listed.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 30 ++++++++++++++++++++++++++++++ src/kimchi/model.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+)
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 1f5178f..04a55b3 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -403,6 +403,36 @@ class MockModel(object): iso_volumes.append(res) return iso_volumes
+ def storageservers_get_list(self, target_type=None): + # FIXME: When added new storage server support, this needs to be updated + target_type = kimchi.model.STORAGE_SOURCES.keys() \ + if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if (pool_info['type'] in target_type and + pool_info['source']['addr'] not in server_list): + server_list.append(pool_info['source']['addr']) + except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server: + return dict(addr=server) + except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup + pass + + raise NotFoundError + def dummy_interfaces(self): interfaces = {} ifaces = {"eth1": "nic", "bond0": "bonding", diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 671af02..3207e72 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -1292,6 +1292,36 @@ class Model(object): else: raise
+ def storageservers_get_list(self, target_type=None): + target_type = STORAGE_SOURCES.keys() if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if (pool_info['type'] in target_type and + pool_info['source']['addr'] not in server_list): not all types of pools have ['source']['addr'], so it will cause a KeyError. + # Avoid to add same server for multiple times + # if it hosts more than one storage type + server_list.append(pool_info['source']['addr']) + except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server: Same here. And I am also wondering any pool doesn't have 'source'? + return dict(addr=server) + except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup + pass + + raise NotFoundError + def _get_screenshot(self, vm_uuid): with self.objstore as session: try:
On 01/14/2014 01:48 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Query all storage pool to retrieve storage server we used. If no query param is given, all supported type will be listed. With param given, only specified type of server is listed.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 30 ++++++++++++++++++++++++++++++ src/kimchi/model.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+)
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 1f5178f..04a55b3 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -403,6 +403,36 @@ class MockModel(object): iso_volumes.append(res) return iso_volumes
+ def storageservers_get_list(self, target_type=None): + # FIXME: When added new storage server support, this needs to be updated
STORAGE_SOURCES only contains 'netfs' but it should also contain 'iscsi'
+ target_type = kimchi.model.STORAGE_SOURCES.keys() \ + if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if (pool_info['type'] in target_type and + pool_info['source']['addr'] not in server_list): + server_list.append(pool_info['source']['addr'])
source has no paremter named 'addr' It should be 'host'
+ except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server: + return dict(addr=server) + except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup + pass + + raise NotFoundError
Add a message while raising the exception above
+ def dummy_interfaces(self): interfaces = {} ifaces = {"eth1": "nic", "bond0": "bonding", diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 671af02..3207e72 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -1292,6 +1292,36 @@ class Model(object): else: raise
+ def storageservers_get_list(self, target_type=None): + target_type = STORAGE_SOURCES.keys() if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if (pool_info['type'] in target_type and + pool_info['source']['addr'] not in server_list): + # Avoid to add same server for multiple times + # if it hosts more than one storage type + server_list.append(pool_info['source']['addr'])
source has no paremter named 'addr' It should be 'host'
+ except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server:
source has no paremter named 'addr' It should be 'host'
+ return dict(addr=server)
You should also return the target_type [ { host: ..., target_type: [...] }, ... ]
+ except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup + pass + + raise NotFoundError + def _get_screenshot(self, vm_uuid): with self.objstore as session: try:
On 01/14/2014 01:48 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Query all storage pool to retrieve storage server we used. If no query param is given, all supported type will be listed. With param given, only specified type of server is listed.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 30 ++++++++++++++++++++++++++++++ src/kimchi/model.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+)
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 1f5178f..04a55b3 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -403,6 +403,36 @@ class MockModel(object): iso_volumes.append(res) return iso_volumes
+ def storageservers_get_list(self, target_type=None): + # FIXME: When added new storage server support, this needs to be updated
STORAGE_SOURCES only contains 'netfs' but it should also contain 'iscsi' True, but can we add iscsi in a independant patch after nfs is merged? I would like this framework support merged first, based on a agreed
On 2014年01月18日 02:24, Aline Manera wrote: framework, the other type can be easily accomplished and more focus on its functionality.
+ target_type = kimchi.model.STORAGE_SOURCES.keys() \ + if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if (pool_info['type'] in target_type and + pool_info['source']['addr'] not in server_list): + server_list.append(pool_info['source']['addr'])
source has no paremter named 'addr' It should be 'host'
No, it is not libvirt response, it is kimchi defined data, see commit acbcf04b8892b1c8876344 which I added to parse storage source information.
+ except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server: + return dict(addr=server) + except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup + pass + + raise NotFoundError
Add a message while raising the exception above
ACK
+ def dummy_interfaces(self): interfaces = {} ifaces = {"eth1": "nic", "bond0": "bonding", diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 671af02..3207e72 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -1292,6 +1292,36 @@ class Model(object): else: raise
+ def storageservers_get_list(self, target_type=None): + target_type = STORAGE_SOURCES.keys() if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if (pool_info['type'] in target_type and + pool_info['source']['addr'] not in server_list): + # Avoid to add same server for multiple times + # if it hosts more than one storage type + server_list.append(pool_info['source']['addr'])
source has no paremter named 'addr' It should be 'host'
Same as above.
+ except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server:
source has no paremter named 'addr' It should be 'host'
Same as above.
+ return dict(addr=server)
You should also return the target_type
[ { host: ..., target_type: [...] }, ... ]
Scenario is : GET /storageservers?target_type=netfs return: { {'host': 'localhost', 'type': 'netfs, iscsi'} } Is that necessary? Users only care about servers.
+ except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup + pass + + raise NotFoundError + def _get_screenshot(self, vm_uuid): with self.objstore as session: try:
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add colleciton of storage targets to API.md. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/API.md b/docs/API.md index 7142117..88787d8 100644 --- a/docs/API.md +++ b/docs/API.md @@ -471,6 +471,15 @@ creation. * **GET**: Retrieve a summarized list of used storage servers. * target_type: Filter server list with given type, currently support 'netfs'. +### Collection: Storage Targets + +**URI:** /storageservers/*:name*/storagetargets + +**Methods:** + +* **GET**: Retrieve a list of available storage targets. + * target_type: Filter target list with given type, currently support 'netfs'. + ### Collection: Distros **URI:** /config/distros -- 1.8.1.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/14/2014 01:48 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add colleciton of storage targets to API.md.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 7142117..88787d8 100644 --- a/docs/API.md +++ b/docs/API.md @@ -471,6 +471,15 @@ creation. * **GET**: Retrieve a summarized list of used storage servers. * target_type: Filter server list with given type, currently support 'netfs'.
+### Collection: Storage Targets + +**URI:** /storageservers/*:name*/storagetargets + +**Methods:** + +* **GET**: Retrieve a list of available storage targets. + * target_type: Filter target list with given type, currently support 'netfs'. + ### Collection: Distros
**URI:** /config/distros
On 01/17/2014 04:38 PM, Aline Manera wrote:
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com>
On 01/14/2014 01:48 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add colleciton of storage targets to API.md.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 7142117..88787d8 100644 --- a/docs/API.md +++ b/docs/API.md @@ -471,6 +471,15 @@ creation. * **GET**: Retrieve a summarized list of used storage servers. * target_type: Filter server list with given type, currently support 'netfs'.
+### Collection: Storage Targets + +**URI:** /storageservers/*:name*/storagetargets + +**Methods:** + +* **GET**: Retrieve a list of available storage targets. + * target_type: Filter target list with given type, currently support 'netfs'. +
You also need to add "Storage target (Resource)" and which parameters GET returns
### Collection: Distros
**URI:** /config/distros
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
On 2014年01月18日 02:46, Aline Manera wrote:
On 01/17/2014 04:38 PM, Aline Manera wrote:
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com>
On 01/14/2014 01:48 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add colleciton of storage targets to API.md.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 7142117..88787d8 100644 --- a/docs/API.md +++ b/docs/API.md @@ -471,6 +471,15 @@ creation. * **GET**: Retrieve a summarized list of used storage servers. * target_type: Filter server list with given type, currently support 'netfs'.
+### Collection: Storage Targets + +**URI:** /storageservers/*:name*/storagetargets + +**Methods:** + +* **GET**: Retrieve a list of available storage targets. + * target_type: Filter target list with given type, currently support 'netfs'. +
You also need to add "Storage target (Resource)" and which parameters GET returns GET /storageservers/*:name*/storagetargets?target_type=netfs returns 2 targets: [{'host': 'localhost', 'type': 'netfs', 'path':'/abcde'},{'host': 'localhost', 'type': 'netfs', 'path':'/efgh'}] Its already complete information.
Do we need more information for individual target:{'host': 'localhost', 'type': 'netfs', 'path':'/abcde'}? I can't figure out what information that will be.
### Collection: Distros
**URI:** /config/distros
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add json schema to validate mandatory param of target_type, also update controller.py. Reload the get_list function because we don't need to query each target. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/API.json | 10 ++++++++++ src/kimchi/control/storageserver.py | 24 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/kimchi/API.json b/src/kimchi/API.json index d951696..2c76446 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -224,6 +224,16 @@ } } }, + "storagetargets_get_list": { + "type": "object", + "properties": { + "target_type": { + "description": "List storage servers of given type", + "type": "string", + "pattern": "^netfs$" + } + } + }, "template_update": { "type": "object", "properties": { diff --git a/src/kimchi/control/storageserver.py b/src/kimchi/control/storageserver.py index 0d4cb05..364f3d0 100644 --- a/src/kimchi/control/storageserver.py +++ b/src/kimchi/control/storageserver.py @@ -21,6 +21,9 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA from kimchi.control.base import Collection, Resource +from kimchi.control.utils import parse_param, validate_params +from kimchi.control.utils import get_class_name, model_fn +import kimchi.template class StorageServers(Collection): @@ -36,3 +39,24 @@ class StorageServer(Resource): @property def data(self): return self.info + + def _cp_dispatch(self, vpath): + if vpath: + subcollection = vpath.pop(0) + if subcollection == 'storagetargets': + # incoming text, from URL, is not unicode, need decode + return StorageTargets(self.model, self.ident.decode("utf-8")) + + +class StorageTargets(Collection): + def __init__(self, model, server): + super(StorageTargets, self).__init__(model) + self.server = server + self.resource_args = [self.server, ] + self.model_args = [self.server, ] + + def get(self, filter_params): + res_list = [] + get_list = getattr(self.model, model_fn(self, 'get_list')) + res_list = get_list(*self.model_args, **filter_params) + return kimchi.template.render(get_class_name(self), res_list) -- 1.8.1.2
On 01/14/2014 01:48 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add json schema to validate mandatory param of target_type, also update controller.py. Reload the get_list function because we don't need to query each target.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/API.json | 10 ++++++++++ src/kimchi/control/storageserver.py | 24 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+)
diff --git a/src/kimchi/API.json b/src/kimchi/API.json index d951696..2c76446 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -224,6 +224,16 @@ } } }, + "storagetargets_get_list": { + "type": "object", + "properties": { + "target_type": { + "description": "List storage servers of given type", + "type": "string", + "pattern": "^netfs$"
Need to allow 'iscsi' too
+ } + } + }, "template_update": { "type": "object", "properties": { diff --git a/src/kimchi/control/storageserver.py b/src/kimchi/control/storageserver.py index 0d4cb05..364f3d0 100644 --- a/src/kimchi/control/storageserver.py +++ b/src/kimchi/control/storageserver.py @@ -21,6 +21,9 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
from kimchi.control.base import Collection, Resource +from kimchi.control.utils import parse_param, validate_params +from kimchi.control.utils import get_class_name, model_fn +import kimchi.template
class StorageServers(Collection): @@ -36,3 +39,24 @@ class StorageServer(Resource): @property def data(self): return self.info + + def _cp_dispatch(self, vpath): + if vpath: + subcollection = vpath.pop(0) + if subcollection == 'storagetargets': + # incoming text, from URL, is not unicode, need decode + return StorageTargets(self.model, self.ident.decode("utf-8")) + + +class StorageTargets(Collection): + def __init__(self, model, server): + super(StorageTargets, self).__init__(model) + self.server = server + self.resource_args = [self.server, ] + self.model_args = [self.server, ] + + def get(self, filter_params): + res_list = [] + get_list = getattr(self.model, model_fn(self, 'get_list')) + res_list = get_list(*self.model_args, **filter_params) + return kimchi.template.render(get_class_name(self), res_list)
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Construct xml to query storage targets information from storage server. Use lxml to parse result instead of etree. Use lxml to parse target query result. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 3207e72..c0d82eb 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -45,6 +45,7 @@ import uuid from cherrypy.process.plugins import BackgroundTask from cherrypy.process.plugins import SimplePlugin from collections import defaultdict +from lxml import objectify from xml.etree import ElementTree @@ -1322,6 +1323,24 @@ class Model(object): raise NotFoundError + def storagetargets_get_list(self, storage_server, target_type=None): + target_types = STORAGE_SOURCES.keys() if not target_type else [target_type] + target_list = list() + + for target_type in target_types: + xml = _get_storage_server_spec(server=storage_server, target_type=target_type) + conn = self.conn.get() + + try: + ret = conn.findStoragePoolSources(target_type, xml, 0) + except libvirt.libvirtError as e: + kimchi_log.warning("Query storage pool source fails because of %s", + e.get_error_message()) + continue + + target_list.extend(_parse_target_source_result(target_type, ret)) + return target_list + def _get_screenshot(self, vm_uuid): with self.objstore as session: try: @@ -1532,6 +1551,35 @@ class LibvirtVMScreenshot(VMScreenshot): os.close(fd) +def _parse_target_source_result(target_type, xml_str): + root = objectify.fromstring(xml_str) + ret = [] + for source in root.getchildren(): + if target_type == 'netfs': + host_name = source.host.get('name') + target_path = source.dir.get('path') + type = source.format.get('type') + ret.append(dict(host=host_name, target_type=type, target=target_path)) + return ret + + +def _get_storage_server_spec(**kwargs): + # Required parameters: + # server: + # target_type: + if kwargs['target_type'] == 'netfs': + kwargs['format_type'] = """<format type='nfs'/>""" + else: + kwargs['format_type'] = "" + xml = """ + <source> + <host name='%(server)s'/> + %(format_type)s + </source> + """ % kwargs + return xml + + class StoragePoolDef(object): @classmethod def create(cls, poolArgs): -- 1.8.1.2
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Construct xml to query storage targets information from storage server. Use lxml to parse result instead of etree. Use lxml to parse target query result.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 3207e72..c0d82eb 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -45,6 +45,7 @@ import uuid from cherrypy.process.plugins import BackgroundTask from cherrypy.process.plugins import SimplePlugin from collections import defaultdict +from lxml import objectify from xml.etree import ElementTree
@@ -1322,6 +1323,24 @@ class Model(object):
raise NotFoundError
+ def storagetargets_get_list(self, storage_server, target_type=None): + target_types = STORAGE_SOURCES.keys() if not target_type else [target_type] + target_list = list() + + for target_type in target_types: + xml = _get_storage_server_spec(server=storage_server, target_type=target_type) + conn = self.conn.get() + + try: + ret = conn.findStoragePoolSources(target_type, xml, 0) + except libvirt.libvirtError as e: + kimchi_log.warning("Query storage pool source fails because of %s", + e.get_error_message()) + continue + + target_list.extend(_parse_target_source_result(target_type, ret)) + return target_list + def _get_screenshot(self, vm_uuid): with self.objstore as session: try: @@ -1532,6 +1551,35 @@ class LibvirtVMScreenshot(VMScreenshot): os.close(fd)
+def _parse_target_source_result(target_type, xml_str): + root = objectify.fromstring(xml_str) + ret = [] + for source in root.getchildren(): + if target_type == 'netfs': + host_name = source.host.get('name') + target_path = source.dir.get('path') + type = source.format.get('type') + ret.append(dict(host=host_name, target_type=type, target=target_path))
On 01/14/2014 11:48 PM, lvroyce0210@gmail.com wrote: the above statement should belong to the if block.
+ return ret + + +def _get_storage_server_spec(**kwargs): + # Required parameters: + # server: + # target_type: + if kwargs['target_type'] == 'netfs': + kwargs['format_type'] = """<format type='nfs'/>""" + else: + kwargs['format_type'] = "" + xml = """ + <source> + <host name='%(server)s'/> + %(format_type)s + </source> + """ % kwargs + return xml You could use lxm.builder to generate XML from arguments. + + class StoragePoolDef(object): @classmethod def create(cls, poolArgs):
On 01/14/2014 01:48 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Construct xml to query storage targets information from storage server. Use lxml to parse result instead of etree. Use lxml to parse target query result.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 3207e72..c0d82eb 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -45,6 +45,7 @@ import uuid from cherrypy.process.plugins import BackgroundTask from cherrypy.process.plugins import SimplePlugin from collections import defaultdict +from lxml import objectify from xml.etree import ElementTree
@@ -1322,6 +1323,24 @@ class Model(object):
raise NotFoundError
+ def storagetargets_get_list(self, storage_server, target_type=None): + target_types = STORAGE_SOURCES.keys() if not target_type else [target_type] + target_list = list() + + for target_type in target_types: + xml = _get_storage_server_spec(server=storage_server, target_type=target_type) + conn = self.conn.get() + + try: + ret = conn.findStoragePoolSources(target_type, xml, 0) + except libvirt.libvirtError as e: + kimchi_log.warning("Query storage pool source fails because of %s", + e.get_error_message()) + continue + + target_list.extend(_parse_target_source_result(target_type, ret)) + return target_list + def _get_screenshot(self, vm_uuid): with self.objstore as session: try: @@ -1532,6 +1551,35 @@ class LibvirtVMScreenshot(VMScreenshot): os.close(fd)
+def _parse_target_source_result(target_type, xml_str): + root = objectify.fromstring(xml_str) + ret = [] + for source in root.getchildren(): + if target_type == 'netfs': + host_name = source.host.get('name') + target_path = source.dir.get('path') + type = source.format.get('type') + ret.append(dict(host=host_name, target_type=type, target=target_path))
You need to add which parameters returned in API
+ return ret + + +def _get_storage_server_spec(**kwargs): + # Required parameters: + # server: + # target_type: + if kwargs['target_type'] == 'netfs': + kwargs['format_type'] = """<format type='nfs'/>""" + else: + kwargs['format_type'] = "" + xml = """ + <source> + <host name='%(server)s'/> + %(format_type)s + </source> + """ % kwargs + return xml + + class StoragePoolDef(object): @classmethod def create(cls, poolArgs):
On 2014年01月18日 02:48, Aline Manera wrote:
On 01/14/2014 01:48 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Construct xml to query storage targets information from storage server. Use lxml to parse result instead of etree. Use lxml to parse target query result.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 3207e72..c0d82eb 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -45,6 +45,7 @@ import uuid from cherrypy.process.plugins import BackgroundTask from cherrypy.process.plugins import SimplePlugin from collections import defaultdict +from lxml import objectify from xml.etree import ElementTree
@@ -1322,6 +1323,24 @@ class Model(object):
raise NotFoundError
+ def storagetargets_get_list(self, storage_server, target_type=None): + target_types = STORAGE_SOURCES.keys() if not target_type else [target_type] + target_list = list() + + for target_type in target_types: + xml = _get_storage_server_spec(server=storage_server, target_type=target_type) + conn = self.conn.get() + + try: + ret = conn.findStoragePoolSources(target_type, xml, 0) + except libvirt.libvirtError as e: + kimchi_log.warning("Query storage pool source fails because of %s", + e.get_error_message()) + continue + + target_list.extend(_parse_target_source_result(target_type, ret)) + return target_list + def _get_screenshot(self, vm_uuid): with self.objstore as session: try: @@ -1532,6 +1551,35 @@ class LibvirtVMScreenshot(VMScreenshot): os.close(fd)
+def _parse_target_source_result(target_type, xml_str): + root = objectify.fromstring(xml_str) + ret = [] + for source in root.getchildren(): + if target_type == 'netfs': + host_name = source.host.get('name') + target_path = source.dir.get('path') + type = source.format.get('type') + ret.append(dict(host=host_name, target_type=type, target=target_path))
You need to add which parameters returned in API I agree, normally, we list the get list returned parameter under the API.md. But now, as added GET accept parameter, I added that in API.md, so the API.md parameter and response need to be refactored. I will do it in a single patch if you agree.
+ return ret + + +def _get_storage_server_spec(**kwargs): + # Required parameters: + # server: + # target_type: + if kwargs['target_type'] == 'netfs': + kwargs['format_type'] = """<format type='nfs'/>""" + else: + kwargs['format_type'] = "" + xml = """ + <source> + <host name='%(server)s'/> + %(format_type)s + </source> + """ % kwargs + return xml + + class StoragePoolDef(object): @classmethod def create(cls, poolArgs):
On 2014年01月14日 23:47, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
v6>v7, adopt lxml to parse xml, move parse params to get() to avoid duplicate code, fix bugs when one server support multiple target type. v5>v6, change GET param support to cover more scenario of filter collected results. v4>v5, remove storage server list reload function, merge storage server and targets v3>v4, fix inconsistency between doc and json schema v1>v3, fix racing problem, fix style. Royce Lv (8): Support params for GET method Add testcase for GET param Storage server: Update API.md storage server: update controller.py storage server: Update model and mockmodel storage target: Update API.md storage target: Update controller and json schema storage target: Add model support
docs/API.md | 22 ++++++++++ src/kimchi/API.json | 20 +++++++++ src/kimchi/control/base.py | 28 +++++++++---- src/kimchi/control/storagepools.py | 4 +- src/kimchi/control/storageserver.py | 62 ++++++++++++++++++++++++++++ src/kimchi/control/storagevolumes.py | 2 +- src/kimchi/control/utils.py | 2 + src/kimchi/mockmodel.py | 30 ++++++++++++++ src/kimchi/model.py | 78 ++++++++++++++++++++++++++++++++++++ src/kimchi/root.py | 2 + tests/test_rest.py | 34 ++++++++++++++++ 11 files changed, 273 insertions(+), 11 deletions(-) create mode 100644 src/kimchi/control/storageserver.py
participants (4)
-
Aline Manera -
lvroyce0210＠gmail.com -
Mark Wu -
Royce Lv