Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Detect the number of CPUs used by Docker (#5464) #5466

Merged
merged 11 commits into from Sep 13, 2019
Merged

Detect the number of CPUs used by Docker (#5464) #5466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
00c232b
#5464 Detect CPUs in used by Docker
uilianries Jul 9, 2019
c1890e6
#5464 Fail safe when reading cgroup
uilianries Jul 10, 2019
d7ac67a
#5464 truncate docker cpus
uilianries Jul 10, 2019
9e95602
#5464 Load cgroup files
uilianries Jul 10, 2019
440bad6
#5464 Ignore docker container
uilianries Jul 11, 2019
75982de
#5464 Validate cpu count in containers
uilianries Jul 11, 2019
5a30048
#5464 Validate cpu quota by zero
uilianries Jul 17, 2019
cb602d9
#5464 Fix tests
uilianries Jul 18, 2019
61ba286
Merge branch 'develop' into hotfix/cpu-count-docker
uilianries Aug 14, 2019
8c91142
#5464 Use CpuProperties directly
uilianries Sep 13, 2019
777f629
#5464 Remove temporary cpu variable
uilianries Sep 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 25 additions & 4 deletions conans/client/tools/oss.py
View file
Open in desktop
Expand Up @@ -4,8 +4,8 @@
import subprocess
import sys
import tempfile
from subprocess import PIPE

import math
from subprocess import CalledProcessError, PIPE
import six

from conans.client.tools.env import environment_append
Expand All @@ -24,14 +24,35 @@ def args_to_string(args):
return " ".join("'" + arg.replace("'", r"'\''") + "'" for arg in args)


class CpuProperties(object):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Do not use bare exceptions, they capture Ctrl+C and other interrupts too

This is a bit too much Object Oriented for me without a real goal:

  • All methods should be static (and are not, this is an indicator)
  • It is mainly designed for testing with mocks, which I get, but you can get the same with one function:
 def get_cpus(quota_file=None, period_file=None):
        try:
            cfs_quota_us = int(load(quota_file or "/sys/fs/cgroup/cpu/cpu.cfs_quota_us"))
            cfs_period_us = int(load(period_file or "/sys/fs/cgroup/cpu/cpu.cfs_period_us"))
            if cfs_quota_us > 0 and cfs_period_us > 0:
                return int(math.ceil(cfs_quota_us / cfs_period_us))
        except Exception:
            pass
        return multiprocessing.cpu_count()

Now it is easy to test too, and also testing is more real, it test also the load of file + conversion to integer


def get_cpu_quota(self):
return int(load("/sys/fs/cgroup/cpu/cpu.cfs_quota_us"))

def get_cpu_period(self):
return int(load("/sys/fs/cgroup/cpu/cpu.cfs_period_us"))

def get_cpus(self):
try:
cfs_quota_us = self.get_cpu_quota()
cfs_period_us = self.get_cpu_period()
if cfs_quota_us > 0 and cfs_period_us > 0:
return int(math.ceil(cfs_quota_us / cfs_period_us))
except:
pass
return multiprocessing.cpu_count()


def cpu_count(output=None):
output = default_output(output, 'conans.client.tools.oss.cpu_count')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why removing this output? This would make the output.warn() below to crash if output=None (otherwise, remove the default and check with an assert for valid output object)

try:
env_cpu_count = os.getenv("CONAN_CPU_COUNT", None)
if env_cpu_count is not None and not env_cpu_count.isdigit():
raise ConanException("Invalid CONAN_CPU_COUNT value '%s', "
"please specify a positive integer" % env_cpu_count)
return int(env_cpu_count) if env_cpu_count else multiprocessing.cpu_count()
if env_cpu_count:
return int(env_cpu_count)
else:
return CpuProperties().get_cpus()
except NotImplementedError:
output.warn("multiprocessing.cpu_count() not implemented. Defaulting to 1 cpu")
return 1 # Safe guess
Expand Down
10 changes: 10 additions & 0 deletions conans/test/unittests/util/tools_test.py
View file
Open in desktop
Expand Up @@ -144,6 +144,16 @@ def cpu_count_test(self):
with six.assertRaisesRegex(self, ConanException, "Invalid CONAN_CPU_COUNT value"):
tools.cpu_count(output=output)

@patch("conans.client.tools.oss.CpuProperties.get_cpu_period")
@patch("conans.client.tools.oss.CpuProperties.get_cpu_quota")
def test_cpu_count_in_container(self, get_cpu_quota_mock, get_cpu_period_mock):
get_cpu_quota_mock.return_value = 12000
get_cpu_period_mock.return_value = 1000

output = ConanOutput(sys.stdout)
cpus = tools.cpu_count(output=output)
self.assertEqual(12, cpus)

def get_env_unit_test(self):
"""
Unit tests tools.get_env
Expand Down