Refactor collections action to use new runner API (#211)

Refactor collections action to use new runner API

Reviewed-by: https://github.com/apps/ansible-zuul

ansible_navigator/actions/collections.py

@@ -3,8 +3,9 @@
 import json
 import os
 import shlex
-import subprocess
+import sys
 
+from copy import deepcopy
 from json.decoder import JSONDecodeError
 
 from typing import Any
@@ -17,6 +18,7 @@
 from ..app import App
 from ..app_public import AppPublic
 from ..configuration_subsystem import ApplicationConfiguration
+from ..runner.api import CommandRunner
 from ..steps import Step
 
 from ..ui_framework import CursesLinePart
@@ -113,12 +115,7 @@ def run(self, interaction: Interaction, app: AppPublic) -> Union[Interaction, No
             [self._name] + shlex.split(self._interaction.action.match.groupdict()["params"] or "")
         )
 
-        if self._args.execution_environment:
-            self._logger.debug("running execution environment")
-            self._try_ee()
-        else:
-            self._logger.debug("running local")
-            self._try_local()
+        self._run_runner()
 
         if not self._collections:
             self._prepare_to_exit(interaction)
@@ -247,96 +244,92 @@ def _build_plugin_content(self):
             index=self.steps.current.index,
         )
 
-    def _try_ee(self) -> None:
-        """run collection catalog in ee"""
+    def _run_runner(self) -> None:
+        """spin up runner"""
+
+        if isinstance(self._args.set_environment_variable, dict):
+            set_environment_variable = deepcopy(self._args.set_environment_variable)
+        else:
+            set_environment_variable = {}
+        set_environment_variable.update({"ANSIBLE_NOCOLOR": "True"})
+
+        kwargs = {
+            "container_engine": self._args.container_engine,
+            "execution_environment_image": self._args.execution_environment_image,
+            "execution_environment": self._args.execution_environment,
+            "navigator_mode": "interactive",
+            "pass_environment_variable": self._args.pass_environment_variable,
+            "set_environment_variable": set_environment_variable,
+        }
+
         if isinstance(self._args.playbook, str):
             playbook_dir = os.path.dirname(self._args.playbook)
         else:
             playbook_dir = os.getcwd()
 
-        self._adjacent_collection_dir = playbook_dir + "/collections"
-
-        cmd = [self._args.container_engine, "run", "-i", "-t"]
+        kwargs.update({"cwd": playbook_dir})
 
+        self._adjacent_collection_dir = os.path.join(playbook_dir, "collections")
         share_directory = self._args.internals.share_directory
