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
Show file tree
Hide file tree
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
24 changes: 23 additions & 1 deletion binderhub/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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="""
Expand Down Expand Up @@ -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,
Expand Down
20 changes: 18 additions & 2 deletions binderhub/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
)
Expand Down
2 changes: 2 additions & 0 deletions binderhub/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down
43 changes: 43 additions & 0 deletions binderhub/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from unittest import mock

import pytest
from traitlets.traitlets import TraitError

from binderhub import utils

Expand Down Expand Up @@ -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
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down