Skip to content

Commit

Permalink
Remove internal command baking
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea committed Jul 10, 2023
1 parent 7584a16 commit bf5cbfc
Show file tree
Hide file tree
Showing 16 changed files with 66 additions and 81 deletions.
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
18 changes: 7 additions & 11 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 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
16 changes: 7 additions & 9 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,7 +185,7 @@ 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):
Expand All @@ -210,7 +210,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 +229,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
11 changes: 6 additions & 5 deletions src/molecule/verifier/testinfra.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,12 @@ def bake(self):
verbose_flag = util.verbose_flag(options)
args = verbose_flag

self._testinfra_command = util.BakedCommand(
cmd=["pytest", *util.dict2args(options), *self._tests, *args],
cwd=self._config.scenario.directory,
env=self.env,
)
self._testinfra_command = [
"pytest",
*util.dict2args(options),
*self._tests,
*args,
]

def execute(self, action_args=None):
if not self.enabled:
Expand Down

0 comments on commit bf5cbfc

Please sign in to comment.