diff options
Diffstat (limited to 'spec/services/ci')
29 files changed, 555 insertions, 203 deletions
diff --git a/spec/services/ci/after_requeue_job_service_spec.rb b/spec/services/ci/after_requeue_job_service_spec.rb index 2465bac7d10..d2acf3ad2f1 100644 --- a/spec/services/ci/after_requeue_job_service_spec.rb +++ b/spec/services/ci/after_requeue_job_service_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Ci::AfterRequeueJobService do let_it_be(:project) { create(:project) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let(:pipeline) { create(:ci_pipeline, project: project) } diff --git a/spec/services/ci/archive_trace_service_spec.rb b/spec/services/ci/archive_trace_service_spec.rb index b08ba6fd5e5..bf2e5302d2e 100644 --- a/spec/services/ci/archive_trace_service_spec.rb +++ b/spec/services/ci/archive_trace_service_spec.rb @@ -15,6 +15,25 @@ RSpec.describe Ci::ArchiveTraceService, '#execute' do expect(job.trace_metadata.trace_artifact).to eq(job.job_artifacts_trace) end + context 'integration hooks' do + it do + stub_feature_flags(datadog_integration_logs_collection: [job.project]) + + expect(job.project).to receive(:execute_integrations) do |data, hook_type| + expect(data).to eq Gitlab::DataBuilder::ArchiveTrace.build(job) + expect(hook_type).to eq :archive_trace_hooks + end + expect { subject }.not_to raise_error + end + + it 'with feature flag disabled' do + stub_feature_flags(datadog_integration_logs_collection: false) + + expect(job.project).not_to receive(:execute_integrations) + expect { subject }.not_to raise_error + end + end + context 'when trace is already archived' do let!(:job) { create(:ci_build, :success, :trace_artifact) } diff --git a/spec/services/ci/create_downstream_pipeline_service_spec.rb b/spec/services/ci/create_downstream_pipeline_service_spec.rb index 2237fd76d07..d61abf6a6ee 100644 --- a/spec/services/ci/create_downstream_pipeline_service_spec.rb +++ b/spec/services/ci/create_downstream_pipeline_service_spec.rb @@ -604,7 +604,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do context 'when configured with bridge job rules' do before do stub_ci_pipeline_yaml_file(config) - downstream_project.add_maintainer(upstream_project.owner) + downstream_project.add_maintainer(upstream_project.first_owner) end let(:config) do @@ -622,13 +622,13 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do end let(:primary_pipeline) do - Ci::CreatePipelineService.new(upstream_project, upstream_project.owner, { ref: 'master' }) + Ci::CreatePipelineService.new(upstream_project, upstream_project.first_owner, { ref: 'master' }) .execute(:push, save_on_errors: false) .payload end let(:bridge) { primary_pipeline.processables.find_by(name: 'bridge-job') } - let(:service) { described_class.new(upstream_project, upstream_project.owner) } + let(:service) { described_class.new(upstream_project, upstream_project.first_owner) } context 'that include the bridge job' do it 'creates the downstream pipeline' do diff --git a/spec/services/ci/create_pipeline_service/cache_spec.rb b/spec/services/ci/create_pipeline_service/cache_spec.rb index f5f162e4578..ca85a8cce64 100644 --- a/spec/services/ci/create_pipeline_service/cache_spec.rb +++ b/spec/services/ci/create_pipeline_service/cache_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do context 'cache' do let(:project) { create(:project, :custom_repo, files: files) } - let(:user) { project.owner } + let(:user) { project.first_owner } let(:ref) { 'refs/heads/master' } let(:source) { :push } let(:service) { described_class.new(project, user, { ref: ref }) } diff --git a/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb b/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb index c69c91593ae..e62a94b6df8 100644 --- a/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb +++ b/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do describe 'creation errors and warnings' do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let(:ref) { 'refs/heads/master' } let(:source) { :push } diff --git a/spec/services/ci/create_pipeline_service/custom_config_content_spec.rb b/spec/services/ci/create_pipeline_service/custom_config_content_spec.rb index f150a4f8b51..a0cbf14d936 100644 --- a/spec/services/ci/create_pipeline_service/custom_config_content_spec.rb +++ b/spec/services/ci/create_pipeline_service/custom_config_content_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let(:ref) { 'refs/heads/master' } let(:service) { described_class.new(project, user, { ref: ref }) } diff --git a/spec/services/ci/create_pipeline_service/custom_yaml_tags_spec.rb b/spec/services/ci/create_pipeline_service/custom_yaml_tags_spec.rb index 026111d59f1..716a929830e 100644 --- a/spec/services/ci/create_pipeline_service/custom_yaml_tags_spec.rb +++ b/spec/services/ci/create_pipeline_service/custom_yaml_tags_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do describe '!reference tags' do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let(:ref) { 'refs/heads/master' } let(:source) { :push } diff --git a/spec/services/ci/create_pipeline_service/dry_run_spec.rb b/spec/services/ci/create_pipeline_service/dry_run_spec.rb index ae43c63b516..9a7e97fb12b 100644 --- a/spec/services/ci/create_pipeline_service/dry_run_spec.rb +++ b/spec/services/ci/create_pipeline_service/dry_run_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let(:ref) { 'refs/heads/master' } let(:service) { described_class.new(project, user, { ref: ref }) } diff --git a/spec/services/ci/create_pipeline_service/include_spec.rb b/spec/services/ci/create_pipeline_service/include_spec.rb index aa01977272a..3116801d50c 100644 --- a/spec/services/ci/create_pipeline_service/include_spec.rb +++ b/spec/services/ci/create_pipeline_service/include_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do context 'include:' do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let(:ref) { 'refs/heads/master' } let(:variables_attributes) { [{ key: 'MYVAR', secret_value: 'hello' }] } diff --git a/spec/services/ci/create_pipeline_service/logger_spec.rb b/spec/services/ci/create_pipeline_service/logger_spec.rb index dfe0859015d..53e5f0dd7f2 100644 --- a/spec/services/ci/create_pipeline_service/logger_spec.rb +++ b/spec/services/ci/create_pipeline_service/logger_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do context 'pipeline logger' do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let(:ref) { 'refs/heads/master' } let(:service) { described_class.new(project, user, { ref: ref }) } @@ -35,7 +35,10 @@ RSpec.describe Ci::CreatePipelineService do 'pipeline_creation_service_duration_s' => a_kind_of(Numeric), 'pipeline_creation_duration_s' => counters, 'pipeline_size_count' => counters, - 'pipeline_step_gitlab_ci_pipeline_chain_seed_duration_s' => counters + 'pipeline_step_gitlab_ci_pipeline_chain_seed_duration_s' => counters, + 'pipeline_seed_build_inclusion_duration_s' => counters, + 'pipeline_builds_tags_count' => a_kind_of(Numeric), + 'pipeline_builds_distinct_tags_count' => a_kind_of(Numeric) } end @@ -81,7 +84,6 @@ RSpec.describe Ci::CreatePipelineService do { 'pipeline_creation_caller' => 'Ci::CreatePipelineService', 'pipeline_source' => 'push', - 'pipeline_id' => nil, 'pipeline_persisted' => false, 'project_id' => project.id, 'pipeline_creation_service_duration_s' => a_kind_of(Numeric), diff --git a/spec/services/ci/create_pipeline_service/merge_requests_spec.rb b/spec/services/ci/create_pipeline_service/merge_requests_spec.rb index a1f85faa69f..de19ef363fb 100644 --- a/spec/services/ci/create_pipeline_service/merge_requests_spec.rb +++ b/spec/services/ci/create_pipeline_service/merge_requests_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do context 'merge requests handling' do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let(:ref) { 'refs/heads/feature' } let(:source) { :push } diff --git a/spec/services/ci/create_pipeline_service/needs_spec.rb b/spec/services/ci/create_pipeline_service/needs_spec.rb index 9070d86f7f6..abd17ccdd6a 100644 --- a/spec/services/ci/create_pipeline_service/needs_spec.rb +++ b/spec/services/ci/create_pipeline_service/needs_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do context 'needs' do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let(:ref) { 'refs/heads/master' } let(:source) { :push } diff --git a/spec/services/ci/create_pipeline_service/parallel_spec.rb b/spec/services/ci/create_pipeline_service/parallel_spec.rb index 6b455bf4874..ae28b74fef5 100644 --- a/spec/services/ci/create_pipeline_service/parallel_spec.rb +++ b/spec/services/ci/create_pipeline_service/parallel_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let(:service) { described_class.new(project, user, { ref: 'master' }) } let(:pipeline) { service.execute(:push).payload } diff --git a/spec/services/ci/create_pipeline_service/parameter_content_spec.rb b/spec/services/ci/create_pipeline_service/parameter_content_spec.rb index 761504ffb58..c28bc9d8c13 100644 --- a/spec/services/ci/create_pipeline_service/parameter_content_spec.rb +++ b/spec/services/ci/create_pipeline_service/parameter_content_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let(:service) { described_class.new(project, user, { ref: 'refs/heads/master' }) } let(:content) do diff --git a/spec/services/ci/create_pipeline_service/pre_post_stages_spec.rb b/spec/services/ci/create_pipeline_service/pre_post_stages_spec.rb index 5e34eeb99db..c6e69862422 100644 --- a/spec/services/ci/create_pipeline_service/pre_post_stages_spec.rb +++ b/spec/services/ci/create_pipeline_service/pre_post_stages_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do describe '.pre/.post stages' do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let(:source) { :push } let(:service) { described_class.new(project, user, { ref: ref }) } diff --git a/spec/services/ci/create_pipeline_service/rules_spec.rb b/spec/services/ci/create_pipeline_service/rules_spec.rb index d0915f099de..d0ce1c5aba8 100644 --- a/spec/services/ci/create_pipeline_service/rules_spec.rb +++ b/spec/services/ci/create_pipeline_service/rules_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do let(:project) { create(:project, :repository) } - let(:user) { project.owner } + let(:user) { project.first_owner } let(:ref) { 'refs/heads/master' } let(:source) { :push } let(:service) { described_class.new(project, user, { ref: ref }) } diff --git a/spec/services/ci/create_pipeline_service/tags_spec.rb b/spec/services/ci/create_pipeline_service/tags_spec.rb index cbbeb870c5f..61c2415fa33 100644 --- a/spec/services/ci/create_pipeline_service/tags_spec.rb +++ b/spec/services/ci/create_pipeline_service/tags_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do describe 'tags:' do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let(:ref) { 'refs/heads/master' } let(:service) { described_class.new(project, user, { ref: ref }) } diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index ef879d536c3..a7026f5062e 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Ci::CreatePipelineService do include ProjectForksHelper let_it_be_with_refind(:project) { create(:project, :repository) } - let_it_be_with_reload(:user) { project.owner } + let_it_be_with_reload(:user) { project.first_owner } let(:ref_name) { 'refs/heads/master' } @@ -146,140 +146,22 @@ RSpec.describe Ci::CreatePipelineService do end context 'when merge requests already exist for this source branch' do - let(:merge_request_1) do + let!(:merge_request_1) do create(:merge_request, source_branch: 'feature', target_branch: "master", source_project: project) end - let(:merge_request_2) do + let!(:merge_request_2) do create(:merge_request, source_branch: 'feature', target_branch: "v1.1.0", source_project: project) end - context 'when related merge request is already merged' do - let!(:merged_merge_request) do - create(:merge_request, source_branch: 'master', target_branch: "branch_2", source_project: project, state: 'merged') - end - - it 'does not schedule update head pipeline job' do - expect(UpdateHeadPipelineForMergeRequestWorker).not_to receive(:perform_async).with(merged_merge_request.id) - - execute_service - end - end - context 'when the head pipeline sha equals merge request sha' do it 'updates head pipeline of each merge request', :sidekiq_might_not_need_inline do - merge_request_1 - merge_request_2 - head_pipeline = execute_service(ref: 'feature', after: nil).payload expect(merge_request_1.reload.head_pipeline).to eq(head_pipeline) expect(merge_request_2.reload.head_pipeline).to eq(head_pipeline) end end - - context 'when the head pipeline sha does not equal merge request sha' do - it 'does not update the head piepeline of MRs' do - merge_request_1 - merge_request_2 - - allow_any_instance_of(Ci::Pipeline).to receive(:latest?).and_return(true) - - expect { execute_service(after: 'ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }.not_to raise_error - - last_pipeline = Ci::Pipeline.last - - expect(merge_request_1.reload.head_pipeline).not_to eq(last_pipeline) - expect(merge_request_2.reload.head_pipeline).not_to eq(last_pipeline) - end - end - - context 'when there is no pipeline for source branch' do - it "does not update merge request head pipeline" do - merge_request = create(:merge_request, source_branch: 'feature', - target_branch: "branch_1", - source_project: project) - - head_pipeline = execute_service.payload - - expect(merge_request.reload.head_pipeline).not_to eq(head_pipeline) - end - end - - context 'when merge request target project is different from source project' do - let!(:project) { fork_project(target_project, nil, repository: true) } - let!(:target_project) { create(:project, :repository) } - let!(:user) { create(:user) } - - before do - project.add_developer(user) - end - - it 'updates head pipeline for merge request', :sidekiq_might_not_need_inline do - merge_request = create(:merge_request, source_branch: 'feature', - target_branch: "master", - source_project: project, - target_project: target_project) - - head_pipeline = execute_service(ref: 'feature', after: nil).payload - - expect(merge_request.reload.head_pipeline).to eq(head_pipeline) - end - end - - context 'when the pipeline is not the latest for the branch' do - it 'does not update merge request head pipeline' do - merge_request = create(:merge_request, source_branch: 'master', - target_branch: "branch_1", - source_project: project) - - allow_any_instance_of(MergeRequest) - .to receive(:find_actual_head_pipeline) { } - - execute_service - - expect(merge_request.reload.head_pipeline).to be_nil - end - end - - context 'when pipeline has errors' do - before do - stub_ci_pipeline_yaml_file('some invalid syntax') - end - - it 'updates merge request head pipeline reference', :sidekiq_might_not_need_inline do - merge_request = create(:merge_request, source_branch: 'master', - target_branch: 'feature', - source_project: project) - - head_pipeline = execute_service.payload - - expect(head_pipeline).to be_persisted - expect(head_pipeline.yaml_errors).to be_present - expect(head_pipeline.messages).to be_present - expect(merge_request.reload.head_pipeline).to eq head_pipeline - end - end - - context 'when pipeline has been skipped' do - before do - allow_any_instance_of(Ci::Pipeline) - .to receive(:git_commit_message) - .and_return('some commit [ci skip]') - end - - it 'updates merge request head pipeline', :sidekiq_might_not_need_inline do - merge_request = create(:merge_request, source_branch: 'master', - target_branch: 'feature', - source_project: project) - - head_pipeline = execute_service.payload - - expect(head_pipeline).to be_skipped - expect(head_pipeline).to be_persisted - expect(merge_request.reload.head_pipeline).to eq head_pipeline - end - end end context 'auto-cancel enabled' do @@ -1655,7 +1537,7 @@ RSpec.describe Ci::CreatePipelineService do expect(pipeline.target_sha).to be_nil end - it 'schedules update for the head pipeline of the merge request' do + it 'schedules update for the head pipeline of the merge request', :sidekiq_inline do expect(UpdateHeadPipelineForMergeRequestWorker) .to receive(:perform_async).with(merge_request.id) diff --git a/spec/services/ci/destroy_pipeline_service_spec.rb b/spec/services/ci/destroy_pipeline_service_spec.rb index 6c1c02b2875..045051c7152 100644 --- a/spec/services/ci/destroy_pipeline_service_spec.rb +++ b/spec/services/ci/destroy_pipeline_service_spec.rb @@ -10,7 +10,7 @@ RSpec.describe ::Ci::DestroyPipelineService do subject { described_class.new(project, user).execute(pipeline) } context 'user is owner' do - let(:user) { project.owner } + let(:user) { project.first_owner } it 'destroys the pipeline' do subject @@ -66,6 +66,28 @@ RSpec.describe ::Ci::DestroyPipelineService do expect { subject }.to change { Ci::DeletedObject.count } end end + + context 'when job has trace chunks' do + let(:connection_params) { Gitlab.config.artifacts.object_store.connection.symbolize_keys } + let(:connection) { ::Fog::Storage.new(connection_params) } + + before do + stub_object_storage(connection_params: connection_params, remote_directory: 'artifacts') + stub_artifacts_object_storage + end + + let!(:trace_chunk) { create(:ci_build_trace_chunk, :fog_with_data, build: build) } + + it 'destroys associated trace chunks' do + subject + + expect { trace_chunk.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'removes data from object store' do + expect { subject }.to change { Ci::BuildTraceChunks::Fog.new.data(trace_chunk) } + end + end end context 'when pipeline is in cancelable state' do diff --git a/spec/services/ci/job_artifacts/delete_project_artifacts_service_spec.rb b/spec/services/ci/job_artifacts/delete_project_artifacts_service_spec.rb new file mode 100644 index 00000000000..74fa42962f3 --- /dev/null +++ b/spec/services/ci/job_artifacts/delete_project_artifacts_service_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::JobArtifacts::DeleteProjectArtifactsService do + let_it_be(:project) { create(:project) } + + subject { described_class.new(project: project) } + + describe '#execute' do + it 'enqueues a Ci::ExpireProjectBuildArtifactsWorker' do + expect(Ci::JobArtifacts::ExpireProjectBuildArtifactsWorker).to receive(:perform_async).with(project.id).and_call_original + + subject.execute + end + end +end diff --git a/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb index e71f1a4266a..e95a449d615 100644 --- a/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb @@ -20,7 +20,7 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s context 'with preloaded relationships' do before do - stub_const("#{described_class}::LOOP_LIMIT", 1) + stub_const("#{described_class}::LARGE_LOOP_LIMIT", 1) end context 'with ci_destroy_unlocked_job_artifacts feature flag disabled' do @@ -53,46 +53,6 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s log = ActiveRecord::QueryRecorder.new { subject } expect(log.count).to be_within(1).of(8) end - - context 'with several locked-unknown artifact records' do - before do - stub_const("#{described_class}::LOOP_LIMIT", 10) - stub_const("#{described_class}::BATCH_SIZE", 2) - end - - let!(:lockable_artifact_records) do - [ - create(:ci_job_artifact, :metadata, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: locked_job), - create(:ci_job_artifact, :junit, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: locked_job), - create(:ci_job_artifact, :sast, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: locked_job), - create(:ci_job_artifact, :cobertura, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: locked_job), - create(:ci_job_artifact, :trace, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: locked_job) - ] - end - - let!(:unlockable_artifact_records) do - [ - create(:ci_job_artifact, :metadata, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: job), - create(:ci_job_artifact, :junit, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: job), - create(:ci_job_artifact, :sast, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: job), - create(:ci_job_artifact, :cobertura, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: job), - create(:ci_job_artifact, :trace, :expired, locked: ::Ci::JobArtifact.lockeds[:unknown], job: job), - artifact - ] - end - - it 'updates the locked status of job artifacts from artifacts-locked pipelines' do - subject - - expect(lockable_artifact_records).to be_all(&:persisted?) - expect(lockable_artifact_records).to be_all { |artifact| artifact.reload.artifact_artifacts_locked? } - end - - it 'unlocks and then destroys job artifacts from artifacts-unlocked pipelines' do - expect { subject }.to change { Ci::JobArtifact.count }.by(-6) - expect(Ci::JobArtifact.where(id: unlockable_artifact_records.map(&:id))).to be_empty - end - end end end @@ -159,7 +119,7 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s let!(:artifact) { create(:ci_job_artifact, :expired, job: job, locked: job.pipeline.locked) } before do - stub_const("#{described_class}::LOOP_LIMIT", 10) + stub_const("#{described_class}::LARGE_LOOP_LIMIT", 10) end context 'when the import fails' do @@ -229,7 +189,8 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s context 'when loop reached loop limit' do before do - stub_const("#{described_class}::LOOP_LIMIT", 1) + stub_feature_flags(ci_artifact_fast_removal_large_loop_limit: false) + stub_const("#{described_class}::SMALL_LOOP_LIMIT", 1) end it 'destroys one artifact' do diff --git a/spec/services/ci/job_artifacts/expire_project_build_artifacts_service_spec.rb b/spec/services/ci/job_artifacts/expire_project_build_artifacts_service_spec.rb new file mode 100644 index 00000000000..fb9dd6b876b --- /dev/null +++ b/spec/services/ci/job_artifacts/expire_project_build_artifacts_service_spec.rb @@ -0,0 +1,157 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::JobArtifacts::ExpireProjectBuildArtifactsService do + let_it_be(:project) { create(:project) } + let_it_be(:pipeline, reload: true) { create(:ci_pipeline, :unlocked, project: project) } + + let(:expiry_time) { Time.current } + + RSpec::Matchers.define :have_locked_status do |expected_status| + match do |job_artifacts| + predicate = "#{expected_status}?".to_sym + job_artifacts.all? { |artifact| artifact.__send__(predicate) } + end + end + + RSpec::Matchers.define :expire_at do |expected_expiry| + match do |job_artifacts| + job_artifacts.all? { |artifact| artifact.expire_at.to_i == expected_expiry.to_i } + end + end + + RSpec::Matchers.define :have_no_expiry do + match do |job_artifacts| + job_artifacts.all? { |artifact| artifact.expire_at.nil? } + end + end + + describe '#execute' do + subject(:execute) { described_class.new(project.id, expiry_time).execute } + + context 'with job containing erasable artifacts' do + let_it_be(:job, reload: true) { create(:ci_build, :erasable, pipeline: pipeline) } + + it 'unlocks erasable job artifacts' do + execute + + expect(job.job_artifacts).to have_locked_status(:artifact_unlocked) + end + + it 'expires erasable job artifacts' do + execute + + expect(job.job_artifacts).to expire_at(expiry_time) + end + end + + context 'with job containing trace artifacts' do + let_it_be(:job, reload: true) { create(:ci_build, :trace_artifact, pipeline: pipeline) } + + it 'does not unlock trace artifacts' do + execute + + expect(job.job_artifacts).to have_locked_status(:artifact_unknown) + end + + it 'does not expire trace artifacts' do + execute + + expect(job.job_artifacts).to have_no_expiry + end + end + + context 'with job from artifact locked pipeline' do + let_it_be(:job, reload: true) { create(:ci_build, pipeline: pipeline) } + let_it_be(:locked_artifact, reload: true) { create(:ci_job_artifact, :locked, job: job) } + + before do + pipeline.artifacts_locked! + end + + it 'does not unlock locked artifacts' do + execute + + expect(job.job_artifacts).to have_locked_status(:artifact_artifacts_locked) + end + + it 'does not expire locked artifacts' do + execute + + expect(job.job_artifacts).to have_no_expiry + end + end + + context 'with job containing both erasable and trace artifacts' do + let_it_be(:job, reload: true) { create(:ci_build, pipeline: pipeline) } + let_it_be(:erasable_artifact, reload: true) { create(:ci_job_artifact, :archive, job: job) } + let_it_be(:trace_artifact, reload: true) { create(:ci_job_artifact, :trace, job: job) } + + it 'unlocks erasable artifacts' do + execute + + expect(erasable_artifact.artifact_unlocked?).to be_truthy + end + + it 'expires erasable artifacts' do + execute + + expect(erasable_artifact.expire_at.to_i).to eq(expiry_time.to_i) + end + + it 'does not unlock trace artifacts' do + execute + + expect(trace_artifact.artifact_unlocked?).to be_falsey + end + + it 'does not expire trace artifacts' do + execute + + expect(trace_artifact.expire_at).to be_nil + end + end + + context 'with multiple pipelines' do + let_it_be(:job, reload: true) { create(:ci_build, :erasable, pipeline: pipeline) } + + let_it_be(:pipeline2, reload: true) { create(:ci_pipeline, :unlocked, project: project) } + let_it_be(:job2, reload: true) { create(:ci_build, :erasable, pipeline: pipeline) } + + it 'unlocks artifacts across pipelines' do + execute + + expect(job.job_artifacts).to have_locked_status(:artifact_unlocked) + expect(job2.job_artifacts).to have_locked_status(:artifact_unlocked) + end + + it 'expires artifacts across pipelines' do + execute + + expect(job.job_artifacts).to expire_at(expiry_time) + expect(job2.job_artifacts).to expire_at(expiry_time) + end + end + + context 'with artifacts belonging to another project' do + let_it_be(:job, reload: true) { create(:ci_build, :erasable, pipeline: pipeline) } + + let_it_be(:another_project, reload: true) { create(:project) } + let_it_be(:another_pipeline, reload: true) { create(:ci_pipeline, project: another_project) } + let_it_be(:another_job, reload: true) { create(:ci_build, :erasable, pipeline: another_pipeline) } + + it 'does not unlock erasable artifacts in other projects' do + execute + + expect(another_job.job_artifacts).to have_locked_status(:artifact_unknown) + end + + it 'does not expire erasable artifacts in other projects' do + execute + + expect(another_job.job_artifacts).to have_no_expiry + end + end + end +end diff --git a/spec/services/ci/pipeline_processing/atomic_processing_service_spec.rb b/spec/services/ci/pipeline_processing/atomic_processing_service_spec.rb index 31e1b0a896d..26bc6f747e1 100644 --- a/spec/services/ci/pipeline_processing/atomic_processing_service_spec.rb +++ b/spec/services/ci/pipeline_processing/atomic_processing_service_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Ci::PipelineProcessing::AtomicProcessingService do describe 'Pipeline Processing Service Tests With Yaml' do let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } where(:test_file_path) do Dir.glob(Rails.root.join('spec/services/ci/pipeline_processing/test_cases/*.yml')) @@ -65,7 +65,7 @@ RSpec.describe Ci::PipelineProcessing::AtomicProcessingService do describe 'Pipeline Processing Service' do let(:project) { create(:project, :repository) } - let(:user) { project.owner } + let(:user) { project.first_owner } let(:pipeline) do create(:ci_empty_pipeline, ref: 'master', project: project) diff --git a/spec/services/ci/pipelines/add_job_service_spec.rb b/spec/services/ci/pipelines/add_job_service_spec.rb index 709a840c644..560724a1c6a 100644 --- a/spec/services/ci/pipelines/add_job_service_spec.rb +++ b/spec/services/ci/pipelines/add_job_service_spec.rb @@ -31,7 +31,7 @@ RSpec.describe Ci::Pipelines::AddJobService do execute end.to change { job.slice(:pipeline, :project, :ref) }.to( pipeline: pipeline, project: pipeline.project, ref: pipeline.ref - ) + ).and change { job.metadata.project }.to(pipeline.project) end it 'returns a service response with the job as payload' do diff --git a/spec/services/ci/play_build_service_spec.rb b/spec/services/ci/play_build_service_spec.rb index 34f77260334..85ef8b60af4 100644 --- a/spec/services/ci/play_build_service_spec.rb +++ b/spec/services/ci/play_build_service_spec.rb @@ -122,7 +122,7 @@ RSpec.describe Ci::PlayBuildService, '#execute' do end context 'when build is not a playable manual action' do - let(:build) { create(:ci_build, when: :manual, pipeline: pipeline) } + let(:build) { create(:ci_build, :success, pipeline: pipeline) } let!(:branch) { create(:protected_branch, :developers_can_merge, name: build.ref, project: project) } it 'duplicates the build' do @@ -138,6 +138,18 @@ RSpec.describe Ci::PlayBuildService, '#execute' do expect(build.user).not_to eq user expect(duplicate.user).to eq user end + + context 'and is not retryable' do + let(:build) { create(:ci_build, :deployment_rejected, pipeline: pipeline) } + + it 'does not duplicate the build' do + expect { service.execute(build) }.not_to change { Ci::Build.count } + end + + it 'does not enqueue the build' do + expect { service.execute(build) }.not_to change { build.status } + end + end end context 'when build is not action' do diff --git a/spec/services/ci/process_sync_events_service_spec.rb b/spec/services/ci/process_sync_events_service_spec.rb index 00b670ff54f..8b7717fe4bf 100644 --- a/spec/services/ci/process_sync_events_service_spec.rb +++ b/spec/services/ci/process_sync_events_service_spec.rb @@ -28,10 +28,10 @@ RSpec.describe Ci::ProcessSyncEventsService do it 'consumes events' do expect { execute }.to change(Projects::SyncEvent, :count).from(2).to(0) - expect(project1.ci_project_mirror).to have_attributes( + expect(project1.reload.ci_project_mirror).to have_attributes( namespace_id: parent_group_1.id ) - expect(project2.ci_project_mirror).to have_attributes( + expect(project2.reload.ci_project_mirror).to have_attributes( namespace_id: parent_group_2.id ) end @@ -71,6 +71,24 @@ RSpec.describe Ci::ProcessSyncEventsService do expect { execute }.not_to change(Projects::SyncEvent, :count) end end + + it 'does not delete non-executed events' do + new_project = create(:project) + sync_event_class.delete_all + + project1.update!(group: parent_group_2) + new_project.update!(group: parent_group_1) + project2.update!(group: parent_group_1) + + new_project_sync_event = new_project.sync_events.last + + allow(sync_event_class).to receive(:preload_synced_relation).and_return( + sync_event_class.where.not(id: new_project_sync_event) + ) + + expect { execute }.to change(Projects::SyncEvent, :count).from(3).to(1) + expect(new_project_sync_event.reload).to be_persisted + end end context 'for Namespaces::SyncEvent' do @@ -88,10 +106,10 @@ RSpec.describe Ci::ProcessSyncEventsService do it 'consumes events' do expect { execute }.to change(Namespaces::SyncEvent, :count).from(2).to(0) - expect(group.ci_namespace_mirror).to have_attributes( + expect(group.reload.ci_namespace_mirror).to have_attributes( traversal_ids: [parent_group_1.id, parent_group_2.id, group.id] ) - expect(parent_group_2.ci_namespace_mirror).to have_attributes( + expect(parent_group_2.reload.ci_namespace_mirror).to have_attributes( traversal_ids: [parent_group_1.id, parent_group_2.id] ) end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index 866015aa523..251159864f5 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -827,11 +827,17 @@ module Ci end context 'when project already has running jobs' do - let!(:build2) { create(:ci_build, :running, pipeline: pipeline, runner: shared_runner) } - let!(:build3) { create(:ci_build, :running, pipeline: pipeline, runner: shared_runner) } + let(:build2) { create(:ci_build, :running, pipeline: pipeline, runner: shared_runner) } + let(:build3) { create(:ci_build, :running, pipeline: pipeline, runner: shared_runner) } + + before do + ::Ci::RunningBuild.upsert_shared_runner_build!(build2) + ::Ci::RunningBuild.upsert_shared_runner_build!(build3) + end it 'counts job queuing time histogram with expected labels' do allow(attempt_counter).to receive(:increment) + expect(job_queue_duration_seconds).to receive(:observe) .with({ shared_runner: expected_shared_runner, jobs_running_for_project: expected_jobs_running_for_project_third_job, @@ -845,6 +851,14 @@ module Ci shared_examples 'metrics collector' do it_behaves_like 'attempt counter collector' it_behaves_like 'jobs queueing time histogram collector' + + context 'when using denormalized data is disabled' do + before do + stub_feature_flags(ci_pending_builds_maintain_denormalized_data: false) + end + + it_behaves_like 'jobs queueing time histogram collector' + end end context 'when shared runner is used' do @@ -875,6 +889,16 @@ module Ci it_behaves_like 'metrics collector' end + context 'when max running jobs bucket size is exceeded' do + before do + stub_const('Gitlab::Ci::Queue::Metrics::JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET', 1) + end + + let(:expected_jobs_running_for_project_third_job) { '1+' } + + it_behaves_like 'metrics collector' + end + context 'when pending job with queued_at=nil is used' do before do pending_job.update!(queued_at: nil) diff --git a/spec/services/ci/register_runner_service_spec.rb b/spec/services/ci/register_runner_service_spec.rb new file mode 100644 index 00000000000..e813a1d8b31 --- /dev/null +++ b/spec/services/ci/register_runner_service_spec.rb @@ -0,0 +1,226 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Ci::RegisterRunnerService do + let(:registration_token) { 'abcdefg123456' } + + before do + stub_feature_flags(runner_registration_control: false) + stub_application_setting(runners_registration_token: registration_token) + stub_application_setting(valid_runner_registrars: ApplicationSetting::VALID_RUNNER_REGISTRAR_TYPES) + end + + describe '#execute' do + let(:token) { } + let(:args) { {} } + + subject { described_class.new.execute(token, args) } + + context 'when no token is provided' do + let(:token) { '' } + + it 'returns nil' do + is_expected.to be_nil + end + end + + context 'when invalid token is provided' do + let(:token) { 'invalid' } + + it 'returns nil' do + is_expected.to be_nil + end + end + + context 'when valid token is provided' do + context 'with a registration token' do + let(:token) { registration_token } + + it 'creates runner with default values' do + is_expected.to be_an_instance_of(::Ci::Runner) + expect(subject.persisted?).to be_truthy + expect(subject.run_untagged).to be true + expect(subject.active).to be true + expect(subject.token).not_to eq(registration_token) + expect(subject).to be_instance_type + end + + context 'with non-default arguments' do + let(:args) do + { + description: 'some description', + active: false, + locked: true, + run_untagged: false, + tag_list: %w(tag1 tag2), + access_level: 'ref_protected', + maximum_timeout: 600, + name: 'some name', + version: 'some version', + revision: 'some revision', + platform: 'some platform', + architecture: 'some architecture', + ip_address: '10.0.0.1', + config: { + gpus: 'some gpu config' + } + } + end + + it 'creates runner with specified values', :aggregate_failures do + is_expected.to be_an_instance_of(::Ci::Runner) + expect(subject.active).to eq args[:active] + expect(subject.locked).to eq args[:locked] + expect(subject.run_untagged).to eq args[:run_untagged] + expect(subject.tags).to contain_exactly( + an_object_having_attributes(name: 'tag1'), + an_object_having_attributes(name: 'tag2') + ) + expect(subject.access_level).to eq args[:access_level] + expect(subject.maximum_timeout).to eq args[:maximum_timeout] + expect(subject.name).to eq args[:name] + expect(subject.version).to eq args[:version] + expect(subject.revision).to eq args[:revision] + expect(subject.platform).to eq args[:platform] + expect(subject.architecture).to eq args[:architecture] + expect(subject.ip_address).to eq args[:ip_address] + end + end + end + + context 'when project token is used' do + let(:project) { create(:project) } + let(:token) { project.runners_token } + + it 'creates project runner' do + is_expected.to be_an_instance_of(::Ci::Runner) + expect(project.runners.size).to eq(1) + is_expected.to eq(project.runners.first) + expect(subject.token).not_to eq(registration_token) + expect(subject.token).not_to eq(project.runners_token) + expect(subject).to be_project_type + end + + context 'when it exceeds the application limits' do + before do + create(:ci_runner, runner_type: :project_type, projects: [project], contacted_at: 1.second.ago) + create(:plan_limits, :default_plan, ci_registered_project_runners: 1) + end + + it 'does not create runner' do + is_expected.to be_an_instance_of(::Ci::Runner) + expect(subject.persisted?).to be_falsey + expect(subject.errors.messages).to eq('runner_projects.base': ['Maximum number of ci registered project runners (1) exceeded']) + expect(project.runners.reload.size).to eq(1) + end + end + + context 'when abandoned runners cause application limits to not be exceeded' do + before do + create(:ci_runner, runner_type: :project_type, projects: [project], created_at: 14.months.ago, contacted_at: 13.months.ago) + create(:plan_limits, :default_plan, ci_registered_project_runners: 1) + end + + it 'creates runner' do + is_expected.to be_an_instance_of(::Ci::Runner) + expect(subject.errors).to be_empty + expect(project.runners.reload.size).to eq(2) + expect(project.runners.recent.size).to eq(1) + end + end + + context 'when valid runner registrars do not include project' do + before do + stub_application_setting(valid_runner_registrars: ['group']) + end + + context 'when feature flag is enabled' do + before do + stub_feature_flags(runner_registration_control: true) + end + + it 'returns 403 error' do + is_expected.to be_nil + end + end + + context 'when feature flag is disabled' do + it 'registers the runner' do + is_expected.to be_an_instance_of(::Ci::Runner) + expect(subject.errors).to be_empty + expect(subject.active).to be true + end + end + end + end + + context 'when group token is used' do + let(:group) { create(:group) } + let(:token) { group.runners_token } + + it 'creates a group runner' do + is_expected.to be_an_instance_of(::Ci::Runner) + expect(subject.errors).to be_empty + expect(group.runners.reload.size).to eq(1) + expect(subject.token).not_to eq(registration_token) + expect(subject.token).not_to eq(group.runners_token) + expect(subject).to be_group_type + end + + context 'when it exceeds the application limits' do + before do + create(:ci_runner, runner_type: :group_type, groups: [group], contacted_at: nil, created_at: 1.month.ago) + create(:plan_limits, :default_plan, ci_registered_group_runners: 1) + end + + it 'does not create runner' do + is_expected.to be_an_instance_of(::Ci::Runner) + expect(subject.persisted?).to be_falsey + expect(subject.errors.messages).to eq('runner_namespaces.base': ['Maximum number of ci registered group runners (1) exceeded']) + expect(group.runners.reload.size).to eq(1) + end + end + + context 'when abandoned runners cause application limits to not be exceeded' do + before do + create(:ci_runner, runner_type: :group_type, groups: [group], created_at: 4.months.ago, contacted_at: 3.months.ago) + create(:ci_runner, runner_type: :group_type, groups: [group], contacted_at: nil, created_at: 4.months.ago) + create(:plan_limits, :default_plan, ci_registered_group_runners: 1) + end + + it 'creates runner' do + is_expected.to be_an_instance_of(::Ci::Runner) + expect(subject.errors).to be_empty + expect(group.runners.reload.size).to eq(3) + expect(group.runners.recent.size).to eq(1) + end + end + + context 'when valid runner registrars do not include group' do + before do + stub_application_setting(valid_runner_registrars: ['project']) + end + + context 'when feature flag is enabled' do + before do + stub_feature_flags(runner_registration_control: true) + end + + it 'returns nil' do + is_expected.to be_nil + end + end + + context 'when feature flag is disabled' do + it 'registers the runner' do + is_expected.to be_an_instance_of(::Ci::Runner) + expect(subject.errors).to be_empty + expect(subject.active).to be true + end + end + end + end + end + end +end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 5d56084faa8..4e8e41ca6e6 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -188,13 +188,6 @@ RSpec.describe Ci::RetryBuildService do expect(new_build).to be_pending end - it 'resolves todos for old build that failed' do - expect(::MergeRequests::AddTodoWhenBuildFailsService) - .to receive_message_chain(:new, :close) - - service.execute(build) - end - context 'when there are subsequent processables that are skipped' do let!(:subsequent_build) do create(:ci_build, :skipped, stage_idx: 2, @@ -272,6 +265,17 @@ RSpec.describe Ci::RetryBuildService do expect(bridge.reload).to be_pending end end + + context 'when there is a failed job todo for the MR' do + let!(:merge_request) { create(:merge_request, source_project: project, author: user, head_pipeline: pipeline) } + let!(:todo) { create(:todo, :build_failed, user: user, project: project, author: user, target: merge_request) } + + it 'resolves the todo for the old failed build' do + expect do + service.execute(build) + end.to change { todo.reload.state }.from('pending').to('done') + end + end end context 'when user does not have ability to execute build' do @@ -367,6 +371,14 @@ RSpec.describe Ci::RetryBuildService do it_behaves_like 'when build with dynamic environment is retried' context 'when create_deployment_in_separate_transaction feature flag is disabled' do + let(:new_build) do + travel_to(1.second.from_now) do + ::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/345668') do + service.clone!(build) + end + end + end + before do stub_feature_flags(create_deployment_in_separate_transaction: false) end |