-        cmd += [
-            "-v",
-            f"{share_directory}/utils:{share_directory}/utils:z",
+
+        pass_through_arg = [
+            f"{share_directory}/utils/catalog_collections.py",
+            "-a",
+            self._adjacent_collection_dir,
+            "-c",
+            self._collection_cache_path,
         ]
 
-        if os.path.exists(self._adjacent_collection_dir):
-            cmd += ["-v", f"{self._adjacent_collection_dir}:{self._adjacent_collection_dir}:z"]
+        kwargs.update({"cmdline": pass_through_arg})
 
-        cmd += ["-v", f"{self._collection_cache_path}:{self._collection_cache_path}:z"]
+        if self._args.execution_environment:
+            self._logger.debug("running collections command with execution environment enabled")
+            python_exec_path = "python3"
 
-        cmd += [self._args.execution_environment_image]
-        cmd += ["python3", f"{self._args.internals.share_directory}/utils/catalog_collections.py"]
-        cmd += ["-a", self._adjacent_collection_dir]
-        cmd += ["-c", self._collection_cache_path]
+            container_volume_mounts = [f"{share_directory}/utils:{share_directory}/utils:z"]
+            if os.path.exists(self._adjacent_collection_dir):
+                container_volume_mounts.append(
+                    f"{self._adjacent_collection_dir}:{self._adjacent_collection_dir}:z"
+                )
 
-        self._logger.debug("ee command: %s", " ".join(cmd))
-        self._dispatch(cmd)
+            container_volume_mounts.append(
+                f"{self._collection_cache_path}:{self._collection_cache_path}:z"
+            )
+            kwargs.update({"container_volume_mounts": container_volume_mounts})
 
-    def _try_local(self) -> None:
-        """run config locally"""
-        if isinstance(self._args.playbook, str):
-            playbook_dir = os.path.dirname(self._args.playbook)
         else:
-            playbook_dir = os.getcwd()
+            self._logger.debug("running collections command locally")
+            python_exec_path = sys.executable
 
-        adjacent_collection_dir = playbook_dir + "/collections"
-
-        cmd = ["python3", f"{self._args.internals.share_directory}/utils/catalog_collections.py"]
-        cmd += ["-a", adjacent_collection_dir]
-        cmd += ["-c", self._collection_cache_path]
-        self._logger.debug("local command: %s", " ".join(cmd))
-        self._dispatch(cmd)
-
-    def _dispatch(self, cmd: List[str]) -> None:
-        """run the individual config commands and parse"""
-        output = self._run_command(cmd)
-        if output is None:
-            return None
-        self._parse(output)
-        return None
-
-    def _run_command(self, cmd) -> Union[None, subprocess.CompletedProcess]:
-        """run a command"""
-        try:
-            proc_out = subprocess.run(
-                " ".join(cmd),
-                stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE,
-                check=True,
-                universal_newlines=True,
-                shell=True,
-            )
-            self._logger.debug("cmd output %s", proc_out.stdout[0:100].replace("\n", " ") + "<...>")
-            return proc_out
+        self._logger.debug(
+            f"Invoke runner with executable_cmd: {python_exec_path}" + f" and kwargs: {kwargs}"
+        )
+        _runner = CommandRunner(executable_cmd=python_exec_path, **kwargs)
+        output, error = _runner.run()
 
-        except subprocess.CalledProcessError as exc:
-            self._logger.debug("command execution failed: '%s'", str(exc))
-            self._logger.debug("command execution failed: '%s'", exc.output)
-            self._logger.debug("command execution failed: '%s'", exc.stderr)
-            return None
+        if error:
+            msg = f"Error while running catalog collection script: {error}"
+            self._logger.error(msg)
+        if output:
+            self._parse(output)
 
     def _parse(self, output) -> None:
         """yaml load the list, and parse the dump
         merge dump int list
         """
         # pylint: disable=too-many-branches
         try:
-            if not output.stdout.startswith("{"):
-                _warnings, json_str = output.stdout.split("{", 1)
+            if not output.startswith("{"):
+                _warnings, json_str = output.split("{", 1)
                 json_str = "{" + json_str
             else:
-                json_str = output.stdout
+                json_str = output
             parsed = json.loads(json_str)
             self._logger.debug("json loading output succeeded")
         except (JSONDecodeError, ValueError) as exc:
             self._logger.error("Unable to extract collection json from stdout")
             self._logger.debug("error json loading output: '%s'", str(exc))
-            self._logger.debug(output.stdout)
+            self._logger.debug(output)
             return None
 
         for error in parsed["errors"]:
@@ -352,11 +345,32 @@ def _parse(self, output) -> None:
             if self._args.execution_environment:
                 if collection["path"].startswith(self._adjacent_collection_dir):
                     collection["__type"] = "bind_mount"
+                elif collection["path"].startswith(os.path.dirname(self._adjacent_collection_dir)):
+                    collection["__type"] = "bind_mount"
+                    error = (
+                        f"{collection['known_as']} was mounted and catalogued in the"
+                        " execution environment but was outside the adjacent 'collections'"
+                        " directory. This may cause issues outside the local development"
+                        " environment."
+                    )
+                    self._logger.error(error)
                 else:
                     collection["__type"] = "contained"
 
         self._stats = parsed["stats"]
 
+        if parsed.get("messages"):
+            for msg in parsed["messages"]:
+                self._logger.info("[catalog_collections]: %s", msg)
+
+        self._logger.debug("catalog collections scan path: %s", parsed["collection_scan_paths"])
         for stat, value in self._stats.items():
             self._logger.debug("%s: %s", stat, value)
+
+        if not parsed["collections"]:
+            env = "execution" if self._args.execution_environment else "local"
+            error = f"No collections found in {env} environment, searched in "
+            error += parsed["collection_scan_paths"]
+            self._logger.warning(error)
+
         return None

share/ansible_navigator/utils/catalog_collections.py

@@ -32,7 +32,7 @@
 from key_value_store import KeyValueStore  # type: ignore
 
 
-PROCESSES = multiprocessing.cpu_count() - 1
+PROCESSES = (multiprocessing.cpu_count() - 1) or 1
 
 
 class CollectionCatalog:
@@ -43,6 +43,7 @@ def __init__(self, directories: List[str]):
         self._directories = directories
         self._collections: OrderedDict[str, Dict] = OrderedDict()
         self._errors: List[Dict[str, str]] = []
+        self._messages: List[str] = []
 
     def _catalog_plugins(self, collection: Dict) -> None:
         """catalog the plugins within a collection"""
@@ -150,6 +151,12 @@ def _one_path(self, directory: str) -> None:
                             self._errors.append({"path": runtime_file, "error": str(exc)})
 
                 self._collections[collection["path"]] = collection
+            else:
+                msg = (
+                    f"collection path '{directory_path}' is ignored as it does not"
+                    " have 'MANIFEST.json' and/or 'galaxy.yml' file(s)."
+                )
+                self._messages.append(msg)
 
     def _find_shadows(self) -> None:
         """for each collection, determin which other collections are hiding it"""
@@ -339,7 +346,8 @@ def main() -> Dict:
     stats["cache_added_success"] = 0
     stats["cache_added_errors"] = 0
 
-    collections, errors = CollectionCatalog(directories=parent_directories).process_directories()
+    cc_obj = CollectionCatalog(directories=parent_directories)
+    collections, errors = cc_obj.process_directories()
     stats["collection_count"] = len(collections)
 
     collection_cache_path = os.path.abspath(os.path.expanduser(args.collection_cache_path))
@@ -361,7 +369,12 @@ def main() -> Dict:
             del collection["plugin_chksums"][no_doc]
 
     collection_cache.close()
-    return {"collections": collections, "errors": errors, "stats": stats}
+    return {
+        "collections": collections,
+        "errors": errors,
+        "stats": stats,
+        "messages": cc_obj._messages,
+    }
 
 
 if __name__ == "__main__":
@@ -375,8 +388,10 @@ def main() -> Dict:
 
     args, parent_directories = parse_args()
 
-    os.environ["ANSIBLE_COLLECTIONS_PATHS"] = ":".join(parent_directories)
+    collection_scan_paths = ":".join(parent_directories)
+    os.environ["ANSIBLE_COLLECTIONS_PATHS"] = collection_scan_paths
 
     result = main()
     result["stats"]["duration"] = (datetime.now() - start_time).total_seconds()
+    result["collection_scan_paths"] = collection_scan_paths
     print(json.dumps(result, default=str))

tests/fixtures/common/collections/ansible_collections/testorg/coll_1/galaxy.yml

@@ -0,0 +1,8 @@
+---
+authors:
+  - Ansible team (ansible-navigator)
+license_file: LICENSE
+name: coll_1
+namespace: testorg
+readme: README.md
+version: 1.0.0

tests/fixtures/common/collections/ansible_collections/testorg/coll_2/galaxy.yml

@@ -0,0 +1,8 @@
+---
+authors:
+  - Ansible team (ansible-navigator)
+license_file: LICENSE
+name: coll_2
+namespace: testorg
+readme: README.md
+version: 2.0.0

tests/fixtures/common/collections/ansible_collections/testorg/coll_2/plugins/lookup/lookup_2.py

@@ -0,0 +1,42 @@
+"""
+The get_path lookup plugin
+"""
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
+
+DOCUMENTATION = """
+    name: lookup_2
+    author: test
+    plugin_type: lookup
+    version_added: "2.0.0"
+    short_description: This is test lookup plugin
+    description:
+        - This is test lookup plugin
+    options:
+        foo:
+            description:
+            - Dummy option I(foo)
+            type: str
+            required: True
+        bar:
+            description:
+            - Dummy option I(bar)
+            default: candidate
+            type: str
+    notes:
+    - This is a dummy lookup plugin
+    """
+
+EXAMPLES = """
+    - name: Retrieve a value deep inside a using a path
+      ansible.builtin.set_fact:
+        value: "{{ lookup('testorg.coll_2.lookup_2', var1, var2) }}"
+    """
+
+RETURN = """
+    _raw:
+        description:
+        - One or more zero-based indices of the matching list items.
+    """

tests/fixtures/common/collections/ansible_collections/testorg/coll_2/plugins/modules/mod_2.py

@@ -0,0 +1,46 @@
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
+
+DOCUMENTATION = """
+module: mod_2
+author:
+- test
+short_description: This is a test module
+description:
+- This is a test module
+version_added: 2.0.0
+options:
+  foo:
+    description:
+    - Dummy option I(foo)
+    type: str
+  bar:
+    description:
+    - Dummy option I(bar)
+    default: candidate
+    type: str
+    choices:
+    - candidate
+    - running
+    aliases:
+    - bam
+notes:
+- This is a dummy module
+"""
+
+EXAMPLES = """
+- name: test task-1
+  testorg.coll_2.mod_2:
+    foo: somevalue
+    bar: candidate
+"""
+
+RETURN = """
+baz:
+    description: test return 1
+    returned: success
+    type: list
+    sample: ['a','b']
+"""