fix(ntp): assert the static service address is really gone before handing over
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful

Second review round on #469 found two ways the migration could silently
leave the address outside VRRP control -- both of which would defeat the
health gating while the play reported success.

1. The one-shot migration had no verification.

   Gating the withdrawal on `keepalived_ntp_static6_removed.changed` is what
   makes it idempotent, but it also means it gets exactly one attempt and
   never retries. Combined with `failed_when: false` on the delete, a
   mismatch would pass silently: `ip addr del` matches on address AND prefix
   length, so a host whose static assignment used a different mask or
   interface than ntp_vip6 / keepalived_ntp_interface would see the delete
   no-op, keep the address permanently outside VRRP control, and still
   report success.

   The idempotency fix from the previous commit created this: making the
   task run once removed the retry that would otherwise have masked it.

   Now asserted explicitly after the delete, with a message naming the
   likely cause and the commands to inspect it.

2. edge-1 -- the host the incident actually happened on -- was not covered.

   The removal tasks are gated to Debian, so they never touch edge-1 (Arch).
   Its static [Address] block belongs to the edge-host role, and
   install-edge-host.yml is a separate play with no ordering relationship to
   this one. Nothing guaranteed it had been re-applied.

   edge_host already sets ntp_keepalived_enabled: true, so the next run of
   install-ntp-ha.yml gives edge-1 the NTP_VIP6 instance immediately. With
   the [Address] block still present it would hold fd40::123 statically AND
   run VRRP for it: keepalived would believe it had withdrawn the address
   while networkd kept it assigned, and the host would go on answering NTP
   with a clock check_ntp had already rejected. That is the failure of
   2026-08-04, on the same host.

   This is not hypothetical -- edge-1 still has the block right now:
     grep -rl fd40::123/128 /etc/systemd/network/
     -> /etc/systemd/network/10-enp1s0.40.network

   The role now detects this and fails fast, naming the play to run first,
   rather than proceeding into a split-ownership state.

Also documents why the post-deploy presence check is meaningful: presence
alone cannot distinguish a VRRP-held address from a static leftover, and
only means what it claims because the role asserts no static assignment
survives on either distro.

Not taken: narrowing serial: 1 so routine runs are not serialised through
the chrony settle pause. Real cost, but correctness of the address handover
comes first at four hosts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-05 22:02:26 +02:00
co-authored by Claude Opus 5
parent fbf5c4dcb8
commit 40ef53fa67
2 changed files with 100 additions and 0 deletions
+5
View File
@@ -110,6 +110,11 @@
#
# `ip -6 addr show ... to <addr>` prints the address when present and
# nothing when absent, so emptiness is the test.
#
# Presence cannot itself distinguish a VRRP-held address from a static
# leftover -- they look identical. It is only meaningful because the
# keepalived-ntp role asserts, on both distros, that no static assignment
# survives: the play fails before reaching here if one does.
- name: Check if this server holds the IPv6 service address
ansible.builtin.command:
argv:
@@ -117,6 +117,101 @@
failed_when: false
notify: restart keepalived
# The migration above gets exactly one attempt -- the gate that makes it
# idempotent also means it never retries -- and the delete swallows its own
# errors. So the result has to be asserted rather than assumed.
#
# `ip addr del` matches on address AND prefix length: if a host's static
# assignment used a different mask or interface than ntp_vip6 /
# keepalived_ntp_interface, the delete is a silent no-op. The address would
# then stay outside VRRP control permanently, with the play reporting success
# and the post-deploy check happily calling it a healthy v6 master -- the
# health gating silently defeated in exactly the way this change exists to
# prevent. Fail loudly here instead.
- name: Confirm the static service address was actually withdrawn
ansible.builtin.command:
argv:
- ip
- -6
- addr
- show
- dev
- "{{ keepalived_ntp_interface }}"
- to
- "{{ keepalived_ntp_vip6 }}"
register: keepalived_ntp_vip6_after
changed_when: false
failed_when: false
when:
- keepalived_ntp_vip6 | length > 0
- keepalived_ntp_static6_removed.changed | default(false)
- name: Assert the service address is no longer assigned outside keepalived
ansible.builtin.assert:
that:
- keepalived_ntp_vip6_after.stdout | default('') | length == 0
fail_msg: >-
{{ keepalived_ntp_vip6 }} is still assigned on
{{ keepalived_ntp_interface }} after the static assignment was removed.
`ip -6 addr del` matches address AND prefix length, so this usually means
the on-host assignment differs from ntp_vip6 or uses another interface.
Leaving it would keep the address outside VRRP control, so it would not
be withdrawn when this host is unfit to serve time. Inspect with
`ip -6 addr show dev {{ keepalived_ntp_interface }}` and
`grep -rn {{ keepalived_ntp_vip6 }} /etc/network/ /etc/systemd/network/`.
success_msg: >-
{{ keepalived_ntp_vip6 }} withdrawn; keepalived now owns it.
when:
- keepalived_ntp_vip6 | length > 0
- keepalived_ntp_static6_removed.changed | default(false)
# On Arch (edge-1) the static assignment lives in a systemd-networkd file owned
# by the edge-host role, so this role must not rewrite it -- but it must not
# quietly proceed either. install-edge-host.yml is a separate play with no
# ordering relationship to this one, so nothing guarantees it has been re-run
# to drop the [Address] block. If it has not, edge-1 would end up holding
# fd40::123 statically AND running a VRRP instance for the same address:
# keepalived would believe it had withdrawn the address while networkd kept it
# assigned, and the host would go on answering NTP with a clock the health
# check had already rejected.
#
# That is the precise failure of 2026-08-04, on the host it happened to, so it
# fails fast rather than being left to inspection.
- name: Detect a static assignment of the service address (systemd-networkd)
ansible.builtin.command:
argv:
- grep
- -rl
- "{{ keepalived_ntp_vip6 }}"
- /etc/systemd/network/
register: keepalived_ntp_static6_networkd
changed_when: false
failed_when: false
when:
- keepalived_ntp_vip6 | length > 0
- ansible_facts['os_family'] == 'Archlinux'
- name: Assert networkd is not also assigning the service address
ansible.builtin.assert:
that:
- keepalived_ntp_static6_networkd.stdout | default('') | length == 0
fail_msg: >-
{{ keepalived_ntp_vip6 }} is still assigned statically by
systemd-networkd on this host
({{ keepalived_ntp_static6_networkd.stdout_lines | default([]) | join(', ') }}).
Handing the address to keepalived while networkd also assigns it means
it would NOT be withdrawn when check_ntp fails -- the host would keep
serving time on an address VRRP believes it has released.
Re-apply the edge-host role first, whose template no longer carries the
[Address] block: ansible-playbook -i inventory/ plays/install-edge-host.yml
--limit {{ inventory_hostname }} --tags vlan40
success_msg: >-
No static networkd assignment of {{ keepalived_ntp_vip6 }}; safe for
keepalived to own it.
when:
- keepalived_ntp_vip6 | length > 0
- ansible_facts['os_family'] == 'Archlinux'
- name: Create keepalived configuration
ansible.builtin.template:
src: keepalived.conf.j2