Add diff and change detection to systemd generation (#608)

Fix #339
Signed-off-by: Sagi Shnaidman <sshnaidm@redhat.com>

plugins/module_utils/podman/common.py

@@ -96,34 +96,93 @@ def run_generate_systemd_command(module, module_params, name, version):
     return rc, systemd, err
 
 
+def compare_systemd_file_content(file_path, file_content):
+    if not os.path.exists(file_path):
+        # File does not exist, so all lines in file_content are different
+        return '', file_content
+    # Read the file
+    with open(file_path, 'r') as unit_file:
+        current_unit_file_content = unit_file.read()
+
+    # Function to remove comments from file content
+    def remove_comments(content):
+        return "\n".join([line for line in content.splitlines() if not line.startswith('#')])
+
+    # Remove comments from both file contents before comparison
+    current_unit_file_content_nocmnt = remove_comments(current_unit_file_content)
+    unit_content_nocmnt = remove_comments(file_content)
+    if current_unit_file_content_nocmnt == unit_content_nocmnt:
+        return None
+
+    # Get the different lines between the two contents
+    diff_in_file = [line
+                    for line in unit_content_nocmnt.splitlines()
+                    if line not in current_unit_file_content_nocmnt.splitlines()]
+    diff_in_string = [line
+                      for line in current_unit_file_content_nocmnt.splitlines()
+                      if line not in unit_content_nocmnt.splitlines()]
+
+    return diff_in_string, diff_in_file
+
+
 def generate_systemd(module, module_params, name, version):
-    empty = {}
+    result = {
+        'changed': False,
+        'systemd': {},
+        'diff': {},
+    }
     sysconf = module_params['generate_systemd']
     rc, systemd, err = run_generate_systemd_command(module, module_params, name, version)
     if rc != 0:
         module.log(
             "PODMAN-CONTAINER-DEBUG: Error generating systemd: %s" % err)
-        return empty
+        return result
     else:
         try:
             data = json.loads(systemd)
+            result['systemd'] = data
             if sysconf.get('path'):
                 full_path = os.path.expanduser(sysconf['path'])
                 if not os.path.exists(full_path):
                     os.makedirs(full_path)
+                    result['changed'] = True
                 if not os.path.isdir(full_path):
                     module.fail_json("Path %s is not a directory! "
                                      "Can not save systemd unit files there!"
                                      % full_path)
                 for file_name, file_content in data.items():
                     file_name += ".service"
+                    if not os.path.exists(os.path.join(full_path, file_name)):
+                        result['changed'] = True
+                        if result['diff'].get('before') is None:
+                            result['diff'] = {'before': {}, 'after': {}}
+                        result['diff']['before'].update({f'systemd_{file_name}.service': ''})
+                        result['diff']['after'].update({f'systemd_{file_name}.service': file_content})
+
+                    else:
+                        diff_ = compare_systemd_file_content(os.path.join(full_path, file_name), file_content)
+                        if diff_:
+                            result['changed'] = True
+                            if result['diff'].get('before') is None:
+                                result['diff'] = {'before': {}, 'after': {}}
+                            result['diff']['before'].update({f'systemd_{file_name}.service': "\n".join(diff_[0])})
+                            result['diff']['after'].update({f'systemd_{file_name}.service': "\n".join(diff_[1])})
                     with open(os.path.join(full_path, file_name), 'w') as f:
                         f.write(file_content)
-            return data
+                diff_before = "\n".join(
+                    [f"{j} - {k}" for j, k in result['diff'].get('before', {}).items() if 'PIDFile' not in k]).strip()
+                diff_after = "\n".join(
+                    [f"{j} - {k}" for j, k in result['diff'].get('after', {}).items() if 'PIDFile' not in k]).strip()
+                if diff_before or diff_after:
+                    result['diff']['before'] = diff_before + "\n"
+                    result['diff']['after'] = diff_after + "\n"
+                else:
+                    result['diff'] = {}
+            return result
         except Exception as e:
             module.log(
                 "PODMAN-CONTAINER-DEBUG: Error writing systemd: %s" % e)
-            return empty
+            return result
 
 
 def delete_systemd(module, module_params, name, version):

plugins/module_utils/podman/podman_container_lib.py

@@ -1567,11 +1567,19 @@ class PodmanManager:
             self.results.update({'diff': self.container.diff})
         if self.module.params['debug'] or self.module_params['debug']:
             self.results.update({'podman_version': self.container.version})
+        sysd = generate_systemd(self.module,
+                                self.module_params,
+                                self.name,
+                                self.container.version)
+        self.results['changed'] = changed or sysd['changed']
         self.results.update(
-            {'podman_systemd': generate_systemd(self.module,
-                                                self.module_params,
-                                                self.name,
-                                                self.container.version)})
+            {'podman_systemd': sysd['systemd']})
+        if sysd['diff']:
+            if 'diff' not in self.results:
+                self.results.update({'diff': sysd['diff']})
+            else:
+                self.results['diff']['before'] += sysd['diff']['before']
+                self.results['diff']['after'] += sysd['diff']['after']
 
     def make_started(self):
         """Run actions if desired state is 'started'."""

plugins/modules/podman_generate_systemd.py

@@ -219,7 +219,7 @@ podman_command:
 import os
 from ansible.module_utils.basic import AnsibleModule
 import json
-
+from ansible_collections.containers.podman.plugins.module_utils.podman.common import compare_systemd_file_content
 
 RESTART_POLICY_CHOICES = [
     'no-restart',
@@ -447,25 +447,7 @@ def generate_systemd(module):
                 )
 
                 # See if we need to write the unit file, default yes
-                need_to_write_file = True
-                # If the unit file already exist, compare it with the
-                # generated content
-                if os.path.exists(unit_file_full_path):
-                    # Read the file
-                    with open(unit_file_full_path, 'r') as unit_file:
-                        current_unit_file_content = unit_file.read()
-                    # If current unit file content is the same as the
-                    # generated content
-                    # Remove comments from files, before comparing
-                    current_unit_file_content_nocmnt = "\n".join([
-                        line for line in current_unit_file_content.splitlines()
-                        if not line.startswith('#')])
-                    unit_content_nocmnt = "\n".join([
-                        line for line in unit_content.splitlines()
-                        if not line.startswith('#')])
-                    if current_unit_file_content_nocmnt == unit_content_nocmnt:
-                        # We don't need to write it
-                        need_to_write_file = False
+                need_to_write_file = bool(compare_systemd_file_content(unit_file_full_path, unit_content))
 
                 # Write the file, if needed
                 if need_to_write_file:

tests/integration/targets/podman_container_idempotency/tasks/idem_systemd.yml

@@ -0,0 +1,123 @@
+# Systemd generation
+- containers.podman.podman_container:
+    executable: "{{ test_executable | default('podman') }}"
+    name: idempotency
+    state: absent
+
+- containers.podman.podman_container:
+    executable: "{{ test_executable | default('podman') }}"
+    image: "{{ idem_image }}"
+    name: idempotency
+    state: started
+    command: 1h
+    generate_systemd:
+      path: /tmp/
+      restart_policy: always
+      time: 120
+      no_header: true
+      names: true
+      pod_prefix: whocares
+      separator: zzzz
+      container_prefix: contain
+  register: system0
+
+- name: Check if the result is changed
+  assert:
+    that:
+      - system0 is changed
+
+- containers.podman.podman_container:
+    executable: "{{ test_executable | default('podman') }}"
+    image: "{{ idem_image }}"
+    name: idempotency
+    state: started
+    command: 1h
+    generate_systemd:
+      path: /tmp/
+      restart_policy: always
+      time: 120
+      no_header: true
+      names: true
+      pod_prefix: whocares
+      separator: zzzz
+      container_prefix: contain
+  register: system1
+
+- name: Check if the result is not changed
+  assert:
+    that:
+      - system1 is not changed
+
+- name: Remove the systemd unit file
+  ansible.builtin.file:
+    path: /tmp/containzzzzidempotency.service
+    state: absent
+
+- containers.podman.podman_container:
+    executable: "{{ test_executable | default('podman') }}"
+    image: "{{ idem_image }}"
+    name: idempotency
+    state: started
+    command: 1h
+    generate_systemd:
+      path: /tmp/
+      restart_policy: always
+      time: 120
+      no_header: true
+      names: true
+      pod_prefix: whocares
+      separator: zzzz
+      container_prefix: contain
+  register: system2
+
+- name: Check if the result is changed
+  assert:
+    that:
+      - system2 is changed
+
+- containers.podman.podman_container:
+    executable: "{{ test_executable | default('podman') }}"
+    image: "{{ idem_image }}"
+    name: idempotency
+    state: started
+    command: 1h
+    generate_systemd:
+      path: /tmp/
+      restart_policy: always
+      time: 120
+      no_header: true
+      names: true
+      pod_prefix: whocares
+      separator: zzzz
+      container_prefix: contain
+  register: system3
+
+- name: Check if the result is not changed
+  assert:
+    that:
+      - system3 is not changed
+
+- name: Add string to change the systemd unit file
+  ansible.builtin.shell: echo 'test=onetwo' >> /tmp/containzzzzidempotency.service
+
+- containers.podman.podman_container:
+    executable: "{{ test_executable | default('podman') }}"
+    image: "{{ idem_image }}"
+    name: idempotency
+    state: started
+    command: 1h
+    generate_systemd:
+      path: /tmp/
+      restart_policy: always
+      time: 120
+      no_header: true
+      names: true
+      pod_prefix: whocares
+      separator: zzzz
+      container_prefix: contain
+  register: system4
+
+- name: Check if the result is changed
+  assert:
+    that:
+      - system4 is changed

tests/integration/targets/podman_container_idempotency/tasks/main.yml

@@ -23,6 +23,9 @@
 - name: Test idempotency of containers in pods
   include_tasks: idem_pods.yml
 
+- name: Test idempotency of systemd generation
+  include_tasks: idem_systemd.yml
+
 - name: Test idempotency of other settings
   include_tasks: idem_all.yml