Skip to content

Commit

Permalink
Merge pull request #7 from terrateamio/pro-578-fix-apply-after-merge-…
Browse files Browse the repository at this point in the history 
…workflow

PRO-578 FIX Apply-after-merge workflow when the PR is not the last to…
  • Loading branch information
@orbitz
orbitz authored Oct 7, 2024
2 parents b8a756b + 8ce0f11 commit e82ba46
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 54 deletions.
136 changes: 107 additions & 29 deletions code/src/terrat/terrat_evaluator3.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,6 @@ module type S = sig
val base_ref : 'a t -> Ref.t
val branch_name : 'a t -> Ref.t
val branch_ref : 'a t -> Ref.t

(** The branch ref based on if the PR is merged or not. *)
val working_branch_ref : 'a t -> Ref.t

val diff : fetched t -> Terrat_change.Diff.t list
val id : 'a t -> int
val is_draft_pr : fetched t -> bool
Expand Down Expand Up @@ -1952,25 +1948,26 @@ module Make (S : S) = struct

let working_branch_ref ctx state =
let open Abbs_future_combinators.Infix_result_monad in
let default_branch_sha =
client ctx state
>>= fun client ->
fetch_remote_repo state.State.request_id client (Event.repo state.State.event)
>>= fun remote_repo ->
let default_branch = S.Remote_repo.default_branch remote_repo in
fetch_branch_sha state.State.request_id client (Event.repo state.State.event) default_branch
>>= function
| Some branch_sha -> Abb.Future.return (Ok branch_sha)
| None -> assert false
in
match Event.pull_request_id_safe state.State.event with
| Some _ ->
| Some _ -> (
pull_request ctx state
>>= fun pull_request ->
Abb.Future.return (Ok (S.Pull_request.working_branch_ref pull_request))
| None -> (
client ctx state
>>= fun client ->
fetch_remote_repo state.State.request_id client (Event.repo state.State.event)
>>= fun remote_repo ->
let default_branch = S.Remote_repo.default_branch remote_repo in
fetch_branch_sha
state.State.request_id
client
(Event.repo state.State.event)
default_branch
>>= function
| Some branch_sha -> Abb.Future.return (Ok branch_sha)
| None -> assert false)
match S.Pull_request.state pull_request with
| Terrat_pull_request.State.Open _ | Terrat_pull_request.State.Closed ->
Abb.Future.return (Ok (S.Pull_request.branch_ref pull_request))
| Terrat_pull_request.State.Merged _ -> default_branch_sha)
| None -> default_branch_sha

