Skip to content

Make MergeService idempotent

Gary Holtz requested to merge 32737-make-mergeservice-idempotent into master

What does this MR do?

Fixes #32737 (closed)

The issue came up as a result of #32185 (comment 220152835), where some projects were getting the error message: Merge failed: Failed to squash. Should be done manually.. Please try again. This was partially fixed by !17406 (merged):

If a user who belongs to an approval group and also is an individual in the approval rule, MergeService will fail in the after_merge, which will cause MergeWorker to retry again. Since the merge has been successfully merged, MergeWorker will encounter an error since MergeService is not idempotent.

This change fixes the issue by using the Array |= join method to add an element to the Array, unless it is already present. This fixes the immediate bug, but we will have to address the idempotency issues later.

But leaves the issue that the two services are not idempotent:

  • MergeService is not idempotent. As a first step, we should not attempt to merge a merge request if it's already in the merged state. We may need to find ways to make the after_merge step idempotent as well.
  • SquashService is not idempotent. We should detect that the commits in question have already been squashed and not return an error.

The linked issue requests that:

  1. Add tests to ensure MergeService is idempotent
  2. SquashService should check whether the merge request is merged (e.g. MergeRequest#merged?).

Does this MR meet the acceptance criteria?

Conformity

Closes #32737 (closed)

Edited by Sean Carroll

Merge request reports