kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Add helper methods to stub Gitlab::ExclusiveLease

Browse source
This commit is contained in:
Douglas Barbosa Alexandre 2018-06-28 15:51:36 -03:00
parent e38af20cc6
commit 34dbccb24b
No known key found for this signature in database
GPG key ID: F1E98EF6393565A0
13 changed files with 188 additions and 140 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
spec/lib/mattermost/session_spec.rb
View file

@ -1,6 +1,8 @@
require 'spec_helper'
describe Mattermost::Session, type: :request do
include ExclusiveLeaseHelpers
let(:user) { create(:user) }
let(:gitlab_url) { "http://gitlab.com" }
@ -97,26 +99,20 @@ describe Mattermost::Session, type: :request do
end
end
context 'with lease' do
before do
allow(subject).to receive(:lease_try_obtain).and_return('aldkfjsldfk')
end
context 'exclusive lease' do
let(:lease_key) { 'mattermost:session' }
it 'tries to obtain a lease' do
expect(subject).to receive(:lease_try_obtain)
expect(Gitlab::ExclusiveLease).to receive(:cancel)
expect_to_obtain_exclusive_lease(lease_key, 'uuid')
expect_to_cancel_exclusive_lease(lease_key, 'uuid')
# Cannot setup a session, but we should still cancel the lease
expect { subject.with_session }.to raise_error(Mattermost::NoSessionError)
end
end
context 'without lease' do
before do
allow(subject).to receive(:lease_try_obtain).and_return(nil)
end
it 'returns a NoSessionError error without lease' do
stub_exclusive_lease_taken(lease_key)
it 'returns a NoSessionError error' do
expect { subject.with_session }.to raise_error(Mattermost::NoSessionError)
end
end

4
spec/models/ci/build_trace_chunk_spec.rb
View file

@ -1,6 +1,8 @@
require 'spec_helper'
describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
include ExclusiveLeaseHelpers
set(:build) { create(:ci_build, :running) }
let(:chunk_index) { 0 }
let(:data_store) { :redis }
@ -322,7 +324,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
describe 'ExclusiveLock' do
before do
allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain) { nil }
stub_exclusive_lease_taken
stub_const('Ci::BuildTraceChunk::WRITE_LOCK_RETRY', 1)
end

11
spec/models/concerns/reactive_caching_spec.rb
View file

@ -1,6 +1,7 @@
require 'spec_helper'
describe ReactiveCaching, :use_clean_rails_memory_store_caching do
include ExclusiveLeaseHelpers
include ReactiveCachingHelpers
class CacheTest
@ -106,8 +107,8 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do
end
it 'takes and releases the lease' do
expect_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return("000000")
expect(Gitlab::ExclusiveLease).to receive(:cancel).with(cache_key, "000000")
expect_to_obtain_exclusive_lease(cache_key, 'uuid')
expect_to_cancel_exclusive_lease(cache_key, 'uuid')
go!
end
@ -153,11 +154,9 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do
end
context 'when the lease is already taken' do
before do
expect_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(nil)
end
it 'skips the calculation' do
stub_exclusive_lease_taken(cache_key)
expect(instance).to receive(:calculate_reactive_cache).never
go!

17
spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb
View file

