Repository: pre-commit/pre-commit

Issue:
Failure to get the container id
With #1888 in place, there's a regression when running inside a container. There's an assumption in https://github.com/pre-commit/pre-commit/blob/master/pre_commit/languages/docker.py#L32 that the hostname is the container ID which is not always the case (it's easy enough to set a different hostname with `docker run --hostname foo`). It causes the `docker inspect` command that follows to fail. A more reliable way to get the container id is from `/proc/1/cpuset` or from the first line in `/proc/1/cgroup` which is already checked in `_is_in_docker`.

Thanks @asottile and @okainov for your work on #1387 and pre-commit in general.

Discussion:
asottile: what happens if the container is configured with cgroup options? does that affect this value

also tests need to be fixed
adarnimrod: If I understand your question correctly, it doesn't matter if the container is created with or without setting any resource limits, the control group is created anyway. From my experience, this is the most reliable way to get the container id from inside the container itself. I'll see what needs to be fixed in the tests. Thanks for the quick reply.
asottile: I meant more https://docs.docker.com/engine/reference/run/#specify-custom-cgroups -- does this not affect the contents of `/proc/1/cgroup` ?
adarnimrod: I did the following test:
```
$ docker run --rm --cgroup-parent foo alpine:3.13 cat /proc/1/cgroup
12:freezer:/foo/1c7532a506b51e4072102771d0b0cc30e2300f3f539cdd6e3d8b3046275bb79f
11:perf_event:/foo/1c7532a506b51e4072102771d0b0cc30e2300f3f539cdd6e3d8b3046275bb79f
10:blkio:/system.slice/containerd.service/foo/1c7532a506b51e4072102771d0b0cc30e2300f3f539cdd6e3d8b3046275bb79f
9:hugetlb:/foo/1c7532a506b51e4072102771d0b0cc30e2300f3f539cdd6e3d8b3046275bb79f
8:pids:/system.slice/containerd.service/foo/1c7532a506b51e4072102771d0b0cc30e2300f3f539cdd6e3d8b3046275bb79f
7:cpu,cpuacct:/system.slice/containerd.service/foo/1c7532a506b51e4072102771d0b0cc30e2300f3f539cdd6e3d8b3046275bb79f
6:cpuset:/foo/1c7532a506b51e4072102771d0b0cc30e2300f3f539cdd6e3d8b3046275bb79f
5:memory:/system.slice/containerd.service/foo/1c7532a506b51e4072102771d0b0cc30e2300f3f539cdd6e3d8b3046275bb79f
4:rdma:/
3:devices:/system.slice/containerd.service/foo/1c7532a506b51e4072102771d0b0cc30e2300f3f539cdd6e3d8b3046275bb79f
2:net_cls,net_prio:/foo/1c7532a506b51e4072102771d0b0cc30e2300f3f539cdd6e3d8b3046275bb79f
1:name=systemd:/system.slice/containerd.service/foo/1c7532a506b51e4072102771d0b0cc30e2300f3f539cdd6e3d8b3046275bb79f
0::/system.slice/containerd.service
```
So it seems that the container ID is still there as expected. I'll go over the failing tests in the weekend.
adarnimrod: @asottile I think I addressed everything and tests are now passing.
asottile: according to this SO post the method you're using is not consistent on all kernel versions: https://stackoverflow.com/q/20995351/812183
adarnimrod: You're going to make this on me, are you? :stuck_out_tongue_winking_eye: 
I'm guessing you're referring to this specific answer https://stackoverflow.com/a/52081984 .
The above test I ran locally using Docker 20.10 and kernel version 5.4.
Here's the content of `/proc/1/cgroup` in Ubuntu Trusty (Docker 1.6 and kernel version 3.13):
```
11:hugetlb:/
10:perf_event:/docker/3eae25f6afca4ec0df8b9288c3658fde47a31dfe88c0a02474cda89d166e307d
9:blkio:/docker/3eae25f6afca4ec0df8b9288c3658fde47a31dfe88c0a02474cda89d166e307d
8:freezer:/docker/3eae25f6afca4ec0df8b9288c3658fde47a31dfe88c0a02474cda89d166e307d
7:devices:/docker/3eae25f6afca4ec0df8b9288c3658fde47a31dfe88c0a02474cda89d166e307d
6:memory:/docker/3eae25f6afca4ec0df8b9288c3658fde47a31dfe88c0a02474cda89d166e307d
5:cpuacct:/docker/3eae25f6afca4ec0df8b9288c3658fde47a31dfe88c0a02474cda89d166e307d
4:cpu:/docker/3eae25f6afca4ec0df8b9288c3658fde47a31dfe88c0a02474cda89d166e307d
3:cpuset:/docker/3eae25f6afca4ec0df8b9288c3658fde47a31dfe88c0a02474cda89d166e307d
2:name=systemd:/
```
And here's the content in Ubuntu Xenial (Docker 18.09 and kernel version 4.4):
```
11:perf_event:/docker/7f2ee26c7145e4a6ac67fbba7284199ff92e4a4a3aea1eefde5edda7e2bee13b
10:cpuset:/docker/7f2ee26c7145e4a6ac67fbba7284199ff92e4a4a3aea1eefde5edda7e2bee13b
9:freezer:/docker/7f2ee26c7145e4a6ac67fbba7284199ff92e4a4a3aea1eefde5edda7e2bee13b
8:net_cls,net_prio:/docker/7f2ee26c7145e4a6ac67fbba7284199ff92e4a4a3aea1eefde5edda7e2bee13b
7:hugetlb:/docker/7f2ee26c7145e4a6ac67fbba7284199ff92e4a4a3aea1eefde5edda7e2bee13b
6:memory:/docker/7f2ee26c7145e4a6ac67fbba7284199ff92e4a4a3aea1eefde5edda7e2bee13b
5:devices:/docker/7f2ee26c7145e4a6ac67fbba7284199ff92e4a4a3aea1eefde5edda7e2bee13b
4:pids:/docker/7f2ee26c7145e4a6ac67fbba7284199ff92e4a4a3aea1eefde5edda7e2bee13b
3:blkio:/docker/7f2ee26c7145e4a6ac67fbba7284199ff92e4a4a3aea1eefde5edda7e2bee13b
2:cpu,cpuacct:/docker/7f2ee26c7145e4a6ac67fbba7284199ff92e4a4a3aea1eefde5edda7e2bee13b
1:name=systemd:/docker/7f2ee26c7145e4a6ac67fbba7284199ff92e4a4a3aea1eefde5edda7e2bee13b
```
From these tests I see that the first line doesn't always have the container id, however the line that starts with `6:memory:` does. Is using the container id from that specific line an OK solution?
adarnimrod: @asottile I think it's ready now.

