fix: the time baseline accepts any NTP daemon, not only timesyncd

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

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

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"}})