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.

Sign up for GitHub

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

Add capabilities/privileged to build container to support running on K8s without Docker #1512

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Add capabilities/privileged to build container to support running on K8s without Docker #1512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5fb8e46
docker_build_host can be empty to disable docker.sock
manics Jul 25, 2022
4e12189
Add build_capabilities (container cap add) for build container
manics Jul 25, 2022
3affc7f
Add privileged build pod option
manics Jul 25, 2022
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
26 changes: 21 additions & 5 deletions binderhub/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
Bytes,
Dict,
Integer,
List,
TraitError,
Type,
Unicode,
Expand Down Expand Up @@ -410,16 +411,19 @@ def _valid_badge_base_url(self, proposal):

Currently, only paths are supported, and they are expected to be available on
all the hosts.

Set to empty to disable the docker socket mount.
""",
)

@validate("build_docker_host")
def docker_build_host_validate(self, proposal):
parts = urlparse(proposal.value)
if parts.scheme != "unix" or parts.netloc != "":
raise TraitError(
"Only unix domain sockets on same node are supported for build_docker_host"
)
if proposal.value:
parts = urlparse(proposal.value)
if parts.scheme != "unix" or parts.netloc != "":
raise TraitError(
"Only unix domain sockets on same node are supported for build_docker_host"
)
return proposal.value

build_docker_config = Dict(
Expand Down Expand Up @@ -499,6 +503,17 @@ def _default_build_namespace(self):
config=True,
)

build_capabilities = List(
[],
help="""
Additional Kubernetes capabilities to add to the build container

If the special string "privileged" is found the container is run in
privileged mode and other capabilities are ignored.
""",
config=True,
)

build_node_selector = Dict(
{},
config=True,
Expand Down Expand Up @@ -802,6 +817,7 @@ def initialize(self, *args, **kwargs):
"ban_networks_min_prefix_len": self.ban_networks_min_prefix_len,
"build_namespace": self.build_namespace,
"build_image": self.build_image,
"build_capabilities": self.build_capabilities,
"build_node_selector": self.build_node_selector,
"build_pool": self.build_pool,
"build_token_check_origin": self.build_token_check_origin,
Expand Down
43 changes: 31 additions & 12 deletions binderhub/build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ def __init__(
build_image,
docker_host,
image_name,
build_capabilities=None,
git_credentials=None,
push_secret=None,
memory_limit=0,
Expand Down Expand Up @@ -118,9 +119,14 @@ def __init__(
The docker socket to use for building the image.
Must be a unix domain socket on a filesystem path accessible on the
node in which the build pod is running.
If empty no socket is mounted.
image_name : str
Full name of the image to build. Includes the tag.
Passed through to repo2docker.
build_capabilities: [str]
Capabilities to add to the build pod.
If the special string "privileged" is found the container is run in
privileged mode and other capabilities are ignored.
git_credentials : str
Git credentials to use to clone private repositories. Passed
through to repo2docker via the GIT_CREDENTIAL_ENV environment
Expand Down Expand Up @@ -162,6 +168,7 @@ def __init__(
self.image_name = image_name
self.push_secret = push_secret
self.build_image = build_image
self.build_capabilities = build_capabilities or []
self.main_loop = IOLoop.current()
self.memory_limit = memory_limit
self.memory_request = memory_request
Expand Down Expand Up @@ -350,20 +357,24 @@ def submit(self):
Progress of the build can be monitored by listening for items in
the Queue passed to the constructor as `q`.
"""
volume_mounts = [
client.V1VolumeMount(
mount_path="/var/run/docker.sock", name="docker-socket"
volume_mounts = []
volumes = []

if self.docker_host:
volume_mounts.append(
client.V1VolumeMount(
mount_path="/var/run/docker.sock", name="docker-socket"
)
)
]
docker_socket_path = urlparse(self.docker_host).path
volumes = [
client.V1Volume(
name="docker-socket",
host_path=client.V1HostPathVolumeSource(
path=docker_socket_path, type="Socket"
),
docker_socket_path = urlparse(self.docker_host).path
volumes.append(
client.V1Volume(
name="docker-socket",
host_path=client.V1HostPathVolumeSource(
path=docker_socket_path, type="Socket"
),
)
)
]

if self.push_secret:
volume_mounts.append(
Expand All @@ -382,6 +393,13 @@ def submit(self):
client.V1EnvVar(name="GIT_CREDENTIAL_ENV", value=self.git_credentials)
)

if "privileged" in self.build_capabilities:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just ask for something like build_security_context or something and just pass it on directly through?

Part of the complexity here is that we want this to be not kubernetes specific. However, I think the way to do that is to refactor out the current class to be a KubeBuilder or something, and use traitlets directly. But until then, I think we should pass config through directly as much as possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not easily, we'd need to recurse through the JSON dict by copying this KubeSpawner code:
https://github.com/jupyterhub/kubespawner/blob/63aaccc567d03110fb83c19cbdbfdc1a30eb5406/kubespawner/utils.py#L92-L188

Good point about the long-term plan to make this into a traitlets Configurable.... let me see if I can come up with something

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuvipanda I've added a new traitlets based build class in #1518

security_context = client.V1SecurityContext(privileged=True)
else:
security_context = client.V1SecurityContext(
capabilities=client.V1Capabilities(add=self.build_capabilities)
)

self.pod = client.V1Pod(
metadata=client.V1ObjectMeta(
name=self.name,
Expand All @@ -405,6 +423,7 @@ def submit(self):
requests={"memory": self.memory_request},
),
env=env,
security_context=security_context,
)
],
tolerations=[
Expand Down
2 changes: 2 additions & 0 deletions binderhub/build_local.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ def __init__(
build_image,
docker_host,
image_name,
build_capabilities=None,
git_credentials=None,
push_secret=None,
memory_limit=0,
Expand Down Expand Up @@ -151,6 +152,7 @@ def __init__(
Ref of repository to build
Passed through to repo2docker.
build_image : ignored
build_capabilities: ignored
docker_host : ignored
image_name : str
Full name of the image to build. Includes the tag.
Expand Down
1 change: 1 addition & 0 deletions binderhub/builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@ async def get(self, provider_prefix, _unescaped_spec):
image_name=image_name,
push_secret=push_secret,
build_image=self.settings["build_image"],
build_capabilities=self.settings["build_capabilities"],
memory_limit=self.settings["build_memory_limit"],
memory_request=self.settings["build_memory_request"],
docker_host=self.settings["build_docker_host"],
Expand Down