diff options
Diffstat (limited to 'spec/services/ci')
11 files changed, 431 insertions, 153 deletions
diff --git a/spec/services/ci/create_pipeline_service/include_spec.rb b/spec/services/ci/create_pipeline_service/include_spec.rb index 5e7dace8e15..aa01977272a 100644 --- a/spec/services/ci/create_pipeline_service/include_spec.rb +++ b/spec/services/ci/create_pipeline_service/include_spec.rb @@ -7,9 +7,11 @@ RSpec.describe Ci::CreatePipelineService do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.owner } - let(:ref) { 'refs/heads/master' } - let(:source) { :push } - let(:service) { described_class.new(project, user, { ref: ref }) } + let(:ref) { 'refs/heads/master' } + let(:variables_attributes) { [{ key: 'MYVAR', secret_value: 'hello' }] } + let(:source) { :push } + + let(:service) { described_class.new(project, user, { ref: ref, variables_attributes: variables_attributes }) } let(:pipeline) { service.execute(source).payload } let(:file_location) { 'spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml' } @@ -24,6 +26,20 @@ RSpec.describe Ci::CreatePipelineService do .and_return(File.read(Rails.root.join(file_location))) end + shared_examples 'not including the file' do + it 'does not include the job in the file' do + expect(pipeline).to be_created_successfully + expect(pipeline.processables.pluck(:name)).to contain_exactly('job') + end + end + + shared_examples 'including the file' do + it 'includes the job in the file' do + expect(pipeline).to be_created_successfully + expect(pipeline.processables.pluck(:name)).to contain_exactly('job', 'rspec') + end + end + context 'with a local file' do let(:config) do <<~EOY @@ -33,13 +49,10 @@ RSpec.describe Ci::CreatePipelineService do EOY end - it 'includes the job in the file' do - expect(pipeline).to be_created_successfully - expect(pipeline.processables.pluck(:name)).to contain_exactly('job', 'rspec') - end + it_behaves_like 'including the file' end - context 'with a local file with rules' do + context 'with a local file with rules with a project variable' do let(:config) do <<~EOY include: @@ -54,19 +67,63 @@ RSpec.describe Ci::CreatePipelineService do context 'when the rules matches' do let(:project_id) { project.id } - it 'includes the job in the file' do - expect(pipeline).to be_created_successfully - expect(pipeline.processables.pluck(:name)).to contain_exactly('job', 'rspec') - end + it_behaves_like 'including the file' end context 'when the rules does not match' do let(:project_id) { non_existing_record_id } - it 'does not include the job in the file' do - expect(pipeline).to be_created_successfully - expect(pipeline.processables.pluck(:name)).to contain_exactly('job') - end + it_behaves_like 'not including the file' + end + end + + context 'with a local file with rules with a predefined pipeline variable' do + let(:config) do + <<~EOY + include: + - local: #{file_location} + rules: + - if: $CI_PIPELINE_SOURCE == "#{pipeline_source}" + job: + script: exit 0 + EOY + end + + context 'when the rules matches' do + let(:pipeline_source) { 'push' } + + it_behaves_like 'including the file' + end + + context 'when the rules does not match' do + let(:pipeline_source) { 'web' } + + it_behaves_like 'not including the file' + end + end + + context 'with a local file with rules with a run pipeline variable' do + let(:config) do + <<~EOY + include: + - local: #{file_location} + rules: + - if: $MYVAR == "#{my_var}" + job: + script: exit 0 + EOY + end + + context 'when the rules matches' do + let(:my_var) { 'hello' } + + it_behaves_like 'including the file' + end + + context 'when the rules does not match' do + let(:my_var) { 'mello' } + + it_behaves_like 'not including the file' end end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 78646665539..c78e19ea62d 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -5,8 +5,8 @@ require 'spec_helper' RSpec.describe Ci::CreatePipelineService do include ProjectForksHelper - let_it_be(:project, reload: true) { create(:project, :repository) } - let_it_be(:user, reload: true) { project.owner } + let_it_be_with_refind(:project) { create(:project, :repository) } + let_it_be_with_reload(:user) { project.owner } let(:ref_name) { 'refs/heads/master' } diff --git a/spec/services/ci/external_pull_requests/create_pipeline_service_spec.rb b/spec/services/ci/external_pull_requests/create_pipeline_service_spec.rb index 04d75630295..d5881d3b204 100644 --- a/spec/services/ci/external_pull_requests/create_pipeline_service_spec.rb +++ b/spec/services/ci/external_pull_requests/create_pipeline_service_spec.rb @@ -26,28 +26,6 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do pull_request.update!(source_branch: source_branch.name, source_sha: source_branch.target) end - context 'when the FF ci_create_external_pr_pipeline_async is disabled' do - before do - stub_feature_flags(ci_create_external_pr_pipeline_async: false) - end - - it 'creates a pipeline for external pull request', :aggregate_failures do - pipeline = execute.payload - - expect(execute).to be_success - expect(pipeline).to be_valid - expect(pipeline).to be_persisted - expect(pipeline).to be_external_pull_request_event - expect(pipeline).to eq(project.ci_pipelines.last) - expect(pipeline.external_pull_request).to eq(pull_request) - expect(pipeline.user).to eq(user) - expect(pipeline.status).to eq('created') - expect(pipeline.ref).to eq(pull_request.source_branch) - expect(pipeline.sha).to eq(pull_request.source_sha) - expect(pipeline.source_sha).to eq(pull_request.source_sha) - end - end - it 'enqueues Ci::ExternalPullRequests::CreatePipelineWorker' do expect { execute } .to change { ::Ci::ExternalPullRequests::CreatePipelineWorker.jobs.count } diff --git a/spec/services/ci/generate_kubeconfig_service_spec.rb b/spec/services/ci/generate_kubeconfig_service_spec.rb new file mode 100644 index 00000000000..b0673d16158 --- /dev/null +++ b/spec/services/ci/generate_kubeconfig_service_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::GenerateKubeconfigService do + describe '#execute' do + let(:project) { create(:project) } + let(:build) { create(:ci_build, project: project) } + let(:agent1) { create(:cluster_agent, project: project) } + let(:agent2) { create(:cluster_agent) } + + let(:template) { instance_double(Gitlab::Kubernetes::Kubeconfig::Template) } + + subject { described_class.new(build).execute } + + before do + expect(Gitlab::Kubernetes::Kubeconfig::Template).to receive(:new).and_return(template) + expect(build.pipeline).to receive(:authorized_cluster_agents).and_return([agent1, agent2]) + end + + it 'adds a cluster, and a user and context for each available agent' do + expect(template).to receive(:add_cluster).with( + name: 'gitlab', + url: Gitlab::Kas.tunnel_url + ).once + + expect(template).to receive(:add_user).with( + name: "agent:#{agent1.id}", + token: "ci:#{agent1.id}:#{build.token}" + ) + expect(template).to receive(:add_user).with( + name: "agent:#{agent2.id}", + token: "ci:#{agent2.id}:#{build.token}" + ) + + expect(template).to receive(:add_context).with( + name: "#{project.full_path}:#{agent1.name}", + cluster: 'gitlab', + user: "agent:#{agent1.id}" + ) + expect(template).to receive(:add_context).with( + name: "#{agent2.project.full_path}:#{agent2.name}", + cluster: 'gitlab', + user: "agent:#{agent2.id}" + ) + + expect(subject).to eq(template) + end + end +end diff --git a/spec/services/ci/job_artifacts/create_service_spec.rb b/spec/services/ci/job_artifacts/create_service_spec.rb index e6d9f208096..6ad3e9ceb54 100644 --- a/spec/services/ci/job_artifacts/create_service_spec.rb +++ b/spec/services/ci/job_artifacts/create_service_spec.rb @@ -49,6 +49,7 @@ RSpec.describe Ci::JobArtifacts::CreateService do expect(new_artifact.file_type).to eq(params['artifact_type']) expect(new_artifact.file_format).to eq(params['artifact_format']) expect(new_artifact.file_sha256).to eq(artifacts_sha256) + expect(new_artifact.locked).to eq(job.pipeline.locked) end it 'does not track the job user_id' do @@ -75,6 +76,7 @@ RSpec.describe Ci::JobArtifacts::CreateService do expect(new_artifact.file_type).to eq('metadata') expect(new_artifact.file_format).to eq('gzip') expect(new_artifact.file_sha256).to eq(artifacts_sha256) + expect(new_artifact.locked).to eq(job.pipeline.locked) end it 'sets expiration date according to application settings' do @@ -175,18 +177,6 @@ RSpec.describe Ci::JobArtifacts::CreateService do hash_including('key' => 'KEY1', 'value' => 'VAR1', 'source' => 'dotenv'), hash_including('key' => 'KEY2', 'value' => 'VAR2', 'source' => 'dotenv')) end - - context 'when ci_synchronous_artifact_parsing feature flag is disabled' do - before do - stub_feature_flags(ci_synchronous_artifact_parsing: false) - end - - it 'does not call parse service' do - expect(Ci::ParseDotenvArtifactService).not_to receive(:new) - - expect(subject[:status]).to eq(:success) - end - end end context 'when artifact_type is metrics' do 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 7a91ad9dcc1..6761f052e18 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 @@ -16,26 +16,43 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s let_it_be(:job) { create(:ci_build, :success, pipeline: pipeline) } context 'when artifact is expired' do - let!(:artifact) { create(:ci_job_artifact, :expired, job: job) } + let!(:artifact) { create(:ci_job_artifact, :expired, job: job, locked: job.pipeline.locked) } context 'with preloaded relationships' do before do stub_const("#{described_class}::LOOP_LIMIT", 1) end - it 'performs the smallest number of queries for job_artifacts' do - log = ActiveRecord::QueryRecorder.new { subject } + context 'with ci_destroy_unlocked_job_artifacts feature flag disabled' do + before do + stub_feature_flags(ci_destroy_unlocked_job_artifacts: false) + end + + it 'performs the smallest number of queries for job_artifacts' do + log = ActiveRecord::QueryRecorder.new { subject } + + # SELECT expired ci_job_artifacts - 3 queries from each_batch + # PRELOAD projects, routes, project_statistics + # BEGIN + # INSERT into ci_deleted_objects + # DELETE loaded ci_job_artifacts + # DELETE security_findings -- for EE + # COMMIT + # SELECT next expired ci_job_artifacts + + expect(log.count).to be_within(1).of(10) + end + end - # SELECT expired ci_job_artifacts - 3 queries from each_batch - # PRELOAD projects, routes, project_statistics - # BEGIN - # INSERT into ci_deleted_objects - # DELETE loaded ci_job_artifacts - # DELETE security_findings -- for EE - # COMMIT - # SELECT next expired ci_job_artifacts + context 'with ci_destroy_unlocked_job_artifacts feature flag enabled' do + before do + stub_feature_flags(ci_destroy_unlocked_job_artifacts: true) + end - expect(log.count).to be_within(1).of(10) + it 'performs the smallest number of queries for job_artifacts' do + log = ActiveRecord::QueryRecorder.new { subject } + expect(log.count).to be_within(1).of(8) + end end end @@ -53,7 +70,7 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s end context 'when the artifact has a file attached to it' do - let!(:artifact) { create(:ci_job_artifact, :expired, :zip, job: job) } + let!(:artifact) { create(:ci_job_artifact, :expired, :zip, job: job, locked: job.pipeline.locked) } it 'creates a deleted object' do expect { subject }.to change { Ci::DeletedObject.count }.by(1) @@ -74,7 +91,7 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s end context 'when artifact is locked' do - let!(:artifact) { create(:ci_job_artifact, :expired, job: locked_job) } + let!(:artifact) { create(:ci_job_artifact, :expired, job: locked_job, locked: locked_job.pipeline.locked) } it 'does not destroy job artifact' do expect { subject }.not_to change { Ci::JobArtifact.count } @@ -83,7 +100,7 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s end context 'when artifact is not expired' do - let!(:artifact) { create(:ci_job_artifact, job: job) } + let!(:artifact) { create(:ci_job_artifact, job: job, locked: job.pipeline.locked) } it 'does not destroy expired job artifacts' do expect { subject }.not_to change { Ci::JobArtifact.count } @@ -91,7 +108,7 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s end context 'when artifact is permanent' do - let!(:artifact) { create(:ci_job_artifact, expire_at: nil, job: job) } + let!(:artifact) { create(:ci_job_artifact, expire_at: nil, job: job, locked: job.pipeline.locked) } it 'does not destroy expired job artifacts' do expect { subject }.not_to change { Ci::JobArtifact.count } @@ -99,7 +116,7 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s end context 'when failed to destroy artifact' do - let!(:artifact) { create(:ci_job_artifact, :expired, job: job) } + let!(:artifact) { create(:ci_job_artifact, :expired, job: job, locked: job.pipeline.locked) } before do stub_const("#{described_class}::LOOP_LIMIT", 10) @@ -135,7 +152,7 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s end context 'when exclusive lease has already been taken by the other instance' do - let!(:artifact) { create(:ci_job_artifact, :expired, job: job) } + let!(:artifact) { create(:ci_job_artifact, :expired, job: job, locked: job.pipeline.locked) } before do stub_exclusive_lease_taken(described_class::EXCLUSIVE_LOCK_KEY, timeout: described_class::LOCK_TIMEOUT) @@ -149,8 +166,8 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s context 'with a second artifact and batch size of 1' do let(:second_job) { create(:ci_build, :success, pipeline: pipeline) } - let!(:second_artifact) { create(:ci_job_artifact, :archive, expire_at: 1.day.ago, job: second_job) } - let!(:artifact) { create(:ci_job_artifact, :expired, job: job) } + let!(:second_artifact) { create(:ci_job_artifact, :archive, expire_at: 1.day.ago, job: second_job, locked: job.pipeline.locked) } + let!(:artifact) { create(:ci_job_artifact, :expired, job: job, locked: job.pipeline.locked) } before do stub_const("#{described_class}::BATCH_SIZE", 1) @@ -206,8 +223,8 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s end context 'when some artifacts are locked' do - let!(:artifact) { create(:ci_job_artifact, :expired, job: job) } - let!(:locked_artifact) { create(:ci_job_artifact, :expired, job: locked_job) } + let!(:artifact) { create(:ci_job_artifact, :expired, job: job, locked: job.pipeline.locked) } + let!(:locked_artifact) { create(:ci_job_artifact, :expired, job: locked_job, locked: locked_job.pipeline.locked) } it 'destroys only unlocked artifacts' do expect { subject }.to change { Ci::JobArtifact.count }.by(-1) @@ -216,7 +233,7 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s end context 'when all artifacts are locked' do - let!(:artifact) { create(:ci_job_artifact, :expired, job: locked_job) } + let!(:artifact) { create(:ci_job_artifact, :expired, job: locked_job, locked: locked_job.pipeline.locked) } it 'destroys no artifacts' do expect { subject }.to change { Ci::JobArtifact.count }.by(0) diff --git a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb index 2cedbf93d74..1cc856734fc 100644 --- a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb @@ -29,7 +29,8 @@ RSpec.describe Ci::JobArtifacts::DestroyBatchService do it 'reports metrics for destroyed artifacts' do expect_next_instance_of(Gitlab::Ci::Artifacts::Metrics) do |metrics| - expect(metrics).to receive(:increment_destroyed_artifacts).with(1).and_call_original + expect(metrics).to receive(:increment_destroyed_artifacts_count).with(1).and_call_original + expect(metrics).to receive(:increment_destroyed_artifacts_bytes).with(107464).and_call_original end execute diff --git a/spec/services/ci/parse_dotenv_artifact_service_spec.rb b/spec/services/ci/parse_dotenv_artifact_service_spec.rb index 7536e04f2de..c4040a426f2 100644 --- a/spec/services/ci/parse_dotenv_artifact_service_spec.rb +++ b/spec/services/ci/parse_dotenv_artifact_service_spec.rb @@ -45,7 +45,7 @@ RSpec.describe Ci::ParseDotenvArtifactService do it 'returns error' do expect(subject[:status]).to eq(:error) - expect(subject[:message]).to eq("Dotenv Artifact Too Big. Maximum Allowable Size: #{described_class::MAX_ACCEPTABLE_DOTENV_SIZE}") + expect(subject[:message]).to eq("Dotenv Artifact Too Big. Maximum Allowable Size: #{service.send(:dotenv_size_limit)}") expect(subject[:http_status]).to eq(:bad_request) end end @@ -186,7 +186,7 @@ RSpec.describe Ci::ParseDotenvArtifactService do context 'when more than limitated variables are specified in dotenv' do let(:blob) do StringIO.new.tap do |s| - (described_class::MAX_ACCEPTABLE_VARIABLES_COUNT + 1).times do |i| + (service.send(:dotenv_variable_limit) + 1).times do |i| s << "KEY#{i}=VAR#{i}\n" end end.string @@ -194,7 +194,7 @@ RSpec.describe Ci::ParseDotenvArtifactService do it 'returns error' do expect(subject[:status]).to eq(:error) - expect(subject[:message]).to eq("Dotenv files cannot have more than #{described_class::MAX_ACCEPTABLE_VARIABLES_COUNT} variables") + expect(subject[:message]).to eq("Dotenv files cannot have more than #{service.send(:dotenv_variable_limit)} variables") expect(subject[:http_status]).to eq(:bad_request) end end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 15c88c9f657..16635c64434 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -323,6 +323,37 @@ RSpec.describe Ci::RetryBuildService do it 'persists expanded environment name' do expect(new_build.metadata.expanded_environment_name).to eq('production') end + + it 'does not create a new environment' do + expect { new_build }.not_to change { Environment.count } + end + end + + context 'when build with dynamic environment is retried' do + let_it_be(:other_developer) { create(:user).tap { |u| project.add_developer(other_developer) } } + + let(:environment_name) { 'review/$CI_COMMIT_REF_SLUG-$GITLAB_USER_ID' } + + let!(:build) do + create(:ci_build, :with_deployment, environment: environment_name, + options: { environment: { name: environment_name } }, + pipeline: pipeline, stage_id: stage.id, project: project, + user: other_developer) + end + + it 're-uses the previous persisted environment' do + expect(build.persisted_environment.name).to eq("review/#{build.ref}-#{other_developer.id}") + + expect(new_build.persisted_environment.name).to eq("review/#{build.ref}-#{other_developer.id}") + end + + it 'creates a new deployment' do + expect { new_build }.to change { Deployment.count }.by(1) + end + + it 'does not create a new environment' do + expect { new_build }.not_to change { Environment.count } + end end context 'when build has needs' do diff --git a/spec/services/ci/unlock_artifacts_service_spec.rb b/spec/services/ci/unlock_artifacts_service_spec.rb index 8d289a867ba..8ee07fc44c8 100644 --- a/spec/services/ci/unlock_artifacts_service_spec.rb +++ b/spec/services/ci/unlock_artifacts_service_spec.rb @@ -3,93 +3,247 @@ require 'spec_helper' RSpec.describe Ci::UnlockArtifactsService do - describe '#execute' do - subject(:execute) { described_class.new(pipeline.project, pipeline.user).execute(ci_ref, before_pipeline) } + using RSpec::Parameterized::TableSyntax + + where(:tag, :ci_update_unlocked_job_artifacts) do + false | false + false | true + true | false + true | true + end + + with_them do + let(:ref) { 'master' } + let(:ref_path) { tag ? "#{::Gitlab::Git::TAG_REF_PREFIX}#{ref}" : "#{::Gitlab::Git::BRANCH_REF_PREFIX}#{ref}" } + let(:ci_ref) { create(:ci_ref, ref_path: ref_path) } + let(:project) { ci_ref.project } + let(:source_job) { create(:ci_build, pipeline: pipeline) } + + let!(:old_unlocked_pipeline) { create(:ci_pipeline, :with_persisted_artifacts, ref: ref, tag: tag, project: project, locked: :unlocked) } + let!(:older_pipeline) { create(:ci_pipeline, :with_persisted_artifacts, ref: ref, tag: tag, project: project, locked: :artifacts_locked) } + let!(:older_ambiguous_pipeline) { create(:ci_pipeline, :with_persisted_artifacts, ref: ref, tag: !tag, project: project, locked: :artifacts_locked) } + let!(:pipeline) { create(:ci_pipeline, :with_persisted_artifacts, ref: ref, tag: tag, project: project, locked: :artifacts_locked) } + let!(:child_pipeline) { create(:ci_pipeline, :with_persisted_artifacts, ref: ref, tag: tag, project: project, locked: :artifacts_locked) } + let!(:newer_pipeline) { create(:ci_pipeline, :with_persisted_artifacts, ref: ref, tag: tag, project: project, locked: :artifacts_locked) } + let!(:other_ref_pipeline) { create(:ci_pipeline, :with_persisted_artifacts, ref: 'other_ref', tag: tag, project: project, locked: :artifacts_locked) } + let!(:sources_pipeline) { create(:ci_sources_pipeline, source_job: source_job, source_project: project, pipeline: child_pipeline, project: project) } before do stub_const("#{described_class}::BATCH_SIZE", 1) + stub_feature_flags(ci_update_unlocked_job_artifacts: ci_update_unlocked_job_artifacts) end - [true, false].each do |tag| - context "when tag is #{tag}" do - let(:ref) { 'master' } - let(:ref_path) { tag ? "#{::Gitlab::Git::TAG_REF_PREFIX}#{ref}" : "#{::Gitlab::Git::BRANCH_REF_PREFIX}#{ref}" } - let(:ci_ref) { create(:ci_ref, ref_path: ref_path) } + describe '#execute' do + subject(:execute) { described_class.new(pipeline.project, pipeline.user).execute(ci_ref, before_pipeline) } + + context 'when running on a ref before a pipeline' do + let(:before_pipeline) { pipeline } + + it 'unlocks artifacts from older pipelines' do + expect { execute }.to change { older_pipeline.reload.locked }.from('artifacts_locked').to('unlocked') + end + + it 'does not unlock artifacts for tag or branch with same name as ref' do + expect { execute }.not_to change { older_ambiguous_pipeline.reload.locked }.from('artifacts_locked') + end + + it 'does not unlock artifacts from newer pipelines' do + expect { execute }.not_to change { newer_pipeline.reload.locked }.from('artifacts_locked') + end + + it 'does not lock artifacts from old unlocked pipelines' do + expect { execute }.not_to change { old_unlocked_pipeline.reload.locked }.from('unlocked') + end + + it 'does not unlock artifacts from the same pipeline' do + expect { execute }.not_to change { pipeline.reload.locked }.from('artifacts_locked') + end - let!(:old_unlocked_pipeline) { create(:ci_pipeline, ref: ref, tag: tag, project: ci_ref.project, locked: :unlocked) } - let!(:older_pipeline) { create(:ci_pipeline, ref: ref, tag: tag, project: ci_ref.project, locked: :artifacts_locked) } - let!(:older_ambiguous_pipeline) { create(:ci_pipeline, ref: ref, tag: !tag, project: ci_ref.project, locked: :artifacts_locked) } - let!(:pipeline) { create(:ci_pipeline, ref: ref, tag: tag, project: ci_ref.project, locked: :artifacts_locked) } - let!(:child_pipeline) { create(:ci_pipeline, ref: ref, tag: tag, project: ci_ref.project, locked: :artifacts_locked) } - let!(:newer_pipeline) { create(:ci_pipeline, ref: ref, tag: tag, project: ci_ref.project, locked: :artifacts_locked) } - let!(:other_ref_pipeline) { create(:ci_pipeline, ref: 'other_ref', tag: tag, project: ci_ref.project, locked: :artifacts_locked) } + it 'does not unlock artifacts for other refs' do + expect { execute }.not_to change { other_ref_pipeline.reload.locked }.from('artifacts_locked') + end - before do - create(:ci_sources_pipeline, - source_job: create(:ci_build, pipeline: pipeline), - source_project: ci_ref.project, - pipeline: child_pipeline, - project: ci_ref.project) + it 'does not unlock artifacts for child pipeline' do + expect { execute }.not_to change { child_pipeline.reload.locked }.from('artifacts_locked') end - context 'when running on a ref before a pipeline' do - let(:before_pipeline) { pipeline } + it 'unlocks job artifact records' do + pending unless ci_update_unlocked_job_artifacts - it 'unlocks artifacts from older pipelines' do - expect { execute }.to change { older_pipeline.reload.locked }.from('artifacts_locked').to('unlocked') - end + expect { execute }.to change { ::Ci::JobArtifact.artifact_unlocked.count }.from(0).to(2) + end + end - it 'does not unlock artifacts for tag or branch with same name as ref' do - expect { execute }.not_to change { older_ambiguous_pipeline.reload.locked }.from('artifacts_locked') - end + context 'when running on just the ref' do + let(:before_pipeline) { nil } - it 'does not unlock artifacts from newer pipelines' do - expect { execute }.not_to change { newer_pipeline.reload.locked }.from('artifacts_locked') - end + it 'unlocks artifacts from older pipelines' do + expect { execute }.to change { older_pipeline.reload.locked }.from('artifacts_locked').to('unlocked') + end - it 'does not lock artifacts from old unlocked pipelines' do - expect { execute }.not_to change { old_unlocked_pipeline.reload.locked }.from('unlocked') - end + it 'unlocks artifacts from newer pipelines' do + expect { execute }.to change { newer_pipeline.reload.locked }.from('artifacts_locked').to('unlocked') + end - it 'does not unlock artifacts from the same pipeline' do - expect { execute }.not_to change { pipeline.reload.locked }.from('artifacts_locked') - end + it 'unlocks artifacts from the same pipeline' do + expect { execute }.to change { pipeline.reload.locked }.from('artifacts_locked').to('unlocked') + end - it 'does not unlock artifacts for other refs' do - expect { execute }.not_to change { other_ref_pipeline.reload.locked }.from('artifacts_locked') - end + it 'does not unlock artifacts for tag or branch with same name as ref' do + expect { execute }.not_to change { older_ambiguous_pipeline.reload.locked }.from('artifacts_locked') + end - it 'does not unlock artifacts for child pipeline' do - expect { execute }.not_to change { child_pipeline.reload.locked }.from('artifacts_locked') - end + it 'does not lock artifacts from old unlocked pipelines' do + expect { execute }.not_to change { old_unlocked_pipeline.reload.locked }.from('unlocked') end - context 'when running on just the ref' do - let(:before_pipeline) { nil } + it 'does not unlock artifacts for other refs' do + expect { execute }.not_to change { other_ref_pipeline.reload.locked }.from('artifacts_locked') + end - it 'unlocks artifacts from older pipelines' do - expect { execute }.to change { older_pipeline.reload.locked }.from('artifacts_locked').to('unlocked') - end + it 'unlocks job artifact records' do + pending unless ci_update_unlocked_job_artifacts - it 'unlocks artifacts from newer pipelines' do - expect { execute }.to change { newer_pipeline.reload.locked }.from('artifacts_locked').to('unlocked') - end + expect { execute }.to change { ::Ci::JobArtifact.artifact_unlocked.count }.from(0).to(8) + end + end + end - it 'unlocks artifacts from the same pipeline' do - expect { execute }.to change { pipeline.reload.locked }.from('artifacts_locked').to('unlocked') - end + describe '#unlock_pipelines_query' do + subject { described_class.new(pipeline.project, pipeline.user).unlock_pipelines_query(ci_ref, before_pipeline) } + + context 'when running on a ref before a pipeline' do + let(:before_pipeline) { pipeline } + + it 'produces the expected SQL string' do + expect(subject.squish).to eq <<~SQL.squish + UPDATE + "ci_pipelines" + SET + "locked" = 0 + WHERE + "ci_pipelines"."id" IN + (SELECT + "ci_pipelines"."id" + FROM + "ci_pipelines" + WHERE + "ci_pipelines"."ci_ref_id" = #{ci_ref.id} + AND "ci_pipelines"."locked" = 1 + AND (ci_pipelines.id < #{before_pipeline.id}) + AND "ci_pipelines"."id" NOT IN + (WITH RECURSIVE + "base_and_descendants" + AS + ((SELECT + "ci_pipelines".* + FROM + "ci_pipelines" + WHERE + "ci_pipelines"."id" = #{before_pipeline.id}) + UNION + (SELECT + "ci_pipelines".* + FROM + "ci_pipelines", + "base_and_descendants", + "ci_sources_pipelines" + WHERE + "ci_sources_pipelines"."pipeline_id" = "ci_pipelines"."id" + AND "ci_sources_pipelines"."source_pipeline_id" = "base_and_descendants"."id" + AND "ci_sources_pipelines"."source_project_id" = "ci_sources_pipelines"."project_id")) + SELECT + "id" + FROM + "base_and_descendants" + AS + "ci_pipelines") + LIMIT 1 + FOR UPDATE + SKIP LOCKED) + RETURNING ("ci_pipelines"."id") + SQL + end + end - it 'does not unlock artifacts for tag or branch with same name as ref' do - expect { execute }.not_to change { older_ambiguous_pipeline.reload.locked }.from('artifacts_locked') - end + context 'when running on just the ref' do + let(:before_pipeline) { nil } + + it 'produces the expected SQL string' do + expect(subject.squish).to eq <<~SQL.squish + UPDATE + "ci_pipelines" + SET + "locked" = 0 + WHERE + "ci_pipelines"."id" IN + (SELECT + "ci_pipelines"."id" + FROM + "ci_pipelines" + WHERE + "ci_pipelines"."ci_ref_id" = #{ci_ref.id} + AND "ci_pipelines"."locked" = 1 + LIMIT 1 + FOR UPDATE + SKIP LOCKED) + RETURNING + ("ci_pipelines"."id") + SQL + end + end + end - it 'does not lock artifacts from old unlocked pipelines' do - expect { execute }.not_to change { old_unlocked_pipeline.reload.locked }.from('unlocked') - end + describe '#unlock_job_artifacts_query' do + subject { described_class.new(pipeline.project, pipeline.user).unlock_job_artifacts_query(pipeline_ids) } + + context 'when running on a ref before a pipeline' do + let(:before_pipeline) { pipeline } + let(:pipeline_ids) { [older_pipeline.id] } + + it 'produces the expected SQL string' do + expect(subject.squish).to eq <<~SQL.squish + UPDATE + "ci_job_artifacts" + SET + "locked" = 0 + WHERE + "ci_job_artifacts"."job_id" IN + (SELECT + "ci_builds"."id" + FROM + "ci_builds" + WHERE + "ci_builds"."type" = 'Ci::Build' + AND "ci_builds"."commit_id" = #{older_pipeline.id}) + RETURNING + ("ci_job_artifacts"."id") + SQL + end + end - it 'does not unlock artifacts for other refs' do - expect { execute }.not_to change { other_ref_pipeline.reload.locked }.from('artifacts_locked') - end + context 'when running on just the ref' do + let(:before_pipeline) { nil } + let(:pipeline_ids) { [older_pipeline.id, newer_pipeline.id, pipeline.id] } + + it 'produces the expected SQL string' do + expect(subject.squish).to eq <<~SQL.squish + UPDATE + "ci_job_artifacts" + SET + "locked" = 0 + WHERE + "ci_job_artifacts"."job_id" IN + (SELECT + "ci_builds"."id" + FROM + "ci_builds" + WHERE + "ci_builds"."type" = 'Ci::Build' + AND "ci_builds"."commit_id" IN (#{pipeline_ids.join(', ')})) + RETURNING + ("ci_job_artifacts"."id") + SQL end end end diff --git a/spec/services/ci/update_build_state_service_spec.rb b/spec/services/ci/update_build_state_service_spec.rb index e4dd3d0500f..937b19beff5 100644 --- a/spec/services/ci/update_build_state_service_spec.rb +++ b/spec/services/ci/update_build_state_service_spec.rb @@ -118,7 +118,7 @@ RSpec.describe Ci::UpdateBuildStateService do expect(metrics) .not_to have_received(:increment_error_counter) - .with(type: :chunks_invalid_checksum) + .with(error_reason: :chunks_invalid_checksum) end end @@ -188,7 +188,7 @@ RSpec.describe Ci::UpdateBuildStateService do expect(metrics) .to have_received(:increment_error_counter) - .with(type: :chunks_invalid_checksum) + .with(error_reason: :chunks_invalid_checksum) end end @@ -210,11 +210,11 @@ RSpec.describe Ci::UpdateBuildStateService do expect(metrics) .not_to have_received(:increment_error_counter) - .with(type: :chunks_invalid_checksum) + .with(error_reason: :chunks_invalid_checksum) expect(metrics) .not_to have_received(:increment_error_counter) - .with(type: :chunks_invalid_size) + .with(error_reason: :chunks_invalid_size) end context 'when using deprecated parameters' do @@ -235,11 +235,11 @@ RSpec.describe Ci::UpdateBuildStateService do expect(metrics) .not_to have_received(:increment_error_counter) - .with(type: :chunks_invalid_checksum) + .with(error_reason: :chunks_invalid_checksum) expect(metrics) .not_to have_received(:increment_error_counter) - .with(type: :chunks_invalid_size) + .with(error_reason: :chunks_invalid_size) end end end @@ -262,11 +262,11 @@ RSpec.describe Ci::UpdateBuildStateService do expect(metrics) .to have_received(:increment_error_counter) - .with(type: :chunks_invalid_checksum) + .with(error_reason: :chunks_invalid_checksum) expect(metrics) .to have_received(:increment_error_counter) - .with(type: :chunks_invalid_size) + .with(error_reason: :chunks_invalid_size) end end @@ -284,7 +284,7 @@ RSpec.describe Ci::UpdateBuildStateService do expect(metrics) .to have_received(:increment_error_counter) - .with(type: :chunks_invalid_checksum) + .with(error_reason: :chunks_invalid_checksum) expect(metrics) .not_to have_received(:increment_trace_operation) @@ -292,7 +292,7 @@ RSpec.describe Ci::UpdateBuildStateService do expect(metrics) .not_to have_received(:increment_error_counter) - .with(type: :chunks_invalid_size) + .with(error_reason: :chunks_invalid_size) end end @@ -376,7 +376,7 @@ RSpec.describe Ci::UpdateBuildStateService do expect(metrics) .not_to have_received(:increment_error_counter) - .with(type: :chunks_invalid_checksum) + .with(error_reason: :chunks_invalid_checksum) end context 'when build pending state is outdated' do |