From d29386e0749e20a16491a1e29373f0feb911d6de Mon Sep 17 00:00:00 2001 From: benmandrew Date: Wed, 10 Jan 2024 13:09:07 +0100 Subject: [PATCH] Separate triggering and nontriggering Docker build arguments --- api/docker.ml | 16 ++++++----- api/docker.mli | 3 ++- api/schema.capnp | 18 +++++++------ bin/client.ml | 20 +++++++++----- ocurrent-plugin/current_ocluster.ml | 42 +++++++++++++++++++++++------ worker/cluster_worker.ml | 3 ++- 6 files changed, 72 insertions(+), 30 deletions(-) diff --git a/api/docker.ml b/api/docker.ml index a4d1decc..8cd0f3bd 100644 --- a/api/docker.ml +++ b/api/docker.ml @@ -36,7 +36,8 @@ module Spec = struct } type options = { - build_args : string list; + triggering_build_args : string list; + nontriggering_build_args : string list; squash : bool; buildkit: bool; include_git : bool [@default true]; @@ -49,7 +50,8 @@ module Spec = struct } let defaults = { - build_args = []; + triggering_build_args = []; + nontriggering_build_args = []; squash = false; buildkit = false; include_git = false; @@ -64,8 +66,9 @@ module Spec = struct | `Contents contents -> Dockerfile.contents_set dockerfile_b contents | `Path path -> Dockerfile.path_set dockerfile_b path end; - let { build_args; squash; buildkit; include_git } = options in - DB.build_args_set_list b build_args |> ignore; + let { triggering_build_args; nontriggering_build_args; squash; buildkit; include_git } = options in + DB.triggering_build_args_set_list b triggering_build_args |> ignore; + DB.nontriggering_build_args_set_list b nontriggering_build_args |> ignore; DB.squash_set b squash; DB.buildkit_set b buildkit; DB.include_git_set b include_git; @@ -89,11 +92,12 @@ module Spec = struct let target = R.push_target_get r in let user = R.push_user_get r in let password = R.push_password_get r in - let build_args = R.build_args_get_list r in + let triggering_build_args = R.triggering_build_args_get_list r in + let nontriggering_build_args = R.nontriggering_build_args_get_list r in let squash = R.squash_get r in let buildkit = R.buildkit_get r in let include_git = R.include_git_get r in - let options = { build_args; squash; buildkit; include_git } in + let options = { triggering_build_args; nontriggering_build_args; squash; buildkit; include_git } in let push_to = match target, user, password with | "", "", "" -> None diff --git a/api/docker.mli b/api/docker.mli index 8ad8ceca..5ecc573c 100644 --- a/api/docker.mli +++ b/api/docker.mli @@ -18,7 +18,8 @@ module Spec : sig } type options = { - build_args : string list; (** "--build-arg" arguments. *) + triggering_build_args : string list; (** "--build-arg" arguments, changing these arguments triggers an OCurrent rebuild *) + nontriggering_build_args : string list; (** Changing these arguments does not trigger a rebuild *) squash : bool; buildkit: bool; include_git : bool; diff --git a/api/schema.capnp b/api/schema.capnp index 1a8c576d..0bfb84f9 100644 --- a/api/schema.capnp +++ b/api/schema.capnp @@ -5,11 +5,11 @@ struct DockerBuild { contents @0 :Text; # The contents of the Dockerfile to build. - path @5 :Text; + path @1 :Text; # The path of the Dockerfile within the context. } - pushTarget @1 :Text; + pushTarget @2 :Text; # If set, the builder will "docker push" to this target on success. # The format is "repo:tag". The tag is not optional. # You'll probably want to provide pushUser and pushPassword too when using this. @@ -18,21 +18,23 @@ struct DockerBuild { # RepoId hash to tag it in the final repository yourself. # Example value: "myorg/staging:job-123" - pushUser @2 :Text; - pushPassword @3 :Text; + pushUser @3 :Text; + pushPassword @4 :Text; - buildArgs @4 :List(Text); + triggeringBuildArgs @5 :List(Text); + nontriggeringBuildArgs @6 :List(Text); # Options to pass to `docker build` using `--build-arg`. + # Triggering args trigger OCurrent rebuilds on changing, nontriggering args do not. - squash @6 :Bool; + squash @7 :Bool; # Squash the image layers together using `--squash`. - buildkit @7 :Bool; + buildkit @8 :Bool; # Use BuildKit for the build. # Note: buildkit builds shared caches, so clients using such builds must all # trust each other not to poison the caches. - includeGit @8 :Bool; + includeGit @9 :Bool; # If set, include the .git directory in the build context. } diff --git a/bin/client.ml b/bin/client.ml index 30fcda51..d022c30b 100644 --- a/bin/client.ml +++ b/bin/client.ml @@ -192,13 +192,21 @@ let push_password_file = ~docv:"PATH" ["push-password"] -let build_args = +let triggering_build_args = Arg.value @@ Arg.(opt_all string) [] @@ Arg.info - ~doc:"Docker build argument." + ~doc:"Docker build argument; triggers OCurrent rebuild on change." ~docv:"ARG" - ["build-arg"] + ["build-arg"; "triggering-build-arg"] + +let nontriggering_build_args = + Arg.value @@ + Arg.(opt_all string) [] @@ + Arg.info + ~doc:"Docker build argument; doesn't trigger OCurrent rebuild on change." + ~docv:"ARG" + ["nontriggering-build-arg"] let secrets = (Arg.value @@ @@ -246,10 +254,10 @@ let push_to = Term.(const make $ push_to $ push_user $ push_password_file) let build_options = - let make build_args squash buildkit include_git = - { Cluster_api.Docker.Spec.build_args; squash; buildkit; include_git } + let make triggering_build_args nontriggering_build_args squash buildkit include_git = + { Cluster_api.Docker.Spec.triggering_build_args; nontriggering_build_args; squash; buildkit; include_git } in - Term.(const make $ build_args $ squash $ buildkit $ include_git) + Term.(const make $ triggering_build_args $ nontriggering_build_args $ squash $ buildkit $ include_git) let submit_options_common = let make submission_path pool repository commits cache_hint urgent secrets = diff --git a/ocurrent-plugin/current_ocluster.ml b/ocurrent-plugin/current_ocluster.ml index 0bb3da67..9ce37fd2 100644 --- a/ocurrent-plugin/current_ocluster.ml +++ b/ocurrent-plugin/current_ocluster.ml @@ -49,7 +49,15 @@ module Op = struct let digest t = Yojson.Safe.to_string (to_yojson t) end - module Value = Current.String + module Value = struct + type t = { + nontriggering_build_args : string list; + } [@@deriving to_yojson] + + let digest t = Yojson.Safe.to_string (to_yojson t) + end + + module Outcome = Current.String (* Convert a list of commits in the same repository to a [(repo, hashes)] pair. Raises an exception if there are commits from different repositories. *) @@ -80,7 +88,7 @@ module Op = struct | Some line -> line | None -> "" - let build t job { Key.action; src; pool } = + let run t job { Key.action; src; pool } { Value.nontriggering_build_args } = let action = match action with | `Docker { dockerfile; options; push_target } -> @@ -91,9 +99,13 @@ module Op = struct ) in begin match dockerfile with - | `Contents content -> Current.Job.write job (Fmt.str "@.Dockerfile:@.@.\o033[34m%s\o033[0m@.@." content) + | `Contents content -> + Current.Job.write job (Fmt.str "@.Dockerfile:@.@.\o033[34m%s\o033[0m@.@." content) | `Path _ -> () end; + let options = + { options with nontriggering_build_args } + in Cluster_api.Submission.docker_build ?push_to ~options dockerfile | `Obuilder { spec = `Contents spec } -> Current.Job.write job (Fmt.str "@.OBuilder spec:@.@.\o033[34m%s\o033[0m@.@." spec); @@ -129,7 +141,7 @@ module Op = struct Capability.with_ref build_job @@ fun build_job -> Connection.run_job ~job build_job - let pp f {Key.action; src; pool } = + let pp f ({Key.action; src; pool }, _) = match action with | `Docker { dockerfile = `Path path; _ } -> Fmt.pf f "@[Build %s using %s in@,%a@]" @@ -141,9 +153,10 @@ module Op = struct (Fmt.Dump.list Git.Commit_id.pp) src let auto_cancel = true + let latched = true end -module Build = Current_cache.Make(Op) +module Build = Current_cache.Generic(Op) open Current.Syntax @@ -167,10 +180,18 @@ module Raw = struct | Some _ -> { t with level } | None -> t + let split_triggering_build_args options = + let nontriggering_build_args = options.Cluster_api.Docker.Spec.nontriggering_build_args in + let options = { options with nontriggering_build_args = [] } in + (options, nontriggering_build_args) + let build_and_push ?level ?cache_hint t ~push_target ~pool ~src ~options dockerfile = let t = with_hint ~cache_hint t in let t = with_level ~level t in - Build.get t { Op.Key.action = `Docker { dockerfile; options; push_target = Some push_target }; src; pool } + let options, nontriggering_build_args = split_triggering_build_args options in + Build.run t + { Op.Key.action = `Docker { dockerfile; options; push_target = Some push_target }; src; pool } + { Op.Value.nontriggering_build_args } |> Current.Primitive.map_result @@ function | Ok "" -> Error (`Msg "No output image (push auth not configured)") | Ok x -> Ok x @@ -179,7 +200,10 @@ module Raw = struct let build ?level ?cache_hint t ~pool ~src ~options dockerfile = let t = with_hint ~cache_hint t in let t = with_level ~level t in - Build.get t { Op.Key.action = `Docker {dockerfile; options; push_target = None}; src; pool } + let options, nontriggering_build_args = split_triggering_build_args options in + Build.run t + { Op.Key.action = `Docker {dockerfile; options; push_target = None}; src; pool } + { Op.Value.nontriggering_build_args } |> Current.Primitive.map_result (Result.map (function | "" -> () | x -> Fmt.failwith "BUG: got a RepoID (%S) but we didn't ask to push!" x @@ -188,7 +212,9 @@ module Raw = struct let build_obuilder ?level ?cache_hint t ~pool ~src spec = let t = with_hint ~cache_hint t in let t = with_level ~level t in - Build.get t { Op.Key.action = `Obuilder spec; src; pool } + Build.run t + { Op.Key.action = `Obuilder spec; src; pool } + { Op.Value.nontriggering_build_args = [] } |> Current.Primitive.map_result (Result.map (fun (_ : string) -> ())) end diff --git a/worker/cluster_worker.ml b/worker/cluster_worker.ml index 2f4f62bc..d8f56100 100644 --- a/worker/cluster_worker.ml +++ b/worker/cluster_worker.ml @@ -416,7 +416,8 @@ let default_build ?obuilder ~switch ~log ~src ~secrets = function | Ok path -> Lwt_result.return path | Error e -> Lwt_result.fail e end >>!= fun dockerpath -> - let { Cluster_api.Docker.Spec.build_args; squash; buildkit; include_git = _ } = options in + let { Cluster_api.Docker.Spec.triggering_build_args; nontriggering_build_args; squash; buildkit; include_git = _ } = options in + let build_args = triggering_build_args @ nontriggering_build_args in let args = List.concat_map (fun x -> ["--build-arg"; x]) build_args @ (if squash then ["--squash"] else [])