diff options
Diffstat (limited to 'spec/services/ci')
21 files changed, 959 insertions, 416 deletions
diff --git a/spec/services/ci/after_requeue_job_service_spec.rb b/spec/services/ci/after_requeue_job_service_spec.rb index 1f692bdb71a..e6f46fb9ebe 100644 --- a/spec/services/ci/after_requeue_job_service_spec.rb +++ b/spec/services/ci/after_requeue_job_service_spec.rb @@ -120,26 +120,6 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do ) end - context 'when the FF ci_requeue_with_dag_object_hierarchy is disabled' do - before do - stub_feature_flags(ci_requeue_with_dag_object_hierarchy: false) - end - - it 'marks subsequent skipped jobs as processable but leaves a3 created' do - execute_after_requeue_service(a1) - - check_jobs_statuses( - a1: 'pending', - a2: 'created', - a3: 'skipped', - b1: 'success', - b2: 'created', - c1: 'created', - c2: 'created' - ) - end - end - context 'when executed by a different user than the original owner' do let(:retryer) { create(:user).tap { |u| project.add_maintainer(u) } } let(:service) { described_class.new(project, retryer) } @@ -312,22 +292,6 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do c: 'created' ) end - - context 'when the FF ci_requeue_with_dag_object_hierarchy is disabled' do - before do - stub_feature_flags(ci_requeue_with_dag_object_hierarchy: false) - end - - it 'marks the next subsequent skipped job as processable but leaves c skipped' do - execute_after_requeue_service(a) - - check_jobs_statuses( - a: 'pending', - b: 'created', - c: 'skipped' - ) - end - end end private diff --git a/spec/services/ci/compare_test_reports_service_spec.rb b/spec/services/ci/compare_test_reports_service_spec.rb index 6d3df0f5383..f259072fe87 100644 --- a/spec/services/ci/compare_test_reports_service_spec.rb +++ b/spec/services/ci/compare_test_reports_service_spec.rb @@ -41,7 +41,7 @@ RSpec.describe Ci::CompareTestReportsService do it 'returns a parsed TestReports success status and failure on the individual suite' do expect(comparison[:status]).to eq(:parsed) expect(comparison.dig(:data, 'status')).to eq('success') - expect(comparison.dig(:data, 'suites', 0, 'status') ).to eq('error') + expect(comparison.dig(:data, 'suites', 0, 'status')).to eq('error') end end diff --git a/spec/services/ci/create_pipeline_service/partitioning_spec.rb b/spec/services/ci/create_pipeline_service/partitioning_spec.rb index 43fbb74ede4..f34d103d965 100644 --- a/spec/services/ci/create_pipeline_service/partitioning_spec.rb +++ b/spec/services/ci/create_pipeline_service/partitioning_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness, :aggregate_failures do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness, :aggregate_failures, +:ci_partitionable do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } @@ -31,7 +32,7 @@ RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectnes end let(:pipeline) { service.execute(:push).payload } - let(:current_partition_id) { 123 } + let(:current_partition_id) { ci_testing_partition_id } before do stub_ci_pipeline_yaml_file(config) diff --git a/spec/services/ci/create_pipeline_service/rules_spec.rb b/spec/services/ci/create_pipeline_service/rules_spec.rb index c737b8cc329..5fdefb2b306 100644 --- a/spec/services/ci/create_pipeline_service/rules_spec.rb +++ b/spec/services/ci/create_pipeline_service/rules_spec.rb @@ -1425,5 +1425,43 @@ RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectnes it_behaves_like 'comparing file changes with workflow rules' end end + + context 'workflow name with rules' do + let(:ref) { 'refs/heads/feature' } + + let(:variables) do + [{ key: 'SOME_VARIABLE', secret_value: 'SOME_VAL' }] + end + + let(:pipeline) do + execute_service do |pipeline| + pipeline.variables.build(variables) + end.payload + end + + let(:config) do + <<-EOY + workflow: + name: '$PIPELINE_NAME $SOME_VARIABLE' + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + PIPELINE_NAME: 'Name 1' + - if: $CI_COMMIT_REF_NAME =~ /feature/ + variables: + PIPELINE_NAME: 'Name 2' + + job: + stage: test + script: echo 'hello' + EOY + end + + it 'substitutes variables in pipeline name' do + expect(response).not_to be_error + expect(pipeline).to be_persisted + expect(pipeline.name).to eq('Name 2 SOME_VAL') + end + end end end diff --git a/spec/services/ci/create_pipeline_service/variables_spec.rb b/spec/services/ci/create_pipeline_service/variables_spec.rb new file mode 100644 index 00000000000..e9e0cf2c6e0 --- /dev/null +++ b/spec/services/ci/create_pipeline_service/variables_spec.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { project.first_owner } + + let(:service) { described_class.new(project, user, { ref: 'master' }) } + let(:pipeline) { service.execute(:push).payload } + + before do + stub_ci_pipeline_yaml_file(config) + end + + context 'when using variables' do + context 'when variables have expand: true/false' do + let(:config) do + <<-YAML + variables: + VAR7: + value: "value 7 $CI_PIPELINE_ID" + expand: false + VAR8: + value: "value 8 $CI_PIPELINE_ID" + expand: false + + rspec: + script: rspec + variables: + VAR1: "JOBID-$CI_JOB_ID" + VAR2: "PIPELINEID-$CI_PIPELINE_ID and $VAR1" + VAR3: + value: "PIPELINEID-$CI_PIPELINE_ID and $VAR1" + expand: false + VAR4: + value: "JOBID-$CI_JOB_ID" + expand: false + VAR5: "PIPELINEID-$CI_PIPELINE_ID and $VAR4" + VAR6: + value: "PIPELINEID-$CI_PIPELINE_ID and $VAR4" + expand: false + VAR7: "overridden value 7 $CI_PIPELINE_ID" + YAML + end + + let(:rspec) { find_job('rspec') } + + it 'creates the pipeline with a job that has variable expanded according to "expand"' do + expect(pipeline).to be_created_successfully + + expect(Ci::BuildRunnerPresenter.new(rspec).runner_variables).to include( + { key: 'VAR1', value: "JOBID-#{rspec.id}", public: true, masked: false }, + { key: 'VAR2', value: "PIPELINEID-#{pipeline.id} and JOBID-#{rspec.id}", public: true, masked: false }, + { key: 'VAR3', value: "PIPELINEID-$CI_PIPELINE_ID and $VAR1", public: true, masked: false, raw: true }, + { key: 'VAR4', value: "JOBID-$CI_JOB_ID", public: true, masked: false, raw: true }, + { key: 'VAR5', value: "PIPELINEID-#{pipeline.id} and $VAR4", public: true, masked: false }, + { key: 'VAR6', value: "PIPELINEID-$CI_PIPELINE_ID and $VAR4", public: true, masked: false, raw: true }, + { key: 'VAR7', value: "overridden value 7 #{pipeline.id}", public: true, masked: false }, + { key: 'VAR8', value: "value 8 $CI_PIPELINE_ID", public: true, masked: false, raw: true } + ) + end + + context 'when the FF ci_raw_variables_in_yaml_config is disabled' do + before do + stub_feature_flags(ci_raw_variables_in_yaml_config: false) + end + + it 'creates the pipeline with a job that has all variables expanded' do + expect(pipeline).to be_created_successfully + + expect(Ci::BuildRunnerPresenter.new(rspec).runner_variables).to include( + { key: 'VAR1', value: "JOBID-#{rspec.id}", public: true, masked: false }, + { key: 'VAR2', value: "PIPELINEID-#{pipeline.id} and JOBID-#{rspec.id}", public: true, masked: false }, + { key: 'VAR3', value: "PIPELINEID-#{pipeline.id} and JOBID-#{rspec.id}", public: true, masked: false }, + { key: 'VAR4', value: "JOBID-#{rspec.id}", public: true, masked: false }, + { key: 'VAR5', value: "PIPELINEID-#{pipeline.id} and JOBID-#{rspec.id}", public: true, masked: false }, + { key: 'VAR6', value: "PIPELINEID-#{pipeline.id} and JOBID-#{rspec.id}", public: true, masked: false }, + { key: 'VAR7', value: "overridden value 7 #{pipeline.id}", public: true, masked: false }, + { key: 'VAR8', value: "value 8 #{pipeline.id}", public: true, masked: false } + ) + end + end + end + + context 'when trigger variables have expand: true/false' do + let(:config) do + <<-YAML + child: + variables: + VAR1: "PROJECTID-$CI_PROJECT_ID" + VAR2: "PIPELINEID-$CI_PIPELINE_ID and $VAR1" + VAR3: + value: "PIPELINEID-$CI_PIPELINE_ID and $VAR1" + expand: false + trigger: + include: child.yml + YAML + end + + let(:child) { find_job('child') } + + it 'creates the pipeline with a trigger job that has downstream_variables expanded according to "expand"' do + expect(pipeline).to be_created_successfully + + expect(child.downstream_variables).to include( + { key: 'VAR1', value: "PROJECTID-#{project.id}" }, + { key: 'VAR2', value: "PIPELINEID-#{pipeline.id} and PROJECTID-$CI_PROJECT_ID" }, + { key: 'VAR3', value: "PIPELINEID-$CI_PIPELINE_ID and $VAR1", raw: true } + ) + end + + context 'when the FF ci_raw_variables_in_yaml_config is disabled' do + before do + stub_feature_flags(ci_raw_variables_in_yaml_config: false) + end + + it 'creates the pipeline with a job that has all variables expanded' do + expect(pipeline).to be_created_successfully + + expect(child.downstream_variables).to include( + { key: 'VAR1', value: "PROJECTID-#{project.id}" }, + { key: 'VAR2', value: "PIPELINEID-#{pipeline.id} and PROJECTID-$CI_PROJECT_ID" }, + { key: 'VAR3', value: "PIPELINEID-#{pipeline.id} and PROJECTID-$CI_PROJECT_ID" } + ) + end + end + end + end + + private + + def find_job(name) + pipeline.processables.find { |job| job.name == name } + end +end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 458692ba1c0..67c13649c6f 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -135,7 +135,7 @@ RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectnes execute_service expect(histogram).to have_received(:observe) - .with({ source: 'push' }, 5) + .with({ source: 'push', plan: project.actual_plan_name }, 5) end it 'tracks included template usage' do @@ -1867,49 +1867,4 @@ RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectnes end end end - - describe '#execute!' do - subject { service.execute!(*args) } - - let(:service) { described_class.new(project, user, ref: ref_name) } - let(:args) { [:push] } - - context 'when user has a permission to create a pipeline' do - let(:user) { create(:user) } - - before do - project.add_developer(user) - end - - it 'does not raise an error' do - expect { subject }.not_to raise_error - end - - it 'creates a pipeline' do - expect { subject }.to change { Ci::Pipeline.count }.by(1) - end - end - - context 'when user does not have a permission to create a pipeline' do - let(:user) { create(:user) } - - it 'raises an error' do - expect { subject } - .to raise_error(described_class::CreateError) - .with_message('Insufficient permissions to create a new pipeline') - end - end - - context 'when a user with permissions has been blocked' do - before do - user.block! - end - - it 'raises an error' do - expect { subject } - .to raise_error(described_class::CreateError) - .with_message('Insufficient permissions to create a new pipeline') - end - 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 030ba84951e..5df590a1b78 100644 --- a/spec/services/ci/job_artifacts/create_service_spec.rb +++ b/spec/services/ci/job_artifacts/create_service_spec.rb @@ -181,8 +181,8 @@ RSpec.describe Ci::JobArtifacts::CreateService do end end - context 'with job partitioning' do - let(:pipeline) { create(:ci_pipeline, project: project, partition_id: 123) } + context 'with job partitioning', :ci_partitionable do + let(:pipeline) { create(:ci_pipeline, project: project, partition_id: ci_testing_partition_id) } let(:job) { create(:ci_build, pipeline: pipeline) } it 'sets partition_id on artifacts' do @@ -190,7 +190,7 @@ RSpec.describe Ci::JobArtifacts::CreateService do artifacts_partitions = job.job_artifacts.map(&:partition_id).uniq - expect(artifacts_partitions).to eq([123]) + expect(artifacts_partitions).to eq([ci_testing_partition_id]) end end 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 54d1cacc068..79920dcb2c7 100644 --- a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb @@ -60,10 +60,9 @@ RSpec.describe Ci::JobArtifacts::DestroyBatchService do execute end - it 'preserves trace artifacts and removes any timestamp' do + it 'preserves trace artifacts' do expect { subject } - .to change { trace_artifact.reload.expire_at }.from(trace_artifact.expire_at).to(nil) - .and not_change { Ci::JobArtifact.exists?(trace_artifact.id) } + .to not_change { Ci::JobArtifact.exists?(trace_artifact.id) } end context 'when artifact belongs to a project that is undergoing stats refresh' do @@ -277,81 +276,5 @@ RSpec.describe Ci::JobArtifacts::DestroyBatchService do is_expected.to eq(destroyed_artifacts_count: 0, statistics_updates: {}, status: :success) end end - - context 'with artifacts that has backfilled expire_at' do - let!(:created_on_00_30_45_minutes_on_21_22_23) do - [ - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-21 00:00:00.000')), - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-21 01:30:00.000')), - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-22 12:00:00.000')), - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-22 12:30:00.000')), - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-23 23:00:00.000')), - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-23 23:30:00.000')), - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-23 06:45:00.000')) - ] - end - - let!(:created_close_to_00_or_30_minutes) do - [ - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-21 00:00:00.001')), - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-21 00:30:00.999')) - ] - end - - let!(:created_on_00_or_30_minutes_on_other_dates) do - [ - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-01 00:00:00.000')), - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-19 12:00:00.000')), - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-24 23:30:00.000')) - ] - end - - let!(:created_at_other_times) do - [ - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-19 00:00:00.000')), - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-19 00:30:00.000')), - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-24 00:00:00.000')), - create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-24 00:30:00.000')) - ] - end - - let(:artifacts_to_keep) { created_on_00_30_45_minutes_on_21_22_23 } - let(:artifacts_to_delete) { created_close_to_00_or_30_minutes + created_on_00_or_30_minutes_on_other_dates + created_at_other_times } - let(:all_artifacts) { artifacts_to_keep + artifacts_to_delete } - - let(:artifacts) { Ci::JobArtifact.where(id: all_artifacts.map(&:id)) } - - it 'deletes job artifacts that do not have expire_at on 00, 30 or 45 minute of 21, 22, 23 of the month' do - expect { subject }.to change { Ci::JobArtifact.count }.by(artifacts_to_delete.size * -1) - end - - it 'keeps job artifacts that have expire_at on 00, 30 or 45 minute of 21, 22, 23 of the month' do - expect { subject }.not_to change { Ci::JobArtifact.where(id: artifacts_to_keep.map(&:id)).count } - end - - it 'removes expire_at on job artifacts that have expire_at on 00, 30 or 45 minute of 21, 22, 23 of the month' do - subject - - expect(artifacts_to_keep.all? { |artifact| artifact.reload.expire_at.nil? }).to be(true) - end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(ci_detect_wrongly_expired_artifacts: false) - end - - it 'deletes all job artifacts' do - expect { subject }.to change { Ci::JobArtifact.count }.by(all_artifacts.size * -1) - end - end - - context 'when fix_expire_at is false' do - let(:service) { described_class.new(artifacts, pick_up_at: Time.current, fix_expire_at: false) } - - it 'deletes all job artifacts' do - expect { subject }.to change { Ci::JobArtifact.count }.by(all_artifacts.size * -1) - end - end - end end end diff --git a/spec/services/ci/job_artifacts/track_artifact_report_service_spec.rb b/spec/services/ci/job_artifacts/track_artifact_report_service_spec.rb index 6d9fc4c8e34..d4d56825e1f 100644 --- a/spec/services/ci/job_artifacts/track_artifact_report_service_spec.rb +++ b/spec/services/ci/job_artifacts/track_artifact_report_service_spec.rb @@ -9,7 +9,9 @@ RSpec.describe Ci::JobArtifacts::TrackArtifactReportService do let_it_be(:user1) { create(:user) } let_it_be(:user2) { create(:user) } - let(:test_event_name) { 'i_testing_test_report_uploaded' } + let(:test_event_name_1) { 'i_testing_test_report_uploaded' } + let(:test_event_name_2) { 'i_testing_coverage_report_uploaded' } + let(:counter) { Gitlab::UsageDataCounters::HLLRedisCounter } let(:start_time) { 1.week.ago } let(:end_time) { 1.week.from_now } @@ -25,15 +27,15 @@ RSpec.describe Ci::JobArtifacts::TrackArtifactReportService do end end - it 'tracks the event using HLLRedisCounter' do - allow(Gitlab::UsageDataCounters::HLLRedisCounter) + it 'tracks the test event using HLLRedisCounter' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter) .to receive(:track_event) - .with(test_event_name, values: user1.id) + .with(test_event_name_1, values: user1.id) .and_call_original expect { track_artifact_report } .to change { - counter.unique_events(event_names: test_event_name, + counter.unique_events(event_names: test_event_name_1, start_date: start_time, end_date: end_time) } @@ -44,12 +46,20 @@ RSpec.describe Ci::JobArtifacts::TrackArtifactReportService do context 'when pipeline does not have test reports' do let_it_be(:pipeline) { create(:ci_empty_pipeline) } - it 'does not track the event' do + it 'does not track the test event' do track_artifact_report expect(Gitlab::UsageDataCounters::HLLRedisCounter) .not_to receive(:track_event) - .with(anything, test_event_name) + .with(anything, test_event_name_1) + end + + it 'does not track the coverage test event' do + track_artifact_report + + expect(Gitlab::UsageDataCounters::HLLRedisCounter) + .not_to receive(:track_event) + .with(anything, test_event_name_2) end end @@ -57,15 +67,15 @@ RSpec.describe Ci::JobArtifacts::TrackArtifactReportService do let_it_be(:pipeline1) { create(:ci_pipeline, :with_test_reports, project: project, user: user1) } let_it_be(:pipeline2) { create(:ci_pipeline, :with_test_reports, project: project, user: user1) } - it 'tracks all pipelines using HLLRedisCounter by one user_id' do - allow(Gitlab::UsageDataCounters::HLLRedisCounter) + it 'tracks all pipelines using HLLRedisCounter by one user_id for the test event' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter) .to receive(:track_event) - .with(test_event_name, values: user1.id) + .with(test_event_name_1, values: user1.id) .and_call_original - allow(Gitlab::UsageDataCounters::HLLRedisCounter) + expect(Gitlab::UsageDataCounters::HLLRedisCounter) .to receive(:track_event) - .with(test_event_name, values: user1.id) + .with(test_event_name_1, values: user1.id) .and_call_original expect do @@ -73,7 +83,7 @@ RSpec.describe Ci::JobArtifacts::TrackArtifactReportService do described_class.new.execute(pipeline2) end .to change { - counter.unique_events(event_names: test_event_name, + counter.unique_events(event_names: test_event_name_1, start_date: start_time, end_date: end_time) } @@ -85,25 +95,92 @@ RSpec.describe Ci::JobArtifacts::TrackArtifactReportService do let_it_be(:pipeline1) { create(:ci_pipeline, :with_test_reports, project: project, user: user1) } let_it_be(:pipeline2) { create(:ci_pipeline, :with_test_reports, project: project, user: user2) } - it 'tracks all pipelines using HLLRedisCounter by multiple users' do - allow(Gitlab::UsageDataCounters::HLLRedisCounter) + it 'tracks all pipelines using HLLRedisCounter by multiple users for test reports' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event) + .with(test_event_name_1, values: user1.id) + .and_call_original + + expect(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event) + .with(test_event_name_1, values: user2.id) + .and_call_original + + expect do + described_class.new.execute(pipeline1) + described_class.new.execute(pipeline2) + end + .to change { + counter.unique_events(event_names: test_event_name_1, + start_date: start_time, + end_date: end_time) + } + .by 2 + end + end + + context 'when pipeline has coverage test reports' do + let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user1) } + + before do + 2.times do + pipeline.builds << build(:ci_build, :coverage_reports, pipeline: pipeline, project: pipeline.project) + end + end + + it 'tracks the coverage test event using HLLRedisCounter' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter) .to receive(:track_event) - .with(test_event_name, values: user1.id) + .with(test_event_name_2, values: user1.id) .and_call_original - allow(Gitlab::UsageDataCounters::HLLRedisCounter) + expect { track_artifact_report } + .to change { + counter.unique_events(event_names: test_event_name_2, + start_date: start_time, + end_date: end_time) + } + .by 1 + end + end + + context 'when a single user started multiple pipelines with coverage reports' do + let_it_be(:pipeline1) { create(:ci_pipeline, :with_coverage_reports, project: project, user: user1) } + let_it_be(:pipeline2) { create(:ci_pipeline, :with_coverage_reports, project: project, user: user1) } + + it 'tracks all pipelines using HLLRedisCounter by one user_id for the coverage test event' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter) .to receive(:track_event) - .with(test_event_name, values: user1.id) + .with(test_event_name_2, values: user1.id) + .twice .and_call_original - allow(Gitlab::UsageDataCounters::HLLRedisCounter) - .to receive(:track_event) - .with(test_event_name, values: user2.id) - .and_call_original + expect do + described_class.new.execute(pipeline1) + described_class.new.execute(pipeline2) + end + .to change { + counter.unique_events(event_names: test_event_name_2, + start_date: start_time, + end_date: end_time) + } + .by 1 + end + end + + context 'when multiple users started multiple pipelines with coverage test reports' do + let_it_be(:pipeline1) { create(:ci_pipeline, :with_coverage_reports, project: project, user: user1) } + let_it_be(:pipeline2) { create(:ci_pipeline, :with_coverage_reports, project: project, user: user2) } + + it 'tracks all pipelines using HLLRedisCounter by multiple users for coverage test reports' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event) + .with(test_event_name_2, values: user1.id) + .and_call_original - allow(Gitlab::UsageDataCounters::HLLRedisCounter) + expect(Gitlab::UsageDataCounters::HLLRedisCounter) .to receive(:track_event) - .with(test_event_name, values: user2.id) + .with(test_event_name_2, values: user2.id) .and_call_original expect do @@ -111,7 +188,7 @@ RSpec.describe Ci::JobArtifacts::TrackArtifactReportService do described_class.new.execute(pipeline2) end .to change { - counter.unique_events(event_names: test_event_name, + counter.unique_events(event_names: test_event_name_2, start_date: start_time, end_date: end_time) } diff --git a/spec/services/ci/pipeline_processing/atomic_processing_service/status_collection_spec.rb b/spec/services/ci/pipeline_processing/atomic_processing_service/status_collection_spec.rb index 7578afa7c50..d0aa1ba4c6c 100644 --- a/spec/services/ci/pipeline_processing/atomic_processing_service/status_collection_spec.rb +++ b/spec/services/ci/pipeline_processing/atomic_processing_service/status_collection_spec.rb @@ -104,7 +104,7 @@ RSpec.describe Ci::PipelineProcessing::AtomicProcessingService::StatusCollection describe '#processing_processables' do it 'returns processables marked as processing' do - expect(collection.processing_processables.map { |processable| processable[:id] } ) + expect(collection.processing_processables.map { |processable| processable[:id] }) .to contain_exactly(build_a.id, build_b.id, test_a.id, test_b.id, deploy.id) 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 06bb6d39fe5..1fbefc1fa22 100644 --- a/spec/services/ci/pipeline_processing/atomic_processing_service_spec.rb +++ b/spec/services/ci/pipeline_processing/atomic_processing_service_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Ci::PipelineProcessing::AtomicProcessingService do check_expectation(test_file.dig('init', 'expect'), "init") test_file['transitions'].each_with_index do |transition, idx| - event_on_jobs(transition['event'], transition['jobs']) + process_events(transition) Sidekiq::Worker.drain_all # ensure that all async jobs are executed check_expectation(transition['expect'], "transition:#{idx}") end @@ -48,20 +48,37 @@ RSpec.describe Ci::PipelineProcessing::AtomicProcessingService do } end + def process_events(transition) + if transition['jobs'] + event_on_jobs(transition['event'], transition['jobs']) + else + event_on_pipeline(transition['event']) + end + end + def event_on_jobs(event, job_names) statuses = pipeline.latest_statuses.by_name(job_names).to_a expect(statuses.count).to eq(job_names.count) # ensure that we have the same counts statuses.each do |status| - if event == 'play' + case event + when 'play' status.play(user) - elsif event == 'retry' + when 'retry' ::Ci::RetryJobService.new(project, user).execute(status) else status.public_send("#{event}!") end end end + + def event_on_pipeline(event) + if event == 'retry' + pipeline.retry_failed(user) + else + pipeline.public_send("#{event}!") + end + end end end diff --git a/spec/services/ci/pipeline_processing/test_cases/stage_one_test_succeeds_one_manual_test_fails_and_retry_manual_build.yml b/spec/services/ci/pipeline_processing/test_cases/stage_one_test_succeeds_one_manual_test_fails_and_retry_manual_build.yml new file mode 100644 index 00000000000..a50fe56f8d4 --- /dev/null +++ b/spec/services/ci/pipeline_processing/test_cases/stage_one_test_succeeds_one_manual_test_fails_and_retry_manual_build.yml @@ -0,0 +1,54 @@ +config: + test1: + script: exit 0 + + test2: + when: manual + script: exit 1 + +init: + expect: + pipeline: pending + stages: + test: pending + jobs: + test1: pending + test2: manual + +transitions: + - event: success + jobs: [test1] + expect: + pipeline: success + stages: + test: success + jobs: + test1: success + test2: manual + - event: play + jobs: [test2] + expect: + pipeline: running + stages: + test: running + jobs: + test1: success + test2: pending + - event: drop + jobs: [test2] + expect: + pipeline: success + stages: + test: success + jobs: + test1: success + test2: failed + - event: retry + jobs: [test2] + expect: + pipeline: running + stages: + test: running + jobs: + test1: success + test2: pending diff --git a/spec/services/ci/pipeline_processing/test_cases/stage_one_test_succeeds_one_manual_test_fails_and_retry_pipeline.yml b/spec/services/ci/pipeline_processing/test_cases/stage_one_test_succeeds_one_manual_test_fails_and_retry_pipeline.yml new file mode 100644 index 00000000000..a6112a95a12 --- /dev/null +++ b/spec/services/ci/pipeline_processing/test_cases/stage_one_test_succeeds_one_manual_test_fails_and_retry_pipeline.yml @@ -0,0 +1,53 @@ +config: + test1: + script: exit 0 + + test2: + when: manual + script: exit 1 + +init: + expect: + pipeline: pending + stages: + test: pending + jobs: + test1: pending + test2: manual + +transitions: + - event: success + jobs: [test1] + expect: + pipeline: success + stages: + test: success + jobs: + test1: success + test2: manual + - event: play + jobs: [test2] + expect: + pipeline: running + stages: + test: running + jobs: + test1: success + test2: pending + - event: drop + jobs: [test2] + expect: + pipeline: success + stages: + test: success + jobs: + test1: success + test2: failed + - event: retry + expect: + pipeline: success + stages: + test: success + jobs: + test1: success + test2: manual diff --git a/spec/services/ci/pipeline_schedules/take_ownership_service_spec.rb b/spec/services/ci/pipeline_schedules/take_ownership_service_spec.rb new file mode 100644 index 00000000000..9a3aad20d89 --- /dev/null +++ b/spec/services/ci/pipeline_schedules/take_ownership_service_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::PipelineSchedules::TakeOwnershipService do + let_it_be(:user) { create(:user) } + let_it_be(:owner) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: owner) } + + before_all do + project.add_maintainer(user) + project.add_maintainer(owner) + project.add_reporter(reporter) + end + + describe '#execute' do + context 'when user does not have permission' do + subject(:service) { described_class.new(pipeline_schedule, reporter) } + + it 'returns ServiceResponse.error' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to eq(_('Failed to change the owner')) + end + end + + context 'when user has permission' do + subject(:service) { described_class.new(pipeline_schedule, user) } + + it 'returns ServiceResponse.success' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.success?).to be(true) + expect(result.payload).to eq(pipeline_schedule) + end + + context 'when schedule update fails' do + subject(:service) { described_class.new(pipeline_schedule, owner) } + + before do + allow(pipeline_schedule).to receive(:update).and_return(false) + + errors = ActiveModel::Errors.new(pipeline_schedule) + errors.add(:base, 'An error occurred') + allow(pipeline_schedule).to receive(:errors).and_return(errors) + end + + it 'returns ServiceResponse.error' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to eq(['An error occurred']) + end + end + end + end +end diff --git a/spec/services/ci/play_build_service_spec.rb b/spec/services/ci/play_build_service_spec.rb index 85ef8b60af4..fc07801b672 100644 --- a/spec/services/ci/play_build_service_spec.rb +++ b/spec/services/ci/play_build_service_spec.rb @@ -87,11 +87,15 @@ RSpec.describe Ci::PlayBuildService, '#execute' do expect(build.reload.job_variables.map(&:key)).to contain_exactly('first', 'second') end - context 'when variables are invalid' do + context 'and variables are invalid' do let(:job_variables) { [{}] } - it 'raises an error' do - expect { subject }.to raise_error(ActiveRecord::RecordInvalid) + it 'resets the attributes of the build' do + build.update!(job_variables_attributes: [{ key: 'old', value: 'old variable' }]) + + subject + + expect(build.job_variables.map(&:key)).to contain_exactly('old') end end diff --git a/spec/services/ci/process_build_service_spec.rb b/spec/services/ci/process_build_service_spec.rb index 2fcb4ce73ff..9301098b083 100644 --- a/spec/services/ci/process_build_service_spec.rb +++ b/spec/services/ci/process_build_service_spec.rb @@ -2,147 +2,118 @@ require 'spec_helper' RSpec.describe Ci::ProcessBuildService, '#execute' do - let(:user) { create(:user) } - let(:project) { create(:project) } + using RSpec::Parameterized::TableSyntax + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, ref: 'master', project: project) } subject { described_class.new(project, user).execute(build, current_status) } - before do + before_all do project.add_maintainer(user) end - context 'when build has on_success option' do - let(:build) { create(:ci_build, :created, when: :on_success, user: user, project: project) } - - context 'when current status is success' do - let(:current_status) { 'success' } - - it 'changes the build status' do - expect { subject }.to change { build.status }.to('pending') - end - end - - context 'when current status is skipped' do - let(:current_status) { 'skipped' } - - it 'changes the build status' do - expect { subject }.to change { build.status }.to('pending') - end - end - - context 'when current status is failed' do - let(:current_status) { 'failed' } - - it 'does not change the build status' do - expect { subject }.to change { build.status }.to('skipped') - end + shared_context 'with enqueue_immediately set' do + before do + build.set_enqueue_immediately! end end - context 'when build has on_failure option' do - let(:build) { create(:ci_build, :created, when: :on_failure, user: user, project: project) } - - context 'when current status is success' do - let(:current_status) { 'success' } - - it 'changes the build status' do - expect { subject }.to change { build.status }.to('skipped') - end - end - - context 'when current status is failed' do - let(:current_status) { 'failed' } - - it 'does not change the build status' do - expect { subject }.to change { build.status }.to('pending') - end + shared_context 'with ci_retry_job_fix disabled' do + before do + stub_feature_flags(ci_retry_job_fix: false) end end - context 'when build has always option' do - let(:build) { create(:ci_build, :created, when: :always, user: user, project: project) } - - context 'when current status is success' do - let(:current_status) { 'success' } - - it 'changes the build status' do - expect { subject }.to change { build.status }.to('pending') - end + context 'for single build' do + let!(:build) { create(:ci_build, *[trait].compact, :created, **conditions, pipeline: pipeline) } + + where(:trait, :conditions, :current_status, :after_status, :retry_after_status, :retry_disabled_after_status) do + nil | { when: :on_success } | 'success' | 'pending' | 'pending' | 'pending' + nil | { when: :on_success } | 'skipped' | 'pending' | 'pending' | 'pending' + nil | { when: :on_success } | 'failed' | 'skipped' | 'skipped' | 'skipped' + nil | { when: :on_failure } | 'success' | 'skipped' | 'skipped' | 'skipped' + nil | { when: :on_failure } | 'skipped' | 'skipped' | 'skipped' | 'skipped' + nil | { when: :on_failure } | 'failed' | 'pending' | 'pending' | 'pending' + nil | { when: :always } | 'success' | 'pending' | 'pending' | 'pending' + nil | { when: :always } | 'skipped' | 'pending' | 'pending' | 'pending' + nil | { when: :always } | 'failed' | 'pending' | 'pending' | 'pending' + :actionable | { when: :manual } | 'success' | 'manual' | 'pending' | 'manual' + :actionable | { when: :manual } | 'skipped' | 'manual' | 'pending' | 'manual' + :actionable | { when: :manual } | 'failed' | 'skipped' | 'skipped' | 'skipped' + :schedulable | { when: :delayed } | 'success' | 'scheduled' | 'pending' | 'scheduled' + :schedulable | { when: :delayed } | 'skipped' | 'scheduled' | 'pending' | 'scheduled' + :schedulable | { when: :delayed } | 'failed' | 'skipped' | 'skipped' | 'skipped' end - context 'when current status is failed' do - let(:current_status) { 'failed' } - - it 'does not change the build status' do - expect { subject }.to change { build.status }.to('pending') + with_them do + it 'updates the job status to after_status' do + expect { subject }.to change { build.status }.to(after_status) end - end - end - - context 'when build has manual option' do - let(:build) { create(:ci_build, :created, :actionable, user: user, project: project) } - context 'when current status is success' do - let(:current_status) { 'success' } + context 'when build is set to enqueue immediately' do + include_context 'with enqueue_immediately set' - it 'changes the build status' do - expect { subject }.to change { build.status }.to('manual') - end - end + it 'updates the job status to retry_after_status' do + expect { subject }.to change { build.status }.to(retry_after_status) + end - context 'when current status is failed' do - let(:current_status) { 'failed' } + context 'when feature flag ci_retry_job_fix is disabled' do + include_context 'with ci_retry_job_fix disabled' - it 'does not change the build status' do - expect { subject }.to change { build.status }.to('skipped') + it "updates the job status to retry_disabled_after_status" do + expect { subject }.to change { build.status }.to(retry_disabled_after_status) + end + end end end end - context 'when build has delayed option' do - before do - allow(Ci::BuildScheduleWorker).to receive(:perform_at) {} + context 'when build is scheduled with DAG' do + let!(:build) do + create( + :ci_build, + *[trait].compact, + :dependent, + :created, + when: build_when, + pipeline: pipeline, + needed: other_build + ) end - let(:build) { create(:ci_build, :created, :schedulable, user: user, project: project) } - - context 'when current status is success' do - let(:current_status) { 'success' } + let!(:other_build) { create(:ci_build, :created, when: :on_success, pipeline: pipeline) } - it 'changes the build status' do - expect { subject }.to change { build.status }.to('scheduled') - end + where(:trait, :build_when, :current_status, :after_status, :retry_after_status, :retry_disabled_after_status) do + nil | :on_success | 'success' | 'pending' | 'pending' | 'pending' + nil | :on_success | 'skipped' | 'skipped' | 'skipped' | 'skipped' + nil | :manual | 'success' | 'manual' | 'pending' | 'manual' + nil | :manual | 'skipped' | 'skipped' | 'skipped' | 'skipped' + nil | :delayed | 'success' | 'manual' | 'pending' | 'manual' + nil | :delayed | 'skipped' | 'skipped' | 'skipped' | 'skipped' + :schedulable | :delayed | 'success' | 'scheduled' | 'pending' | 'scheduled' + :schedulable | :delayed | 'skipped' | 'skipped' | 'skipped' | 'skipped' end - context 'when current status is failed' do - let(:current_status) { 'failed' } - - it 'does not change the build status' do - expect { subject }.to change { build.status }.to('skipped') + with_them do + it 'updates the job status to after_status' do + expect { subject }.to change { build.status }.to(after_status) end - end - end - context 'when build is scheduled with DAG' do - using RSpec::Parameterized::TableSyntax + context 'when build is set to enqueue immediately' do + include_context 'with enqueue_immediately set' - let(:pipeline) { create(:ci_pipeline, ref: 'master', project: project) } - let!(:build) { create(:ci_build, :created, when: build_when, pipeline: pipeline, scheduling_type: :dag) } - let!(:other_build) { create(:ci_build, :created, when: :on_success, pipeline: pipeline) } - let!(:build_on_other_build) { create(:ci_build_need, build: build, name: other_build.name) } - - where(:build_when, :current_status, :after_status) do - :on_success | 'success' | 'pending' - :on_success | 'skipped' | 'skipped' - :manual | 'success' | 'manual' - :manual | 'skipped' | 'skipped' - :delayed | 'success' | 'manual' - :delayed | 'skipped' | 'skipped' - end + it 'updates the job status to retry_after_status' do + expect { subject }.to change { build.status }.to(retry_after_status) + end - with_them do - it 'proceeds the build' do - expect { subject }.to change { build.status }.to(after_status) + context 'when feature flag ci_retry_job_fix is disabled' do + include_context 'with ci_retry_job_fix disabled' + + it "updates the job status to retry_disabled_after_status" do + expect { subject }.to change { build.status }.to(retry_disabled_after_status) + end + end end end end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index e2e760b9812..f40f5cc5a62 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -14,25 +14,29 @@ module Ci let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) } describe '#execute' do - context 'checks database loadbalancing stickiness' do - subject { described_class.new(shared_runner).execute } + subject { described_class.new(shared_runner).execute } + context 'checks database loadbalancing stickiness' do before do project.update!(shared_runners_enabled: false) end - it 'result is valid if replica did caught-up' do + it 'result is valid if replica did caught-up', :aggregate_failures do expect(ApplicationRecord.sticking).to receive(:all_caught_up?) .with(:runner, shared_runner.id) { true } expect(subject).to be_valid + expect(subject.build).to be_nil + expect(subject.build_json).to be_nil end - it 'result is invalid if replica did not caught-up' do + it 'result is invalid if replica did not caught-up', :aggregate_failures do expect(ApplicationRecord.sticking).to receive(:all_caught_up?) .with(:runner, shared_runner.id) { false } expect(subject).not_to be_valid + expect(subject.build).to be_nil + expect(subject.build_json).to be_nil end end @@ -954,6 +958,7 @@ module Ci expect(result).not_to be_valid expect(result.build).to be_nil + expect(result.build_json).to be_nil end end diff --git a/spec/services/ci/retry_job_service_spec.rb b/spec/services/ci/retry_job_service_spec.rb index 69f19c5acf2..540e700efa6 100644 --- a/spec/services/ci/retry_job_service_spec.rb +++ b/spec/services/ci/retry_job_service_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe Ci::RetryJobService do + using RSpec::Parameterized::TableSyntax let_it_be(:reporter) { create(:user) } let_it_be(:developer) { create(:user) } let_it_be(:project) { create(:project, :repository) } @@ -11,11 +12,11 @@ RSpec.describe Ci::RetryJobService do end let_it_be(:stage) do - create(:ci_stage, project: project, - pipeline: pipeline, - name: 'test') + create(:ci_stage, pipeline: pipeline, name: 'test') end + let_it_be(:deploy_stage) { create(:ci_stage, pipeline: pipeline, name: 'deploy', position: stage.position + 1) } + let(:job_variables_attributes) { [{ key: 'MANUAL_VAR', value: 'manual test var' }] } let(:user) { developer } @@ -26,24 +27,11 @@ RSpec.describe Ci::RetryJobService do project.add_reporter(reporter) end - shared_context 'retryable bridge' do - let_it_be(:downstream_project) { create(:project, :repository) } - - let_it_be_with_refind(:job) do - create(:ci_bridge, :success, - pipeline: pipeline, downstream: downstream_project, description: 'a trigger job', ci_stage: stage - ) - end - - let_it_be(:job_to_clone) { job } - - before do - job.update!(retried: false) + shared_context 'retryable build' do + let_it_be_with_reload(:job) do + create(:ci_build, :success, pipeline: pipeline, ci_stage: stage) end - end - shared_context 'retryable build' do - let_it_be_with_refind(:job) { create(:ci_build, :success, pipeline: pipeline, ci_stage: stage) } let_it_be(:another_pipeline) { create(:ci_empty_pipeline, project: project) } let_it_be(:job_to_clone) do @@ -60,6 +48,12 @@ RSpec.describe Ci::RetryJobService do end end + shared_context 'with ci_retry_job_fix disabled' do + before do + stub_feature_flags(ci_retry_job_fix: false) + end + end + shared_examples_for 'clones the job' do let(:job) { job_to_clone } @@ -87,8 +81,7 @@ RSpec.describe Ci::RetryJobService do context 'when the job has needs' do before do - create(:ci_build_need, build: job, name: 'build1') - create(:ci_build_need, build: job, name: 'build2') + create_list(:ci_build_need, 2, build: job) end it 'bulk inserts all the needs' do @@ -123,16 +116,12 @@ RSpec.describe Ci::RetryJobService do end context 'when there are subsequent processables that are skipped' do - let_it_be(:stage) { create(:ci_stage, pipeline: pipeline, name: 'deploy') } - let!(:subsequent_build) do - create(:ci_build, :skipped, stage_idx: 2, - pipeline: pipeline, - ci_stage: stage) + create(:ci_build, :skipped, pipeline: pipeline, ci_stage: deploy_stage) end let!(:subsequent_bridge) do - create(:ci_bridge, :skipped, stage_idx: 2, pipeline: pipeline, ci_stage: stage) + create(:ci_bridge, :skipped, pipeline: pipeline, ci_stage: deploy_stage) end it 'resumes pipeline processing in the subsequent stage' do @@ -152,10 +141,9 @@ RSpec.describe Ci::RetryJobService do end context 'when the pipeline has other jobs' do - let!(:stage2) { create(:ci_stage, project: project, pipeline: pipeline, name: 'deploy') } - let!(:build2) { create(:ci_build, pipeline: pipeline, ci_stage: stage ) } - let!(:deploy) { create(:ci_build, pipeline: pipeline, ci_stage: stage2) } - let!(:deploy_needs_build2) { create(:ci_build_need, build: deploy, name: build2.name) } + let!(:other_test_build) { create(:ci_build, pipeline: pipeline, ci_stage: stage) } + let!(:deploy) { create(:ci_build, pipeline: pipeline, ci_stage: deploy_stage) } + let!(:deploy_needs_build2) { create(:ci_build_need, build: deploy, name: other_test_build.name) } context 'when job has a nil scheduling_type' do before do @@ -166,7 +154,7 @@ RSpec.describe Ci::RetryJobService do it 'populates scheduling_type of processables' do expect(new_job.scheduling_type).to eq('stage') expect(job.reload.scheduling_type).to eq('stage') - expect(build2.reload.scheduling_type).to eq('stage') + expect(other_test_build.reload.scheduling_type).to eq('stage') expect(deploy.reload.scheduling_type).to eq('dag') end end @@ -193,6 +181,13 @@ RSpec.describe Ci::RetryJobService do end end + shared_examples_for 'checks enqueue_immediately?' do + it "returns enqueue_immediately" do + subject + expect(new_job.enqueue_immediately?).to eq enqueue_immediately + end + end + describe '#clone!' do let(:new_job) { service.clone!(job) } @@ -200,20 +195,6 @@ RSpec.describe Ci::RetryJobService do expect { service.clone!(create(:ci_build).present) }.to raise_error(TypeError) end - context 'when the job to be cloned is a bridge' do - include_context 'retryable bridge' - - it_behaves_like 'clones the job' - - context 'when given variables' do - let(:new_job) { service.clone!(job, variables: job_variables_attributes) } - - it 'does not give variables to the new bridge' do - expect { new_job }.not_to raise_error - end - end - end - context 'when the job to be cloned is a build' do include_context 'retryable build' @@ -224,7 +205,7 @@ RSpec.describe Ci::RetryJobService do context 'when a build with a deployment is retried' do let!(:job) do create(:ci_build, :with_deployment, :deploy_to_production, - pipeline: pipeline, ci_stage: stage, project: project) + pipeline: pipeline, ci_stage: stage) end it 'creates a new deployment' do @@ -247,7 +228,6 @@ RSpec.describe Ci::RetryJobService do options: { environment: { name: environment_name } }, pipeline: pipeline, ci_stage: stage, - project: project, user: other_developer) end @@ -282,24 +262,44 @@ RSpec.describe Ci::RetryJobService do end end end - end - describe '#execute' do - let(:new_job) { service.execute(job)[:job] } + context 'when enqueue_if_actionable is provided' do + let!(:job) do + create(:ci_build, *[trait].compact, :failed, pipeline: pipeline, ci_stage: stage) + end - context 'when the job to be retried is a bridge' do - include_context 'retryable bridge' + let(:new_job) { subject } - it_behaves_like 'retries the job' + subject { service.clone!(job, enqueue_if_actionable: enqueue_if_actionable) } - context 'when given variables' do - let(:new_job) { service.clone!(job, variables: job_variables_attributes) } + where(:enqueue_if_actionable, :trait, :enqueue_immediately) do + true | nil | false + true | :manual | true + true | :expired_scheduled | true + + false | nil | false + false | :manual | false + false | :expired_scheduled | false + end + + with_them do + it_behaves_like 'checks enqueue_immediately?' - it 'does not give variables to the new bridge' do - expect { new_job }.not_to raise_error + context 'when feature flag is disabled' do + include_context 'with ci_retry_job_fix disabled' + + it_behaves_like 'checks enqueue_immediately?' do + let(:enqueue_immediately) { false } + end end end end + end + + describe '#execute' do + let(:new_job) { subject[:job] } + + subject { service.execute(job) } context 'when the job to be retried is a build' do include_context 'retryable build' @@ -307,24 +307,18 @@ RSpec.describe Ci::RetryJobService do it_behaves_like 'retries the job' context 'when there are subsequent jobs that are skipped' do - let_it_be(:stage) { create(:ci_stage, pipeline: pipeline, name: 'deploy') } - let!(:subsequent_build) do - create(:ci_build, :skipped, stage_idx: 2, - pipeline: pipeline, - stage_id: stage.id) + create(:ci_build, :skipped, pipeline: pipeline, ci_stage: deploy_stage) end let!(:subsequent_bridge) do - create(:ci_bridge, :skipped, stage_idx: 2, - pipeline: pipeline, - stage_id: stage.id) + create(:ci_bridge, :skipped, pipeline: pipeline, ci_stage: deploy_stage) end it 'does not cause an N+1 when updating the job ownership' do control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { service.execute(job) }.count - create_list(:ci_build, 2, :skipped, stage_idx: job.stage_idx + 1, pipeline: pipeline, stage_id: stage.id) + create_list(:ci_build, 2, :skipped, pipeline: pipeline, ci_stage: deploy_stage) expect { service.execute(job) }.not_to exceed_all_query_limit(control_count) end @@ -352,5 +346,161 @@ RSpec.describe Ci::RetryJobService do end end end + + context 'when job being retried has jobs in previous stages' do + let!(:job) do + create( + :ci_build, + :failed, + name: 'deploy_a', + pipeline: pipeline, + ci_stage: deploy_stage + ) + end + + before do + create( + :ci_build, + previous_stage_job_status, + name: 'test_a', + pipeline: pipeline, + ci_stage: stage + ) + end + + where(:previous_stage_job_status, :after_status) do + :created | 'created' + :pending | 'created' + :running | 'created' + :manual | 'created' + :scheduled | 'created' + :success | 'pending' + :failed | 'skipped' + :skipped | 'pending' + end + + with_them do + it 'updates the new job status to after_status' do + expect(subject).to be_success + expect(new_job.status).to eq after_status + end + + context 'when feature flag is disabled' do + include_context 'with ci_retry_job_fix disabled' + + it 'enqueues the new job' do + expect(subject).to be_success + expect(new_job).to be_pending + end + end + end + end + + context 'when job being retried has DAG dependencies' do + let!(:job) do + create( + :ci_build, + :failed, + :dependent, + name: 'deploy_a', + pipeline: pipeline, + ci_stage: deploy_stage, + needed: dependency + ) + end + + let(:dependency) do + create( + :ci_build, + dag_dependency_status, + name: 'test_a', + pipeline: pipeline, + ci_stage: stage + ) + end + + where(:dag_dependency_status, :after_status) do + :created | 'created' + :pending | 'created' + :running | 'created' + :manual | 'created' + :scheduled | 'created' + :success | 'pending' + :failed | 'skipped' + :skipped | 'skipped' + end + + with_them do + it 'updates the new job status to after_status' do + expect(subject).to be_success + expect(new_job.status).to eq after_status + end + + context 'when feature flag is disabled' do + include_context 'with ci_retry_job_fix disabled' + + it 'enqueues the new job' do + expect(subject).to be_success + expect(new_job).to be_pending + end + end + end + end + + context 'when there are other manual/scheduled jobs' do + let_it_be(:test_manual_build) do + create(:ci_build, :manual, pipeline: pipeline, ci_stage: stage) + end + + let_it_be(:subsequent_manual_build) do + create(:ci_build, :manual, pipeline: pipeline, ci_stage: deploy_stage) + end + + let_it_be(:test_scheduled_build) do + create(:ci_build, :scheduled, pipeline: pipeline, ci_stage: stage) + end + + let_it_be(:subsequent_scheduled_build) do + create(:ci_build, :scheduled, pipeline: pipeline, ci_stage: deploy_stage) + end + + let!(:job) do + create(:ci_build, *[trait].compact, :failed, pipeline: pipeline, ci_stage: stage) + end + + where(:trait, :enqueue_immediately) do + nil | false + :manual | true + :expired_scheduled | true + end + + with_them do + it 'retries the given job but not the other manual/scheduled jobs' do + expect { subject } + .to change { Ci::Build.count }.by(1) + .and not_change { test_manual_build.reload.status } + .and not_change { subsequent_manual_build.reload.status } + .and not_change { test_scheduled_build.reload.status } + .and not_change { subsequent_scheduled_build.reload.status } + + expect(new_job).to be_pending + end + + it_behaves_like 'checks enqueue_immediately?' + + context 'when feature flag is disabled' do + include_context 'with ci_retry_job_fix disabled' + + it 'enqueues the new job' do + expect(subject).to be_success + expect(new_job).to be_pending + end + + it_behaves_like 'checks enqueue_immediately?' do + let(:enqueue_immediately) { false } + end + end + end + end end end diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index 96437290ae3..77345096537 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -5,14 +5,16 @@ require 'spec_helper' RSpec.describe Ci::RetryPipelineService, '#execute' do include ProjectForksHelper - let(:user) { create(:user) } - let(:project) { create(:project) } + let_it_be_with_refind(:user) { create(:user) } + let_it_be_with_refind(:project) { create(:project) } + let(:pipeline) { create(:ci_pipeline, project: project) } - let(:service) { described_class.new(project, user) } let(:build_stage) { create(:ci_stage, name: 'build', position: 0, pipeline: pipeline) } let(:test_stage) { create(:ci_stage, name: 'test', position: 1, pipeline: pipeline) } let(:deploy_stage) { create(:ci_stage, name: 'deploy', position: 2, pipeline: pipeline) } + subject(:service) { described_class.new(project, user) } + context 'when user has full ability to modify pipeline' do before do project.add_developer(user) @@ -272,6 +274,21 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do expect(pipeline.reload).to be_running end end + + context 'when there is a failed manual action' do + before do + create_build('rspec', :success, build_stage) + create_build('manual-rspec', :failed, build_stage, when: :manual, allow_failure: true) + end + + it 'processes the manual action' do + service.execute(pipeline) + + expect(build('rspec')).to be_success + expect(build('manual-rspec')).to be_manual + expect(pipeline.reload).to be_success + end + end end it 'closes all todos about failed jobs for pipeline' do diff --git a/spec/services/ci/runners/bulk_delete_runners_service_spec.rb b/spec/services/ci/runners/bulk_delete_runners_service_spec.rb index 8e9fc4e3012..fa8af1100df 100644 --- a/spec/services/ci/runners/bulk_delete_runners_service_spec.rb +++ b/spec/services/ci/runners/bulk_delete_runners_service_spec.rb @@ -5,78 +5,180 @@ require 'spec_helper' RSpec.describe ::Ci::Runners::BulkDeleteRunnersService, '#execute' do subject(:execute) { described_class.new(**service_args).execute } - let(:service_args) { { runners: runners_arg } } + let_it_be(:admin_user) { create(:user, :admin) } + let_it_be_with_refind(:owner_user) { create(:user) } # discard memoized ci_owned_runners + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + + let(:user) {} + let(:service_args) { { runners: runners_arg, current_user: user } } let(:runners_arg) {} context 'with runners specified' do let!(:instance_runner) { create(:ci_runner) } - let!(:group_runner) { create(:ci_runner, :group) } - let!(:project_runner) { create(:ci_runner, :project) } + let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let!(:project_runner) { create(:ci_runner, :project, projects: [project]) } shared_examples 'a service deleting runners in bulk' do + let!(:expected_deleted_ids) { expected_deleted_runners.map(&:id) } + it 'destroys runners', :aggregate_failures do - expect { subject }.to change { Ci::Runner.count }.by(-2) + expect { execute }.to change { Ci::Runner.count }.by(-expected_deleted_ids.count) is_expected.to be_success - expect(execute.payload).to eq({ deleted_count: 2, deleted_ids: [instance_runner.id, project_runner.id] }) - expect(instance_runner[:errors]).to be_nil - expect(project_runner[:errors]).to be_nil + expect(execute.payload).to eq( + { + deleted_count: expected_deleted_ids.count, + deleted_ids: expected_deleted_ids, + errors: [] + }) expect { project_runner.runner_projects.first.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect { group_runner.reload }.not_to raise_error - expect { instance_runner.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect { project_runner.reload }.to raise_error(ActiveRecord::RecordNotFound) + expected_deleted_runners.each do |deleted_runner| + expect(deleted_runner[:errors]).to be_nil + expect { deleted_runner.reload }.to raise_error(ActiveRecord::RecordNotFound) + end end - context 'with some runners already deleted' do + context 'with too many runners specified' do before do - instance_runner.destroy! + stub_const("#{described_class}::RUNNER_LIMIT", 1) end - let(:runners_arg) { [instance_runner.id, project_runner.id] } - - it 'destroys runners and returns only deleted runners', :aggregate_failures do - expect { subject }.to change { Ci::Runner.count }.by(-1) + it 'deletes only first RUNNER_LIMIT runners', :aggregate_failures do + expect { execute }.to change { Ci::Runner.count }.by(-1) is_expected.to be_success - expect(execute.payload).to eq({ deleted_count: 1, deleted_ids: [project_runner.id] }) - expect(instance_runner[:errors]).to be_nil - expect(project_runner[:errors]).to be_nil - expect { project_runner.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect(execute.payload).to eq( + { + deleted_count: 1, + deleted_ids: expected_deleted_ids.take(1), + errors: ["Can only delete up to 1 runners per call. Ignored the remaining runner(s)."] + }) end end + end - context 'with too many runners specified' do + context 'when the user cannot delete runners' do + let(:runners_arg) { Ci::Runner.all } + + context 'when user is not group owner' do before do - stub_const("#{described_class}::RUNNER_LIMIT", 1) + group.add_developer(user) end - it 'deletes only first RUNNER_LIMIT runners' do - expect { subject }.to change { Ci::Runner.count }.by(-1) + let(:user) { create(:user) } - is_expected.to be_success - expect(execute.payload).to eq({ deleted_count: 1, deleted_ids: [instance_runner.id] }) + it 'does not delete any runner and returns error', :aggregate_failures do + expect { execute }.not_to change { Ci::Runner.count } + expect(execute[:errors]).to match_array("User does not have permission to delete any of the runners") end end - end - context 'with runners specified as relation' do - let(:runners_arg) { Ci::Runner.not_group_type } + context 'when user is not part of the group' do + let(:user) { create(:user) } - include_examples 'a service deleting runners in bulk' + it 'does not delete any runner and returns error', :aggregate_failures do + expect { execute }.not_to change { Ci::Runner.count } + expect(execute[:errors]).to match_array("User does not have permission to delete any of the runners") + end + end end - context 'with runners specified as array of IDs' do - let(:runners_arg) { Ci::Runner.not_group_type.ids } + context 'when the user can delete runners' do + context 'when user is an admin', :enable_admin_mode do + include_examples 'a service deleting runners in bulk' do + let(:runners_arg) { Ci::Runner.all } + let!(:expected_deleted_runners) { [instance_runner, group_runner, project_runner] } + let(:user) { admin_user } + end + + context 'with a runner already deleted' do + before do + group_runner.destroy! + end + + include_examples 'a service deleting runners in bulk' do + let(:runners_arg) { Ci::Runner.all } + let!(:expected_deleted_runners) { [instance_runner, project_runner] } + let(:user) { admin_user } + end + end + + context 'when deleting a single runner' do + let(:runners_arg) { Ci::Runner.all } + + it 'avoids N+1 cached queries', :use_sql_query_cache, :request_store do + # Run this once to establish a baseline + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + execute + end + + additional_runners = 1 + + create_list(:ci_runner, 1 + additional_runners, :instance) + create_list(:ci_runner, 1 + additional_runners, :group, groups: [group]) + create_list(:ci_runner, 1 + additional_runners, :project, projects: [project]) + + service = described_class.new(runners: runners_arg, current_user: user) + + # Base cost per runner is: + # - 1 `SELECT * FROM "taggings"` query + # - 1 `SAVEPOINT` query + # - 1 `DELETE FROM "ci_runners"` query + # - 1 `RELEASE SAVEPOINT` query + # Project runners have an additional query: + # - 1 `DELETE FROM "ci_runner_projects"` query, given the call to `destroy_all` + instance_runner_cost = 4 + group_runner_cost = 4 + project_runner_cost = 5 + expect { service.execute } + .not_to exceed_all_query_limit(control_count) + .with_threshold(additional_runners * (instance_runner_cost + group_runner_cost + project_runner_cost)) + end + end + end - include_examples 'a service deleting runners in bulk' + context 'when user is group owner' do + before do + group.add_owner(user) + end + + include_examples 'a service deleting runners in bulk' do + let(:runners_arg) { Ci::Runner.not_instance_type } + let!(:expected_deleted_runners) { [group_runner, project_runner] } + let(:user) { owner_user } + end + + context 'with a runner non-authorised to be deleted' do + let(:runners_arg) { Ci::Runner.all } + let!(:expected_deleted_runners) { [project_runner] } + let(:user) { owner_user } + + it 'destroys only authorised runners', :aggregate_failures do + allow(Ability).to receive(:allowed?).and_call_original + expect(Ability).to receive(:allowed?).with(user, :delete_runner, instance_runner).and_return(false) + + expect { execute }.to change { Ci::Runner.count }.by(-2) + + is_expected.to be_success + expect(execute.payload).to eq( + { + deleted_count: 2, + deleted_ids: [group_runner.id, project_runner.id], + errors: ["User does not have permission to delete runner(s) ##{instance_runner.id}"] + }) + end + end + end end context 'with no arguments specified' do let(:runners_arg) { nil } + let(:user) { owner_user } it 'returns 0 deleted runners' do is_expected.to be_success - expect(execute.payload).to eq({ deleted_count: 0, deleted_ids: [] }) + expect(execute.payload).to eq({ deleted_count: 0, deleted_ids: [], errors: [] }) end end end diff --git a/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb b/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb index 1f44612947b..e5cba80d567 100644 --- a/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb +++ b/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb @@ -67,6 +67,19 @@ RSpec.describe ::Ci::Runners::SetRunnerAssociatedProjectsService, '#execute' do expect(runner.projects.ids).to match_array([owner_project.id] + project_ids) end end + + context 'when disassociating all projects' do + let(:project_ids) { [] } + + it 'reassigns associated projects and returns success response' do + expect(execute).to be_success + + runner.reload + + expect(runner.owner_project).to eq(owner_project) + expect(runner.projects.ids).to contain_exactly(owner_project.id) + end + end end context 'with failing assign_to requests' do |