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

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
 

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

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