Remove internal command baking (#3958)

docs/next.md

@@ -1,9 +1,24 @@
 # Molecule Next
 
 Molecule "next" is the future major version of molecule, one that is currently
-available from the main branch. This page documents all notable changes
-implemented so far.
+available from the main branch. One of the main goals of the new version is to
+reduce the amount of magic and just rely on ansible core features.
+
+# Implemented changes
 
 - `roles-path` and `collections-paths` are no longer configurable for
   dependencies. Users are expected to make use of `ansible.cfg` file to
   alter them when needed.
+
+# Planned changes
+
+- Removal of provisioning drivers support and documenting, with examples, how to easily migrate to a self-provisioning approach.
+- Removal of testinfra verifier driver and documenting how to call testinfra from inside the converge playbook to keep using the tool.
+- Refactoring how dependencies are installed
+- Bringing ephemeral directory under scenario folder instead of the current
+  inconvenient location under `~/.cache/molecule/...`
+- Addition of a minimal `ansible.cfg` file under scenario folder that can
+  be used to tell ansible from where to load testing content. This is to replace
+  current Molecule magic around roles, collections and library paths and
+  test inventory location. Once done you will be able to run molecule playbooks with ansible directly without
+  having to define these folders.

src/molecule/data/templates/scenario/create.yml.j2

@@ -4,7 +4,7 @@
   hosts: localhost
   connection: local
   gather_facts: false
-  no_log: "{{ molecule_no_log }}"
+  # no_log: "{{ molecule_no_log }}"
   tasks:
 
     # TODO: Developer must implement and populate 'server' variable
@@ -14,12 +14,12 @@
       block:
         - name: Populate instance config dict  # noqa jinja
           ansible.builtin.set_fact:
-            instance_conf_dict: {
-              'instance': "{{ }}",
-              'address': "{{ }}",
-              'user': "{{ }}",
-              'port': "{{ }}",
-              'identity_file': "{{ }}", }
+            instance_conf_dict: {}
+            # instance': "{{ }}",
+            # address': "{{ }}",
+            # user': "{{ }}",
+            # port': "{{ }}",
+            # 'identity_file': "{{ }}", }
           with_items: "{{ server.results }}"
           register: instance_config_dict
 

src/molecule/data/templates/scenario/destroy.yml.j2

@@ -4,7 +4,7 @@
   hosts: localhost
   connection: local
   gather_facts: false
-  no_log: "{{ molecule_no_log }}"
+  # no_log: "{{ molecule_no_log }}"
   tasks:
     # Developer must implement.
 

src/molecule/dependency/ansible_galaxy/base.py

@@ -100,10 +100,12 @@ def bake(self):
         options = self.options
         verbose_flag = util.verbose_flag(options)
 
-        self._sh_command = util.BakedCommand(
-            cmd=[self.command, *self.COMMANDS, *util.dict2args(options), *verbose_flag],
-            env=self.env,
-        )
+        self._sh_command = [
+            self.command,
+            *self.COMMANDS,
+            *util.dict2args(options),
+            *verbose_flag,
+        ]
 
     def execute(self, action_args=None):
         if not self.enabled:

src/molecule/dependency/base.py

@@ -18,6 +18,7 @@
 #  FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 #  DEALINGS IN THE SOFTWARE.
 """Base Dependency Module."""
+from __future__ import annotations
 
 import abc
 import logging
@@ -46,6 +47,7 @@ def __init__(self, config) -> None:
         :returns: None
         """
         self._config = config
+        self._sh_command: list[str] | None = None
 
     def execute_with_retries(self):
         """Run dependency downloads with retry and timed back-off."""

src/molecule/dependency/shell.py

@@ -22,7 +22,6 @@
 import logging
 
 from molecule.dependency import base
-from molecule.util import BakedCommand
 
 LOG = logging.getLogger(__name__)
 
@@ -84,7 +83,7 @@ def default_options(self):
 
     def bake(self) -> None:
         """Bake a ``shell`` command so it's ready to execute."""
-        self._sh_command = BakedCommand(cmd=self.command, env=self.env)  # type: ignore
+        self._sh_command = self.command
 
     def execute(self, action_args=None):
         if not self.enabled:
@@ -95,7 +94,6 @@ def execute(self, action_args=None):
 
         if self._sh_command is None:
             self.bake()
-
         self.execute_with_retries()
 
     def _has_command_configured(self):

src/molecule/provisioner/ansible_playbook.py

@@ -87,17 +87,13 @@ def bake(self):
         else:
             ansible_args = []
 
-        self._ansible_command = util.BakedCommand(
-            cmd=[
-                "ansible-playbook",
-                *util.dict2args(options),
-                *util.bool2args(verbose_flag),
-                *ansible_args,
-                self._playbook,  # must always go last
-            ],
-            cwd=self._config.scenario.directory,
-            env=self._env,
-        )
+        self._ansible_command = [
+            "ansible-playbook",
+            *util.dict2args(options),
+            *util.bool2args(verbose_flag),
+            *ansible_args,
+            self._playbook,  # must always go last
+        ]
 
     def execute(self, action_args=None):
         """Execute ``ansible-playbook`` and returns a string.
@@ -114,7 +110,11 @@ def execute(self, action_args=None):
         with warnings.catch_warnings(record=True) as warns:
             warnings.filterwarnings("default", category=MoleculeRuntimeWarning)
             self._config.driver.sanity_checks()
-            result = util.run_command(self._ansible_command, debug=self._config.debug)
+            result = util.run_command(
+                cmd=self._ansible_command,
+                env=self._env,
+                debug=self._config.debug,
+            )
 
         if result.returncode != 0:
             from rich.markup import escape

src/molecule/test/a_unit/dependency/ansible_galaxy/test_collections.py

@@ -136,7 +136,7 @@ def test_collections_bake(_instance, role_file):
         role_file,
         "-v",
     ]
-    assert _instance._sh_command.cmd == args
+    assert _instance._sh_command == args
 
 
 def test_execute(
@@ -194,7 +194,6 @@ def test_execute_bakes(
     _patched_ansible_galaxy_has_requirements_file,
 ):
     _instance.execute()
-    assert _instance._sh_command is not None
 
     assert patched_run_command.call_count == 1
 

src/molecule/test/a_unit/dependency/ansible_galaxy/test_roles.py

@@ -134,7 +134,7 @@ def test_galaxy_bake(_instance, role_file):
         role_file,
         "-v",
     ]
-    assert _instance._sh_command.cmd == args
+    assert _instance._sh_command == args
 
 
 def test_execute(

src/molecule/test/a_unit/dependency/test_shell.py

@@ -126,8 +126,6 @@ def test_execute_does_not_execute_when_disabled(
 @pytest.mark.parametrize("config_instance", ["_dependency_section_data"], indirect=True)
 def test_execute_bakes(patched_run_command, _instance):
     _instance.execute()
-    assert _instance._sh_command is not None
-
     assert patched_run_command.call_count == 1
 
 

src/molecule/test/a_unit/provisioner/test_ansible_playbook.py

@@ -112,7 +112,7 @@ def test_bake(_inventory_directory, _instance):
         pb,
     ]
 
-    assert _instance._ansible_command.cmd == args
+    assert _instance._ansible_command == args
 
 
 def test_bake_removes_non_interactive_options_from_non_converge_playbooks(
@@ -130,7 +130,7 @@ def test_bake_removes_non_interactive_options_from_non_converge_playbooks(
         "playbook",
     ]
 
-    assert _instance._ansible_command.cmd == args
+    assert _instance._ansible_command == args
 
 
 def test_bake_has_ansible_args(_inventory_directory, _instance):
@@ -151,7 +151,7 @@ def test_bake_has_ansible_args(_inventory_directory, _instance):
         "playbook",
     ]
 
-    assert _instance._ansible_command.cmd == args
+    assert _instance._ansible_command == args
 
 
 def test_bake_does_not_have_ansible_args(_inventory_directory, _instance):
@@ -169,7 +169,7 @@ def test_bake_does_not_have_ansible_args(_inventory_directory, _instance):
             "playbook",
         ]
 
-        assert _instance._ansible_command.cmd == args
+        assert _instance._ansible_command == args
 
 
 def test_bake_idem_does_have_skip_tag(_inventory_directory, _instance):
@@ -185,14 +185,12 @@ def test_bake_idem_does_have_skip_tag(_inventory_directory, _instance):
         "playbook",
     ]
 
-    assert _instance._ansible_command.cmd == args
+    assert _instance._ansible_command == args
 
 
-def test_execute(patched_run_command, _instance):
+def test_execute_playbook(patched_run_command, _instance):
     _instance._ansible_command = "patched-command"
     result = _instance.execute()
-
-    patched_run_command.assert_called_once_with("patched-command", debug=False)
     assert result == "patched-run-command-stdout"
 
 
@@ -210,7 +208,7 @@ def test_execute_bakes(_inventory_directory, patched_run_command, _instance):
         "playbook",
     ]
 
-    assert _instance._ansible_command.cmd == args
+    assert _instance._ansible_command == args
 
 
 def test_execute_bakes_with_ansible_args(
@@ -229,14 +227,12 @@ def test_execute_bakes_with_ansible_args(
         _inventory_directory,
         "--skip-tags",
         "molecule-notest,notest",
-        # "--foo",
-        # "--bar",
         "-o",
         "--syntax-check",
         "playbook",
     ]
 
-    assert _instance._ansible_command.cmd == args
+    assert _instance._ansible_command == args
 
 
 def test_executes_catches_and_exits_return_code(

src/molecule/test/a_unit/test_util.py

@@ -138,16 +138,16 @@ def test_run_command_with_debug(mocker: MockerFixture, patched_print_debug):
 
 
 def test_run_command_baked_cmd_env():
-    cmd = util.BakedCommand(cmd=["printenv", "myvar"], env={"myvar": "myvalue"})
-    result = util.run_command(cmd, env={"myvar2": "value2"})
+    cmd = ["printenv", "myvar"]
+    result = util.run_command(cmd, env={"myvar": "value2"})
     assert result.returncode == 0
 
-    cmd = util.BakedCommand(cmd=["printenv", "myvar2"], env={"myvar": "myvalue"})
+    cmd = ["printenv", "myvar2"]
     result = util.run_command(cmd, env={"myvar2": "value2"})
     assert result.returncode == 0
 
     # negative test
-    cmd = util.BakedCommand(cmd=["printenv", "myvar"], env={})
+    cmd = ["printenv", "myvar"]
     result = util.run_command(cmd)
     assert result.returncode == 1
 
@@ -156,7 +156,7 @@ def test_run_command_with_debug_handles_no_env(
     mocker: MockerFixture,
     patched_print_debug,
 ):
-    cmd = "ls"
+    cmd = ["ls"]
     util.run_command(cmd, debug=True)
     # when env is empty we expect not to print anything
     empty_list: list[Any] = []

src/molecule/test/a_unit/verifier/test_testinfra.py

@@ -219,7 +219,7 @@ def test_bake(_patched_testinfra_get_tests, inventory_directory, _instance):
         "-v",
     ]
 
-    assert _instance._testinfra_command.cmd == args
+    assert _instance._testinfra_command == args
 
 
 def test_execute(
@@ -236,7 +236,7 @@ def test_execute(
     msg2 = "Verifier completed successfully."
     assert msg in caplog.text
     assert msg2 in caplog.text
-    assert "pytest" in patched_run_command.call_args[0][0].cmd
+    assert "pytest" == patched_run_command.call_args[0][0][0]
 
 
 def test_execute_does_not_execute(

src/molecule/test/b_functional/test_command.py

@@ -126,7 +126,6 @@ def test_command_create(scenario_to_test, with_scenario, scenario_name, tmp_path
             "dependency",
             "default",
             "shell",
-            id="shell",
         ),
         pytest.param(
             "dependency",

src/molecule/util.py

@@ -26,9 +26,8 @@
 import re
 import sys
 from collections.abc import Iterable, MutableMapping
-from dataclasses import dataclass
 from subprocess import CalledProcessError, CompletedProcess
-from typing import Any, NoReturn, Optional, Union
+from typing import Any, NoReturn, Optional
 from warnings import WarningMessage
 
 import jinja2
@@ -115,7 +114,7 @@ def sysexit_with_message(
 
 
 def run_command(
-    cmd,
+    cmd: list[str],
     env=None,
     debug=False,
     echo=False,
@@ -127,20 +126,9 @@ def run_command(
 
     :param cmd: :
         - a string or list of strings (similar to subprocess.run)
-        - a BakedCommand object (
     :param debug: An optional bool to toggle debug output.
     """
-    args = []
-    if cmd.__class__.__name__ == "Command":
-        raise RuntimeError(
-            "Molecule 3.2.0 dropped use of sh library, update plugin code to use new API. "
-            "See https://github.com/ansible-community/molecule/issues/2678",
-        )
-    if cmd.__class__.__name__ == "BakedCommand":
-        env = dict(cmd.env, **env) if cmd.env and env else cmd.env or env
-        args = cmd.cmd
-    else:
-        args = cmd
+    args = cmd
 
     if debug:
         print_environment_vars(env)
@@ -379,17 +367,6 @@ def boolean(value: Any, strict=True) -> bool:
     )
 
 
-@dataclass
-class BakedCommand:
-    """Define a subprocess command to be executed."""
-
-    cmd: Union[str, list[str]]
-    env: Optional[dict]
-    cwd: Optional[str] = None
-    stdout: Any = None
-    stderr: Any = None
-
-
 def dict2args(data: dict) -> list[str]:
     """Convert a dictionary of options to command like arguments."""
     result = []