Commit f62889b3 authored by Robert Speicher's avatar Robert Speicher
Browse files

Merge branch 'da-exclusive-lease-helpers' into 'master'

Add helper methods to stub Gitlab::ExclusiveLease

See merge request gitlab-org/gitlab-ce!20253
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 @@
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
......
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 @@ def external_data_counter
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
......
require 'spec_helper'
describe ReactiveCaching, :use_clean_rails_memory_store_caching do
include ExclusiveLeaseHelpers
include ReactiveCachingHelpers
class CacheTest
......@@ -106,8 +107,8 @@ def result
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 @@ def result
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!
......
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 @@
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 @@
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)
......
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 '#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
......
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
......@@ -247,8 +247,10 @@
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
......
......@@ -76,8 +76,10 @@ def checksum
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 @@ def checksum
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
......
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 '#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 @@
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))
......
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 }
it 'skips when project no longer exists' do
expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
subject.perform(-1)
end
context 'when 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(true)
end
expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
it 'skips when project no longer exists' do
nonexistent_id = 999999999999
subject.perform(pending_delete_project.id)
end
expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
subject.perform(nonexistent_id)
end
it 'delegates removal to service class when have exclusive lease' do
stub_exclusive_lease(lease_key, 'uuid', timeout: lease_timeout)
it 'skips when project is pending delete' do
expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
migration_service = spy
subject.perform(pending_delete_project.id)
end
allow(::Projects::HashedStorageMigrationService)
.to receive(:new).with(project, subject.logger)
.and_return(migration_service)
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)
subject.perform(project.id)
end
expect(migration_service).to have_received(:execute)
end
context 'when dont have exclusive lease' do
before do
lease = subject.lease_for(project.id)
allow(Gitlab::ExclusiveLease).to receive(:new).and_return(lease)
allow(lease).to receive(:try_obtain).and_return(false)
end
it 'skips when dont have lease when dont have exclusive lease' do
stub_exclusive_lease_taken(lease_key, timeout: lease_timeout)
it 'skips when dont have lease' do
expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
expect(::Projects::HashedStorageMigrationService).not_to receive(:new)
subject.perform(project.id)
end
subject.perform(project.id)
end
end
end
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'
})
stub_exclusive_lease("propagate_service_template_worker:#{template.id}",
timeout: PropagateServiceTemplateWorker::LEASE_TIMEOUT)
expect(Projects::PropagateServiceTemplate)
.to receive(:propagate)
.with(template)
subject.perform(service_template.id)
subject.perform(template.id)
end
end
end
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
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)
context 'when project exists' do
it 'removes remote from repository' do
masterrev = project.repository.find_branch('master').dereferenced_target
expect(subject)
.to receive(:log_error)
.with('Cannot obtain an exclusive lease. There must be another instance already in execution.')
create_remote_branch(remote_name, 'remote_branch', masterrev)
subject.perform(project.id, remote_name)
end
expect_any_instance_of(Repository).to receive(:remove_remote).with(remote_name).and_call_original
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)
worker.perform(project.id, remote_name)
end
expect(project.repository)
.to receive(:remove_remote)
.with(remote_name)
.and_call_original
subject.perform(project.id, remote_name)
end
end
end
......@@ -47,6 +53,7 @@ def create_remote_branch(remote_name, branch_name, target)
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
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 @@
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 @@
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 @@
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 @@
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 @@
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