From 466d177e910a055d5ce4305a908bbda81dc9ed31 Mon Sep 17 00:00:00 2001 From: "shipmill-romamo[bot]" <338557544+shipmill-romamo[bot]@users.noreply.github.com> Date: Tue, 20 Oct 2026 12:34:22 +0300 Subject: [PATCH 1/2] Gate type-none components on their systemd unit A component declaring healthCheck: {type: none} (a queue worker with no HTTP endpoint) passed schema validation, but the deploy role always ran the HTTP uri gate with healthCheck.scheme/port/path, which are undefined for none. The task failed, the rescue ran, and a first release was stopped. The HTTP gate now runs only for type http. For type none the gate requires the service unit to be active right after the restart and still active after a fixed settle time (role default cloudfall_deploy_unit_settle_seconds, 10 s), so a worker that crashes on start fails inside the block and goes through the same rescue and rollback. Check mode still skips the gate; the receipt is unchanged. Fixes #96 Co-Authored-By: Claude Opus 5.5 --- changelog.d/96-healthcheck-none.md | 3 + .../roles/cloudfall_deploy/defaults/main.yml | 3 + .../cloudfall_deploy/meta/argument_specs.yml | 3 + .../roles/cloudfall_deploy/tasks/main.yml | 37 ++++- engine/tests/test_deploy_role.py | 151 ++++++++++++++++++ 5 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 changelog.d/96-healthcheck-none.md create mode 100644 engine/tests/test_deploy_role.py diff --git a/changelog.d/96-healthcheck-none.md b/changelog.d/96-healthcheck-none.md new file mode 100644 index 0000000..73683e4 --- /dev/null +++ b/changelog.d/96-healthcheck-none.md @@ -0,0 +1,3 @@ +### Fixed + +- A deploy of a component with `healthCheck: {type: none}` no longer fails its health gate on the missing HTTP fields and stops the release: the gate now requires the systemd unit to be active right after the restart and still active 10 seconds later, so a worker that crashes on start still fails and rolls back (#96) diff --git a/engine/ansible/roles/cloudfall_deploy/defaults/main.yml b/engine/ansible/roles/cloudfall_deploy/defaults/main.yml index 674dc89..155d83f 100644 --- a/engine/ansible/roles/cloudfall_deploy/defaults/main.yml +++ b/engine/ansible/roles/cloudfall_deploy/defaults/main.yml @@ -1,6 +1,9 @@ --- cloudfall_deploy_environment_file: "" cloudfall_deploy_receipt_directory: "" +# A component without an HTTP health check passes its gate when its unit is +# active right after the restart and still active this many seconds later. +cloudfall_deploy_unit_settle_seconds: 10 cloudfall_deploy_uv_version: "0.12.10" cloudfall_deploy_uv_sha256: >- 173d95a0c32d18c896c46ba6fafbf3cf9c14ab74b033f81b76c883ef492a976b diff --git a/engine/ansible/roles/cloudfall_deploy/meta/argument_specs.yml b/engine/ansible/roles/cloudfall_deploy/meta/argument_specs.yml index 800ca9c..b3bed2d 100644 --- a/engine/ansible/roles/cloudfall_deploy/meta/argument_specs.yml +++ b/engine/ansible/roles/cloudfall_deploy/meta/argument_specs.yml @@ -95,6 +95,9 @@ argument_specs: cloudfall_deploy_receipt_directory: type: str default: "" + cloudfall_deploy_unit_settle_seconds: + type: int + default: 10 cloudfall_deploy_uv_version: type: str default: "0.12.10" diff --git a/engine/ansible/roles/cloudfall_deploy/tasks/main.yml b/engine/ansible/roles/cloudfall_deploy/tasks/main.yml index bf75e20..bcb45c9 100644 --- a/engine/ansible/roles/cloudfall_deploy/tasks/main.yml +++ b/engine/ansible/roles/cloudfall_deploy/tasks/main.yml @@ -203,7 +203,42 @@ until: >- cloudfall_deploy_health.status in cloudfall_deploy_component.healthCheck.expectedStatuses - when: not ansible_check_mode + when: + - not ansible_check_mode + - cloudfall_deploy_component.healthCheck.type == "http" + + - name: Gate the release on an active service unit + ansible.builtin.command: + argv: + - /usr/bin/systemctl + - is-active + - --quiet + - "{{ cloudfall_deploy_component.service.name }}.service" + register: cloudfall_deploy_unit_started + changed_when: false + when: + - not ansible_check_mode + - cloudfall_deploy_component.healthCheck.type == "none" + + - name: Let the service unit settle + ansible.builtin.pause: + seconds: "{{ cloudfall_deploy_unit_settle_seconds }}" + when: + - not ansible_check_mode + - cloudfall_deploy_component.healthCheck.type == "none" + + - name: Gate the release on a service unit still active after settling + ansible.builtin.command: + argv: + - /usr/bin/systemctl + - is-active + - --quiet + - "{{ cloudfall_deploy_component.service.name }}.service" + register: cloudfall_deploy_unit_settled + changed_when: false + when: + - not ansible_check_mode + - cloudfall_deploy_component.healthCheck.type == "none" rescue: - name: Restore the previous release symlink ansible.builtin.file: diff --git a/engine/tests/test_deploy_role.py b/engine/tests/test_deploy_role.py new file mode 100644 index 0000000..81334f0 --- /dev/null +++ b/engine/tests/test_deploy_role.py @@ -0,0 +1,151 @@ +"""The deploy role's health gate, evaluated with Ansible's own templating. + +The role needs a Debian host with systemd, so these tests replay what it +decides rather than running it: which gate tasks run for each health-check +type, and which rescue steps run when a gate task fails. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from ansible.parsing.dataloader import DataLoader +from ansible.template import Templar, trust_as_template + +ROOT = Path(__file__).parents[2] +ROLE = ROOT / "engine" / "ansible" / "roles" / "cloudfall_deploy" +ACTIVATE = "Activate the release behind the health gate" +HTTP_GATE = "Gate the release on its declared health check" +UNIT_STARTED = "Gate the release on an active service unit" +UNIT_SETTLE = "Let the service unit settle" +UNIT_SETTLED = "Gate the release on a service unit still active after settling" +UNIT_GATE = [UNIT_STARTED, UNIT_SETTLE, UNIT_SETTLED] +RESTORE = "Restore the previous release symlink" +RESTART_RESTORED = "Restart the restored release" +STOP_FIRST = "Stop the failed first release" +FAIL = "Fail the deployment after rolling back" +HTTP_CHECK: dict[str, object] = { + "type": "http", + "scheme": "http", + "port": 8100, + "path": "/healthz", + "expectedStatuses": [200], + "timeoutSeconds": 5, + "attempts": 10, +} +NO_CHECK: dict[str, object] = {"type": "none"} + + +def _load(path: Path) -> object: + return DataLoader().load_from_file(str(path)) + + +def _activation() -> dict[str, object]: + tasks = _load(ROLE / "tasks" / "main.yml") + assert isinstance(tasks, list) + return next(t for t in tasks if t["name"] == ACTIVATE) + + +def _section(key: str) -> list[dict[str, object]]: + section = _activation()[key] + assert isinstance(section, list) + return section + + +def _task(name: str, tasks: list[dict[str, object]]) -> dict[str, object]: + return next(t for t in tasks if t["name"] == name) + + +def _templar(variables: dict[str, object]) -> Templar: + return Templar(loader=DataLoader(), variables=variables) + + +def _holds(expression: object, variables: dict[str, object]) -> bool: + return bool( + _templar(variables).evaluate_conditional(trust_as_template(str(expression))) + ) + + +def _conditions(task: dict[str, object]) -> list[object]: + value = task.get("when", []) + return value if isinstance(value, list) else [value] + + +def _gate(health_check: dict[str, object], *, check_mode: bool) -> list[str]: + """Name, in order, every gate task whose ``when`` holds for this component.""" + variables: dict[str, object] = { + "ansible_check_mode": check_mode, + "cloudfall_deploy_component": { + "service": {"name": "crm-worker"}, + "healthCheck": health_check, + }, + } + return [ + str(task["name"]) + for task in _section("block") + if task["name"] in {HTTP_GATE, *UNIT_GATE} + and all(_holds(c, variables) for c in _conditions(task)) + ] + + +def _rescue(previous_directory: str) -> set[str]: + """Name every rescue step that runs once a gate task has failed.""" + variables: dict[str, object] = { + "cloudfall_deploy_previous_directory": previous_directory + } + return { + str(task["name"]) + for task in _section("rescue") + if all(_holds(c, variables) for c in _conditions(task)) + } + + +def test_an_http_component_keeps_its_http_gate() -> None: + assert _gate(HTTP_CHECK, check_mode=False) == [HTTP_GATE] + + +def test_a_component_without_http_is_gated_on_its_unit_staying_active() -> None: + """The unit must be active after the restart and still active after settling.""" + assert _gate(NO_CHECK, check_mode=False) == UNIT_GATE + + +@pytest.mark.parametrize("health_check", [HTTP_CHECK, NO_CHECK]) +def test_check_mode_skips_the_gate(health_check: dict[str, object]) -> None: + assert _gate(health_check, check_mode=True) == [] + + +@pytest.mark.parametrize("name", [UNIT_STARTED, UNIT_SETTLED]) +def test_an_inactive_unit_fails_its_gate_task(name: str) -> None: + """``systemctl is-active`` exits non-zero for an exited unit, failing the task.""" + task = _task(name, _section("block")) + command = task["ansible.builtin.command"] + assert isinstance(command, dict) + + assert command["argv"][:3] == ["/usr/bin/systemctl", "is-active", "--quiet"] + assert "failed_when" not in task + assert "ignore_errors" not in task + assert task["changed_when"] is False + + +def test_the_unit_settles_for_the_role_default_before_its_second_check() -> None: + defaults = _load(ROLE / "defaults" / "main.yml") + assert isinstance(defaults, dict) + pause = _task(UNIT_SETTLE, _section("block"))["ansible.builtin.pause"] + assert isinstance(pause, dict) + + seconds = _templar(defaults).template(trust_as_template(pause["seconds"])) + + assert int(str(seconds)) == 10 + + +def test_a_first_release_whose_unit_exits_is_stopped() -> None: + assert _rescue("") == {STOP_FIRST, FAIL} + + +def test_a_release_whose_unit_exits_rolls_back() -> None: + assert _rescue("/srv/apps/crm/worker/releases/r1") == { + RESTORE, + RESTART_RESTORED, + FAIL, + } From fef420ede793681685eae88cf6998007db5f7890 Mon Sep 17 00:00:00 2001 From: "shipmill-romamo[bot]" <338557544+shipmill-romamo[bot]@users.noreply.github.com> Date: Tue, 20 Oct 2026 12:49:47 +0300 Subject: [PATCH 2/2] Fail the type-none gate when the unit restarted while settling The unit template sets Restart=on-failure, so a worker that crash-loops can read as active at the second is-active check. The gate now reads NRestarts before and after the settle pause and fails, inside the same block so the rescue still runs, when it went up or is unreadable. Co-Authored-By: Claude Opus 5.5 --- changelog.d/96-healthcheck-none.md | 2 +- .../roles/cloudfall_deploy/tasks/main.yml | 44 +++++++++++++++ engine/tests/test_deploy_role.py | 54 ++++++++++++++++++- 3 files changed, 98 insertions(+), 2 deletions(-) diff --git a/changelog.d/96-healthcheck-none.md b/changelog.d/96-healthcheck-none.md index 73683e4..2a208b6 100644 --- a/changelog.d/96-healthcheck-none.md +++ b/changelog.d/96-healthcheck-none.md @@ -1,3 +1,3 @@ ### Fixed -- A deploy of a component with `healthCheck: {type: none}` no longer fails its health gate on the missing HTTP fields and stops the release: the gate now requires the systemd unit to be active right after the restart and still active 10 seconds later, so a worker that crashes on start still fails and rolls back (#96) +- A deploy of a component with `healthCheck: {type: none}` no longer fails its health gate on the missing HTTP fields and stops the release: the gate now requires the systemd unit to be active right after the restart and still active, with no automatic restart in between, 10 seconds later, so a worker that crashes or crash-loops on start still fails and rolls back (#96) diff --git a/engine/ansible/roles/cloudfall_deploy/tasks/main.yml b/engine/ansible/roles/cloudfall_deploy/tasks/main.yml index bcb45c9..5469efb 100644 --- a/engine/ansible/roles/cloudfall_deploy/tasks/main.yml +++ b/engine/ansible/roles/cloudfall_deploy/tasks/main.yml @@ -220,6 +220,20 @@ - not ansible_check_mode - cloudfall_deploy_component.healthCheck.type == "none" + - name: Read the service unit restart count before settling + ansible.builtin.command: + argv: + - /usr/bin/systemctl + - show + - --property=NRestarts + - --value + - "{{ cloudfall_deploy_component.service.name }}.service" + register: cloudfall_deploy_unit_restarts_before + changed_when: false + when: + - not ansible_check_mode + - cloudfall_deploy_component.healthCheck.type == "none" + - name: Let the service unit settle ansible.builtin.pause: seconds: "{{ cloudfall_deploy_unit_settle_seconds }}" @@ -239,6 +253,36 @@ when: - not ansible_check_mode - cloudfall_deploy_component.healthCheck.type == "none" + + - name: Read the service unit restart count after settling + ansible.builtin.command: + argv: + - /usr/bin/systemctl + - show + - --property=NRestarts + - --value + - "{{ cloudfall_deploy_component.service.name }}.service" + register: cloudfall_deploy_unit_restarts_after + changed_when: false + when: + - not ansible_check_mode + - cloudfall_deploy_component.healthCheck.type == "none" + + - name: Gate the release on a service unit that did not restart while settling + ansible.builtin.assert: + that: + - cloudfall_deploy_unit_restarts_before.stdout is match('^[0-9]+$') + - cloudfall_deploy_unit_restarts_after.stdout is match('^[0-9]+$') + - cloudfall_deploy_unit_restarts_after.stdout | int + == cloudfall_deploy_unit_restarts_before.stdout | int + fail_msg: >- + {{ cloudfall_deploy_component.service.name }}.service restarted + while settling (NRestarts {{ + cloudfall_deploy_unit_restarts_before.stdout + }} to {{ cloudfall_deploy_unit_restarts_after.stdout }}) + when: + - not ansible_check_mode + - cloudfall_deploy_component.healthCheck.type == "none" rescue: - name: Restore the previous release symlink ansible.builtin.file: diff --git a/engine/tests/test_deploy_role.py b/engine/tests/test_deploy_role.py index 81334f0..1889e54 100644 --- a/engine/tests/test_deploy_role.py +++ b/engine/tests/test_deploy_role.py @@ -18,9 +18,19 @@ ACTIVATE = "Activate the release behind the health gate" HTTP_GATE = "Gate the release on its declared health check" UNIT_STARTED = "Gate the release on an active service unit" +RESTARTS_BEFORE = "Read the service unit restart count before settling" UNIT_SETTLE = "Let the service unit settle" UNIT_SETTLED = "Gate the release on a service unit still active after settling" -UNIT_GATE = [UNIT_STARTED, UNIT_SETTLE, UNIT_SETTLED] +RESTARTS_AFTER = "Read the service unit restart count after settling" +NO_RESTART = "Gate the release on a service unit that did not restart while settling" +UNIT_GATE = [ + UNIT_STARTED, + RESTARTS_BEFORE, + UNIT_SETTLE, + UNIT_SETTLED, + RESTARTS_AFTER, + NO_RESTART, +] RESTORE = "Restore the previous release symlink" RESTART_RESTORED = "Restart the restored release" STOP_FIRST = "Stop the failed first release" @@ -128,6 +138,48 @@ def test_an_inactive_unit_fails_its_gate_task(name: str) -> None: assert task["changed_when"] is False +@pytest.mark.parametrize("name", [RESTARTS_BEFORE, RESTARTS_AFTER]) +def test_the_restart_count_is_read_from_systemd(name: str) -> None: + task = _task(name, _section("block")) + command = task["ansible.builtin.command"] + assert isinstance(command, dict) + + assert command["argv"][:4] == [ + "/usr/bin/systemctl", + "show", + "--property=NRestarts", + "--value", + ] + assert task["changed_when"] is False + + +def _settled(before: str, after: str) -> bool: + """Whether the no-restart assertion holds for these ``NRestarts`` readings.""" + assertion = _task(NO_RESTART, _section("block"))["ansible.builtin.assert"] + assert isinstance(assertion, dict) + variables: dict[str, object] = { + "cloudfall_deploy_unit_restarts_before": {"stdout": before}, + "cloudfall_deploy_unit_restarts_after": {"stdout": after}, + } + return all(_holds(c, variables) for c in assertion["that"]) + + +def test_a_unit_that_did_not_restart_while_settling_passes() -> None: + assert _settled("0", "0") + assert _settled("3", "3") + + +def test_a_crash_looping_unit_fails_even_when_active_at_the_second_check() -> None: + """``Restart=on-failure`` can bring a crashing worker back before the check.""" + assert not _settled("0", "1") + assert not _settled("2", "5") + + +@pytest.mark.parametrize(("before", "after"), [("", ""), ("0", "[not set]")]) +def test_an_unreadable_restart_count_fails_the_gate(before: str, after: str) -> None: + assert not _settled(before, after) + + def test_the_unit_settles_for_the_role_default_before_its_second_check() -> None: defaults = _load(ROLE / "defaults" / "main.yml") assert isinstance(defaults, dict)