vrrp-wan: a flap holdoff must not tear down a live WAN session

ppp_dial() checked the flap holdoff and returned BEFORE renewing
/run/vrrp-wan/may-dial. That lease is what vrrp-wan-guard expires after
LEASE_TTL, so tripping the damper stopped the renew and the guard hung up
pppoe0 on the MASTER ~80s later. A damper meant to suppress repeated DIALS
was tearing down a working WAN instead.

Observed in labsim, end to end:

  DIAL FLAP: >=6 attempts in 600s -- holding off 900s
  GUARD: lease stale (81s > 75s) -- hanging up pppoe0

An established session now outranks every check below it: ppp_active renews
the lease and returns first. Everything after it only decides whether to
start a NEW session.

Two supporting fixes for how that storm started. The dial attempts were all
no-ops because /etc/ppp/peers/pppoe0 was missing, and nothing said so --
systemd logs "skipped because of an unmet condition check" exactly once and
the gate looks identical to a healthy backup. ppp_dial() now reports it, and
distinguishes "configured but not rendered" (re-commit the subtree) from "no
pppoe0 in config at all", which is what a reboot leaves behind when a commit
was never saved. That is precisely how the sim secondary lost its WAN.

Also `cat | wc -l` rather than `wc -l < file`: redirections are applied left
to right, so the missing-file error escapes the 2>/dev/null on every
first-ever dial.

Harness: T11 copied-then-removed instead of mv, and verifies the restore --
losing that file strands a router permanently, which cost a debugging
session. preflight now refuses to run if either router lacks the peers file
or the pppoe0 config, since every failover result would otherwise be a false
negative blamed on the ISP. New T12 forges a 900s holdoff against a live
session and asserts it survives.

labsim/labsim-pppoe-ha-test.sh

@@ -97,8 +97,21 @@ preflight() {
         fi
         [ "$(r "$h" 'systemctl is-active vrrp-wan-guard.timer')" = active ] \
             || { fail "$h vrrp-wan-guard.timer not active"; rc=1; }
+        # A router with no peers file CANNOT dial, and says so only once in the
+        # journal. Every failover result in the run would then be a false
+        # negative blamed on the ISP. Check both, and check the config that
+        # renders it -- an unsaved commit reverts on reboot and takes pppoe0
+        # with it, which is how the sim secondary silently stopped dialling.
+        if [ "$(r "$h" '[ -f /etc/ppp/peers/pppoe0 ] && echo y')" != y ]; then
+            fail "$h has no /etc/ppp/peers/pppoe0 -- it cannot dial; re-commit the pppoe subtree"
+            rc=1
+        fi
+        if ! r "$h" 'show configuration commands' 2>/dev/null | grep -q 'interfaces pppoe pppoe0 source-interface'; then
+            fail "$h has no pppoe0 in config (unsaved commit lost on reboot?)"
+            rc=1
+        fi
     done
-    [ "$rc" -eq 0 ] && pass "scaffold dropped, gate present, guard running on both"
+    [ "$rc" -eq 0 ] && pass "scaffold dropped, gate present, guard running, both can dial"
     return $rc
 }
 
