From e745a1baccc17ca71debf81e3b44f3aebe58b2e9 Mon Sep 17 00:00:00 2001 From: Romamo Date: Wed, 30 Sep 2026 13:04:00 +0300 Subject: [PATCH] fix: the time baseline accepts any NTP daemon, not only timesyncd `timedatectl set-ntp` switches systemd-timesyncd and nothing else. On a host whose clock ntp, ntpsec, or chrony keeps, timedatectl reports CanNTP=no and NTP=no while NTPSynchronized=yes, so the role called the host drifted: check mode reported "Would enable network time synchronization" and a deploy failed with "Failed to set ntp: NTP not supported". Four of the six atlas-fleet servers (Debian 11 and 12 on ntp/ntpsec) hit it. The role now reads CanNTP, switches NTP on only where systemd-timesyncd is installed, and otherwise requires NTPSynchronized=yes. Check mode warns when nothing keeps time; a deploy that waits 60 seconds without synchronizing fails with a message naming the daemons to check, rather than the retry loop's bare command result. Tests replay the role's when and assert expressions through Ansible's own templating for four host states; the other-daemon case fails on the old role. A check-mode run of this role against all six fleet servers reports changed=0. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 8 ++ .../roles/cloudfall_time/tasks/main.yml | 34 ++++- engine/tests/test_time_role.py | 119 ++++++++++++++++++ 3 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 engine/tests/test_time_role.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 506be80..9d034a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -111,6 +111,14 @@ Notable changes to Cloudfall. The format follows ### Fixed +- The time baseline accepts a host whose clock another daemon keeps + (ntp, ntpsec, chrony). `timedatectl set-ntp` drives systemd-timesyncd + only, so on such hosts check mode reported "Would enable network time + synchronization" and a deploy failed with "NTP not supported". The role + now switches NTP on only where systemd-timesyncd is installed, and + otherwise requires the clock to be synchronized; check mode warns when + nothing keeps time, and a deploy fails with a message naming the daemon + to check when the clock does not synchronize within 60 seconds - `cloudfall add` into a read-only or full project failed with a Python traceback and could leave a new server type behind when only the server file failed. It now removes what it wrote and fails with diff --git a/engine/ansible/roles/cloudfall_time/tasks/main.yml b/engine/ansible/roles/cloudfall_time/tasks/main.yml index 38f1f6c..d6346f6 100644 --- a/engine/ansible/roles/cloudfall_time/tasks/main.yml +++ b/engine/ansible/roles/cloudfall_time/tasks/main.yml @@ -16,10 +16,15 @@ - --property=LocalRTC - --property=NTP - --property=NTPSynchronized + - --property=CanNTP changed_when: false check_mode: false register: cloudfall_time_before +# timedatectl's NTP switch drives systemd-timesyncd only (CanNTP=yes). A host +# without it keeps time with another daemon such as ntp, ntpsec, or chrony, +# which Cloudfall leaves in place and judges by NTPSynchronized alone. + - name: Report timezone drift in check mode ansible.builtin.debug: msg: >- @@ -70,8 +75,20 @@ changed_when: true when: - ansible_check_mode + - "'CanNTP=yes' in cloudfall_time_before.stdout_lines" - "'NTP=yes' not in cloudfall_time_before.stdout_lines" +- name: Report an unsynchronized clock without systemd-timesyncd in check mode + ansible.builtin.debug: + msg: >- + The clock is not synchronized and systemd-timesyncd is not installed; + a deploy waits for the host's own NTP daemon and fails if it does not + synchronize + when: + - ansible_check_mode + - "'CanNTP=yes' not in cloudfall_time_before.stdout_lines" + - "'NTPSynchronized=yes' not in cloudfall_time_before.stdout_lines" + - name: Enable network time synchronization ansible.builtin.command: argv: @@ -81,6 +98,7 @@ changed_when: true when: - not ansible_check_mode + - "'CanNTP=yes' in cloudfall_time_before.stdout_lines" - "'NTP=yes' not in cloudfall_time_before.stdout_lines" - name: Read applied time configuration @@ -91,6 +109,7 @@ - --property=Timezone - --property=LocalRTC - --property=NTP + - --property=CanNTP changed_when: false check_mode: false register: cloudfall_time_after @@ -100,7 +119,10 @@ that: - ansible_check_mode or 'Timezone=' ~ cloudfall_time_timezone in cloudfall_time_after.stdout_lines - ansible_check_mode or 'LocalRTC=no' in cloudfall_time_after.stdout_lines - - ansible_check_mode or 'NTP=yes' in cloudfall_time_after.stdout_lines + - >- + ansible_check_mode + or 'CanNTP=yes' not in cloudfall_time_after.stdout_lines + or 'NTP=yes' in cloudfall_time_after.stdout_lines fail_msg: The host did not converge to the Cloudfall UTC time baseline - name: Wait for synchronized network time @@ -116,4 +138,14 @@ until: cloudfall_time_synchronized.stdout == "yes" retries: 12 delay: 5 + failed_when: false + when: not ansible_check_mode + +- name: Require synchronized network time + ansible.builtin.assert: + that: + - cloudfall_time_synchronized.stdout == "yes" + fail_msg: >- + The clock did not synchronize within 60 seconds; check the host's NTP + daemon (systemd-timesyncd, ntp, ntpsec, or chrony) when: not ansible_check_mode diff --git a/engine/tests/test_time_role.py b/engine/tests/test_time_role.py new file mode 100644 index 0000000..f46c971 --- /dev/null +++ b/engine/tests/test_time_role.py @@ -0,0 +1,119 @@ +"""The time role's decisions, evaluated with Ansible's own templating. + +The role only runs on Debian with systemd, so these tests replay what it +decides from ``timedatectl show`` output rather than running it: each +task's ``when`` and each assertion's ``that`` go through Ansible's +evaluator against the host states Cloudfall fleets report. +""" + +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] +TASKS = ROOT / "engine" / "ansible" / "roles" / "cloudfall_time" / "tasks" / "main.yml" +BASELINE = ["Timezone=Etc/UTC", "LocalRTC=no"] +ENABLE = "Enable network time synchronization" +REPORT_DRIFT = "Report NTP drift in check mode" +REPORT_UNSYNCHRONIZED = ( + "Report an unsynchronized clock without systemd-timesyncd in check mode" +) + +# systemd-timesyncd installed and running +TIMESYNCD_ON = [*BASELINE, "NTP=yes", "NTPSynchronized=yes", "CanNTP=yes"] +# systemd-timesyncd installed but switched off +TIMESYNCD_OFF = [*BASELINE, "NTP=no", "NTPSynchronized=yes", "CanNTP=yes"] +# ntp, ntpsec, or chrony keeps time; timedatectl cannot switch it +OTHER_DAEMON = [*BASELINE, "NTP=no", "NTPSynchronized=yes", "CanNTP=no"] +# nothing keeps time +NO_DAEMON = [*BASELINE, "NTP=no", "NTPSynchronized=no", "CanNTP=no"] + + +def _tasks() -> list[dict[str, object]]: + tasks = DataLoader().load_from_file(str(TASKS)) + assert isinstance(tasks, list) + return tasks + + +def _holds(expression: object, variables: dict[str, object]) -> bool: + templar = Templar(loader=DataLoader(), variables=variables) + return bool(templar.evaluate_conditional(trust_as_template(str(expression)))) + + +def _conditions(task: dict[str, object], key: str) -> list[object]: + value = task.get(key, []) + return value if isinstance(value, list) else [value] + + +def _variables( + before: list[str], *, check_mode: bool, after: list[str] | None = None +) -> dict[str, object]: + return { + "ansible_check_mode": check_mode, + "cloudfall_time_timezone": "Etc/UTC", + "cloudfall_time_before": {"stdout_lines": before}, + "cloudfall_time_after": {"stdout_lines": after or before}, + } + + +def _runs(before: list[str], *, check_mode: bool) -> set[str]: + """Name every task whose ``when`` holds for this host and mode.""" + variables = _variables(before, check_mode=check_mode) + return { + str(task["name"]) + for task in _tasks() + if all(_holds(c, variables) for c in _conditions(task, "when")) + } + + +def _baseline_assertion_holds(after: list[str]) -> bool: + task = next( + t for t in _tasks() if t["name"] == "Require applied UTC time configuration" + ) + variables = _variables(after, check_mode=False, after=after) + assert isinstance(task["ansible.builtin.assert"], dict) + return all(_holds(c, variables) for c in task["ansible.builtin.assert"]["that"]) + + +def test_check_mode_reports_timesyncd_switched_off() -> None: + runs = _runs(TIMESYNCD_OFF, check_mode=True) + + assert REPORT_DRIFT in runs + assert ENABLE not in runs + + +def test_a_deploy_switches_timesyncd_on_and_requires_it() -> None: + assert ENABLE in _runs(TIMESYNCD_OFF, check_mode=False) + assert _baseline_assertion_holds(TIMESYNCD_ON) + assert not _baseline_assertion_holds(TIMESYNCD_OFF) + + +@pytest.mark.parametrize("check_mode", [True, False]) +def test_another_synchronized_daemon_is_left_alone(*, check_mode: bool) -> None: + """A host on ntp, ntpsec, or chrony has no timesyncd for set-ntp to start.""" + runs = _runs(OTHER_DAEMON, check_mode=check_mode) + + assert REPORT_DRIFT not in runs + assert REPORT_UNSYNCHRONIZED not in runs + assert ENABLE not in runs + assert _baseline_assertion_holds(OTHER_DAEMON) + + +def test_check_mode_warns_when_nothing_keeps_time() -> None: + runs = _runs(NO_DAEMON, check_mode=True) + + assert REPORT_UNSYNCHRONIZED in runs + assert ENABLE not in runs + + +def test_a_deploy_fails_when_the_clock_never_synchronizes() -> None: + task = next(t for t in _tasks() if t["name"] == "Require synchronized network time") + assert isinstance(task["ansible.builtin.assert"], dict) + (condition,) = task["ansible.builtin.assert"]["that"] + + assert not _holds(condition, {"cloudfall_time_synchronized": {"stdout": "no"}}) + assert _holds(condition, {"cloudfall_time_synchronized": {"stdout": "yes"}})