Commit 847a814c authored by Steve Abrams's avatar Steve Abrams Committed by Heinrich Lee Yu

Fix registry race condition

parent ce272f6b
...@@ -145,9 +145,14 @@ class ContainerRepository < ApplicationRecord ...@@ -145,9 +145,14 @@ class ContainerRepository < ApplicationRecord
name: path.repository_name) name: path.repository_name)
end end
def self.create_from_path!(path) def self.find_or_create_from_path(path)
safe_find_or_create_by!(project: path.repository_project, repository = safe_find_or_create_by(
name: path.repository_name) project: path.repository_project,
name: path.repository_name
)
return repository if repository.persisted?
find_by_path!(path)
end end
def self.build_root_repository(project) def self.build_root_repository(project)
......
...@@ -156,7 +156,7 @@ module Auth ...@@ -156,7 +156,7 @@ module Auth
return if path.has_repository? return if path.has_repository?
return unless actions.include?('push') return unless actions.include?('push')
ContainerRepository.create_from_path!(path) ContainerRepository.find_or_create_from_path(path)
end end
# Overridden in EE # Overridden in EE
......
...@@ -223,9 +223,9 @@ RSpec.describe ContainerRepository do ...@@ -223,9 +223,9 @@ RSpec.describe ContainerRepository do
end end
end end
describe '.create_from_path!' do describe '.find_or_create_from_path' do
let(:repository) do let(:repository) do
described_class.create_from_path!(ContainerRegistry::Path.new(path)) described_class.find_or_create_from_path(ContainerRegistry::Path.new(path))
end end
let(:repository_path) { ContainerRegistry::Path.new(path) } let(:repository_path) { ContainerRegistry::Path.new(path) }
...@@ -291,6 +291,35 @@ RSpec.describe ContainerRepository do ...@@ -291,6 +291,35 @@ RSpec.describe ContainerRepository do
expect(repository.id).to eq(container_repository.id) expect(repository.id).to eq(container_repository.id)
end end
end end
context 'when many of the same repository are created at the same time' do
let(:path) { ContainerRegistry::Path.new(project.full_path + '/some/image') }
it 'does not throw validation errors and only creates one repository' do
expect { repository_creation_race(path) }.to change { ContainerRepository.count }.by(1)
end
it 'retrieves a persisted repository for all concurrent calls' do
repositories = repository_creation_race(path).map(&:value)
expect(repositories).to all(be_persisted)
end
end
def repository_creation_race(path)
# create a race condition - structure from https://blog.arkency.com/2015/09/testing-race-conditions/
wait_for_it = true
threads = Array.new(10) do |i|
Thread.new do
true while wait_for_it
::ContainerRepository.find_or_create_from_path(path)
end
end
wait_for_it = false
threads.each(&:join)
end
end end
describe '.build_root_repository' do describe '.build_root_repository' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment