Look for new branches more carefully
In certain cases, GitLab can miss a PostReceive invocation the first time a branch is pushed. When this happens, the "branch created" hooks are not run, which means various features don't work until the branch is deleted and pushed again. This MR changes the `Git::BranchPushService` so it checks the cache of existing branches in addition to the `oldrev` reported for the branch. If the branch name isn't in the cache, chances are we haven't run the service yet (it's what refreshes the cache), so we can go ahead and run it, even through `oldrev` is set. If the cache has been cleared by some other means in the meantime, then we'll still fail to run the hooks when we should. Fixing that in the general case is a larger problem, and we'd need to devote significant engineering effort to it. There's a chance that we'll run the relevant hooks *multiple times* with this change, if there's a race between the branch being created, and the `PostReceive` worker being run multiple times, but this can already happen, since Sidekiq is "at-least-once" execution of jobs. So, this should be safe.
This commit is contained in:
parent
c7b72a6ee4
commit
b2c73fde79
3 changed files with 46 additions and 1 deletions
|
@ -105,8 +105,14 @@ module Git
|
|||
CreateGpgSignatureWorker.perform_async(signable, project.id)
|
||||
end
|
||||
|
||||
# It's not sufficient to just check for a blank SHA as it's possible for the
|
||||
# branch to be pushed, but for the `post-receive` hook to never run:
|
||||
# https://gitlab.com/gitlab-org/gitlab-ce/issues/59257
|
||||
def creating_branch?
|
||||
Gitlab::Git.blank_ref?(params[:oldrev])
|
||||
strong_memoize(:creating_branch) do
|
||||
Gitlab::Git.blank_ref?(params[:oldrev]) ||
|
||||
!project.repository.branch_exists?(branch_name)
|
||||
end
|
||||
end
|
||||
|
||||
def updating_branch?
|
||||
|
|
5
changelogs/unreleased/59257-find-new-branches-harder.yml
Normal file
5
changelogs/unreleased/59257-find-new-branches-harder.yml
Normal file
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Look for new branches more carefully
|
||||
merge_request: 29761
|
||||
author:
|
||||
type: fixed
|
|
@ -344,4 +344,38 @@ describe Git::BranchHooksService do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'New branch detection' do
|
||||
let(:branch) { 'fix' }
|
||||
|
||||
context 'oldrev is the blank SHA' do
|
||||
let(:oldrev) { Gitlab::Git::BLANK_SHA }
|
||||
|
||||
it 'is treated as a new branch' do
|
||||
expect(service).to receive(:branch_create_hooks)
|
||||
|
||||
service.execute
|
||||
end
|
||||
end
|
||||
|
||||
context 'oldrev is set' do
|
||||
context 'Gitaly does not know about the branch' do
|
||||
it 'is treated as a new branch' do
|
||||
allow(project.repository).to receive(:branch_names) { [] }
|
||||
|
||||
expect(service).to receive(:branch_create_hooks)
|
||||
|
||||
service.execute
|
||||
end
|
||||
end
|
||||
|
||||
context 'Gitaly knows about the branch' do
|
||||
it 'is not treated as a new branch' do
|
||||
expect(service).not_to receive(:branch_create_hooks)
|
||||
|
||||
service.execute
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue