[PATCH v3 0/4] Download remote image to storage pool
Changelog from the previous version (v2): - Update src/kimchi/API.json with the new parameter "url" in function "storagevolumes_create". - New commit: "storagevolume: Set target URI when creating Task". Crístian Viana (4): storagevolume: Download remote images to a storage pool storagevolume: Check storage pool before adding a volume storagevolume: Add download progress to task storagevolume: Set target URI when creating Task docs/API.md | 1 + src/kimchi/API.json | 6 ++++ src/kimchi/i18n.py | 1 + src/kimchi/mockmodel.py | 42 ++++++++++++++++++++++------ src/kimchi/model/storagevolumes.py | 57 ++++++++++++++++++++++++++++++++++---- tests/test_model.py | 24 ++++++++++++++++ tests/test_rest.py | 14 +++++++++- 7 files changed, 131 insertions(+), 14 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/API.json | 6 ++++++ src/kimchi/i18n.py | 1 + src/kimchi/mockmodel.py | 20 ++++++++++++++++++++ src/kimchi/model/storagevolumes.py | 32 ++++++++++++++++++++++++++++++++ tests/test_model.py | 24 ++++++++++++++++++++++++ tests/test_rest.py | 12 ++++++++++++ 7 files changed, 96 insertions(+) diff --git a/docs/API.md b/docs/API.md index 0c4a641..9b1bff3 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/API.json b/src/kimchi/API.json index 2e2f9b0..8a95804 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -182,6 +182,12 @@ "type": "string", "pattern": "^qcow2|raw$", "error": "KCHVOL0015E" + }, + "url": { + "description": "The remote URL of the storage volume", + "type": "string", + "pattern": "^(http|ftp)[s]?://", + "error": "KCHVOL0021E" } } }, diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 1e8b47a..bbe4b02 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -188,6 +188,7 @@ messages = { "KCHVOL0018E": _("Only one of %(param)s can be specified"), "KCHVOL0019E": _("Creating volume from %(param)s is not supported"), "KCHVOL0020E": _("Storage volume capacity must be an integer number."), + "KCHVOL0021E": _("Storage volume URL must be http://, https://, ftp:// or ftps://."), "KCHIFACE0001E": _("Interface %(name)s does not exist"), diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index b94c3fe..5241696 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 7b8dfc2..0df435d 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 5241696..a3f720f 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 0df435d..567a954 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
Commit f8f43b3 has exposed the field "target_uri" out of the Task structure. The function "storagevolumes_create" creates Tasks to perform that operation asynchronously but the corresponding "target_uri" isn't set. Set the field "target_uri" when creating a storage volume according to the format "/storagepools/<pool-name>/storagevolumes/<vol-name>". Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 3 ++- src/kimchi/model/storagevolumes.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index a3f720f..908b0b7 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -505,7 +505,8 @@ class MockModel(object): raise InvalidOperation("KCHVOL0001E", {'name': name}) params['pool'] = pool_name - taskid = self.add_task('', create_func, params) + targeturi = '/storagepools/%s/storagevolumes/%s' % (pool_name, name) + taskid = self.add_task(targeturi, create_func, params) return self.task_lookup(taskid) def _create_volume_with_capacity(self, cb, params): diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index a1c8c9a..6037953 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -79,7 +79,8 @@ class StorageVolumesModel(object): raise InvalidParameter('KCHVOL0001E', {'name': name}) params['pool'] = pool_name - taskid = add_task('', create_func, self.objstore, params) + targeturi = '/storagepools/%s/storagevolumes/%s' % (pool_name, name) + taskid = add_task(targeturi, create_func, self.objstore, params) return self.task.lookup(taskid) def _create_volume_with_capacity(self, cb, params): -- 1.9.3
participants (2)
-
Aline Manera -
Crístian Viana