From 8d5eac6bbf8230cd7a9c2fa8e460f2f7637239eb Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Wed, 28 Oct 2026 12:34:12 +0100 Subject: [PATCH] fix(sys-svc-container): probe one packet size at the egress MTU automtu searched the path MTU between its payload bounds, and a lost echo during that search moved the result between 1400 and 1398 from one deploy to the next. Every move changed the daemon MTU, rewrote daemon.json and restarted dockerd. The probe window is now a single payload: the smaller of 1500 and the egress MTU automtu reads itself (`--print-mtu egress`), minus the 28 bytes of IP and ICMP header, with `--pmtu-policy max` so one target answering at that size decides. registry-1.docker.io leaves the targets because it does not answer ICMP echo. The assert now names both the egress and the effective value. Local repeat deploy: the probe took 0.83 s and no handler restarted dockerd. Co-Authored-By: Claude Opus 5.5 (1M context) --- roles/sys-svc-container/tasks/_mtu.yml | 24 +++- roles/sys-svc-container/vars/main.yml | 4 +- .../roles/sys-svc-container/test_mtu_probe.py | 113 ++++++++++++++++++ 3 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 tests/unit/python/roles/sys-svc-container/test_mtu_probe.py diff --git a/roles/sys-svc-container/tasks/_mtu.yml b/roles/sys-svc-container/tasks/_mtu.yml index f7972ff789..5baa98b2e8 100644 --- a/roles/sys-svc-container/tasks/_mtu.yml +++ b/roles/sys-svc-container/tasks/_mtu.yml @@ -12,6 +12,17 @@ - iproute - iputils-ping +- name: 📏 Read the egress MTU automtu measures against + ansible.builtin.command: + argv: + - "{{ ansible_python_interpreter }}" + - -m + - automtu + - --print-mtu + - egress + register: automtu_egress + changed_when: false + - name: 🔍 Detect safe Docker MTU ansible.builtin.command: argv: @@ -20,20 +31,29 @@ - automtu - --pmtu-target - "{{ SYS_SVC_DOCKER_AUTOMTU_TARGETS | join(',') }}" + - --pmtu-min-payload + - "{{ automtu_probe_payload }}" + - --pmtu-max-payload + - "{{ automtu_probe_payload }}" - --pmtu-policy - - min + - max - --print-mtu - effective + vars: + automtu_probe_payload: >- + {{ (([SYS_SVC_DOCKER_AUTOMTU_MTU_CEILING, automtu_egress.stdout | int] | min) + - SYS_SVC_DOCKER_AUTOMTU_ICMP_HEADER_BYTES) | string }} register: automtu_mtu changed_when: false - name: ✅ Assert automtu returned an integer MTU ansible.builtin.assert: that: + - automtu_egress.stdout is match('^[0-9]+$') - automtu_mtu.stdout is match('^[0-9]+$') - (automtu_mtu.stdout | int) >= 576 - (automtu_mtu.stdout | int) <= 9000 - fail_msg: "automtu returned unexpected MTU output: '{{ automtu_mtu.stdout | default('') }}'" + fail_msg: "automtu returned unexpected MTU output: egress '{{ automtu_egress.stdout }}', effective '{{ automtu_mtu.stdout }}'" - name: 🔧 Set docker daemon MTU fact ansible.builtin.set_fact: diff --git a/roles/sys-svc-container/vars/main.yml b/roles/sys-svc-container/vars/main.yml index b24f95cc26..e15e50fb1d 100644 --- a/roles/sys-svc-container/vars/main.yml +++ b/roles/sys-svc-container/vars/main.yml @@ -3,7 +3,9 @@ SYS_SVC_DOCKER_DIR: "/etc/docker/" SYS_SVC_DOCKER_DAEMON: "{{ [SYS_SVC_DOCKER_DIR, 'daemon.json' ] | path_join }}" SYS_SVC_DOCKER_AUTOMTU_TARGETS: >- - {{ ['registry-1.docker.io', 'github.com'] + NETWORK_PUBLIC_DNS_RESOLVERS }} + {{ ['github.com'] + NETWORK_PUBLIC_DNS_RESOLVERS }} +SYS_SVC_DOCKER_AUTOMTU_MTU_CEILING: 1500 +SYS_SVC_DOCKER_AUTOMTU_ICMP_HEADER_BYTES: 28 SYS_SVC_CONTAINER_DOCKER_REPO_DISTRO_BY_DISTRIBUTION: Debian: debian diff --git a/tests/unit/python/roles/sys-svc-container/test_mtu_probe.py b/tests/unit/python/roles/sys-svc-container/test_mtu_probe.py new file mode 100644 index 0000000000..72fe0a2b12 --- /dev/null +++ b/tests/unit/python/roles/sys-svc-container/test_mtu_probe.py @@ -0,0 +1,113 @@ +import unittest + +from jinja2 import StrictUndefined +from jinja2.nativetypes import NativeEnvironment + +from utils import PROJECT_ROOT +from utils.cache.yaml import load_yaml_any +from utils.roles.mapping import ROLE_FILE_VARS_MAIN + +ROLE = PROJECT_ROOT / "roles" / "sys-svc-container" +EGRESS_MTUS = (1200, 1280, 1400, 1500, 9000) +IPV4_ICMP_HEADER_BYTES = 28 +INTERNET_MTU = 1500 + +_ENV = NativeEnvironment(undefined=StrictUndefined) + + +def _render(value, context): + if isinstance(value, str) and "{{" in value: + return _ENV.from_string(value).render(context) + return value + + +def _task(tasks, register): + return next(task for task in tasks if task.get("register") == register) + + +def _option(argv, flag): + return int(argv[argv.index(flag) + 1]) + + +class TestAutomtuProbeWindow(unittest.TestCase): + @classmethod + def setUpClass(cls) -> None: + cls.tasks = load_yaml_any(ROLE / "tasks" / "_mtu.yml") + networks = load_yaml_any( + PROJECT_ROOT / "group_vars" / "all" / "08_networks.yml" + ) + cls.context = { + "ansible_python_interpreter": "python3", + "NETWORK_PUBLIC_DNS_RESOLVERS": networks["NETWORK_PUBLIC_DNS_RESOLVERS"], + } + for key, value in load_yaml_any(ROLE / ROLE_FILE_VARS_MAIN).items(): + if key.startswith("SYS_SVC_DOCKER_AUTOMTU_"): + cls.context[key] = _render(value, cls.context) + + def _detect_argv(self, egress_mtu): + task = _task(self.tasks, "automtu_mtu") + context = {**self.context, "automtu_egress": {"stdout": str(egress_mtu)}} + for key, value in task["vars"].items(): + context[key] = _render(value, context) + return [ + str(_render(arg, context)) + for arg in task["ansible.builtin.command"]["argv"] + ] + + def test_the_probe_window_is_one_size_at_the_largest_reachable_mtu(self) -> None: + for egress_mtu in EGRESS_MTUS: + with self.subTest(egress_mtu=egress_mtu): + argv = self._detect_argv(egress_mtu) + low = _option(argv, "--pmtu-min-payload") + high = _option(argv, "--pmtu-max-payload") + self.assertEqual( + low, + high, + "a binary search turns every lost echo into a lower MTU; a " + "one-size window confirms the ceiling and only searches " + "below it when that size fails", + ) + self.assertEqual( + high + IPV4_ICMP_HEADER_BYTES, + min(egress_mtu, INTERNET_MTU), + "automtu returns min(egress, measured): a probe above the egress " + "MTU or above the internet MTU only buys a 1 s timeout, a probe " + "below it lowers the result", + ) + + def test_one_target_confirming_the_window_decides(self) -> None: + argv = self._detect_argv(1400) + self.assertEqual( + argv[argv.index("--pmtu-policy") + 1], + "max", + "under min one target's lost echoes lower daemon.json, and every " + "change restarts dockerd; max keeps the largest size a target " + "confirmed end to end, which never exceeds the egress MTU", + ) + + def test_the_window_comes_from_automtu_not_from_gathered_facts(self) -> None: + egress = _task(self.tasks, "automtu_egress") + argv = egress["ansible.builtin.command"]["argv"] + self.assertEqual(argv[-2:], ["--print-mtu", "egress"]) + self.assertNotIn("--pmtu-target", argv) + detect = _task(self.tasks, "automtu_mtu") + self.assertLess(self.tasks.index(egress), self.tasks.index(detect)) + for fact in ("ansible_facts", "ansible_default_ipv4", "ansible_default_ipv6"): + self.assertNotIn( + fact, + repr(detect), + "this file installs iproute, so a fresh container gathered no " + "network facts and default_ipv4 is undefined when the probe runs", + ) + + def test_every_target_answers_icmp_echo(self) -> None: + self.assertNotIn( + "registry-1.docker.io", + self.context["SYS_SVC_DOCKER_AUTOMTU_TARGETS"], + "registry-1.docker.io drops ICMP echo, so it only adds four 1 s " + "timeouts to every deploy", + ) + + +if __name__ == "__main__": + unittest.main()