@ -1,11 +1,13 @@
require 'spec_helper'
describe Clusters::Applications::CheckIngressIpAddressService do
include ExclusiveLeaseHelpers
let(:application) { create(:clusters_applications_ingress, :installed) }
let(:service) { described_class.new(application) }
let(:kubeclient) { double(::Kubeclient::Client, get_service: kube_service) }
let(:ingress) { [{ ip: '111.222.111.222' }] }
let(:exclusive_lease) { instance_double(Gitlab::ExclusiveLease, try_obtain: true) }
let(:lease_key) { "check_ingress_ip_address_service:#{application.id}" }
let(:kube_service) do
::Kubeclient::Resource.new(
@ -22,11 +24,8 @@ describe Clusters::Applications::CheckIngressIpAddressService do
subject { service.execute }
before do
stub_exclusive_lease(lease_key, timeout: 15.seconds.to_i)
allow(application.cluster).to receive(:kubeclient).and_return(kubeclient)
allow(Gitlab::ExclusiveLease)
.to receive(:new)
.with("check_ingress_ip_address_service:#{application.id}", timeout: 15.seconds.to_i)
.and_return(exclusive_lease)
end
describe '#execute' do
@ -47,13 +46,9 @@ describe Clusters::Applications::CheckIngressIpAddressService do
end
context 'when the exclusive lease cannot be obtained' do
before do
allow(exclusive_lease)
.to receive(:try_obtain)
.and_return(false)
end
it 'does not call kubeclient' do
stub_exclusive_lease_taken(lease_key, timeout: 15.seconds.to_i)
subject
expect(kubeclient).not_to have_received(:get_service)

10
spec/services/users/refresh_authorized_projects_service_spec.rb
View file

@ -1,6 +1,8 @@
require 'spec_helper'
describe Users::RefreshAuthorizedProjectsService do
include ExclusiveLeaseHelpers
# We're using let! here so that any expectations for the service class are not
# triggered twice.
let!(:project) { create(:project) }
@ -10,12 +12,10 @@ describe Users::RefreshAuthorizedProjectsService do
describe '#execute', :clean_gitlab_redis_shared_state do
it 'refreshes the authorizations using a lease' do
expect_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain)
.and_return('foo')
expect(Gitlab::ExclusiveLease).to receive(:cancel)
.with(an_instance_of(String), 'foo')
lease_key = "refresh_authorized_projects:#{user.id}"
expect_to_obtain_exclusive_lease(lease_key, 'uuid')
expect_to_cancel_exclusive_lease(lease_key, 'uuid')
expect(service).to receive(:execute_without_lease)
service.execute

36
spec/support/helpers/exclusive_lease_helpers.rb Normal file
View file

@ -0,0 +1,36 @@
module ExclusiveLeaseHelpers
def stub_exclusive_lease(key = nil, uuid = 'uuid', renew: false, timeout: nil)
key ||= instance_of(String)
timeout ||= instance_of(Integer)
lease = instance_double(
Gitlab::ExclusiveLease,
try_obtain: uuid,
exists?: true,
renew: renew
)
allow(Gitlab::ExclusiveLease)
.to receive(:new)
.with(key, timeout: timeout)
.and_return(lease)
lease
end
def stub_exclusive_lease_taken(key = nil, timeout: nil)
stub_exclusive_lease(key, nil, timeout: timeout)
end
def expect_to_obtain_exclusive_lease(key, uuid = 'uuid', timeout: nil)
lease = stub_exclusive_lease(key, uuid, timeout: timeout)
expect(lease).to receive(:try_obtain)
end
def expect_to_cancel_exclusive_lease(key, uuid)
expect(Gitlab::ExclusiveLease)
.to receive(:cancel)
.with(key, uuid)
end
end

4
spec/support/shared_examples/ci_trace_shared_examples.rb
View file

@ -247,8 +247,10 @@ shared_examples_for 'common trace features' do
end
context 'when another process has already been archiving', :clean_gitlab_redis_shared_state do
include ExclusiveLeaseHelpers
before do
Gitlab::ExclusiveLease.new("trace:archive:#{trace.job.id}", timeout: 1.hour).try_obtain
stub_exclusive_lease_taken("trace:archive:#{trace.job.id}", timeout: 1.hour)
end
it 'blocks concurrent archiving' do

8
spec/support/shared_examples/uploaders/object_storage_shared_examples.rb
View file

