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

[SVRENG-609] boardwalk (bug): correct job class initialization #86

Merged
merged 2 commits into from
Oct 28, 2024
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
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,14 @@ render-d2:
.PHONY: test
test: test-pytest test-ruff test-pyright test-semgrep

# Run pytest
# Run pytest verbosely if we're running manually, but normally if we're in a CI environment.
.PHONY: test-pytest
test-pytest: develop
ifndef CI
poetry run pytest --verbose
else
poetry run pytest
endif

# Run all available Ruff checks
.PHONY: test-ruff
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ requires-python = ">=3.11"

[tool.poetry]
name = "boardwalk"
version = "0.8.22"
version = "0.8.23"
description = "Boardwalk is a linear Ansible workflow engine"
readme = "README.md"
authors = [
Expand Down
5 changes: 3 additions & 2 deletions src/boardwalk/ansible.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class RunnerKwargs(TypedDict, total=False):
suppress_env_files: bool
playbook: RunnerPlaybook | AnsibleTasksType
verbosity: int
extravars: dict | None
extravars: dict[str, Any] | None

class RunnerKwargsEnvvars(TypedDict, total=False):
ANSIBLE_BECOME_ASK_PASS: str
Expand Down Expand Up @@ -120,6 +120,7 @@ def ansible_runner_run_tasks(
quiet: bool = True,
timeout: int | None = None,
verbosity: int = 0,
extra_vars: dict = {},
) -> ansible_runner.Runner:
"""
Wraps ansible_runner.run to run Ansible tasks with some defaults for
Expand Down Expand Up @@ -161,7 +162,7 @@ def ansible_runner_run_tasks(
if job_type == boardwalk.manifest.JobTypes.PLAYBOOK:
# Executing a (list of) playbook(s) requires some different settings
runner_kwargs["limit"] = hosts
runner_kwargs["extravars"] = {"boardwalk_operation": True}
runner_kwargs["extravars"] = {"boardwalk_operation": True} | extra_vars
runner_kwargs["playbook"] = tasks

output_msg_prefix = f"{hosts}: ansible_runner invocation"
Expand Down
2 changes: 2 additions & 0 deletions src/boardwalk/cli_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,7 @@ def execute_workflow_jobs(host: Host, workspace: Workspace, job_kind: str, verbo
invocation_msg=f"{job_kind}_{job.job_type.name}_Job_{job.name}",
quiet=False,
tasks=tasks,
extra_vars=job.options,
)


Expand Down Expand Up @@ -754,6 +755,7 @@ def execute_host_workflow(host: Host, workspace: Workspace, verbosity: int):
)
host.set_remote_state(remote_state, become_password, _check_mode)

logger.success(f"{host.name}: Host completed successfully; wrapping up")
if boardwalkd_client:
boardwalkd_client.queue_event(
WorkspaceEvent(
Expand Down
2 changes: 2 additions & 0 deletions src/boardwalk/host.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ def ansible_run(
check: bool = False,
gather_facts: bool = True,
quiet: bool = True,
extra_vars: dict = {},
) -> Runner:
"""
Wraps ansible_runner_run_tasks for performing Ansible tasks against this host
Expand All @@ -64,6 +65,7 @@ def ansible_run(
check=check,
gather_facts=gather_facts,
quiet=quiet,
extra_vars=extra_vars,
)

def is_locked(self) -> str | bool:
Expand Down
13 changes: 7 additions & 6 deletions src/boardwalk/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ def _check_options(self, options: dict[str, Any]):
class TaskJob(BaseJob):
"""Defines a single Job as methods, used to execute Tasks"""

def __init__(self):
super().__init__()
def __init__(self, options: dict[str, Any] = dict()):
super().__init__(options=options)
self.job_type = JobTypes.TASK

def tasks(self) -> AnsibleTasksType:
Expand All @@ -171,20 +171,21 @@ class Job(TaskJob):
Deprecated in favor of TaskJob.
"""

def __init__(self):
def __init__(self, options: dict[str, Any] = dict()):
warnings.warn(
"The job type Job is deprecated, and will be removed in a future release. Use TaskJob or PlaybookJob, as appropriate.",
DeprecationWarning,
)
super().__init__()
super().__init__(options=options)


class PlaybookJob(BaseJob):
"""Defines a single Job as methods, used to execute Playbooks"""

def __init__(self):
super().__init__()
def __init__(self, options: dict[str, Any] = dict()):
super().__init__(options=options)
self.job_type = JobTypes.PLAYBOOK
self.extra_vars = options

def tasks(self) -> AnsibleTasksType:
"""Helper method used to return the contents of the self.playbooks() function."""
Expand Down
35 changes: 34 additions & 1 deletion test/boardwalk/test_manifest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,39 @@
import pytest

from boardwalk import Job
from boardwalk import Job, PlaybookJob, TaskJob
from boardwalk.manifest import JobTypes


@pytest.mark.parametrize(
("job_class", "job_type"),
[
pytest.param(TaskJob, JobTypes.TASK),
pytest.param(
Job,
JobTypes.TASK,
marks=pytest.mark.filterwarnings("ignore:The job type Job is deprecated:DeprecationWarning"),
),
pytest.param(PlaybookJob, JobTypes.PLAYBOOK),
],
)
def test_verify_job_types_match_expected_types(job_class, job_type):
job = job_class()
assert job_type == job.job_type


@pytest.mark.parametrize(
("job_class", "function_name"),
[
pytest.param(TaskJob, "tasks"),
pytest.param(Job, "tasks"),
pytest.param(PlaybookJob, "playbooks"),
pytest.param(PlaybookJob, "tasks"),
],
)
def test_verify_job_classes_have_expected_task_functions(job_class, function_name):
# The Job type needs to have the expected function name, and also be a callable function
assert hasattr(job_class, function_name)
assert callable(eval(f"{job_class.__name__}.{function_name}"))


def test_using_not_differentiated_Job_class_warns_about_deprecation():
Expand Down
27 changes: 27 additions & 0 deletions test/integration/test_workspaces.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import platform
import signal
from pathlib import Path
from typing import Any
Expand All @@ -16,6 +17,28 @@
pytest.param("ShouldSucceedTestWorkspace", False, ""),
pytest.param("ShouldSucceedPlaybookExecutionTestWorkspace", False, ""),
pytest.param("UITestVeryVeryLongWorkSpaceNameWorkspace", False, ""),
# Next four are from test/server-client/pylib/regression_bz_svreng_608.py
pytest.param("TaskJobWithOptionsShouldSucceedWorkspace", False, ""),
pytest.param("PlaybookJobWithOptionsShouldSucceedWorkspace", False, ""),
pytest.param(
"TaskJobWithPreconditionsShouldSucceedIfHostIsMacOSXWorkspace",
False,
"",
marks=pytest.mark.skipif(
condition="macos" not in platform.platform().lower(),
reason="Workspace's preconditions depends on the host being MacOS.",
),
),
# Technically this one doesn't _fail_, but this lets it fit neatly into the parameterized tests.
pytest.param(
"TaskJobWithPreconditionsShouldBeSkippedIfHostIsMacOSXWorkspace",
True,
"No hosts meet preconditions",
marks=pytest.mark.skipif(
condition="macos" not in platform.platform().lower(),
reason="Workspace's preconditions depends on the host being MacOS.",
),
),
pytest.param(
"ShouldFailTestWorkspace",
True,
Expand Down Expand Up @@ -54,15 +77,19 @@ async def test_development_workspaces(
with fail_after(delay=90) as scope:
async with await open_process(command=command, env=new_environ) as process:
async for text in TextReceiveStream(process.stdout): # type:ignore
# To allow for reading what was received, if the test ends up failing.
print(text)
output_stdout.append(text)
if failure_expected and "Waiting for release before continuing" in text:
process.send_signal(signal.SIGINT)
async for text in TextReceiveStream(process.stderr): # type:ignore
print(text)
output_stderr.append(text)

assert not scope.cancelled_caught

if failure_expected:
assert failure_msg in "".join(output_stdout)
else:
assert "Host completed successfully; wrapping up" in "".join(output_stdout)
assert process.returncode == 0
12 changes: 7 additions & 5 deletions test/server-client/Boardwalkfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import os
from typing import TYPE_CHECKING

from pylib.regression_bz_svreng_609 import * # noqa: F403

from boardwalk import PlaybookJob, TaskJob, Workflow, Workspace, WorkspaceConfig, path

if TYPE_CHECKING:
Expand Down Expand Up @@ -147,8 +149,8 @@ class ShouldSucceedPlaybookExecutionTestJob(PlaybookJob):

def playbooks(self) -> AnsibleTasksType:
return [
{"ansible.builtin.import_playbook": path("playbook-job-test-should-succeed.yml")},
{"ansible.builtin.import_playbook": path("playbook-job-test-should-be-skipped.yml")},
{"ansible.builtin.import_playbook": path("playbooks/playbook-job-test-should-succeed.yml")},
{"ansible.builtin.import_playbook": path("playbooks/playbook-job-test-should-be-skipped.yml")},
]


Expand All @@ -159,7 +161,7 @@ class ShouldFailPlaybookExecutionTestJob(PlaybookJob):

def playbooks(self) -> AnsibleTasksType:
return [
{"ansible.builtin.import_playbook": path("playbook-job-test-should-succeed.yml")},
{"ansible.builtin.import_playbook": path("playbook-job-test-should-be-skipped.yml")},
{"ansible.builtin.import_playbook": path("playbook-job-test-should-fail.yml")},
{"ansible.builtin.import_playbook": path("playbooks/playbook-job-test-should-succeed.yml")},
{"ansible.builtin.import_playbook": path("playbooks/playbook-job-test-should-be-skipped.yml")},
{"ansible.builtin.import_playbook": path("playbooks/playbook-job-test-should-fail.yml")},
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
- name: Test echoing a set variable
hosts: localhost
tasks:
- name: Tell me my variable, please!
ansible.builtin.debug:
msg: "Your variable is: {{ boardwalk_playbookjob_test_variable }}"
Loading
Loading