lxc_container: replace subprocess.Popen() with run_command() (#11204)

* lxc_container: replace subprocess.Popen() with run_command()

* Update plugins/modules/lxc_container.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* add changelog frag

* retain Popen logic in module_utils

* Update plugins/module_utils/_lxc.py

Co-authored-by: Felix Fontein <felix@fontein.de>

---------

Co-authored-by: Felix Fontein <felix@fontein.de>

.github/BOTMETA.yml

@@ -368,6 +368,8 @@ files:
   $module_utils/jenkins.py:
     labels: jenkins
     maintainers: russoz
+  $module_utils/_lxc.py:
+    maintainers: russoz
   $module_utils/manageiq.py:
     labels: manageiq
     maintainers: $team_manageiq
@@ -1504,7 +1506,7 @@ files:
     maintainers: vbotka
   $modules/sssd_info.py:
     maintainers: a-gabidullin
-#########################
+  #########################
   docs/docsite/rst/filter_guide.rst: {}
   docs/docsite/rst/filter_guide_abstract_informations.rst: {}
   docs/docsite/rst/filter_guide_abstract_informations_counting_elements_in_sequence.rst:
@@ -1573,7 +1575,7 @@ files:
     maintainers: russoz
   docs/docsite/rst/test_guide.rst:
     maintainers: felixfontein
-#########################
+  #########################
   tests/:
     labels: tests
   tests/integration:

changelogs/fragments/11204-lxc-container-popen.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - lxc_container - refactor function ``create_script``, using ``subprocess.Popen()``, to a new module_utils ``_lxc`` (https://github.com/ansible-collections/community.general/pull/11204).

plugins/module_utils/_lxc.py

@@ -0,0 +1,61 @@
+# Copyright (c) 2014, Kevin Carter <kevin.carter@rackspace.com>
+# Copyright (c) 2025, Alexei Znamensky <russoz@gmail.com>
+# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+from __future__ import annotations
+
+import os
+import subprocess
+import tempfile
+
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.common.text.converters import to_bytes
+
+
+# This is used to attach to a running container and execute commands from
+# within the container on the host.  This will provide local access to a
+# container without using SSH.  The template will attempt to work within the
+# home directory of the user that was attached to the container and source
+# that users environment variables by default.
+ATTACH_TEMPLATE = """#!/usr/bin/env bash
+pushd "$(getent passwd $(whoami)|cut -f6 -d':')"
+    if [[ -f ".bashrc" ]];then
+        source .bashrc
+        unset HOSTNAME
+    fi
+popd
+
+# User defined command
+{}
+"""
+
+
+def create_script(command: str, module: AnsibleModule) -> None:
+    """Write out a script onto a target.
+
+    This method should be backward compatible with Python when executing
+    from within the container.
+
+    :param command: command to run, this can be a script and can use spacing
+                    with newlines as separation.
+    :param module: AnsibleModule to run commands with.
+    """
+
+    script_file = ""
+    try:
+        f = tempfile.NamedTemporaryFile(prefix="lxc-attach-script", delete=False, mode="wb")
+        f.write(to_bytes(ATTACH_TEMPLATE.format(command), errors="surrogate_or_strict"))
+        script_file = f.name
+        f.flush()
+        f.close()
+
+        os.chmod(script_file, 0o0700)
+
+        with tempfile.NamedTemporaryFile(prefix="lxc-attach-script-log", delete=False, mode="ab") as stdout_file:
+            with tempfile.NamedTemporaryFile(prefix="lxc-attach-script-err", delete=False, mode="ab") as stderr_file:
+                subprocess.Popen([script_file], stdout=stdout_file, stderr=stderr_file).communicate()
+
+    finally:
+        if script_file:
+            os.remove(script_file)

plugins/modules/lxc_container.py

@@ -408,7 +408,6 @@ lxc_container:
 
 import os
 import re
-import subprocess
 import tempfile
 import time
 import shlex
@@ -424,6 +423,7 @@ from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.parsing.convert_bool import BOOLEANS_FALSE
 from ansible.module_utils.common.text.converters import to_text, to_bytes
 
+from ansible_collections.community.general.plugins.module_utils._lxc import create_script
 
 # LXC_COMPRESSION_MAP is a map of available compression types when creating
 # an archive of a container.
@@ -504,64 +504,6 @@ LXC_ANSIBLE_STATES = {
 }
 
 
-# This is used to attach to a running container and execute commands from
-# within the container on the host.  This will provide local access to a
-# container without using SSH.  The template will attempt to work within the
-# home directory of the user that was attached to the container and source
-# that users environment variables by default.
-ATTACH_TEMPLATE = """#!/usr/bin/env bash
-pushd "$(getent passwd $(whoami)|cut -f6 -d':')"
-    if [[ -f ".bashrc" ]];then
-        source .bashrc
-        unset HOSTNAME
-    fi
-popd
-
-# User defined command
-%(container_command)s
-"""
-
-
-def create_script(command):
-    """Write out a script onto a target.
-
-    This method should be backward compatible with Python when executing
-    from within the container.
-
-    :param command: command to run, this can be a script and can use spacing
-                    with newlines as separation.
-    :type command: ``str``
-    """
-
-    (fd, script_file) = tempfile.mkstemp(prefix="lxc-attach-script")
-    f = os.fdopen(fd, "wb")
-    try:
-        f.write(to_bytes(ATTACH_TEMPLATE % {"container_command": command}, errors="surrogate_or_strict"))
-        f.flush()
-    finally:
-        f.close()
-
-    # Ensure the script is executable.
-    os.chmod(script_file, int("0700", 8))
-
-    # Output log file.
-    stdout_file = os.fdopen(tempfile.mkstemp(prefix="lxc-attach-script-log")[0], "ab")
-
-    # Error log file.
-    stderr_file = os.fdopen(tempfile.mkstemp(prefix="lxc-attach-script-err")[0], "ab")
-
-    # Execute the script command.
-    try:
-        subprocess.Popen([script_file], stdout=stdout_file, stderr=stderr_file).communicate()
-    finally:
-        # Close the log files.
-        stderr_file.close()
-        stdout_file.close()
-
-        # Remove the script file upon completion of execution.
-        os.remove(script_file)
-
-
 class LxcContainerManagement:
     def __init__(self, module):
         """Management of LXC containers via Ansible.
@@ -865,7 +807,7 @@ class LxcContainerManagement:
             elif container_state == "stopped":
                 self._container_startup()
 
-            self.container.attach_wait(create_script, container_command)
+            self.container.attach_wait(create_script, (container_command, self.module))
             self.state_change = True
 
     def _container_startup(self, timeout=60):

tests/sanity/ignore-2.17.txt

@@ -2,7 +2,6 @@ plugins/modules/consul_session.py validate-modules:parameter-state-invalid-choic
 plugins/modules/homectl.py import-3.11  # Uses deprecated stdlib library 'crypt'
 plugins/modules/homectl.py import-3.12  # Uses deprecated stdlib library 'crypt'
 plugins/modules/iptables_state.py validate-modules:undocumented-parameter             # params _back and _timeout used by action plugin
-plugins/modules/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/osx_defaults.py validate-modules:parameter-state-invalid-choice
 plugins/modules/parted.py validate-modules:parameter-state-invalid-choice
 plugins/modules/rhevm.py validate-modules:parameter-state-invalid-choice

tests/sanity/ignore-2.18.txt

@@ -2,7 +2,6 @@ plugins/modules/consul_session.py validate-modules:parameter-state-invalid-choic
 plugins/modules/homectl.py import-3.11  # Uses deprecated stdlib library 'crypt'
 plugins/modules/homectl.py import-3.12  # Uses deprecated stdlib library 'crypt'
 plugins/modules/iptables_state.py validate-modules:undocumented-parameter             # params _back and _timeout used by action plugin
-plugins/modules/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/osx_defaults.py validate-modules:parameter-state-invalid-choice
 plugins/modules/parted.py validate-modules:parameter-state-invalid-choice
 plugins/modules/rhevm.py validate-modules:parameter-state-invalid-choice

tests/sanity/ignore-2.19.txt

@@ -2,7 +2,6 @@ plugins/modules/consul_session.py validate-modules:parameter-state-invalid-choic
 plugins/modules/homectl.py import-3.11  # Uses deprecated stdlib library 'crypt'
 plugins/modules/homectl.py import-3.12  # Uses deprecated stdlib library 'crypt'
 plugins/modules/iptables_state.py validate-modules:undocumented-parameter             # params _back and _timeout used by action plugin
-plugins/modules/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/osx_defaults.py validate-modules:parameter-state-invalid-choice
 plugins/modules/parted.py validate-modules:parameter-state-invalid-choice
 plugins/modules/rhevm.py validate-modules:parameter-state-invalid-choice

tests/sanity/ignore-2.20.txt

@@ -2,7 +2,6 @@ plugins/modules/consul_session.py validate-modules:parameter-state-invalid-choic
 plugins/modules/homectl.py import-3.11  # Uses deprecated stdlib library 'crypt'
 plugins/modules/homectl.py import-3.12  # Uses deprecated stdlib library 'crypt'
 plugins/modules/iptables_state.py validate-modules:undocumented-parameter             # params _back and _timeout used by action plugin
-plugins/modules/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/osx_defaults.py validate-modules:parameter-state-invalid-choice
 plugins/modules/parted.py validate-modules:parameter-state-invalid-choice
 plugins/modules/rhevm.py validate-modules:parameter-state-invalid-choice

tests/sanity/ignore-2.21.txt

@@ -7,7 +7,6 @@ plugins/modules/interfaces_file.py validate-modules:bad-return-value-key  # TODO
 plugins/modules/iptables_state.py validate-modules:undocumented-parameter             # params _back and _timeout used by action plugin
 plugins/modules/keycloak_realm_info.py validate-modules:bad-return-value-key  # TODO: rename offending return values if possible, or adjust this comment in case the name is OK
 plugins/modules/keycloak_realm_keys_metadata_info.py validate-modules:bad-return-value-key  # TODO: rename offending return values if possible, or adjust this comment in case the name is OK
-plugins/modules/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/nosh.py validate-modules:bad-return-value-key  # TODO: rename offending return values if possible, or adjust this comment in case the name is OK
 plugins/modules/omapi_host.py validate-modules:bad-return-value-key  # TODO: rename offending return values if possible, or adjust this comment in case the name is OK
 plugins/modules/osx_defaults.py validate-modules:parameter-state-invalid-choice