Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry PR merge if they fail for merge-check issues #702

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
2 changes: 1 addition & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ RSpec/NestedGroups:
Max: 4

RSpec/MultipleMemoizedHelpers:
Max: 12
Max: 15

RSpec/MessageChain:
Enabled: false
Expand Down
67 changes: 67 additions & 0 deletions app/jobs/commit/continuous_backmerge_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
class Commit::ContinuousBackmergeJob < ApplicationJob
MAX_RETRIES = 32
include Loggable
include Backoffable
queue_as :high

def perform(commit_id, is_head_commit: false, count: 0)
@commit = Commit.find(commit_id)

return if skip_backmerge?(is_head_commit)

result = release.with_lock do
return unless backmerge_allowed?
Triggers::PatchPullRequest.call(release, commit)
end

return unless result
commit.reload # ensure the commit is updated with the latest data, and no stale state is associated with it
return handle_success(commit.pull_request) if result.ok?
return handle_retry(count, is_head_commit) if retryable?(result)
handle_failure(result.error)
end

private

def handle_failure(err)
elog(err)
commit.update!(backmerge_failure: true)
release.event_stamp!(reason: :backmerge_failure, kind: :error, data: stamp_data)
commit.notify!("Backmerge to the working branch failed", :backmerge_failed, commit.notification_params)
end

def handle_retry(count, is_head_commit)
if count < MAX_RETRIES
attempt = count + 1
Commit::ContinuousBackmergeJob
.set(wait: backoff_in(attempt:, period: :minutes, type: :linear, factor: 5))
.perform_later(commit.id, is_head_commit:, count: attempt)
end
end

def skip_backmerge?(is_head_commit)
release.organization.single_pr_backmerge_for_multi_commit_push? && !is_head_commit
end

def backmerge_allowed?
train.almost_trunk? && train.continuous_backmerge? && release.committable? && release.stability_commit?(commit)
end

def retryable?(result)
!result.ok? && result.error.is_a?(Triggers::PullRequest::RetryableMergeError)
end

def handle_success(pr)
return unless pr
logger.debug { "Patch Pull Request: Created a patch PR successfully: #{pr}" }
release.event_stamp!(reason: :backmerge_pr_created, kind: :success, data: stamp_data(pr))
end

def stamp_data(pr = nil)
{url: pr&.url, number: pr&.number, commit_url: commit.url, commit_sha: commit.short_sha}
end

attr_reader :commit
delegate :train, to: :release
delegate :release, to: :commit
end
10 changes: 0 additions & 10 deletions app/jobs/releases/backmerge_commit_job.rb

This file was deleted.

4 changes: 1 addition & 3 deletions app/jobs/v2/trigger_submissions_job.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
class V2::TriggerSubmissionsJob < ApplicationJob
include Loggable

MAX_RETRIES = 3

include Loggable
queue_as :high

def perform(workflow_run_id, retry_count = 0)
Expand Down
1 change: 1 addition & 0 deletions app/jobs/webhooks/close_pull_request_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ def perform(train_id, pr_attributes)
Train.find(train_id).open_active_prs_for(head_ref).where(number:).find_each do |pr|
pr.update_or_insert!(pr_attributes)
pr.release.event_stamp!(reason: :pr_merged, kind: :success, data: {url: pr.url, number: pr.number, base_branch: pr.base_ref})
Action.complete_release!(pr.release) if pr.post_release?
end
end
end
4 changes: 2 additions & 2 deletions app/libs/coordinators/finalize_release.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ def call
result = HANDLERS[train.branching_strategy].call(release)
release.reload

if result.ok?
if result.ok? && release.pull_requests.open.none?
release.finish!
on_finish!
else
elog(result.error) unless result.ok?
release.fail_post_release_phase!
elog(result.error)
on_failure!
end
end
Expand Down
4 changes: 2 additions & 2 deletions app/libs/coordinators/process_commits.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ def fetch_commit_parents(commit)

def attempt_backmerge!(created_head_commit, created_rest_commits)
return unless created_head_commit
Releases::BackmergeCommitJob.perform_later(created_head_commit.id, is_head_commit: true)
created_rest_commits.each { |c| Releases::BackmergeCommitJob.perform_later(c.id, is_head_commit: false) }
Commit::ContinuousBackmergeJob.perform_later(created_head_commit.id, is_head_commit: true)
created_rest_commits.each { |c| Commit::ContinuousBackmergeJob.perform_later(c.id, is_head_commit: false) }
end

