From 6f88426cf1dc7f3a84fb6750bef1bf56b772f5d1 Mon Sep 17 00:00:00 2001
From: Alexei Znamensky <103110+russoz@users.noreply.github.com>
Date: Sat, 15 Oct 2022 09:28:20 +1300
Subject: [PATCH] lxc_container: minor refactor (#5358)

* lxc_container: minor refactor

* add changelog fragment
---
 .../fragments/5358-lxc-container-refactor.yml |  2 +
 plugins/modules/cloud/lxc/lxc_container.py    | 92 +++++++++----------
 tests/sanity/ignore-2.11.txt                  |  1 -
 tests/sanity/ignore-2.12.txt                  |  1 -
 tests/sanity/ignore-2.13.txt                  |  1 -
 tests/sanity/ignore-2.14.txt                  |  3 +-
 tests/sanity/ignore-2.15.txt                  |  3 +-
 7 files changed, 46 insertions(+), 57 deletions(-)
 create mode 100644 changelogs/fragments/5358-lxc-container-refactor.yml

diff --git a/changelogs/fragments/5358-lxc-container-refactor.yml b/changelogs/fragments/5358-lxc-container-refactor.yml
new file mode 100644
index 0000000000..46beee907d
--- /dev/null
+++ b/changelogs/fragments/5358-lxc-container-refactor.yml
@@ -0,0 +1,2 @@
+minor_changes:
+  - lxc_container - minor refactoring (https://github.com/ansible-collections/community.general/pull/5358).
diff --git a/plugins/modules/cloud/lxc/lxc_container.py b/plugins/modules/cloud/lxc/lxc_container.py
index 9eeb0b65d5..7871f13972 100644
--- a/plugins/modules/cloud/lxc/lxc_container.py
+++ b/plugins/modules/cloud/lxc/lxc_container.py
@@ -85,7 +85,7 @@ options:
         type: str
     lxc_path:
         description:
-          - Place container under PATH.
+          - Place container under C(PATH).
         type: path
     container_log:
         description:
@@ -104,7 +104,7 @@ options:
           - debug
           - DEBUG
         description:
-          - Set the log level for a container where *container_log* was set.
+          - Set the log level for a container where I(container_log) was set.
         type: str
         required: false
         default: INFO
@@ -171,17 +171,17 @@ notes:
   - Containers must have a unique name. If you attempt to create a container
     with a name that already exists in the users namespace the module will
     simply return as "unchanged".
-  - The "container_command" can be used with any state except "absent". If
-    used with state "stopped" the container will be "started", the command
-    executed, and then the container "stopped" again. Likewise if the state
-    is "stopped" and the container does not exist it will be first created,
-    "started", the command executed, and then "stopped". If you use a "|"
+  - The I(container_command) can be used with any state except C(absent). If
+    used with state C(stopped) the container will be C(started), the command
+    executed, and then the container C(stopped) again. Likewise if I(state=stopped)
+    and the container does not exist it will be first created,
+    C(started), the command executed, and then C(stopped). If you use a "|"
     in the variable you can use common script formatting within the variable
-    itself The "container_command" option will always execute as BASH.
-    When using "container_command" a log file is created in the /tmp/ directory
-    which contains both stdout and stderr of any command executed.
-  - If "archive" is **true** the system will attempt to create a compressed
-    tarball of the running container. The "archive" option supports LVM backed
+    itself. The I(container_command) option will always execute as BASH.
+    When using I(container_command), a log file is created in the C(/tmp/) directory
+    which contains both C(stdout) and C(stderr) of any command executed.
+  - If I(archive=true) the system will attempt to create a compressed
+    tarball of the running container. The I(archive) option supports LVM backed
     containers and will create a snapshot of the running container when
     creating the archive.
   - If your distro does not have a package for C(python3-lxc), which is a
@@ -433,7 +433,7 @@ else:
     HAS_LXC = True
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.parsing.convert_bool import BOOLEANS_FALSE, BOOLEANS_TRUE
+from ansible.module_utils.parsing.convert_bool import boolean, BOOLEANS_FALSE
 from ansible.module_utils.common.text.converters import to_text, to_bytes
 
 
@@ -607,10 +607,10 @@ class LxcContainerManagement(object):
         :type module: ``object``
         """
         self.module = module
-        self.state = self.module.params.get('state', None)
+        self.state = self.module.params['state']
         self.state_change = False
         self.lxc_vg = None
-        self.lxc_path = self.module.params.get('lxc_path', None)
+        self.lxc_path = self.module.params['lxc_path']
         self.container_name = self.module.params['name']
         self.container = self.get_container_bind()
         self.archive_info = None
@@ -643,10 +643,7 @@ class LxcContainerManagement(object):
         :returns: True or False if the container is found.
         :rtype: ``bol``
         """
-        if [i for i in lxc.list_containers(config_path=lxc_path) if i == container_name]:
-            return True
-        else:
-            return False
+        return any(c == container_name for c in lxc.list_containers(config_path=lxc_path))
 
     @staticmethod
     def _add_variables(variables_dict, build_command):
@@ -678,13 +675,13 @@ class LxcContainerManagement(object):
         for v in LXC_BACKING_STORE[self.module.params['backing_store']]:
             variables.pop(v, None)
 
-        return_dict = dict()
         false_values = BOOLEANS_FALSE.union([None, ''])
-        for k, v in variables.items():
-            _var = self.module.params.get(k)
-            if _var not in false_values:
-                return_dict[v] = _var
-        return return_dict
+        result = dict(
+            (k, v)
+            for k, v in variables.items()
+            if self.module.params[k] not in false_values
+        )
+        return result
 
     def _config(self):
         """Configure an LXC container.
@@ -694,7 +691,7 @@ class LxcContainerManagement(object):
         restart the container upon completion.
         """
 
-        _container_config = self.module.params.get('container_config')
+        _container_config = self.module.params['container_config']
         if not _container_config:
             return False
 
@@ -784,12 +781,12 @@ class LxcContainerManagement(object):
         )
 
         # Load logging for the instance when creating it.
-        if self.module.params.get('clone_snapshot') in BOOLEANS_TRUE:
+        if self.module.params['clone_snapshot']:
             build_command.append('--snapshot')
         # Check for backing_store == overlayfs if so force the use of snapshot
         # If overlay fs is used and snapshot is unset the clone command will
         # fail with an unsupported type.
-        elif self.module.params.get('backing_store') == 'overlayfs':
+        elif self.module.params['backing_store'] == 'overlayfs':
             build_command.append('--snapshot')
 
         rc, return_data, err = self.module.run_command(build_command)
@@ -837,7 +834,7 @@ class LxcContainerManagement(object):
         )
 
         # Load logging for the instance when creating it.
