Move pod quota checking into separate class

[manics]

Whilst working on #1521 I realised the Kubernetes client is still used a few places.

This moves the quota checking code from builder.BuildHandler.launch into it's own class. The logic should be unchanged.

After this the remaining use of the kubernetes_client is in the health check:

binderhub/binderhub/health.py

Lines 108 to 113 in 656204b

	@at_most_every
	async def _get_pods(self):
	"""Get information about build and user pods"""
	namespace = self.settings["build_namespace"]
	k8s = self.settings["kubernetes_client"]
	pool = self.settings["executor"]

Note I haven't tested this on a real system yet.

[consideRatio]

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?

[consideRatio]

Practically, does having a launch quota of 100 imply:

• you can at most have 100 build workloads, if you do, you must have 0 user servers started at this point in time
• you can at most have 100 user servers, if you do, you must have 0 build workloads running at this point in time

[manics]

Previously the quota was handled inside the builder.BuildHandler.launch method. By moving it into a separate class the launch() 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 of label_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 a repo_config parameter, which is created by BinderHub using some parameters:

binderhub/binderhub/app.py

Lines 292 to 334 in e80f841

	per_repo_quota = Integer(
	0,
	help="""
	Maximum number of concurrent users running from a given repo.
	Limits the amount of Binder that can be consumed by a single repo.
	0 (default) means no quotas.
	""",
	config=True,
	)
	pod_quota = Integer(
	None,
	help="""
	The number of concurrent pods this hub has been designed to support.
	This quota is used as an indication for how much above or below the
	design capacity a hub is running.
	Attempts to launch new pods once the quota has been reached will fail.
	The default corresponds to no quota, 0 means the hub can't accept pods
	(maybe because it is in maintenance mode), and any positive integer
	sets the quota.
	""",
	allow_none=True,
	config=True,
	)
	per_repo_quota_higher = Integer(
	0,
	help="""
	Maximum number of concurrent users running from a higher-quota repo.
	Limits the amount of Binder that can be consumed by a single repo. This
	quota is a second limit for repos with special status. See the
	`high_quota_specs` parameter of RepoProvider classes for usage.
	0 (default) means no quotas.
	""",
	config=True,
	)

and a RepoProvider based class:

binderhub/binderhub/repoproviders.py

Lines 134 to 166 in e80f841

	def repo_config(self, settings):
	"""
	Return configuration for this repository.
	"""
	repo_config = {}
	# Defaults and simple overrides
	if self.has_higher_quota():
	repo_config["quota"] = settings.get("per_repo_quota_higher")
	else:
	repo_config["quota"] = settings.get("per_repo_quota")
	# Spec regex-based configuration
	for item in self.spec_config:
	pattern = item.get("pattern", None)
	config = item.get("config", None)
	if not isinstance(pattern, str):
	raise ValueError(
	"Spec-pattern configuration expected "
	"a regex pattern string, not "
	f"type {type(pattern)}"
	)
	if not isinstance(config, dict):
	raise ValueError(
	"Spec-pattern configuration expected "
	"a specification configuration dict, not "
	f"type {type(config)}"
	)
	# Ignore case, because most git providers do not
	# count DS-100/textbook as different from ds-100/textbook
	if re.match(pattern, self.spec, re.IGNORECASE):
	repo_config.update(config)
	return repo_config

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.

[consideRatio]

Thanks for the thorough clarfication @manics!!

[consideRatio]

Nice looks great! ❤️ 🎉 🌻!!