Docker backend #127

MisterDA · 2022-10-17T12:31:10Z

No description provided.

tmcgilchrist · 2023-01-09T01:34:16Z

lib/docker.mli

+    of [head] and [cmd] to be given to Docker command, as Docker
+    [--entrypoint] takes only one argument. *)
+
+(** Wrappers for various Docker client commands, exposing file descritors. *)


Suggested change

(** Wrappers for various Docker client commands, exposing file descritors. *)

(** Wrappers for various Docker client commands, exposing file descriptors. *)

tmcgilchrist · 2023-01-09T02:54:39Z

stress/stress.ml

+    let module Builder = Obuilder.Builder(Store)(Native_sandbox)(Obuilder.Docker_extract) in
+    Native_sandbox.create ~state_dir:(Store.state_dir store / "sandbox") conf >|= fun sandbox ->
+    let builder = Builder.v ~store ~sandbox in
+    Builder ((module Builder), builder)


It would be useful to have a stress test for the obuilder on Docker setup.

tmcgilchrist · 2023-01-09T03:19:38Z

lib/tar_transfer.mli

+(** [transform_files ~src_dir ~from_tar ~src_manifest ~dst ~user ~to_untar]
+    renames the _unique_ file found in [from_tar], a tar archive streamed in
+    input, to [dst], and writes the resulting tar-format stream to
+    [to_untar]. All files are listed as being owned by [user]. *)


Add tests for this?

tmcgilchrist · 2023-01-09T03:20:56Z

lib/s.ml

@@ -128,3 +128,106 @@ module type FETCHER = sig
      @param log Used for outputting the progress of the fetch
      @param rootfs The directory in which to extract the base image *)
 end
+
+(** Wrappers for various Docker client commands.  *)
+module type DOCKER_CMD = sig


Write documentation comments for this module.

tmcgilchrist · 2023-01-09T03:25:40Z

lib/docker.ml

