Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
19 changes: 18 additions & 1 deletion binderhub/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ def _valid_badge_base_url(self, proposal):
)

push_secret = Unicode(
'binder-push-secret',
'binder-build-docker-config',
Copy link
Member

Choose a reason for hiding this comment

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

Is this change of default a breaking change?

Copy link
Member

Choose a reason for hiding this comment

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

I discussed it in the PR summary i updated above. Yes it can in theory cause a new build pod get stuck pending if started in a timeframe of a second for 1 replica binder deployments or a 10 second window for 2 replica deployments. I consider this acceptable.

If chart is upgraded, the old secret us gone, and an old binderhub replica awaiting termination until another binder pod starts up during a rolling upgrade, would starts a new build pod - then it would get stuck pending not being able to mount old k8s secret.

allow_none=True,
help="""
A kubernetes secret object that provides credentials for pushing built images.
Expand Down Expand Up @@ -375,6 +375,22 @@ def docker_build_host_validate(self, proposal):
raise TraitError("Only unix domain sockets on same node are supported for build_docker_host")
return proposal.value

build_docker_config = Dict(
Copy link
Member

Choose a reason for hiding this comment

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

Hello from the distant future! I'm reviewing the BinderHub config properties as part of #1521 and as far as I can tell this property isn't used. It's passed to builder.py, and referenced in a conditional, but its value is never used. Instead the helm chart references .Values.config.BinderHub.buildDockerConfig in helm-chart/binderhub/templates/_helpers.tpl.

Can you remember what's happening here?

None,
allow_none=True,
help="""
A dict which will be merged into the .docker/config.json of the build container (repo2docker)
Here, you could for example pass proxy settings as described here:
https://docs.docker.com/network/proxy/#configure-the-docker-client

Note: if you provide your own push_secret, this values wont
have an effect, as the push_secrets will overwrite
.docker/config.json
In this case, make sure that you include your config in your push_secret
""",
config=True
)

hub_api_token = Unicode(
help="""API token for talking to the JupyterHub API""",
config=True,
Expand Down Expand Up @@ -693,6 +709,7 @@ def initialize(self, *args, **kwargs):
"build_memory_limit": self.build_memory_limit,
"build_memory_request": self.build_memory_request,
"build_docker_host": self.build_docker_host,
"build_docker_config": self.build_docker_config,
"base_url": self.base_url,
"badge_base_url": self.badge_base_url,
"static_path": os.path.join(HERE, "static"),
Expand Down
4 changes: 2 additions & 2 deletions binderhub/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,9 @@ def submit(self):
)]

if self.push_secret:
volume_mounts.append(client.V1VolumeMount(mount_path="/root/.docker", name='docker-push-secret'))
volume_mounts.append(client.V1VolumeMount(mount_path="/root/.docker", name='docker-config'))
volumes.append(client.V1Volume(
name='docker-push-secret',
name='docker-config',
secret=client.V1SecretVolumeSource(secret_name=self.push_secret)
))

Expand Down
2 changes: 1 addition & 1 deletion binderhub/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ async def get(self, provider_prefix, _unescaped_spec):
# Prepare to build
q = Queue()

if self.settings['use_registry']:
if self.settings['use_registry'] or self.settings['build_docker_config']:
push_secret = self.settings['push_secret']
else:
push_secret = None
Expand Down
20 changes: 11 additions & 9 deletions helm-chart/binderhub/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ We truncate at 63 chars because some Kubernetes name fields are limited to this
{{- end -}}

{{/*
Render docker config.json for the registry-publishing secret.
Render docker config.json for the registry-publishing secret and other docker configuration.
*/}}
{{- define "registryDockerConfig" -}}
{{- define "buildDockerConfig" -}}

{{- /* default auth url */ -}}
{{- $url := (default "https://index.docker.io/v1" .Values.registry.url) }}
Expand All @@ -36,11 +36,13 @@ Render docker config.json for the registry-publishing secret.
{{- end }}
{{- $username := .Values.registry.username -}}

{
"auths": {
"{{ $url }}": {
"auth": "{{ printf "%s:%s" $username .Values.registry.password | b64enc }}"
}
}
}
{{- /* initialize a dict to represent a docker config with registry credentials */}}
{{- $dockerConfig := dict "auths" (dict $url (dict "auth" (printf "%s:%s" $username .Values.registry.password | b64enc))) }}

{{- /* augment our initialized docker config with buildDockerConfig */}}
{{- if .Values.config.BinderHub.buildDockerConfig }}
{{- $dockerConfig := merge $dockerConfig .Values.config.BinderHub.buildDockerConfig }}
{{- end }}

{{- $dockerConfig | toPrettyJson }}
{{- end }}
6 changes: 3 additions & 3 deletions helm-chart/binderhub/templates/secret.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ data:
{{- end }}
values.yaml: {{ $values | toYaml | b64enc | quote }}
---
{{- if .Values.config.BinderHub.use_registry }}
{{- if or .Values.config.BinderHub.use_registry .Values.config.BinderHub.buildDockerConfig }}
kind: Secret
apiVersion: v1
metadata:
name: binder-push-secret
name: binder-build-docker-config
type: Opaque
data:
config.json: {{ include "registryDockerConfig" . | b64enc | quote }}
config.json: {{ include "buildDockerConfig" . | b64enc | quote }}
{{- end }}