From 19c5d7e77e0ed7b3661f8f8663b399394c1e12ec Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Sun, 5 Jan 2020 13:34:02 -0500 Subject: [PATCH] tripleo-container-manage: fix config_data in Config/Labels config_data should not contain the container name, just the actual config data which is the value of the container_data dict. It removes the add of start_order into the compare, this isn't necessary and breaks idempotency for containers which don't have start_order in their config_data. Also adds more unit tests to cover all situations handled by the filter. Change-Id: I0b64bf34c8f7498128b3b1fceb7c727f8544cec6 --- .../ansible_plugins/filter/helpers.py | 28 +++++++---------- .../tasks/podman/create.yml | 2 +- .../tests/plugins/filter/test_helpers.py | 31 ++++++++++++++++--- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/tripleo_ansible/ansible_plugins/filter/helpers.py b/tripleo_ansible/ansible_plugins/filter/helpers.py index c6a241762..3a94db792 100644 --- a/tripleo_ansible/ansible_plugins/filter/helpers.py +++ b/tripleo_ansible/ansible_plugins/filter/helpers.py @@ -14,12 +14,20 @@ # License for the specific language governing permissions and limitations # under the License. +import ast import json +import six from collections import OrderedDict from operator import itemgetter +# cmp() doesn't exist on python3 +if six.PY3: + def cmp(a, b): + return 0 if a == b else 1 + + class FilterModule(object): def filters(self): return { @@ -126,24 +134,12 @@ class FilterModule(object): except KeyError: continue - # Build c_facts so it can be compared later with config_data; - # both will be json.dumps objects. - c_facts = json.dumps( - json.loads(c_facts[0]).get(c_name) - ) if len(c_facts) == 1 else {} + # Build c_facts so it can be compared later with config_data + c_facts = ast.literal_eval(c_facts[0]) if ( + len(c_facts)) == 1 else dict() - # 0 was picked since it's the null_value for the subsort filter. - # When a container config doesn't provide the start_order, it'll be - # 0 by default, therefore it needs to be added in the config_data - # when comparing with the actual container_infos results. - if 'start_order' not in config_data: - config_data['start_order'] = 0 - - # TODO(emilien) double check the comparing here and see if - # types are accurate (string vs dict, etc) - if c_facts != json.dumps(config_data): + if cmp(c_facts, config_data) != 0: to_delete += [c_name] - continue return to_delete diff --git a/tripleo_ansible/roles/tripleo-container-manage/tasks/podman/create.yml b/tripleo_ansible/roles/tripleo-container-manage/tasks/podman/create.yml index 71afbdfb2..ee4fd1d07 100644 --- a/tripleo_ansible/roles/tripleo-container-manage/tasks/podman/create.yml +++ b/tripleo_ansible/roles/tripleo-container-manage/tasks/podman/create.yml @@ -42,7 +42,7 @@ config_id: "{{ tripleo_container_manage_config_id }}" container_name: "{{ lookup('dict', container_data).key }}" managed_by: tripleo_ansible - config_data: "{{ container_data | to_json }}" + config_data: "{{ lookup('dict', container_data).value }}" log_driver: 'k8s-file' log_opt: "path={{ tripleo_container_manage_log_path }}/{{ lookup('dict', container_data).key }}.log" memory: "{{ lookup('dict', container_data).value.mem_limit | default(omit) }}" diff --git a/tripleo_ansible/tests/plugins/filter/test_helpers.py b/tripleo_ansible/tests/plugins/filter/test_helpers.py index bd7af46c1..b0edf97ec 100644 --- a/tripleo_ansible/tests/plugins/filter/test_helpers.py +++ b/tripleo_ansible/tests/plugins/filter/test_helpers.py @@ -287,7 +287,19 @@ class TestHelperFilters(tests_base.TestCase): 'config_id': 'tripleo_step1', 'container_name': 'swift', 'name': 'swift', - 'config_data': 'foo', + 'config_data': "{'foo': 'bar'}", + } + } + }, + { + 'Name': 'heat', + 'Config': { + 'Labels': { + 'managed_by': 'tripleo_ansible', + 'config_id': 'tripleo_step1', + 'container_name': 'heat', + 'name': 'heat', + 'config_data': "{'start_order': 0}", } } }, @@ -295,7 +307,8 @@ class TestHelperFilters(tests_base.TestCase): 'Name': 'haproxy', 'Config': { 'Labels': { - 'config_id': 'test' + 'managed_by': 'tripleo_ansible', + 'config_id': 'tripleo_step1', } } }, @@ -315,13 +328,23 @@ class TestHelperFilters(tests_base.TestCase): }, ] config = { + # we don't want that container to be touched: no restart 'mysql': '', + # container has no Config, therefore no Labels: restart needed 'rabbitmq': '', + # container has no config_data: restart needed 'haproxy': '', + # container isn't part of config_id: no restart 'tripleo': '', - 'doesnt_exist': '' + # container isn't in container_infos but not part of config_id: + # no restart. + 'doesnt_exist': '', + # config_data didn't change: no restart + 'swift': {'foo': 'bar'}, + # config_data changed: restart needed + 'heat': {'start_order': 1}, } - expected_list = ['rabbitmq'] + expected_list = ['rabbitmq', 'haproxy', 'heat'] result = self.filters.needs_delete(container_infos=data, config=config, config_id='tripleo_step1')