Patch:
diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py
index 5b21ec9..5bb3395 100644
--- a/pre_commit/languages/docker.py
+++ b/pre_commit/languages/docker.py
@@ -1,7 +1,6 @@
 import hashlib
 import json
 import os
-import socket
 from typing import Sequence
 from typing import Tuple
 
@@ -26,12 +25,24 @@ def _is_in_docker() -> bool:
         return False
 
 
+def _get_container_id() -> str:
+    # It's assumed that we already check /proc/1/cgroup in _is_in_docker. The
+    # cpuset cgroup controller existed since cgroups were introduced so this
+    # way of getting the container ID is pretty reliable.
+    with open('/proc/1/cgroup', 'rb') as f:
+        for line in f.readlines():
+            if line.split(b':')[1] == b'cpuset':
+                return os.path.basename(line.split(b':')[2]).strip().decode()
+    raise RuntimeError('Failed to find the container ID in /proc/1/cgroup.')
+
+
 def _get_docker_path(path: str) -> str:
     if not _is_in_docker():
         return path
-    hostname = socket.gethostname()
 
-    _, out, _ = cmd_output_b('docker', 'inspect', hostname)
+    container_id = _get_container_id()
+
+    _, out, _ = cmd_output_b('docker', 'inspect', container_id)
 
     container, = json.loads(out)
     for mount in container['Mounts']:


