From c8c63861ecb7002db3063df9520f36a5f3471958 Mon Sep 17 00:00:00 2001 From: "shipmill-romamo[bot]" <338557544+shipmill-romamo[bot]@users.noreply.github.com> Date: Wed, 28 Oct 2026 13:33:05 +0200 Subject: [PATCH 1/2] Keep a hand-made unit and restore it when a first release fails (#151) The deploy role keeps a same-name unit without the Cloudfall header at .service.pre-cloudfall, records whether it was enabled and active, and on a failed first release copies it back, restores its enabled state and starts it when it was running. Check mode reports the unit it would keep. Co-Authored-By: Claude Opus 5.5 --- changelog.d/151-keep-hand-made-unit.md | 3 + engine/README.md | 11 +- .../roles/cloudfall_deploy/tasks/main.yml | 163 +++++++++- engine/tests/test_deploy_role.py | 292 +++++++++++++++++- 4 files changed, 459 insertions(+), 10 deletions(-) create mode 100644 changelog.d/151-keep-hand-made-unit.md diff --git a/changelog.d/151-keep-hand-made-unit.md b/changelog.d/151-keep-hand-made-unit.md new file mode 100644 index 0000000..c87edd8 --- /dev/null +++ b/changelog.d/151-keep-hand-made-unit.md @@ -0,0 +1,3 @@ +### Fixed + +- A first deploy over a hand-made systemd unit of the same name no longer loses it when the release fails its health gate: the deploy role keeps a unit without the `# Managed by Cloudfall` header at `/etc/systemd/system/.service.pre-cloudfall` before installing the managed unit, and on a failed first release copies it back, restores its enabled state and starts it again when it was active; check mode reports the unit it would keep, and a hand-made unit that differs from an earlier kept copy refuses the deploy (#151) diff --git a/engine/README.md b/engine/README.md index a98c436..2946d50 100644 --- a/engine/README.md +++ b/engine/README.md @@ -234,7 +234,16 @@ systemd unit from the declared service command, and then activates behind a health gate: symlink switch, restart, and the declared HTTP health check. If the health check fails, the previous release is restored and restarted automatically and the run fails; a first release with no predecessor is -stopped instead. Successful runs write a schema-valid release receipt on the +stopped instead. A unit of the same name that Cloudfall did not render (no +`# Managed by Cloudfall` header, such as a hand-made unit adopted on a +brownfield move) is kept at `/etc/systemd/system/.service.pre-cloudfall` +before the managed unit replaces it, together with whether it was enabled and +active; when that first release fails its health check, the kept unit is +copied back, its enabled state restored, and it is started again if it was +running. The kept copy stays after a successful deploy and is never +overwritten; a later hand-made unit that differs from it refuses the deploy. +Check mode only reports the unit it would keep. Successful runs write a +schema-valid release receipt on the controller. The first slice supports Python components managed by uv on x86_64 Debian hosts. diff --git a/engine/ansible/roles/cloudfall_deploy/tasks/main.yml b/engine/ansible/roles/cloudfall_deploy/tasks/main.yml index a12a33f..959dacb 100644 --- a/engine/ansible/roles/cloudfall_deploy/tasks/main.yml +++ b/engine/ansible/roles/cloudfall_deploy/tasks/main.yml @@ -28,6 +28,12 @@ /tmp/cloudfall-{{ cloudfall_deploy_component.id }}-{{ cloudfall_deploy_release }}.tar.gz + cloudfall_deploy_unit_path: >- + /etc/systemd/system/{{ cloudfall_deploy_component.service.name }}.service + cloudfall_deploy_unit_kept_path: >- + /etc/systemd/system/{{ + cloudfall_deploy_component.service.name + }}.service.pre-cloudfall - name: Check the installed uv version ansible.builtin.command: @@ -187,11 +193,103 @@ else "" }} +- name: Inspect the existing service unit + ansible.builtin.stat: + path: "{{ cloudfall_deploy_unit_path }}" + follow: false + checksum_algorithm: sha256 + register: cloudfall_deploy_unit_existing + +- name: Read the existing service unit + ansible.builtin.slurp: + src: "{{ cloudfall_deploy_unit_path }}" + register: cloudfall_deploy_unit_existing_content + when: cloudfall_deploy_unit_existing.stat.isreg | default(false) + +- name: Recognize a hand-made service unit + ansible.builtin.set_fact: + cloudfall_deploy_unit_hand_made: >- + {{ + (cloudfall_deploy_unit_existing.stat.isreg | default(false)) + and not ( + cloudfall_deploy_unit_existing_content.content | b64decode + ).startswith('# Managed by Cloudfall') + }} + +- name: Inspect the kept pre-Cloudfall service unit + ansible.builtin.stat: + path: "{{ cloudfall_deploy_unit_kept_path }}" + follow: false + checksum_algorithm: sha256 + register: cloudfall_deploy_unit_kept_before + when: cloudfall_deploy_unit_hand_made | bool + +- name: Refuse to replace a hand-made unit that differs from the one already kept + ansible.builtin.assert: + that: + - not cloudfall_deploy_unit_kept_before.stat.exists + or cloudfall_deploy_unit_kept_before.stat.checksum + == cloudfall_deploy_unit_existing.stat.checksum + fail_msg: >- + {{ cloudfall_deploy_unit_path }} is not managed by Cloudfall and differs + from the unit already kept at {{ cloudfall_deploy_unit_kept_path }}; + move one of them away so Cloudfall keeps exactly the unit it replaces + when: cloudfall_deploy_unit_hand_made | bool + +- name: Record whether the hand-made service unit is enabled + ansible.builtin.command: + argv: + - /usr/bin/systemctl + - is-enabled + - "{{ cloudfall_deploy_component.service.name }}.service" + register: cloudfall_deploy_unit_was_enabled + changed_when: false + failed_when: false + when: + - not ansible_check_mode + - cloudfall_deploy_unit_hand_made | bool + +- name: Record whether the hand-made service unit is active + ansible.builtin.command: + argv: + - /usr/bin/systemctl + - is-active + - --quiet + - "{{ cloudfall_deploy_component.service.name }}.service" + register: cloudfall_deploy_unit_was_active + changed_when: false + failed_when: false + when: + - not ansible_check_mode + - cloudfall_deploy_unit_hand_made | bool + +- name: Report the hand-made service unit a deploy would replace + ansible.builtin.debug: + msg: >- + {{ cloudfall_deploy_unit_path }} is not managed by Cloudfall; a deploy + keeps it at {{ cloudfall_deploy_unit_kept_path }} and replaces it with + the managed unit + when: + - ansible_check_mode + - cloudfall_deploy_unit_hand_made | bool + +- name: Keep the hand-made service unit before replacing it + ansible.builtin.copy: + src: "{{ cloudfall_deploy_unit_path }}" + dest: "{{ cloudfall_deploy_unit_kept_path }}" + remote_src: true + force: false + owner: root + group: root + mode: "0644" + when: + - not ansible_check_mode + - cloudfall_deploy_unit_hand_made | bool + - name: Install the managed service unit ansible.builtin.template: src: component.service.j2 - dest: >- - /etc/systemd/system/{{ cloudfall_deploy_component.service.name }}.service + dest: "{{ cloudfall_deploy_unit_path }}" owner: root group: root mode: "0644" @@ -333,6 +431,55 @@ state: stopped when: cloudfall_deploy_previous_directory | length == 0 + - name: Restore the pre-Cloudfall service unit + ansible.builtin.copy: + src: "{{ cloudfall_deploy_unit_kept_path }}" + dest: "{{ cloudfall_deploy_unit_path }}" + remote_src: true + force: true + owner: root + group: root + mode: "0644" + when: + - cloudfall_deploy_previous_directory | length == 0 + - not ansible_check_mode + - cloudfall_deploy_unit_hand_made | bool + + - name: Reload systemd for the restored pre-Cloudfall unit + ansible.builtin.systemd_service: + daemon_reload: true + when: + - cloudfall_deploy_previous_directory | length == 0 + - not ansible_check_mode + - cloudfall_deploy_unit_hand_made | bool + + - name: Restore the enabled state of the pre-Cloudfall unit + ansible.builtin.command: + argv: + - /usr/bin/systemctl + - >- + {{ + 'reenable' + if cloudfall_deploy_unit_was_enabled.stdout == 'enabled' + else 'disable' + }} + - "{{ cloudfall_deploy_component.service.name }}.service" + changed_when: true + when: + - cloudfall_deploy_previous_directory | length == 0 + - not ansible_check_mode + - cloudfall_deploy_unit_hand_made | bool + + - name: Start the pre-Cloudfall unit that was active before the deploy + ansible.builtin.systemd_service: + name: "{{ cloudfall_deploy_component.service.name }}.service" + state: started + when: + - cloudfall_deploy_previous_directory | length == 0 + - not ansible_check_mode + - cloudfall_deploy_unit_hand_made | bool + - cloudfall_deploy_unit_was_active.rc == 0 + - name: Fail the deployment after rolling back ansible.builtin.fail: msg: >- @@ -342,6 +489,18 @@ ~ (cloudfall_deploy_previous_directory | basename) if cloudfall_deploy_previous_directory | length > 0 else 'stopped because no previous release exists' + }}{{ + '; the pre-Cloudfall unit was restored from ' + ~ cloudfall_deploy_unit_kept_path + ~ ( + ' and started' + if cloudfall_deploy_unit_was_active.rc == 0 + else ' and left stopped as it was before the deploy' + ) + if cloudfall_deploy_previous_directory | length == 0 + and not ansible_check_mode + and cloudfall_deploy_unit_hand_made | bool + else '' }} - name: Ensure the controller receipt directory exists diff --git a/engine/tests/test_deploy_role.py b/engine/tests/test_deploy_role.py index 1889e54..6a4b28e 100644 --- a/engine/tests/test_deploy_role.py +++ b/engine/tests/test_deploy_role.py @@ -2,11 +2,13 @@ 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. +type, which rescue steps run when a gate task fails, and how a hand-made +unit of the same name is kept and restored. """ from __future__ import annotations +import base64 from pathlib import Path import pytest @@ -35,6 +37,30 @@ RESTART_RESTORED = "Restart the restored release" STOP_FIRST = "Stop the failed first release" FAIL = "Fail the deployment after rolling back" +RESTORE_UNIT = "Restore the pre-Cloudfall service unit" +RELOAD_UNIT = "Reload systemd for the restored pre-Cloudfall unit" +ENABLE_UNIT = "Restore the enabled state of the pre-Cloudfall unit" +START_UNIT = "Start the pre-Cloudfall unit that was active before the deploy" +RECOGNIZE = "Recognize a hand-made service unit" +INSPECT_KEPT = "Inspect the kept pre-Cloudfall service unit" +REFUSE_STALE = ( + "Refuse to replace a hand-made unit that differs from the one already kept" +) +WAS_ENABLED = "Record whether the hand-made service unit is enabled" +WAS_ACTIVE = "Record whether the hand-made service unit is active" +REPORT_KEEP = "Report the hand-made service unit a deploy would replace" +KEEP = "Keep the hand-made service unit before replacing it" +ADOPTION = [INSPECT_KEPT, REFUSE_STALE, WAS_ENABLED, WAS_ACTIVE, REPORT_KEEP, KEEP] +INSTALL = "Install the managed service unit" +HAND_MADE_UNIT = """[Unit] +Description=asrest + +[Service] +ExecStart=/opt/asrest/bin/asrest + +[Install] +WantedBy=multi-user.target +""" HTTP_CHECK: dict[str, object] = { "type": "http", "scheme": "http", @@ -51,10 +77,14 @@ def _load(path: Path) -> object: return DataLoader().load_from_file(str(path)) -def _activation() -> dict[str, object]: +def _main() -> list[dict[str, object]]: tasks = _load(ROLE / "tasks" / "main.yml") assert isinstance(tasks, list) - return next(t for t in tasks if t["name"] == ACTIVATE) + return tasks + + +def _activation() -> dict[str, object]: + return _task(ACTIVATE, _main()) def _section(key: str) -> list[dict[str, object]]: @@ -99,11 +129,43 @@ def _gate(health_check: dict[str, object], *, check_mode: bool) -> list[str]: ] -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 +def _rescue_variables( + previous_directory: str, + *, + hand_made: bool = False, + was_active: bool = True, + was_enabled: str = "enabled", + check_mode: bool = False, +) -> dict[str, object]: + return { + "ansible_check_mode": check_mode, + "cloudfall_deploy_previous_directory": previous_directory, + "cloudfall_deploy_unit_hand_made": hand_made, + "cloudfall_deploy_unit_was_active": {"rc": 0 if was_active else 3}, + "cloudfall_deploy_unit_was_enabled": {"stdout": was_enabled}, + "cloudfall_deploy_unit_path": "/etc/systemd/system/asrest.service", + "cloudfall_deploy_unit_kept_path": ( + "/etc/systemd/system/asrest.service.pre-cloudfall" + ), + "cloudfall_deploy_release": "r2", + "cloudfall_deploy_component": {"service": {"name": "asrest"}}, } + + +def _rescue( + previous_directory: str, + *, + hand_made: bool = False, + was_active: bool = True, + check_mode: bool = False, +) -> set[str]: + """Name every rescue step that runs once a gate task has failed.""" + variables = _rescue_variables( + previous_directory, + hand_made=hand_made, + was_active=was_active, + check_mode=check_mode, + ) return { str(task["name"]) for task in _section("rescue") @@ -201,3 +263,219 @@ def test_a_release_whose_unit_exits_rolls_back() -> None: RESTART_RESTORED, FAIL, } + + +def _hand_made(content: str | None) -> object: + """Decide, as the role does, whether the unit file found is hand-made.""" + expression = _task(RECOGNIZE, _main())["ansible.builtin.set_fact"] + assert isinstance(expression, dict) + variables: dict[str, object] = ( + { + "cloudfall_deploy_unit_existing": {"stat": {"exists": False}}, + "cloudfall_deploy_unit_existing_content": {"skipped": True}, + } + if content is None + else { + "cloudfall_deploy_unit_existing": {"stat": {"exists": True, "isreg": True}}, + "cloudfall_deploy_unit_existing_content": { + "content": base64.b64encode(content.encode()).decode() + }, + } + ) + return _templar(variables).template( + trust_as_template(expression["cloudfall_deploy_unit_hand_made"]) + ) + + +def _adoption(*, hand_made: bool, check_mode: bool) -> list[str]: + """Name, in order, every adoption task whose ``when`` holds.""" + variables: dict[str, object] = { + "ansible_check_mode": check_mode, + "cloudfall_deploy_unit_hand_made": hand_made, + } + return [ + str(task["name"]) + for task in _main() + if task["name"] in ADOPTION + and all(_holds(c, variables) for c in _conditions(task)) + ] + + +def test_a_unit_without_the_cloudfall_header_is_hand_made() -> None: + assert _hand_made(HAND_MADE_UNIT) is True + + +def test_a_unit_rendered_by_cloudfall_is_not_hand_made() -> None: + template = (ROLE / "templates" / "component.service.j2").read_text() + + assert _hand_made(template.splitlines()[0] + "\n[Unit]\n") is False + + +def test_a_missing_unit_is_not_hand_made() -> None: + assert _hand_made(None) is False + + +def test_a_hand_made_unit_is_kept_with_its_state_before_the_install() -> None: + names = [str(task["name"]) for task in _main()] + + assert _adoption(hand_made=True, check_mode=False) == [ + INSPECT_KEPT, + REFUSE_STALE, + WAS_ENABLED, + WAS_ACTIVE, + KEEP, + ] + assert names.index(KEEP) < names.index(INSTALL) + + +def test_a_cloudfall_unit_takes_no_copy() -> None: + assert _adoption(hand_made=False, check_mode=False) == [] + assert _adoption(hand_made=False, check_mode=True) == [] + + +def test_check_mode_reports_the_hand_made_unit_without_copying_it() -> None: + assert _adoption(hand_made=True, check_mode=True) == [ + INSPECT_KEPT, + REFUSE_STALE, + REPORT_KEEP, + ] + + +def test_the_kept_copy_is_root_owned_beside_the_unit_and_never_overwritten() -> None: + copy = _task(KEEP, _main())["ansible.builtin.copy"] + assert isinstance(copy, dict) + paths = _task("Compute release paths", _main())["ansible.builtin.set_fact"] + assert isinstance(paths, dict) + variables: dict[str, object] = { + "cloudfall_deploy_component": {"service": {"name": "asrest"}} + } + + kept = _templar(variables).template( + trust_as_template(paths["cloudfall_deploy_unit_kept_path"]) + ) + + assert kept == "/etc/systemd/system/asrest.service.pre-cloudfall" + assert copy["remote_src"] is True + assert copy["force"] is False + assert (copy["owner"], copy["group"], copy["mode"]) == ("root", "root", "0644") + + +@pytest.mark.parametrize( + ("stale", "outcome"), + [(None, "accepted"), ("a" * 64, "accepted"), ("b" * 64, "refused")], +) +def test_a_kept_copy_of_another_unit_refuses_the_deploy( + stale: str | None, outcome: str +) -> None: + assertion = _task(REFUSE_STALE, _main())["ansible.builtin.assert"] + assert isinstance(assertion, dict) + variables: dict[str, object] = { + "cloudfall_deploy_unit_existing": {"stat": {"checksum": "a" * 64}}, + "cloudfall_deploy_unit_kept_before": { + "stat": {"exists": False} + if stale is None + else {"exists": True, "checksum": stale} + }, + } + + holds = all(_holds(c, variables) for c in assertion["that"]) + + assert holds is (outcome == "accepted") + + +def test_a_failed_first_release_over_a_hand_made_unit_restores_and_starts_it() -> None: + assert _rescue("", hand_made=True) == { + STOP_FIRST, + RESTORE_UNIT, + RELOAD_UNIT, + ENABLE_UNIT, + START_UNIT, + FAIL, + } + names = [str(task["name"]) for task in _section("rescue")] + assert names.index(STOP_FIRST) < names.index(RESTORE_UNIT) + assert names.index(RESTORE_UNIT) < names.index(RELOAD_UNIT) + assert names.index(RELOAD_UNIT) < names.index(START_UNIT) + + +def test_a_restored_unit_that_was_stopped_stays_stopped() -> None: + assert _rescue("", hand_made=True, was_active=False) == { + STOP_FIRST, + RESTORE_UNIT, + RELOAD_UNIT, + ENABLE_UNIT, + FAIL, + } + + +def test_the_restore_copies_the_kept_unit_back_over_the_managed_one() -> None: + copy = _task(RESTORE_UNIT, _section("rescue"))["ansible.builtin.copy"] + assert isinstance(copy, dict) + + assert (copy["src"], copy["dest"]) == ( + "{{ cloudfall_deploy_unit_kept_path }}", + "{{ cloudfall_deploy_unit_path }}", + ) + assert copy["remote_src"] is True + assert copy["force"] is True + + +@pytest.mark.parametrize( + ("was_enabled", "verb"), + [("enabled", "reenable"), ("disabled", "disable"), ("static", "disable")], +) +def test_the_restored_unit_gets_back_its_enabled_state( + was_enabled: str, verb: str +) -> None: + command = _task(ENABLE_UNIT, _section("rescue"))["ansible.builtin.command"] + assert isinstance(command, dict) + variables = _rescue_variables("", hand_made=True, was_enabled=was_enabled) + + argv = [ + _templar(variables).template(trust_as_template(part)) + for part in command["argv"] + ] + + assert argv == ["/usr/bin/systemctl", verb, "asrest.service"] + + +def test_a_rollback_to_a_previous_release_leaves_a_kept_unit_alone() -> None: + assert _rescue("/srv/apps/asrest/releases/r1", hand_made=True) == { + RESTORE, + RESTART_RESTORED, + FAIL, + } + + +def test_a_first_release_over_a_cloudfall_unit_does_not_resurrect_a_kept_copy() -> None: + """``current`` removed by hand leaves no previous release, not a hand-made unit.""" + assert _rescue("", hand_made=False) == {STOP_FIRST, FAIL} + + +def test_check_mode_never_restores_a_unit() -> None: + assert _rescue("", hand_made=True, check_mode=True) == {STOP_FIRST, FAIL} + + +def _failure(*, hand_made: bool, was_active: bool = True) -> str: + fail = _task(FAIL, _section("rescue"))["ansible.builtin.fail"] + assert isinstance(fail, dict) + variables = _rescue_variables("", hand_made=hand_made, was_active=was_active) + return str(_templar(variables).template(trust_as_template(fail["msg"]))) + + +def test_the_failure_says_the_hand_made_unit_was_restored() -> None: + assert _failure(hand_made=True) == ( + "Release r2 failed its health check and was stopped because no previous" + " release exists; the pre-Cloudfall unit was restored from" + " /etc/systemd/system/asrest.service.pre-cloudfall and started" + ) + assert _failure(hand_made=True, was_active=False).endswith( + " and left stopped as it was before the deploy" + ) + + +def test_the_failure_of_a_first_release_without_a_kept_unit_is_unchanged() -> None: + assert _failure(hand_made=False) == ( + "Release r2 failed its health check and was stopped because no previous" + " release exists" + ) From 38342fe86bd99f771b13ed121fd5d8d0eb95e73c Mon Sep 17 00:00:00 2001 From: "shipmill-romamo[bot]" <338557544+shipmill-romamo[bot]@users.noreply.github.com> Date: Wed, 28 Oct 2026 13:44:50 +0200 Subject: [PATCH 2/2] Compute the service unit paths in their own task (#151) The S-002-12 harness runs "Compute release paths" with a component that declares no service, so templating the unit paths there failed the play. Co-Authored-By: Claude Opus 5.5 --- .../ansible/roles/cloudfall_deploy/tasks/main.yml | 15 +++++++++------ engine/tests/test_deploy_role.py | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/engine/ansible/roles/cloudfall_deploy/tasks/main.yml b/engine/ansible/roles/cloudfall_deploy/tasks/main.yml index 959dacb..c4e5381 100644 --- a/engine/ansible/roles/cloudfall_deploy/tasks/main.yml +++ b/engine/ansible/roles/cloudfall_deploy/tasks/main.yml @@ -28,12 +28,6 @@ /tmp/cloudfall-{{ cloudfall_deploy_component.id }}-{{ cloudfall_deploy_release }}.tar.gz - cloudfall_deploy_unit_path: >- - /etc/systemd/system/{{ cloudfall_deploy_component.service.name }}.service - cloudfall_deploy_unit_kept_path: >- - /etc/systemd/system/{{ - cloudfall_deploy_component.service.name - }}.service.pre-cloudfall - name: Check the installed uv version ansible.builtin.command: @@ -193,6 +187,15 @@ else "" }} +- name: Compute service unit paths + ansible.builtin.set_fact: + cloudfall_deploy_unit_path: >- + /etc/systemd/system/{{ cloudfall_deploy_component.service.name }}.service + cloudfall_deploy_unit_kept_path: >- + /etc/systemd/system/{{ + cloudfall_deploy_component.service.name + }}.service.pre-cloudfall + - name: Inspect the existing service unit ansible.builtin.stat: path: "{{ cloudfall_deploy_unit_path }}" diff --git a/engine/tests/test_deploy_role.py b/engine/tests/test_deploy_role.py index 6a4b28e..fda5b8a 100644 --- a/engine/tests/test_deploy_role.py +++ b/engine/tests/test_deploy_role.py @@ -344,7 +344,7 @@ def test_check_mode_reports_the_hand_made_unit_without_copying_it() -> None: def test_the_kept_copy_is_root_owned_beside_the_unit_and_never_overwritten() -> None: copy = _task(KEEP, _main())["ansible.builtin.copy"] assert isinstance(copy, dict) - paths = _task("Compute release paths", _main())["ansible.builtin.set_fact"] + paths = _task("Compute service unit paths", _main())["ansible.builtin.set_fact"] assert isinstance(paths, dict) variables: dict[str, object] = { "cloudfall_deploy_component": {"service": {"name": "asrest"}}