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

Simulate json output from docker buildx build #1413

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

yuvipanda
Copy link
Collaborator

We still use the docker python api to push images, and to run. So we need to set string_output to false, and simulate the json output on buildx. Alternatively we could completely get rid of the python API, but that's a much bigger lift (especially around registry credentials) that I don't want us to do right now.

Without this, push progress is reported as raw bytes in the binderhub UI

We still use the docker python api to push images, and to run.
So we need to set string_output to false, and simulate the json
output on buildx. Alternatively we could completely get rid of the
python API, but that's a much bigger lift (especially around registry
credentials) that I don't want us to do right now.

Without this, push progress is reported as raw bytes in the
binderhub UI
@minrk
Copy link
Member

minrk commented Feb 14, 2025

Is there a simple test that would have caught this?

I was able to test manually by running a local registry:

docker run --rm 127.0.0.1:9000:5000 registry

and then pushing to it:

python3 -m repo2docker --no-run --json-logs --image-name=localhost:9000/test/image .

before:

{"message": "Reusing existing image (localhost:9000/test/image), not building."}
{"message": "b'{\"status\":\"The push refers to repository [localhost:9000/test/image]\"}\\r\\n'", "phase": "pushing"}
{"message": "b'{\"status\":\"Preparing\",\"progressDetail\":{},\"id\":\"9ffc2f2e8fb8\"}\\r\\n{\"status\":\"Preparing\",\"progressDetail\":{},\"id\":\"d12bcb450636\"}\\r\\n{\"status\":\"Preparing\",\"progressDetail\":{},\"id\":\"c20414e1266b\"}\\r\\n{\"status\":\"Preparing\",\"progressDetail\":{},\"id\":\"6838097029a0\"}\\r\\n{\"status\":\"Preparing\",\"progressDetail\":{},\"id\":\"3ec51339a507\"}\\r\\n'", "phase": "pushing"}

after:

{"message": "View build details: docker-desktop://dashboard/build/multi/multi0/ys76161hbgd0qn0clt5vfngly\n", "phase": "building"}
{"message": "Successfully pushed localhost:9000/test/image:1739524755", "phase": "pushing"}

but I'm not sure how to test this practically without launching a local registry for the test.

@yuvipanda yuvipanda merged commit 80d186a into jupyterhub:main Feb 14, 2025
20 checks passed
@yuvipanda
Copy link
Collaborator Author

image

Seems to work fine with this!

yuvipanda added a commit to yuvipanda/mybinder.org-deploy that referenced this pull request Feb 14, 2025
Brings in:

repo2docker (among others):
- jupyterhub/repo2docker#1402
- jupyterhub/repo2docker#1413

binderhub:
- jupyterhub/binderhub#1929
- jupyterhub/binderhub#1930

Note that this *could* cause some issues for the very few repos
that depend on internal details of the non buildx old builder.
For example, see scikit-learn/scikit-learn#30835
yuvipanda added a commit to yuvipanda/mybinder.org-deploy that referenced this pull request Feb 14, 2025
Brings in:

repo2docker (among others):
- jupyterhub/repo2docker#1402
- jupyterhub/repo2docker#1413

binderhub:
- jupyterhub/binderhub#1929
- jupyterhub/binderhub#1930

Note that this *could* cause some issues for the very few repos
that depend on internal details of the non buildx old builder.
For example, see scikit-learn/scikit-learn#30835

We also remove memory limits, as those are not supported right now
via the docker buildx build. We will need to add support for custom
buildkit builders in binderhub, and then we can use those.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants