diff --git a/changelog.d/96-healthcheck-none.md b/changelog.d/96-healthcheck-none.md new file mode 100644 index 0000000..2a208b6 --- /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, 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/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..5469efb 100644 --- a/engine/ansible/roles/cloudfall_deploy/tasks/main.yml +++ b/engine/ansible/roles/cloudfall_deploy/tasks/main.yml @@ -203,7 +203,86 @@ 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: 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 }}" + 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" + + - 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 new file mode 100644 index 0000000..1889e54 --- /dev/null +++ b/engine/tests/test_deploy_role.py @@ -0,0 +1,203 @@ +"""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" +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" +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" +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 + + +@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) + 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, + }