@ -76,8 +76,10 @@ shared_examples "migrates" do |to_store:, from_store: nil|
end
context 'when migrate! is occupied by another process' do
include ExclusiveLeaseHelpers
before do
@uuid = Gitlab::ExclusiveLease.new(subject.exclusive_lease_key, timeout: 1.hour.to_i).try_obtain
stub_exclusive_lease_taken(subject.exclusive_lease_key, timeout: 1.hour.to_i)
end
it 'does not execute migrate!' do
@ -91,10 +93,6 @@ shared_examples "migrates" do |to_store:, from_store: nil|
expect { subject.use_file }.to raise_error(ObjectStorage::ExclusiveLeaseTaken)
end
after do
Gitlab::ExclusiveLease.cancel(subject.exclusive_lease_key, @uuid)
end
end
context 'migration is unsuccessful' do

15
spec/workers/project_cache_worker_spec.rb
View file

@ -1,14 +1,17 @@
require 'spec_helper'
describe ProjectCacheWorker do
include ExclusiveLeaseHelpers
let(:worker) { described_class.new }
let(:project) { create(:project, :repository) }
let(:statistics) { project.statistics }
let(:lease_key) { "project_cache_worker:#{project.id}:update_statistics" }
let(:lease_timeout) { ProjectCacheWorker::LEASE_TIMEOUT }
describe '#perform' do
before do
allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain)
.and_return(true)
stub_exclusive_lease(lease_key, timeout: lease_timeout)
end
context 'with a non-existing project' do
@ -63,9 +66,7 @@ describe ProjectCacheWorker do
describe '#update_statistics' do
context 'when a lease could not be obtained' do
it 'does not update the repository size' do
allow(worker).to receive(:try_obtain_lease_for)
.with(project.id, :update_statistics)
.and_return(false)
stub_exclusive_lease_taken(lease_key, timeout: lease_timeout)
expect(statistics).not_to receive(:refresh!)
@ -75,9 +76,7 @@ describe ProjectCacheWorker do
context 'when a lease could be obtained' do
it 'updates the project statistics' do
allow(worker).to receive(:try_obtain_lease_for)
.with(project.id, :update_statistics)
.and_return(true)
stub_exclusive_lease(lease_key, timeout: lease_timeout)
expect(statistics).to receive(:refresh!)
.with(only: %i(repository_size))

70
spec/workers/project_migrate_hashed_storage_worker_spec.rb
View file

@ -1,53 +1,47 @@
require 'spec_helper'
describe ProjectMigrateHashedStorageWorker, :clean_gitlab_redis_shared_state do
include ExclusiveLeaseHelpers
describe '#perform' do
let(:project) { create(:project, :empty_repo) }
let(:pending_delete_project) { create(:project, :empty_repo, pending_delete: true) }
let(:lease_key) { "project_migrate_hashed_storage_worker:#{project.id}" }
let(:lease_timeout) { ProjectMigrateHashedStorageWorker::LEASE_TIMEOUT }
context 'when have exclusive lease' do
before do
lease = subject.lease_for(project.id)
it 'skips when project no longer exists' do
expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
allow(Gitlab::ExclusiveLease).to receive(:new).and_return(lease)
allow(lease).to receive(:try_obtain).and_return(true)
end
it 'skips when project no longer exists' do
nonexistent_id = 999999999999
expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
subject.perform(nonexistent_id)
end
it 'skips when project is pending delete' do
expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
subject.perform(pending_delete_project.id)
end
it 'delegates removal to service class' do
service = double('service')
expect(::Projects::HashedStorageMigrationService).to receive(:new).with(project, subject.logger).and_return(service)
expect(service).to receive(:execute)
subject.perform(project.id)
end
subject.perform(-1)
end
context 'when dont have exclusive lease' do
before do
lease = subject.lease_for(project.id)
it 'skips when project is pending delete' do
pending_delete_project = create(:project, :empty_repo, pending_delete: true)
allow(Gitlab::ExclusiveLease).to receive(:new).and_return(lease)
allow(lease).to receive(:try_obtain).and_return(false)
end
expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
it 'skips when dont have lease' do
expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
subject.perform(pending_delete_project.id)
end
subject.perform(project.id)
end
it 'delegates removal to service class when have exclusive lease' do
stub_exclusive_lease(lease_key, 'uuid', timeout: lease_timeout)
migration_service = spy
allow(::Projects::HashedStorageMigrationService)
.to receive(:new).with(project, subject.logger)
.and_return(migration_service)
subject.perform(project.id)
expect(migration_service).to have_received(:execute)
end
it 'skips when dont have lease when dont have exclusive lease' do
stub_exclusive_lease_taken(lease_key, timeout: lease_timeout)
expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
subject.perform(project.id)
end
end
end

