[PATCH 0/3] Download remote image to storage pool
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..d1a19ff 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('downloading %s/%s' % (downloaded_size, remote_size)) except Exception, e: raise OperationFailed('KCHVOL0007E', {'name': name, 'pool': pool_name, -- 1.9.3
On 09/03/2014 05:47 PM, Crístian Viana wrote:
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..d1a19ff 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('downloading %s/%s' % (downloaded_size, remote_size))
Record only the values <downloaded_size>/<remote_size> so UI can use it as needed. And also translate the message to the selected language.
except Exception, e: raise OperationFailed('KCHVOL0007E', {'name': name, 'pool': pool_name,
On 03-09-2014 22:39, Aline Manera wrote:
Record only the values <downloaded_size>/<remote_size> so UI can use it as needed. And also translate the message to the selected language.
OK, I'll remove the text.
participants (2)
-
Aline Manera -
Crístian Viana