[Kimchi-devel] [PATCH 3/4] Upload storage volume

Aline Manera alinefm at linux.vnet.ibm.com
Mon May 11 18:34:31 UTC 2015


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 at linux.vnet.ibm.com>
Signed-off-by: Royce Lv <lvroyce at 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




More information about the Kimchi-devel mailing list