diff --git a/datadog/dogstatsd/container.py b/datadog/dogstatsd/container.py index 076de1501..26a9e974c 100644 --- a/datadog/dogstatsd/container.py +++ b/datadog/dogstatsd/container.py @@ -47,10 +47,13 @@ class Cgroup(object): def __init__(self): # type: () -> None - if self._is_host_cgroup_namespace(): - self.container_id = self._read_cgroup_path() - return - self.container_id = self._get_cgroup_from_inode() + # Always attempt to parse the container ID from the cgroup path first. + # Only fall back to the inode approach when the path yields nothing and we + # are not in the host cgroup namespace (i.e. we are in a container running + # with a private cgroup namespace, typical of cgroup v2 setups). + self.container_id = self._read_cgroup_path() + if self.container_id is None and not self._is_host_cgroup_namespace(): + self.container_id = self._get_cgroup_from_inode() def _is_host_cgroup_namespace(self): # type: () -> bool @@ -90,30 +93,37 @@ def _read_cgroup_path(self): def _get_cgroup_from_inode(self): # type: () -> Optional[str] """Read the container ID from the cgroup inode.""" - # Parse /proc/self/cgroup and get a map of controller to its associated cgroup node path. - cgroup_controllers_paths = {} - with open(self.CGROUP_PATH, mode="r") as fp: - for line in fp: - tokens = line.strip().split(":") - if len(tokens) != 3: - continue - if tokens[1] == self.CGROUPV1_BASE_CONTROLLER or tokens[1] == self.CGROUPV2_BASE_CONTROLLER: - cgroup_controllers_paths[tokens[1]] = tokens[2] - - # Retrieve the cgroup inode from "/sys/fs/cgroup + controller + cgroupNodePath" - for controller in [ - self.CGROUPV1_BASE_CONTROLLER, - self.CGROUPV2_BASE_CONTROLLER, - ]: - if controller in cgroup_controllers_paths: - inode_path = os.path.join( - self.CGROUP_MOUNT_PATH, - controller, - cgroup_controllers_paths[controller] if cgroup_controllers_paths[controller] != "/" else "", - ) - inode = os.stat(inode_path).st_ino - # 0 is not a valid inode. 1 is a bad block inode and 2 is the root of a filesystem. - if inode > 2: - return "in-{0}".format(inode) + try: + # Parse /proc/self/cgroup and get a map of controller to its associated cgroup node path. + cgroup_controllers_paths = {} + with open(self.CGROUP_PATH, mode="r") as fp: + for line in fp: + tokens = line.strip().split(":") + if len(tokens) != 3: + continue + if tokens[1] == self.CGROUPV1_BASE_CONTROLLER or tokens[1] == self.CGROUPV2_BASE_CONTROLLER: + cgroup_controllers_paths[tokens[1]] = tokens[2] + + # Retrieve the cgroup inode from "/sys/fs/cgroup + controller + cgroupNodePath". + # Use lstrip("/") so that absolute node paths (e.g. "/docker/abc123") are treated + # as relative when joined with the mount base, preventing os.path.join from silently + # discarding the mount prefix. + for controller in [ + self.CGROUPV1_BASE_CONTROLLER, + self.CGROUPV2_BASE_CONTROLLER, + ]: + if controller in cgroup_controllers_paths: + node_path = cgroup_controllers_paths[controller] + inode_path = os.path.join( + self.CGROUP_MOUNT_PATH, + controller, + node_path.lstrip("/"), + ) + inode = os.stat(inode_path).st_ino + # 0 is not a valid inode. 1 is a bad block inode and 2 is the root of a filesystem. + if inode > 2: + return "in-{0}".format(inode) + except Exception: + pass return None diff --git a/tests/unit/dogstatsd/test_container.py b/tests/unit/dogstatsd/test_container.py index c97d80784..bdb742a83 100644 --- a/tests/unit/dogstatsd/test_container.py +++ b/tests/unit/dogstatsd/test_container.py @@ -106,6 +106,14 @@ def get_mock_open(read_data=None): """, "ci-34dc0b5e626f2c5c4c5170e34b10e765-1234567890", ), + # K8s systemd cgroup v1 with .slice/.scope hierarchy + # Container ID is embedded as "docker-{id}.scope" at the leaf + ( + """ +1:name=systemd:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-pod2d3da189_6407_48e3_9ab6_78188d75e609.slice/docker-3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860.scope + """, + "ci-3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860", + ), # Linux non-containerized file ( """ @@ -143,7 +151,9 @@ def test_container_id_inode(): with mock.patch("os.stat", mock.MagicMock(return_value=mock.Mock(st_ino=1234))): reader = Cgroup() assert reader.container_id == "in-1234" - mock_open.assert_called_once_with("/proc/self/cgroup", mode="r") + # open is called twice: once in _read_cgroup_path (returns None) and once in _get_cgroup_from_inode. + assert mock_open.call_count == 2 + mock_open.assert_called_with("/proc/self/cgroup", mode="r") cgroupv1_priority = """ 12:cpu,cpuacct:/ @@ -180,4 +190,142 @@ def inode_stat_mock(path): "/sys/fs/cgroup/memory/", "/sys/fs/cgroup/" ] - mock_open.assert_called_once_with("/proc/self/cgroup", mode="r") + # open is called twice: once in _read_cgroup_path (returns None) and once in _get_cgroup_from_inode. + assert mock_open.call_count == 2 + mock_open.assert_called_with("/proc/self/cgroup", mode="r") + + +def test_container_id_inode_absolute_node_path(): + """ + Test that inode lookup constructs the correct filesystem path when the cgroup + node path is an absolute non-root path (e.g. "/system.slice/docker-xyz.scope"). + + os.path.join silently discards all preceding components when it encounters an + absolute path segment. Without lstrip("/") the joined path would lose the + cgroup mount prefix, e.g.: + + os.path.join("/sys/fs/cgroup", "", "/system.slice/foo") == "/system.slice/foo" + + With lstrip("/") the correct path is produced: + + os.path.join("/sys/fs/cgroup", "", "system.slice/foo") == "/sys/fs/cgroup/system.slice/foo" + + The scope name intentionally does NOT contain a 64-char hex container ID so + that _read_cgroup_path() returns None and we exercise the inode fallback. + """ + # cgroup v2 unified hierarchy: path is an absolute non-root scope with no parseable container ID + cgroup_contents = "0::/system.slice/docker-deadbeef.scope\n" + expected_inode_path = "/sys/fs/cgroup/system.slice/docker-deadbeef.scope" + + paths_checked = [] + + def inode_stat_mock(path): + paths_checked.append(path) + if path == expected_inode_path: + return mock.Mock(st_ino=42) + return mock.Mock(st_ino=0) + + with mock.patch("datadog.dogstatsd.container.open", mock.mock_open(read_data=cgroup_contents)): + with mock.patch("os.stat", mock.MagicMock(side_effect=inode_stat_mock)): + reader = Cgroup() + + assert reader.container_id == "in-42" + assert expected_inode_path in paths_checked, ( + "Expected the inode to be looked up under the cgroup mount; " + "got paths: {}".format(paths_checked) + ) + + +def test_container_id_cgroup_path_takes_priority_over_inode(): + """ + When the cgroup path contains a parseable container ID, it should be returned + even when the process is *not* in the host cgroup namespace. The inode + fallback is only used when the path yields nothing. + """ + container_id = "3e74d3fd9db4c9dd921ae05c2502fb984d0cde1b36e581b13f79c639da4518a1" + cgroup_contents = "5:memory:/kubepods/besteffort/pod123/{}\n0::/\n".format(container_id) + + stat_calls = [] + + def stat_mock(path): + stat_calls.append(path) + # Simulate a private (non-host) cgroup namespace inode + if path == Cgroup.CGROUP_NS_PATH: + return mock.Mock(st_ino=9999) + return mock.Mock(st_ino=42) + + with mock.patch("datadog.dogstatsd.container.open", mock.mock_open(read_data=cgroup_contents)): + with mock.patch("os.path.exists", return_value=True): + with mock.patch("os.stat", mock.MagicMock(side_effect=stat_mock)): + reader = Cgroup() + + assert reader.container_id == "ci-{}".format(container_id) + # Inode lookup paths should not have been consulted + inode_paths = [p for p in stat_calls if p != Cgroup.CGROUP_NS_PATH] + assert inode_paths == [], "Unexpected inode lookup calls: {}".format(inode_paths) + + +# --------------------------------------------------------------------------- +# _is_host_cgroup_namespace +# --------------------------------------------------------------------------- + + +def test_is_host_cgroup_namespace_returns_false_when_file_missing(): + """When /proc/self/ns/cgroup does not exist the method returns False.""" + reader = Cgroup.__new__(Cgroup) + with mock.patch("os.path.exists", return_value=False): + assert reader._is_host_cgroup_namespace() is False + + +def test_is_host_cgroup_namespace_returns_true_when_inode_matches(): + reader = Cgroup.__new__(Cgroup) + with mock.patch("os.path.exists", return_value=True): + with mock.patch("os.stat", return_value=mock.Mock(st_ino=Cgroup.HOST_CGROUP_NAMESPACE_INODE)): + assert reader._is_host_cgroup_namespace() is True + + +def test_is_host_cgroup_namespace_returns_false_when_inode_differs(): + reader = Cgroup.__new__(Cgroup) + with mock.patch("os.path.exists", return_value=True): + with mock.patch("os.stat", return_value=mock.Mock(st_ino=9999)): + assert reader._is_host_cgroup_namespace() is False + + +def test_is_host_cgroup_namespace_returns_false_on_exception(): + """Any OS error during namespace check should not propagate.""" + reader = Cgroup.__new__(Cgroup) + with mock.patch("os.path.exists", side_effect=PermissionError("denied")): + assert reader._is_host_cgroup_namespace() is False + + +# --------------------------------------------------------------------------- +# _get_cgroup_from_inode: cgroup v1 inode=0 falls through to v2 controller +# --------------------------------------------------------------------------- + + +def test_container_id_inode_v1_falls_through_to_v2_when_v1_inode_is_zero(): + """ + When the cgroup v1 memory controller inode is 0 (invalid), _get_cgroup_from_inode + must fall through to the cgroup v2 (CGROUPV2_BASE_CONTROLLER="") entry. + """ + # cgroup file with both a v1 memory controller and a cgroup v2 unified entry + cgroup_contents = "7:memory:/\n0::/\n" + + paths_checked = [] + + def inode_stat_mock(path): + paths_checked.append(path) + if path == "/sys/fs/cgroup/memory/": + # Inode 0 is invalid — should be skipped + return mock.Mock(st_ino=0) + if path == "/sys/fs/cgroup/": + return mock.Mock(st_ino=5678) + return mock.Mock(st_ino=0) + + with mock.patch("datadog.dogstatsd.container.open", mock.mock_open(read_data=cgroup_contents)): + with mock.patch("os.stat", mock.MagicMock(side_effect=inode_stat_mock)): + reader = Cgroup() + + assert reader.container_id == "in-5678" + assert "/sys/fs/cgroup/memory/" in paths_checked, "v1 memory path should have been checked" + assert "/sys/fs/cgroup/" in paths_checked, "v2 root path should have been checked as fallback"