From 4a511f1174f1dcea8b46c9d42989723033d0de06 Mon Sep 17 00:00:00 2001 From: Oleksii Date: Thu, 19 Aug 2021 18:40:27 +0200 Subject: [PATCH 1/2] CPU requests and limits for build pod --- binderhub/app.py | 24 +++++++++- binderhub/build.py | 20 ++++++++- binderhub/builder.py | 2 + binderhub/tests/test_utils.py | 43 ++++++++++++++++++ binderhub/utils.py | 82 ++++++++++++++++++++++++++++++++++- 5 files changed, 167 insertions(+), 4 deletions(-) diff --git a/binderhub/app.py b/binderhub/app.py index e64e021cc..d4ca8719a 100755 --- a/binderhub/app.py +++ b/binderhub/app.py @@ -57,7 +57,7 @@ DataverseProvider) from .metrics import MetricsHandler -from .utils import ByteSpecification, url_path_join +from .utils import CPUSpecification, ByteSpecification, url_path_join from .events import EventLog @@ -335,6 +335,26 @@ def _valid_badge_base_url(self, proposal): config=True ) + build_cpu_request = CPUSpecification( + 0, + help=""" + Amount of cpu to request when scheduling a build + + 0 reserves no cpu. + + """, + config=True, + ) + build_cpu_limit = CPUSpecification( + 0, + help=""" + Max amount of cpu allocated for each image build process. + + 0 sets no limit. + """, + config=True, + ) + build_memory_request = ByteSpecification( 0, help=""" @@ -773,6 +793,8 @@ def initialize(self, *args, **kwargs): "jinja2_env": jinja_env, "build_memory_limit": self.build_memory_limit, "build_memory_request": self.build_memory_request, + "build_cpu_limit": self.build_cpu_limit, + "build_cpu_request": self.build_cpu_request, "build_docker_host": self.build_docker_host, "build_docker_config": self.build_docker_config, "base_url": self.base_url, diff --git a/binderhub/build.py b/binderhub/build.py index 2ec263e23..f130797f9 100644 --- a/binderhub/build.py +++ b/binderhub/build.py @@ -51,6 +51,8 @@ def __init__( image_name, git_credentials=None, push_secret=None, + cpu_limit=0, + cpu_request=0, memory_limit=0, memory_request=0, node_selector=None, @@ -95,6 +97,18 @@ def __init__( https://git-scm.com/docs/gitcredentials for more information. push_secret : str Kubernetes secret containing credentials to push docker image to registry. + cpu_limit + CPU limit for the docker build process. Can be an integer (1), fraction (0.5) or + millicore specification (100m). Value should adhere to K8s specification + for CPU meaning. See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#meaning-of-cpu + for more information + cpu_request + CPU request of the build pod. The actual building happens in the + docker daemon, but setting request in the build pod makes sure that + cpu is reserved for the docker build in the node by the kubernetes + scheduler. Value should adhere to K8s specification for CPU meaning. + See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#meaning-of-cpu + for more information memory_limit Memory limit for the docker build process. Can be an integer in bytes, or a byte specification (like 6M). @@ -129,6 +143,8 @@ def __init__( self.push_secret = push_secret self.build_image = build_image self.main_loop = IOLoop.current() + self.cpu_limit = cpu_limit + self.cpu_request = cpu_request self.memory_limit = memory_limit self.memory_request = memory_request self.docker_host = docker_host @@ -343,8 +359,8 @@ def submit(self): args=self.get_cmd(), volume_mounts=volume_mounts, resources=client.V1ResourceRequirements( - limits={'memory': self.memory_limit}, - requests={'memory': self.memory_request}, + limits={'memory': self.memory_limit, 'cpu': self.cpu_limit}, + requests={'memory': self.memory_request, 'cpu': self.cpu_request}, ), env=env ) diff --git a/binderhub/builder.py b/binderhub/builder.py index ca41c2270..ca683a404 100644 --- a/binderhub/builder.py +++ b/binderhub/builder.py @@ -411,6 +411,8 @@ async def get(self, provider_prefix, _unescaped_spec): image_name=image_name, push_secret=push_secret, build_image=self.settings['build_image'], + cpu_limit=self.settings['build_cpu_limit'], + cpu_request=self.settings['build_cpu_request'], memory_limit=self.settings['build_memory_limit'], memory_request=self.settings['build_memory_request'], docker_host=self.settings['build_docker_host'], diff --git a/binderhub/tests/test_utils.py b/binderhub/tests/test_utils.py index 2e96348d9..22b7dca84 100644 --- a/binderhub/tests/test_utils.py +++ b/binderhub/tests/test_utils.py @@ -2,6 +2,7 @@ from unittest import mock import pytest +from traitlets.traitlets import TraitError from binderhub import utils @@ -137,3 +138,45 @@ def test_ip_in_networks(ip, cidrs, found): def test_ip_in_networks_invalid(): with pytest.raises(ValueError): utils.ip_in_networks("1.2.3.4", {}, 0) + +@pytest.mark.parametrize( + "value, coerced", + [ + ("2", 2), + (2, 2), + ("0.2", 0.2), + (0.2, 0.2), + ("200m", "200m"), + (None, 0), + (0, 0), + ("0", 0), + ] +) +def test_cpu_specification_valid(value, coerced): + cpu_spec = utils.CPUSpecification() + assert cpu_spec.validate(None, value) == coerced + +@pytest.mark.parametrize( + "value", + [ + ("0.2m"), + ("deadbeef"), + ("m"), + (""), + ("200M"), + ("200k"), + ("-1"), + ("-1m"), + ("-0.1m"), + (-0.1), + (-1), + (False), + (True), + ([]), + ({}), + ] +) +def test_cpu_specification_invalid(value): + cpu_spec = utils.CPUSpecification() + with pytest.raises(TraitError): + _ = cpu_spec.validate(None, value) \ No newline at end of file diff --git a/binderhub/utils.py b/binderhub/utils.py index 4bf041adf..29a240755 100644 --- a/binderhub/utils.py +++ b/binderhub/utils.py @@ -4,7 +4,7 @@ import ipaddress import time -from traitlets import Integer, TraitError +from traitlets import Unicode, Integer, TraitError # default _request_timeout for kubernetes api requests @@ -42,6 +42,86 @@ def rendezvous_rank(buckets, key): return [b for (s, b) in sorted(ranking, reverse=True)] +class CPUSpecification(Unicode): + """ + Allows specifying CPU limits + + Suffixes allowed are: + - m -> millicore + + """ + + # Default to allowing None as a value + allow_none = True + + def validate(self, obj, value): + """ + Validate that the passed in value is a valid cpu specification + in the K8s CPU meaning. + + See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#meaning-of-cpu + + It could either be a pure int or float, when it is taken as a value. + In case of integer it can optionally have 'm' suffix to designate millicores. + """ + + def raise_error(value): + raise TraitError( + "{val} is not a valid cpu specification".format( + val=value + ) + ) + + # Positive filter for numberic values + only_positive = lambda v : v if v >= 0 else raise_error(v) + + if value is None: + return 0 + + if isinstance(value, bool): + raise_error(value) + + if isinstance(value, int): + return only_positive(int(value)) + + if isinstance(value, float): + return only_positive(float(value)) + + # Must be string + if not isinstance(value, str): + raise_error(value) + + # Try treat it as integer + _int_value = None + try: + _int_value = int(value) + except ValueError: + pass + + if isinstance(_int_value, int): + return only_positive(_int_value) + + # Try treat it as float + _float_value = None + try: + _float_value = float(value) + except ValueError: + pass + + if isinstance(_float_value, float): + return only_positive(_float_value) + + # Try treat it as millicore spec + try: + _unused = only_positive(int(value[:-1])) + except ValueError: + raise_error(value) + + if value[-1] not in ['m']: + raise_error(value) + + return value + class ByteSpecification(Integer): """ Allow easily specifying bytes in units of 1024 with suffixes From 618c2894b1415826264b1332b0ce3707b2040995 Mon Sep 17 00:00:00 2001 From: Tim Head Date: Fri, 24 Sep 2021 09:39:30 +0200 Subject: [PATCH 2/2] Update binderhub/utils.py --- binderhub/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/binderhub/utils.py b/binderhub/utils.py index 29a240755..3fffda9e9 100644 --- a/binderhub/utils.py +++ b/binderhub/utils.py @@ -57,7 +57,7 @@ class CPUSpecification(Unicode): def validate(self, obj, value): """ Validate that the passed in value is a valid cpu specification - in the K8s CPU meaning. + using Kubernetes formatting. See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#meaning-of-cpu