diff options
Diffstat (limited to 'spec/workers')
21 files changed, 469 insertions, 248 deletions
diff --git a/spec/workers/ci/archive_traces_cron_worker_spec.rb b/spec/workers/ci/archive_traces_cron_worker_spec.rb new file mode 100644 index 00000000000..9af51b7d4d8 --- /dev/null +++ b/spec/workers/ci/archive_traces_cron_worker_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe Ci::ArchiveTracesCronWorker do + subject { described_class.new.perform } + + before do + stub_feature_flags(ci_enable_live_trace: true) + end + + shared_examples_for 'archives trace' do + it do + subject + + build.reload + expect(build.job_artifacts_trace).to be_exist + end + end + + shared_examples_for 'does not archive trace' do + it do + subject + + build.reload + expect(build.job_artifacts_trace).to be_nil + end + end + + context 'when a job was succeeded' do + let!(:build) { create(:ci_build, :success, :trace_live) } + + it_behaves_like 'archives trace' + + context 'when archive raised an exception' do + let!(:build) { create(:ci_build, :success, :trace_artifact, :trace_live) } + let!(:build2) { create(:ci_build, :success, :trace_live) } + + it 'archives valid targets' do + expect(Rails.logger).to receive(:error).with("Failed to archive stale live trace. id: #{build.id} message: Already archived") + + subject + + build2.reload + expect(build2.job_artifacts_trace).to be_exist + end + end + end + + context 'when a job was cancelled' do + let!(:build) { create(:ci_build, :canceled, :trace_live) } + + it_behaves_like 'archives trace' + end + + context 'when a job is running' do + let!(:build) { create(:ci_build, :running, :trace_live) } + + it_behaves_like 'does not archive trace' + end +end diff --git a/spec/workers/delete_diff_files_worker_spec.rb b/spec/workers/delete_diff_files_worker_spec.rb new file mode 100644 index 00000000000..e0edd313922 --- /dev/null +++ b/spec/workers/delete_diff_files_worker_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +describe DeleteDiffFilesWorker do + describe '#perform' do + let(:merge_request) { create(:merge_request) } + let(:merge_request_diff) { merge_request.merge_request_diff } + + it 'deletes all merge request diff files' do + expect { described_class.new.perform(merge_request_diff.id) } + .to change { merge_request_diff.merge_request_diff_files.count } + .from(20).to(0) + end + + it 'updates state to without_files' do + expect { described_class.new.perform(merge_request_diff.id) } + .to change { merge_request_diff.reload.state } + .from('collected').to('without_files') + end + + it 'does nothing if diff was already marked as "without_files"' do + merge_request_diff.clean! + + expect_any_instance_of(MergeRequestDiff).not_to receive(:clean!) + + described_class.new.perform(merge_request_diff.id) + end + + it 'rollsback if something goes wrong' do + expect(MergeRequestDiffFile).to receive_message_chain(:where, :delete_all) + .and_raise + + expect { described_class.new.perform(merge_request_diff.id) } + .to raise_error + + merge_request_diff.reload + + expect(merge_request_diff.state).to eq('collected') + expect(merge_request_diff.merge_request_diff_files.count).to eq(20) + end + end +end diff --git a/spec/workers/delete_user_worker_spec.rb b/spec/workers/delete_user_worker_spec.rb index 36594515005..06d9e125105 100644 --- a/spec/workers/delete_user_worker_spec.rb +++ b/spec/workers/delete_user_worker_spec.rb @@ -5,15 +5,17 @@ describe DeleteUserWorker do let!(:current_user) { create(:user) } it "calls the DeleteUserWorker with the params it was given" do - expect_any_instance_of(Users::DestroyService).to receive(:execute) - .with(user, {}) + expect_next_instance_of(Users::DestroyService) do |service| + expect(service).to receive(:execute).with(user, {}) + end described_class.new.perform(current_user.id, user.id) end it "uses symbolized keys" do - expect_any_instance_of(Users::DestroyService).to receive(:execute) - .with(user, test: "test") + expect_next_instance_of(Users::DestroyService) do |service| + expect(service).to receive(:execute).with(user, test: "test") + end described_class.new.perform(current_user.id, user.id, "test" => "test") end diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index 9e3b99b3502..2106959e23c 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -13,7 +13,7 @@ describe 'Every Sidekiq worker' do file_worker_queues = Gitlab::SidekiqConfig.worker_queues.to_set worker_queues = Gitlab::SidekiqConfig.workers.map(&:queue).to_set - worker_queues << ActionMailer::DeliveryJob.queue_name + worker_queues << ActionMailer::DeliveryJob.new.queue_name worker_queues << 'default' missing_from_file = worker_queues - file_worker_queues diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb index 74539a7e493..e39dec556fc 100644 --- a/spec/workers/git_garbage_collect_worker_spec.rb +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -11,36 +11,63 @@ describe GitGarbageCollectWorker do subject { described_class.new } describe "#perform" do - shared_examples 'flushing ref caches' do |gitaly| - context 'with active lease_uuid' do + context 'with active lease_uuid' do + before do + allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid) + end + + it "flushes ref caches when the task if 'gc'" do + expect(subject).to receive(:renew_lease).with(lease_key, lease_uuid).and_call_original + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect) + .and_return(nil) + expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original + expect_any_instance_of(Repository).to receive(:branch_names).and_call_original + expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original + expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original + + subject.perform(project.id, :gc, lease_key, lease_uuid) + end + end + + context 'with different lease than the active one' do + before do + allow(subject).to receive(:get_lease_uuid).and_return(SecureRandom.uuid) + end + + it 'returns silently' do + expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original + expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original + expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original + + subject.perform(project.id, :gc, lease_key, lease_uuid) + end + end + + context 'with no active lease' do + before do + allow(subject).to receive(:get_lease_uuid).and_return(false) + end + + context 'when is able to get the lease' do before do - allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid) + allow(subject).to receive(:try_obtain_lease).and_return(SecureRandom.uuid) end it "flushes ref caches when the task if 'gc'" do - expect(subject).to receive(:renew_lease).with(lease_key, lease_uuid).and_call_original - expect(subject).to receive(:command).with(:gc).and_return([:the, :command]) - - if gitaly - expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect) - .and_return(nil) - else - expect(Gitlab::Popen).to receive(:popen) - .with([:the, :command], project.repository.path_to_repo).and_return(["", 0]) - end - + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect) + .and_return(nil) expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original expect_any_instance_of(Repository).to receive(:branch_names).and_call_original expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original - subject.perform(project.id, :gc, lease_key, lease_uuid) + subject.perform(project.id) end end - context 'with different lease than the active one' do + context 'when no lease can be obtained' do before do - allow(subject).to receive(:get_lease_uuid).and_return(SecureRandom.uuid) + expect(subject).to receive(:try_obtain_lease).and_return(false) end it 'returns silently' do @@ -49,65 +76,11 @@ describe GitGarbageCollectWorker do expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original - subject.perform(project.id, :gc, lease_key, lease_uuid) - end - end - - context 'with no active lease' do - before do - allow(subject).to receive(:get_lease_uuid).and_return(false) - end - - context 'when is able to get the lease' do - before do - allow(subject).to receive(:try_obtain_lease).and_return(SecureRandom.uuid) - end - - it "flushes ref caches when the task if 'gc'" do - expect(subject).to receive(:command).with(:gc).and_return([:the, :command]) - - if gitaly - expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect) - .and_return(nil) - else - expect(Gitlab::Popen).to receive(:popen) - .with([:the, :command], project.repository.path_to_repo).and_return(["", 0]) - end - - expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original - expect_any_instance_of(Repository).to receive(:branch_names).and_call_original - expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original - expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original - - subject.perform(project.id) - end - end - - context 'when no lease can be obtained' do - before do - expect(subject).to receive(:try_obtain_lease).and_return(false) - end - - it 'returns silently' do - expect(subject).not_to receive(:command) - expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original - expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original - expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original - - subject.perform(project.id) - end + subject.perform(project.id) end end end - context "with Gitaly turned on" do - it_should_behave_like 'flushing ref caches', true - end - - context "with Gitaly turned off", :skip_gitaly_mock do - it_should_behave_like 'flushing ref caches', false - end - context "repack_full" do before do expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid) @@ -218,7 +191,9 @@ describe GitGarbageCollectWorker do # Create a new commit on a random new branch def create_objects(project) - rugged = project.repository.rugged + rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.rugged + end old_commit = rugged.branches.first.target new_commit_sha = Rugged::Commit.create( rugged, @@ -237,7 +212,9 @@ describe GitGarbageCollectWorker do end def packs(project) - Dir["#{project.repository.path_to_repo}/objects/pack/*.pack"] + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Dir["#{project.repository.path_to_repo}/objects/pack/*.pack"] + end end def packed_refs(project) diff --git a/spec/workers/gitlab/github_import/stage/import_lfs_objects_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_lfs_objects_worker_spec.rb new file mode 100644 index 00000000000..b19884d7991 --- /dev/null +++ b/spec/workers/gitlab/github_import/stage/import_lfs_objects_worker_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Stage::ImportLfsObjectsWorker do + let(:project) { create(:project) } + let(:worker) { described_class.new } + + describe '#import' do + it 'imports all the lfs objects' do + importer = double(:importer) + waiter = Gitlab::JobWaiter.new(2, '123') + + expect(Gitlab::GithubImport::Importer::LfsObjectsImporter) + .to receive(:new) + .with(project, nil) + .and_return(importer) + + expect(importer) + .to receive(:execute) + .and_return(waiter) + + expect(Gitlab::GithubImport::AdvanceStageWorker) + .to receive(:perform_async) + .with(project.id, { '123' => 2 }, :finish) + + worker.import(project) + end + end +end diff --git a/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb index 098d2d55386..94cff9e4e80 100644 --- a/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb @@ -21,7 +21,7 @@ describe Gitlab::GithubImport::Stage::ImportNotesWorker do expect(Gitlab::GithubImport::AdvanceStageWorker) .to receive(:perform_async) - .with(project.id, { '123' => 2 }, :finish) + .with(project.id, { '123' => 2 }, :lfs_objects) worker.import(client, project) end diff --git a/spec/workers/project_cache_worker_spec.rb b/spec/workers/project_cache_worker_spec.rb index 6b1f2ff3227..8c4daac5f80 100644 --- a/spec/workers/project_cache_worker_spec.rb +++ b/spec/workers/project_cache_worker_spec.rb @@ -1,49 +1,58 @@ 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) - end + before do + stub_exclusive_lease(lease_key, timeout: lease_timeout) + allow(Project).to receive(:find_by) + .with(id: project.id) + .and_return(project) + end + + describe '#perform' do context 'with a non-existing project' do - it 'does nothing' do - expect(worker).not_to receive(:update_statistics) + it 'does not update statistic' do + allow(Project).to receive(:find_by).with(id: -1).and_return(nil) - worker.perform(-1) + expect(subject).not_to receive(:update_statistics) + + subject.perform(-1) end end context 'with an existing project without a repository' do - it 'does nothing' do - allow_any_instance_of(Repository).to receive(:exists?).and_return(false) + it 'does not update statistics' do + allow(project.repository).to receive(:exists?).and_return(false) - expect(worker).not_to receive(:update_statistics) + expect(subject).not_to receive(:update_statistics) - worker.perform(project.id) + subject.perform(project.id) end end context 'with an existing project' do it 'updates the project statistics' do - expect(worker).to receive(:update_statistics) - .with(kind_of(Project), %i(repository_size)) - .and_call_original + expect(subject).to receive(:update_statistics) + .with(%w(repository_size)) + .and_call_original - worker.perform(project.id, [], %w(repository_size)) + subject.perform(project.id, [], %w(repository_size)) end it 'refreshes the method caches' do - expect_any_instance_of(Repository).to receive(:refresh_method_caches) - .with(%i(readme)) - .and_call_original + expect(project.repository).to receive(:refresh_method_caches) + .with(%i(readme)) + .and_call_original - worker.perform(project.id, %w(readme)) + subject.perform(project.id, %w(readme)) end context 'with plain readme' do @@ -51,39 +60,40 @@ describe ProjectCacheWorker do allow(MarkupHelper).to receive(:gitlab_markdown?).and_return(false) allow(MarkupHelper).to receive(:plain?).and_return(true) - expect_any_instance_of(Repository).to receive(:refresh_method_caches) - .with(%i(readme)) - .and_call_original - worker.perform(project.id, %w(readme)) + expect(project.repository).to receive(:refresh_method_caches) + .with(%i(readme)) + .and_call_original + + subject.perform(project.id, %w(readme)) end end end - end - 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!) + expect(project.statistics).not_to receive(:refresh!) - worker.update_statistics(project) + subject.perform(project.id, [], %w(repository_size)) end end 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(project.statistics).to receive(:refresh!) + .with(only: %i(repository_size)) + .and_call_original + + subject.perform(project.id, [], %i(repository_size)) + end - expect(statistics).to receive(:refresh!) - .with(only: %i(repository_size)) - .and_call_original + it 'cancels the lease after statistics has been updated' do + expect(subject).to receive(:release_lease).with('uuid') - worker.update_statistics(project, %i(repository_size)) + subject.perform(project.id, [], %i(repository_size)) end end end diff --git a/spec/workers/project_destroy_worker_spec.rb b/spec/workers/project_destroy_worker_spec.rb index f19c9dff941..42e1d86e3bb 100644 --- a/spec/workers/project_destroy_worker_spec.rb +++ b/spec/workers/project_destroy_worker_spec.rb @@ -2,7 +2,11 @@ require 'spec_helper' describe ProjectDestroyWorker do let(:project) { create(:project, :repository, pending_delete: true) } - let(:path) { project.repository.path_to_repo } + let(:path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.path_to_repo + end + end subject { described_class.new } diff --git a/spec/workers/project_migrate_hashed_storage_worker_spec.rb b/spec/workers/project_migrate_hashed_storage_worker_spec.rb index 2e3951e7afc..9551e358af1 100644 --- a/spec/workers/project_migrate_hashed_storage_worker_spec.rb +++ b/spec/workers/project_migrate_hashed_storage_worker_spec.rb @@ -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 } + + 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 diff --git a/spec/workers/propagate_service_template_worker_spec.rb b/spec/workers/propagate_service_template_worker_spec.rb index b8b65ead9b3..af1fb80a51d 100644 --- a/spec/workers/propagate_service_template_worker_spec.rb +++ b/spec/workers/propagate_service_template_worker_spec.rb @@ -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' + }) + + 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 diff --git a/spec/workers/prune_web_hook_logs_worker_spec.rb b/spec/workers/prune_web_hook_logs_worker_spec.rb new file mode 100644 index 00000000000..d7d64a1f641 --- /dev/null +++ b/spec/workers/prune_web_hook_logs_worker_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe PruneWebHookLogsWorker do + describe '#perform' do + before do + hook = create(:project_hook) + + 5.times do + create(:web_hook_log, web_hook: hook, created_at: 5.months.ago) + end + + create(:web_hook_log, web_hook: hook, response_status: '404') + end + + it 'removes all web hook logs older than one month' do + described_class.new.perform + + expect(WebHookLog.count).to eq(1) + expect(WebHookLog.first.response_status).to eq('404') + end + end +end diff --git a/spec/workers/repository_check/batch_worker_spec.rb b/spec/workers/repository_check/batch_worker_spec.rb index 6cd27d2fafb..6bc551be9ad 100644 --- a/spec/workers/repository_check/batch_worker_spec.rb +++ b/spec/workers/repository_check/batch_worker_spec.rb @@ -1,14 +1,19 @@ require 'spec_helper' describe RepositoryCheck::BatchWorker do + let(:shard_name) { 'default' } subject { described_class.new } + before do + Gitlab::ShardHealthCache.update([shard_name]) + end + it 'prefers projects that have never been checked' do projects = create_list(:project, 3, created_at: 1.week.ago) projects[0].update_column(:last_repository_check_at, 4.months.ago) projects[2].update_column(:last_repository_check_at, 3.months.ago) - expect(subject.perform).to eq(projects.values_at(1, 0, 2).map(&:id)) + expect(subject.perform(shard_name)).to eq(projects.values_at(1, 0, 2).map(&:id)) end it 'sorts projects by last_repository_check_at' do @@ -17,7 +22,7 @@ describe RepositoryCheck::BatchWorker do projects[1].update_column(:last_repository_check_at, 4.months.ago) projects[2].update_column(:last_repository_check_at, 3.months.ago) - expect(subject.perform).to eq(projects.values_at(1, 2, 0).map(&:id)) + expect(subject.perform(shard_name)).to eq(projects.values_at(1, 2, 0).map(&:id)) end it 'excludes projects that were checked recently' do @@ -26,7 +31,14 @@ describe RepositoryCheck::BatchWorker do projects[1].update_column(:last_repository_check_at, 2.months.ago) projects[2].update_column(:last_repository_check_at, 3.days.ago) - expect(subject.perform).to eq([projects[1].id]) + expect(subject.perform(shard_name)).to eq([projects[1].id]) + end + + it 'excludes projects on another shard' do + projects = create_list(:project, 2, created_at: 1.week.ago) + projects[0].update_column(:repository_storage, 'other') + + expect(subject.perform(shard_name)).to eq([projects[1].id]) end it 'does nothing when repository checks are disabled' do @@ -34,13 +46,20 @@ describe RepositoryCheck::BatchWorker do stub_application_setting(repository_checks_enabled: false) - expect(subject.perform).to eq(nil) + expect(subject.perform(shard_name)).to eq(nil) + end + + it 'does nothing when shard is unhealthy' do + shard_name = 'broken' + create(:project, created_at: 1.week.ago, repository_storage: shard_name) + + expect(subject.perform(shard_name)).to eq(nil) end it 'skips projects created less than 24 hours ago' do project = create(:project) project.update_column(:created_at, 23.hours.ago) - expect(subject.perform).to eq([]) + expect(subject.perform(shard_name)).to eq([]) end end diff --git a/spec/workers/repository_check/dispatch_worker_spec.rb b/spec/workers/repository_check/dispatch_worker_spec.rb new file mode 100644 index 00000000000..20a4f1f5344 --- /dev/null +++ b/spec/workers/repository_check/dispatch_worker_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe RepositoryCheck::DispatchWorker do + subject { described_class.new } + + it 'does nothing when repository checks are disabled' do + stub_application_setting(repository_checks_enabled: false) + + expect(RepositoryCheck::BatchWorker).not_to receive(:perform_async) + + subject.perform + end + + it 'dispatches work to RepositoryCheck::BatchWorker' do + expect(RepositoryCheck::BatchWorker).to receive(:perform_async).at_least(:once) + + subject.perform + end + + context 'with unhealthy shard' do + let(:default_shard_name) { 'default' } + let(:unhealthy_shard_name) { 'unhealthy' } + let(:default_shard) { Gitlab::HealthChecks::Result.new(true, nil, shard: default_shard_name) } + let(:unhealthy_shard) { Gitlab::HealthChecks::Result.new(false, '14:Connect Failed', shard: unhealthy_shard_name) } + + before do + allow(Gitlab::HealthChecks::GitalyCheck).to receive(:readiness).and_return([default_shard, unhealthy_shard]) + end + + it 'only triggers RepositoryCheck::BatchWorker for healthy shards' do + expect(RepositoryCheck::BatchWorker).to receive(:perform_async).with('default') + + subject.perform + end + end +end diff --git a/spec/workers/repository_check/single_repository_worker_spec.rb b/spec/workers/repository_check/single_repository_worker_spec.rb index a021235aed6..22fc64c1536 100644 --- a/spec/workers/repository_check/single_repository_worker_spec.rb +++ b/spec/workers/repository_check/single_repository_worker_spec.rb @@ -88,7 +88,9 @@ describe RepositoryCheck::SingleRepositoryWorker do end def break_wiki(project) - break_repo(wiki_path(project)) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + break_repo(wiki_path(project)) + end end def wiki_path(project) @@ -96,7 +98,9 @@ describe RepositoryCheck::SingleRepositoryWorker do end def break_project(project) - break_repo(project.repository.path_to_repo) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + break_repo(project.repository.path_to_repo) + end end def break_repo(repo) diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb index 4b3c1736ea0..ac8716ecfb1 100644 --- a/spec/workers/repository_fork_worker_spec.rb +++ b/spec/workers/repository_fork_worker_spec.rb @@ -55,10 +55,15 @@ describe RepositoryForkWorker do it 'flushes various caches' do expect_fork_repository.and_return(true) - expect_any_instance_of(Repository).to receive(:expire_emptiness_caches) + # Works around https://github.com/rspec/rspec-mocks/issues/910 + expect(Project).to receive(:find).with(fork_project.id).and_return(fork_project) + expect(fork_project.repository).to receive(:expire_emptiness_caches) .and_call_original - - expect_any_instance_of(Repository).to receive(:expire_exists_cache) + expect(fork_project.repository).to receive(:expire_exists_cache) + .and_call_original + expect(fork_project.wiki.repository).to receive(:expire_emptiness_caches) + .and_call_original + expect(fork_project.wiki.repository).to receive(:expire_exists_cache) .and_call_original perform! @@ -87,13 +92,6 @@ describe RepositoryForkWorker do end it_behaves_like 'RepositoryForkWorker performing' - - it 'logs a message about forking with old-style arguments' do - allow(Rails.logger).to receive(:info).with(anything) # To compensate for other logs - expect(Rails.logger).to receive(:info).with("Project #{fork_project.id} is being forked using old-style arguments.") - - perform! - end end end end diff --git a/spec/workers/repository_import_worker_spec.rb b/spec/workers/repository_import_worker_spec.rb index 84d1b38ef19..f0884ad0aff 100644 --- a/spec/workers/repository_import_worker_spec.rb +++ b/spec/workers/repository_import_worker_spec.rb @@ -22,8 +22,11 @@ describe RepositoryImportWorker do expect_any_instance_of(Projects::ImportService).to receive(:execute) .and_return({ status: :ok }) - expect_any_instance_of(Repository).to receive(:expire_emptiness_caches) - expect_any_instance_of(Project).to receive(:import_finish) + # Works around https://github.com/rspec/rspec-mocks/issues/910 + expect(Project).to receive(:find).with(project.id).and_return(project) + expect(project.repository).to receive(:expire_emptiness_caches) + expect(project.wiki.repository).to receive(:expire_emptiness_caches) + expect(project).to receive(:import_finish) subject.perform(project.id) end @@ -34,9 +37,11 @@ describe RepositoryImportWorker do expect_any_instance_of(Projects::ImportService).to receive(:execute) .and_return({ status: :ok }) - expect_any_instance_of(Project).to receive(:after_import).and_call_original - expect_any_instance_of(Repository).to receive(:expire_emptiness_caches) - expect_any_instance_of(Project).to receive(:import_finish) + # Works around https://github.com/rspec/rspec-mocks/issues/910 + expect(Project).to receive(:find).with(project.id).and_return(project) + expect(project.repository).to receive(:expire_emptiness_caches) + expect(project.wiki.repository).to receive(:expire_emptiness_caches) + expect(project).to receive(:import_finish) subject.perform(project.id) end diff --git a/spec/workers/repository_remove_remote_worker_spec.rb b/spec/workers/repository_remove_remote_worker_spec.rb index f22d7c1d073..a653f6f926c 100644 --- a/spec/workers/repository_remove_remote_worker_spec.rb +++ b/spec/workers/repository_remove_remote_worker_spec.rb @@ -1,50 +1,59 @@ 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) - context 'when project exists' do - it 'removes remote from repository' do - masterrev = project.repository.find_branch('master').dereferenced_target + expect(project.repository) + .not_to receive(:remove_remote) - create_remote_branch(remote_name, 'remote_branch', masterrev) + expect(subject) + .to receive(:log_error) + .with('Cannot obtain an exclusive lease. There must be another instance already in execution.') - expect_any_instance_of(Repository).to receive(:remove_remote).with(remote_name).and_call_original + subject.perform(project.id, remote_name) + end + + 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) + + expect(project.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 def create_remote_branch(remote_name, branch_name, target) - rugged = project.repository.rugged + 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 diff --git a/spec/workers/storage_migrator_worker_spec.rb b/spec/workers/storage_migrator_worker_spec.rb index ff625164142..815432aacce 100644 --- a/spec/workers/storage_migrator_worker_spec.rb +++ b/spec/workers/storage_migrator_worker_spec.rb @@ -2,29 +2,24 @@ require 'spec_helper' describe StorageMigratorWorker do subject(:worker) { described_class.new } - let(:projects) { create_list(:project, 2, :legacy_storage) } + let(:projects) { create_list(:project, 2, :legacy_storage, :empty_repo) } + let(:ids) { projects.map(&:id) } describe '#perform' do - let(:ids) { projects.map(&:id) } + it 'delegates to MigratorService' do + expect_any_instance_of(Gitlab::HashedStorage::Migrator).to receive(:bulk_migrate).with(5, 10) - it 'enqueue jobs to ProjectMigrateHashedStorageWorker' do - expect(ProjectMigrateHashedStorageWorker).to receive(:perform_async).twice - - worker.perform(ids.min, ids.max) + worker.perform(5, 10) end - it 'sets projects as read only' do - allow(ProjectMigrateHashedStorageWorker).to receive(:perform_async).twice - worker.perform(ids.min, ids.max) + it 'migrates projects in the specified range' do + Sidekiq::Testing.inline! do + worker.perform(ids.min, ids.max) + end projects.each do |project| - expect(project.reload.repository_read_only?).to be_truthy + expect(project.reload.hashed_storage?(:attachments)).to be_truthy end end - - it 'rescues and log exceptions' do - allow_any_instance_of(Project).to receive(:migrate_to_hashed_storage!).and_raise(StandardError) - expect { worker.perform(ids.min, ids.max) }.not_to raise_error - end end end diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb index bdc64c6785b..856886e3df5 100644 --- a/spec/workers/stuck_ci_jobs_worker_spec.rb +++ b/spec/workers/stuck_ci_jobs_worker_spec.rb @@ -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,20 +135,26 @@ 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 - expect(Gitlab::ExclusiveLease).to receive(:cancel).with(described_class::EXCLUSIVE_LEASE_KEY, exclusive_lease_uuid) + 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) + worker.perform end end diff --git a/spec/workers/update_merge_requests_worker_spec.rb b/spec/workers/update_merge_requests_worker_spec.rb index 80137815d2b..0b553db0ca4 100644 --- a/spec/workers/update_merge_requests_worker_spec.rb +++ b/spec/workers/update_merge_requests_worker_spec.rb @@ -18,13 +18,9 @@ describe UpdateMergeRequestsWorker do end it 'executes MergeRequests::RefreshService with expected values' do - expect(MergeRequests::RefreshService).to receive(:new) - .with(project, user).and_wrap_original do |method, *args| - method.call(*args).tap do |refresh_service| - expect(refresh_service) - .to receive(:execute).with(oldrev, newrev, ref) - end - end + expect_next_instance_of(MergeRequests::RefreshService, project, user) do |refresh_service| + expect(refresh_service).to receive(:execute).with(oldrev, newrev, ref) + end perform end |