boardwalk (bug): Ensure workflow is set as succeeded false when beginning a workflow (#105)

* boardwalk (bug): Ensure workflow is set as succeeded false when beginning a workflow

This PR adds a test/fix to ensure that when reusing a workspace -- for example, updating
the target version to upgrade an operating system to -- the remote state file is updated
to reflect the fact that the Workspace's Workflow has not yet successfully completed,
prior to said workflow actually beginning.

This ensures that if the reused Workspace's workflow completed successfully, a failure
within the current workflow won't requite manual user intervention to reset the remote
state file's workflow success attribute to false. This allows the workflow to be resumed,
as prior to this if a host met preconditions, and as a result of the workflow they
altered to a state where they are no longer met -- e.g., an OS upgrade -- the workflow
would refuse to allow resumption of the failed workflow, because -- to Boardwalk -- the
host already succeeded.

Compatible with previous release.

Author: asullivan-blze

.github/workflows/make-test.yml

@@ -2,15 +2,19 @@
 name: Run Boardwalk test suites
 
 on:
-  workflow_dispatch:
   pull_request:
+  push:
+    branches:
+      - main
+  workflow_dispatch:
 
 permissions:
   contents: read
   pull-requests: read
 
 jobs:
-  make-test:
+  python-tests:
+    name: Boardwalk Test Suite
     runs-on: ubuntu-latest
 
     steps:
@@ -79,10 +83,35 @@ jobs:
       # Run pytest
       - run: make test-pytest
 
-      # Check code formatting and import sorting
-      - run: make test-ruff
-
       # Commented out because we're planning on switching to a different static
       # typechecker, and frankly whatever is taking pyright _minutes_ to run is
       # a little excessive, for now.
       # - run: make test-pyright
+
+  ansible-lint:
+    name: Ansible Lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Run ansible-lint
+        uses: ansible/[email protected]
+        # optional (see below):
+        with:
+          args: "--config-file ${{ github.workspace }}/test/ansible-lint.yaml"
+          working_directory: ${{ github.workspace }}/test
+
+  ruff-lint-check:
+    name: Ruff - Linting check
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: astral-sh/ruff-action@v1
+
+  ruff-formatting-check:
+    name: Ruff - Formatting check
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: astral-sh/ruff-action@v1
+        with:
+          args: "format --check"

CONTRIBUTING.md

@@ -88,6 +88,18 @@ Automated tests should be developed for cases that clearly improve Boardwalk's
 reliability, user and developer experience. Otherwise, there is no specific
 enforcement of test coverage.
 
+#### `ansible-lint`
+
+Both
+[Ansible](https://github.com/ansible/ansible/tree/devel?tab=GPL-3.0-1-ov-file#readme)
+and
+[`ansible-lint`](https://github.com/ansible/ansible-lint?tab=GPL-3.0-1-ov-file#readme)
+are licensed under the GNU GPLv3. Consequently, to guard against the GPLv3.0
+license attaching to Boardwalk, `ansible-lint` is not included as a development
+dependency, even as an optional development dependency. Consequently, to execute
+`make test-ansible-lint`, `ansible-lint` will need to be available (e.g., via a
+`pipx` install, or similar).
+
 ### Versioning
 
 The boardwalk pip module uses semantic versioning. Please make sure to update

Makefile

@@ -89,7 +89,11 @@ render-d2:
 
 # Runs all available tests
 .PHONY: test
-test: test-pytest test-ruff test-pyright test-semgrep
+test: test-pytest test-ruff test-pyright test-semgrep test-ansible-lint
+
+.PHONY: test-ansible-lint
+test-ansible-lint: develop
+	-cd test && poetry run ansible-lint --config-file ansible-lint.yaml
 
 # Run pytest verbosely if we're running manually, but normally if we're in a CI environment.
 .PHONY: test-pytest

poetry.lock

@@ -11,7 +11,7 @@ requires-python = ">=3.11"
 
 [tool.poetry]
 name = "boardwalk"
-version = "0.8.24"
+version = "0.8.25"
 description = "Boardwalk is a linear Ansible workflow engine"
 readme = "README.md"
 authors = [
@@ -41,23 +41,24 @@ include = [
 "Issues" = "https://github.com/Backblaze/boardwalk/issues"
 
 [tool.poetry.dependencies]
-python = ">=3.11,<4"
+aiohttp = "^3.11.7"                 # Required by slack-bolt's AsyncApp
 ansible-runner = ">=2.3.0"
 click = ">=8.1.3"
 cryptography = ">=38.0.3"
-email-validator = ">=1.3.0"  # Required by pydantic to validate emails using EmailStr
+email-validator = ">=1.3.0"         # Required by pydantic to validate emails using EmailStr
+loguru = "^0.7.2"
 pydantic = ">=2.4.2"
-tornado = ">=6.2"
+python = ">=3.11,<4"
 slack-bolt = "^1.18.1"
-aiohttp = "^3.10.11"           # Required by slack-bolt's AsyncApp
-loguru = "^0.7.2"
+tornado = ">=6.4.2"
+
 
 [tool.poetry.group.dev.dependencies]
+anyio = "^4.6.2.post1"
 pyright = "==1.1.350"
-semgrep = ">=1.92.0"
-ruff = "^0.7.4"
 pytest = "^8.1.1"
-anyio = "^4.6.2.post1"
+ruff = "^0.8.0"
+semgrep = ">=1.92.0"
 
 [tool.poetry.group.docs]
 optional = true

pyproject.toml

@@ -716,8 +716,11 @@ def execute_host_workflow(host: Host, workspace: Workspace, verbosity: int):
     remote_state = host.get_remote_state()
     try:
         remote_state.workspaces[workspace.name].workflow.started = True
+        remote_state.workspaces[workspace.name].workflow.succeeded = False
     except KeyError:
-        remote_state.workspaces[workspace.name] = RemoteStateWorkspace(workflow=RemoteStateWorkflow(started=True))
+        remote_state.workspaces[workspace.name] = RemoteStateWorkspace(
+            workflow=RemoteStateWorkflow(started=True, succeeded=False)
+        )
     host.set_remote_state(remote_state, become_password, _check_mode)
 
     if boardwalkd_client:

src/boardwalk/cli_run.py

@@ -0,0 +1,32 @@
+---
+# .ansible-lint
+
+# exclude_paths included in this file are parsed relative to this file's location
+# and not relative to the CWD of execution. CLI arguments passed to the --exclude
+# option are parsed relative to the CWD of execution.
+exclude_paths:
+  - .cache/ # implicit unless exclude_paths is defined in config
+  - semgrep-rules.yml
+  - server-client/playbooks/malformed_playbook.yml
+# parseable: true
+quiet: true
+# strict: true
+# verbosity: 1
+
+use_default_rules: true
+
+# Ansible-lint does not automatically load rules that have the 'opt-in' tag.
+# You must enable opt-in rules by listing each rule 'id' below.
+enable_list:
+  - args
+  - empty-string-compare # opt-in
+  - no-log-password # opt-in
+  - no-same-owner # opt-in
+  - name[prefix] # opt-in
+  - galaxy-version-incorrect # opt-in
+  # add yaml here if you want to avoid ignoring yaml checks when yamllint
+  # library is missing. Normally its absence just skips using that rule.
+  - yaml
+
+# Allow setting custom prefix for name[prefix] rule
+task_name_prefix: "{stem} | "

test/ansible-lint.yaml

@@ -1,5 +1,6 @@
 import os
 import platform
+import re
 import signal
 from pathlib import Path
 from typing import Any
@@ -9,6 +10,52 @@
 from anyio.streams.text import TextReceiveStream
 
 
+async def execute_boardwalk_workspace_test(
+    workspace_name: str,
+    failure_expected: bool,
+    failure_msg: str,
+    get_become_password_file_path: Path,
+    use_isolated_boardwalk_directory,
+):
+    WORKSPACE_CAUGHT_REGEX = re.compile(
+        rf"The {workspace_name} workspace is remotely caught on .+ Waiting for release before continuing"
+    )
+    envvars: dict[str, Any] = {
+        "ANSIBLE_BECOME_PASSWORD_FILE": get_become_password_file_path,
+        "BOARDWALK_VERBOSITY": "2",
+    }
+    new_environ = os.environ | envvars
+    new_environ.pop("ANSIBLE_BECOME_ASK_PASS", None)
+    commands: tuple[str, ...] = (
+        f"boardwalk -vv workspace use {workspace_name}",
+        "boardwalk init",
+        "boardwalk run",
+    )
+    output_stdout = []
+    output_stderr = []
+    os.chdir(use_isolated_boardwalk_directory)
+    async with create_task_group():
+        for command in commands:
+            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 re.search(WORKSPACE_CAUGHT_REGEX, 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
+
+
 @pytest.mark.anyio
 @pytest.mark.skipif(condition="CI" in os.environ, reason="Not yet able to execute non-interactively.")
 @pytest.mark.parametrize(
@@ -58,38 +105,37 @@ async def test_development_workspaces(
     get_become_password_file_path: Path,
     use_isolated_boardwalk_directory,
 ):
-    envvars: dict[str, Any] = {
-        "ANSIBLE_BECOME_PASSWORD_FILE": get_become_password_file_path,
-        "BOARDWALK_VERBOSITY": "2",
-    }
-    new_environ = os.environ | envvars
-    new_environ.pop("ANSIBLE_BECOME_ASK_PASS", None)
-    commands: tuple[str, ...] = (
-        f"boardwalk -vv workspace use {workspace_name}",
-        "boardwalk init",
-        "boardwalk run",
+    await execute_boardwalk_workspace_test(
+        workspace_name=workspace_name,
+        failure_msg=failure_msg,
+        failure_expected=failure_expected,
+        get_become_password_file_path=get_become_password_file_path,
+        use_isolated_boardwalk_directory=use_isolated_boardwalk_directory,
     )
-    output_stdout = []
-    output_stderr = []
-    os.chdir(use_isolated_boardwalk_directory)
-    async with create_task_group():
-        for command in commands:
-            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
+@pytest.mark.anyio
+@pytest.mark.skipif(condition="CI" in os.environ, reason="Not yet able to execute non-interactively.")
+async def test_ensure_remote_workflow_success_state_false_during_workflow_execution(
+    get_become_password_file_path: Path,
+    use_isolated_boardwalk_directory,
+    workspace_name: str = "RemoteStateShouldHaveWorkflowSucceededValueSetAsFalseDuringExecutionWorkspace",
+    failure_expected: bool = False,
+    failure_msg: str = "",
+):
+    # This first execution ensures the Workspace completes at least once (ensuring the state is set)
+    await execute_boardwalk_workspace_test(
+        workspace_name=workspace_name,
+        failure_msg=failure_msg,
+        failure_expected=failure_expected,
+        get_become_password_file_path=get_become_password_file_path,
+        use_isolated_boardwalk_directory=use_isolated_boardwalk_directory,
+    )
+    # This second execution actually verifies that the succeeded state is False
+    await execute_boardwalk_workspace_test(
+        workspace_name=workspace_name,
+        failure_msg=failure_msg,
+        failure_expected=failure_expected,
+        get_become_password_file_path=get_become_password_file_path,
+        use_isolated_boardwalk_directory=use_isolated_boardwalk_directory,
+    )

test/integration/test_workspaces.py

@@ -4,6 +4,7 @@
 from typing import TYPE_CHECKING
 
 from pylib.regression_bz_svreng_609 import *  # noqa: F403
+from pylib.remote_state_set_unsuccessful_during_active_workflow import *  # noqa: F403
 
 from boardwalk import PlaybookJob, TaskJob, Workflow, Workspace, WorkspaceConfig, path
 
@@ -139,7 +140,7 @@ class MalformedYAMLJob(TaskJob):
     """
 
     def tasks(self) -> AnsibleTasksType:
-        return [{"ansible.builtin.import_tasks": path("malformed_playbook.yml")}]
+        return [{"ansible.builtin.import_tasks": path("playbooks/malformed_playbook.yml")}]
 
 
 class ShouldSucceedPlaybookExecutionTestJob(PlaybookJob):