diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-05-12 12:10:24 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-05-12 12:10:24 +0000 |
commit | 71a67d17b02e7b8dec2f4c257f6734dc7818fb1e (patch) | |
tree | 6b45633257869a67fd534bd2cf899b93a48794c0 /spec/workers | |
parent | 133ec9237af290062aae70e6f115db69b51c88de (diff) | |
download | gitlab-ce-71a67d17b02e7b8dec2f4c257f6734dc7818fb1e.tar.gz |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec/workers')
4 files changed, 346 insertions, 32 deletions
diff --git a/spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb b/spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb index 45f6d05cd4e..04f568515ed 100644 --- a/spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb +++ b/spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb @@ -5,11 +5,11 @@ require 'spec_helper' RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do using RSpec::Parameterized::TableSyntax - let_it_be(:repository, refind: true) { create(:container_repository, :cleanup_scheduled) } - let_it_be(:project) { repository.project } - let_it_be(:policy) { project.container_expiration_policy } - let_it_be(:other_repository) { create(:container_repository) } + let_it_be(:repository, refind: true) { create(:container_repository, :cleanup_scheduled, expiration_policy_started_at: 1.month.ago) } + let_it_be(:other_repository, refind: true) { create(:container_repository, expiration_policy_started_at: 15.days.ago) } + let(:project) { repository.project } + let(:policy) { project.container_expiration_policy } let(:worker) { described_class.new } describe '#perform_work' do @@ -19,7 +19,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do policy.update_column(:enabled, true) end - RSpec.shared_examples 'handling all repository conditions' do + shared_examples 'handling all repository conditions' do it 'sends the repository for cleaning' do service_response = cleanup_service_response(repository: repository) expect(ContainerExpirationPolicies::CleanupService) @@ -72,11 +72,21 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do end end + context 'with an erroneous cleanup' do + it 'logs an error' do + service_response = ServiceResponse.error(message: 'cleanup in an error') + expect(ContainerExpirationPolicies::CleanupService) + .to receive(:new).with(repository).and_return(double(execute: service_response)) + expect_log_extra_metadata(service_response: service_response, cleanup_status: :error) + + subject + end + end + context 'with policy running shortly' do before do - repository.project - .container_expiration_policy - .update_column(:next_run_at, 1.minute.from_now) + repository.cleanup_unfinished! if loopless_enabled? + policy.update_column(:next_run_at, 1.minute.from_now) end it 'skips the repository' do @@ -84,24 +94,285 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do expect(worker).to receive(:log_extra_metadata_on_done).with(:container_repository_id, repository.id) expect(worker).to receive(:log_extra_metadata_on_done).with(:project_id, repository.project.id) expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_status, :skipped) - expect { subject }.to change { ContainerRepository.waiting_for_cleanup.count }.from(1).to(0) + expect(repository.reload.cleanup_unscheduled?).to be_truthy end end context 'with disabled policy' do before do - repository.project - .container_expiration_policy - .disable! + policy.disable! end it 'skips the repository' do expect(ContainerExpirationPolicies::CleanupService).not_to receive(:new) - expect { subject }.to change { ContainerRepository.waiting_for_cleanup.count }.from(1).to(0) - expect(repository.reload.cleanup_unscheduled?).to be_truthy + if loopless_enabled? + expect { subject } + .to not_change { ContainerRepository.waiting_for_cleanup.count } + .and not_change { repository.reload.expiration_policy_cleanup_status } + else + expect { subject }.to change { ContainerRepository.waiting_for_cleanup.count }.from(1).to(0) + expect(repository.reload.cleanup_unscheduled?).to be_truthy + end + end + end + end + + context 'with loopless enabled' do + before do + stub_feature_flags(container_registry_expiration_policies_loopless: true) + end + + context 'with repository in cleanup unscheduled state' do + before do + policy.update_column(:next_run_at, 5.minutes.ago) + end + + it_behaves_like 'handling all repository conditions' + end + + context 'with repository in cleanup unfinished state' do + before do + repository.cleanup_unfinished! + end + + it_behaves_like 'handling all repository conditions' + end + + context 'container repository selection' do + where(:repository_cleanup_status, :repository_policy_status, :other_repository_cleanup_status, :other_repository_policy_status, :expected_selected_repository) do + :unscheduled | :disabled | :unscheduled | :disabled | :none + :unscheduled | :disabled | :unscheduled | :runnable | :other_repository + :unscheduled | :disabled | :unscheduled | :not_runnable | :none + + :unscheduled | :disabled | :scheduled | :disabled | :none + :unscheduled | :disabled | :scheduled | :runnable | :other_repository + :unscheduled | :disabled | :scheduled | :not_runnable | :none + + :unscheduled | :disabled | :unfinished | :disabled | :none + :unscheduled | :disabled | :unfinished | :runnable | :other_repository + :unscheduled | :disabled | :unfinished | :not_runnable | :other_repository + + :unscheduled | :disabled | :ongoing | :disabled | :none + :unscheduled | :disabled | :ongoing | :runnable | :none + :unscheduled | :disabled | :ongoing | :not_runnable | :none + + :unscheduled | :runnable | :unscheduled | :disabled | :repository + :unscheduled | :runnable | :unscheduled | :runnable | :repository + :unscheduled | :runnable | :unscheduled | :not_runnable | :repository + + :unscheduled | :runnable | :scheduled | :disabled | :repository + :unscheduled | :runnable | :scheduled | :runnable | :repository + :unscheduled | :runnable | :scheduled | :not_runnable | :repository + + :unscheduled | :runnable | :unfinished | :disabled | :repository + :unscheduled | :runnable | :unfinished | :runnable | :repository + :unscheduled | :runnable | :unfinished | :not_runnable | :repository + + :unscheduled | :runnable | :ongoing | :disabled | :repository + :unscheduled | :runnable | :ongoing | :runnable | :repository + :unscheduled | :runnable | :ongoing | :not_runnable | :repository + + :scheduled | :disabled | :unscheduled | :disabled | :none + :scheduled | :disabled | :unscheduled | :runnable | :other_repository + :scheduled | :disabled | :unscheduled | :not_runnable | :none + + :scheduled | :disabled | :scheduled | :disabled | :none + :scheduled | :disabled | :scheduled | :runnable | :other_repository + :scheduled | :disabled | :scheduled | :not_runnable | :none + + :scheduled | :disabled | :unfinished | :disabled | :none + :scheduled | :disabled | :unfinished | :runnable | :other_repository + :scheduled | :disabled | :unfinished | :not_runnable | :other_repository + + :scheduled | :disabled | :ongoing | :disabled | :none + :scheduled | :disabled | :ongoing | :runnable | :none + :scheduled | :disabled | :ongoing | :not_runnable | :none + + :scheduled | :runnable | :unscheduled | :disabled | :repository + :scheduled | :runnable | :unscheduled | :runnable | :other_repository + :scheduled | :runnable | :unscheduled | :not_runnable | :repository + + :scheduled | :runnable | :scheduled | :disabled | :repository + :scheduled | :runnable | :scheduled | :runnable | :repository + :scheduled | :runnable | :scheduled | :not_runnable | :repository + + :scheduled | :runnable | :unfinished | :disabled | :repository + :scheduled | :runnable | :unfinished | :runnable | :repository + :scheduled | :runnable | :unfinished | :not_runnable | :repository + + :scheduled | :runnable | :ongoing | :disabled | :repository + :scheduled | :runnable | :ongoing | :runnable | :repository + :scheduled | :runnable | :ongoing | :not_runnable | :repository + + :scheduled | :not_runnable | :unscheduled | :disabled | :none + :scheduled | :not_runnable | :unscheduled | :runnable | :other_repository + :scheduled | :not_runnable | :unscheduled | :not_runnable | :none + + :scheduled | :not_runnable | :scheduled | :disabled | :none + :scheduled | :not_runnable | :scheduled | :runnable | :other_repository + :scheduled | :not_runnable | :scheduled | :not_runnable | :none + + :scheduled | :not_runnable | :unfinished | :disabled | :none + :scheduled | :not_runnable | :unfinished | :runnable | :other_repository + :scheduled | :not_runnable | :unfinished | :not_runnable | :other_repository + + :scheduled | :not_runnable | :ongoing | :disabled | :none + :scheduled | :not_runnable | :ongoing | :runnable | :none + :scheduled | :not_runnable | :ongoing | :not_runnable | :none + + :unfinished | :disabled | :unscheduled | :disabled | :none + :unfinished | :disabled | :unscheduled | :runnable | :other_repository + :unfinished | :disabled | :unscheduled | :not_runnable | :none + + :unfinished | :disabled | :scheduled | :disabled | :none + :unfinished | :disabled | :scheduled | :runnable | :other_repository + :unfinished | :disabled | :scheduled | :not_runnable | :none + + :unfinished | :disabled | :unfinished | :disabled | :none + :unfinished | :disabled | :unfinished | :runnable | :other_repository + :unfinished | :disabled | :unfinished | :not_runnable | :other_repository + + :unfinished | :disabled | :ongoing | :disabled | :none + :unfinished | :disabled | :ongoing | :runnable | :none + :unfinished | :disabled | :ongoing | :not_runnable | :none + + :unfinished | :runnable | :unscheduled | :disabled | :repository + :unfinished | :runnable | :unscheduled | :runnable | :other_repository + :unfinished | :runnable | :unscheduled | :not_runnable | :repository + + :unfinished | :runnable | :scheduled | :disabled | :repository + :unfinished | :runnable | :scheduled | :runnable | :other_repository + :unfinished | :runnable | :scheduled | :not_runnable | :repository + + :unfinished | :runnable | :unfinished | :disabled | :repository + :unfinished | :runnable | :unfinished | :runnable | :repository + :unfinished | :runnable | :unfinished | :not_runnable | :repository + + :unfinished | :runnable | :ongoing | :disabled | :repository + :unfinished | :runnable | :ongoing | :runnable | :repository + :unfinished | :runnable | :ongoing | :not_runnable | :repository + + :unfinished | :not_runnable | :unscheduled | :disabled | :repository + :unfinished | :not_runnable | :unscheduled | :runnable | :other_repository + :unfinished | :not_runnable | :unscheduled | :not_runnable | :repository + + :unfinished | :not_runnable | :scheduled | :disabled | :repository + :unfinished | :not_runnable | :scheduled | :runnable | :other_repository + :unfinished | :not_runnable | :scheduled | :not_runnable | :repository + + :unfinished | :not_runnable | :unfinished | :disabled | :repository + :unfinished | :not_runnable | :unfinished | :runnable | :repository + :unfinished | :not_runnable | :unfinished | :not_runnable | :repository + + :unfinished | :not_runnable | :ongoing | :disabled | :repository + :unfinished | :not_runnable | :ongoing | :runnable | :repository + :unfinished | :not_runnable | :ongoing | :not_runnable | :repository + + :ongoing | :disabled | :unscheduled | :disabled | :none + :ongoing | :disabled | :unscheduled | :runnable | :other_repository + :ongoing | :disabled | :unscheduled | :not_runnable | :none + + :ongoing | :disabled | :scheduled | :disabled | :none + :ongoing | :disabled | :scheduled | :runnable | :other_repository + :ongoing | :disabled | :scheduled | :not_runnable | :none + + :ongoing | :disabled | :unfinished | :disabled | :none + :ongoing | :disabled | :unfinished | :runnable | :other_repository + :ongoing | :disabled | :unfinished | :not_runnable | :other_repository + + :ongoing | :disabled | :ongoing | :disabled | :none + :ongoing | :disabled | :ongoing | :runnable | :none + :ongoing | :disabled | :ongoing | :not_runnable | :none + + :ongoing | :runnable | :unscheduled | :disabled | :none + :ongoing | :runnable | :unscheduled | :runnable | :other_repository + :ongoing | :runnable | :unscheduled | :not_runnable | :none + + :ongoing | :runnable | :scheduled | :disabled | :none + :ongoing | :runnable | :scheduled | :runnable | :other_repository + :ongoing | :runnable | :scheduled | :not_runnable | :none + + :ongoing | :runnable | :unfinished | :disabled | :none + :ongoing | :runnable | :unfinished | :runnable | :other_repository + :ongoing | :runnable | :unfinished | :not_runnable | :other_repository + + :ongoing | :runnable | :ongoing | :disabled | :none + :ongoing | :runnable | :ongoing | :runnable | :none + :ongoing | :runnable | :ongoing | :not_runnable | :none + + :ongoing | :not_runnable | :unscheduled | :disabled | :none + :ongoing | :not_runnable | :unscheduled | :runnable | :other_repository + :ongoing | :not_runnable | :unscheduled | :not_runnable | :none + + :ongoing | :not_runnable | :scheduled | :disabled | :none + :ongoing | :not_runnable | :scheduled | :runnable | :other_repository + :ongoing | :not_runnable | :scheduled | :not_runnable | :none + + :ongoing | :not_runnable | :unfinished | :disabled | :none + :ongoing | :not_runnable | :unfinished | :runnable | :other_repository + :ongoing | :not_runnable | :unfinished | :not_runnable | :other_repository + + :ongoing | :not_runnable | :ongoing | :disabled | :none + :ongoing | :not_runnable | :ongoing | :runnable | :none + :ongoing | :not_runnable | :ongoing | :not_runnable | :none + end + + with_them do + before do + update_container_repository(repository, repository_cleanup_status, repository_policy_status) + update_container_repository(other_repository, other_repository_cleanup_status, other_repository_policy_status) + end + + subject { worker.send(:container_repository) } + + if params[:expected_selected_repository] == :none + it 'does not select any repository' do + expect(subject).to eq(nil) + end + else + it 'does select a repository' do + selected_repository = expected_selected_repository == :repository ? repository : other_repository + + expect(subject).to eq(selected_repository) + end + end + + def update_container_repository(container_repository, cleanup_status, policy_status) + container_repository.update_column(:expiration_policy_cleanup_status, "cleanup_#{cleanup_status}") + + policy = container_repository.project.container_expiration_policy + + case policy_status + when :disabled + policy.update!(enabled: false) + when :runnable + policy.update!(enabled: true) + policy.update_column(:next_run_at, 5.minutes.ago) + when :not_runnable + policy.update!(enabled: true) + policy.update_column(:next_run_at, 5.minutes.from_now) + end + end + end + end + + context 'with another repository in cleanup unfinished state' do + let_it_be(:another_repository) { create(:container_repository, :cleanup_unfinished) } + + before do + policy.update_column(:next_run_at, 5.minutes.ago) + end + + it 'process the cleanup scheduled repository first' do + service_response = cleanup_service_response(repository: repository) + expect(ContainerExpirationPolicies::CleanupService) + .to receive(:new).with(repository).and_return(double(execute: service_response)) + expect_log_extra_metadata(service_response: service_response) + + subject end end end @@ -230,17 +501,18 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do end expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_tags_service_truncated, truncated) expect(worker).to receive(:log_extra_metadata_on_done).with(:running_jobs_count, 0) + + if service_response.error? + expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_error_message, service_response.message) + end end end describe '#remaining_work_count' do subject { worker.remaining_work_count } - context 'with loopless disabled' do - before do - stub_feature_flags(container_registry_expiration_policies_loopless: false) - end - context 'with container repositoires waiting for cleanup' do + shared_examples 'handling all conditions' do + context 'with container repositories waiting for cleanup' do let_it_be(:unfinished_repositories) { create_list(:container_repository, 2, :cleanup_unfinished) } it { is_expected.to eq(3) } @@ -259,6 +531,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do context 'with no container repositories waiting for cleanup' do before do repository.cleanup_ongoing! + policy.update_column(:next_run_at, 5.minutes.from_now) end it { is_expected.to eq(0) } @@ -274,6 +547,32 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do end end end + + context 'with loopless enabled' do + let_it_be(:disabled_repository) { create(:container_repository, :cleanup_scheduled) } + + let(:capacity) { 10 } + + before do + stub_feature_flags(container_registry_expiration_policies_loopless: true) + stub_application_setting(container_registry_expiration_policies_worker_capacity: capacity) + + # loopless mode is more accurate that non loopless: policies need to be enabled + ContainerExpirationPolicy.update_all(enabled: true) + repository.project.container_expiration_policy.update_column(:next_run_at, 5.minutes.ago) + disabled_repository.project.container_expiration_policy.update_column(:enabled, false) + end + + it_behaves_like 'handling all conditions' + end + + context 'with loopless disabled' do + before do + stub_feature_flags(container_registry_expiration_policies_loopless: false) + end + + it_behaves_like 'handling all conditions' + end end describe '#max_running_jobs' do @@ -285,20 +584,14 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do stub_application_setting(container_registry_expiration_policies_worker_capacity: capacity) end - context 'with loopless disabled' do + it { is_expected.to eq(capacity) } + + context 'with feature flag disabled' do before do - stub_feature_flags(container_registry_expiration_policies_loopless: false) + stub_feature_flags(container_registry_expiration_policies_throttling: false) end - it { is_expected.to eq(capacity) } - - context 'with feature flag disabled' do - before do - stub_feature_flags(container_registry_expiration_policies_throttling: false) - end - - it { is_expected.to eq(0) } - end + it { is_expected.to eq(0) } end end @@ -306,4 +599,8 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do expect(worker.logger) .to receive(:info).with(worker.structured_payload(structure)) end + + def loopless_enabled? + Feature.enabled?(:container_registry_expiration_policies_loopless) + end end diff --git a/spec/workers/issuable/label_links_destroy_worker_spec.rb b/spec/workers/issuable/label_links_destroy_worker_spec.rb new file mode 100644 index 00000000000..a838f1c8017 --- /dev/null +++ b/spec/workers/issuable/label_links_destroy_worker_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Issuable::LabelLinksDestroyWorker do + let(:job_args) { [1, 'MergeRequest'] } + let(:service) { double } + + include_examples 'an idempotent worker' do + it 'calls the Issuable::DestroyLabelLinksService' do + expect(::Issuable::DestroyLabelLinksService).to receive(:new).twice.and_return(service) + expect(service).to receive(:execute).twice + + subject + end + end +end diff --git a/spec/workers/project_service_worker_spec.rb b/spec/workers/project_service_worker_spec.rb index c638b7472ff..237f501e0ec 100644 --- a/spec/workers/project_service_worker_spec.rb +++ b/spec/workers/project_service_worker_spec.rb @@ -6,7 +6,7 @@ RSpec.describe ProjectServiceWorker, '#perform' do let(:service) { JiraService.new } before do - allow(Service).to receive(:find).and_return(service) + allow(Integration).to receive(:find).and_return(service) end it 'executes service with given data' do diff --git a/spec/workers/projects/post_creation_worker_spec.rb b/spec/workers/projects/post_creation_worker_spec.rb index b15b7b76b56..c2f42f03299 100644 --- a/spec/workers/projects/post_creation_worker_spec.rb +++ b/spec/workers/projects/post_creation_worker_spec.rb @@ -18,7 +18,7 @@ RSpec.describe Projects::PostCreationWorker do let(:job_args) { [nil] } it 'does not create prometheus service' do - expect { subject }.not_to change { Service.count } + expect { subject }.not_to change { Integration.count } end end |