fix(agent): scan only root mount in is_container() cgroup-v2 fallback
Closes #58135 On a cgroup-v2 host with Docker's containerd image store, is_container() false-positives whenever any container is running. The mountinfo fallback scanned the entire file for containerd/crio/kubepods substrings, but each running container contributes an overlay mount whose option string carries lowerdir=/var/lib/containerd/..., so a plain host was misclassified as a container. The result is cached per-process, making it depend on whether a container happened to be running at first call — which then flipped subprocess HOME and broke browser tool launches (Chrome not found). Fix: only inspect the root ('/') mount line. Inside a container the root mount is the runtime's overlay/snapshot and carries the marker; on a host the root is a real block device and container overlays live at non-root mount points. Adds a regression test reproducing the host-running-containers case. (cherry picked from commit 588f55113ebc16813b8216772feb761b6292d922) (cherry picked from commit f42295814d7e2b5fe681e6d1b96ea72026058195)
This commit is contained in:
@@ -1059,8 +1059,23 @@ def _detect_container() -> bool:
|
||||
or _proc_file_has_marker("/proc/1/cgroup", ("docker", "podman", "/lxc/", "kubepods", "containerd", "crio"))
|
||||
):
|
||||
return True
|
||||
# cgroup v2: /proc/1/cgroup is just "0::/"; the runtime still shows in mountinfo.
|
||||
return _proc_file_has_marker("/proc/self/mountinfo", ("kubepods", "containerd", "crio"))
|
||||
# cgroup v2: /proc/1/cgroup is just "0::/"; the runtime still shows in mountinfo — but ONLY on
|
||||
# the root ("/") mount line. A host that merely *runs* containers exposes every container's
|
||||
# overlay lowerdir (``lowerdir=/var/lib/containerd/...``) at non-root mount points, which a
|
||||
# whole-file scan misread as "inside a container" and flipped subprocess HOME (#58135).
|
||||
return _root_mount_has_marker("/proc/self/mountinfo", ("kubepods", "containerd", "crio"))
|
||||
|
||||
|
||||
def _root_mount_has_marker(path: str, markers: tuple[str, ...]) -> bool:
|
||||
try:
|
||||
with open(path, "r", encoding="utf-8") as f:
|
||||
for line in f:
|
||||
fields = line.split() # mountinfo field 5 (index 4) is the mount point
|
||||
if len(fields) >= 5 and fields[4] == "/":
|
||||
return any(marker in line for marker in markers)
|
||||
except OSError:
|
||||
pass
|
||||
return False
|
||||
|
||||
|
||||
def get_config_path() -> Path:
|
||||
|
||||
@@ -363,6 +363,44 @@ class TestIsContainer:
|
||||
|
||||
|
||||
|
||||
def test_host_running_containers_not_false_positive(self, monkeypatch, tmp_path):
|
||||
"""A host that merely RUNS containers must not be classified as one.
|
||||
|
||||
Regression for NousResearch/hermes-agent#58135: on a cgroup-v2 host
|
||||
with Docker's containerd image store, each running container adds an
|
||||
overlay mount whose option string contains
|
||||
``lowerdir=/var/lib/containerd/...``. The marker appears only in
|
||||
non-root mount lines, so scanning the whole file produced a false
|
||||
positive. Only the root ('/') mount line should be inspected.
|
||||
"""
|
||||
import builtins
|
||||
self._reset_cache(monkeypatch)
|
||||
monkeypatch.delenv("KUBERNETES_SERVICE_HOST", raising=False)
|
||||
monkeypatch.setattr(os.path, "exists", lambda p: False)
|
||||
cgroup_file = tmp_path / "cgroup"
|
||||
cgroup_file.write_text("0::/\n") # cgroup v2 — no runtime marker
|
||||
mountinfo_file = tmp_path / "mountinfo"
|
||||
mountinfo_file.write_text(
|
||||
# Root is a real block device on the host.
|
||||
"25 1 259:2 / / rw,relatime shared:1 - ext4 /dev/nvme0n1p2 rw\n"
|
||||
# A running container's overlay rootfs mounted elsewhere — its
|
||||
# lowerdir references containerd but must NOT flip the host.
|
||||
"469 554 0:94 / /var/lib/docker/rootfs/overlayfs/7dda83 rw,relatime "
|
||||
"shared:247 - overlay overlay rw,lowerdir=/var/lib/containerd/"
|
||||
"io.containerd.snapshotter.v1.overlayfs/snapshots/33509/fs\n"
|
||||
)
|
||||
_real_open = builtins.open
|
||||
|
||||
def _fake_open(p, *a, **kw):
|
||||
if p == "/proc/1/cgroup":
|
||||
return _real_open(str(cgroup_file), *a, **kw)
|
||||
if p == "/proc/self/mountinfo":
|
||||
return _real_open(str(mountinfo_file), *a, **kw)
|
||||
return _real_open(p, *a, **kw)
|
||||
|
||||
monkeypatch.setattr("builtins.open", _fake_open)
|
||||
assert is_container() is False
|
||||
|
||||
def test_caches_result(self, monkeypatch):
|
||||
"""Second call uses cached value without re-probing."""
|
||||
monkeypatch.setattr(hermes_constants, "_container_detected", True)
|
||||
|
||||
Reference in New Issue
Block a user