diff options
3 files changed, 155 insertions, 5 deletions
diff --git a/ansible/module_utils/kolla_container_worker.py b/ansible/module_utils/kolla_container_worker.py index c3889bab4..4895820c6 100644 --- a/ansible/module_utils/kolla_container_worker.py +++ b/ansible/module_utils/kolla_container_worker.py @@ -12,11 +12,13 @@ from abc import ABC from abc import abstractmethod +import logging import shlex from ansible.module_utils.kolla_systemd_worker import SystemdWorker COMPARE_CONFIG_CMD = ['/usr/local/bin/kolla_set_configs', '--check'] +LOG = logging.getLogger(__name__) class ContainerWorker(ABC): @@ -205,6 +207,73 @@ class ContainerWorker(ABC): def compare_volumes(self, container_info): pass + def dimensions_differ(self, a, b, key): + """Compares two docker dimensions + + As there are two representations of dimensions in docker, we should + normalize them to compare if they are the same. + + If the dimension is no more supported due docker update, + an error is thrown to operator to fix the dimensions' config. + + The available representations can be found at: + + https://docs.docker.com/config/containers/resource_constraints/ + + + :param a: Integer or String that represents a number followed or not + by "b", "k", "m" or "g". + :param b: Integer or String that represents a number followed or not + by "b", "k", "m" or "g". + :return: True if 'a' has the same logical value as 'b' or else + False. + """ + + if a is None or b is None: + msg = ("The dimension [%s] is no more supported by Docker, " + "please remove it from yours configs or change " + "to the new one.") % key + LOG.error(msg) + self.module.fail_json( + failed=True, + msg=msg + ) + return + + unit_sizes = { + 'b': 1, + 'k': 1024 + } + unit_sizes['m'] = unit_sizes['k'] * 1024 + unit_sizes['g'] = unit_sizes['m'] * 1024 + a = str(a) + b = str(b) + a_last_char = a[-1].lower() + b_last_char = b[-1].lower() + error_msg = ("The docker dimension unit [%s] is not supported for " + "the dimension [%s]. The currently supported units " + "are ['b', 'k', 'm', 'g'].") + if not a_last_char.isnumeric(): + if a_last_char in unit_sizes: + a = str(int(a[:-1]) * unit_sizes[a_last_char]) + else: + LOG.error(error_msg, a_last_char, a) + self.module.fail_json( + failed=True, + msg=error_msg % (a_last_char, a) + ) + + if not b_last_char.isnumeric(): + if b_last_char in unit_sizes: + b = str(int(b[:-1]) * unit_sizes[b_last_char]) + else: + LOG.error(error_msg, b_last_char, b) + self.module.fail_json( + failed=True, + msg=error_msg % (b_last_char, b) + ) + return a != b + def compare_dimensions(self, container_info): new_dimensions = self.params.get('dimensions') @@ -223,12 +292,14 @@ class ContainerWorker(ABC): # check for a match. Otherwise, ensure it is set to the default. if key1 in new_dimensions: if key1 == 'ulimits': - if self.compare_ulimits(new_dimensions[key1], - current_dimensions[key2]): + if self.compare_ulimits(new_dimensions.get(key1), + current_dimensions.get(key2)): return True - elif new_dimensions[key1] != current_dimensions[key2]: + elif self.dimensions_differ(new_dimensions.get(key1), + current_dimensions.get(key2), + key1): return True - elif current_dimensions[key2]: + elif current_dimensions.get(key2): # The default values of all currently supported resources are # '' or 0 - both falsy. return True diff --git a/releasenotes/notes/fix-restarting-container-even-with-no-changes-when-using-dimensions-ad94b657b6c29cfc.yaml b/releasenotes/notes/fix-restarting-container-even-with-no-changes-when-using-dimensions-ad94b657b6c29cfc.yaml new file mode 100644 index 000000000..704de4373 --- /dev/null +++ b/releasenotes/notes/fix-restarting-container-even-with-no-changes-when-using-dimensions-ad94b657b6c29cfc.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fixes the dimensions comparison when we set + values like `1g` in the container dimensions + configuration, making the docker container + getting restarted even with no changes, as + we are comparing `1g` with `1073741824`, + which is displayed in the docker inspect + while `1g` is in the configuration. diff --git a/tests/kolla_container_tests/test_docker_worker.py b/tests/kolla_container_tests/test_docker_worker.py index dc3064c08..021bdf7e6 100644 --- a/tests/kolla_container_tests/test_docker_worker.py +++ b/tests/kolla_container_tests/test_docker_worker.py @@ -1382,7 +1382,76 @@ class TestAttrComp(base.BaseTestCase): 'CpusetMems': '', 'MemorySwap': 0, 'MemoryReservation': 0, 'Ulimits': []} self.dw = get_DockerWorker(self.fake_data['params']) - self.assertTrue(self.dw.compare_dimensions(container_info)) + resp = self.dw.compare_dimensions(container_info) + self.dw.module.fail_json.assert_not_called() + self.assertTrue(resp) + + def test_compare_dimensions_using_short_notation_not_changed(self): + self.fake_data['params']['dimensions'] = { + 'mem_limit': '1024', 'mem_reservation': '3M', + 'memswap_limit': '2g'} + container_info = dict() + container_info['HostConfig'] = { + 'CpuPeriod': 0, 'KernelMemory': 0, 'Memory': 1024, 'CpuQuota': 0, + 'CpusetCpus': '', 'CpuShares': 0, 'BlkioWeight': 0, + 'CpusetMems': '', 'MemorySwap': 2 * 1024 * 1024 * 1024, + 'MemoryReservation': 3 * 1024 * 1024, 'Ulimits': []} + self.dw = get_DockerWorker(self.fake_data['params']) + resp = self.dw.compare_dimensions(container_info) + self.dw.module.fail_json.assert_not_called() + self.assertFalse(resp) + + def test_compare_dimensions_key_no_more_supported(self): + self.fake_data['params']['dimensions'] = { + 'mem_limit': '1024', 'mem_reservation': '3M', + 'memswap_limit': '2g', 'kernel_memory': '4M'} + container_info = dict() + container_info['HostConfig'] = { + 'CpuPeriod': 0, 'Memory': 1024, 'CpuQuota': 0, + 'CpusetCpus': '', 'CpuShares': 0, 'BlkioWeight': 0, + 'CpusetMems': '', 'MemorySwap': 2 * 1024 * 1024 * 1024, + 'MemoryReservation': 3 * 1024 * 1024, 'Ulimits': []} + self.dw = get_DockerWorker(self.fake_data['params']) + self.dw.compare_dimensions(container_info) + expected_msg = ("The dimension [kernel_memory] is no more " + "supported by Docker, please remove it from " + "yours configs or change to the new one.") + self.dw.module.fail_json.assert_called_once_with( + failed=True, msg=expected_msg) + + def test_compare_dimensions_invalid_unit(self): + self.fake_data['params']['dimensions'] = { + 'mem_limit': '1024', 'mem_reservation': '3M', + 'memswap_limit': '2g', 'kernel_memory': '4E'} + container_info = dict() + container_info['HostConfig'] = { + 'CpuPeriod': 0, 'KernelMemory': 0, 'Memory': 1024, 'CpuQuota': 0, + 'CpusetCpus': '', 'CpuShares': 0, 'BlkioWeight': 0, + 'CpusetMems': '', 'MemorySwap': 2 * 1024 * 1024 * 1024, + 'MemoryReservation': 3 * 1024 * 1024, 'Ulimits': []} + self.dw = get_DockerWorker(self.fake_data['params']) + self.dw.compare_dimensions(container_info) + expected_msg = ("The docker dimension unit [e] is " + "not supported for the dimension [4E]." + " The currently supported units are " + "['b', 'k', 'm', 'g'].") + self.dw.module.fail_json.assert_called_once_with( + failed=True, msg=expected_msg) + + def test_compare_dimensions_using_short_notation_changed(self): + self.fake_data['params']['dimensions'] = { + 'mem_limit': '10m', 'mem_reservation': '3M', + 'memswap_limit': '1g'} + container_info = dict() + container_info['HostConfig'] = { + 'CpuPeriod': 0, 'KernelMemory': 0, 'Memory': 1024, 'CpuQuota': 0, + 'CpusetCpus': '', 'CpuShares': 0, 'BlkioWeight': 0, + 'CpusetMems': '', 'MemorySwap': 2 * 1024 * 1024 * 1024, + 'MemoryReservation': 3 * 1024 * 1024, 'Ulimits': []} + self.dw = get_DockerWorker(self.fake_data['params']) + resp = self.dw.compare_dimensions(container_info) + self.dw.module.fail_json.assert_not_called() + self.assertTrue(resp) def test_compare_dimensions_neg(self): self.fake_data['params']['dimensions'] = { |