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.

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

CPU requests and limits for build pod #1348

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
CPU requests and limits for build pod
noroutine committed Aug 28, 2021
commit 4a511f1174f1dcea8b46c9d42989723033d0de06
24 changes: 23 additions & 1 deletion binderhub/app.py
Original file line number Diff line number Diff line change
@@ -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,
20 changes: 18 additions & 2 deletions binderhub/build.py
Original file line number Diff line number Diff line change
@@ -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
)
2 changes: 2 additions & 0 deletions binderhub/builder.py
Original file line number Diff line number Diff line change
@@ -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'],
43 changes: 43 additions & 0 deletions binderhub/tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -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)
82 changes: 81 additions & 1 deletion binderhub/utils.py
Original file line number Diff line number Diff line change
@@ -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.
betatim marked this conversation as resolved.
Show resolved Hide resolved

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