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 <strigazi@gmail.com>

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)

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: "
+                                         "<number>[<unit>],"
+                                         "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:

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.")

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:
+
+        <unsignedNumber><suffix>
+        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.
 

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
 

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')

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")

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):
 

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,