From 2b667eba052e785fef90d93b94c4ef9b13059787 Mon Sep 17 00:00:00 2001 From: Eli Qiao Date: Fri, 8 Apr 2016 14:52:51 +0800 Subject: [PATCH] Fix container-create memory not passed Commit I34e5e49ae650219f986a2b0032df65672c319ec6 tried to fix mem_limit not passed to docker when create container for docker API vesion >=1.19 in a wrong way, this patch fixes it by passing Memory unit to host_config Closes-Bug: #1567834 Change-Id: Id8da5e40cf165317a9a5453036490cc028bd2e0d Co-Authored-By: Spyros Trigazis --- magnum/api/controllers/v1/container.py | 2 ++ magnum/api/utils.py | 18 ++++++++++ magnum/common/exception.py | 4 +++ magnum/common/utils.py | 36 +++++++++++++++++++ magnum/conductor/handlers/docker_conductor.py | 6 ++-- .../unit/api/controllers/v1/test_container.py | 24 ++++++++++--- .../unit/api/controllers/v1/test_utils.py | 9 +++++ magnum/tests/unit/common/test_utils.py | 12 +++++++ .../handlers/test_docker_conductor.py | 4 +-- 9 files changed, 107 insertions(+), 8 deletions(-) diff --git a/magnum/api/controllers/v1/container.py b/magnum/api/controllers/v1/container.py index b96eb29cb9..5f1fa63c04 100644 --- a/magnum/api/controllers/v1/container.py +++ b/magnum/api/controllers/v1/container.py @@ -401,6 +401,8 @@ class ContainersController(rest.RestController): context = pecan.request.context container_dict['project_id'] = context.project_id container_dict['user_id'] = context.user_id + if 'memory' in container_dict: + api_utils.validate_docker_memory(container_dict['memory']) new_container = objects.Container(context, **container_dict) new_container.create() res_container = pecan.request.rpcapi.container_create(new_container) diff --git a/magnum/api/utils.py b/magnum/api/utils.py index 6bd7761460..4a77c5ca48 100644 --- a/magnum/api/utils.py +++ b/magnum/api/utils.py @@ -31,6 +31,9 @@ JSONPATCH_EXCEPTIONS = (jsonpatch.JsonPatchException, KeyError) +DOCKER_MINIMUM_MEMORY = 4 * 1024 * 1024 + + def validate_limit(limit): if limit is not None and limit <= 0: raise wsme.exc.ClientSideError(_("Limit must be positive")) @@ -49,6 +52,21 @@ def validate_sort_dir(sort_dir): return sort_dir +def validate_docker_memory(mem_str): + """Docker require that Minimum memory limit >= 4M.""" + try: + mem = utils.get_docker_quanity(mem_str) + except exception.UnsupportedDockerQuantityFormat: + raise wsme.exc.ClientSideError(_("Invalid docker memory specified. " + "Acceptable values are format: " + "[]," + "where unit = b, k, m or g")) + if mem < DOCKER_MINIMUM_MEMORY: + raise wsme.exc.ClientSideError(_("Docker Minimum memory limit" + "allowed is %d B.") + % DOCKER_MINIMUM_MEMORY) + + def apply_jsonpatch(doc, patch): for p in patch: if p['op'] == 'add' and p['path'].count('/') == 1: diff --git a/magnum/common/exception.py b/magnum/common/exception.py index d8273c783b..fe5f5ee50d 100644 --- a/magnum/common/exception.py +++ b/magnum/common/exception.py @@ -533,6 +533,10 @@ class UnsupportedK8sQuantityFormat(MagnumException): message = _("Unsupported quantity format for k8s bay.") +class UnsupportedDockerQuantityFormat(MagnumException): + message = _("Unsupported quantity format for Swarm bay.") + + class FlavorNotFound(ResourceNotFound): """The code here changed to 400 according to the latest document.""" message = _("Unable to find flavor %(flavor)s.") diff --git a/magnum/common/utils.py b/magnum/common/utils.py index 4bfc504c9a..d452c097d7 100644 --- a/magnum/common/utils.py +++ b/magnum/common/utils.py @@ -82,6 +82,13 @@ MEMORY_UNITS = { '': 1 } +DOCKER_MEMORY_UNITS = { + 'b': 1, + 'k': 2 ** 10, + 'm': 2 ** 20, + 'g': 2 ** 30, +} + def _get_root_helper(): return 'sudo magnum-rootwrap %s' % CONF.rootwrap_config @@ -593,6 +600,35 @@ def get_k8s_quantity(quantity): raise exception.UnsupportedK8sQuantityFormat() +def get_docker_quanity(quantity): + """This function is used to get swarm Memory quantity. + + Memory format must be in the format of: + + + suffix = b | k | m | g + + eg: 100m = 104857600 + :raises: exception.UnsupportedDockerQuantityFormat if the quantity string + is a unsupported value + """ + matched_unsigned_number = re.search(r"(^\d+)", quantity) + + if matched_unsigned_number is None: + raise exception.UnsupportedDockerQuantityFormat() + else: + unsigned_number = matched_unsigned_number.group(0) + + suffix = quantity.replace(unsigned_number, '', 1) + if suffix == '': + return int(quantity) + + if re.search(r"^(b|k|m|g)$", suffix): + return int(unsigned_number) * DOCKER_MEMORY_UNITS[suffix] + + raise exception.UnsupportedDockerQuantityFormat() + + def generate_password(length, symbolgroups=None): """Generate a random password from the supplied symbol groups. diff --git a/magnum/conductor/handlers/docker_conductor.py b/magnum/conductor/handlers/docker_conductor.py index dfe4eb7fbe..8daa353681 100644 --- a/magnum/conductor/handlers/docker_conductor.py +++ b/magnum/conductor/handlers/docker_conductor.py @@ -20,6 +20,7 @@ import six from magnum.common import docker_utils from magnum.common import exception +from magnum.common import utils as magnum_utils from magnum.i18n import _LE from magnum import objects from magnum.objects import fields @@ -85,8 +86,9 @@ class Handler(object): 'environment': container.environment} if docker_utils.is_docker_api_version_atleast(docker, '1.19'): if container.memory is not None: - kwargs['host_config'] = {'mem_limit': - container.memory} + kwargs['host_config'] = { + 'Memory': + magnum_utils.get_docker_quanity(container.memory)} else: kwargs['mem_limit'] = container.memory diff --git a/magnum/tests/unit/api/controllers/v1/test_container.py b/magnum/tests/unit/api/controllers/v1/test_container.py index 0fca1d565c..c923565467 100644 --- a/magnum/tests/unit/api/controllers/v1/test_container.py +++ b/magnum/tests/unit/api/controllers/v1/test_container.py @@ -230,14 +230,30 @@ class TestContainerController(api_base.FunctionalTest): self.assertTrue(mock_container_create.not_called) @patch('magnum.conductor.api.API.container_create') - def test_create_container_invalid_long_name(self, mock_container_create): + def _test_create_container_invalid_params(self, params, + mock_container_create): + self.assertRaises(AppError, self.app.post, '/v1/containers', + params=params, content_type='application/json') + self.assertTrue(mock_container_create.not_called) + + def test_create_container_invalid_long_name(self): # Long name params = ('{"name": "' + 'i' * 256 + '", "image": "ubuntu",' '"command": "env", "memory": "512m",' '"bay_uuid": "fff114da-3bfa-4a0f-a123-c0dffad9718e"}') - self.assertRaises(AppError, self.app.post, '/v1/containers', - params=params, content_type='application/json') - self.assertTrue(mock_container_create.not_called) + self._test_create_container_invalid_params(params) + + def test_create_container_no_memory_unit(self): + params = ('{"name": "ubuntu", "image": "ubuntu",' + '"command": "env", "memory": "512",' + '"bay_uuid": "fff114da-3bfa-4a0f-a123-c0dffad9718e"}') + self._test_create_container_invalid_params(params) + + def test_create_container_bad_memory_unit(self): + params = ('{"name": "ubuntu", "image": "ubuntu",' + '"command": "env", "memory": "512S",' + '"bay_uuid": "fff114da-3bfa-4a0f-a123-c0dffad9718e"}') + self._test_create_container_invalid_params(params) @patch('magnum.conductor.api.API.container_show') @patch('magnum.objects.Container.list') diff --git a/magnum/tests/unit/api/controllers/v1/test_utils.py b/magnum/tests/unit/api/controllers/v1/test_utils.py index fec86340ca..2af7f8f2de 100644 --- a/magnum/tests/unit/api/controllers/v1/test_utils.py +++ b/magnum/tests/unit/api/controllers/v1/test_utils.py @@ -150,3 +150,12 @@ class TestApiUtils(base.FunctionalTest): self.assertEqual( "The attribute /node_count has existed, please use " "'replace' operation instead.", exc.faultstring) + + def test_validate_docker_memory(self): + utils.validate_docker_memory('512m') + utils.validate_docker_memory('512g') + self.assertRaises(wsme.exc.ClientSideError, + utils.validate_docker_memory, "512gg") + # Docker require that Minimum memory limit >= 4M + self.assertRaises(wsme.exc.ClientSideError, + utils.validate_docker_memory, "3m") diff --git a/magnum/tests/unit/common/test_utils.py b/magnum/tests/unit/common/test_utils.py index c93db889e3..754ca662b3 100644 --- a/magnum/tests/unit/common/test_utils.py +++ b/magnum/tests/unit/common/test_utils.py @@ -133,6 +133,18 @@ class UtilsTestCase(base.TestCase): self.assertRaises(exception.UnsupportedK8sQuantityFormat, utils.get_k8s_quantity, '1E1E') + def test_get_docker_quanity(self): + self.assertEqual(512, utils.get_docker_quanity('512')) + self.assertEqual(512, utils.get_docker_quanity('512b')) + self.assertEqual(512 * 1024, utils.get_docker_quanity('512k')) + self.assertEqual(512 * 1024 * 1024, utils.get_docker_quanity('512m')) + self.assertEqual(512 * 1024 * 1024 * 1024, + utils.get_docker_quanity('512g')) + self.assertRaises(exception.UnsupportedDockerQuantityFormat, + utils.get_docker_quanity, '512bb') + self.assertRaises(exception.UnsupportedDockerQuantityFormat, + utils.get_docker_quanity, '512B') + class ExecuteTestCase(base.TestCase): diff --git a/magnum/tests/unit/conductor/handlers/test_docker_conductor.py b/magnum/tests/unit/conductor/handlers/test_docker_conductor.py index 9b2cefeed5..1d4e40dbd3 100644 --- a/magnum/tests/unit/conductor/handlers/test_docker_conductor.py +++ b/magnum/tests/unit/conductor/handlers/test_docker_conductor.py @@ -83,14 +83,14 @@ class TestDockerHandler(base.BaseTestCase): 'uuid': 'some-uuid', 'image': 'test_image:some_tag', 'command': None, - 'memory': 512, + 'memory': '100m', 'environment': None, } expected_kwargs = { 'name': 'some-name', 'hostname': 'some-uuid', 'command': None, - 'host_config': {'mem_limit': 512}, + 'host_config': {'Memory': 100 * 1024 * 1024}, 'environment': None, } self._test_container_create(container_dict, expected_kwargs,