8:46 a.m.
Changelog from previous version (v1):
- Remove the text "downloading" from the download task status message.
Now it only contains the data: <downloaded size>/<total size>.
Crístian Viana (3):
storagevolume: Download remote images to a storage pool
storagevolume: Check storage pool before adding a volume
storagevolume: Add download progress to task
docs/API.md | 1 +
src/kimchi/mockmodel.py | 39 ++++++++++++++++++++++-----
src/kimchi/model/storagevolumes.py | 54 +++++++++++++++++++++++++++++++++++---
tests/test_model.py | 24 +++++++++++++++++
tests/test_rest.py | 14 +++++++++-
5 files changed, 120 insertions(+), 12 deletions(-)
--
1.9.3
8:46 a.m.
New subject: [PATCH v2 1/3] storagevolume: Download remote images to a storage pool
In order to simplify the creation of a remote image in Kimchi, the user
will be able to provide a remote URL and the Kimchi server will download
it and put it on a storage pool.
Download a remote image to a storage pool
Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
docs/API.md | 1 +
src/kimchi/mockmodel.py | 20 ++++++++++++++++++++
src/kimchi/model/storagevolumes.py | 32 ++++++++++++++++++++++++++++++++
tests/test_model.py | 24 ++++++++++++++++++++++++
tests/test_rest.py | 12 ++++++++++++
5 files changed, 89 insertions(+)
diff --git a/docs/API.md b/docs/API.md
index 298441f..282c8e0 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -418,6 +418,7 @@ A interface represents available network interface on VM.
* capacity: The total space which can be used to store volumes
The unit is MBytes
* format: The format of the defined Storage Volume
+ * url: URL to be downloaded
### Resource: Storage Volume
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
index 05e3fa4..9fab036 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -524,6 +524,26 @@ class MockModel(object):
pool._volumes[name] = volume
cb('OK', True)
+ def _create_volume_with_url(self, cb, params):
+ pool_name = params['pool']
+ name = params['name']
+ url = params['url']
+
+ pool = self._get_storagepool(pool_name)
+
+ file_path = os.path.join(pool.info['path'], name)
+
+ with open(file_path, 'w') as file:
+ file.write(url)
+
+ params['path'] = file_path
+ params['type'] = 'file'
+
+ volume = MockStorageVolume(pool, name, params)
+ pool._volumes[name] = volume
+
+ cb('OK', True)
+
def storagevolume_lookup(self, pool, name):
if self._get_storagepool(pool).info['state'] != 'active':
raise InvalidOperation("KCHVOL0005E", {'pool': pool,
diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py
index 6b001f7..3062e78 100644
--- a/src/kimchi/model/storagevolumes.py
+++ b/src/kimchi/model/storagevolumes.py
@@ -17,7 +17,9 @@
# License along with this library; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import contextlib
import os
+import urllib2
import libvirt
@@ -39,6 +41,9 @@ VOLUME_TYPE_MAP = {0: 'file',
3: 'network'}
+DOWNLOAD_CHUNK_SIZE = 1048576 # 1 MiB
+
+
class StorageVolumesModel(object):
def __init__(self, **kargs):
self.conn = kargs['conn']
@@ -112,6 +117,33 @@ class StorageVolumesModel(object):
cb('', True)
+ def _create_volume_with_url(self, cb, params):
+ pool_name = params['pool']
+ name = params['name']
+ url = params['url']
+
+ pool_model = StoragePoolModel(conn=self.conn,
+ objstore=self.objstore)
+ pool = pool_model.lookup(pool_name)
+ file_path = os.path.join(pool['path'], name)
+
+ with contextlib.closing(urllib2.urlopen(url)) as response,\
+ open(file_path, 'w') as volume_file:
+ try:
+ while True:
+ chunk_data = response.read(DOWNLOAD_CHUNK_SIZE)
+ if not chunk_data:
+ break
+
+ volume_file.write(chunk_data)
+ except Exception, e:
+ raise OperationFailed('KCHVOL0007E', {'name': name,
+ 'pool': pool_name,
+ 'err': e.message})
+
+ StoragePoolModel.get_storagepool(pool_name, self.conn).refresh()
+ cb('OK', True)
+
def get_list(self, pool_name):
pool = StoragePoolModel.get_storagepool(pool_name, self.conn)
if not pool.isActive():
diff --git a/tests/test_model.py b/tests/test_model.py
index 5ee824d..4e9ba97 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -36,6 +36,7 @@ import iso_gen
import kimchi.objectstore
import utils
from kimchi import netinfo
+from kimchi.config import paths
from kimchi.exception import InvalidOperation, InvalidParameter
from kimchi.exception import NotFoundError, OperationFailed
from kimchi.iscsi import TargetClient
@@ -557,6 +558,29 @@ class ModelTests(unittest.TestCase):
poolinfo = inst.storagepool_lookup(pool)
self.assertEquals(len(vols), poolinfo['nr_volumes'])
+ # download remote volume
+ # 1) try an invalid URL
+ params = {'name': 'foo', 'url':
'http://www.invalid.url'}
+ taskid = inst.storagevolumes_create(pool, params)['id']
+ self._wait_task(inst, taskid)
+ self.assertEquals('failed',
inst.task_lookup(taskid)['status'])
+ # 2) download Kimchi's "COPYING" from Github and compare its
+ # content to the corresponding local file's
+ url = 'https://github.com/kimchi-project/kimchi/raw/master/COPYING'
+ params = {'name': 'copying', 'url': url}
+ taskid = inst.storagevolumes_create(pool, params)['id']
+ self._wait_task(inst, taskid)
+ self.assertEquals('finished',
inst.task_lookup(taskid)['status'])
+ rollback.prependDefer(inst.storagevolume_delete, pool,
+ params['name'])
+ vol_path = os.path.join(args['path'], params['name'])
+ self.assertTrue(os.path.isfile(vol_path))
+ with open(vol_path) as vol_file:
+ vol_content = vol_file.read()
+ with open(os.path.join(paths.get_prefix(), 'COPYING')) as cp_file:
+ cp_content = cp_file.read()
+ self.assertEquals(vol_content, cp_content)
+
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
def test_template_storage_customise(self):
inst = model.Model(objstore_loc=self.tmp_store)
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 9e14b6d..b325201 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -1026,6 +1026,18 @@ class RestTests(unittest.TestCase):
self.assertEquals('/var/lib/libvirt/images/volume-1',
storagevolume['path'])
+ req = json.dumps({'name': 'downloaded',
+ 'url': 'https://anyurl.wor.kz'})
+ resp = self.request('/storagepools/pool-1/storagevolumes', req,
'POST')
+ self.assertEquals(202, resp.status)
+ task = json.loads(resp.read())
+ self._wait_task(task['id'])
+ task = json.loads(self.request('/tasks/%s' % task['id']).read())
+ self.assertEquals('finished', task['status'])
+ resp = self.request('/storagepools/pool-1/storagevolumes/downloaded',
+ '{}', 'GET')
+ self.assertEquals(200, resp.status)
+
# Now remove the StoragePool from mock model
self._delete_pool('pool-1')
--
1.9.3
8:46 a.m.
New subject: [PATCH v2 2/3] storagevolume: Check storage pool before adding a volume
In order to add a volume to a storage pool, it must not be one of a
read-only type, it must be active and the volume name must not already
exist. That verification should happen regardless of the storage volume
creation method (i.e. download, upload).
Add the verifications mentioned above when creating any type of volume by
adding them to the common "create" function.
Also, fix a test case which was broken (commit ff1bb81) because a
storage volume with an existing name was being added.
Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
src/kimchi/mockmodel.py | 21 +++++++++++++--------
src/kimchi/model/storagevolumes.py | 17 +++++++++++++----
tests/test_rest.py | 2 +-
3 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
index 9fab036..a772f50 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -43,7 +43,7 @@ except ImportError:
from kimchi import config
from kimchi.asynctask import AsyncTask
-from kimchi.config import config as kconfig
+from kimchi.config import READONLY_POOL_TYPE, config as kconfig
from kimchi.distroloader import DistroLoader
from kimchi.exception import InvalidOperation, InvalidParameter
from kimchi.exception import MissingParameter, NotFoundError, OperationFailed
@@ -480,6 +480,8 @@ class MockModel(object):
def storagevolumes_create(self, pool_name, params):
vol_source = ['file', 'url', 'capacity']
+ name = params['name']
+
index_list = list(i for i in range(len(vol_source))
if vol_source[i] in params)
if len(index_list) != 1:
@@ -492,6 +494,16 @@ class MockModel(object):
except AttributeError:
raise InvalidParameter("KCHVOL0019E",
{'param': vol_source[index_list[0]]})
+
+ pool = self._get_storagepool(pool_name)
+ if pool.info['type'] in READONLY_POOL_TYPE:
+ raise InvalidParameter("KCHVOL0012E", {'type':
pool.info['type']})
+ if pool.info['state'] == 'inactive':
+ raise InvalidParameter('KCHVOL0003E', {'pool': pool_name,
+ 'volume': name})
+ if name in pool._volumes:
+ raise InvalidOperation("KCHVOL0001E", {'name': name})
+
params['pool'] = pool_name
taskid = self.add_task('', create_func, params)
return self.task_lookup(taskid)
@@ -499,10 +511,6 @@ class MockModel(object):
def _create_volume_with_capacity(self, cb, params):
pool_name = params.pop('pool')
pool = self._get_storagepool(pool_name)
- if pool.info['state'] == 'inactive':
- raise InvalidOperation("KCHVOL0003E",
- {'pool': pool_name,
- 'volume': params['name']})
try:
name = params['name']
@@ -518,9 +526,6 @@ class MockModel(object):
raise MissingParameter("KCHVOL0004E",
{'item': str(item), 'volume': name})
- if name in pool._volumes:
- raise InvalidOperation("KCHVOL0001E", {'name': name})
-
pool._volumes[name] = volume
cb('OK', True)
diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py
index 3062e78..fac34d5 100644
--- a/src/kimchi/model/storagevolumes.py
+++ b/src/kimchi/model/storagevolumes.py
@@ -53,6 +53,8 @@ class StorageVolumesModel(object):
def create(self, pool_name, params):
vol_source = ['file', 'url', 'capacity']
+ name = params['name']
+
index_list = list(i for i in range(len(vol_source))
if vol_source[i] in params)
if len(index_list) != 1:
@@ -65,6 +67,17 @@ class StorageVolumesModel(object):
except AttributeError:
raise InvalidParameter("KCHVOL0019E",
{'param': vol_source[index_list[0]]})
+
+ pool_info = StoragePoolModel(conn=self.conn,
+ objstore=self.objstore).lookup(pool_name)
+ if pool_info['type'] in READONLY_POOL_TYPE:
+ raise InvalidParameter("KCHVOL0012E", {'type':
pool_info['type']})
+ if pool_info['state'] == 'inactive':
+ raise InvalidParameter('KCHVOL0003E', {'pool': pool_name,
+ 'volume': name})
+ if name in self.get_list(pool_name):
+ raise InvalidParameter('KCHVOL0001E', {'name': name})
+
params['pool'] = pool_name
taskid = add_task('', create_func, self.objstore, params)
return self.task.lookup(taskid)
@@ -95,10 +108,6 @@ class StorageVolumesModel(object):
raise MissingParameter("KCHVOL0004E", {'item': str(item),
'volume': name})
- pool_info = StoragePoolModel(conn=self.conn,
- objstore=self.objstore).lookup(pool_name)
- if pool_info['type'] in READONLY_POOL_TYPE:
- raise InvalidParameter("KCHVOL0012E", {'type':
pool_info['type']})
try:
pool.createXML(xml, 0)
except libvirt.libvirtError as e:
diff --git a/tests/test_rest.py b/tests/test_rest.py
index b325201..227e5ac 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -1052,7 +1052,7 @@ class RestTests(unittest.TestCase):
'format': 'raw'})
resp = self.request('/storagepools/pool-2/storagevolumes/',
req, 'POST')
- self.assertEquals(202, resp.status)
+ self.assertEquals(400, resp.status)
resp = self.request('/storagepools/pool-2/activate', '{}',
'POST')
self.assertEquals(200, resp.status)
resp = self.request('/storagepools/pool-2/storagevolumes/',
--
1.9.3
8:46 a.m.
New subject: [PATCH v2 3/3] storagevolume: Add download progress to task
The download operation can take a long time, so it's nice to have some
way to track its current progress.
Update the task's message with the number of bytes downloaded. The
format is "<bytes downloaded>/<total bytes>". If the number of
bytes
cannot be read from the remote URL, <total bytes> will be "-".
Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
src/kimchi/model/storagevolumes.py | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py
index fac34d5..a1c8c9a 100644
--- a/src/kimchi/model/storagevolumes.py
+++ b/src/kimchi/model/storagevolumes.py
@@ -138,6 +138,9 @@ class StorageVolumesModel(object):
with contextlib.closing(urllib2.urlopen(url)) as response,\
open(file_path, 'w') as volume_file:
+ remote_size = response.info().getheader('Content-Length',
'-')
+ downloaded_size = 0
+
try:
while True:
chunk_data = response.read(DOWNLOAD_CHUNK_SIZE)
@@ -145,6 +148,8 @@ class StorageVolumesModel(object):
break
volume_file.write(chunk_data)
+ downloaded_size += len(chunk_data)
+ cb('%s/%s' % (downloaded_size, remote_size))
except Exception, e:
raise OperationFailed('KCHVOL0007E', {'name': name,
'pool': pool_name,
--
1.9.3
10:40 a.m.
Hi Cristian, seems something wrong when I send a request to create
volume from remote URL.
Remote Address:9.123.141.150:8001
Request URL:https://9.123.141.150:8001/storagepools/ISO/storagevolumes
Request Method:POST
Status Code:400 Bad Request
Request Payload:
{
name: "CorePure64-5.3.iso"
url:
"http://distro.ibiblio.org/tinycorelinux/5.x/x86_64/release/CorePure64-5.3.iso"
}
The response:
{
"reason":"KCHVOL0019E: Creating volume from url is not supported",
"code":"400 Bad Request"
}
On 09/04/2014 09:46 PM, Crístian Viana wrote:
Changelog from previous version (v1):
- Remove the text "downloading" from the download task status message.
Now it only contains the data: <downloaded size>/<total size>.
Crístian Viana (3):
storagevolume: Download remote images to a storage pool
storagevolume: Check storage pool before adding a volume
storagevolume: Add download progress to task
docs/API.md | 1 +
src/kimchi/mockmodel.py | 39 ++++++++++++++++++++++-----
src/kimchi/model/storagevolumes.py | 54 +++++++++++++++++++++++++++++++++++---
tests/test_model.py | 24 +++++++++++++++++
tests/test_rest.py | 14 +++++++++-
5 files changed, 120 insertions(+), 12 deletions(-)
10:58 a.m.
Hi Hongliang. Thanks for testing this patchset!
I don't understand why this isn't working for you. I just executed the
same command here on my laptop and it worked fine. Take a look at my
commands:
$ curl -k -u vianac:<password> -H "Content-Type: application/json" -H
"Accept: application/json"
https://127.0.0.1:8001/storagepools/default/storagevolumes -X POST -d
'{"name": "CorePure64-5.3.iso", "url":
"http://distro.ibiblio.org/tinycorelinux/5.x/x86_64/release/CorePure64-5.3.iso"}'
{
"status":"running",
"message":"OK",
"id":"2",
"target_uri":""
}
$ curl -k -u vianac:<password> -H "Content-Type: application/json" -H
"Accept: application/json" https://127.0.0.1:8001/tasks/2
{
"status":"running",
"message":"4194304/10772480",
"id":"2"
}
$ curl -k -u vianac:<password> -H "Content-Type: application/json" -H
"Accept: application/json" https://127.0.0.1:8001/tasks/2
{
"status":"running",
"message":"5242880/10772480",
"id":"2"
}
And there was no error stack trace on the console.
Are you sure you have these 3 patches applied? Could you please check if
the function "_create_volume_with_url" is defined at
src/kimchi/model/storagevolumes.py, line 129? The exception you
mentioned is supposed to be raised when that function doesn't exist.
On 04-09-2014 12:40, Hongliang Wang wrote:
Hi Cristian, seems something wrong when I send a request to create
volume from remote URL.
Remote Address:9.123.141.150:8001
Request URL:https://9.123.141.150:8001/storagepools/ISO/storagevolumes
Request Method:POST
Status Code:400 Bad Request
Request Payload:
{
name: "CorePure64-5.3.iso"
url:
"http://distro.ibiblio.org/tinycorelinux/5.x/x86_64/release/CorePure64-5.3.iso"
}
The response:
{
"reason":"KCHVOL0019E: Creating volume from url is not supported",
"code":"400 Bad Request"
}
On 09/04/2014 09:46 PM, Crístian Viana wrote:
> Changelog from previous version (v1):
>
> - Remove the text "downloading" from the download task status message.
> Now it only contains the data: <downloaded size>/<total size>.
>
> Crístian Viana (3):
> storagevolume: Download remote images to a storage pool
> storagevolume: Check storage pool before adding a volume
> storagevolume: Add download progress to task
>
> docs/API.md | 1 +
> src/kimchi/mockmodel.py | 39 ++++++++++++++++++++++-----
> src/kimchi/model/storagevolumes.py | 54
> +++++++++++++++++++++++++++++++++++---
> tests/test_model.py | 24 +++++++++++++++++
> tests/test_rest.py | 14 +++++++++-
> 5 files changed, 120 insertions(+), 12 deletions(-)
>
9:31 p.m.
Yes the patches were applied though I forgot to restart the server so
they didn't take effect. I restarted it and it works fine now! Thanks!
On 09/04/2014 11:58 PM, Crístian Viana wrote:
Hi Hongliang. Thanks for testing this patchset!
I don't understand why this isn't working for you. I just executed the
same command here on my laptop and it worked fine. Take a look at my
commands:
$ curl -k -u vianac:<password> -H "Content-Type: application/json" -H
"Accept: application/json"
https://127.0.0.1:8001/storagepools/default/storagevolumes -X POST -d
'{"name": "CorePure64-5.3.iso", "url":
"http://distro.ibiblio.org/tinycorelinux/5.x/x86_64/release/CorePure64-5.3.iso"}'
{
"status":"running",
"message":"OK",
"id":"2",
"target_uri":""
}
$ curl -k -u vianac:<password> -H "Content-Type: application/json" -H
"Accept: application/json" https://127.0.0.1:8001/tasks/2
{
"status":"running",
"message":"4194304/10772480",
"id":"2"
}
$ curl -k -u vianac:<password> -H "Content-Type: application/json" -H
"Accept: application/json" https://127.0.0.1:8001/tasks/2
{
"status":"running",
"message":"5242880/10772480",
"id":"2"
}
And there was no error stack trace on the console.
Are you sure you have these 3 patches applied? Could you please check
if the function "_create_volume_with_url" is defined at
src/kimchi/model/storagevolumes.py, line 129? The exception you
mentioned is supposed to be raised when that function doesn't exist.
On 04-09-2014 12:40, Hongliang Wang wrote:
> Hi Cristian, seems something wrong when I send a request to create
> volume from remote URL.
>
> Remote Address:9.123.141.150:8001
> Request URL:https://9.123.141.150:8001/storagepools/ISO/storagevolumes
> Request Method:POST
> Status Code:400 Bad Request
>
> Request Payload:
> {
> name: "CorePure64-5.3.iso"
> url:
>
"http://distro.ibiblio.org/tinycorelinux/5.x/x86_64/release/CorePure64-5.3.iso"
> }
>
> The response:
>
> {
> "reason":"KCHVOL0019E: Creating volume from url is not
supported",
> "code":"400 Bad Request"
> }
>
> On 09/04/2014 09:46 PM, Crístian Viana wrote:
>> Changelog from previous version (v1):
>>
>> - Remove the text "downloading" from the download task status message.
>> Now it only contains the data: <downloaded size>/<total size>.
>>
>> Crístian Viana (3):
>> storagevolume: Download remote images to a storage pool
>> storagevolume: Check storage pool before adding a volume
>> storagevolume: Add download progress to task
>>
>> docs/API.md | 1 +
>> src/kimchi/mockmodel.py | 39 ++++++++++++++++++++++-----
>> src/kimchi/model/storagevolumes.py | 54
>> +++++++++++++++++++++++++++++++++++---
>> tests/test_model.py | 24 +++++++++++++++++
>> tests/test_rest.py | 14 +++++++++-
>> 5 files changed, 120 insertions(+), 12 deletions(-)
>>
>
3524
days inactive
3525
days old
7 comments
3 participants
participants (3)
-
Aline Manera -
Crístian Viana -
Hongliang Wang