refactor: rework ci and testing

.drone.jsonnet

@@ -5,9 +5,10 @@ local PythonVersion(pyversion='3.8') = {
     PY_COLORS: 1,
   },
   commands: [
-    'pip install -r dev-requirements.txt -qq',
+    'pip install poetry -qq',
-    'pip install -r test/unit/requirements.txt -qq',
+    'poetry config experimental.new-installer false',
-    'python -m pytest --cov --cov-append --no-cov-on-fail',
+    'poetry install',
+    'poetry run pytest',
   ],
   depends_on: [
     'clone',
@@ -23,14 +24,31 @@ local PipelineLint = {
   },
   steps: [
     {
-      name: 'flake8',
+      name: 'check-format',
-      image: 'python:3.10',
+      image: 'python:3.11',
       environment: {
         PY_COLORS: 1,
       },
       commands: [
-        'pip install -r dev-requirements.txt -qq',
+        'git fetch -tq',
-        'flake8',
+        'pip install poetry -qq',
+        'poetry config experimental.new-installer false',
+        'poetry install',
+        'poetry run yapf -dr ./plugins',
+      ],
+    },
+    {
+      name: 'check-coding',
+      image: 'python:3.11',
+      environment: {
+        PY_COLORS: 1,
+      },
+      commands: [
+        'git fetch -tq',
+        'pip install poetry -qq',
+        'poetry config experimental.new-installer false',
+        'poetry install',
+        'poetry run ruff ./plugins',
       ],
     },
   ],
@@ -50,6 +68,7 @@ local PipelineTest = {
     PythonVersion(pyversion='3.8'),
     PythonVersion(pyversion='3.9'),
     PythonVersion(pyversion='3.10'),
+    PythonVersion(pyversion='3.11'),
   ],
   depends_on: [
     'lint',
@@ -69,11 +88,13 @@ local PipelineBuild = {
   steps: [
     {
       name: 'build',
-      image: 'python:3.9',
+      image: 'python:3.11',
       commands: [
         'GALAXY_VERSION=${DRONE_TAG##v}',
         "sed -i 's/version: 0.0.0/version: '\"$${GALAXY_VERSION:-0.0.0}\"'/g' galaxy.yml",
-        'pip install ansible -qq',
+        'pip install poetry -qq',
+        'poetry config experimental.new-installer false',
+        'poetry install',
         'ansible-galaxy collection build --output-path dist/',
       ],
     },

.drone.yml

@@ -7,11 +7,25 @@ platform:
   arch: amd64
 
 steps:
-  - name: flake8
+  - name: check-format
-    image: python:3.10
+    image: python:3.11
     commands:
-      - pip install -r dev-requirements.txt -qq
+      - git fetch -tq
-      - flake8
+      - pip install poetry -qq
+      - poetry config experimental.new-installer false
+      - poetry install
+      - poetry run yapf -dr ./plugins
+    environment:
+      PY_COLORS: 1
+
+  - name: check-coding
+    image: python:3.11
+    commands:
+      - git fetch -tq
+      - pip install poetry -qq
+      - poetry config experimental.new-installer false
+      - poetry install
+      - poetry run ruff ./plugins
     environment:
       PY_COLORS: 1
 
@@ -33,9 +47,10 @@ steps:
   - name: python38-pytest
     image: python:3.8
     commands:
-      - pip install -r dev-requirements.txt -qq
+      - pip install poetry -qq
-      - pip install -r test/unit/requirements.txt -qq
+      - poetry config experimental.new-installer false
-      - python -m pytest --cov --cov-append --no-cov-on-fail
+      - poetry install
+      - poetry run pytest
     environment:
       PY_COLORS: 1
     depends_on:
@@ -44,9 +59,10 @@ steps:
   - name: python39-pytest
     image: python:3.9
     commands:
-      - pip install -r dev-requirements.txt -qq
+      - pip install poetry -qq
-      - pip install -r test/unit/requirements.txt -qq
+      - poetry config experimental.new-installer false
-      - python -m pytest --cov --cov-append --no-cov-on-fail
+      - poetry install
+      - poetry run pytest
     environment:
       PY_COLORS: 1
     depends_on:
@@ -55,9 +71,22 @@ steps:
   - name: python310-pytest
     image: python:3.10
     commands:
-      - pip install -r dev-requirements.txt -qq
+      - pip install poetry -qq
-      - pip install -r test/unit/requirements.txt -qq
+      - poetry config experimental.new-installer false
-      - python -m pytest --cov --cov-append --no-cov-on-fail
+      - poetry install
+      - poetry run pytest
+    environment:
+      PY_COLORS: 1
+    depends_on:
+      - clone
+
+  - name: python311-pytest
+    image: python:3.11
+    commands:
+      - pip install poetry -qq
+      - poetry config experimental.new-installer false
+      - poetry install
+      - poetry run pytest
     environment:
       PY_COLORS: 1
     depends_on:
@@ -82,11 +111,13 @@ platform:
 
 steps:
   - name: build
-    image: python:3.9
+    image: python:3.11
     commands:
       - GALAXY_VERSION=${DRONE_TAG##v}
       - "sed -i 's/version: 0.0.0/version: '\"$${GALAXY_VERSION:-0.0.0}\"'/g' galaxy.yml"
-      - pip install ansible -qq
+      - pip install poetry -qq
+      - poetry config experimental.new-installer false
+      - poetry install
       - ansible-galaxy collection build --output-path dist/
 
   - name: checksum
@@ -195,6 +226,6 @@ depends_on:
 
 ---
 kind: signature
-hmac: 93f735c3d2fbaf499fd96b79301f6de2455349051dc320c511e6e62c8ba04a4d
+hmac: e13ec0f71bb6c29530344335056ad14820d4028a28e6e74b40de0db2afbfe568
 
 ...

.gitignore

@@ -108,3 +108,4 @@ docs/public/
 resources/_gen/
 
 CHANGELOG.md
+tests/output

dev-requirements.txt

@@ -1,18 +0,0 @@
-pydocstyle
-flake8
-flake8-blind-except
-flake8-builtins
-flake8-docstrings
-flake8-isort
-flake8-logging-format
-flake8-polyfill
-flake8-quotes
-flake8-pep3101
-flake8-eradicate
-pep8-naming
-wheel
-pytest
-pytest-mock
-pytest-cov
-bandit
-yapf

plugins/filter/prefix.py

@@ -1,11 +1,15 @@
 """Filter to prefix all itams from a list."""
 
+from __future__ import (absolute_import, division, print_function)
+
+__metaclass__ = type
+
 
 def prefix(value, prefix="--"):
     return [prefix + x for x in value]
 
 
-class FilterModule(object):
+class FilterModule(object):  # noqa
 
     def filters(self):
         return {"prefix": prefix}

plugins/filter/wrap.py

@@ -1,11 +1,15 @@
 """Filter to wrap all items from a list."""
 
+from __future__ import (absolute_import, division, print_function)
+
+__metaclass__ = type
+
 
 def wrap(value, wrapper="'"):
     return [wrapper + x + wrapper for x in value]
 
 
-class FilterModule(object):
+class FilterModule(object):  # noqa
 
     def filters(self):
         return {"wrap": wrap}

plugins/inventory/proxmox.py

@@ -1,11 +1,10 @@
-# -*- coding: utf-8 -*-
 # Copyright (c) 2014, Mathieu GAUTHIER-LAFAYE <gauthierl@lapth.cnrs.fr>
 # Copyright (c) 2016, Matt Harris <matthaeus.harris@gmail.com>
 # Copyright (c) 2020, Robert Kaussow <mail@thegeeklab.de>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 """Dynamic inventory plugin for Proxmox VE."""
 
-from __future__ import absolute_import, division, print_function
+from __future__ import (absolute_import, division, print_function)
 
 __metaclass__ = type
 
@@ -13,7 +12,7 @@ DOCUMENTATION = """
     name: proxmox
     plugin_type: inventory
     short_description: Proxmox VE inventory source
-    version_added: 1.0.0
+    version_added: 1.1.0
     description:
         - Get inventory hosts from the proxmox service.
         - "Uses a configuration file as an inventory source, it must end in C(.proxmox.yml) or C(.proxmox.yaml) and has a C(plugin: xoxys.general.proxmox) entry."
@@ -46,7 +45,7 @@ DOCUMENTATION = """
       verify_ssl:
         description: Skip SSL certificate verification.
         type: boolean
-        default: yes
+        default: True
       auth_timeout:
         description: Proxmox VE authentication timeout.
         type: int
@@ -68,8 +67,8 @@ DOCUMENTATION = """
       want_facts:
         description: Toggle, if C(true) the plugin will retrieve host facts from the server
         type: boolean
-        default: yes
+        default: True
-""" # noqa
+"""  # noqa
 
 EXAMPLES = """
 # proxmox.yml
@@ -82,14 +81,14 @@ password: secure
 import json
 import re
 import socket
+from collections import defaultdict
+from distutils.version import LooseVersion
 
 from ansible.errors import AnsibleError
 from ansible.module_utils._text import to_native
 from ansible.module_utils.parsing.convert_bool import boolean
 from ansible.module_utils.six import iteritems
 from ansible.plugins.inventory import BaseInventoryPlugin
-from collections import defaultdict
-from distutils.version import LooseVersion
 
 try:
     from proxmoxer import ProxmoxAPI
@@ -97,16 +96,33 @@ try:
 except ImportError:
     HAS_PROXMOXER = False
 
+try:
+    from requests.packages import urllib3
+    HAS_URLLIB3 = True
+except ImportError:
+    try:
+        import urllib3
+        HAS_URLLIB3 = True
+    except ImportError:
+        HAS_URLLIB3 = False
+
 
 class InventoryModule(BaseInventoryPlugin):
+    """Provide Proxmox VE inventory."""
+
     NAME = "xoxys.general.proxmox"
 
     def _auth(self):
+        verify_ssl = boolean(self.get_option("verify_ssl"), strict=False)
+
+        if not verify_ssl and HAS_URLLIB3:
+            urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)
+
         return ProxmoxAPI(
             self.get_option("server"),
             user=self.get_option("user"),
             password=self.get_option("password"),
-            verify_ssl=boolean(self.get_option("password"), strict=False),
+            verify_ssl=verify_ssl,
             timeout=self.get_option("auth_timeout")
         )
 
@@ -117,14 +133,12 @@ class InventoryModule(BaseInventoryPlugin):
         return LooseVersion(self.client.version.get()["release"])
 
     def _get_names(self, pve_list, pve_type):
-        names = []
-
         if pve_type == "node":
-            names = [node["node"] for node in pve_list]
+            return [node["node"] for node in pve_list]
-        elif pve_type == "pool":
+        if pve_type == "pool":
-            names = [pool["poolid"] for pool in pve_list]
+            return [pool["poolid"] for pool in pve_list]
 
-        return names
+        return []
 
     def _get_variables(self, pve_list, pve_type):
         variables = {}
@@ -143,11 +157,9 @@ class InventoryModule(BaseInventoryPlugin):
         def validate(address):
             try:
                 # IP address validation
-                if socket.inet_aton(address):
+                if socket.inet_aton(address) and address != "127.0.0.1":
-                    # Ignore localhost
+                    return address
-                    if address != "127.0.0.1":
+            except OSError:
-                        return address
-            except socket.error:
                 return False
 
         address = False
@@ -159,19 +171,18 @@ class InventoryModule(BaseInventoryPlugin):
                     networks = self.client.nodes(pve_node).get(
                         "qemu", vmid, "agent", "network-get-interfaces"
                     )["result"]
-            except Exception:
+            except Exception:  # noqa
                 pass
 
-            if networks:
+            if networks and type(networks) is list:
-                if type(networks) is list:
+                for network in networks:
-                    for network in networks:
+                    for ip_address in network["ip-addresses"]:
-                        for ip_address in network["ip-addresses"]:
+                        address = validate(ip_address["ip-address"])
-                            address = validate(ip_address["ip-address"])
         else:
             try:
                 config = self.client.nodes(pve_node).get(pve_type, vmid, "config")
                 address = re.search(r"ip=(\d*\.\d*\.\d*\.\d*)", config["net0"]).group(1)
-            except Exception:
+            except Exception:  # noqa
                 pass
 
         return address
@@ -197,8 +208,8 @@ class InventoryModule(BaseInventoryPlugin):
             try:
                 qemu_list = self._exclude(self.client.nodes(node).qemu.get())
                 container_list = self._exclude(self.client.nodes(node).lxc.get())
-            except Exception as e:
+            except Exception as e:  # noqa
-                raise AnsibleError("Proxmoxer API error: {0}".format(to_native(e)))
+                raise AnsibleError(f"Proxmoxer API error: {to_native(e)}") from e
 
             # Merge QEMU and Containers lists from this node
             instances = self._get_variables(qemu_list, "qemu").copy()
@@ -217,8 +228,8 @@ class InventoryModule(BaseInventoryPlugin):
                                                               "config")["description"]
                 except KeyError:
                     description = None
-                except Exception as e:
+                except Exception as e:  # noqa
-                    raise AnsibleError("Proxmoxer API error: {0}".format(to_native(e)))
+                    raise AnsibleError(f"Proxmoxer API error: {to_native(e)}") from e
 
                 try:
                     metadata = json.loads(description)
@@ -252,8 +263,8 @@ class InventoryModule(BaseInventoryPlugin):
         for pool in self._get_names(self.client.pools.get(), "pool"):
             try:
                 pool_list = self._exclude(self.client.pool(pool).get()["members"])
-            except Exception as e:
+            except Exception as e:  # noqa
-                raise AnsibleError("Proxmoxer API error: {0}".format(to_native(e)))
+                raise AnsibleError(f"Proxmoxer API error: {to_native(e)}") from e
 
             members = [
                 member["name"]
@@ -266,13 +277,13 @@ class InventoryModule(BaseInventoryPlugin):
 
     def verify_file(self, path):
         """Verify the Proxmox VE configuration file."""
-        if super(InventoryModule, self).verify_file(path):
+        if super().verify_file(path):
             endings = ("proxmox.yaml", "proxmox.yml")
-            if any((path.endswith(ending) for ending in endings)):
+            if any(path.endswith(ending) for ending in endings):
                 return True
         return False
 
-    def parse(self, inventory, loader, path, cache=True):
+    def parse(self, inventory, loader, path, cache=True):  # noqa
         """Dynamically parse the Proxmox VE cloud inventory."""
         if not HAS_PROXMOXER:
             raise AnsibleError(
@@ -280,7 +291,7 @@ class InventoryModule(BaseInventoryPlugin):
                 "https://pypi.org/project/proxmoxer/"
             )
 
-        super(InventoryModule, self).parse(inventory, loader, path)
+        super().parse(inventory, loader, path)
 
         self._read_config_data(path)
         self.client = self._auth()

plugins/modules/iptables_raw.py

@@ -1,3 +1,4 @@
+#!/usr/bin/python
 # -*- coding: utf-8 -*-
 """
 IPtables raw module.
@@ -18,16 +19,20 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 GNU General Public License for more details.
 
 You should have received a copy of the GNU General Public License
-along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+along with Ansible. If not, see <http://www.gnu.org/licenses/>.
 """
 
+from __future__ import (absolute_import, division, print_function)
+
+__metaclass__ = type
+
 ANSIBLE_METADATA = {'status': ['preview'], 'supported_by': 'community', 'metadata_version': '1.0'}
 
 DOCUMENTATION = r'''
 ---
 module: iptables_raw
 short_description: Manage iptables rules
-version_added: "2.4"
+version_added: 1.1.0
 description:
   - Add/remove iptables rules while keeping state.
 options:
@@ -35,13 +40,14 @@ options:
     description:
       - Create a backup of the iptables state file before overwriting it.
     required: false
-    choices: ["yes", "no"]
+    type: bool
-    default: "no"
+    default: False
   ipversion:
     description:
       - Target the IP version this rule is for.
     required: false
     default: "4"
+    type: str
     choices: ["4", "6"]
   keep_unmanaged:
     description:
@@ -54,8 +60,8 @@ options:
         first time, since if you don't specify correct rules, you can block
         yourself out of the managed host."
     required: false
-    choices: ["yes", "no"]
+    type: bool
-    default: "yes"
+    default: True
   name:
     description:
       - Name that will be used as an identifier for these rules. It can contain
@@ -64,17 +70,20 @@ options:
         C(state=absent) to flush all rules in the selected table, or even all
         tables with C(table=*).
     required: true
+    type: str
   rules:
     description:
       - The rules that we want to add. Accepts multiline values.
       - "Note: You can only use C(-A)/C(--append), C(-N)/C(--new-chain), and
         C(-P)/C(--policy) to specify rules."
     required: false
+    type: str
   state:
     description:
       - The state this rules fragment should be in.
     choices: ["present", "absent"]
     required: false
+    type: str
     default: present
   table:
     description:
@@ -82,12 +91,13 @@ options:
         with C(name=*) and C(state=absent) to flush all rules in all tables.
     choices: ["filter", "nat", "mangle", "raw", "security", "*"]
     required: false
+    type: str
     default: filter
   weight:
     description:
       - Determines the order of the rules. Lower C(weight) means higher
         priority. Supported range is C(0 - 99)
-    choices: ["0 - 99"]
+    type: int
     required: false
     default: 40
 notes:
@@ -116,7 +126,7 @@ EXAMPLES = '''
 - iptables_raw:
     name: default_rules
     weight: 10
-    keep_unmanaged: no
+    keep_unmanaged: false
     rules: |
       -A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT
       -A INPUT -i lo -j ACCEPT
@@ -156,12 +166,12 @@ RETURN = '''
 state:
     description: state of the rules
     returned: success
-    type: string
+    type: str
     sample: present
 name:
     description: name of the rules
     returned: success
-    type: string
+    type: str
     sample: open_tcp_80
 weight:
     description: weight of the rules
@@ -176,22 +186,22 @@ ipversion:
 rules:
     description: passed rules
     returned: success
-    type: string
+    type: str
     sample: "-A INPUT -p tcp -m tcp --dport 80 -j ACCEPT"
 table:
     description: iptables table used
     returned: success
-    type: string
+    type: str
     sample: filter
 backup:
     description: if the iptables file should backed up
     returned: success
-    type: boolean
+    type: bool
     sample: False
 keep_unmanaged:
     description: if it should keep unmanaged rules
     returned: success
-    type: boolean
+    type: bool
     sample: True
 '''
 

plugins/modules/openssl_pkcs12.py

@@ -1,25 +1,36 @@
+#!/usr/bin/python
 # -*- coding: utf-8 -*-
+
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 """OpenSSL PKCS12 module."""
 
+from __future__ import (absolute_import, division, print_function)
+
+__metaclass__ = type
+
 ANSIBLE_METADATA = {"metadata_version": "1.0", "status": ["preview"], "supported_by": "community"}
 
 DOCUMENTATION = """
 ---
 module: openssl_pkcs12
 author: "Guillaume Delpierre (@gdelpierre)"
-version_added: "2.4"
+version_added: 1.1.0
 short_description: Generate OpenSSL pkcs12 archive.
 description:
     - "This module allows one to (re-)generate PKCS#12."
 requirements:
     - "python-pyOpenSSL"
+extends_documentation_fragment: files
 options:
     ca_certificates:
         required: False
+        type: list
+        elements: str
         description:
             - List of CA certificate to include.
     cert_path:
         required: False
+        type: path
         description:
             - The path to read certificates and private keys from.
               Must be in PEM format.
@@ -27,61 +38,70 @@ options:
         required: False
         default: "export"
         choices: ["parse", "export"]
+        type: str
         description:
             - Create (export) or parse a PKCS#12.
     src:
         required: False
+        type: path
         description:
             - PKCS#12 file path to parse.
     path:
         required: True
-        default: null
+        type: path
         description:
             - Filename to write the PKCS#12 file to.
     force:
         required: False
         default: False
+        type: bool
         description:
             - Should the file be regenerated even it it already exists.
     friendly_name:
         required: False
-        default: null
+        type: str
-        aliases: "name"
+        aliases:
+            - "name"
         description:
             - Specifies the friendly name for the certificate and private key.
     iter_size:
         required: False
         default: 2048
+        type: int
         description:
             - Number of times to repeat the encryption step.
     maciter_size:
         required: False
         default: 1
+        type: int
         description:
             - Number of times to repeat the MAC step.
     mode:
         required: False
-        default: 0400
+        default: "0400"
+        type: str
         description:
             - Default mode for the generated PKCS#12 file.
     passphrase:
         required: False
-        default: null
+        type: str
         description:
             - The PKCS#12 password.
     privatekey_path:
         required: False
+        type: path
         description:
             - File to read private key from.
     privatekey_passphrase:
         required: False
-        default: null
+        type: str
         description:
             - Passphrase source to decrypt any input private keys with.
     state:
         required: False
         default: "present"
         choices: ["present", "absent"]
+        type: str
         description:
             - Whether the file should exist or not.
 """
@@ -133,7 +153,7 @@ RETURN = """
 filename:
     description: Path to the generate PKCS#12 file.
     returned: changed or success
-    type: string
+    type: str
     sample: /opt/certs/ansible.p12
 """
 
@@ -151,11 +171,11 @@ else:
     pyopenssl_found = True
 
 
-class PkcsError(Exception):
+class PkcsError(Exception):  # noqa
     pass
 
 
-class Pkcs(object):
+class Pkcs(object):  # noqa
 
     def __init__(self, module):
         self.path = module.params["path"]
@@ -181,36 +201,37 @@ class Pkcs(object):
     def load_privatekey(self, path, passphrase=None):
         """Load the specified OpenSSL private key."""
         try:
-            if passphrase:
+            privatekey = crypto.load_privatekey(
-                privatekey = crypto.load_privatekey(
+                crypto.FILETYPE_PEM,
-                    crypto.FILETYPE_PEM,
+                open(path, "rb").read(),  # noqa
-                    open(path, "rb").read(), passphrase
+                passphrase
-                )
+            ) if passphrase else crypto.load_privatekey(
-            else:
+                crypto.FILETYPE_PEM,
-                privatekey = crypto.load_privatekey(crypto.FILETYPE_PEM, open(path, "rb").read())
+                open(path, "rb").read()  # noqa
+            )
 
             return privatekey
-        except (IOError, OSError) as exc:
+        except OSError as exc:
-            raise PkcsError(exc)
+            raise PkcsError(exc) from exc
 
     def load_certificate(self, path):
         """Load the specified certificate."""
         try:
-            cert_content = open(path, "rb").read()
+            cert_content = open(path, "rb").read()  # noqa
             cert = crypto.load_certificate(crypto.FILETYPE_PEM, cert_content)
             return cert
-        except (IOError, OSError) as exc:
+        except OSError as exc:
-            raise PkcsError(exc)
+            raise PkcsError(exc) from exc
 
     def load_pkcs12(self, path, passphrase=None):
         """Load pkcs12 file."""
         try:
             if passphrase:
-                return crypto.load_pkcs12(open(path, "rb").read(), passphrase)
+                return crypto.load_pkcs12(open(path, "rb").read(), passphrase)  # noqa
-            else:
+
-                return crypto.load_pkcs12(open(path, "rb").read())
+            return crypto.load_pkcs12(open(path, "rb").read())  # noqa
-        except (IOError, OSError) as exc:
+        except OSError as exc:
-            raise PkcsError(exc)
+            raise PkcsError(exc) from exc
 
     def dump_privatekey(self, path):
         """Dump the specified OpenSSL private key."""
@@ -219,8 +240,8 @@ class Pkcs(object):
                 crypto.FILETYPE_PEM,
                 self.load_pkcs12(path).get_privatekey()
             )
-        except (IOError, OSError) as exc:
+        except OSError as exc:
-            raise PkcsError(exc)
+            raise PkcsError(exc) from exc
 
     def dump_certificate(self, path):
         """Dump the specified certificate."""
@@ -229,8 +250,8 @@ class Pkcs(object):
                 crypto.FILETYPE_PEM,
                 self.load_pkcs12(path).get_certificate()
             )
-        except (IOError, OSError) as exc:
+        except OSError as exc:
-            raise PkcsError(exc)
+            raise PkcsError(exc) from exc
 
     def generate(self, module):
         """Generate PKCS#12 file archive."""
@@ -264,9 +285,9 @@ class Pkcs(object):
                     )
                 module.set_mode_if_different(self.path, self.mode, False)
                 self.changed = True
-            except (IOError, OSError) as exc:
+            except OSError as exc:
                 self.remove()
-                raise PkcsError(exc)
+                raise PkcsError(exc) from exc
 
         file_args = module.load_file_common_arguments(module.params)
         if module.set_fs_attributes_if_different(file_args, False):
@@ -281,14 +302,12 @@ class Pkcs(object):
 
                 with open(self.path, "wb") as content:
                     content.write(
-                        "{0}{1}".format(
+                        f"{self.dump_privatekey(self.src)}{self.dump_certificate(self.src)}"
-                            self.dump_privatekey(self.src), self.dump_certificate(self.src)
-                        )
                     )
                 module.set_mode_if_different(self.path, self.mode, False)
                 self.changed = True
-            except IOError as exc:
+            except OSError as exc:
-                raise PkcsError(exc)
+                raise PkcsError(exc) from exc
 
         file_args = module.load_file_common_arguments(module.params)
         if module.set_fs_attributes_if_different(file_args, False):
@@ -302,11 +321,11 @@ class Pkcs(object):
             self.changed = True
         except OSError as exc:
             if exc.errno != errno.ENOENT:
-                raise PkcsError(exc)
+                raise PkcsError(exc) from exc
-            else:
-                pass
 
-    def check(self, module, perms_required=True):
+            pass
+
+    def check(self, module, perms_required=True):  # noqa
 
         def _check_pkey_passphrase():
             if self.privatekey_passphrase:
@@ -337,19 +356,20 @@ class Pkcs(object):
 
 def main():
     argument_spec = dict(
-        action=dict(default="export", choices=["parse", "export"], type="str"),
+        action=dict(default="export", choices=["parse", "export"], type="str", required=False),
-        ca_certificates=dict(type="list"),
+        ca_certificates=dict(type="list", elements="str", required=False),
         cert_path=dict(type="path"),
         force=dict(default=False, type="bool"),
         friendly_name=dict(type="str", aliases=["name"]),
         iter_size=dict(default=2048, type="int"),
         maciter_size=dict(default=1, type="int"),
         passphrase=dict(type="str", no_log=True),
-        path=dict(required=True, type="path"),
+        path=dict(type="path", required=True),
         privatekey_path=dict(type="path"),
         privatekey_passphrase=dict(type="str", no_log=True),
         state=dict(default="present", choices=["present", "absent"], type="str"),
         src=dict(type="path"),
+        mode=dict(default="0400", type="str", required=False)
     )
 
     required_if = [
@@ -376,8 +396,7 @@ def main():
     if not os.path.isdir(base_dir):
         module.fail_json(
             name=base_dir,
-            msg="The directory {0} does not exist or "
+            msg=f"The directory {base_dir} does not exist or the file is not a directory"
-            "the file is not a directory".format(base_dir)
         )
 
     pkcs12 = Pkcs(module)

plugins/modules/proxmox_kvm.py

@@ -1,20 +1,69 @@
+#!/usr/bin/python
 # -*- coding: utf-8 -*-
+
 # Copyright: (c) 2016, Abdoul Bah (@helldorado) <bahabdoul at gmail.com>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
-"""Module to control Proxmox KVM machines."""
+"""Control Proxmox KVM machines."""
 
-from __future__ import absolute_import, division, print_function
+from __future__ import (absolute_import, division, print_function)
 
 __metaclass__ = type
 
-DOCUMENTATION = """
+DOCUMENTATION = r"""
 ---
 module: proxmox_kvm
 short_description: Management of Qemu(KVM) Virtual Machines in Proxmox VE cluster.
 description:
   - Allows you to create/delete/stop Qemu(KVM) Virtual Machines in Proxmox VE cluster.
 author: "Abdoul Bah (@helldorado) <bahabdoul at gmail.com>"
+version_added: 1.1.0
 options:
+  vmid:
+    description:
+      - Specifies the instance ID.
+      - If not set the next available ID will be fetched from ProxmoxAPI.
+    type: int
+  node:
+    description:
+      - Proxmox VE node on which to operate.
+      - Only required for I(state=present).
+      - For every other states it will be autodiscovered.
+    type: str
+  pool:
+    description:
+      - Add the new VM to the specified pool.
+    type: str
+  api_host:
+    description:
+      - Specify the target host of the Proxmox VE cluster.
+    type: str
+    required: true
+  api_user:
+    description:
+      - Specify the user to authenticate with.
+    type: str
+    required: true
+  api_password:
+    description:
+      - Specify the password to authenticate with.
+      - You can use C(PROXMOX_PASSWORD) environment variable.
+    type: str
+  api_token_id:
+    description:
+      - Specify the token ID.
+    type: str
+    version_added: 1.3.0
+  api_token_secret:
+    description:
+      - Specify the token secret.
+    type: str
+    version_added: 1.3.0
+  validate_certs:
+    description:
+      - If C(false), SSL certificates will not be validated.
+      - This should only be used on personally controlled sites using self-signed certificates.
+    type: bool
+    default: True
   acpi:
     description:
       - Specify if ACPI should be enabled/disabled.
@@ -65,12 +114,10 @@ options:
     description:
       - 'cloud-init: Specify custom files to replace the automatically generated ones at start.'
     type: str
-    version_added: 1.3.0
   cipassword:
     description:
       - 'cloud-init: password of default user to create.'
     type: str
-    version_added: 1.3.0
   citype:
     description:
       - 'cloud-init: Specifies the cloud-init configuration format.'
@@ -78,12 +125,10 @@ options:
       - We use the C(nocloud) format for Linux, and C(configdrive2) for Windows.
     type: str
     choices: ['nocloud', 'configdrive2']
-    version_added: 1.3.0
   ciuser:
     description:
       - 'cloud-init: username of default user to create.'
     type: str
-    version_added: 1.3.0
   clone:
     description:
       - Name of VM to be cloned. If C(vmid) is setted, C(clone) can take arbitrary value but required for initiating the clone.
@@ -197,7 +242,6 @@ options:
       - For IPv6 the special string 'auto' can be used to use stateless autoconfiguration.
       - If cloud-init is enabled and neither an IPv4 nor an IPv6 address is specified, it defaults to using dhcp on IPv4.
     type: dict
-    version_added: 1.3.0
   keyboard:
     description:
       - Sets the keyboard layout for VNC server.
@@ -249,7 +293,6 @@ options:
       - If unset, PVE host settings are used.
     type: list
     elements: str
-    version_added: 1.3.0
   net:
     description:
       - A hash/dictionary of network interfaces for the VM. C(net='{"key":"value", "key":"value"}').
@@ -341,7 +384,6 @@ options:
       - If unset, PVE host settings are used.
     type: list
     elements: str
-    version_added: 1.3.0
   serial:
     description:
       - A hash/dictionary of serial device to create inside the VM. C('{"key":"value", "key":"value"}').
@@ -379,7 +421,6 @@ options:
     description:
       - 'cloud-init: SSH key to assign to the default user. NOT TESTED with multiple keys but a multi-line value should work.'
     type: str
-    version_added: 1.3.0
   startdate:
     description:
       - Sets the initial date of the real time clock.
@@ -415,7 +456,6 @@ options:
       - Tags are only available in Proxmox 6+.
     type: list
     elements: str
-    version_added: 2.3.0
   target:
     description:
       - Target node. Only allowed if the original VM is on shared storage.
@@ -477,10 +517,6 @@ options:
     choices:
       - compatibility
       - no_defaults
-    version_added: "1.3.0"
-extends_documentation_fragment:
-  - xoxys.general.proxmox.documentation
-  - xoxys.general.proxmox.selection
 """  # noqa
 
 EXAMPLES = """
@@ -711,7 +747,7 @@ EXAMPLES = """
     name: spynal
     node: sabrewulf
     revert: 'template,cpulimit'
-"""  # noqa
+"""
 
 RETURN = """
 vmid:
@@ -735,9 +771,10 @@ import re
 import string
 import time
 import traceback
-from distutils.version import LooseVersion
-from ansible.module_utils.six.moves.urllib.parse import quote
 from collections import defaultdict
+from distutils.version import LooseVersion
+
+from ansible.module_utils.six.moves.urllib.parse import quote
 
 try:
     from proxmoxer import ProxmoxAPI
@@ -745,17 +782,17 @@ try:
 except ImportError:
     HAS_PROXMOXER = False
 
-from ansible.module_utils.basic import AnsibleModule, env_fallback
 from ansible.module_utils._text import to_native
+from ansible.module_utils.basic import AnsibleModule, env_fallback
 
 
 def get_nextvmid(module, proxmox):
     try:
         vmid = proxmox.cluster.nextid.get()
         return vmid
-    except Exception as e:
+    except Exception as e:  # noqa
         module.fail_json(
-            msg="Unable to get next vmid. Failed with exception: {}".format(to_native(e)),
+            msg=f"Unable to get next vmid. Failed with exception: {to_native(e)}",
             exception=traceback.format_exc()
         )
 
@@ -775,14 +812,13 @@ def node_check(proxmox, node):
 
 
 def get_vminfo(module, proxmox, node, vmid, **kwargs):
-    global results  # noqa
+    global results
     results = {}
     try:
         vm = proxmox.nodes(node).qemu(vmid).config.get()
-    except Exception as e:
+    except Exception as e:  # noqa
         module.fail_json(
-            msg="Getting information for VM with vmid = {} failed with exception: {}".
+            msg=f"Getting information for VM with vmid = {vmid} failed with exception: {e}"
-            format(vmid, e)
         )
 
     # Sanitize kwargs. Remove not defined args and ensure True and False converted to int.
@@ -800,16 +836,13 @@ def get_vminfo(module, proxmox, node, vmid, **kwargs):
     results["_raw"] = vm
 
 
-def settings(module, proxmox, vmid, node, name, **kwargs):
+def settings(module, proxmox, vmid, node, name, **kwargs):  # noqa
     proxmox_node = proxmox.nodes(node)
 
     # Sanitize kwargs. Remove not defined args and ensure True and False converted to int.
     kwargs = dict((k, v) for k, v in kwargs.items() if v is not None)
 
-    if proxmox_node.qemu(vmid).config.set(**kwargs) is None:
+    return (proxmox_node.qemu(vmid).config.set(**kwargs) is None)
-        return True
-    else:
-        return False
 
 
 def wait_for_task(module, proxmox, node, taskid):
@@ -840,7 +873,7 @@ def create_vm(
     clone_params = {}
     # Default args for vm. Note: -args option is for experts only. It allows you
     # to pass arbitrary arguments to kvm.
-    vm_args = "-serial unix:/var/run/qemu-server/{0}.serial,server,nowait".format(vmid)
+    vm_args = f"-serial unix:/var/run/qemu-server/{vmid}.serial,server,nowait"
 
     proxmox_node = proxmox.nodes(node)
 
@@ -849,13 +882,13 @@ def create_vm(
     kwargs.update(dict([k, int(v)] for k, v in kwargs.items() if isinstance(v, bool)))
 
     # The features work only on PVE 4+
-    if PVE_MAJOR_VERSION < 4:  # noqa
+    if PVE_MAJOR_VERSION < 4:
         for p in only_v4:
             if p in kwargs:
                 del kwargs[p]
 
     # The features work only on PVE 6
-    if PVE_MAJOR_VERSION < 6:  # noqa
+    if PVE_MAJOR_VERSION < 6:
         for p in only_v6:
             if p in kwargs:
                 del kwargs[p]
@@ -875,19 +908,19 @@ def create_vm(
 
     # If update, ensure existing disks are not recreated.
     if update:
-        for k, v in disks.items():
+        for k, _v in disks.items():
             if results["disks"].get(k):
-                kwargs[k.rstrip(string.digits)][k] = "{0}:{1},{2}".format(
+                storage_id = results["disks"][k]["storage_id"]
-                    results["disks"][k]["storage_id"], results["disks"][k]["storage_opts"],
+                storage_opts = results["disks"][k]["storage_opts"]
-                    ",".join(disks[k]["opts"])
+                opts = ",".join(disks[k]["opts"])
-                )
+                kwargs[k.rstrip(string.digits)][k] = f"{storage_id}:{storage_opts},{opts}"
 
-        for k, v in nets.items():
+        for k, _v in nets.items():
             if results["nets"].get(k):
-                kwargs[k.rstrip(string.digits)][k] = "{0}={1},{2}".format(
+                net_id = results["nets"][k]["net_id"]
-                    results["nets"][k]["net_id"], results["nets"][k]["net_opts"],
+                net_opts = results["nets"][k]["net_opts"]
-                    ",".join(nets[k]["opts"])
+                opts = ",".join(nets[k]["opts"])
-                )
+                kwargs[k.rstrip(string.digits)][k] = f"{net_id}={net_opts},{opts}"
 
     # Convert all dict in kwargs to elements.
     for k in list(kwargs.keys()):
@@ -914,13 +947,15 @@ def create_vm(
     if "tags" in kwargs:
         for tag in kwargs["tags"]:
             if not re.match(r"^[a-z0-9_][a-z0-9_\-\+\.]*$", tag):
-                module.fail_json(msg="{} is not a valid tag".format(tag))
+                module.fail_json(msg=f"{tag} is not a valid tag")
         kwargs["tags"] = ",".join(kwargs["tags"])
 
     # -args and skiplock require root@pam user - but can not use api tokens
-    if module.params["api_user"] == "root@pam" and module.params["args"] is None:
+    if (
-        if not update and module.params["proxmox_default_behavior"] == "compatibility":
+        module.params["api_user"] == "root@pam" and module.params["args"] is None and not update
-            kwargs["args"] = vm_args
+        and module.params["proxmox_default_behavior"] == "compatibility"
+    ):
+        kwargs["args"] = vm_args
     elif module.params["api_user"] == "root@pam" and module.params["args"] is not None:
         kwargs["args"] = module.params["args"]
     elif module.params["api_user"] != "root@pam" and module.params["args"] is not None:
@@ -930,13 +965,12 @@ def create_vm(
         module.fail_json(msg="skiplock parameter require root@pam user. ")
 
     if update:
-        if proxmox_node.qemu(vmid).config.set(
+        return (
-            name=name, memory=memory, cpu=cpu, cores=cores, sockets=sockets, **kwargs
+            proxmox_node.qemu(vmid).config.
-        ) is None:
+            set(name=name, memory=memory, cpu=cpu, cores=cores, sockets=sockets, **kwargs) is None
-            return True
+        )
-        else:
+
-            return False
+    if module.params["clone"] is not None:
-    elif module.params["clone"] is not None:
         for param in valid_clone_params:
             if module.params[param] is not None:
                 clone_params[param] = module.params[param]
@@ -952,8 +986,7 @@ def create_vm(
     if not wait_for_task(module, proxmox, node, taskid):
         module.fail_json(
             msg="Reached timeout while waiting for creating VM."
-            "Last line in task before timeout: {}".
+            f"Last line in task before timeout: {proxmox_node.tasks(taskid).log.get()[:1]}"
-            format(proxmox_node.tasks(taskid).log.get()[:1])
         )
         return False
     return True
@@ -966,8 +999,7 @@ def start_vm(module, proxmox, vm):
     if not wait_for_task(module, proxmox, vm[0]["node"], taskid):
         module.fail_json(
             msg="Reached timeout while waiting for starting VM."
-            "Last line in task before timeout: {}".
+            f"Last line in task before timeout: {proxmox_node.tasks(taskid).log.get()[:1]}"
-            format(proxmox_node.tasks(taskid).log.get()[:1])
         )
         return False
     return True
@@ -980,8 +1012,7 @@ def stop_vm(module, proxmox, vm, force):
     if not wait_for_task(module, proxmox, vm[0]["node"], taskid):
         module.fail_json(
             msg="Reached timeout while waiting for stopping VM."
-            "Last line in task before timeout: {}".
+            f"Last line in task before timeout: {proxmox_node.tasks(taskid).log.get()[:1]}"
-            format(proxmox_node.tasks(taskid).log.get()[:1])
         )
         return False
     return True
@@ -998,7 +1029,7 @@ def main():
             acpi=dict(type="bool"),
             agent=dict(type="bool"),
             args=dict(type="str"),
-            api_host=dict(required=True),
+            api_host=dict(required=True, type="str"),
             api_password=dict(no_log=True, fallback=(env_fallback, ["PROXMOX_PASSWORD"])),
             api_token_id=dict(no_log=True),
             api_token_secret=dict(no_log=True),
@@ -1044,7 +1075,7 @@ def main():
             nameservers=dict(type="list", elements="str"),
             net=dict(type="dict"),
             newid=dict(type="int"),
-            node=dict(),
+            node=dict(type="str"),
             numa=dict(type="dict"),
             numa_enabled=dict(type="bool"),
             onboot=dict(type="bool"),
@@ -1076,7 +1107,7 @@ def main():
             sockets=dict(type="int"),
             sshkeys=dict(type="str", no_log=False),
             startdate=dict(type="str"),
-            startup=dict(),
+            startup=dict(type="str"),
             state=dict(
                 default="present",
                 choices=["present", "absent", "stopped", "started", "restarted", "current"]
@@ -1089,7 +1120,7 @@ def main():
             template=dict(type="bool"),
             timeout=dict(type="int", default=30),
             update=dict(type="bool", default=False),
-            validate_certs=dict(type="bool", default=False),
+            validate_certs=dict(type="bool", default=True),
             vcpus=dict(type="int"),
             vga=dict(
                 choices=[
@@ -1099,7 +1130,7 @@ def main():
             ),
             virtio=dict(type="dict"),
             vmid=dict(type="int"),
-            watchdog=dict(),
+            watchdog=dict(type="str"),
             proxmox_default_behavior=dict(type="str", choices=["compatibility", "no_defaults"]),
         ),
         mutually_exclusive=[("delete", "revert"), ("delete", "update"), ("revert", "update"),
@@ -1168,13 +1199,11 @@ def main():
 
     try:
         proxmox = ProxmoxAPI(api_host, verify_ssl=validate_certs, **auth_args)
-        global PVE_MAJOR_VERSION  # noqa
+        global PVE_MAJOR_VERSION
         version = proxmox_version(proxmox)
         PVE_MAJOR_VERSION = 3 if version < LooseVersion("4.0") else version.version[0]
-    except Exception as e:
+    except Exception as e:  # noqa
-        module.fail_json(
+        module.fail_json(msg=f"authorization on proxmox cluster failed with exception: {e}")
-            msg="authorization on proxmox cluster failed with exception: {}".format(e)
-        )
 
     # If vmid is not defined then retrieve its value from the vm name,
     # the cloned vm name or retrieve the next free VM id from ProxmoxAPI.
@@ -1182,16 +1211,16 @@ def main():
         if state == "present" and not update and not clone and not delete and not revert:
             try:
                 vmid = get_nextvmid(module, proxmox)
-            except Exception:
+            except Exception:  # noqa
                 module.fail_json(
-                    msg="Can't get the next vmid for VM {0} automatically."
+                    msg=f"Can't get the next vmid for VM {name} automatically."
-                    "Ensure your cluster state is good".format(name)
+                    "Ensure your cluster state is good"
                 )
         else:
             clone_target = clone or name
             try:
                 vmid = get_vmid(proxmox, clone_target)[0]
-            except Exception:
+            except Exception:  # noqa
                 vmid = -1
 
     if clone is not None:
@@ -1199,50 +1228,40 @@ def main():
         if not newid:
             try:
                 newid = get_nextvmid(module, proxmox)
-            except Exception:
+            except Exception:  # noqa
                 module.fail_json(
-                    msg="Can't get the next vmid for VM {0} automatically."
+                    msg=f"Can't get the next vmid for VM {name} automatically."
-                    "Ensure your cluster state is good".format(name)
+                    "Ensure your cluster state is good"
                 )
 
         # Ensure source VM name exists when cloning
         if -1 == vmid:
-            module.fail_json(msg="VM with name = {} does not exist in cluster".format(clone))
+            module.fail_json(msg=f"VM with name = {clone} does not exist in cluster")
 
         # Ensure source VM id exists when cloning
         if not get_vm(proxmox, vmid):
-            module.fail_json(
+            module.fail_json(vmid=vmid, msg=f"VM with vmid = {vmid} does not exist in cluster")
-                vmid=vmid, msg="VM with vmid = {} does not exist in cluster".format(vmid)
-            )
 
         # Ensure the choosen VM name doesn't already exist when cloning
         if get_vmid(proxmox, name):
-            module.exit_json(
+            module.exit_json(changed=False, vmid=vmid, msg=f"VM with name <{name}> already exists")
-                changed=False, vmid=vmid, msg="VM with name <{}> already exists".format(name)
-            )
 
         # Ensure the choosen VM id doesn't already exist when cloning
         if get_vm(proxmox, newid):
             module.exit_json(
-                changed=False,
+                changed=False, vmid=vmid, msg=f"vmid {newid} with VM name {name} already exists"
-                vmid=vmid,
-                msg="vmid {} with VM name {} already exists".format(newid, name)
             )
 
     if delete is not None:
         try:
             settings(module, proxmox, vmid, node, name, delete=delete)
             module.exit_json(
-                changed=True,
+                changed=True, vmid=vmid, msg=f"Settings has deleted on VM {name} with vmid {vmid}"
-                vmid=vmid,
-                msg="Settings has deleted on VM {0} with vmid {1}".format(name, vmid)
             )
-        except Exception as e:
+        except Exception as e:  # noqa
             module.fail_json(
                 vmid=vmid,
-                msg="Unable to delete settings on VM {0} with vmid {1}: {2}".format(
+                msg=f"Unable to delete settings on VM {name} with vmid {vmid}: {str(e)}"
-                    name, vmid, str(e)
-                )
             )
 
     if revert is not None:
@@ -1251,29 +1270,29 @@ def main():
             module.exit_json(
                 changed=True,
                 vmid=vmid,
-                msg="Settings has reverted on VM {0} with vmid {1}".format(name, vmid)
+                msg=f"Settings has reverted on VM {name} with vmid {vmid}"
             )
-        except Exception as e:
+        except Exception as e:  # noqa
             module.fail_json(
                 vmid=vmid,
-                msg="Unable to revert settings on VM {0} with vmid {1}:"
+                msg=f"Unable to revert settings on VM {name} with vmid {vmid}:"
-                "Maybe is not a pending task...".format(name, vmid) + str(e)
+                f"Maybe is not a pending task...{str(e)}"
             )
 
     if state == "present":
         try:
             if get_vm(proxmox, vmid) and not (update or clone):
                 module.exit_json(
-                    changed=False, vmid=vmid, msg="VM with vmid <{}> already exists".format(vmid)
+                    changed=False, vmid=vmid, msg=f"VM with vmid <{vmid}> already exists"
                 )
             elif get_vmid(proxmox, name) and not (update or clone):
                 module.exit_json(
-                    changed=False, vmid=vmid, msg="VM with name <{}> already exists".format(name)
+                    changed=False, vmid=vmid, msg=f"VM with name <{name}> already exists"
                 )
             elif not (node, name):
                 module.fail_json(msg="node, name is mandatory for creating/updating vm")
             elif not node_check(proxmox, node):
-                module.fail_json(msg="node '{}' does not exist in cluster".format(node))
+                module.fail_json(msg=f"node '{node}' does not exist in cluster")
 
             if not clone:
                 get_vminfo(
@@ -1362,130 +1381,100 @@ def main():
 
             if update:
                 module.exit_json(
-                    changed=True, vmid=vmid, msg="VM {} with vmid {} updated".format(name, vmid)
+                    changed=True, vmid=vmid, msg=f"VM {name} with vmid {vmid} updated"
                 )
             elif clone is not None:
                 module.exit_json(
                     changed=True,
                     vmid=vmid,
-                    msg="VM {} with newid {} cloned from vm with vmid {}".format(
+                    msg=f"VM {name} with newid {newid} cloned from vm with vmid {vmid}"
-                        name, newid, vmid
-                    )
                 )
             else:
                 module.exit_json(
-                    changed=True,
+                    changed=True, msg=f"VM {name} with vmid {vmid} deployed", **results
-                    msg="VM {} with vmid {} deployed".format(name, vmid),
-                    **results  # noqa
                 )
-        except Exception as e:
+        except Exception as e:  # noqa
             if update:
                 module.fail_json(
-                    vmid=vmid,
+                    vmid=vmid, msg=f"Unable to update vm {name} with vmid {vmid}=" + str(e)
-                    msg="Unable to update vm {0} with vmid {1}=".format(name, vmid) + str(e)
                 )
             elif clone is not None:
                 module.fail_json(
-                    vmid=vmid,
+                    vmid=vmid, msg=f"Unable to clone vm {name} from vmid {vmid}=" + str(e)
-                    msg="Unable to clone vm {0} from vmid {1}=".format(name, vmid) + str(e)
                 )
             else:
                 module.fail_json(
                     vmid=vmid,
-                    msg="creation of qemu VM {} with vmid {} failed with exception={}".format(
+                    msg=f"creation of qemu VM {name} with vmid {vmid} failed with exception={e}"
-                        name, vmid, e
-                    )
                 )
 
     elif state == "started":
         status = {}
         try:
             if -1 == vmid:
-                module.fail_json(msg="VM with name = {} does not exist in cluster".format(name))
+                module.fail_json(msg=f"VM with name = {name} does not exist in cluster")
             vm = get_vm(proxmox, vmid)
             if not vm:
-                module.fail_json(
+                module.fail_json(vmid=vmid, msg=f"VM with vmid <{vmid}> does not exist in cluster")
-                    vmid=vmid, msg="VM with vmid <{}> does not exist in cluster".format(vmid)
-                )
             status["status"] = vm[0]["status"]
             if vm[0]["status"] == "running":
                 module.exit_json(
-                    changed=False,
+                    changed=False, vmid=vmid, msg=f"VM {vmid} is already running", **status
-                    vmid=vmid,
-                    msg="VM {} is already running".format(vmid),
-                    **status
                 )
 
             if start_vm(module, proxmox, vm):
-                module.exit_json(
+                module.exit_json(changed=True, vmid=vmid, msg=f"VM {vmid} started", **status)
-                    changed=True, vmid=vmid, msg="VM {} started".format(vmid), **status
+        except Exception as e:  # noqa
-                )
-        except Exception as e:
             module.fail_json(
-                vmid=vmid,
+                vmid=vmid, msg=f"starting of VM {vmid} failed with exception: {e}", **status
-                msg="starting of VM {} failed with exception: {}".format(vmid, e),
-                **status
             )
 
     elif state == "stopped":
         status = {}
         try:
             if -1 == vmid:
-                module.fail_json(msg="VM with name = {} does not exist in cluster".format(name))
+                module.fail_json(msg=f"VM with name = {name} does not exist in cluster")
 
             vm = get_vm(proxmox, vmid)
             if not vm:
-                module.fail_json(
+                module.fail_json(vmid=vmid, msg=f"VM with vmid = {vmid} does not exist in cluster")
-                    vmid=vmid, msg="VM with vmid = {} does not exist in cluster".format(vmid)
-                )
 
             status["status"] = vm[0]["status"]
             if vm[0]["status"] == "stopped":
                 module.exit_json(
-                    changed=False,
+                    changed=False, vmid=vmid, msg=f"VM {vmid} is already stopped", **status
-                    vmid=vmid,
-                    msg="VM {} is already stopped".format(vmid),
-                    **status
                 )
 
             if stop_vm(module, proxmox, vm, force=module.params["force"]):
                 module.exit_json(
-                    changed=True, vmid=vmid, msg="VM {} is shutting down".format(vmid), **status
+                    changed=True, vmid=vmid, msg=f"VM {vmid} is shutting down", **status
                 )
-        except Exception as e:
+        except Exception as e:  # noqa
             module.fail_json(
-                vmid=vmid,
+                vmid=vmid, msg=f"stopping of VM {vmid} failed with exception: {e}", **status
-                msg="stopping of VM {} failed with exception: {}".format(vmid, e),
-                **status
             )
 
     elif state == "restarted":
         status = {}
         try:
             if -1 == vmid:
-                module.fail_json(msg="VM with name = {} does not exist in cluster".format(name))
+                module.fail_json(msg=f"VM with name = {name} does not exist in cluster")
 
             vm = get_vm(proxmox, vmid)
             if not vm:
-                module.fail_json(
+                module.fail_json(vmid=vmid, msg=f"VM with vmid = {vmid} does not exist in cluster")
-                    vmid=vmid, msg="VM with vmid = {} does not exist in cluster".format(vmid)
-                )
             status["status"] = vm[0]["status"]
             if vm[0]["status"] == "stopped":
                 module.exit_json(
-                    changed=False, vmid=vmid, msg="VM {} is not running".format(vmid), **status
+                    changed=False, vmid=vmid, msg=f"VM {vmid} is not running", **status
                 )
 
             if stop_vm(module, proxmox, vm,
                        force=module.params["force"]) and start_vm(module, proxmox, vm):
-                module.exit_json(
+                module.exit_json(changed=True, vmid=vmid, msg=f"VM {vmid} is restarted", **status)
-                    changed=True, vmid=vmid, msg="VM {} is restarted".format(vmid), **status
+        except Exception as e:  # noqa
-                )
-        except Exception as e:
             module.fail_json(
-                vmid=vmid,
+                vmid=vmid, msg=f"restarting of VM {vmid} failed with exception: {e}", **status
-                msg="restarting of VM {} failed with exception: {}".format(vmid, e),
-                **status
             )
 
     elif state == "absent":
@@ -1504,28 +1493,26 @@ def main():
                     module.exit_json(
                         changed=False,
                         vmid=vmid,
-                        msg="VM {} is running. Stop it before deletion or use force=yes.".
+                        msg=f"VM {vmid} is running. Stop it before deletion or use force=yes."
-                        format(vmid)
                     )
             taskid = proxmox_node.qemu.delete(vmid)
             if not wait_for_task(module, proxmox, vm[0]["node"], taskid):
                 module.fail_json(
                     msg="Reached timeout while waiting for removing VM."
-                    "Last line in task before timeout: {}".
+                    f"Last line in task before timeout: {proxmox_node.tasks(taskid).log.get()[:1]}"
-                    format(proxmox_node.tasks(taskid).log.get()[:1])
                 )
             else:
-                module.exit_json(changed=True, vmid=vmid, msg="VM {} removed".format(vmid))
+                module.exit_json(changed=True, vmid=vmid, msg=f"VM {vmid} removed")
-        except Exception as e:
+        except Exception as e:  # noqa
-            module.fail_json(msg="deletion of VM {} failed with exception: {}".format(vmid, e))
+            module.fail_json(msg=f"deletion of VM {vmid} failed with exception: {e}")
 
     elif state == "current":
         status = {}
         if -1 == vmid:
-            module.fail_json(msg="VM with name = {} does not exist in cluster".format(name))
+            module.fail_json(msg=f"VM with name = {name} does not exist in cluster")
         vm = get_vm(proxmox, vmid)
         if not vm:
-            module.fail_json(msg="VM with vmid = {} does not exist in cluster".format(vmid))
+            module.fail_json(msg=f"VM with vmid = {vmid} does not exist in cluster")
         if not name:
             name = vm[0]["name"]
         current = proxmox.nodes(vm[0]["node"]).qemu(vmid).status.current.get()["status"]
@@ -1534,7 +1521,7 @@ def main():
             module.exit_json(
                 changed=False,
                 vmid=vmid,
-                msg="VM {} with vmid = {} is {}".format(name, vmid, current),
+                msg=f"VM {name} with vmid = {vmid} is {current}",
                 **status
             )
 
@@ -1561,11 +1548,13 @@ def _extract_nets(item):
         if re.match(r"net[0-9]", k):
             nets[k]["opts"] = []
             for val in v.split(","):
-                if any(val.startswith(s) for s in ["e1000", "rtl8139", "virtio", "vmxnet3"]):
+                if (
-                    if len(val.split("=")) == 2:
+                    any(val.startswith(s) for s in ["e1000", "rtl8139", "virtio", "vmxnet3"])
-                        net = val.split("=")
+                    and len(val.split("=")) == 2
-                        nets[k]["net_id"] = net[0]
+                ):
-                        nets[k]["net_opts"] = net[1]
+                    net = val.split("=")
+                    nets[k]["net_id"] = net[0]
+                    nets[k]["net_opts"] = net[1]
                 else:
                     nets[k]["opts"].append(val)
 

plugins/modules/ucr.py

@@ -1,5 +1,12 @@
+#!/usr/bin/python
 # -*- coding: utf-8 -*-
-"""Module to control Univention Corporate Registry."""
+
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+"""Control Univention Corporate Registry."""
+
+from __future__ import (absolute_import, division, print_function)
+
+__metaclass__ = type
 
 ANSIBLE_METADATA = {"metadata_version": "1.1", "status": ["preview"], "supported_by": "community"}
 
@@ -7,7 +14,7 @@ DOCUMENTATION = """
 ---
 module: ucr
 short_description: Manage variables in univention configuration registry.
-version_added: "2.6"
+version_added: 1.1.0
 description:
     - "This module allows to manage variables inside the univention configuration registry
        on a univention corporate server (UCS)."
@@ -15,16 +22,20 @@ options:
     path:
         description:
             - Path for the variable
+        aliases:
+            - name
         required: True
-        default: null
+        type: str
     value:
         description:
             - New value of the variable
         required: False
+        type: str
     state:
         required: False
         default: "present"
         choices: ["present", "absent"]
+        type: str
         description:
             - Whether the variable should be exist or not.
 author:
@@ -49,39 +60,41 @@ RETURN = """
 original_message:
     description: The original name param that was passed in
     type: str
+    returned: success
 message:
     description: The output message that the sample module generates
+    type: str
+    returned: success
 """
 
 from ansible.module_utils.basic import AnsibleModule
-from univention.config_registry import ConfigRegistry  # noqa
+
-from univention.config_registry.frontend import ucr_update  # noqa
+try:
+    from univention.config_registry import ConfigRegistry
+    from univention.config_registry.frontend import ucr_update
+    HAS_UNIVENTION = True
+except ImportError:
+    HAS_UNIVENTION = False
 
 
 def get_variable(ucr, path):
     ucr.load()
-    if path in ucr:
+    return ucr.get(path) if path in ucr else None
-        value = ucr.get(path)
-    else:
-        value = None
-    return value
 
 
-def set_variable(ucr, path, value, result):
+def set_variable(ucr, path, value, result):  # noqa
     org_value = get_variable(ucr, path)
     ucr_update(ucr, {path: value})
     new_value = get_variable(ucr, path)
-    return not org_value == new_value
+    return org_value != new_value
 
 
-def dry_variable(ucr, path, value, result):
+def dry_variable(ucr, path, value, result):  # noqa
     org_value = get_variable(ucr, path)
-    return not org_value == value
+    return org_value != value
 
 
 def main():
-    ucr = ConfigRegistry()
-
     module_args = dict(
         path=dict(type="str", required=True, aliases=["name"]),
         value=dict(type="str", required=False, default=""),
@@ -94,13 +107,17 @@ def main():
         argument_spec=module_args, supports_check_mode=True, required_if=required_if
     )
 
+    if not HAS_UNIVENTION:
+        module.fail_json(msg="univention required for this module")
+
+    ucr = ConfigRegistry()
+
     result = dict(changed=False, original_message="", message="")
 
     path = module.params["path"]
     value = module.params["value"]
-    if module.params["state"] == "present":
+    if module.params["state"] == "present" and (value is None or value == "None"):
-        if value is None or value == "None":
+        value = ""
-            value = ""
     elif module.params["state"] == "absent":
         value = None
 

pyproject.toml

@@ -0,0 +1,128 @@
+[tool.poetry]
+authors = ["Robert Kaussow <mail@thegeeklab.de>"]
+classifiers = [
+  "Development Status :: 5 - Production/Stable",
+  "Environment :: Console",
+  "License :: OSI Approved :: MIT License",
+  "Intended Audience :: Developers",
+  "Intended Audience :: Information Technology",
+  "Intended Audience :: System Administrators",
+  "Natural Language :: English",
+  "Operating System :: POSIX",
+  "Programming Language :: Python :: 3",
+  "Programming Language :: Python :: 3.8",
+  "Programming Language :: Python :: 3.9",
+  "Programming Language :: Python :: 3.10",
+  "Programming Language :: Python :: 3.11",
+  "Topic :: Utilities",
+  "Topic :: Software Development",
+  "Topic :: Software Development :: Documentation",
+]
+description = "Build environment for Ansible Collection."
+license = "MIT"
+name = "xoxys.general"
+readme = "README.md"
+repository = "https://gitea.rknet.org/ansible/xoxys.general"
+version = "0.0.0"
+
+[tool.poetry.dependencies]
+python = "^3.7.0"
+ansible-core = "<=2.14.0"
+pyopenssl = "23.0.0"
+proxmoxer = "2.0.1"
+hcloud = "1.18.2"
+
+[tool.poetry.group.dev.dependencies]
+ruff = "0.0.230"
+pytest = "7.2.1"
+pytest-mock = "3.10.0"
+pytest-cov = "4.0.0"
+toml = "0.10.2"
+yapf = "0.32.0"
+pycodestyle = "2.10.0"
+yamllint = "1.29.0"
+pylint = "<=2.15.0"
+voluptuous = "^0.13.1"
+
+[tool.pytest.ini_options]
+addopts = "--cov --cov-report=xml:coverage.xml --cov-report=term --cov-append --no-cov-on-fail"
+pythonpath = [
+  "."
+]
+filterwarnings = [
+  "ignore::FutureWarning",
+  "ignore::DeprecationWarning",
+  "ignore:.*pep8.*:FutureWarning",
+]
+
+[tool.coverage.run]
+omit = ["**/tests/*"]
+
+[build-system]
+build-backend = "poetry.core.masonry.api"
+requires = ["poetry-core>=1.0.0", "poetry-dynamic-versioning"]
+
+[tool.ruff]
+exclude = [
+    ".git",
+    "__pycache__",
+    "build",
+    "dist",
+    "tests",
+    "*.pyc",
+    "*.egg-info",
+    ".cache",
+    ".eggs",
+    "env*",
+    "iptables_raw.py",
+]
+# Explanation of errors
+#
+# D102: Missing docstring in public method
+# D103: Missing docstring in public function
+# D105: Missing docstring in magic method
+# D107: Missing docstring in __init__
+# D202: No blank lines allowed after function docstring
+# D203: One blank line required before class docstring
+# E402: Module level import not at top of file
+# SIM105: Use `contextlib.suppress(Exception)` instead of try-except-pass
+# C402: Unnecessary generator (rewrite as a `dict` comprehension)
+# C408: Unnecessary `dict` call (rewrite as a literal)
+ignore = [
+    "D102",
+    "D103",
+    "D105",
+    "D107",
+    "D202",
+    "D203",
+    "D212",
+    "E402",
+    "SIM105",
+    "C402",
+    "C408",
+]
+line-length = 99
+select = [
+    "D",
+    "E",
+    "F",
+    "Q",
+    "W",
+    "I",
+    "S",
+    "BLE",
+    "N",
+    "UP",
+    "B",
+    "A",
+    "C4",
+    "T20",
+    "SIM",
+    "RET",
+    "ARG",
+    "ERA",
+    "RUF",
+]
+
+[tool.ruff.flake8-quotes]
+inline-quotes = "double"

requirements.txt

@@ -1,4 +0,0 @@
-ansible-core
-pyopenssl
-proxmoxer
-hcloud

setup.cfg

@@ -1,42 +1,6 @@
-[isort]
-default_section = THIRDPARTY
-sections = FUTURE,STDLIB,THIRDPARTY,FIRSTPARTY,LOCALFOLDER
-force_single_line = true
-line_length = 99
-skip_glob = **/.venv*,**/venv/*,**/docs/*,**/inventory/*,**/modules/*
-
 [yapf]
 based_on_style = google
 column_limit = 99
 dedent_closing_brackets = true
 coalesce_brackets = true
 split_before_logical_operator = true
-
-[tool:pytest]
-filterwarnings =
-    ignore::FutureWarning
-    ignore:.*collections.*:DeprecationWarning
-    ignore:.*pep8.*:FutureWarning
-
-[coverage:run]
-omit =
-    **/test/*
-    **/.venv/*
-
-[flake8]
-ignore = D101, D102, D103, D105, D107, D202, E402, W503, B902
-max-line-length = 99
-inline-quotes = double
-exclude =
-    .git
-    .tox
-    __pycache__
-    build
-    dist
-    test
-    *.pyc
-    *.egg-info
-    .cache
-    .eggs
-    env*
-    iptables_raw.py

test/unit/plugins/inventory/__init__.py

@@ -1 +0,0 @@
-# noqa

test/unit/requirements.txt

@@ -1,11 +0,0 @@
-ansible
-
-# requirement for the proxmox modules
-proxmoxer
-requests
-
-# requirement for the corenetworks modules
-corenetworks
-
-# requirement for the openssl_pkcs12 module
-pyOpenSSL

tests/config.yml

@@ -0,0 +1,2 @@
+modules:
+  python_requires: ">=3.8"

tests/unit/plugins/inventory/test_proxmox.py

@@ -1,10 +1,11 @@
 """Test inventory plugin proxmox."""
-# -*- coding: utf-8 -*-
 # Copyright (c) 2020, Robert Kaussow <mail@thegeeklab.de>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import (absolute_import, division, print_function)
 
+__metaclass__ = type
+
 import pytest
 
 proxmox = pytest.importorskip("proxmoxer")
@@ -58,7 +59,7 @@ def test_get_ip_address(inventory, mocker):
     inventory.client = mocker.MagicMock()
     inventory.client.nodes.return_value.get.return_value = networks
 
-    assert "10.0.0.1" == inventory._get_ip_address("qemu", None, None)
+    assert inventory._get_ip_address("qemu", None, None) == "10.0.0.1"
 
 
 def test_exclude(inventory, mocker):