Include md5 in function package path
This is the preliminary patch for function versioning support. In order to create a new function version, we need to check if the function package data has been changed compared with the latest version. The package md5 is stored as part of the package file path. Change-Id: Icf7c3999033989762b31bb629571bab6ad06b05e Story: #2001829 Task: #12586
This commit is contained in:
parent
16b5d8adad
commit
546d03e7ac
|
@ -107,7 +107,8 @@ class FunctionsController(rest.RestController):
|
|||
source = func_db.code['source']
|
||||
|
||||
if source == constants.PACKAGE_FUNCTION:
|
||||
f = self.storage_provider.retrieve(ctx.projectid, id)
|
||||
f = self.storage_provider.retrieve(ctx.projectid, id,
|
||||
func_db.code['md5sum'])
|
||||
elif source == constants.SWIFT_FUNCTION:
|
||||
container = func_db.code['swift']['container']
|
||||
obj = func_db.code['swift']['object']
|
||||
|
@ -187,14 +188,19 @@ class FunctionsController(rest.RestController):
|
|||
'Trust creation failed for function.'
|
||||
)
|
||||
|
||||
# Create function and store the package data inside a db transaction so
|
||||
# that the function won't be created if any error happened during
|
||||
# package store.
|
||||
with db_api.transaction():
|
||||
func_db = db_api.create_function(values)
|
||||
|
||||
if store:
|
||||
try:
|
||||
ctx = context.get_ctx()
|
||||
self.storage_provider.store(ctx.projectid, func_db.id,
|
||||
data, md5sum=md5sum)
|
||||
actual_md5 = self.storage_provider.store(
|
||||
ctx.projectid, func_db.id, data, md5sum=md5sum
|
||||
)
|
||||
values['code'].update({"md5sum": actual_md5})
|
||||
func_db = db_api.update_function(func_db.id, values)
|
||||
except Exception as e:
|
||||
LOG.exception("Failed to store function package.")
|
||||
keystone_util.delete_trust(values['trust_id'])
|
||||
|
@ -254,7 +260,8 @@ class FunctionsController(rest.RestController):
|
|||
|
||||
source = func_db.code['source']
|
||||
if source == constants.PACKAGE_FUNCTION:
|
||||
self.storage_provider.delete(func_db.project_id, id)
|
||||
self.storage_provider.delete(func_db.project_id, id,
|
||||
func_db.code['md5sum'])
|
||||
|
||||
# Delete all resources created by orchestrator asynchronously.
|
||||
self.engine_client.delete_function(id)
|
||||
|
@ -282,6 +289,8 @@ class FunctionsController(rest.RestController):
|
|||
values = {}
|
||||
for key in UPDATE_ALLOWED:
|
||||
if kwargs.get(key) is not None:
|
||||
if key == "code":
|
||||
kwargs[key] = json.loads(kwargs[key])
|
||||
values.update({key: kwargs[key]})
|
||||
|
||||
LOG.info('Update function %s, params: %s', id, values)
|
||||
|
@ -291,6 +300,7 @@ class FunctionsController(rest.RestController):
|
|||
func_db = db_api.update_function(id, values)
|
||||
else:
|
||||
source = values.get('code', {}).get('source')
|
||||
md5sum = values.get('code', {}).get('md5sum')
|
||||
with db_api.transaction():
|
||||
pre_func = db_api.get_function(id)
|
||||
|
||||
|
@ -300,10 +310,15 @@ class FunctionsController(rest.RestController):
|
|||
)
|
||||
|
||||
pre_source = pre_func.code['source']
|
||||
pre_md5sum = pre_func.code['md5sum']
|
||||
if source and source != pre_source:
|
||||
raise exc.InputException(
|
||||
"The function code type can not be changed."
|
||||
)
|
||||
if md5sum and md5sum == pre_md5sum:
|
||||
raise exc.InputException(
|
||||
"The function code checksum is not changed."
|
||||
)
|
||||
if source == constants.IMAGE_FUNCTION:
|
||||
raise exc.InputException(
|
||||
"The image type function code can not be changed."
|
||||
|
@ -312,10 +327,14 @@ class FunctionsController(rest.RestController):
|
|||
values.get('package') is not None):
|
||||
# Update the package data.
|
||||
data = values['package'].file.read()
|
||||
self.storage_provider.store(
|
||||
md5sum = self.storage_provider.store(
|
||||
ctx.projectid,
|
||||
id,
|
||||
data
|
||||
data,
|
||||
md5sum=md5sum
|
||||
)
|
||||
values.setdefault('code', {}).update(
|
||||
{"md5sum": md5sum, "source": pre_source}
|
||||
)
|
||||
values.pop('package')
|
||||
if pre_source == constants.SWIFT_FUNCTION:
|
||||
|
|
|
@ -27,15 +27,31 @@ class PackageStorage(object):
|
|||
"""PackageStorage interface."""
|
||||
|
||||
@abc.abstractmethod
|
||||
def store(self, project_id, funtion, data, **kwargs):
|
||||
def store(self, project_id, function, data, **kwargs):
|
||||
"""Store the function package data.
|
||||
|
||||
:param project_id: Project ID.
|
||||
:param function: Function ID.
|
||||
:param data: Package file content.
|
||||
:param kwargs: A dict may including
|
||||
- md5sum: The MD5 provided by the user.
|
||||
:return: MD5 value of the package.
|
||||
"""
|
||||
raise NotImplementedError
|
||||
|
||||
@abc.abstractmethod
|
||||
def retrieve(self, project_id, function):
|
||||
def retrieve(self, project_id, function, md5sum):
|
||||
"""Get function package data.
|
||||
|
||||
:param project_id: Project ID.
|
||||
:param function: Function ID.
|
||||
:param md5sum: The function MD5.
|
||||
:return: File descriptor that needs to close outside.
|
||||
"""
|
||||
raise NotImplementedError
|
||||
|
||||
@abc.abstractmethod
|
||||
def delete(self, project_id, function):
|
||||
def delete(self, project_id, function, md5sum):
|
||||
raise NotImplementedError
|
||||
|
||||
|
||||
|
|
|
@ -23,6 +23,9 @@ from qinling.storage import base
|
|||
from qinling.utils import common
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
PACKAGE_NAME_TEMPLATE = "%s_%s.zip"
|
||||
# Package path name including project ID
|
||||
PACKAGE_PATH_TEMPLATE = "%s/%s_%s.zip"
|
||||
|
||||
|
||||
class FileSystemStorage(base.PackageStorage):
|
||||
|
@ -37,6 +40,8 @@ class FileSystemStorage(base.PackageStorage):
|
|||
:param project_id: Project ID.
|
||||
:param function: Function ID.
|
||||
:param data: Package file content.
|
||||
:param md5sum: The MD5 provided by the user.
|
||||
:return: MD5 value of the package.
|
||||
"""
|
||||
LOG.debug(
|
||||
'Store package, function: %s, project: %s', function, project_id
|
||||
|
@ -45,14 +50,16 @@ class FileSystemStorage(base.PackageStorage):
|
|||
project_path = os.path.join(self.base_path, project_id)
|
||||
fileutils.ensure_tree(project_path)
|
||||
|
||||
new_func_zip = os.path.join(project_path, '%s.zip.new' % function)
|
||||
func_zip = os.path.join(project_path, '%s.zip' % function)
|
||||
|
||||
# Check md5
|
||||
md5_actual = common.md5(content=data)
|
||||
if md5sum and md5_actual != md5sum:
|
||||
raise exc.InputException("Package md5 mismatch.")
|
||||
|
||||
# The md5 is contained in the package path.
|
||||
new_func_zip = os.path.join(project_path, '%s.zip.new' % function)
|
||||
func_zip = os.path.join(project_path,
|
||||
PACKAGE_NAME_TEMPLATE % (function, md5_actual))
|
||||
|
||||
# Store package
|
||||
with open(new_func_zip, 'wb') as fd:
|
||||
fd.write(data)
|
||||
|
@ -62,20 +69,24 @@ class FileSystemStorage(base.PackageStorage):
|
|||
raise exc.InputException("Package is not a valid ZIP package.")
|
||||
|
||||
os.rename(new_func_zip, func_zip)
|
||||
return md5_actual
|
||||
|
||||
def retrieve(self, project_id, function):
|
||||
def retrieve(self, project_id, function, md5sum):
|
||||
"""Get function package data.
|
||||
|
||||
:param project_id: Project ID.
|
||||
:param function: Function ID.
|
||||
:param md5sum: The function MD5.
|
||||
:return: File descriptor that needs to close outside.
|
||||
"""
|
||||
LOG.debug(
|
||||
'Get package data, function: %s, project: %s', function, project_id
|
||||
'Getting package data, function: %s, md5sum: %s, project: %s',
|
||||
function, md5sum, project_id
|
||||
)
|
||||
|
||||
func_zip = os.path.join(
|
||||
self.base_path, '%s/%s.zip' % (project_id, function)
|
||||
self.base_path,
|
||||
PACKAGE_PATH_TEMPLATE % (project_id, function, md5sum)
|
||||
)
|
||||
|
||||
if not os.path.exists(func_zip):
|
||||
|
@ -85,18 +96,19 @@ class FileSystemStorage(base.PackageStorage):
|
|||
)
|
||||
|
||||
f = open(func_zip, 'rb')
|
||||
LOG.debug('Found package data')
|
||||
LOG.debug('Found package data for function %s', function)
|
||||
|
||||
return f
|
||||
|
||||
def delete(self, project_id, function):
|
||||
def delete(self, project_id, function, md5sum):
|
||||
LOG.debug(
|
||||
'Delete package data, function: %s, project: %s', function,
|
||||
project_id
|
||||
'Deleting package data, function: %s, md5sum: %s, project: %s',
|
||||
function, md5sum, project_id
|
||||
)
|
||||
|
||||
func_zip = os.path.join(
|
||||
self.base_path, '%s/%s.zip' % (project_id, function)
|
||||
self.base_path,
|
||||
PACKAGE_PATH_TEMPLATE % (project_id, function, md5sum)
|
||||
)
|
||||
|
||||
if os.path.exists(func_zip):
|
||||
|
|
|
@ -37,6 +37,8 @@ class TestFunctionController(base.APITest):
|
|||
|
||||
@mock.patch('qinling.storage.file_system.FileSystemStorage.store')
|
||||
def test_post(self, mock_store):
|
||||
mock_store.return_value = 'fake_md5'
|
||||
|
||||
with tempfile.NamedTemporaryFile() as f:
|
||||
body = {
|
||||
'name': self.rand_name('function', prefix=TEST_CASE_NAME),
|
||||
|
@ -52,7 +54,12 @@ class TestFunctionController(base.APITest):
|
|||
self.assertEqual(201, resp.status_int)
|
||||
self.assertEqual(1, mock_store.call_count)
|
||||
|
||||
body.update({'entry': 'main.main', 'code': {"source": "package"}})
|
||||
body.update(
|
||||
{
|
||||
'entry': 'main.main',
|
||||
'code': {"source": "package", "md5sum": "fake_md5"}
|
||||
}
|
||||
)
|
||||
self._assertDictContainsSubset(resp.json, body)
|
||||
|
||||
@mock.patch('qinling.utils.openstack.keystone.get_swiftclient')
|
||||
|
@ -90,7 +97,7 @@ class TestFunctionController(base.APITest):
|
|||
)
|
||||
expected = {
|
||||
'id': db_func.id,
|
||||
"code": {"source": "package"},
|
||||
"code": {"source": "package", "md5sum": "fake_md5"},
|
||||
"name": db_func.name,
|
||||
'entry': 'main.main',
|
||||
"project_id": unit_base.DEFAULT_PROJECT_ID,
|
||||
|
@ -107,7 +114,6 @@ class TestFunctionController(base.APITest):
|
|||
)
|
||||
expected = {
|
||||
'id': db_func.id,
|
||||
"code": json.dumps({"source": "package"}),
|
||||
"name": db_func.name,
|
||||
'entry': 'main.main',
|
||||
"project_id": unit_base.DEFAULT_PROJECT_ID,
|
||||
|
@ -140,6 +146,7 @@ class TestFunctionController(base.APITest):
|
|||
db_func = self.create_function(
|
||||
runtime_id=self.runtime_id, prefix=TEST_CASE_NAME
|
||||
)
|
||||
mock_store.return_value = "fake_md5_changed"
|
||||
|
||||
with tempfile.NamedTemporaryFile() as f:
|
||||
resp = self.app.put(
|
||||
|
@ -150,9 +157,29 @@ class TestFunctionController(base.APITest):
|
|||
|
||||
self.assertEqual(200, resp.status_int)
|
||||
self.assertEqual(1, mock_store.call_count)
|
||||
self.assertEqual('fake_md5_changed', resp.json['code'].get('md5sum'))
|
||||
mock_delete_func.assert_called_once_with(db_func.id)
|
||||
mock_etcd_del.assert_called_once_with(db_func.id)
|
||||
|
||||
def test_put_package_same_md5(self):
|
||||
db_func = self.create_function(
|
||||
runtime_id=self.runtime_id, prefix=TEST_CASE_NAME
|
||||
)
|
||||
|
||||
with tempfile.NamedTemporaryFile() as f:
|
||||
resp = self.app.put(
|
||||
'/v1/functions/%s' % db_func.id,
|
||||
params={
|
||||
"code": json.dumps(
|
||||
{"md5sum": "fake_md5", "source": "package"}
|
||||
)
|
||||
},
|
||||
upload_files=[('package', f.name, f.read())],
|
||||
expect_errors=True
|
||||
)
|
||||
|
||||
self.assertEqual(400, resp.status_int)
|
||||
|
||||
@mock.patch('qinling.utils.etcd_util.delete_function')
|
||||
@mock.patch('qinling.rpc.EngineClient.delete_function')
|
||||
@mock.patch('qinling.storage.file_system.FileSystemStorage.delete')
|
||||
|
@ -164,7 +191,7 @@ class TestFunctionController(base.APITest):
|
|||
|
||||
self.assertEqual(204, resp.status_int)
|
||||
mock_delete.assert_called_once_with(
|
||||
unit_base.DEFAULT_PROJECT_ID, db_func.id
|
||||
unit_base.DEFAULT_PROJECT_ID, db_func.id, "fake_md5"
|
||||
)
|
||||
mock_delete_func.assert_called_once_with(db_func.id)
|
||||
mock_etcd_delete.assert_called_once_with(db_func.id)
|
||||
|
|
|
@ -186,7 +186,7 @@ class DbTestCase(BaseTest):
|
|||
{
|
||||
'name': self.rand_name('function', prefix=prefix),
|
||||
'runtime_id': runtime_id,
|
||||
'code': {"source": "package"},
|
||||
'code': {"source": "package", "md5sum": "fake_md5"},
|
||||
'entry': 'main.main',
|
||||
# 'auth_enable' is disabled by default, we create runtime for
|
||||
# default tenant.
|
||||
|
|
|
@ -21,6 +21,7 @@ from qinling import config
|
|||
from qinling import exceptions as exc
|
||||
from qinling.storage import file_system
|
||||
from qinling.tests.unit import base
|
||||
from qinling.utils import common
|
||||
|
||||
CONF = cfg.CONF
|
||||
FAKE_STORAGE_PATH = 'TMP_DIR'
|
||||
|
@ -46,20 +47,23 @@ class TestFileSystemStorage(base.BaseTest):
|
|||
function = self.rand_name('function', prefix='TestFileSystemStorage')
|
||||
# For python3, data should be encoded into bytes before hashing.
|
||||
function_data = "Some data".encode('utf8')
|
||||
md5 = common.md5(content=function_data)
|
||||
|
||||
self.storage.store(self.project_id, function, function_data)
|
||||
|
||||
temp_package_path = os.path.join(FAKE_STORAGE_PATH, self.project_id,
|
||||
'%s.zip.new' % function)
|
||||
package_path = os.path.join(
|
||||
FAKE_STORAGE_PATH,
|
||||
file_system.PACKAGE_PATH_TEMPLATE % (self.project_id, function,
|
||||
md5)
|
||||
)
|
||||
ensure_tree_mock.assert_called_once_with(
|
||||
os.path.join(FAKE_STORAGE_PATH, self.project_id))
|
||||
os.path.join(FAKE_STORAGE_PATH, self.project_id)
|
||||
)
|
||||
fake_fd.write.assert_called_once_with(function_data)
|
||||
is_zipfile_mock.assert_called_once_with(
|
||||
os.path.join(FAKE_STORAGE_PATH, self.project_id,
|
||||
'%s.zip.new' % function))
|
||||
rename_mock.assert_called_once_with(
|
||||
os.path.join(FAKE_STORAGE_PATH, self.project_id,
|
||||
'%s.zip.new' % function),
|
||||
os.path.join(FAKE_STORAGE_PATH, self.project_id,
|
||||
'%s.zip' % function))
|
||||
is_zipfile_mock.assert_called_once_with(temp_package_path)
|
||||
rename_mock.assert_called_once_with(temp_package_path, package_path)
|
||||
|
||||
@mock.patch('oslo_utils.fileutils.ensure_tree')
|
||||
def test_store_md5_mismatch(self, ensure_tree_mock):
|
||||
|
@ -113,15 +117,15 @@ class TestFileSystemStorage(base.BaseTest):
|
|||
open_mock.return_value = fake_fd
|
||||
function = self.rand_name('function', prefix='TestFileSystemStorage')
|
||||
|
||||
ret = self.storage.retrieve(self.project_id, function)
|
||||
ret = self.storage.retrieve(self.project_id, function, "fake_md5")
|
||||
|
||||
exists_mock.assert_called_once_with(
|
||||
os.path.join(FAKE_STORAGE_PATH, self.project_id,
|
||||
'%s.zip' % function))
|
||||
open_mock.assert_called_once_with(
|
||||
os.path.join(FAKE_STORAGE_PATH, self.project_id,
|
||||
'%s.zip' % function),
|
||||
'rb')
|
||||
package_path = os.path.join(
|
||||
FAKE_STORAGE_PATH,
|
||||
file_system.PACKAGE_PATH_TEMPLATE % (self.project_id, function,
|
||||
"fake_md5")
|
||||
)
|
||||
exists_mock.assert_called_once_with(package_path)
|
||||
open_mock.assert_called_once_with(package_path, 'rb')
|
||||
self.assertEqual(fake_fd, ret)
|
||||
|
||||
@mock.patch('os.path.exists')
|
||||
|
@ -134,11 +138,17 @@ class TestFileSystemStorage(base.BaseTest):
|
|||
"^Package of function %s for project %s not found\.$" % (
|
||||
function, self.project_id),
|
||||
self.storage.retrieve,
|
||||
self.project_id, function)
|
||||
self.project_id,
|
||||
function,
|
||||
"fake_md5"
|
||||
)
|
||||
|
||||
exists_mock.assert_called_once_with(
|
||||
os.path.join(FAKE_STORAGE_PATH, self.project_id,
|
||||
'%s.zip' % function))
|
||||
package_path = os.path.join(
|
||||
FAKE_STORAGE_PATH,
|
||||
file_system.PACKAGE_PATH_TEMPLATE % (self.project_id, function,
|
||||
"fake_md5")
|
||||
)
|
||||
exists_mock.assert_called_once_with(package_path)
|
||||
|
||||
@mock.patch('os.path.exists')
|
||||
@mock.patch('os.remove')
|
||||
|
@ -146,14 +156,15 @@ class TestFileSystemStorage(base.BaseTest):
|
|||
exists_mock.return_value = True
|
||||
function = self.rand_name('function', prefix='TestFileSystemStorage')
|
||||
|
||||
self.storage.delete(self.project_id, function)
|
||||
self.storage.delete(self.project_id, function, "fake_md5")
|
||||
|
||||
exists_mock.assert_called_once_with(
|
||||
os.path.join(FAKE_STORAGE_PATH, self.project_id,
|
||||
'%s.zip' % function))
|
||||
remove_mock.assert_called_once_with(
|
||||
os.path.join(FAKE_STORAGE_PATH, self.project_id,
|
||||
'%s.zip' % function))
|
||||
package_path = os.path.join(
|
||||
FAKE_STORAGE_PATH,
|
||||
file_system.PACKAGE_PATH_TEMPLATE % (self.project_id, function,
|
||||
"fake_md5")
|
||||
)
|
||||
exists_mock.assert_called_once_with(package_path)
|
||||
remove_mock.assert_called_once_with(package_path)
|
||||
|
||||
@mock.patch('os.path.exists')
|
||||
@mock.patch('os.remove')
|
||||
|
@ -161,9 +172,12 @@ class TestFileSystemStorage(base.BaseTest):
|
|||
exists_mock.return_value = False
|
||||
function = self.rand_name('function', prefix='TestFileSystemStorage')
|
||||
|
||||
self.storage.delete(self.project_id, function)
|
||||
self.storage.delete(self.project_id, function, "fake_md5")
|
||||
|
||||
exists_mock.assert_called_once_with(
|
||||
os.path.join(FAKE_STORAGE_PATH, self.project_id,
|
||||
'%s.zip' % function))
|
||||
package_path = os.path.join(
|
||||
FAKE_STORAGE_PATH,
|
||||
file_system.PACKAGE_PATH_TEMPLATE % (self.project_id, function,
|
||||
"fake_md5")
|
||||
)
|
||||
exists_mock.assert_called_once_with(package_path)
|
||||
remove_mock.assert_not_called()
|
||||
|
|
Loading…
Reference in New Issue