Storage volume upload will use the same REST API as 'capacity' to create the storage volume for upload. To do that, the 'upload' parameter must be set accordingly. POST /storagepools/<pool-name>/storagevolumes/ {"capacity": 1000000, "format": "raw", "name": "volume-1", "upload": true} After storage volume is created, the data transfer will be done through several PUT requests: PUT /storagepools/<pool-name>/storagevolumes/volume-1 {"chunk_size": "1024", "chunk": form-data} PUT /storagepools/<pool-name>/storagevolumes/volume-1 {"chunk_size": "1024", "chunk": form-data} ... PUT /storagepools/<pool-name>/storagevolumes/volume-1 {"chunk_size": "1024", "chunk": form-data} Kimchi will keep the file offset internally to avoid data corruption. So all the PUT requests must be sequential. This patch also updates MockModel to manually does the upload process as the virStorageVol.upload() function is not supported by the libvirt Test driver. The test cases were also updated accordindly to those changes. Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 5 +++ src/kimchi/API.json | 22 ++++++++++ src/kimchi/i18n.py | 6 +++ src/kimchi/mockmodel.py | 18 +++++++- src/kimchi/model/storagevolumes.py | 53 ++++++++++++++++++++--- tests/test_model_storagevolume.py | 89 +++++++++++++++++++++++++++++++------- tests/test_rest.py | 45 +------------------ 7 files changed, 173 insertions(+), 65 deletions(-) diff --git a/docs/API.md b/docs/API.md index 62e4063..88c5fec 100644 --- a/docs/API.md +++ b/docs/API.md @@ -487,6 +487,8 @@ A interface represents available network interface on VM. The unit is bytes * format: The format of the defined Storage Volume. Only used when creating a storage volume with 'capacity'. + * upload: True to start an upload process. False, otherwise. + Only used when creating a storage volume 'capacity' parameter. * file: File to be uploaded, passed through form data * url: URL to be downloaded @@ -515,6 +517,9 @@ A interface represents available network interface on VM. * **DELETE**: Remove the Storage Volume * **POST**: *See Storage Volume Actions* +* **PUT**: Upload storage volume chunk + * chunk_size: Chunk size of the slice in Bytes. + * chunk: Actual data of uploaded file **Actions (POST):** diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 474661c..a6330ae 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -202,6 +202,11 @@ "minimum": 1, "error": "KCHVOL0020E" }, + "upload": { + "description": "When the storage volume will be uploaded", + "type": "boolean", + "error": "KCHVOL0025E" + }, "allocation": { "description": "The size(MiB) of allocation when create the storage volume", "type": "number", @@ -222,6 +227,23 @@ } } }, + "storagevolume_update": { + "type": "object", + "properties": { + "chunk": { + "description": "Upload storage volume chunk", + "error": "KCHVOL0024E", + "required": true + }, + "chunk_size": { + "description": "Chunk size of uploaded storage volume", + "type": "string", + "error": "KCHVOL0024E", + "required": true + } + }, + "additionalProperties": false + }, "vms_create": { "type": "object", "error": "KCHVM0016E", diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 18e84bc..9f169ab 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -214,6 +214,12 @@ messages = { "KCHVOL0021E": _("Storage volume URL must be http://, https://, ftp:// or ftps://."), "KCHVOL0022E": _("Unable to access file %(url)s. Please, check it."), "KCHVOL0023E": _("Unable to clone storage volume '%(name)s' in pool '%(pool)s'. Details: %(err)s"), + "KCHVOL0024E": _("Specify chunk data and its size to upload a file."), + "KCHVOL0025E": _("In order to upload a storage volume, specify the 'upload' parameter."), + "KCHVOL0026E": _("Unable to upload chunk data as it does not match with requested chunk size."), + "KCHVOL0027E": _("The storage volume %(vol)s is not under an upload process."), + "KCHVOL0028E": _("The upload chunk data will exceed the storage volume size."), + "KCHVOL0029E": _("Unable to upload chunk data to storage volume. Details: %(err)s."), "KCHIFACE0001E": _("Interface %(name)s does not exist"), diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index ae97354..4395883 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -38,7 +38,7 @@ from kimchi.model.libvirtstoragepool import IscsiPoolDef, NetfsPoolDef from kimchi.model.libvirtstoragepool import StoragePoolDef from kimchi.model.model import Model from kimchi.model.storagepools import StoragePoolModel -from kimchi.model.storagevolumes import StorageVolumesModel +from kimchi.model.storagevolumes import StorageVolumeModel, StorageVolumesModel from kimchi.model.templates import LibvirtVMTemplate from kimchi.model.users import PAMUsersModel from kimchi.model.groups import PAMGroupsModel @@ -112,6 +112,7 @@ class MockModel(Model): DeviceModel.lookup = self._mock_device_lookup StoragePoolModel._update_lvm_disks = self._update_lvm_disks StorageVolumesModel.get_list = self._mock_storagevolumes_get_list + StorageVolumeModel.doUpload = self._mock_storagevolume_doUpload DebugReportsModel._gen_debugreport_file = self._gen_debugreport_file LibvirtVMTemplate._get_volume_path = self._get_volume_path VMTemplate.get_iso_info = self._probe_image @@ -313,6 +314,21 @@ class MockModel(Model): return self._model_storagevolume_lookup(pool, vol) + def _mock_storagevolume_doUpload(self, vol, offset, data, data_size): + vol_path = vol.path() + + # MockModel does not create the storage volume as a file + # So create it to do the file upload + if offset == 0: + dirname = os.path.dirname(vol_path) + if not os.path.exists(dirname): + os.makedirs(dirname) + open(vol_path, 'w').close() + + with open(vol_path, 'a') as fd: + fd.seek(offset) + fd.write(data) + def _mock_partitions_get_list(self): return self._mock_partitions.partitions.keys() diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index cf4611d..0afc74b 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -21,6 +21,7 @@ import contextlib import lxml.etree as ET import os import tempfile +import threading import time import urllib2 from lxml.builder import E @@ -44,12 +45,11 @@ VOLUME_TYPE_MAP = {0: 'file', 2: 'directory', 3: 'network'} - READ_CHUNK_SIZE = 1048576 # 1 MiB - - REQUIRE_NAME_PARAMS = ['capacity'] +upload_volumes = dict() + class StorageVolumesModel(object): def __init__(self, **kargs): @@ -186,10 +186,13 @@ class StorageVolumesModel(object): objstore=self.objstore).lookup(pool_name, name) + vol_path = vol_info['path'] + if params.get('upload', False): + upload_volumes[vol_path] = {'lock': threading.Lock(), 'offset': 0} + try: with self.objstore as session: - session.store('storagevolume', vol_info['path'], - {'ref_cnt': 0}) + session.store('storagevolume', vol_path, {'ref_cnt': 0}) except Exception as e: # If the storage volume was created flawlessly, then lets hide this # error to avoid more error in the VM creation process @@ -491,6 +494,46 @@ class StorageVolumeModel(object): cb('OK', True) + def doUpload(self, vol, offset, data, data_size): + try: + st = self.conn.get().newStream(0) + vol.upload(st, offset, data_size) + st.send(data) + st.finish() + except Exception as e: + st and st.abort() + raise OperationFailed("KCHVOL0029E", {"err": e.message}) + + def update(self, pool, name, params): + chunk_data = params['chunk'].fullvalue() + chunk_size = int(params['chunk_size']) + + if len(chunk_data) != chunk_size: + raise OperationFailed("KCHVOL0026E") + + vol = StorageVolumeModel.get_storagevolume(pool, name, self.conn) + vol_path = vol.path() + vol_capacity = vol.info()[1] + + vol_data = upload_volumes.get(vol_path) + if vol_data is None: + raise OperationFailed("KCHVOL0027E", {"vol": vol_path}) + + lock = vol_data['lock'] + offset = vol_data['offset'] + if (offset + chunk_size) > vol_capacity: + raise OperationFailed("KCHVOL0028E") + + with lock: + self.doUpload(vol, offset, chunk_data, chunk_size) + + vol_data['offset'] += chunk_size + if vol_data['offset'] == vol_capacity: + del upload_volumes[vol_path] + + # Refresh to make sure volume can be found in following lookup + StoragePoolModel.get_storagepool(pool, self.conn).refresh(0) + class IsoVolumesModel(object): def __init__(self, **kargs): diff --git a/tests/test_model_storagevolume.py b/tests/test_model_storagevolume.py index a3c3ce3..005ac75 100644 --- a/tests/test_model_storagevolume.py +++ b/tests/test_model_storagevolume.py @@ -149,24 +149,83 @@ def _do_volume_test(self, model, host, ssl_port, pool_name): resp = self.request(vol_uri) self.assertEquals(404, resp.status) - # Create storage volume with 'file' - filepath = os.path.join(paths.get_prefix(), 'COPYING.LGPL') - url = 'https://%s:%s' % (host, ssl_port) + uri - with open(filepath, 'rb') as fd: - r = requests.post(url, files={'file': fd}, - verify=False, - headers=fake_auth_header()) - + # Storage volume upload + # It is done through a sequence of POST and several PUT requests + filename = 'COPYING.LGPL' + filepath = os.path.join(paths.get_prefix(), filename) + filesize = os.stat(filepath).st_size + + # Create storage volume for upload + req = json.dumps({'name': filename, 'format': 'raw', + 'capacity': filesize, 'upload': True}) + resp = self.request(uri, req, 'POST') if pool_info['type'] in READONLY_POOL_TYPE: - self.assertEquals(r.status_code, 400) + self.assertEquals(400, resp.status) else: - rollback.prependDefer(model.storagevolume_delete, pool_name, - 'COPYING.LGPL') - self.assertEquals(r.status_code, 202) - task = r.json() - wait_task(_task_lookup, task['id']) - resp = self.request(uri + '/COPYING.LGPL') + rollback.prependDefer(rollback_wrapper, model.storagevolume_delete, + pool_name, filename) + self.assertEquals(202, resp.status) + task_id = json.loads(resp.read())['id'] + wait_task(_task_lookup, task_id) + status = json.loads(self.request('/tasks/%s' % task_id).read()) + self.assertEquals('finished', status['status']) + + # Upload volume content + url = 'https://%s:%s' % (host, ssl_port) + uri + '/' + filename + + # Create a file with 5M to upload + # Max body size is set to 4M so the upload will fail with 413 + newfile = '/tmp/5m-file' + with open(newfile, 'wb') as fd: + fd.seek(5*1024*1024-1) + fd.write("\0") + rollback.prependDefer(os.remove, newfile) + + with open(newfile, 'rb') as fd: + with open(newfile + '.tmp', 'wb') as tmp_fd: + data = fd.read() + tmp_fd.write(data) + + with open(newfile + '.tmp', 'rb') as tmp_fd: + r = requests.put(url, data={'chunk_size': len(data)}, + files={'chunk': tmp_fd}, + verify=False, + headers=fake_auth_header()) + self.assertEquals(r.status_code, 413) + + # Do upload + index = 0 + chunk_size = 2 * 1024 + + with open(filepath, 'rb') as fd: + while True: + with open(filepath + '.tmp', 'wb') as tmp_fd: + fd.seek(index*chunk_size) + data = fd.read(chunk_size) + tmp_fd.write(data) + + with open(filepath + '.tmp', 'rb') as tmp_fd: + r = requests.put(url, data={'chunk_size': len(data)}, + files={'chunk': tmp_fd}, + verify=False, + headers=fake_auth_header()) + self.assertEquals(r.status_code, 200) + index = index + 1 + + if len(data) < chunk_size: + break + + rollback.prependDefer(os.remove, filepath + '.tmp') + resp = self.request(uri + '/' + filename) self.assertEquals(200, resp.status) + uploaded_path = json.loads(resp.read())['path'] + with open(filepath) as fd: + content = fd.read() + + with open(uploaded_path) as fd: + uploaded_content = fd.read() + + self.assertEquals(content, uploaded_content) # Create storage volume with 'url' url = 'https://github.com/kimchi-project/kimchi/raw/master/COPYING' diff --git a/tests/test_rest.py b/tests/test_rest.py index c2ebd88..914b602 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -21,7 +21,6 @@ import json import os import re -import requests import time import unittest import urllib2 @@ -35,7 +34,7 @@ import kimchi.server from kimchi.osinfo import get_template_default from kimchi.rollbackcontext import RollbackContext from kimchi.utils import add_task -from utils import fake_auth_header, get_free_port, patch_auth, request +from utils import get_free_port, patch_auth, request from utils import run_server, wait_task @@ -1188,48 +1187,6 @@ class RestTests(unittest.TestCase): resp = self.request('%s/fedora-fake' % base_uri, '{}', 'DELETE') self.assertEquals(204, resp.status) - def test_upload(self): - with RollbackContext() as rollback: - url = "https://%s:%s/storagepools/default-pool/storagevolumes" % \ - (host, ssl_port) - - # Create a file with 3M to upload - vol_path = '/tmp/3m-file' - with open(vol_path, 'wb') as fd: - fd.seek(3*1024*1024-1) - fd.write("\0") - rollback.prependDefer(os.remove, vol_path) - - with open(vol_path, 'rb') as fd: - r = requests.post(url, - files={'file': fd}, - verify=False, - headers=fake_auth_header()) - - self.assertEquals(r.status_code, 202) - task = r.json() - wait_task(self._task_lookup, task['id'], 15) - uri = '/storagepools/default-pool/storagevolumes/%s' - resp = self.request(uri % task['target_uri'].split('/')[-1]) - - self.assertEquals(200, resp.status) - - # Create a file with 5M to upload - # Max body size is set to 4M so the upload will fail with 413 - vol_path = '/tmp/5m-file' - with open(vol_path, 'wb') as fd: - fd.seek(5*1024*1024-1) - fd.write("\0") - rollback.prependDefer(os.remove, vol_path) - - with open(vol_path, 'rb') as fd: - r = requests.post(url, - files={'file': fd}, - verify=False, - headers=fake_auth_header()) - - self.assertEquals(r.status_code, 413) - class HttpsRestTests(RestTests): """ -- 2.1.0