@@ -201,12 +214,25 @@ T5() {  # 10gig down -> PPPoE carries traffic
 T11() { # a blessed box with no peers file must not restart-loop
     log "T11 missing peers file must not restart-loop"
     local h; h="$(holder)"
-    r "$h" 'sudo mv /etc/ppp/peers/pppoe0 /tmp/peers.bak; sudo systemctl restart ppp@pppoe0'
+    # COPY then remove, never move: only a commit touching the pppoe subtree
+    # re-renders this file, so losing it strands the box permanently -- the gate
+    # blocks every dial, systemd says "skipped because of an unmet condition
+    # check" exactly once, and nothing else complains. An earlier `mv` pair did
+    # exactly that to the sim secondary and cost a debugging session.
+    r "$h" 'sudo cp -a /etc/ppp/peers/pppoe0 /run/peers.bak && sudo rm -f /etc/ppp/peers/pppoe0; sudo systemctl restart ppp@pppoe0'
     sleep 12
     local n; n="$(r "$h" 'systemctl show ppp@pppoe0 -p NRestarts --value')"
     [ "${n:-99}" -le 1 ] && pass "NRestarts=$n (gate refused the start)" \
         || fail "NRestarts=$n -- restart loop is back"
-    r "$h" 'sudo mv /tmp/peers.bak /etc/ppp/peers/pppoe0'
+    # Restore on the SAME host we broke, and prove it landed. Do not trust the
+    # copy back: if it silently failed, every later test in the run would be
+    # measuring a router that physically cannot dial.
+    r "$h" 'sudo cp -a /run/peers.bak /etc/ppp/peers/pppoe0'
+    if r "$h" 'test -f /etc/ppp/peers/pppoe0'; then
+        pass "peers file restored on $h"
+    else
+        fail "peers file NOT restored on $h -- that router can no longer dial"
+    fi
     save_evidence T11-no-peers-file
     settle 24 >/dev/null
 }
@@ -271,9 +297,29 @@ T4() {  # hard failover across all three AC session-control policies
     log "    AC restored to session-control=replace"
 }
 
+T12() { # the flap damper must never tear down an ESTABLISHED session
+    log "T12 flap holdoff must not kill a live session"
+    local h; h="$(holder)"
+    [ "$h" = none ] && { fail "no master to test"; return; }
+    # Forge a holdoff far in the future, as a dial storm would. Before the fix
+    # ppp_dial() returned here BEFORE renewing may-dial, the lease went stale,
+    # and vrrp-wan-guard hung up the master's working WAN ~80s later.
+    r "$h" 'sudo sh -c "echo $(( $(date +%s) + 900 )) > /run/vrrp-wan/holdoff"'
+    # Sleep past LEASE_TTL (75s) so a non-renewed lease would definitely expire.
+    sleep 100
+    if [ "$(ppp_on "$h")" = 1 ]; then
+        pass "session survived a 900s holdoff (lease still renewed)"
+    else
+        fail "holdoff killed the live session -- damper is tearing down the WAN"
+    fi
+    r "$h" 'sudo rm -f /run/vrrp-wan/holdoff /run/vrrp-wan/dials'
+    save_evidence T12-holdoff-keeps-session
+    settle 24 >/dev/null
+}
+
 case "${1:---all}" in
-  --list) echo "T0 baseline | T3 clean failover | T4 hard failover x policy | T5 10gig-down | T8 lease expiry | T11 no-peers-file"; exit 0 ;;
-  --all)  preflight || exit 1; T0; T3; T5; T8; T11 ;;
+  --list) echo "T0 baseline | T3 clean failover | T4 hard failover x policy | T5 10gig-down | T8 lease expiry | T11 no-peers-file | T12 holdoff-keeps-session"; exit 0 ;;
+  --all)  preflight || exit 1; T0; T3; T5; T8; T11; T12 ;;
   --hard) preflight || exit 1; T4 ;;
   *)      preflight || exit 1; "$1" ;;
 esac

migration/vrrp-wan-reconcile

@@ -104,6 +104,19 @@ fi
 
 # --- PPPoE: the systemd plane ---------------------------------------------
 ppp_dial() {
+    # An ESTABLISHED session outranks every guard below, and this must be the
+    # first thing here. `may-dial` is a lease the guard expires after
+    # LEASE_TTL, so any early `return 1` before this renew silently hands the
+    # guard a live session to kill.
+    #
+    # Observed in labsim: the flap damper tripped, returned early, the lease
+    # went stale at 81s > 75s and the guard hung up pppoe0 ON THE MASTER --
+    # a damper meant to suppress repeat DIALS tore down a working WAN instead.
+    # Everything below only decides whether to start a NEW session.
+    if ppp_active; then
+        touch "$STATE/may-dial"
+        return 0
+    fi
     # Refuse to bless a box whose gate is missing. /etc is per-image, so a VyOS
     # upgrade silently drops the drop-in -- and without it BOTH routers dial on
     # the next commit that touches the pppoe subtree. Failing closed turns a
@@ -112,18 +125,36 @@ ppp_dial() {
         logger -t vrrp-wan "REFUSING to dial: gate drop-in $DROPIN is missing (VyOS upgrade?)"
         return 1
     fi
+    # The peers file is pppd's options file AND the gate's second condition, so
+    # without it this box silently never dials: systemd logs "skipped because of
+    # an unmet condition check" once and nothing else complains. Only a commit
+    # that touches the pppoe subtree re-renders it.
+    #
+    # Seen in labsim: config applied but never `save`d, the router rebooted, and
+    # came back with no pppoe0 node at all -- so no peers file, and a master that
+    # dialled every tick into silence. Say so loudly rather than looking healthy.
+    if [ ! -f /etc/ppp/peers/pppoe0 ]; then
+        if cfg | grep -q "interfaces pppoe pppoe0 source-interface"; then
+            logger -t vrrp-wan "CANNOT dial: pppoe0 is configured but /etc/ppp/peers/pppoe0 is missing -- re-commit the pppoe subtree to re-render it"
+        else
+            logger -t vrrp-wan "CANNOT dial: no pppoe0 in config (did a reboot revert an unsaved commit?)"
+        fi
+        return 1
+    fi
     now=$(date +%s)
     if [ -f "$STATE/holdoff" ] && [ "$now" -lt "$(cat "$STATE/holdoff" 2>/dev/null || echo 0)" ]; then
         return 1
     fi
     touch "$STATE/may-dial"          # renew the lease every tick
-    ppp_active && return 0
     # Trim the dial log to the window, then decide.
     if [ -f "$STATE/dials" ]; then
         awk -v c="$((now - FLAP_WINDOW))" '$1 > c' "$STATE/dials" > "$STATE/dials.new" 2>/dev/null
         mv "$STATE/dials.new" "$STATE/dials" 2>/dev/null
     fi
-    if [ "$(wc -l < "$STATE/dials" 2>/dev/null || echo 0)" -ge "$FLAP_MAX" ]; then
+    # `cat | wc`, not `wc -l < file`: the shell applies redirections left to
+    # right, so a missing file fails the `<` BEFORE `2>/dev/null` is in effect
+    # and dash prints "No such file or directory" on every first-ever dial.
+    if [ "$(cat "$STATE/dials" 2>/dev/null | wc -l)" -ge "$FLAP_MAX" ]; then
         echo $((now + FLAP_HOLDOFF)) > "$STATE/holdoff"
         logger -t vrrp-wan "DIAL FLAP: >=${FLAP_MAX} attempts in ${FLAP_WINDOW}s -- holding off ${FLAP_HOLDOFF}s"
         return 1

migration/vyos-mode-delta.py

@@ -243,22 +243,38 @@ def build_delta(inv: dict, priority: int, wan_user: str, with_wan: bool,
         ]
 
     if not with_wan:
-        # The backup carries the identical WAN and NAT config but with the
-        # interfaces administratively DOWN. The cloned MAC is therefore never
-        # live on two boxes at once, while everything needed to route and
-        # masquerade is already present -- taking over is enabling two
-        # interfaces, not rebuilding a config under pressure.
+        # Both boxes carry the identical WAN and NAT config; only the RESTING
+        # STATE differs, and only for the DHCP line. Takeover is no longer a
+        # human deleting two lines under pressure -- vrrp-wan-reconcile does it,
+        # driven by who holds the management VIP. See migration/PPPOE-HA.md.
+        #
+        # bond0.53 stays here, on the CONFIG plane, because its lease is bound
+        # to a cloned MAC and only VyOS config can move a MAC between boxes.
+        # This is the "nothing to follow" default: a freshly built or PXE'd box
+        # has no live master to imitate, so it must come up unable to claim that
+        # MAC. On a running pair the model follows reality instead -- see the
+        # export-before-apply rule in migration/PPPOE-HA.md.
+        #
+        # pppoe0 is deliberately NOT disabled here any more. `disable` unlinks
+        # /etc/ppp/peers/pppoe0, which is pppd's own options file, so it
+        # destroys what the promotion path needs and leaves ppp@pppoe0
+        # restart-looping. Dialling is gated at the systemd unit instead.
+        #
+        # ORDERING TRAP for a rebuilt box: because pppoe0 is left ENABLED,
+        # interfaces_pppoe.py will try to dial on the first commit that touches
+        # the pppoe subtree. Install the gate FIRST --
+        # `migration/vrrp-wan-install --vip <mgmt VIP> --host vyos@<box>` --
+        # or the new box will take the single ISP session off the live master.
         #
         # NAT rules naming a down interface are harmless: VyOS warns at commit
         # ("Interface ... does not exist!") and commits anyway, verified.
         out += [
             "",
-            "# --- WAN held DOWN on this box -----------------------------",
-            "# Enable these two to take over the internet path:",
-            f"#   set interfaces bonding bond0 vif {WAN_DHCP_VIF.split('.')[1]} disable  <- delete this",
-            f"#   set interfaces pppoe {WAN_PPPOE_IF} disable                <- and this",
+            "# --- 10 gig held DOWN on this box --------------------------",
+            "# Do NOT enable by hand: vrrp-wan-reconcile owns this, keyed on",
+            "# whoever holds the management VIP. pppoe0 is gated at the unit",
+            "# (ppp@pppoe0.service.d/10-vrrp-wan-gate.conf), not in config.",
             f"set interfaces bonding bond0 vif {WAN_DHCP_VIF.split('.')[1]} disable",
-            f"set interfaces pppoe {WAN_PPPOE_IF} disable",
         ]
 
     if True: