Skip to content

Docker backend#75

Closed
MisterDA wants to merge 26 commits intoocurrent:masterfrom
MisterDA:docker
Closed

Docker backend#75
MisterDA wants to merge 26 commits intoocurrent:masterfrom
MisterDA:docker

Conversation

@MisterDA
Copy link
Copy Markdown
Contributor

@MisterDA MisterDA commented Jul 1, 2021

This PR implements the Docker backend for OBuilder. It should support running on Windows with Docker for Windows, and on Linux with the usual Docker.
This PR contains a batch of prerequisites, and then the backend.
I have tested it on Windows and Linux, on basic examples.

Copy link
Copy Markdown
Contributor

@talex5 talex5 left a comment

Choose a reason for hiding this comment

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

There shouldn't be any docker-specific stuff in build.ml. Can the sandbox API be extended to allow making all this generic?

@talex5
Copy link
Copy Markdown
Contributor

talex5 commented Jul 6, 2021

Thinking on about this... the Build module creates a builder from a store and a sandbox. That will work on Linux (runc+btrfs), mac (users+zfs) and probably on FreeBSD (jails+zfs). But it doesn't work with Docker, because Docker already combines a store and a sandbox. So probably this should be an alternative to the build.ml module instead.

@MisterDA
Copy link
Copy Markdown
Contributor Author

MisterDA commented Jul 6, 2021

Thanks for the review, I agree with your conclusion and will refactor the code to merge the Docker sandbox, Docker_storage, the Docker-related code in build.ml, and maybe the code to talk to the client in the Docker module into a single Docker_build module.

A note regarding the use of the Docker Engine API: I wanted to replace the process calls to the docker client with the Docker Engine API, talking directly to dockerd with sockets or pipes. However

I'm putting this on hold for now, and I'll stay with the docker client until openapi-generator is fixed.

@tmcgilchrist tmcgilchrist mentioned this pull request Oct 10, 2022
6 tasks
@MisterDA MisterDA mentioned this pull request Oct 10, 2022
7 tasks
Otherwise embedded text get crlf line endings.
Mounts are read-write to preserve current behaviour.
This allows to specify the entrypoint of a `Sandbox.run job`. This is
only needed for the Docker sandbox, to override the Docker image
entrypoint.
Granted, it's more difficult to read, but way easier to copy-paste to
inspect manually what's happening.
This prevents errors on Windows where a file cannot be renamed, moved
or deleted if it is open by another process. It also ensure that the
database is left in a clean state.
…q-cmdliner', 'MisterDA/docker-windows-spec' and 'MisterDA/docker-windows-prerequisites' into docker
Checking the result for the Docker backend requires asynchronous calls
to Docker.
With the Docker backend on Windows, it's easier to store logs in a
location where they're not moved, as a directory cannot be renamed if
it contains open file descriptors.

For the btrfs and zfs backends, the original location is retained.
@MisterDA
Copy link
Copy Markdown
Contributor Author

I lost the race to merge before macOS. The rebase wasn't easy, I've probably broken one or two new things in my code.

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