let base_ref ctx state =
let open Abbs_future_combinators.Infix_result_monad in
Expand Down Expand Up @@ -2677,12 +2674,18 @@ module Make (S : S) = struct
with
| [], work_manifests ->
let states = states_of_work_manifests work_manifests in
Abbs_future_combinators.List_result.iter
~f:(run_success ctx state)
work_manifests
>>= fun () ->
Abb.Future.return
(Error
(`Clone ({ state with State.st = St.Work_manifest_completed }, states)))
| [ self ], work_manifests ->
let state = { state with State.work_manifest_id = Some self.Wm.id } in
run_success ctx state self
Abbs_future_combinators.List_result.iter
~f:(run_success ctx state)
(self :: work_manifests)
>>= fun () ->
let states = states_of_work_manifests work_manifests in
Abb.Future.return
Expand Down Expand Up @@ -2795,11 +2798,13 @@ module Make (S : S) = struct
Some work_manifest_id ) -> (
Logs.info (fun m ->
m
"EVALUATOR : %s : WORK_MANIFEST_ITER : %s : INITIATE : id=%a"
"EVALUATOR : %s : WORK_MANIFEST_ITER : %s : INITIATE : id=%a : run_id=%s : sha=%s"
state.State.request_id
name
Uuidm.pp
work_manifest_id);
work_manifest_id
run_id
sha);
query_work_manifest state.State.request_id ctx.Ctx.storage work_manifest_id
>>= function
| Some ({ Wm.state = Wm.State.(Queued | Running); _ } as work_manifest) -> (
Expand Down Expand Up @@ -3087,6 +3092,23 @@ module Make (S : S) = struct
>>= fun base_ref' ->
Dv.repo_config ctx state
>>= fun repo_config ->
Dv.repo_tree_branch ctx state
>>= fun repo_tree ->
Dv.base_branch_name ctx state
>>= fun base_branch_name ->
Dv.branch_name ctx state
>>= fun branch_name ->
let repo_config =
Terrat_base_repo_config_v1.derive
~ctx:
(Terrat_base_repo_config_v1.Ctx.make
~dest_branch:(S.Ref.to_string base_branch_name)
~branch:(S.Ref.to_string branch_name)
())
~index:Terrat_base_repo_config_v1.Index.empty
~file_list:repo_tree
repo_config
in
let module I = Terrat_api_components.Work_manifest_index in
let dirs =
wm.Wm.changes
Expand Down Expand Up @@ -3492,10 +3514,13 @@ module Make (S : S) = struct
>>= fun work_manifest ->
Logs.info (fun m ->
m
"EVALUATOR : %s : CREATED_WORK_MANIFEST : id=%a : env=%s"
"EVALUATOR : %s : CREATED_WORK_MANIFEST : id=%a : base_ref=%s : branch_ref=%s : \
env=%s"
state.State.request_id
Uuidm.pp
work_manifest.Wm.id
(S.Ref.to_string base_ref)
(S.Ref.to_string branch_ref)
(CCOption.get_or ~default:"" work_manifest.Wm.environment));
run_interactive ctx state (fun () ->
Dv.client ctx state
Expand Down Expand Up @@ -3662,10 +3687,13 @@ module Make (S : S) = struct
>>= fun work_manifest ->
Logs.info (fun m ->
m
"EVALUATOR : %s : CREATED_WORK_MANIFEST : id=%a : env=%s"
"EVALUATOR : %s : CREATED_WORK_MANIFEST : id=%a : base_ref=%s : branch_ref=%s : \
env=%s"
state.State.request_id
Uuidm.pp
work_manifest.Wm.id
(S.Ref.to_string base_ref)
(S.Ref.to_string branch_ref)
(CCOption.get_or ~default:"" work_manifest.Wm.environment));
run_interactive ctx state (fun () ->
Dv.client ctx state
Expand Down Expand Up @@ -3813,6 +3841,29 @@ module Make (S : S) = struct
| Wm.Step.Plan ->
Dv.repo_config ctx state
>>= fun repo_config ->
Dv.repo_tree_branch ctx state
>>= fun repo_tree ->
Dv.base_branch_name ctx state
>>= fun base_branch_name ->
Dv.branch_name ctx state
>>= fun branch_name ->
Dv.query_index ctx state
>>= fun index ->
let repo_config =
Terrat_base_repo_config_v1.derive
~ctx:
(Terrat_base_repo_config_v1.Ctx.make
~dest_branch:(S.Ref.to_string base_branch_name)
~branch:(S.Ref.to_string branch_name)
())
~index:
(CCOption.map_or
~default:Terrat_base_repo_config_v1.Index.empty
(fun { Index.index; _ } -> index)
index)
~file_list:repo_tree
repo_config
in
Dv.dirspaces ctx state
>>= fun (base_dirspaces, dirspaces) ->
Abb.Future.return
Expand All @@ -3837,6 +3888,29 @@ module Make (S : S) = struct
| Wm.Step.(Apply | Unsafe_apply) ->
Dv.repo_config ctx state
>>= fun repo_config ->
Dv.repo_tree_branch ctx state
>>= fun repo_tree ->
Dv.base_branch_name ctx state
>>= fun base_branch_name ->
Dv.branch_name ctx state
>>= fun branch_name ->
Dv.query_index ctx state
>>= fun index ->
let repo_config =
Terrat_base_repo_config_v1.derive
~ctx:
(Terrat_base_repo_config_v1.Ctx.make
~dest_branch:(S.Ref.to_string base_branch_name)
~branch:(S.Ref.to_string branch_name)
())
~index:
(CCOption.map_or
~default:Terrat_base_repo_config_v1.Index.empty
(fun { Index.index; _ } -> index)
index)
~file_list:repo_tree
repo_config
in
Abb.Future.return
(Ok
(Some
Expand Down Expand Up @@ -4041,10 +4115,12 @@ module Make (S : S) = struct
>>= fun () ->
Logs.info (fun m ->
m
"EVALUATOR : %s : CREATED_WORK_MANIFEST : id=%a"
"EVALUATOR : %s : CREATED_WORK_MANIFEST : id=%a : base_ref=%s : branch_ref=%s"
state.State.request_id
Uuidm.pp
work_manifest.Wm.id);
work_manifest.Wm.id
(S.Ref.to_string base_ref')
(S.Ref.to_string working_branch_ref'));
Abb.Future.return (Ok [ work_manifest ]))
~update:(fun ctx state work_manifest ->
let module Wm = Terrat_work_manifest3 in
Expand Down Expand Up @@ -5308,10 +5384,12 @@ module Make (S : S) = struct
>>= fun () ->
Logs.info (fun m ->
m
"EVALUATOR : %s : CREATED_WORK_MANIFEST : id=%a"
"EVALUATOR : %s : CREATED_WORK_MANIFEST : id=%a : base_ref=%s : branch_ref=%s"
state.State.request_id
Uuidm.pp
work_manifest.Wm.id);
work_manifest.Wm.id
(S.Ref.to_string base_ref')
(S.Ref.to_string working_branch_ref'));
Abb.Future.return (Ok [ work_manifest ]))
~update:(fun ctx state work_manifest -> raise (Failure "nyi"))
~run_success:(fun ctx state work_manifest ->
Expand Down
4 changes: 0 additions & 4 deletions code/src/terrat/terrat_evaluator3.mli
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,6 @@ module type S = sig
val base_ref : 'a t -> Ref.t
val branch_name : 'a t -> Ref.t
val branch_ref : 'a t -> Ref.t

(** The branch ref based on if the PR is merged or not. *)
val working_branch_ref : 'a t -> Ref.t

val diff : fetched t -> Terrat_change.Diff.t list
val id : 'a t -> int
val is_draft_pr : fetched t -> bool
Expand Down
7 changes: 0 additions & 7 deletions code/src/terrat/terrat_github_evaluator3.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -773,13 +773,6 @@ module S = struct
let base_ref t = t.base_ref
let branch_name t = t.branch_name
let branch_ref t = t.branch_ref

let working_branch_ref t =
let module St = State in
match t.state with
| St.Open _ | St.Closed -> t.branch_ref
| St.Merged { St.Merged.merged_hash; _ } -> merged_hash

let diff t = t.value.diff
let id t = t.id
let is_draft_pr t = t.value.is_draft_pr
Expand Down
19 changes: 5 additions & 14 deletions code/src/terrat_files/tmpl/github_mismatched_refs.tmpl
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,7 @@
# Could not run operation
# Operation Canceled

## Why?
## Why did this happen?
This pull request was already merged, and another pull request was merged between the initiation of this operation and when the action was executed. To ensure accuracy, Terrateam cancels the operation if changes occur during this window.

This pull request is merged and another pull request has been merged after this pull request. In order to avoid drift, Terrateam runs all operations against the most recent contents of a branch. Because a pull request has been merged after this pull request, this pull request is not the most recent contents of the destination branch and Terrateam cannot run any operations against this pull request.

## What to do next?

The recommended action is to revert this change, then create a new pull request and plan and apply it.

## What to do in the future?

In order to avoid this, it is recommended to do one of the following:

1. If you wish to have an apply-after-merge workflow, enable `autoapply`. Terrateam will apply any changes immediately after merge. See the [documentation](https://terrateam.io/docs/advanced-workflows/apply-after-merge/) for how to enable `autoapply`.
2. Use an apply-before-merge workflow. This is the most robust because an apply may fail for a variety of reasons. An apply-before-merge workflow gives the most flexibility.
## What should you do?
Initiate the operation again via a GitHub comment. Terrateam will perform the operation against the most recent commit of the destination branch (HEAD).

0 comments on commit e82ba46

Please sign in to comment.