-
Notifications
You must be signed in to change notification settings - Fork 398
Move pod quota checking into separate class #1526
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,161 @@ | ||
""" | ||
Singleuser server quotas | ||
""" | ||
|
||
import asyncio | ||
import json | ||
import os | ||
from collections import namedtuple | ||
|
||
import kubernetes.config | ||
from kubernetes import client | ||
from traitlets import Any, Integer, Unicode, default | ||
from traitlets.config import LoggingConfigurable | ||
|
||
from .utils import KUBE_REQUEST_TIMEOUT | ||
|
||
|
||
class LaunchQuotaExceeded(Exception): | ||
"""Raised when a quota will be exceeded by a launch""" | ||
|
||
def __init__(self, message, *, quota, used, status): | ||
""" | ||
message: User-facing message | ||
quota: Quota limit | ||
used: Quota used | ||
status: String indicating the type of quota | ||
""" | ||
super().__init__() | ||
self.message = message | ||
self.quota = quota | ||
self.used = used | ||
self.status = status | ||
|
||
|
||
ServerQuotaCheck = namedtuple("ServerQuotaCheck", ["total", "matching", "quota"]) | ||
|
||
|
||
class LaunchQuota(LoggingConfigurable): | ||
|
||
executor = Any( | ||
allow_none=True, help="Optional Executor to use for blocking operations" | ||
) | ||
|
||
total_quota = Integer( | ||
None, | ||
help=""" | ||
The number of concurrent singleuser servers that can be run. | ||
|
||
None: no quota | ||
0: the hub can't run any singleuser servers (e.g. in maintenance mode) | ||
Positive integer: sets the quota | ||
""", | ||
allow_none=True, | ||
config=True, | ||
) | ||
|
||
async def check_repo_quota(self, image_name, repo_config, repo_url): | ||
""" | ||
Check whether launching a repository would exceed a quota. | ||
|
||
Parameters | ||
---------- | ||
image_name: str | ||
repo_config: dict | ||
repo_url: str | ||
|
||
Returns | ||
------- | ||
If quotas are disabled returns None | ||
If quotas are exceeded raises LaunchQuotaExceeded | ||
Otherwise returns: | ||
- total servers | ||
- matching servers running image_name | ||
- quota | ||
""" | ||
return None | ||
|
||
|
||
class KubernetesLaunchQuota(LaunchQuota): | ||
|
||
api = Any( | ||
help="Kubernetes API object to make requests (kubernetes.client.CoreV1Api())", | ||
) | ||
|
||
@default("api") | ||
def _default_api(self): | ||
try: | ||
kubernetes.config.load_incluster_config() | ||
except kubernetes.config.ConfigException: | ||
kubernetes.config.load_kube_config() | ||
return client.CoreV1Api() | ||
|
||
namespace = Unicode(help="Kubernetes namespace to check", config=True) | ||
|
||
@default("namespace") | ||
def _default_namespace(self): | ||
return os.getenv("BUILD_NAMESPACE", "default") | ||
|
||
async def check_repo_quota(self, image_name, repo_config, repo_url): | ||
# the image name (without tag) is unique per repo | ||
# use this to count the number of pods running with a given repo | ||
# if we added annotations/labels with the repo name via KubeSpawner | ||
# we could do this better | ||
image_no_tag = image_name.rsplit(":", 1)[0] | ||
|
||
# TODO: put busy users in a queue rather than fail? | ||
# That would be hard to do without in-memory state. | ||
repo_quota = repo_config.get("quota") | ||
pod_quota = self.total_quota | ||
|
||
# Fetch info on currently running users *only* if quotas are set | ||
if pod_quota is not None or repo_quota: | ||
matching_pods = 0 | ||
|
||
# TODO: run a watch to keep this up to date in the background | ||
f = self.executor.submit( | ||
self.api.list_namespaced_pod, | ||
self.namespace, | ||
label_selector="app=jupyterhub,component=singleuser-server", | ||
_request_timeout=KUBE_REQUEST_TIMEOUT, | ||
_preload_content=False, | ||
) | ||
resp = await asyncio.wrap_future(f) | ||
pods = json.loads(resp.read())["items"] | ||
total_pods = len(pods) | ||
|
||
if pod_quota is not None and total_pods >= pod_quota: | ||
# check overall quota first | ||
self.log.error(f"BinderHub is full: {total_pods}/{pod_quota}") | ||
raise LaunchQuotaExceeded( | ||
"Too many users on this BinderHub! Try again soon.", | ||
quota=pod_quota, | ||
used=total_pods, | ||
status="pod_quota", | ||
) | ||
|
||
for pod in pods: | ||
for container in pod["spec"]["containers"]: | ||
# is the container running the same image as us? | ||
# if so, count one for the current repo. | ||
image = container["image"].rsplit(":", 1)[0] | ||
if image == image_no_tag: | ||
matching_pods += 1 | ||
break | ||
|
||
if repo_quota and matching_pods >= repo_quota: | ||
self.log.error( | ||
f"{repo_url} has exceeded quota: {matching_pods}/{repo_quota} ({total_pods} total)" | ||
) | ||
raise LaunchQuotaExceeded( | ||
f"Too many users running {repo_url}! Try again soon.", | ||
quota=repo_quota, | ||
used=matching_pods, | ||
status="repo_quota", | ||
) | ||
|
||
return ServerQuotaCheck( | ||
total=total_pods, matching=matching_pods, quota=repo_quota | ||
) | ||
|
||
return None |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the context of binderhub, it would be great to be explicit about what this means. I now understand it as "the amount of user servers scheduled-to-run-after-build/starting/running by binderhub in the jupyterhub that binderhub manages", but I'm open to it meaning something related to build pods as well, or that it could be coupled to possible jupyterhub user servers unmanaged by binderhub.
Is "launch quota" a name introduced in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Practically, does having a launch quota of 100 imply:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously the quota was handled inside the
builder.BuildHandler.launch
method. By moving it into a separate class thelaunch()
method becomes independent of the platform (k8s, docker, HPC, etc).The quota check determines whether BinderHub should launch a repository which is why I've called the base class
LaunchQuota
. The decision on whether to allow a launch or not is an implementation detail. The current default (K8s only) method is to look at the total number oflabel_selector="app=jupyterhub,component=singleuser-server"
pods, and the total number of those pods that are running an image from the repo being launched. In future there's scope for taking other factors into account such as build pods, total CPU/memory usage across all pods running a particular repo, etc.It's not a perfect abstraction since
LaunchQuota.check_repo_quota(self, image_name, repo_config, repo_url)
takes arepo_config
parameter, which is created by BinderHub using some parameters:binderhub/binderhub/app.py
Lines 292 to 334 in e80f841
and a RepoProvider based class:
binderhub/binderhub/repoproviders.py
Lines 134 to 166 in e80f841
but I'm trying to minimise the changes to the codebase, assuming I've coded this correctly there should be zero change in the existing behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the thorough clarfication @manics!!