Skip to content

Draft: Properly handle empty repository.ff_merge in FromTrainRef merge strategy

Hordur Freyr Yngvason requested to merge handle-empty-ff-merge-in-from-train-ref-strategy into master

What does this MR do and why?

The Gitaly API returns an empty response instead of an error if the ref update portion of the fast-forward merge fails due to a race with another ref upate. This could cause a merge request to incorrectly appear merged if the target branch got updated while merge train was performing a fast-forward merge.

See https://gitlab.com/gitlab-com/ops-sub-department/section-ops-request-for-help/-/issues/377#note_2035700772

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

You can simulate the race condition as follows:

  1. Review the gitaly contribution guide
  2. In your GDK's gitaly source, modify internal/gitaly/service/operations/ff_branch.go to add a time.Sleep(1 * time.Minute) just before the call to s.updateReferenceWithHooks(...), run make build in the gitaly directory and restart gitaly with gdk restart gitaly.
  3. In a project, enable merge trains and the fast-forward merge method
  4. Create a merge request
  5. Start the merge traina
  6. Wait for the pipeline to pass
  7. 10-30 seconds after the pipeline passes, push a change to the target branch
  8. If the push succeeds, proceed to validate results. If the push fails, you need to try again with a new MR

Expected results:

  • Without this change, the merge succeeds but the MR's code does not land on the target branch
  • With this code, the merge should fail
Edited by Hordur Freyr Yngvason

Merge request reports