Vendoring solver service #634

moyodiallo · 2022-11-24T13:37:00Z

Solver-service replace the local solver. And ocam-ci-service can submit for solving to a scheduler that manage a solver-worker.

ocaml-ci-service can be depolyed like the previous version, and also possible to launch it by providing a submission cap file for a scheduler that manage a solver-worker.

moyodiallo · 2022-11-28T12:52:58Z

It is a specific branch (https://github.com/ocurrent/solver-service/tree/solver-ci) that is vendored. The difference to the main (https://github.com/ocurrent/solver-service/tree/main) is that all the vendored dependencies is removed, because of some conflict (ocaml-dockerfile, ocurrent, ocluster).

After the merge of ocaml/dune#6577 and the next release after that, it's going to be OK to vendor the main branch of solver-service and override its conflict dependencies.

MisterDA

This looks great, I'll test tomorrow and I'll test on Windows too!

test/web/test.mli

service/local.ml

service/main.ml

maiste · 2022-11-29T15:02:26Z

Just a note for GitHub to simplify issue tracking. This addresses #446.

MisterDA · 2022-12-01T16:00:25Z

Can you rebase? There are some conflicts introduced by Etienne's merge of migrations.

moyodiallo · 2022-12-01T17:29:11Z

Rebase done.

maiste

LGTM 👍
Small question but will the solver-service be released in opam? The current system removes the possibility of building ocaml-ci deps with opam install --deps-only as it can't find the solver service.

gitlab/main.ml

service/local.ml

service/main.ml

moyodiallo · 2022-12-02T15:58:15Z

Small question but will the solver-service be released in opam?

I think this is the goal, but at the moment it's not a big deal because of vendoring make it as a part of the project.

MisterDA

lib/solver_pool.ml is dead code, remove this file.

MisterDA · 2022-12-05T10:38:37Z

I'm ok with the changes, but I've opened a bunch of PRs on upstream solver-service. I'd like your input there, and then you can update the submodule here, and I'll keep testing on Windows after.

maiste · 2022-12-05T11:42:24Z

Should this be deployed on live-engine with GitHub to ensure it can support the charge?

moyodiallo · 2022-12-05T12:01:48Z

Should this be deployed on live-engine with GitHub to ensure it can support the charge?

Not ready yet, I talk with @mtelvers and @tmcgilchrist about this deployment and we notice it needs some investigation about how many process is spawned for each request. It seems each time the solver-worker receive a request there's more than one process spawned for solving. And what expected is one request for one process(make more sense).

moyodiallo · 2022-12-13T18:13:55Z

After the merge of ocurrent/solver-service#30 and ocurrent/solver-service#31, I think it's OK to merge and start deploying in order to test the change. We know now, how the solver-service is spawning processes (ocurrent/solver-service#31).

@tmcgilchrist @mtelvers

moyodiallo · 2022-12-15T14:23:38Z

This is PR is ready to be merged. Any suggestion ?

maiste · 2022-12-15T16:51:17Z

Could we try it on live-engine before merging as we have done for the MacOS worker?

maiste

This version is working on my local test 👍

tmcgilchrist · 2023-01-03T01:28:20Z

Dockerfile.gitlab

@@ -34,8 +39,11 @@ RUN opam pin add -yn current_docker.dev "./ocurrent" && \
    opam pin add -yn ocaml-version.dev "./ocaml-version" && \
    opam pin add -yn dockerfile.dev "./ocaml-dockerfile" && \
    opam pin add -yn dockerfile-opam.dev "./ocaml-dockerfile" && \
+    opam pin add -yn solver-service-api.dev "./solver-service" && \
+    opam pin add -yn solver-service.dev "./solver-service" && \
+    opam pin add -yn solver-worker.dev "./solver-service" && \


Suggested change

opam pin add -yn solver-worker.dev "./solver-service" && \

Pinning the solver-worker is not required here.

~~You're right, even solver-service.dev is not needed, only solver-service_api.dev~~

tmcgilchrist · 2023-01-03T01:28:28Z

Dockerfile.gitlab

+COPY --chown=opam \
+	solver-service/solver-service-api.opam \
+	solver-service/solver-service.opam \
+	solver-service/solver-worker.opam \


Suggested change

solver-service/solver-worker.opam \

tmcgilchrist · 2023-01-03T01:42:07Z

Dockerfile

+COPY --chown=opam \
+	solver-service/solver-service-api.opam \
+	solver-service/solver-service.opam \
+	solver-service/solver-worker.opam \


Suggested change

solver-service/solver-worker.opam \

tmcgilchrist · 2023-01-03T01:42:17Z

Dockerfile

@@ -32,8 +37,11 @@ RUN opam pin add -yn current_docker.dev "./ocurrent" && \
    opam pin add -yn ocaml-version.dev "./ocaml-version" && \
    opam pin add -yn dockerfile.dev "./ocaml-dockerfile" && \
    opam pin add -yn dockerfile-opam.dev "./ocaml-dockerfile" && \
+    opam pin add -yn solver-service-api.dev "./solver-service" && \
+    opam pin add -yn solver-service.dev "./solver-service" && \
+    opam pin add -yn solver-worker.dev "./solver-service" && \


Suggested change

opam pin add -yn solver-worker.dev "./solver-service" && \

tmcgilchrist · 2023-01-03T01:43:27Z

lib/backend_solver.ml

+
+type t =
+  [ `Remote of Current_ocluster.Connection.t
+  | `Local of Ocaml_ci_api.Solver.t Lwt.t ]


Since these constructors aren't used elsewhere they can just be regular variants.

tmcgilchrist · 2023-01-03T01:45:13Z

lib/platform.ml

@@ -140,7 +140,7 @@ module Query = struct
  let run No_context job { Key.docker_context; variant }
      { Value.image; host_image } =
    Current.Job.start job ~level:Current.Level.Mostly_harmless >>= fun () ->
-    let prep_image = Fmt.str "ocurrent/ocaml-ci:%a" Variant.pp variant in
+    let prep_image = "ocurrent/ocaml-ci:" ^ Variant.docker_tag variant in


Suggested change

let prep_image = "ocurrent/ocaml-ci:" ^ Variant.docker_tag variant in

let prep_image = Fmt.str "ocurrent/ocaml-ci:%s" (Variant.docker_tag variant) in

tmcgilchrist · 2023-01-03T01:53:46Z

test/service/test_spec.ml

@@ -62,7 +62,7 @@ let expected_linux_spec =
 (* Create testable Sexp for Alcotest. *)
 let sexp = Alcotest.testable Sexplib0__Sexp.pp_hum Sexplib0__Sexp.equal

-let test_macos_spec () =
+let test_macos_spec _ () =


I don't understand why these tests need to use Alcotest LWT now. They do not require any LWT.

This avoids splitting the tests in LWT and none.

And to fit the tests in two categories test/service and test/web.

test/service/dune

The solver move to https://github.com/ocurrent/solver-service, and become a service. It's also possible to run the local pipeline with remote solver(cap file) or local solver(spawning a local solver).

moyodiallo force-pushed the vendoring-solver-service branch 2 times, most recently from 3b1dfb3 to ea3a65c Compare November 25, 2022 09:09

moyodiallo requested review from MisterDA, tmcgilchrist, novemberkilo and maiste November 25, 2022 09:41

MisterDA requested changes Nov 28, 2022

View reviewed changes

test/web/test.mli Outdated Show resolved Hide resolved

service/local.ml Outdated Show resolved Hide resolved

service/main.ml Outdated Show resolved Hide resolved

moyodiallo force-pushed the vendoring-solver-service branch from ee68148 to 9b22024 Compare December 1, 2022 17:22

maiste approved these changes Dec 2, 2022

View reviewed changes

gitlab/main.ml Outdated Show resolved Hide resolved

service/local.ml Outdated Show resolved Hide resolved

service/main.ml Outdated Show resolved Hide resolved

MisterDA self-requested a review December 3, 2022 11:27

MisterDA requested changes Dec 3, 2022

View reviewed changes

moyodiallo requested review from MisterDA and maiste December 3, 2022 12:02

MisterDA approved these changes Dec 5, 2022

View reviewed changes

tmcgilchrist mentioned this pull request Dec 13, 2022

Adapt solver-service for ocaml-ci and ocaml-docs-CI ocurrent/solver-service#17

Open

5 tasks

moyodiallo force-pushed the vendoring-solver-service branch from b8862d1 to feae61e Compare December 13, 2022 17:52

moyodiallo force-pushed the vendoring-solver-service branch from feae61e to 0c48dd7 Compare December 15, 2022 11:20

moyodiallo force-pushed the vendoring-solver-service branch 2 times, most recently from b126e21 to 1ec71a2 Compare December 16, 2022 09:51

moyodiallo force-pushed the vendoring-solver-service branch from 1ec71a2 to df4c7b7 Compare December 16, 2022 10:53

maiste approved these changes Dec 16, 2022

View reviewed changes

tmcgilchrist reviewed Jan 3, 2023

View reviewed changes

test/service/dune Outdated Show resolved Hide resolved

moyodiallo added 9 commits January 3, 2023 11:58

Vendoring solver-service and delete local solver

2d292ca

The solver move to https://github.com/ocurrent/solver-service, and become a service. It's also possible to run the local pipeline with remote solver(cap file) or local solver(spawning a local solver).

Fix job starting twice

e11c510

Update dockerfile

b7d8749

Fix little concerns

5a74da5

Add to solver .cap file parameter to gitlab

15109b0

Fix little concerns

2d1dd77

Some fix after rebase

a95e5c8

Update submodules

7ab8529

Fix some concerns

e59612d

moyodiallo force-pushed the vendoring-solver-service branch from 317ec42 to e59612d Compare January 3, 2023 11:07

moyodiallo requested a review from tmcgilchrist January 3, 2023 11:09

Tidy up tests and remove un-used solver code.

7680d01

tmcgilchrist force-pushed the vendoring-solver-service branch from c069a4d to 7680d01 Compare January 4, 2023 05:30

tmcgilchrist merged commit 2b8bc91 into ocurrent:master Jan 4, 2023

maiste mentioned this pull request Jan 10, 2023

Make ocaml-ci solver an ocluster service. #446

Closed

moyodiallo deleted the vendoring-solver-service branch December 5, 2023 14:22

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vendoring solver service #634

Vendoring solver service #634

moyodiallo commented Nov 24, 2022

moyodiallo commented Nov 28, 2022

MisterDA left a comment

maiste commented Nov 29, 2022

MisterDA commented Dec 1, 2022

moyodiallo commented Dec 1, 2022

maiste left a comment •

edited

Loading

moyodiallo commented Dec 2, 2022

MisterDA left a comment

MisterDA commented Dec 5, 2022

maiste commented Dec 5, 2022

moyodiallo commented Dec 5, 2022

moyodiallo commented Dec 13, 2022

moyodiallo commented Dec 15, 2022

maiste commented Dec 15, 2022

maiste left a comment

tmcgilchrist Jan 3, 2023

moyodiallo Jan 3, 2023 •

edited

Loading

tmcgilchrist Jan 3, 2023

tmcgilchrist Jan 3, 2023

tmcgilchrist Jan 3, 2023

tmcgilchrist Jan 3, 2023

tmcgilchrist Jan 3, 2023

tmcgilchrist Jan 3, 2023

moyodiallo Jan 3, 2023

moyodiallo Jan 3, 2023

	let prep_image = "ocurrent/ocaml-ci:" ^ Variant.docker_tag variant in
	let prep_image = Fmt.str "ocurrent/ocaml-ci:%s" (Variant.docker_tag variant) in

Vendoring solver service #634

Vendoring solver service #634

Conversation

moyodiallo commented Nov 24, 2022

moyodiallo commented Nov 28, 2022

MisterDA left a comment

Choose a reason for hiding this comment

maiste commented Nov 29, 2022

MisterDA commented Dec 1, 2022

moyodiallo commented Dec 1, 2022

maiste left a comment • edited Loading

Choose a reason for hiding this comment

moyodiallo commented Dec 2, 2022

MisterDA left a comment

Choose a reason for hiding this comment

MisterDA commented Dec 5, 2022

maiste commented Dec 5, 2022

moyodiallo commented Dec 5, 2022

moyodiallo commented Dec 13, 2022

moyodiallo commented Dec 15, 2022

maiste commented Dec 15, 2022

maiste left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moyodiallo Jan 3, 2023 • edited Loading

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

Choose a reason for hiding this comment

maiste left a comment •

edited

Loading

moyodiallo Jan 3, 2023 •

edited

Loading