Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove internal command baking #3958

Merged
merged 1 commit into from
Jul 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions docs/next.md
Original file line number Diff line number Diff line change
@@ -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.
14 changes: 7 additions & 7 deletions src/molecule/data/templates/scenario/create.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/molecule/data/templates/scenario/destroy.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
10 changes: 6 additions & 4 deletions src/molecule/dependency/ansible_galaxy/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions src/molecule/dependency/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."""
Expand Down
4 changes: 1 addition & 3 deletions src/molecule/dependency/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import logging

from molecule.dependency import base
from molecule.util import BakedCommand

LOG = logging.getLogger(__name__)

Expand Down Expand Up @@ -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:
Expand All @@ -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):
Expand Down
24 changes: 12 additions & 12 deletions src/molecule/provisioner/ansible_playbook.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 0 additions & 2 deletions src/molecule/test/a_unit/dependency/test_shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
20 changes: 8 additions & 12 deletions src/molecule/test/a_unit/provisioner/test_ansible_playbook.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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"


Expand All @@ -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(
Expand All @@ -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(
Expand Down
10 changes: 5 additions & 5 deletions src/molecule/test/a_unit/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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] = []
Expand Down
4 changes: 2 additions & 2 deletions src/molecule/test/a_unit/verifier/test_testinfra.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand Down
1 change: 0 additions & 1 deletion src/molecule/test/b_functional/test_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
29 changes: 3 additions & 26 deletions src/molecule/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -115,7 +114,7 @@ def sysexit_with_message(


def run_command(
cmd,
cmd: list[str],
env=None,
debug=False,
echo=False,
Expand All @@ -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)
Expand Down Expand Up @@ -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 = []
Expand Down
Loading