delegate :train, to: :release
Expand Down
11 changes: 2 additions & 9 deletions app/libs/installations/bitbucket/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -251,17 +251,10 @@ def get_commit(repo_slug, sha, transforms)
def patch_pr(repo_slug, branch, patch_branch_name, sha, pr_title, pr_description, transforms)
# create patch branch from the sha - 1 api call
# create a PR - 1 api call
# merge the PR - 1 api call
# get the updated PR - 1 api call

# TOTAL - 3-4 api calls
# TOTAL - 2 api calls
create_branch!(repo_slug, sha, patch_branch_name, source_type: :commit)
pr = create_pr!(repo_slug, branch, patch_branch_name, pr_title, pr_description, transforms, close_source_branch: true)
merge_pr!(repo_slug, pr[:number])
get_pr(repo_slug, pr[:number], transforms)
rescue Installations::Error => ex
raise ex unless ex.reason == :pull_request_not_mergeable
pr
create_pr!(repo_slug, branch, patch_branch_name, pr_title, pr_description, transforms, close_source_branch: true)
end

# CI/CD
Expand Down
4 changes: 2 additions & 2 deletions app/libs/installations/bitbucket/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ class Bitbucket::Error < Installations::Error
decorated_reason: :pull_request_not_mergeable
},
{
message_matcher: /failed merge checks/i,
decorated_reason: :pull_request_not_mergeable
message_matcher: /failed merge check/i,
decorated_reason: :pull_request_failed_merge_check
}
]

Expand Down
44 changes: 17 additions & 27 deletions app/libs/triggers/patch_pull_request.rb
Original file line number Diff line number Diff line change
@@ -1,35 +1,33 @@
class Triggers::PatchPullRequest
def self.create!(release, commit)
new(release, commit).create!
def self.call(release, commit)
new(release, commit).call
end

delegate :train, to: :release
delegate :working_branch, to: :train

def initialize(release, commit)
@release = release
@commit = commit
@pull_request = Triggers::PullRequest.new(
release: release,
new_pull_request: (commit.build_pull_request(release:, phase: :ongoing) if commit.pull_request.blank?),
to_branch_ref: working_branch,
from_branch_ref: patch_branch,
title: pr_title,
description: pr_description,
existing_pr: commit.pull_request,
patch_pr: true,
patch_commit: commit
)
end

def create!
GitHub::Result.new do
repo_integration.create_patch_pr!(working_branch, patch_branch, commit.commit_hash, pr_title, pr_description)
rescue Installations::Error => ex
raise ex unless ex.reason == :pull_request_already_exists
logger.debug { "Patch Pull Request: Pull Request Already exists for #{commit.short_sha} to #{working_branch}" }
repo_integration.find_pr(working_branch, patch_branch)
end.then do |value|
pr = commit.build_pull_request(release:, phase: :ongoing).update_or_insert!(**value)
logger.debug { "Patch Pull Request: Created a patch PR successfully: #{pr}" }
repo_integration.enable_auto_merge!(pr.number)
stamp_pr_success(pr)
GitHub::Result.new { value }
end
def call
@pull_request.create_and_merge!
end

private

delegate :logger, to: Rails
delegate :train, to: :release
delegate :working_branch, to: :train
attr_reader :release, :commit

def pr_title
Expand All @@ -48,12 +46,4 @@ def pr_description
def patch_branch
"patch-#{working_branch}-#{commit.short_sha}"
end

def stamp_pr_success(pr)
release.event_stamp!(reason: :backmerge_pr_created, kind: :success, data: {url: pr.url, number: pr.number, commit_url: commit.url, commit_sha: commit.short_sha}) if pr
end

def repo_integration
train.vcs_provider
end
end
41 changes: 32 additions & 9 deletions app/libs/triggers/pull_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ class Triggers::PullRequest

CreateError = Class.new(StandardError)
MergeError = Class.new(StandardError)
RetryableMergeError = Class.new(MergeError)

