opkg: refactor module to use StateModuleHelper and CmdRunner (#5718)

* opkg: refactor module to use StateModuleHelper and CmdRunner

* add changelog fragment

* Update plugins/modules/opkg.py

Co-authored-by: joergho <48011876+joergho@users.noreply.github.com>

* Update plugins/modules/opkg.py

Co-authored-by: joergho <48011876+joergho@users.noreply.github.com>

* Update plugins/modules/opkg.py

Co-authored-by: joergho <48011876+joergho@users.noreply.github.com>

* Update plugins/modules/opkg.py

Co-authored-by: joergho <48011876+joergho@users.noreply.github.com>

* Update plugins/modules/opkg.py

Co-authored-by: joergho <48011876+joergho@users.noreply.github.com>

* Update plugins/modules/opkg.py

Co-authored-by: joergho <48011876+joergho@users.noreply.github.com>

* Update plugins/modules/opkg.py

Co-authored-by: joergho <48011876+joergho@users.noreply.github.com>

* generate message outcome as before

* aggregated changes from 5688

* fix package query

* add unit tests

* fix sanity error

* Update plugins/modules/opkg.py

Co-authored-by: joergho <48011876+joergho@users.noreply.github.com>

* add test for specifying version

* refactor parameter name

Co-authored-by: joergho <48011876+joergho@users.noreply.github.com>

changelogs/fragments/5718-opkg-refactor.yaml

@@ -0,0 +1,2 @@
+minor_changes:
+  - opkg - refactored module to use ``CmdRunner`` for executing ``opkg`` (https://github.com/ansible-collections/community.general/pull/5718).

plugins/modules/opkg.py

@@ -97,40 +97,51 @@ EXAMPLES = '''
     force: overwrite
 '''
 
-from ansible.module_utils.basic import AnsibleModule
+from ansible_collections.community.general.plugins.module_utils.cmd_runner import CmdRunner, cmd_runner_fmt
-from ansible.module_utils.six.moves import shlex_quote
+from ansible_collections.community.general.plugins.module_utils.module_helper import StateModuleHelper
 
 
-def update_package_db(module, opkg_path):
+class Opkg(StateModuleHelper):
-    """ Updates packages list. """
+    module = dict(
+        argument_spec=dict(
+            name=dict(aliases=["pkg"], required=True, type="list", elements="str"),
+            state=dict(default="present", choices=["present", "installed", "absent", "removed"]),
+            force=dict(default="", choices=["", "depends", "maintainer", "reinstall", "overwrite", "downgrade", "space", "postinstall", "remove",
+                                            "checksum", "removal-of-dependent-packages"]),
+            update_cache=dict(default=False, type='bool'),
+        ),
+    )
 
-    rc, out, err = module.run_command([opkg_path, "update"])
+    def __init_module__(self):
+        self.vars.set("install_c", 0, output=False, change=True)
+        self.vars.set("remove_c", 0, output=False, change=True)
 
-    if rc != 0:
+        state_map = dict(
-        module.fail_json(msg="could not update package db")
+            query="list-installed",
+            present="install",
+            installed="install",
+            absent="remove",
+            removed="remove",
+        )
 
+        def _force(value):
+            if value == "":
+                value = None
+            return cmd_runner_fmt.as_optval("--force-")(value, ctx_ignore_none=True)
 
-def query_package(module, opkg_path, name, version=None, state="present"):
+        self.runner = CmdRunner(
-    """ Returns whether a package is installed or not. """
+            self.module,
+            command="opkg",
+            arg_formats=dict(
+                package=cmd_runner_fmt.as_list(),
+                state=cmd_runner_fmt.as_map(state_map),
+                force=cmd_runner_fmt.as_func(_force),
+                update_cache=cmd_runner_fmt.as_bool("update")
+            ),
+        )
 
-    if state == "present":
+    @staticmethod
-        rc, out, err = module.run_command([opkg_path, "list-installed", name])
+    def split_name_and_version(package):
-        if rc != 0:
-            return False
-        # variable out is one line if the package is installed:
-        # "NAME - VERSION - DESCRIPTION"
-        if version is not None:
-            if not out.startswith("%s - %s " % (name, version)):
-                return False
-        else:
-            if not out.startswith(name + " "):
-                return False
-        return True
-    else:
-        raise NotImplementedError()
-
-
-def split_name_and_version(module, package):
         """ Split the name and the version when using the NAME=VERSION syntax """
         splitted = package.split('=', 1)
         if len(splitted) == 1:
@@ -138,103 +149,54 @@ def split_name_and_version(module, package):
         else:
             return splitted[0], splitted[1]
 
+    def _package_in_desired_state(self, name, want_installed, version=None):
+        dummy, out, dummy = self.runner("state package").run(state="query", package=name)
 
-def remove_packages(module, opkg_path, packages):
+        has_package = out.startswith(name + " - %s" % ("" if not version else (version + " ")))
-    """ Uninstalls one or more packages if installed. """
+        return want_installed == has_package
 
-    p = module.params
+    def state_present(self):
-    force = p["force"]
+        if self.vars.update_cache:
-    if force:
+            dummy, rc, dummy = self.runner("update_cache").run()
-        force = "--force-%s" % force
+            if rc != 0:
-
+                self.do_raise("could not update package db")
-    remove_c = 0
+        with self.runner("state force package") as ctx:
-    # Using a for loop in case of error, we can report the package that failed
+            for package in self.vars.name:
-    for package in packages:
+                pkg_name, pkg_version = self.split_name_and_version(package)
-        package, version = split_name_and_version(module, package)
+                if not self._package_in_desired_state(pkg_name, want_installed=True, version=pkg_version) or self.vars.force == "reinstall":
-
+                    ctx.run(package=package)
-        # Query the package first, to see if we even need to remove
+                    if not self._package_in_desired_state(pkg_name, want_installed=True, version=pkg_version):
-        if not query_package(module, opkg_path, package):
+                        self.do_raise("failed to install %s" % package)
-            continue
+                    self.vars.install_c += 1
-
+        if self.vars.install_c > 0:
-        if force:
+            self.vars.msg = "installed %s package(s)" % (self.vars.install_c)
-            rc, out, err = module.run_command([opkg_path, "remove", force, package])
         else:
-            rc, out, err = module.run_command([opkg_path, "remove", package])
+            self.vars.msg = "package(s) already present"
 
-        if query_package(module, opkg_path, package):
+    def state_absent(self):
-            module.fail_json(msg="failed to remove %s: %s" % (package, out))
+        if self.vars.update_cache:
-
+            dummy, rc, dummy = self.runner("update_cache").run()
-        remove_c += 1
+            if rc != 0:
-
+                self.do_raise("could not update package db")
-    if remove_c > 0:
+        with self.runner("state force package") as ctx:
-
+            for package in self.vars.name:
-        module.exit_json(changed=True, msg="removed %s package(s)" % remove_c)
+                package, dummy = self.split_name_and_version(package)
-
+                if not self._package_in_desired_state(package, want_installed=False):
-    module.exit_json(changed=False, msg="package(s) already absent")
+                    ctx.run(package=package)
-
+                    if not self._package_in_desired_state(package, want_installed=False):
-
+                        self.do_raise("failed to remove %s" % package)
-def install_packages(module, opkg_path, packages):
+                    self.vars.remove_c += 1
-    """ Installs one or more packages if not already installed. """
+        if self.vars.remove_c > 0:
-
+            self.vars.msg = "removed %s package(s)" % (self.vars.remove_c)
-    p = module.params
-    force = p["force"]
-    if force:
-        force = "--force-%s" % force
-
-    install_c = 0
-
-    for package in packages:
-        package, version = split_name_and_version(module, package)
-
-        if query_package(module, opkg_path, package, version) and (force != '--force-reinstall'):
-            continue
-
-        if version is not None:
-            version_str = "=%s" % version
         else:
-            version_str = ""
+            self.vars.msg = "package(s) already absent"
 
-        if force:
+    state_installed = state_present
-            rc, out, err = module.run_command([opkg_path, "install", force, package + version_str])
+    state_removed = state_absent
-        else:
-            rc, out, err = module.run_command([opkg_path, "install", package + version_str])
-
-        if not query_package(module, opkg_path, package, version):
-            module.fail_json(msg="failed to install %s%s: %s" % (package, version_str, out))
-
-        install_c += 1
-
-    if install_c > 0:
-        module.exit_json(changed=True, msg="installed %s package(s)" % (install_c))
-
-    module.exit_json(changed=False, msg="package(s) already present")
 
 
 def main():
-    module = AnsibleModule(
+    Opkg.execute()
-        argument_spec=dict(
-            name=dict(aliases=["pkg"], required=True, type="list", elements="str"),
-            state=dict(default="present", choices=["present", "installed", "absent", "removed"]),
-            force=dict(default="", choices=["", "depends", "maintainer", "reinstall", "overwrite", "downgrade", "space", "postinstall", "remove",
-                                            "checksum", "removal-of-dependent-packages"]),
-            update_cache=dict(default=False, type='bool'),
-        )
-    )
-
-    opkg_path = module.get_bin_path('opkg', True, ['/bin'])
-
-    p = module.params
-
-    if p["update_cache"]:
-        update_package_db(module, opkg_path)
-
-    pkgs = p["name"]
-
-    if p["state"] in ["present", "installed"]:
-        install_packages(module, opkg_path, pkgs)
-
-    elif p["state"] in ["absent", "removed"]:
-        remove_packages(module, opkg_path, pkgs)
 
 
 if __name__ == '__main__':

tests/unit/plugins/modules/test_opkg.py

@@ -0,0 +1,193 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 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 (absolute_import, division, print_function)
+__metaclass__ = type
+
+import json
+
+from collections import namedtuple
+from ansible_collections.community.general.plugins.modules import opkg
+
+import pytest
+
+TESTED_MODULE = opkg.__name__
+
+
+ModuleTestCase = namedtuple("ModuleTestCase", ["id", "input", "output", "run_command_calls"])
+RunCmdCall = namedtuple("RunCmdCall", ["command", "environ", "rc", "out", "err"])
+
+
+@pytest.fixture
+def patch_opkg(mocker):
+    mocker.patch('ansible.module_utils.basic.AnsibleModule.get_bin_path', return_value='/testbin/opkg')
+
+
+TEST_CASES = [
+    ModuleTestCase(
+        id="install_zlibdev",
+        input={"name": "zlib-dev", "state": "present"},
+        output={
+            "msg": "installed 1 package(s)"
+        },
+        run_command_calls=[
+            RunCmdCall(
+                command=["/testbin/opkg", "list-installed", "zlib-dev"],
+                environ={'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': False},
+                rc=0,
+                out="",
+                err="",
+            ),
+            RunCmdCall(
+                command=["/testbin/opkg", "install", "zlib-dev"],
+                environ={'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': False},
+                rc=0,
+                out=(
+                    "Installing zlib-dev (1.2.11-6) to root..."
+                    "Downloading https://downloads.openwrt.org/releases/22.03.0/packages/mips_24kc/base/zlib-dev_1.2.11-6_mips_24kc.ipk"
+                    "Installing zlib (1.2.11-6) to root..."
+                    "Downloading https://downloads.openwrt.org/releases/22.03.0/packages/mips_24kc/base/zlib_1.2.11-6_mips_24kc.ipk"
+                    "Configuring zlib."
+                    "Configuring zlib-dev."
+                ),
+                err="",
+            ),
+            RunCmdCall(
+                command=["/testbin/opkg", "list-installed", "zlib-dev"],
+                environ={'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': False},
+                rc=0,
+                out="zlib-dev - 1.2.11-6\n",
+                err="",
+            ),
+        ],
+    ),
+    ModuleTestCase(
+        id="install_zlibdev_present",
+        input={"name": "zlib-dev", "state": "present"},
+        output={
+            "msg": "package(s) already present"
+        },
+        run_command_calls=[
+            RunCmdCall(
+                command=["/testbin/opkg", "list-installed", "zlib-dev"],
+                environ={'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': False},
+                rc=0,
+                out="zlib-dev - 1.2.11-6\n",
+                err="",
+            ),
+        ],
+    ),
+    ModuleTestCase(
+        id="install_zlibdev_force_reinstall",
+        input={"name": "zlib-dev", "state": "present", "force": "reinstall"},
+        output={
+            "msg": "installed 1 package(s)"
+        },
+        run_command_calls=[
+            RunCmdCall(
+                command=["/testbin/opkg", "list-installed", "zlib-dev"],
+                environ={'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': False},
+                rc=0,
+                out="zlib-dev - 1.2.11-6\n",
+                err="",
+            ),
+            RunCmdCall(
+                command=["/testbin/opkg", "install", "--force-reinstall", "zlib-dev"],
+                environ={'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': False},
+                rc=0,
+                out=(
+                    "Installing zlib-dev (1.2.11-6) to root...\n"
+                    "Downloading https://downloads.openwrt.org/releases/22.03.0/packages/mips_24kc/base/zlib-dev_1.2.11-6_mips_24kc.ipk\n"
+                    "Configuring zlib-dev.\n"
+                ),
+                err="",
+            ),
+            RunCmdCall(
+                command=["/testbin/opkg", "list-installed", "zlib-dev"],
+                environ={'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': False},
+                rc=0,
+                out="zlib-dev - 1.2.11-6\n",
+                err="",
+            ),
+        ],
+    ),
+    ModuleTestCase(
+        id="install_zlibdev_with_version",
+        input={"name": "zlib-dev=1.2.11-6", "state": "present"},
+        output={
+            "msg": "installed 1 package(s)"
+        },
+        run_command_calls=[
+            RunCmdCall(
+                command=["/testbin/opkg", "list-installed", "zlib-dev"],
+                environ={'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': False},
+                rc=0,
+                out="",
+                err="",
+            ),
+            RunCmdCall(
+                command=["/testbin/opkg", "install", "zlib-dev=1.2.11-6"],
+                environ={'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': False},
+                rc=0,
+                out=(
+                    "Installing zlib-dev (1.2.11-6) to root..."
+                    "Downloading https://downloads.openwrt.org/releases/22.03.0/packages/mips_24kc/base/zlib-dev_1.2.11-6_mips_24kc.ipk"
+                    "Installing zlib (1.2.11-6) to root..."
+                    "Downloading https://downloads.openwrt.org/releases/22.03.0/packages/mips_24kc/base/zlib_1.2.11-6_mips_24kc.ipk"
+                    "Configuring zlib."
+                    "Configuring zlib-dev."
+                ),
+                err="",
+            ),
+            RunCmdCall(
+                command=["/testbin/opkg", "list-installed", "zlib-dev"],
+                environ={'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': False},
+                rc=0,
+                out="zlib-dev - 1.2.11-6 \n",   # This output has the extra space at the end, to satisfy the behaviour of Yocto/OpenEmbedded's opkg
+                err="",
+            ),
+        ],
+    ),
+]
+TEST_CASES_IDS = [item.id for item in TEST_CASES]
+
+
+@pytest.mark.parametrize('patch_ansible_module, testcase',
+                         [[x.input, x] for x in TEST_CASES],
+                         ids=TEST_CASES_IDS,
+                         indirect=['patch_ansible_module'])
+@pytest.mark.usefixtures('patch_ansible_module')
+def test_opkg(mocker, capfd, patch_opkg, testcase):
+    """
+    Run unit tests for test cases listen in TEST_CASES
+    """
+
+    run_cmd_calls = testcase.run_command_calls
+
+    # Mock function used for running commands first
+    call_results = [(x.rc, x.out, x.err) for x in run_cmd_calls]
+    mock_run_command = mocker.patch('ansible.module_utils.basic.AnsibleModule.run_command', side_effect=call_results)
+
+    # Try to run test case
+    with pytest.raises(SystemExit):
+        opkg.main()
+
+    out, err = capfd.readouterr()
+    results = json.loads(out)
+    print("testcase =\n%s" % str(testcase))
+    print("results =\n%s" % results)
+
+    for test_result in testcase.output:
+        assert results[test_result] == testcase.output[test_result], \
+            "'{0}': '{1}' != '{2}'".format(test_result, results[test_result], testcase.output[test_result])
+
+    call_args_list = [(item[0][0], item[1]) for item in mock_run_command.call_args_list]
+    expected_call_args_list = [(item.command, item.environ) for item in run_cmd_calls]
+    print("call args list =\n%s" % call_args_list)
+    print("expected args list =\n%s" % expected_call_args_list)
+
+    assert mock_run_command.call_count == len(run_cmd_calls)
+    if mock_run_command.call_count:
+        assert call_args_list == expected_call_args_list