[PATCH v2 0/3] Download remote image to storage pool
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
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@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
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@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
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@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
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.is..." } 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(-)
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.is..." }
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(-)
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.is..." }
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(-)
participants (3)
-
Aline Manera -
Crístian Viana -
Hongliang Wang