def self.create_and_merge!(**args)
new(**args).create_and_merge!
end

def initialize(release:, new_pull_request:, to_branch_ref:, from_branch_ref:, title:, description:, allow_without_diff: true, existing_pr: nil)
def initialize(release:, new_pull_request:, to_branch_ref:, from_branch_ref:, title:, description:, allow_without_diff: true, existing_pr: nil, patch_pr: false, patch_commit: nil)
@release = release
@to_branch_ref = to_branch_ref
@from_branch_ref = from_branch_ref
Expand All @@ -17,12 +18,14 @@ def initialize(release:, new_pull_request:, to_branch_ref:, from_branch_ref:, ti
@new_pull_request = new_pull_request
@allow_without_diff = allow_without_diff
@existing_pr = existing_pr
@patch_pr = patch_pr
@patch_commit = patch_commit
end

delegate :train, to: :release

def create_and_merge!
pr_in_work = existing_pr
pr_in_work = existing_pr if existing_pr&.persisted?

if pr_in_work.present?
pr_data = repo_integration.get_pr(pr_in_work.number)
Expand Down Expand Up @@ -51,6 +54,12 @@ def create_and_merge!
pr_in_work.reload
return GitHub::Result.new { pr_in_work } if pr_in_work.closed?

# enable auto-merge if possible and avoid merging manually
if enable_auto_merge?
repo_integration.enable_auto_merge!(pr_in_work.number)
return GitHub::Result.new { pr_in_work }
end

# try and merge, when:
# - create PR is successful
# - or PR already exists and is _not_ already closed
Expand All @@ -63,10 +72,10 @@ def create_and_merge!

def create!
GitHub::Result.new do
repo_integration.create_pr!(to_branch_ref, from_branch_ref, title, description)
create_new_pr!
rescue Installations::Error => ex
return repo_integration.find_pr(to_branch_ref, from_branch_ref) if ex.reason == :pull_request_already_exists
raise ex
raise ex unless ex.reason == :pull_request_already_exists
repo_integration.find_pr(to_branch_ref, from_branch_ref)
end
end

Expand All @@ -77,17 +86,31 @@ def merge!(pr)
if ex.reason == :pull_request_not_mergeable
release.event_stamp!(reason: :pull_request_not_mergeable, kind: :error, data: {url: pr.url, number: pr.number})
raise MergeError, "Failed to merge the Pull Request"
elsif ex.reason == :pull_request_failed_merge_check
raise RetryableMergeError, "Failed to merge the Pull Request because of merge checks."
else
raise ex
end
end
end

memoize def repo_integration
train.vcs_provider
end

def pr_without_commits_error?(result)
result.error.is_a?(Installations::Error) && result.error.reason == :pull_request_without_commits
end

def create_new_pr!
if @patch_pr && @patch_commit
repo_integration.create_patch_pr!(to_branch_ref, from_branch_ref, @patch_commit&.commit_hash, title, description)
else
repo_integration.create_pr!(to_branch_ref, from_branch_ref, title, description)
end
end

def enable_auto_merge?
repo_integration.enable_auto_merge?
end

memoize def repo_integration
train.vcs_provider
end
end
40 changes: 0 additions & 40 deletions app/libs/triggers/release_backmerge.rb

This file was deleted.

2 changes: 2 additions & 0 deletions app/models/bitbucket_integration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ def further_setup?
false
end

def enable_auto_merge? = false

def public_icon_img
PUBLIC_ICON
end
Expand Down
1 change: 1 addition & 0 deletions app/models/commit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
# release_platform_run_id :uuid indexed
#
class Commit < ApplicationRecord
has_paper_trail
include Passportable
include Commitable

Expand Down
2 changes: 2 additions & 0 deletions app/models/github_integration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ def further_setup?
false
end

def enable_auto_merge? = true

def connection_data
return unless integration.metadata
"Organization: #{integration.metadata["account_name"]} (#{integration.metadata["account_id"]})"
Expand Down
2 changes: 2 additions & 0 deletions app/models/gitlab_integration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ def further_setup?
false
end

def enable_auto_merge? = false

def find_or_create_webhook!(id:, train_id:)
GitHub::Result.new do
if id
Expand Down
Loading
Loading