38
spec/workers/propagate_service_template_worker_spec.rb
View file

@ -1,29 +1,29 @@
require 'spec_helper'
describe PropagateServiceTemplateWorker do
let!(:service_template) do
PushoverService.create(
template: true,
active: true,
properties: {
device: 'MyDevice',
sound: 'mic',
priority: 4,
user_key: 'asdf',
api_key: '123456789'
})
end
before do
allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain)
.and_return(true)
end
include ExclusiveLeaseHelpers
describe '#perform' do
it 'calls the propagate service with the template' do
expect(Projects::PropagateServiceTemplate).to receive(:propagate).with(service_template)
template = PushoverService.create(
template: true,
active: true,
properties: {
device: 'MyDevice',
sound: 'mic',
priority: 4,
user_key: 'asdf',
api_key: '123456789'
})
subject.perform(service_template.id)
stub_exclusive_lease("propagate_service_template_worker:#{template.id}",
timeout: PropagateServiceTemplateWorker::LEASE_TIMEOUT)
expect(Projects::PropagateServiceTemplate)
.to receive(:propagate)
.with(template)
subject.perform(template.id)
end
end
end

57
spec/workers/repository_remove_remote_worker_spec.rb
View file

@ -1,44 +1,50 @@
require 'rails_helper'
describe RepositoryRemoveRemoteWorker do
subject(:worker) { described_class.new }
include ExclusiveLeaseHelpers
describe '#perform' do
let(:remote_name) { 'joe'}
let!(:project) { create(:project, :repository) }
let(:remote_name) { 'joe'}
let(:lease_key) { "remove_remote_#{project.id}_#{remote_name}" }
let(:lease_timeout) { RepositoryRemoveRemoteWorker::LEASE_TIMEOUT }
context 'when it cannot obtain lease' do
it 'logs error' do
allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain) { nil }
expect_any_instance_of(Repository).not_to receive(:remove_remote)
expect(worker).to receive(:log_error).with('Cannot obtain an exclusive lease. There must be another instance already in execution.')
worker.perform(project.id, remote_name)
end
it 'returns nil when project does not exist' do
expect(subject.perform(-1, 'remote_name')).to be_nil
end
context 'when it gets the lease' do
context 'when project exists' do
before do
allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(true)
allow(Project)
.to receive(:find_by)
.with(id: project.id)
.and_return(project)
end
context 'when project does not exist' do
it 'returns nil' do
expect(worker.perform(-1, 'remote_name')).to be_nil
end
it 'does not remove remote when cannot obtain lease' do
stub_exclusive_lease_taken(lease_key, timeout: lease_timeout)
expect(project.repository)
.not_to receive(:remove_remote)
expect(subject)
.to receive(:log_error)
.with('Cannot obtain an exclusive lease. There must be another instance already in execution.')
subject.perform(project.id, remote_name)
end
context 'when project exists' do
it 'removes remote from repository' do
masterrev = project.repository.find_branch('master').dereferenced_target
it 'removes remote from repository when obtain a lease' do
stub_exclusive_lease(lease_key, timeout: lease_timeout)
masterrev = project.repository.find_branch('master').dereferenced_target
create_remote_branch(remote_name, 'remote_branch', masterrev)
create_remote_branch(remote_name, 'remote_branch', masterrev)
expect(project.repository)
.to receive(:remove_remote)
.with(remote_name)
.and_call_original
expect_any_instance_of(Repository).to receive(:remove_remote).with(remote_name).and_call_original
worker.perform(project.id, remote_name)
end
subject.perform(project.id, remote_name)
end
end
end
@ -47,6 +53,7 @@ describe RepositoryRemoveRemoteWorker do
rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
project.repository.rugged
end
rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id)
end
end