Test patch:
diff --git a/tests/languages/docker_test.py b/tests/languages/docker_test.py
index 01b5e27..06a08bc 100644
--- a/tests/languages/docker_test.py
+++ b/tests/languages/docker_test.py
@@ -9,6 +9,41 @@ import pytest
 
 from pre_commit.languages import docker
 
+DOCKER_CGROUP_EXAMPLE = b'''\
+12:hugetlb:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
+11:blkio:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
+10:freezer:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
+9:cpu,cpuacct:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
+8:pids:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
+7:rdma:/
+6:net_cls,net_prio:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
+5:cpuset:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
+4:devices:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
+3:memory:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
+2:perf_event:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
+1:name=systemd:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
+0::/system.slice/containerd.service
+'''  # noqa: E501
+
+# The ID should match the above cgroup example.
+CONTAINER_ID = 'c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7'  # noqa: E501
+
+NON_DOCKER_CGROUP_EXAMPLE = b'''\
+12:perf_event:/
+11:hugetlb:/
+10:devices:/
+9:blkio:/
+8:rdma:/
+7:cpuset:/
+6:cpu,cpuacct:/
+5:freezer:/
+4:memory:/
+3:pids:/
+2:net_cls,net_prio:/
+1:name=systemd:/init.scope
+0::/init.scope
+'''
+
 
 def test_docker_fallback_user():
     def invalid_attribute():
@@ -37,45 +72,25 @@ def _mock_open(data):
 
 
 def test_in_docker_docker_in_file():
-    docker_cgroup_example = b'''\
-12:hugetlb:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
-11:blkio:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
-10:freezer:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
-9:cpu,cpuacct:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
-8:pids:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
-7:rdma:/
-6:net_cls,net_prio:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
-5:cpuset:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
-4:devices:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
-3:memory:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
-2:perf_event:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
-1:name=systemd:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7
-0::/system.slice/containerd.service
-'''  # noqa: E501
-    with _mock_open(docker_cgroup_example):
+    with _mock_open(DOCKER_CGROUP_EXAMPLE):
         assert docker._is_in_docker() is True
 
 
 def test_in_docker_docker_not_in_file():
-    non_docker_cgroup_example = b'''\
-12:perf_event:/
-11:hugetlb:/
-10:devices:/
-9:blkio:/
-8:rdma:/
-7:cpuset:/
-6:cpu,cpuacct:/
-5:freezer:/
-4:memory:/
-3:pids:/
-2:net_cls,net_prio:/
-1:name=systemd:/init.scope
-0::/init.scope
-'''
-    with _mock_open(non_docker_cgroup_example):
+    with _mock_open(NON_DOCKER_CGROUP_EXAMPLE):
         assert docker._is_in_docker() is False
 
 
+def test_get_container_id():
+    with _mock_open(DOCKER_CGROUP_EXAMPLE):
+        assert docker._get_container_id() == CONTAINER_ID
+
+
+def test_get_container_id_failure():
+    with _mock_open(b''), pytest.raises(RuntimeError):
+        docker._get_container_id()
+
+
 def test_get_docker_path_not_in_docker_returns_same():
     with mock.patch.object(docker, '_is_in_docker', return_value=False):
         assert docker._get_docker_path('abc') == 'abc'
@@ -84,7 +99,10 @@ def test_get_docker_path_not_in_docker_returns_same():
 @pytest.fixture
 def in_docker():
     with mock.patch.object(docker, '_is_in_docker', return_value=True):
-        yield
+        with mock.patch.object(
+            docker, '_get_container_id', return_value=CONTAINER_ID,
+        ):
+            yield
 
 
 def _linux_commonpath():