Skip to content

Commit d6e7193

Browse files
committed
Merge same-mechanism limit layers into one launcher invocation
The triple-stack scenarios declare memory and CPU layers on the same container and nested cgroup. Composing every layer linearly nested a second docker run inside the first, applied the address-space rlimit to the docker client instead of the payload, and had two cgroup scripts writing the same path, so the scenario could never reach a measurement. Layers sharing an enforcement context are now grouped onto one launcher. Also corrects the Windows job object CPU rate to a share of total machine capacity, makes the Kubernetes probe check controller delegation as documented, and stops discarding dependency install errors under a non-root user. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BEZUSri3baWU58Ge1kAVnT
1 parent d78c999 commit d6e7193

12 files changed

Lines changed: 831 additions & 45 deletions

tests/e2e/launchers/docker.py

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,14 @@
99
Also used, with ``privileged=True, cgroupns_private=True``, as the outermost
1010
layer of the triple-stack scenario -- the nested-cgroup layer's launcher
1111
(:mod:`tests.e2e.launchers.nested_cgroup`) supplies the container's CMD.
12+
13+
``memory_bytes`` and a CPU axis (``cpu_millicores`` or ``cpuset_millicores``)
14+
may be supplied *together*: the triple-stack scenarios declare a separate
15+
``DOCKER_MEMORY`` layer and a separate ``DOCKER_CPU_QUOTA`` layer for the same
16+
container, and :mod:`tests.e2e.launchers.factory` merges both onto **one**
17+
``DockerLauncher`` instance -- one ``docker run`` imposing both ceilings at
18+
once (``--memory ... --memory-swap ... --cpus ...``), never a second, nested
19+
``docker run`` (iter-2 fix for the "docker-in-docker" composition blocker).
1220
"""
1321

1422
from __future__ import annotations
@@ -37,9 +45,10 @@ def __init__(
3745
cgroupns_private: bool = False,
3846
image: str = DEFAULT_IMAGE,
3947
):
40-
set_count = sum(v is not None for v in (memory_bytes, cpu_millicores, cpuset_millicores))
41-
if set_count != 1:
42-
raise ValueError("DockerLauncher takes exactly one of memory_bytes/cpu_millicores/cpuset_millicores")
48+
if cpu_millicores is not None and cpuset_millicores is not None:
49+
raise ValueError("DockerLauncher takes at most one of cpu_millicores/cpuset_millicores (two different CPU mechanisms)")
50+
if memory_bytes is None and cpu_millicores is None and cpuset_millicores is None:
51+
raise ValueError("DockerLauncher requires at least one of memory_bytes/cpu_millicores/cpuset_millicores")
4352
self.memory_bytes = memory_bytes
4453
self.cpu_millicores = cpu_millicores
4554
self.cpuset_millicores = cpuset_millicores
@@ -61,9 +70,9 @@ def wrap_argv(self, inner_argv: Sequence[str]) -> list[str]:
6170
argv += ["--cgroupns", "private"]
6271
if self.memory_bytes is not None:
6372
argv += ["--memory", str(self.memory_bytes), "--memory-swap", str(self.memory_bytes)]
64-
elif self.cpu_millicores is not None:
73+
if self.cpu_millicores is not None:
6574
argv += ["--cpus", f"{self.cpu_millicores / 1000:g}"]
66-
else:
75+
elif self.cpuset_millicores is not None:
6776
n_cores = self.cpuset_millicores // 1000
6877
cpuset = f"0-{n_cores - 1}" if n_cores > 1 else "0"
6978
argv += ["--cpuset-cpus", cpuset]
@@ -75,8 +84,24 @@ def wrap_argv(self, inner_argv: Sequence[str]) -> list[str]:
7584
"-w", _CONTAINER_WORKDIR,
7685
"-e", f"PYTHONPATH={_CONTAINER_WORKDIR}/src",
7786
]
87+
if self.user is not None:
88+
# Non-root: `pip install --user` needs a resolvable, writable user
89+
# site-packages directory. The stock upstream image has no
90+
# /etc/passwd entry (and so no $HOME) for an arbitrary numeric
91+
# --user uid; Python's home-directory lookup checks $HOME first,
92+
# before ever falling back to the passwd database, so setting it
93+
# explicitly sidesteps the missing-passwd-entry problem entirely
94+
# rather than depending on one existing (iter-2 fix -- this used
95+
# to fail silently and kill the payload with an unlogged
96+
# ModuleNotFoundError). /tmp is world-writable (mode 1777) in
97+
# every upstream image, so it works for any uid.
98+
argv += ["-e", "HOME=/tmp"]
7899
argv.append(self.image)
79100
pip_install = "pip install --user -q psutil" if self.user else "pip install -q psutil"
80-
prelude = f"{pip_install} >/dev/null 2>&1"
81-
argv += ["sh", "-c", f"{prelude}; exec {shlex.join(inner_argv)}"]
101+
# `set -e` + `&&` (never `;` with output discarded): a failed install
102+
# must abort the container and surface its own stderr in the captured
103+
# output, never be swallowed by `>/dev/null 2>&1` and leave the
104+
# payload to die on a bare, unexplained `ModuleNotFoundError` with no
105+
# sentinel line (iter-2 fix).
106+
argv += ["sh", "-c", f"set -e; {pip_install} && exec {shlex.join(inner_argv)}"]
82107
return argv

tests/e2e/launchers/factory.py

Lines changed: 131 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,27 @@
33
The one place layer-type -> launcher wiring lives; everything else (the
44
registry, the outer harness) works in terms of :class:`Layer`/:class:`Launcher`
55
only.
6+
7+
**Merging same-mechanism layers (iter-2 fix for the triple-stack composition
8+
blocker).** A naive one-launcher-per-layer mapping is wrong whenever a
9+
scenario declares two layers that are really the *same enforcement context*
10+
imposing two different axes -- e.g. the triple-stack scenarios' separate
11+
``DOCKER_MEMORY``/``DOCKER_CPU_QUOTA`` layers are one container, not two
12+
nested ``docker run`` invocations, and their separate
13+
``NESTED_CGROUP_MEMORY``/``NESTED_CGROUP_CPU`` layers are one nested cgroup,
14+
not two independently-created cgroups at the same path. :func:`build_launchers`
15+
(and :func:`build_launcher_groups`, which also reports which original layers
16+
each merged launcher covers) group a scenario's layers by *launcher family*
17+
first, merge every layer of a multi-axis family (currently docker and the
18+
nested-cgroup layer) onto one launcher instance, and leave every other layer
19+
type exactly one-to-one with its own launcher -- preserving outside-in order
20+
by each family's first occurrence in ``scenario.layers``.
621
"""
722

823
from __future__ import annotations
924

25+
from typing import Sequence
26+
1027
from tests.e2e.launchers.base import Launcher
1128
from tests.e2e.launchers.cgroup import CgroupLauncher
1229
from tests.e2e.launchers.docker import DockerLauncher
@@ -24,6 +41,24 @@
2441
_MEMORY_KWARG = "memory_bytes"
2542
_CPU_KWARG = "cpu_millicores"
2643

44+
# Layer types that merge onto one launcher instance per scenario when a
45+
# scenario declares more than one of them -- the "same enforcement context,
46+
# two axes" case. Every LayerType not listed here is its own singleton family:
47+
# exactly one launcher per layer, as before.
48+
_DOCKER_TYPES = frozenset({LayerType.DOCKER_MEMORY, LayerType.DOCKER_CPU_QUOTA, LayerType.DOCKER_CPUSET})
49+
_NESTED_CGROUP_TYPES = frozenset({LayerType.NESTED_CGROUP_MEMORY, LayerType.NESTED_CGROUP_CPU})
50+
51+
_FAMILY_DOCKER = "docker"
52+
_FAMILY_NESTED_CGROUP = "nested_cgroup"
53+
54+
55+
def _layer_family(layer: Layer) -> str:
56+
if layer.type in _DOCKER_TYPES:
57+
return _FAMILY_DOCKER
58+
if layer.type in _NESTED_CGROUP_TYPES:
59+
return _FAMILY_NESTED_CGROUP
60+
return layer.type.value # singleton family: this layer type never merges
61+
2762

2863
def build_launcher(layer: Layer) -> Launcher:
2964
hints = dict(layer.launcher_hints)
@@ -76,6 +111,100 @@ def build_launcher(layer: Layer) -> Launcher:
76111
raise ValueError(f"no launcher factory registered for layer type {layer.type}") # pragma: no cover
77112

78113

114+
def _merge_hints(layers: Sequence[Layer]) -> dict[str, object]:
115+
"""Union launcher_hints across a merged group, raising on a genuine
116+
conflict (two layers of the same family disagreeing on the same hint) --
117+
every triple-stack docker layer declares matching privileged/cgroupns_private
118+
hints, so a conflict here would mean a bad registry entry, not something
119+
to silently resolve one way or the other."""
120+
merged: dict[str, object] = {}
121+
for layer in layers:
122+
for key, value in layer.launcher_hints.items():
123+
if key in merged and merged[key] != value:
124+
raise ValueError(
125+
f"conflicting launcher_hint {key!r} ({merged[key]!r} vs {value!r}) "
126+
f"across layers merged into one launcher: {[l.type for l in layers]}"
127+
)
128+
merged[key] = value
129+
return merged
130+
131+
132+
def _merge_docker_layers(layers: Sequence[Layer]) -> DockerLauncher:
133+
"""One DockerLauncher for every DOCKER_* layer in ``layers`` -- one
134+
``docker run`` imposing every axis declared (``--memory``/``--memory-swap``
135+
together with ``--cpus`` or ``--cpuset-cpus``), never a second, nested
136+
``docker run`` for the CPU axis."""
137+
kwargs: dict[str, object] = _merge_hints(layers)
138+
for layer in layers:
139+
if layer.type is LayerType.DOCKER_MEMORY:
140+
kwargs[_MEMORY_KWARG] = layer.imposed
141+
elif layer.type is LayerType.DOCKER_CPU_QUOTA:
142+
kwargs[_CPU_KWARG] = layer.imposed
143+
elif layer.type is LayerType.DOCKER_CPUSET:
144+
kwargs["cpuset_millicores"] = layer.imposed
145+
return DockerLauncher(**kwargs)
146+
147+
148+
def _merge_nested_cgroup_layers(layers: Sequence[Layer]) -> NestedCgroupLauncher:
149+
"""One NestedCgroupLauncher for every NESTED_CGROUP_* layer in ``layers``
150+
-- one script that mkdir's the nested cgroup once, probes it once, and
151+
writes both ``memory.max``/``memory.swap.max`` and ``cpu.max`` to it,
152+
never two independent scripts racing to create/write the same path."""
153+
kwargs: dict[str, object] = {}
154+
probe_key: str | None = None
155+
for layer in layers:
156+
if layer.probe_key:
157+
if probe_key is not None and probe_key != layer.probe_key:
158+
raise ValueError(
159+
f"conflicting probe_key across merged nested-cgroup layers: {probe_key!r} vs {layer.probe_key!r}"
160+
)
161+
probe_key = layer.probe_key
162+
if layer.type is LayerType.NESTED_CGROUP_MEMORY:
163+
kwargs[_MEMORY_KWARG] = layer.imposed
164+
elif layer.type is LayerType.NESTED_CGROUP_CPU:
165+
kwargs[_CPU_KWARG] = layer.imposed
166+
if probe_key:
167+
kwargs["probe_key"] = probe_key
168+
return NestedCgroupLauncher(**kwargs)
169+
170+
171+
def build_launcher_groups(scenario: Scenario) -> list[tuple[Launcher, tuple[Layer, ...]]]:
172+
"""Group ``scenario.layers`` by enforcement-context family, merge each
173+
multi-axis family (docker, nested-cgroup) onto one launcher instance, and
174+
return ``(launcher, layers_it_covers)`` pairs outermost-first -- ordered by
175+
each family's *first* occurrence in ``scenario.layers``, so genuinely
176+
nested contexts (container -> nested cgroup -> rlimit -> payload) still
177+
compose outside-in via :func:`~tests.e2e.launchers.base.compose_argv`.
178+
179+
Every layer type outside the two merging families is its own singleton
180+
group: one launcher per layer, exactly as before this fix.
181+
"""
182+
groups: dict[str, list[Layer]] = {}
183+
order: list[str] = []
184+
for layer in scenario.layers:
185+
family = _layer_family(layer)
186+
if family not in groups:
187+
groups[family] = []
188+
order.append(family)
189+
groups[family].append(layer)
190+
191+
result: list[tuple[Launcher, tuple[Layer, ...]]] = []
192+
for family in order:
193+
group = tuple(groups[family])
194+
if family == _FAMILY_DOCKER:
195+
result.append((_merge_docker_layers(group), group))
196+
elif family == _FAMILY_NESTED_CGROUP:
197+
result.append((_merge_nested_cgroup_layers(group), group))
198+
else:
199+
assert len(group) == 1, f"unexpected multiple layers grouped under singleton family {family!r}: {group}"
200+
result.append((build_launcher(group[0]), group))
201+
return result
202+
203+
79204
def build_launchers(scenario: Scenario) -> list[Launcher]:
80-
"""One launcher per layer, outermost-first, matching scenario.layers order."""
81-
return [build_launcher(layer) for layer in scenario.layers]
205+
"""One launcher per *enforcement context* (same-mechanism, multi-axis
206+
layers merged -- see module docstring), outermost-first. Use
207+
:func:`build_launcher_groups` when the caller also needs to know which
208+
original layers a given launcher covers (e.g. to fold probe results back
209+
per layer)."""
210+
return [launcher for launcher, _layers in build_launcher_groups(scenario)]

tests/e2e/launchers/kubernetes.py

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,23 +45,62 @@ def __init__(
4545
self.namespace = namespace
4646

4747
def probe(self) -> ProbeResult:
48+
"""Structurally confirm cgroup v2 delegation *into the kind node*,
49+
not just that the cluster is reachable (iter-2 fix: the previous
50+
implementation only ran ``kubectl get nodes`` -- general cluster
51+
liveness -- which can report a fully Ready cluster even when cgroup
52+
delegation into the node itself is broken, e.g. a cgroup-driver
53+
mismatch or a controller not delegated).
54+
55+
A kind "node" is itself a docker container (the container's hostname
56+
matches the Kubernetes node name by kind's own convention), so its own
57+
``/sys/fs/cgroup/cgroup.controllers`` can be inspected the same way
58+
:class:`~tests.e2e.launchers.podman.PodmanLauncher.probe` inspects
59+
podman's host cgroup version: a cheap, structural presence check,
60+
never a behavioral one.
61+
"""
4862
if shutil.which("kubectl") is None:
4963
return ProbeResult(False, "kubectl binary not found on PATH")
5064
try:
5165
nodes = subprocess.run(
52-
["kubectl", "get", "nodes", "-o", "json"], capture_output=True, text=True, timeout=30,
66+
["kubectl", "get", "nodes", "-o", "jsonpath={.items[0].metadata.name}"],
67+
capture_output=True, text=True, timeout=30,
5368
)
5469
except (OSError, subprocess.TimeoutExpired) as exc:
5570
return ProbeResult(False, f"kubectl get nodes failed: {exc}")
5671
if nodes.returncode != 0:
5772
return ProbeResult(False, f"kubectl get nodes exited {nodes.returncode}: {nodes.stderr.strip()}")
58-
try:
59-
parsed = json.loads(nodes.stdout)
60-
except json.JSONDecodeError as exc:
61-
return ProbeResult(False, f"kubectl get nodes returned invalid JSON: {exc}")
62-
if not parsed.get("items"):
73+
node_name = nodes.stdout.strip()
74+
if not node_name:
6375
return ProbeResult(False, "kind cluster reports zero nodes -- cgroup delegation not confirmed")
64-
return ProbeResult(True, "kind cluster reachable and reports at least one Ready node")
76+
77+
if shutil.which("docker") is None:
78+
return ProbeResult(
79+
False, "docker binary not found on PATH; cannot inspect the kind node's cgroup delegation"
80+
)
81+
try:
82+
controllers = subprocess.run(
83+
["docker", "exec", node_name, "cat", "/sys/fs/cgroup/cgroup.controllers"],
84+
capture_output=True, text=True, timeout=30,
85+
)
86+
except (OSError, subprocess.TimeoutExpired) as exc:
87+
return ProbeResult(False, f"docker exec into kind node {node_name!r} failed: {exc}")
88+
if controllers.returncode != 0:
89+
return ProbeResult(
90+
False,
91+
f"docker exec into kind node {node_name!r} exited {controllers.returncode}: "
92+
f"{controllers.stderr.strip()} -- cgroup delegation not confirmed",
93+
)
94+
listed = controllers.stdout.split()
95+
if "memory" not in listed or "cpu" not in listed:
96+
return ProbeResult(
97+
False,
98+
f"kind node {node_name!r} cgroup.controllers is missing memory/cpu delegation: "
99+
f"{controllers.stdout.strip()!r}",
100+
)
101+
return ProbeResult(
102+
True, f"kind node {node_name!r} cgroup.controllers lists memory and cpu; delegation structurally confirmed"
103+
)
65104

66105
def _resource_limits(self) -> dict[str, str]:
67106
if self.memory_bytes is not None:

tests/e2e/launchers/nested_cgroup.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,15 @@
2020
This layer's memory.swap.max=0 is set independently of the container's own
2121
``--memory-swap`` -- it does NOT inherit that setting (design decision #13
2222
explicit requirement).
23+
24+
``memory_bytes`` and ``cpu_millicores`` may be supplied *together*: the
25+
triple-stack scenarios declare a separate ``NESTED_CGROUP_MEMORY`` layer and a
26+
separate ``NESTED_CGROUP_CPU`` layer for the same nested cgroup, and
27+
:mod:`tests.e2e.launchers.factory` merges both onto **one**
28+
``NestedCgroupLauncher`` instance -- one ``mkdir``/probe/write script setting
29+
both ``memory.max``+``memory.swap.max`` and ``cpu.max`` on the same nested
30+
cgroup, never two independent scripts racing to create/write the same path
31+
(iter-2 fix for the "two nested-cgroup scripts" composition blocker).
2332
"""
2433

2534
from __future__ import annotations
@@ -40,8 +49,8 @@ def __init__(
4049
cpu_millicores: int | None = None,
4150
probe_key: str = "nested-cgroup-write-access",
4251
):
43-
if (memory_bytes is None) == (cpu_millicores is None):
44-
raise ValueError("NestedCgroupLauncher takes exactly one of memory_bytes/cpu_millicores")
52+
if memory_bytes is None and cpu_millicores is None:
53+
raise ValueError("NestedCgroupLauncher requires at least one of memory_bytes/cpu_millicores")
4554
self.memory_bytes = memory_bytes
4655
self.cpu_millicores = cpu_millicores
4756
self.probe_key = probe_key
@@ -85,7 +94,7 @@ def wrap_argv(self, inner_argv: Sequence[str]) -> list[str]:
8594
if self.memory_bytes is not None:
8695
lines.append(f"echo {self.memory_bytes} > {_CHILD_CGROUP}/memory.max")
8796
lines.append(f"echo 0 > {_CHILD_CGROUP}/memory.swap.max")
88-
else:
97+
if self.cpu_millicores is not None:
8998
quota_us = int(self.cpu_millicores * _CFS_PERIOD_US / 1000)
9099
lines.append(f"echo '{quota_us} {_CFS_PERIOD_US}' > {_CHILD_CGROUP}/cpu.max")
91100
lines.append(f"echo $$ > {_CHILD_CGROUP}/cgroup.procs")

0 commit comments

Comments
 (0)