38
spec/workers/stuck_ci_jobs_worker_spec.rb
View file

@ -1,14 +1,21 @@
require 'spec_helper'
describe StuckCiJobsWorker do
include ExclusiveLeaseHelpers
let!(:runner) { create :ci_runner }
let!(:job) { create :ci_build, runner: runner }
let(:worker) { described_class.new }
let(:exclusive_lease_uuid) { SecureRandom.uuid }
let(:trace_lease_key) { "trace:archive:#{job.id}" }
let(:trace_lease_uuid) { SecureRandom.uuid }
let(:worker_lease_key) { StuckCiJobsWorker::EXCLUSIVE_LEASE_KEY }
let(:worker_lease_uuid) { SecureRandom.uuid }
subject(:worker) { described_class.new }
before do
stub_exclusive_lease(worker_lease_key, worker_lease_uuid)
stub_exclusive_lease(trace_lease_key, trace_lease_uuid)
job.update!(status: status, updated_at: updated_at)
allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(exclusive_lease_uuid)
end
shared_examples 'job is dropped' do
@ -44,16 +51,19 @@ describe StuckCiJobsWorker do
context 'when job was not updated for more than 1 day ago' do
let(:updated_at) { 2.days.ago }
it_behaves_like 'job is dropped'
end
context 'when job was updated in less than 1 day ago' do
let(:updated_at) { 6.hours.ago }
it_behaves_like 'job is unchanged'
end
context 'when job was not updated for more than 1 hour ago' do
let(:updated_at) { 2.hours.ago }
it_behaves_like 'job is unchanged'
end
end
@ -65,11 +75,14 @@ describe StuckCiJobsWorker do
context 'when job was not updated for more than 1 hour ago' do
let(:updated_at) { 2.hours.ago }
it_behaves_like 'job is dropped'
end
context 'when job was updated in less than 1 hour ago' do
context 'when job was updated in less than 1
hour ago' do
let(:updated_at) { 30.minutes.ago }
it_behaves_like 'job is unchanged'
end
end
@ -80,11 +93,13 @@ describe StuckCiJobsWorker do
context 'when job was not updated for more than 1 hour ago' do
let(:updated_at) { 2.hours.ago }
it_behaves_like 'job is dropped'
end
context 'when job was updated in less than 1 hour ago' do
let(:updated_at) { 30.minutes.ago }
it_behaves_like 'job is unchanged'
end
end
@ -93,6 +108,7 @@ describe StuckCiJobsWorker do
context "when job is #{status}" do
let(:status) { status }
let(:updated_at) { 2.days.ago }
it_behaves_like 'job is unchanged'
end
end
@ -119,23 +135,27 @@ describe StuckCiJobsWorker do
it 'is guard by exclusive lease when executed concurrently' do
expect(worker).to receive(:drop).at_least(:once).and_call_original
expect(worker2).not_to receive(:drop)
worker.perform
allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(false)
stub_exclusive_lease_taken(worker_lease_key)
worker2.perform
end
it 'can be executed in sequence' do
expect(worker).to receive(:drop).at_least(:once).and_call_original
expect(worker2).to receive(:drop).at_least(:once).and_call_original
worker.perform
worker2.perform
end
it 'cancels exclusive lease after worker perform' do
worker.perform
it 'cancels exclusive leases after worker perform' do
expect_to_cancel_exclusive_lease(trace_lease_key, trace_lease_uuid)
expect_to_cancel_exclusive_lease(worker_lease_key, worker_lease_uuid)
expect(Gitlab::ExclusiveLease.new(described_class::EXCLUSIVE_LEASE_KEY, timeout: 1.hour))
.not_to be_exists
worker.perform
end
end
end