From 21b0097db0f1b68db0ea1d6e4dae25f2d788cc3e Mon Sep 17 00:00:00 2001 From: Sergey <6213510+sshnaidm@users.noreply.github.com> Date: Sun, 19 Oct 2025 19:33:42 +0300 Subject: [PATCH] Fix issue with --rm and service in Quadlet (#982) Fix #913 Signed-off-by: Sagi Shnaidman --- plugins/module_utils/podman/quadlet.py | 41 +++++++- .../targets/podman_container/tasks/main.yml | 96 +++++++++++++++++++ 2 files changed, 132 insertions(+), 5 deletions(-) diff --git a/plugins/module_utils/podman/quadlet.py b/plugins/module_utils/podman/quadlet.py index 8d1080e..f109117 100644 --- a/plugins/module_utils/podman/quadlet.py +++ b/plugins/module_utils/podman/quadlet.py @@ -21,6 +21,7 @@ class Quadlet: def __init__(self, section: str, params: dict): self.section = section + self.service_section = {} self.custom_params = self.custom_prepare_params(params) self.dict_params = self.prepare_params() @@ -31,6 +32,20 @@ class Quadlet: # This should be implemented in child classes if needed. return params + def _map_restart_policy(self, policy: str) -> str: + """ + Map Podman restart policies to systemd Restart values. + """ + # Podman restart policies: no, on-failure, always, unless-stopped + # systemd Restart values: no, on-success, on-failure, on-abnormal, on-watchdog, on-abort, always + policy_map = { + "no": "no", + "on-failure": "on-failure", + "always": "always", + "unless-stopped": "always", # systemd doesn't have unless-stopped, use always + } + return policy_map.get(policy, policy) + def prepare_params(self) -> dict: """ Convert parameter values as per param_map. @@ -55,10 +70,22 @@ class Quadlet: Construct the quadlet content as a string. """ custom_user_options = self.custom_params.get("quadlet_options") + + # Build main section + content = f"[{self.section}]\n" + "\n".join(f"{key}={value}" for key, value in self.dict_params) + + # Add Service section if there are service parameters + # Check if user hasn't already added a [Service] section in quadlet_options + has_service_in_custom = False + if custom_user_options: + has_service_in_custom = any("[Service]" in opt for opt in custom_user_options) + + if self.service_section and not has_service_in_custom: + service_text = "\n\n[Service]\n" + "\n".join(f"{key}={value}" for key, value in self.service_section.items()) + content += service_text + custom_text = "\n" + "\n".join(custom_user_options) if custom_user_options else "" - return ( - f"[{self.section}]\n" + "\n".join(f"{key}={value}" for key, value in self.dict_params) + custom_text + "\n" - ) + return content + custom_text + "\n" def write_to_file(self, path: str): """ @@ -341,8 +368,10 @@ class ContainerQuadlet(Quadlet): params["podman_args"].append(f"--rdt-class {params['rdt_class']}") if params["requires"]: params["podman_args"].append(f"--requires {','.join(params['requires'])}") + # restart_policy is handled in the [Service] section, not as a podman argument + # to avoid conflicts with --rm option in Quadlet files if params["restart_policy"]: - params["podman_args"].append(f"--restart {params['restart_policy']}") + self.service_section["Restart"] = self._map_restart_policy(params["restart_policy"]) if params["retry"]: params["podman_args"].append(f"--retry {params['retry']}") if params["retry_delay"]: @@ -530,8 +559,10 @@ class PodQuadlet(Quadlet): params["podman_args"].append(f"--pid {params['pid']}") if params["pod_id_file"]: params["podman_args"].append(f"--pod-id-file {params['pod_id_file']}") + # restart_policy is handled in the [Service] section, not as a podman argument + # to avoid conflicts with --rm option in Quadlet files if params["restart_policy"]: - params["podman_args"].append(f"--restart={params['restart_policy']}") + self.service_section["Restart"] = self._map_restart_policy(params["restart_policy"]) if params["security_opt"]: for security_opt in params["security_opt"]: params["podman_args"].append(f"--security-opt {security_opt}") diff --git a/tests/integration/targets/podman_container/tasks/main.yml b/tests/integration/targets/podman_container/tasks/main.yml index 208e401..a74b8de 100644 --- a/tests/integration/targets/podman_container/tasks/main.yml +++ b/tests/integration/targets/podman_container/tasks/main.yml @@ -1523,6 +1523,102 @@ - quad3 is changed - "'127.0.0.45' in quad3.diff.after" + # Test for issue #913: restart_policy should go to [Service] section, not PodmanArgs + - name: Create a Quadlet for container with restart_policy + containers.podman.podman_container: + executable: "{{ test_executable | default('podman') }}" + name: container-restart-test + image: alpine + state: quadlet + quadlet_dir: /tmp + restart_policy: always + register: quad_restart + + - name: Check if Quadlet file contains [Service] section with Restart + shell: cat /tmp/container-restart-test.container + register: quad_restart_content + + - name: Verify restart_policy is in [Service] section and not in PodmanArgs + assert: + that: + - quad_restart is changed + - "'[Service]' in quad_restart_content.stdout" + - "'Restart=always' in quad_restart_content.stdout" + - "'--restart' not in quad_restart_content.stdout" + fail_msg: "restart_policy should be in [Service] section, not as --restart in PodmanArgs" + + - name: Test different restart policies + containers.podman.podman_container: + executable: "{{ test_executable | default('podman') }}" + name: "container-restart-{{ item.name }}" + image: alpine + state: quadlet + quadlet_dir: /tmp + restart_policy: "{{ item.policy }}" + loop: + - { name: "on-failure", policy: "on-failure" } + - { name: "unless-stopped", policy: "unless-stopped" } + register: quad_restart_policies + + - name: Verify all restart policies are correctly mapped + shell: cat /tmp/container-restart-{{ item.item.name }}.container + loop: "{{ quad_restart_policies.results }}" + register: quad_restart_files + + - name: Check restart policy mapping + assert: + that: + - "'[Service]' in item.stdout" + - "'Restart=' in item.stdout" + - "'--restart' not in item.stdout" + loop: "{{ quad_restart_files.results }}" + + # Test for issue #913: Verify no duplicate [Service] section when user provides custom one + - name: Create Quadlet with restart_policy and custom Service section in quadlet_options + containers.podman.podman_container: + executable: "{{ test_executable | default('podman') }}" + name: container-restart-custom-service + image: alpine + state: quadlet + quadlet_dir: /tmp + restart_policy: always + command: sleep infinity + quadlet_options: + - | + [Service] + TimeoutStopSec=70 + Restart=on-failure + register: quad_custom_service + + - name: Read Quadlet file with custom Service section + shell: cat /tmp/container-restart-custom-service.container + register: quad_custom_service_content + + - name: Count [Service] sections in Quadlet file + shell: grep -c "^\[Service\]" /tmp/container-restart-custom-service.container + register: service_section_count + failed_when: false + + - name: Verify only one [Service] section exists (user's custom one takes precedence) + assert: + that: + - quad_custom_service is changed + - service_section_count.stdout == "1" + - "'TimeoutStopSec=70' in quad_custom_service_content.stdout" + - "'Restart=on-failure' in quad_custom_service_content.stdout" + - "'--restart' not in quad_custom_service_content.stdout" + fail_msg: "Should have exactly one [Service] section when user provides custom one" + + - name: Cleanup restart test Quadlet files + file: + path: "{{ item }}" + state: absent + loop: + - /tmp/container-restart-test.container + - /tmp/container-restart-on-failure.container + - /tmp/container-restart-unless-stopped.container + - /tmp/container-restart-custom-service.container + always: - name: Remove container