-        if self.module.params.get('container_log') in BOOLEANS_TRUE:
+        if self.module.params['container_log']:
             # Set the logging path to the /var/log/lxc if uid is root. else
             # set it to the home folder of the user executing.
             try:
@@ -862,7 +859,7 @@ class LxcContainerManagement(object):
             ])
 
         # Add the template commands to the end of the command if there are any
-        template_options = self.module.params.get('template_options', None)
+        template_options = self.module.params['template_options']
         if template_options:
             build_command.append('--')
             build_command += shlex.split(template_options)
@@ -919,7 +916,7 @@ class LxcContainerManagement(object):
     def _execute_command(self):
         """Execute a shell command."""
 
-        container_command = self.module.params.get('container_command')
+        container_command = self.module.params['container_command']
         if container_command:
             container_state = self._get_state()
             if container_state == 'frozen':
@@ -939,17 +936,16 @@ class LxcContainerManagement(object):
 
         self.container = self.get_container_bind()
         for dummy in range(timeout):
-            if self._get_state() != 'running':
-                self.container.start()
-                self.state_change = True
-                # post startup sleep for 1 second.
-                time.sleep(1)
-            else:
+            if self._get_state() == 'running':
                 return True
+
+            self.container.start()
+            self.state_change = True
+            # post startup sleep for 1 second.
+            time.sleep(1)
         self.failure(
             lxc_container=self._container_data(),
-            error='Failed to start container'
-                  ' [ %s ]' % self.container_name,
+            error='Failed to start container [ %s ]' % self.container_name,
             rc=1,
             msg='The container [ %s ] failed to start. Check to lxc is'
                 ' available and that the container is in a functional'
@@ -962,7 +958,7 @@ class LxcContainerManagement(object):
         This will store archive_info in as self.archive_info
         """
 
-        if self.module.params.get('archive') in BOOLEANS_TRUE:
+        if self.module.params['archive']:
             self.archive_info = {
                 'archive': self._container_create_tar()
             }
@@ -973,7 +969,7 @@ class LxcContainerManagement(object):
         This will store archive_info in as self.archive_info
         """
 
-        clone_name = self.module.params.get('clone_name')
+        clone_name = self.module.params['clone_name']
         if clone_name:
             if not self._container_exists(container_name=clone_name, lxc_path=self.lxc_path):
                 self.clone_info = {
@@ -1339,11 +1335,11 @@ class LxcContainerManagement(object):
 
         old_umask = os.umask(int('0077', 8))
 
-        archive_path = self.module.params.get('archive_path')
+        archive_path = self.module.params['archive_path']
         if not os.path.isdir(archive_path):
             os.makedirs(archive_path)
 
-        archive_compression = self.module.params.get('archive_compression')
+        archive_compression = self.module.params['archive_compression']
         compression_type = LXC_COMPRESSION_MAP[archive_compression]
 
         # remove trailing / if present.
@@ -1357,9 +1353,7 @@ class LxcContainerManagement(object):
 
         build_command = [
             self.module.get_bin_path('tar', True),
-            '--directory=%s' % os.path.realpath(
-                os.path.expanduser(source_dir)
-            ),
+            '--directory=%s' % os.path.realpath(source_dir),
             compression_type['argument'],
             archive_name,
             '.'
@@ -1702,7 +1696,6 @@ def main():
             ),
             clone_name=dict(
                 type='str',
-                required=False
             ),
             clone_snapshot=dict(
                 type='bool',
@@ -1731,9 +1724,8 @@ def main():
             msg='The `lxc` module is not importable. Check the requirements.'
         )
 
-    lv_name = module.params.get('lv_name')
-    if not lv_name:
-        module.params['lv_name'] = module.params.get('name')
+    if not module.params['lv_name']:
+        module.params['lv_name'] = module.params['name']
 
     lxc_manage = LxcContainerManagement(module=module)
     lxc_manage.run()
diff --git a/tests/sanity/ignore-2.11.txt b/tests/sanity/ignore-2.11.txt
index 855883d64a..924e4f863b 100644
--- a/tests/sanity/ignore-2.11.txt
+++ b/tests/sanity/ignore-2.11.txt
@@ -4,7 +4,6 @@
 .azure-pipelines/scripts/publish-codecov.py compile-3.5!skip # Uses Python 3.6+ syntax
 .azure-pipelines/scripts/publish-codecov.py future-import-boilerplate
 .azure-pipelines/scripts/publish-codecov.py metaclass-boilerplate
-plugins/modules/cloud/lxc/lxc_container.py use-argspec-type-path
 plugins/modules/cloud/lxc/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/cloud/lxd/lxd_project.py use-argspec-type-path # expanduser() applied to constants
 plugins/modules/cloud/misc/rhevm.py validate-modules:parameter-state-invalid-choice
diff --git a/tests/sanity/ignore-2.12.txt b/tests/sanity/ignore-2.12.txt
index 39c97f6bf8..65cd74872a 100644
--- a/tests/sanity/ignore-2.12.txt
+++ b/tests/sanity/ignore-2.12.txt
@@ -1,5 +1,4 @@
 .azure-pipelines/scripts/publish-codecov.py replace-urlopen
-plugins/modules/cloud/lxc/lxc_container.py use-argspec-type-path
 plugins/modules/cloud/lxc/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/cloud/lxd/lxd_project.py use-argspec-type-path # expanduser() applied to constants
 plugins/modules/cloud/misc/rhevm.py validate-modules:parameter-state-invalid-choice
diff --git a/tests/sanity/ignore-2.13.txt b/tests/sanity/ignore-2.13.txt
index 39c97f6bf8..65cd74872a 100644
--- a/tests/sanity/ignore-2.13.txt
+++ b/tests/sanity/ignore-2.13.txt
@@ -1,5 +1,4 @@
 .azure-pipelines/scripts/publish-codecov.py replace-urlopen
-plugins/modules/cloud/lxc/lxc_container.py use-argspec-type-path
 plugins/modules/cloud/lxc/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/cloud/lxd/lxd_project.py use-argspec-type-path # expanduser() applied to constants
 plugins/modules/cloud/misc/rhevm.py validate-modules:parameter-state-invalid-choice
diff --git a/tests/sanity/ignore-2.14.txt b/tests/sanity/ignore-2.14.txt
index a2ccf20db3..4e5d3334b0 100644
--- a/tests/sanity/ignore-2.14.txt
+++ b/tests/sanity/ignore-2.14.txt
@@ -1,6 +1,4 @@
 .azure-pipelines/scripts/publish-codecov.py replace-urlopen
-plugins/modules/cloud/univention/udm_user.py import-3.11  # Uses deprecated stdlib library 'crypt'
-plugins/modules/cloud/lxc/lxc_container.py use-argspec-type-path
 plugins/modules/cloud/lxc/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/cloud/lxd/lxd_project.py use-argspec-type-path # expanduser() applied to constants
 plugins/modules/cloud/misc/rhevm.py validate-modules:parameter-state-invalid-choice
@@ -15,6 +13,7 @@ plugins/modules/cloud/spotinst/spotinst_aws_elastigroup.py validate-modules:para
 plugins/modules/cloud/spotinst/spotinst_aws_elastigroup.py validate-modules:undocumented-parameter
 plugins/modules/cloud/univention/udm_share.py validate-modules:parameter-list-no-elements
 plugins/modules/cloud/univention/udm_user.py validate-modules:parameter-list-no-elements
+plugins/modules/cloud/univention/udm_user.py import-3.11  # Uses deprecated stdlib library 'crypt'
 plugins/modules/clustering/consul/consul.py validate-modules:doc-missing-type
 plugins/modules/clustering/consul/consul.py validate-modules:undocumented-parameter
 plugins/modules/clustering/consul/consul_session.py validate-modules:parameter-state-invalid-choice
diff --git a/tests/sanity/ignore-2.15.txt b/tests/sanity/ignore-2.15.txt
index a2ccf20db3..4e5d3334b0 100644
--- a/tests/sanity/ignore-2.15.txt
+++ b/tests/sanity/ignore-2.15.txt
@@ -1,6 +1,4 @@
 .azure-pipelines/scripts/publish-codecov.py replace-urlopen
-plugins/modules/cloud/univention/udm_user.py import-3.11  # Uses deprecated stdlib library 'crypt'
-plugins/modules/cloud/lxc/lxc_container.py use-argspec-type-path
 plugins/modules/cloud/lxc/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/cloud/lxd/lxd_project.py use-argspec-type-path # expanduser() applied to constants
 plugins/modules/cloud/misc/rhevm.py validate-modules:parameter-state-invalid-choice
@@ -15,6 +13,7 @@ plugins/modules/cloud/spotinst/spotinst_aws_elastigroup.py validate-modules:para
 plugins/modules/cloud/spotinst/spotinst_aws_elastigroup.py validate-modules:undocumented-parameter
 plugins/modules/cloud/univention/udm_share.py validate-modules:parameter-list-no-elements
 plugins/modules/cloud/univention/udm_user.py validate-modules:parameter-list-no-elements
+plugins/modules/cloud/univention/udm_user.py import-3.11  # Uses deprecated stdlib library 'crypt'
 plugins/modules/clustering/consul/consul.py validate-modules:doc-missing-type
 plugins/modules/clustering/consul/consul.py validate-modules:undocumented-parameter
 plugins/modules/clustering/consul/consul_session.py validate-modules:parameter-state-invalid-choice