+  let stderr = Option.value ~default:(`FD_move_safely Os.stderr) stderr in
+  Os.exec_result ~stdin ~stdout ~stderr ?is_success ~pp cmd
+
+module Cmd = struct


Good to have integration tests for these commands. Run against docker linux, macos and windows.

tmcgilchrist · 2023-01-09T03:27:03Z

lib/docker.ml

+  let run_result ?stdin ~log ?name ?rm docker_argv image argv =
+    with_log ~log (fun ~stdout ~stderr ->
+        Cmd.run_result ?stdin ~stdout ~stderr ?name ?rm docker_argv image argv)
+
+  let run_result' ?stdin ?stdout ~log ?name ?rm docker_argv image argv =
+    with_stderr_log ~log (fun ~stderr ->
+        Cmd.run_result' ?stdin ?stdout ~stderr ?name ?rm docker_argv image argv)


Why have these almost duplicate functions?

I want some invocations of Docker to log directly to the build log, but not all. Likewise, some of the logging has to be captured, and some can be ignored.
Granted, this could be a lot cleaner, but my hope is to be able to talk with the Docker daemon with its REST API rather than the command-line client.

tmcgilchrist · 2023-01-09T03:29:08Z

lib/build.mli

+  include S.BUILDER with type context := Context.t
+
+  val v : store:Store.t -> sandbox:Docker_sandbox.t -> t
+end


Question here, why is this necessary and not just have a Docker version of S.STORE and S.SANDBOX?

The problem is that the concept of store and sandbox isn't as tightly decoupled with Docker. There are pieces of info that need sharing in that context that we don't need with runc and fs stores. It was a problem with the build IDs, and when to clean the images and the containers (Docker_sandbox.teardown), which makes the Builder code slightly different. Besides, you can't swap the Docker sandbox or store with a non-Docker sandbox or store while using the other Docker store or sandbox, so it makes some sense to integrate them fully.
Another option would be to write an even more generic Builder to encompass all use cases, but this was rejected when I started writing the backend on the account that it might break the non-Docker builds.

using Alcotest 1.7.0

MisterDA · 2023-04-07T17:08:40Z

Alea jacta est

@MisterDA

CHANGES: - Add a Docker backend for Windows and Linux jobs. (@MisterDA ocurrent/obuilder#127 ocurrent/obuilder#75, reviewed by @talex5 and @tmcgilchrist) - Add FreeBSD sandbox backend using jail(8) (@dustanddreams ocurrent/obuilder#156 ocurrent/obuilder#174, reviewed by @tmcgilchrist, @MisterDA, and @mtelvers) - Add Macos ZFS sandbox (@mtelvers ocurrent/obuilder#164, reviewed by @tmcgilchrist) - Support XFS store (@mtelvers ocurrent/obuilder#170, reviewed by @tmcgilchrist) - Search for bash rather than assume it lies in /bin (@dustanddreams ocurrent/obuilder#159, reviewed by @tmcgilchrist) - Prune builds one at a time up to the limit (@mtelvers ocurrent/obuilder#157) - Specify upper bound on number of items in the store (@mtelvers ocurrent/obuilder#158, reviewed by @MisterDA) - Fix case where BTRFS is not fully allocated (@mtelvers ocurrent/obuilder#162) - Avoid pruning parent cache objects (@mtelvers ocurrent/obuilder#176, reviewed by @tmcgilchrist)

@MisterDA

CHANGES: - Add a Docker backend for Windows and Linux jobs. (@MisterDA ocurrent/obuilder#127 ocurrent/obuilder#75, reviewed by @talex5 and @tmcgilchrist) - Add FreeBSD sandbox backend using jail(8) (@dustanddreams ocurrent/obuilder#156 ocurrent/obuilder#174, reviewed by @tmcgilchrist, @MisterDA, and @mtelvers) - Add Macos ZFS sandbox (@mtelvers ocurrent/obuilder#164, reviewed by @tmcgilchrist) - Support XFS store (@mtelvers ocurrent/obuilder#170, reviewed by @tmcgilchrist) - Search for bash rather than assume it lies in /bin (@dustanddreams ocurrent/obuilder#159, reviewed by @tmcgilchrist) - Prune builds one at a time up to the limit (@mtelvers ocurrent/obuilder#157) - Specify upper bound on number of items in the store (@mtelvers ocurrent/obuilder#158, reviewed by @MisterDA) - Fix case where BTRFS is not fully allocated (@mtelvers ocurrent/obuilder#162) - Avoid pruning parent cache objects (@mtelvers ocurrent/obuilder#176, reviewed by @tmcgilchrist)

MisterDA force-pushed the docker2 branch 3 times, most recently from 984772f to a2b9bea Compare October 19, 2022 17:34

MisterDA marked this pull request as draft November 2, 2022 14:37

MisterDA force-pushed the docker2 branch from a2b9bea to fd0725d Compare November 7, 2022 15:37

MisterDA force-pushed the docker2 branch 2 times, most recently from af61709 to 1dcd93a Compare November 18, 2022 11:15

MisterDA mentioned this pull request Jan 4, 2023

Hook Obuilder Docker backend ocurrent/ocluster#143

Merged

1 task

MisterDA force-pushed the docker2 branch 3 times, most recently from be3dcdd to 805244a Compare January 5, 2023 16:59

tmcgilchrist reviewed Jan 9, 2023

View reviewed changes

MisterDA force-pushed the docker2 branch 4 times, most recently from 27f2f8d to e6228e4 Compare January 13, 2023 11:25

MisterDA force-pushed the docker2 branch 2 times, most recently from 8d4e204 to 163904e Compare January 17, 2023 13:34

MisterDA force-pushed the docker2 branch 3 times, most recently from 8099bb6 to b04dbcf Compare February 2, 2023 10:13

MisterDA force-pushed the master branch 2 times, most recently from 89e84ac to ccd3f2f Compare February 3, 2023 12:39

MisterDA force-pushed the docker2 branch from b04dbcf to 8b63966 Compare February 10, 2023 13:20

MisterDA force-pushed the docker2 branch 2 times, most recently from 525cf43 to add2cde Compare February 13, 2023 16:56

MisterDA force-pushed the docker2 branch from 5d92f71 to 847955c Compare February 27, 2023 12:25

MisterDA force-pushed the docker2 branch 4 times, most recently from 0281700 to 856bb3c Compare March 15, 2023 19:23

MisterDA marked this pull request as ready for review March 15, 2023 22:41

MisterDA added 9 commits April 4, 2023 17:56

Introduce a Docker backend in OBuilder for Windows and Linux

34b1009

Check that there isn't a left-over container of the build

aa15e27

gha: restore testing Docker backend on Ubuntu

f9114d5

Document the Docker module

a0744f7

Remove dead code

079d242

Update examples

905f940

Remove outdated Windows description

e99fa9a

Skip tests instead of not adding them

2276757

using Alcotest 1.7.0

Explicitly preserve permissions when extracting tarballs

72f552d

MisterDA force-pushed the docker2 branch 3 times, most recently from 7379de6 to 6038f2a Compare April 7, 2023 16:35

Add a document explaining design choices in the Docker backend

87692c1

MisterDA force-pushed the docker2 branch from 6038f2a to 87692c1 Compare April 7, 2023 17:07

MisterDA merged commit 59c4b29 into ocurrent:master Apr 7, 2023

MisterDA deleted the docker2 branch April 7, 2023 17:09

benmandrew mentioned this pull request Jan 31, 2024

[new release] obuilder (2 packages) (0.6.0) ocaml/opam-repository#25147

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docker backend #127

Docker backend #127

MisterDA commented Oct 17, 2022

tmcgilchrist Jan 9, 2023

tmcgilchrist Jan 9, 2023

tmcgilchrist Jan 9, 2023

tmcgilchrist Jan 9, 2023

tmcgilchrist Jan 9, 2023

tmcgilchrist Jan 9, 2023

MisterDA Jan 12, 2023

tmcgilchrist Jan 9, 2023

MisterDA Jan 9, 2023 •

edited

Loading

MisterDA commented Apr 7, 2023

	(** Wrappers for various Docker client commands, exposing file descritors. *)
	(** Wrappers for various Docker client commands, exposing file descriptors. *)

Docker backend #127

Docker backend #127

Conversation

MisterDA commented Oct 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MisterDA Jan 9, 2023 • edited Loading

Choose a reason for hiding this comment

MisterDA commented Apr 7, 2023

MisterDA Jan 9, 2023 •

edited

Loading