From 191f4c4377eaa2c422651949cfd6f983fae75444 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Sat, 10 Oct 2026 17:49:14 +0000 Subject: [PATCH] fix: only the other member of the pair gates a promote; the others are told after (keel#137) In cloud simple, one member outside the pair that was down (web-2) made every keel vip promote wait two 10 s timeouts on it, for its epoch and for the announcement: a planned promote took about 21 s on a real pair, and the write gap went up to 11 s. The same 10 s were the new primary's follow waiting for the VIP on wg0 (keel#138's 10 s). Before etcd, the promote now asks the other member of the pair alone for its epoch, asks it to release, and tells it the claim; each is waited for in full (the safety rule of keel#108: nothing fences a live old primary without etcd). The VIP is carried once that member took the claim; when it refuses or does not answer, the claim is released and the promote exits non-zero. The members outside the pair are told after, in a thread that is not a daemon, and what each said is logged. With --old-primary-gone every peer is asked and told and the claim is carried on a majority of those that answer, as before. With etcd the peers are told in the background too. A replica that holds the primary's claim asks the release at the next epoch at once; it asks the epoch only when the primary refuses that as stale, then asks the release again. So a planned promote has two round trips with the other member, not three (3.44 s at 250 ms with three). Tests: a member outside the pair down does not hold up the promote, and learns the claim after; no epoch is asked that is known here, and a stale release asks it; the other member refusing the claim stops it; a first promote with the other member down is refused, and taken with --old-primary-gone. New job vip-simple / trixie: every spec in cloud simple, C (outside the pair) cut off, a planned promote takes the VIP in less than 3 s at 250 ms and 2 % loss, and tests/vip_overlap.py proves no two nodes carry it at one instant. --- .github/workflows/tests.yml | 17 ++++ debian/changelog | 19 +++++ docs/vip.md | 28 ++++--- keel/__init__.py | 2 +- keel/mesh/vipnode.py | 24 +++--- keel/mesh/vippromote.py | 144 ++++++++++++++++++++++++++++++++- pyproject.toml | 2 +- tests/test_mesh_vipmove.py | 131 +++++++++++++++++++++++++++++- tests/test_mesh_vippair.py | 3 +- tests/test_vip_simple_netns.py | 97 ++++++++++++++++++++++ tests/vip_helpers.py | 6 ++ tests/vip_simple_netns.py | 80 ++++++++++++++++++ 12 files changed, 528 insertions(+), 25 deletions(-) create mode 100644 tests/test_vip_simple_netns.py create mode 100644 tests/vip_simple_netns.py diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d3891e5..f1b9726 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -104,6 +104,23 @@ jobs: etcd-server etcd-client util-linux KEEL_VIP_CLAIM_NETNS=1 PYTHONPATH=. python3 -m pytest -q -s \ tests/test_vip_claim_netns.py + # keel vip promote in cloud simple with a member outside the pair down + # (keel#137), on the vip job's nodes and links, every spec in cloud + # simple: a planned promote takes the VIP in less than 3 s, and no two + # nodes carry it at one instant (tests/test_vip_simple_netns.py). The + # check is "vip-simple / trixie". + vip-simple: + uses: keel-linux/.github/.github/workflows/lxc-trixie.yml@main + with: + systemd: true + timeout: 30 + run: | + apt-get update -qq + apt-get install -y -qq --no-install-recommends python3-pytest \ + python3-yaml wireguard-tools iproute2 iputils-ping openssl \ + etcd-server etcd-client util-linux + KEEL_VIP_SIMPLE_NETNS=1 PYTHONPATH=. python3 -m pytest -q -s \ + tests/test_vip_simple_netns.py # Upgrades on a pair whose VIP is active (the maintainer's requirement # of 2026-10-10), on the nodes and links of the vip job: keel-vip.service # try-restarted on the holder and the replica as keel-overlay-vip's diff --git a/debian/changelog b/debian/changelog index 955c6c8..faf736c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,22 @@ +keel (0.23.15) unstable; urgency=medium + + * In cloud simple, one member outside the pair that is down made every + keel vip promote wait two 10 s timeouts, for its epoch and for the + announcement: a planned promote took about 21 s on a real pair + (keel#137). Now the other member of the pair alone gates the + handover: its epoch, its release and its taking the claim, each + waited for in full, as nothing fences a live old primary without + etcd. The members outside the pair are told after, in the + background, and what each said is logged. With --old-primary-gone + every peer is asked and told, and the claim is carried on a + majority of those that answer, as before. With etcd the peers are + told in the background too. A replica that holds the primary's + claim asks the release at the next epoch at once, and asks the + epoch only when the primary refuses that as stale: two round trips + with the other member, not three. + + -- Marcos Méndez Sat, 10 Oct 2026 12:00:00 +0000 + keel (0.23.14) unstable; urgency=medium * keel vip promote failed with "etcd did not take the claim: etcdctl diff --git a/docs/vip.md b/docs/vip.md index 4dbf2ef..925ce4e 100644 --- a/docs/vip.md +++ b/docs/vip.md @@ -184,21 +184,28 @@ in the next. On the node that is to become the primary, which must hold the pair record: -1. it finds the newest claim: before etcd, every peer is asked its epoch - at once; with etcd, the counter; +1. it finds the newest claim: before etcd, the claim of the other + member of the pair that this node holds, else that member is asked + its epoch (every peer with `--old-primary-gone`); a release refused + as stale asks the epoch, then the release again; with etcd, the + counter; 2. when another node holds the VIP, it asks it to release, for the next epoch: **the old primary drops the address first and answers after**. When the old primary does not answer, **it refuses, unless `--old-primary-gone`**. With etcd and `--old-primary-gone`, it waits for the old primary's lease to expire, up to its TTL, and **never revokes another node's lease** (third round, point 5); -3. it claims at the next epoch. Before etcd, the claim is announced to - every peer, and **this node carries the VIP only when a majority of - the peers that answered took it**; with a majority refusing, or none - answering without `--old-primary-gone`, it exits non-zero and does - not carry it. With `--old-primary-gone` and no peer answering (a mesh - of the two nodes alone, the other one gone), the operator's flag is - the acceptance. With +3. it claims at the next epoch. Before etcd, **the other member of the + pair alone gates the handover** (keel#137): this node carries the VIP + only once that member took the claim; when it refuses or does not + answer, it exits non-zero and does not carry it. The members outside + the pair are told after, in the background, and what each said is + logged: one that is down no longer holds up the promote (two 10 s + timeouts on keel 0.23.13). With `--old-primary-gone` the other member + is declared gone, so every peer is asked and told, and **this node + carries the VIP only when a majority of the peers that answered took + it**; with no peer answering (a mesh of the two nodes alone, the + other one gone), the operator's flag is the acceptance. With etcd, the claim is the counter's compare-and-swap, and the controller adds the address once it renewed the claim's lease. A transaction that etcd does not answer in time may have committed (keel#135): the @@ -210,7 +217,8 @@ record: leases of the tries that did not win are revoked, so a late transaction with one of them fails in etcd; 4. each peer routes the VIP with one `wg set`; one that did not answer - takes it at its next check. + takes it at its next check. With etcd, the peers are told in the + background too. With `--old-primary-gone`, the operator declared the other member of the pair gone. **With etcd**, its lease fences it: each ask of it in diff --git a/keel/__init__.py b/keel/__init__.py index a8a76db..73036bd 100644 --- a/keel/__init__.py +++ b/keel/__init__.py @@ -12,7 +12,7 @@ # an image's tree read offline): debian/changelog's and pyproject.toml's, # which tests/test_version.py keeps in step. Installed, the metadata # dpkg put beside the package answers (keel#98). -FALLBACK_VERSION = "0.23.14" +FALLBACK_VERSION = "0.23.15" def version() -> str: diff --git a/keel/mesh/vipnode.py b/keel/mesh/vipnode.py index e7e97f6..c52316a 100644 --- a/keel/mesh/vipnode.py +++ b/keel/mesh/vipnode.py @@ -495,12 +495,15 @@ def drop_fenced(here: Here, vip: str, why: str, f"{'; ' + problem if problem else ''}") -def announce(here: Here, claim: Claim) -> dict[str, str]: - """The claim told to every peer at once, as the new holder does; each - peer's address to what it said: "applied", "known" (both accepted - it), "refused: why" (it answered no) or "unreachable: why" +def announce(here: Here, claim: Claim, + only: set[str] | None = None) -> dict[str, str]: + """The claim told to every peer at once (or to those of `only`), as + the new holder does; each peer's address to what it said: "applied", + "known" (both accepted it), "refused: why" (it answered no) or + "unreachable: why" """ - peers = [one.address for one in here.node.peers("")] + peers = [one.address for one in here.node.peers("") + if only is None or one.address in only] def one(at: str) -> str: try: @@ -518,10 +521,13 @@ def one(at: str) -> str: return dict(zip(peers, pool.map(one, peers))) -def epochs(here: Here, vip: str) -> dict[str, Claim | None | str]: - """Every peer asked, at once, for the newest claim it took of `vip`: - its address to that claim, verified, None for none, or why not""" - peers = [one.address for one in here.node.peers("")] +def epochs(here: Here, vip: str, + only: set[str] | None = None) -> dict[str, Claim | None | str]: + """Every peer (or those of `only`) asked, at once, for the newest + claim it took of `vip`: its address to that claim, verified, None + for none, or why not""" + peers = [one.address for one in here.node.peers("") + if only is None or one.address in only] def one(at: str) -> Claim | None | str: try: diff --git a/keel/mesh/vippromote.py b/keel/mesh/vippromote.py index e8ebaa1..65cb47e 100644 --- a/keel/mesh/vippromote.py +++ b/keel/mesh/vippromote.py @@ -36,6 +36,7 @@ """ import json +import threading from collections.abc import Callable from datetime import datetime @@ -252,9 +253,146 @@ def released(here: Here, vip: str, newest: Claim, epoch: int, gone: bool, return True +# the announcements to the members outside the pair, told after the +# handover in the background (keel#137); `announced` waits for them. +# BACKGROUND False tells them before the promote returns (the tests that +# read the peers right after a promote) +PENDING: list[threading.Thread] = [] +BACKGROUND = True + + +def pair_peer(here: Here, vip: str) -> str | None: + """The other member of the pair's overlay address, or None""" + try: + record = vipnode.kept(here, vip) + own = here.own_key() + except VipError: + return None + return next((vipnode.peer_at(here, key) for key in record.members + if not same_key(key, own)), None) + + def promoted(here: Here, vip: str, gone: bool, out: Callable[[str], None]) -> int: - """Before etcd: release, take, announce""" + """Before etcd: release, take, announce. The other member of the + pair alone gates the handover (keel#137): its epoch, its release, + its taking the claim, each waited for in full (nothing fences a live + old primary without etcd); the members outside the pair are told + after, in the background. With --old-primary-gone the other member + is declared gone, so every peer is asked and the claim is carried + on a majority of those that answer, as before""" + other = None if gone else pair_peer(here, vip) + if other is None: + return promoted_by_all(here, vip, gone, out) + own = here.own_key() + held = vipnode.current(here, vip) + # the other member's claim known here: the release is asked at the + # next epoch at once, and its epoch only when it refuses that as + # stale (two round trips with it, not three) + known = held.claim is not None and not same_key(held.claim.holder, own) + newest = held.claim if known else asked_epoch(here, vip, other, held, + out) + if newest is not None and same_key(newest.holder, own) and \ + held.holds(own) and vipnet.carried( + here.iface(), vip, here.node.output): + out(f"this node holds {vip} already, at epoch {newest.epoch}") + return exits.OK + epoch = (newest.epoch if newest else 0) + 1 + if newest is not None and not same_key(newest.holder, own): + why = asked_release(here, vip, newest, epoch, out) + if why == STALE and known: + newest = asked_epoch(here, vip, other, held, out) + epoch = (newest.epoch if newest else 0) + 1 + why = None if newest is None or same_key(newest.holder, own) \ + else asked_release(here, vip, newest, epoch, out) + if why: + out(f"the primary at {newest.address} did not release it: {why}." + " Nothing was changed. Only you know whether that node is" + " gone: if it is, run keel vip promote --old-primary-gone") + return exits.MESH_REFUSED + made = vipnode.signed_claim(here, vip, epoch) + vipnode.hold(here, made, False) + what = vipnode.announce(here, made, {other}).get( + other, "unreachable: not a peer of this node") + out(f" {other}: {what}") + if what not in ACCEPTED: + vipnode.release(here, vip, lambda lease: None) + out(f"the other member of the pair, at {other}, did not take the" + f" claim at epoch {epoch}: this node does not carry {vip}") + return exits.MESH_REFUSED + if not vipnode.carry_held(here, vip): + out(f"the VIP could not be added to {here.iface()}") + return exits.APPLY_FAILED + out(f"this node holds {vip} at epoch {epoch}, on {here.iface()}," + " taken by the other member of the pair; the other peers are told" + " in the background") + in_background(here, made, {other}) + return exits.OK + + +STALE = "stale" + + +def asked_epoch(here: Here, vip: str, other: str, held, + out: Callable[[str], None]) -> Claim | None: + """The newest claim this node and the other member of the pair know""" + answers = vipnode.epochs(here, vip, {other}) + for at, one in answers.items(): + if isinstance(one, str): + out(f" {at}: {one}") + claims = [one for one in answers.values() if isinstance(one, Claim)] + return vipnode.newest(claims + ([held.claim] if held.claim else [])) + + +def asked_release(here: Here, vip: str, newest: Claim, epoch: int, + out: Callable[[str], None]) -> str | None: + """The holder asked to release, for `epoch`: None once it did, STALE + when it knows that epoch or a newer one, else why not""" + out(f"asking the primary, {newest.holder} at {newest.address}, to" + f" release {vip}…") + try: + here.say("release", newest.address, {"vip": vip, "epoch": epoch}) + except (LinkError, VipError, SigningError) as e: + if "stale release" in str(e): + out(f"the primary at {newest.address} knows a newer epoch;" + " asking it") + return STALE + return str(e) + out(f"released by {newest.address}") + return None + + +def in_background(here: Here, made: Claim, told_already: set[str]) -> None: + """The claim told to every other peer in a thread of its own, not a + daemon (the process ends after it), what each said logged""" + peers = {one.address for one in here.node.peers("")} - told_already + if not peers: + return + + def tell() -> None: + for at, what in sorted(vipnode.announce(here, made, peers).items()): + here.err(f"vip {made.vip}: {at}: {what}") + if not BACKGROUND: + tell() + return + worker = threading.Thread(target=tell) + PENDING.append(worker) + worker.start() + + +def announced(timeout: float | None = None) -> int: + """Wait for the announcements in the background; how many there were""" + done = 0 + while PENDING: + PENDING.pop(0).join(timeout) + done += 1 + return done + + +def promoted_by_all(here: Here, vip: str, gone: bool, + out: Callable[[str], None]) -> int: + """Every peer asked and told, the claim carried on a majority of the + peers that answer (the flag is the acceptance when none does)""" own = here.own_key() held = vipnode.current(here, vip) answers = vipnode.epochs(here, vip) @@ -393,7 +531,9 @@ def promoted_etcd(here: Here, vip: str, gone: bool, f" {CARRY_WAIT:g} s: is it running? Its lease expires" f" {vipetcd.TTL} s after its last renewal") return exits.APPLY_FAILED - told(here, made[0], out) + # etcd holds the claim; the peers are told in the background (keel#137) + out("the peers are told in the background") + in_background(here, made[0], set()) return exits.OK diff --git a/pyproject.toml b/pyproject.toml index 788005c..52c0f48 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "keel" -version = "0.23.14" +version = "0.23.15" authors = [{name = "KeelLinux maintainers"}] description = "Declarative management of one appliance instance" readme = "README.md" diff --git a/tests/test_mesh_vipmove.py b/tests/test_mesh_vipmove.py index eb086e5..5e4105f 100644 --- a/tests/test_mesh_vipmove.py +++ b/tests/test_mesh_vipmove.py @@ -116,6 +116,132 @@ def slow(at: str, iface: str, body: bytes) -> bytes: self.assertFalse(self.carried(0)) self.assertTrue(self.carried(1)) + def slow(self, index: int, host: str, seconds: float, + answers: bool = True): + """Node `index`'s exchanges with `host` take `seconds`, then + answer, or fail as a dead node does""" + import time as clock + real = self.all[index].exchange + + def exchange(at: str, iface: str, body: bytes) -> bytes: + if at == host: + clock.sleep(seconds) + if not answers: + raise LinkError(f"[{at}]:51821 through {iface}: timed out") + return real(at, iface, body) + self.all[index].exchange = exchange + + def test_a_member_outside_the_pair_down_does_not_hold_up_the_promote( + self): + """keel#137: one unreachable member outside the pair added two + 10 s timeouts to every promote. Only the other member of the + pair gates the handover; the others are told after, in the + background, and what they said is logged""" + import time as clock + from unittest import mock + self.nodes() + self.promote(0) + self.slow(1, address(2), 1.0, answers=False) + started = clock.monotonic() + with mock.patch.object(vippromote, "BACKGROUND", True): + code, said = self.promote(1) + took = clock.monotonic() - started + self.assertEqual(code, exits.OK, said) + self.assertLess(took, 0.8, said) + self.assertTrue(self.carried(1)) + self.assertFalse(self.carried(0)) + self.assertIn(f"released by {address(0)}", said) + self.assertIn("taken by the other member of the pair", said) + self.assertEqual(vippromote.announced(), 1) + self.assertIn(f"vip {VIP}: {address(2)}: unreachable", + self.text(1)) + + def test_a_member_outside_the_pair_still_learns_the_claim_after(self): + from unittest import mock + self.nodes() + self.promote(0) + self.slow(1, address(2), 0.5) + with mock.patch.object(vippromote, "BACKGROUND", True): + code, said = self.promote(1) + self.assertEqual(code, exits.OK, said) + self.assertEqual(vippromote.announced(), 1) + self.assertEqual(self.routed(2), KEYS[1]) + self.assertIn(f"vip {VIP}: {address(2)}: applied", self.text(1)) + + def kinds(self, index: int) -> list[tuple[str, str]]: + """Every message node `index` sends: (to, kind)""" + real = self.all[index].exchange + sent: list[tuple[str, str]] = [] + + def exchange(at: str, iface: str, body: bytes) -> bytes: + sent.append((at, vipmsg.loads(body).kind)) + return real(at, iface, body) + self.all[index].exchange = exchange + return sent + + def test_a_planned_promote_asks_no_epoch_it_knows(self): + """Two round trips with the other member, not three: the replica + holds the primary's claim, so it asks the release at the next + epoch at once""" + self.nodes() + self.promote(0) + sent = self.kinds(1) + code, said = self.promote(1) + self.assertEqual(code, exits.OK, said) + self.assertEqual([kind for at, kind in sent if at == address(0)], + [vipmsg.RELEASE, vipmsg.CLAIM]) + + def test_a_release_refused_as_stale_asks_the_epoch_and_again(self): + """B missed A's newer claim: A refuses the release as stale, B + asks its epoch, and asks the release again at the next one""" + self.nodes() + self.promote(0) + self.down.add(address(1)) + self.promote(0) + held = vipnode.current(self.all[0], VIP) + self.down.clear() + # A claims again at a newer epoch while B is away + made = vipnode.signed_claim(self.all[0], VIP, held.epoch + 1) + vipnode.hold(self.all[0], made, True) + sent = self.kinds(1) + code, said = self.promote(1) + self.assertEqual(code, exits.OK, said) + kinds = [kind for at, kind in sent if at == address(0)] + self.assertEqual(kinds, [vipmsg.RELEASE, vipmsg.EPOCH, + vipmsg.RELEASE, vipmsg.CLAIM], said) + self.assertEqual(vipnode.current(self.all[1], VIP).epoch, + held.epoch + 2) + self.assertFalse(self.carried(0)) + + def test_the_other_member_refusing_the_claim_stops_it(self): + self.nodes() + self.promote(0) + real = self.all[1].exchange + seen: list[str] = [] + + def exchange(at: str, iface: str, body: bytes) -> bytes: + message = vipmsg.loads(body) + seen.append(message.kind) + if at == address(0) and message.kind == vipmsg.CLAIM: + raise LinkError(f"[{at}]:51821 refused: stale claim") + return real(at, iface, body) + self.all[1].exchange = exchange + code, said = self.promote(1) + self.assertEqual(code, exits.MESH_REFUSED, said) + self.assertIn("did not take the claim", said) + self.assertFalse(self.carried(1)) + self.assertTrue(vipnode.current(self.all[1], VIP).released) + + def test_a_first_promote_with_the_other_member_down_is_refused(self): + self.nodes() + self.down.add(address(1)) + code, said = self.promote(0) + self.assertEqual(code, exits.MESH_REFUSED, said) + self.assertFalse(self.carried(0)) + # --old-primary-gone: the operator's word, as before + code, said = self.promote(0, gone=True) + self.assertEqual(code, exits.OK, said) + def test_a_node_that_declares_no_vip_promotes_nothing(self): self.nodes() code, said = self.promote(2) @@ -148,7 +274,10 @@ def test_a_peer_that_does_not_answer_is_said(self): self.down.add(address(2)) code, said = self.promote(0) self.assertEqual(code, exits.OK) - self.assertIn(f"{address(2)}: [{address(2)}]:51821", said) + # a member outside the pair is told after the handover, and what + # it said is logged (keel#137) + self.assertIn(f"{address(2)}: unreachable: [{address(2)}]:51821", + self.text(0)) class TestTheChannelRefuses(Pair): diff --git a/tests/test_mesh_vippair.py b/tests/test_mesh_vippair.py index 8c92c3e..5ef8bba 100644 --- a/tests/test_mesh_vippair.py +++ b/tests/test_mesh_vippair.py @@ -255,7 +255,8 @@ def test_no_peer_answering_is_no_majority(self): self.down.update({address(1), address(2)}) code, said = promote(self.all[0]) self.assertEqual(code, exits.MESH_REFUSED) - self.assertIn("0 of the 0", said) + # the other member of the pair gates the handover (keel#137) + self.assertIn("did not take the claim", said) self.assertFalse(self.carried(0)) def test_a_carry_that_fails_after_the_majority(self): diff --git a/tests/test_vip_simple_netns.py b/tests/test_vip_simple_netns.py new file mode 100644 index 0000000..7d01445 --- /dev/null +++ b/tests/test_vip_simple_netns.py @@ -0,0 +1,97 @@ +# Copyright (c) 2026 KeelLinux maintainers +"""keel vip promote in cloud simple with a member outside the pair down +(keel#137) + +tests/vip_simple_netns.py, as root in a network namespace of its own, on +tests/vip_netns.py's nodes and links (250 ms ±25 ms, 2 % loss), every +spec in cloud simple: with C, outside the pair, cut off, a planned +promote takes the VIP in less than 3 s (on keel 0.23.13 it waited two +10 s timeouts on C), and at no instant do two nodes carry it +(tests/vip_overlap.py). In CI it runs in its own job, "vip-simple / +trixie", which sets KEEL_VIP_SIMPLE_NETNS. +""" + +import json +import os +import shutil +import subprocess +import sys +import unittest + +import wgtools +from test_etcd_netns import etcd_dir + +REPO = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +DRIVER = os.path.join(REPO, "tests", "vip_simple_netns.py") + + +class TestAMemberOutsideThePairDown(unittest.TestCase): + found: dict = {} + + def setUp(self): + if os.environ.get("CI") and \ + not os.environ.get("KEEL_VIP_SIMPLE_NETNS"): + self.skipTest("runs in the vip-simple / trixie job") + tools = wgtools.require(self) + etcd = etcd_dir() + if etcd is None: + if os.environ.get("CI"): + self.fail("etcd is not installed in CI") + self.skipTest("etcd not found (set KEEL_ETCD_DIR)") + if not os.path.exists("/sys/module/wireguard") and \ + not os.environ.get("CI"): + self.skipTest("the wireguard kernel module is not loaded") + if not shutil.which("tc") and not os.environ.get("CI"): + self.skipTest("tc not found (iproute2)") + if not self.found: + self.found.update(self.run_driver(f"{tools}:{etcd}")) + + def run_driver(self, path: str) -> dict: + prefix = wgtools.namespace_prefix() + if prefix is None: + if os.environ.get("CI"): + self.fail("no way to run wg-quick as root in a namespace") + self.skipTest("neither root, user namespaces nor sudo -n") + done = subprocess.run( + prefix + ["env", f"PATH={path}:{os.environ.get('PATH', '')}", + f"PYTHONPATH={REPO}", + f"TMPDIR={os.environ.get('TMPDIR') or '/tmp'}", + sys.executable, DRIVER], + capture_output=True, text=True, check=False, timeout=1500) + for line in done.stdout.splitlines(): + if line.startswith("RESULT "): + print(line, file=sys.stderr) + return json.loads(line[len("RESULT "):]) + self.fail(f"no result from {DRIVER}:\n{done.stdout[-3000:]}\n" + f"{done.stderr[-3000:]}") + + def test_the_links_are_the_design_case(self): + link = self.found["link"] + self.assertGreater(link["rtt_min_ms"], 220, link) + self.assertLess(link["rtt_median_ms"], 300, link) + self.assertNotIn("error", self.found, self.found) + self.assertEqual(self.found["a"]["code"], 0, self.found["a"]) + # cloud simple: no lease + self.assertFalse(any("etcd lease" in line for line in + self.found["a_status"]["lines"]), self.found) + + def test_a_planned_promote_with_a_member_down_takes_under_3_s(self): + found = self.found + print(f"planned promote, C down: {found['b_s']} s", file=sys.stderr) + self.assertEqual(found["b"]["code"], 0, found["b"]) + self.assertLess(found["b_s"], 3.0, found["b"]) + self.assertIsNotNone(found["b_holds_s"], found) + self.assertTrue(found["a_dropped"], found) + self.assertEqual(found["holders_after"], ["B"], found) + + def test_no_two_nodes_carry_it_at_one_instant(self): + atomic = self.found["atomic"] + self.assertTrue(atomic["ended"], atomic) + self.assertGreater(atomic["rounds"], 1000, atomic) + self.assertEqual(atomic["overlaps"], [], atomic) + self.assertEqual(self.found["max_holders"], 1, + self.found["double_holder_samples"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/vip_helpers.py b/tests/vip_helpers.py index 8d1455d..ec1e2aa 100644 --- a/tests/vip_helpers.py +++ b/tests/vip_helpers.py @@ -230,6 +230,12 @@ def setUp(self): return_value=True) patcher.start() self.addCleanup(patcher.stop) + # the peers outside the pair told before a promote returns, so a + # test reads them right after it; tests/test_mesh_vipmove.py + # turns the background on for keel#137's own tests + patcher = mock.patch("keel.mesh.vippromote.BACKGROUND", False) + patcher.start() + self.addCleanup(patcher.stop) self.all: list[Here] = [] self.nets: list[FakeNet] = [] self.said: dict[int, list[str]] = {} diff --git a/tests/vip_simple_netns.py b/tests/vip_simple_netns.py new file mode 100644 index 0000000..2c4e34a --- /dev/null +++ b/tests/vip_simple_netns.py @@ -0,0 +1,80 @@ +# Copyright (c) 2026 KeelLinux maintainers +"""keel vip promote in cloud simple with a member outside the pair down +(keel#137), on tests/vip_netns.py's nodes and links + +Run as root in a network namespace of its own by +tests/test_vip_simple_netns.py; never run by hand on a machine. The +nodes, the links (250 ms ±25 ms, 2 % loss, measured first), the members' +channel, the VIP's helpers and the watcher of two holders at one instant +(tests/vip_overlap.py) are tests/vip_netns.py's, with every spec in +cloud simple and the etcd overlay disabled: the VIP moves by the +members' channel alone. A and B are the pair, C a member outside it. + +1. A promotes: the VIP at A; +2. C is cut off, both ways; +3. B promotes, planned (A releases): timed. On keel 0.23.13 each promote + waited two 10 s timeouts on C (the epoch query, the announcement); +4. C back: it learns the claim at its next check, or from the + announcement it missed. + +The result is one JSON line, `RESULT {...}`. +""" + +import time + + +def spec_of(root: str, index: int, pairs) -> str: + """vip_netns.spec_of, in cloud simple with etcd disabled""" + import yaml + path = ORIGINAL(root, index, pairs) + with open(path) as fob: + doc = yaml.safe_load(fob) + doc["installation"]["mode"] = "cloud_simple" + doc["overlays"] = {"etcd": "disabled"} + with open(path, "w") as fob: + yaml.safe_dump(doc, fob, sort_keys=False) + return path + + +ORIGINAL = None + + +def play(report: dict, pids: list[int], pairs, roots: list[str], + samples: list[dict]) -> None: + import vip_netns + from vip_netns import NAMES, agent_send, holds, leg, sh, wait_until + a_pid, b_pid, c_pid = pids + report["a"] = agent_send("A", "promote") + report["a_holds_s"] = wait_until(lambda: holds(a_pid), 60) + report["a_status"] = agent_send("A", "status") + time.sleep(3) + leg("to3", "100%") + sh("tc", "qdisc", "replace", "dev", "uplink", "root", "netem", "loss", + "100%", pid=c_pid) + started = time.time() + report["b"] = agent_send("B", "promote", 300) + report["b_s"] = round(time.time() - started, 2) + report["b_holds_s"] = wait_until(lambda: holds(b_pid), 60) + report["a_dropped"] = not holds(a_pid) + vip_netns.heal(pids, 2) + time.sleep(5) + report["holders_after"] = [n for n, p in zip(NAMES, pids) if holds(p)] + report["b_status"] = agent_send("B", "status") + for process in vip_netns.AGENTS.values(): + try: + process.stdin.write("quit\n") + process.stdin.close() + except OSError: + pass + + +def main() -> None: + global ORIGINAL + import vip_netns + ORIGINAL = vip_netns.spec_of + vip_netns.spec_of = spec_of + vip_netns.main(play=play) + + +if __name__ == "__main__": + main()