build: Convert to module

[Mantisus]

Closes: #1
Closes: #2

[Pijukatel]

This is really great. It is a big change, and I want to review it carefully, so I will do it in steps over several days. Some comments are more like open questions, not necessarily a change request.

This is the first-step review:
Public documentation + test result glance
(without looking at internal implementation)

Follow-up reviews will be:

• Interface and usage
• Implementation internals
• Test internals
• CI

[Pijukatel]

For this and some other reference values, why not use psutils where we know it correctly estimates?

[Mantisus]

Ideally, I'd like to keep this package zero-dependency. That guarantees it runs anywhere Python does. psutil ships a C extension, so it needs a prebuilt wheel matching the platform, or a compiler on the machine to build one.

[Pijukatel]

Have you considered using explicit Pytest markers for tests?
https://docs.pytest.org/en/stable/example/markers.html#marking-test-functions-and-selecting-them-for-a-run

The reader of the test suite can be surprised by entries like this and wonder whether it is missing some coverage due to runners not being set up correctly or if this is the desired behavior:

SKIPPED [1] tests/e2e/test_docker.py:174: docker uses the 'cgroupfs' cgroup driver; this needs systemd
SKIPPED [1] tests/e2e/test_kubernetes.py:182: kind, kubectl and docker are needed to run a cluster
SKIPPED [1] tests/e2e/test_kubernetes.py:202: kind, kubectl and docker are needed to run a cluster
SKIPPED [1] tests/e2e/test_machine.py:50: the controllers are not on the unified hierarchy

If it is known in advance which environment has which capability, then we should be able to call just the right subset of the tests through markers.

[Mantisus]

Great idea!

[Pijukatel]

Off Linux every limit reads as None

Should we rather raise when not running in the expected environment for now?

Could you also elaborate on the environment, like a Linux container that is running through different mechanisms on MacOS or Windows?

[Mantisus]

hould we rather raise when not running in the expected environment for now?

None says there are no limits, or that they could not be determined.On an unsupported environment that is the correct answer.

We can add an is_supported() function as a simple way to check whether the current environment supports cgroups.

Could you also elaborate on the environment, like a Linux container that is running through different mechanisms on MacOS or Windows?

This is about macOS and Windows themselves. A Linux container should work correctly wherever it runs. With one correction: the machine-wide values come from the virtual machine, not from the physical host.

[Pijukatel]

What is the reasoning behind memory.total, memory.total - memory.available not being the default(when there are no tighther restrictions) of budget.limit, budget.working_set? Is it to avoid dependencies?

or if we keep it this way, could we turn def memory_budget() into a utility function closed behind psutils as an optional dependency?

I can imagine this or a similar utility function would be repeated by a huge number of users.

(Ok, here I am going wild and just throwing ideas. We could also keep this package completely clear of dependencies and later add a second package like "psutils_cgropups_aware" which would be thin wrapper around the original psutils, delegating almost everything back to psutils and patching just some return values using new functionality you have introduced here. The motivation would be to have a "drop-in replacement" like import psutils_cgropups_aware as psutils)

[Pijukatel]

From just this description, I could not understand whether the load.sample() is measuring against its previous call or what it is actually measuring. I need to go inside the function to understand it. Could you please try to explain it more in this readme so that the readers do not need to check the internals for top level description. (Maybe it is just me being slow though :D )

[Pijukatel]

The levels searched are not the levels a reading came from This sounds strange in isolation, could you please further explain or rephrase it.

My dumb interpretation without knowing the internals yet. "We searched some levels, but they are unrelated to the reading :-)"

[Pijukatel]

"at least the memory of the machine" , does it mean that memory limit can be higher than that?

Maybe something like "the limit is limited(or saturated) by the memory of the machine" ?

[Pijukatel]

It would be great if each notice carried at least one real-world example of when such a notice can arrive

[Pijukatel]

Haha, I am so curious about an example of when this happens. I want to know what the unknown enforcement is :-)

[Mantisus]

printf 'not a number\n' > /tmp/fake-memory-max && docker run --rm --memory 512m \
  -v "$PWD/src:/sensor:ro" -e PYTHONPATH=/sensor \
  -v /tmp/fake-memory-max:/sys/fs/cgroup/memory.max:ro \
  python:3.13-alpine python -c '
import dataclasses, json, cgroups_sensor
print(json.dumps(dataclasses.asdict(cgroups_sensor.describe()), indent=2, default=str))'

[Pijukatel]

Let's keep it as a working placeholder name for now, but have an internal debate with the rest of the team before publication
(I am also bad with names)

[Mantisus]

Unless we find something better, my suggestion is synomys or prairidog (уeah, that's the best I could come up with).

[Pijukatel]

A few more comments.

I also asked Claude to come up with scenarios where estimation is not working as expected. It did not come up with any normal scenario, just some edge cases:

"""Regressions: readings that go wrong on layouts found in the wild.

The first two pin the hybrid machine whose unified hierarchy carries a stray controller. A kernel offers
controllers younger than the systemd that mounts the cgroup v1 hierarchies (`misc`, `rdma`, `hugetlb`), so
they stay bound to the otherwise controller-less cgroup2 - `cgroup.controllers` there is then not empty.
The CPU limits still live on cgroup v1, and a runtime with the cgroupfs driver names its groups only there,
so the time counted on the unified hierarchy belongs to another scope entirely.
"""

from __future__ import annotations

from typing import TYPE_CHECKING

import pytest

import cgroups_sensor

from .conftest import HYBRID_MOUNTINFO, HYBRID_SELF_CGROUP
from .test_sensor import fake_time, notice_codes

if TYPE_CHECKING:
    from collections.abc import Callable
    from pathlib import Path


HYBRID_STRAY_FILES = {
    # The stray controller: the only thing bound to the unified hierarchy.
    'unified/cgroup.controllers': 'hugetlb\n',
    # systemd tracks this process as the docker service on the unified hierarchy, so the time counted
    # there is the daemon's scope, not the container's.
    'unified/system.slice/docker.service/cpu.stat': 'usage_usec 999000000\n',
    'unified/cpu.stat': 'usage_usec 999000000\n',
    # The quota and the time of the container, both on cgroup v1 where the runtime wrote them.
    'cpu,cpuacct/docker/abc/cpu.cfs_quota_us': '50000\n',
    'cpu,cpuacct/docker/abc/cpu.cfs_period_us': '100000\n',
    'cpu,cpuacct/docker/abc/cpuacct.usage': '3000000000\n',
}
"""A hybrid layout with `hugetlb` riding the unified mount and the CPU metrics on cgroup v1."""


def test_get_cpu_usage_hybrid_stray_controller_reads_the_quota_interface(
    fake_cgroup: Callable[..., Path],
) -> None:
    """Reads the time where the limits live, not on a unified hierarchy that merely carries some controller.

    `cpu.stat` exists on every cgroup v2 group whether or not the CPU controller is bound there, so a stray
    `hugetlb` must not make it the source: the group it names belongs to the service, and its counter says
    999 seconds where the container's `cpuacct` says 3.
    """
    root = fake_cgroup(
        mountinfo=HYBRID_MOUNTINFO,
        self_cgroup=HYBRID_SELF_CGROUP,
        files=HYBRID_STRAY_FILES,
    )

    assert cgroups_sensor.get_cpu_usage() == pytest.approx(3.0)

    description = cgroups_sensor.describe()
    assert description.cpu_usage_source is not None
    assert description.cpu_usage_source.interface is cgroups_sensor.Interface.CGROUP_V1
    # The rate has to be measurable where the quota binds, and the comount carries the counter right there.
    assert description.cpu_rate_level == str(root / 'cpu,cpuacct' / 'docker' / 'abc')


def test_get_cpu_used_ratio_hybrid_stray_controller(
    fake_cgroup: Callable[..., Path], monkeypatch: pytest.MonkeyPatch
) -> None:
    """Measures the rate against the cgroup v1 quota, instead of reporting a scope mismatch.

    The counter sits next to the quota in the `cpu,cpuacct` comount. Translating the quota's level into the
    unified hierarchy and giving up when it is not there loses a rate that was one file away.
    """
    root = fake_cgroup(
        mountinfo=HYBRID_MOUNTINFO,
        self_cgroup=HYBRID_SELF_CGROUP,
        files=HYBRID_STRAY_FILES,
    )

    def advance(_seconds: float) -> None:
        # A quarter of a CPU second over one second of the fake clock, against half a core: half the allowance.
        (root / 'cpu,cpuacct' / 'docker' / 'abc' / 'cpuacct.usage').write_text('3250000000\n')

    fake_time(monkeypatch, sleep=advance)

    assert cgroups_sensor.get_cpu_used_ratio() == pytest.approx(0.5)
    assert notice_codes('cpu') == ()


def test_describe_subtree_mount_names_the_hidden_ancestors(fake_cgroup: Callable[..., Path]) -> None:
    """Says that levels above the mount exist and cannot be read, the way an unreadable file already does.

    A cgroup v1 runtime exposes only the container's own subtree: the mount root is the container's path. A
    limit on an ancestor - a tenant slice, `--cgroup-parent`, the pod - then binds without being visible, and
    the kernel kills at a cap this process was told does not exist. The values are unknowable, but their
    possibility is not: the mount root says levels were cut off, so a notice has to say so too.
    """
    fake_cgroup(
        mountinfo='30 25 0:26 /tenant/app {root}/memory rw,nosuid shared:14 - cgroup cgroup rw,memory',
        self_cgroup='2:memory:/tenant/app\n',
        files={
            'memory/memory.limit_in_bytes': '9223372036854771712\n',
            'memory/memory.usage_in_bytes': '1000\n',
            'memory/memory.stat': 'total_inactive_file 400\n',
        },
    )

    assert 'mount-hides-ancestors' in notice_codes()

[Pijukatel]

These values confuse me. Child consuming more than the parents' limit. Isn't this an impossible scenario?

Can we rewrite these values to prove the behavior in a real scenario?

[Pijukatel]

This makes me confused

...'memory', v2_probe=_V2.memory_usage, v1_probe=_V1.memory_usage...

Here it looks like coincidental coupling, while it should be an implication, or?

v2_probe maps to exactly one v1_probe and exactly one v1_name

Maybe the arguments to this function could be reduced and some relations hard-coded to avoid this illusion of flexibility.

[Pijukatel]

_read_hierarchies has a quite complex return type. Why not just return
unified as another key in the hierarchy dict?

[Pijukatel]

Together with some other comments, I think that it could be sufficient to have just two arguments:

hierarchies: dict[str, _Hierarchy],
v1_name: str,

where unified can be trivially extracted from hierarchies and v1_name implies v2_probe and v1_probe

[Pijukatel]

count_cpu_list("0,0") returns 2
or
count_cpu_list("0-2, 1-3") returns 6

The Kernel can still understand those ill-defined sets correctly.

Maybe we should internally try to create such a set to handle duplicates
cpus=set(...)
and just return its length

[Pijukatel]

Seems like exactly the same test as the one on line 890 test_read_memory_ancestor_without_usage_drops_the_distance , with just different comments