diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-09-20 02:18:09 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-09-20 02:18:09 +0300 |
commit | 6ed4ec3e0b1340f96b7c043ef51d1b33bbe85fde (patch) | |
tree | dc4d20fe6064752c0bd323187252c77e0a89144b /spec/services/ci | |
parent | 9868dae7fc0655bd7ce4a6887d4e6d487690eeed (diff) |
Add latest changes from gitlab-org/gitlab@15-4-stable-eev15.4.0-rc42
Diffstat (limited to 'spec/services/ci')
54 files changed, 1261 insertions, 218 deletions
diff --git a/spec/services/ci/after_requeue_job_service_spec.rb b/spec/services/ci/after_requeue_job_service_spec.rb index fb67ee18fb2..1f692bdb71a 100644 --- a/spec/services/ci/after_requeue_job_service_spec.rb +++ b/spec/services/ci/after_requeue_job_service_spec.rb @@ -112,7 +112,7 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do check_jobs_statuses( a1: 'pending', a2: 'created', - a3: 'skipped', + a3: 'created', b1: 'success', b2: 'created', c1: 'created', @@ -120,6 +120,26 @@ 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) } @@ -140,7 +160,7 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do expect(jobs_name_status_owner_needs).to contain_exactly( { 'name' => 'a1', 'status' => 'pending', 'user_id' => user.id, 'needs' => [] }, { 'name' => 'a2', 'status' => 'created', 'user_id' => retryer.id, 'needs' => ['a1'] }, - { 'name' => 'a3', 'status' => 'skipped', 'user_id' => user.id, 'needs' => ['a2'] }, + { 'name' => 'a3', 'status' => 'created', 'user_id' => retryer.id, 'needs' => ['a2'] }, { 'name' => 'b1', 'status' => 'success', 'user_id' => user.id, 'needs' => [] }, { 'name' => 'b2', 'status' => 'created', 'user_id' => retryer.id, 'needs' => ['a2'] }, { 'name' => 'c1', 'status' => 'created', 'user_id' => retryer.id, 'needs' => ['b2'] }, @@ -237,6 +257,79 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do end end + context 'with same-stage needs' do + let(:config) do + <<-EOY + a: + script: exit $(($RANDOM % 2)) + + b: + script: exit 0 + needs: [a] + + c: + script: exit 0 + needs: [b] + EOY + end + + let(:pipeline) do + Ci::CreatePipelineService.new(project, user, { ref: 'master' }).execute(:push).payload + end + + let(:a) { find_job('a') } + + before do + stub_ci_pipeline_yaml_file(config) + check_jobs_statuses( + a: 'pending', + b: 'created', + c: 'created' + ) + + a.drop! + check_jobs_statuses( + a: 'failed', + b: 'skipped', + c: 'skipped' + ) + + new_a = Ci::RetryJobService.new(project, user).clone!(a) + new_a.enqueue! + check_jobs_statuses( + a: 'pending', + b: 'skipped', + c: 'skipped' + ) + end + + it 'marks subsequent skipped jobs as processable' do + execute_after_requeue_service(a) + + check_jobs_statuses( + a: 'pending', + b: 'created', + 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 def find_job(name) diff --git a/spec/services/ci/archive_trace_service_spec.rb b/spec/services/ci/archive_trace_service_spec.rb index bf2e5302d2e..359ea0699e4 100644 --- a/spec/services/ci/archive_trace_service_spec.rb +++ b/spec/services/ci/archive_trace_service_spec.rb @@ -17,21 +17,12 @@ RSpec.describe Ci::ArchiveTraceService, '#execute' do context 'integration hooks' do it do - stub_feature_flags(datadog_integration_logs_collection: [job.project]) - expect(job.project).to receive(:execute_integrations) do |data, hook_type| expect(data).to eq Gitlab::DataBuilder::ArchiveTrace.build(job) expect(hook_type).to eq :archive_trace_hooks end expect { subject }.not_to raise_error end - - it 'with feature flag disabled' do - stub_feature_flags(datadog_integration_logs_collection: false) - - expect(job.project).not_to receive(:execute_integrations) - expect { subject }.not_to raise_error - end end context 'when trace is already archived' do diff --git a/spec/services/ci/build_erase_service_spec.rb b/spec/services/ci/build_erase_service_spec.rb new file mode 100644 index 00000000000..e750a163621 --- /dev/null +++ b/spec/services/ci/build_erase_service_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::BuildEraseService do + let_it_be(:user) { user } + + let(:build) { create(:ci_build, :artifacts, :trace_artifact, artifacts_expire_at: 100.days.from_now) } + + subject(:service) { described_class.new(build, user) } + + describe '#execute' do + context 'when build is erasable' do + before do + allow(build).to receive(:erasable?).and_return(true) + end + + it 'is successful' do + result = service.execute + + expect(result).to be_success + end + + it 'erases artifacts' do + service.execute + + expect(build.artifacts_file).not_to be_present + expect(build.artifacts_metadata).not_to be_present + end + + it 'erases trace' do + service.execute + + expect(build.trace).not_to exist + end + + it 'records erasure detail' do + freeze_time do + service.execute + + expect(build.erased_by).to eq(user) + expect(build.erased_at).to eq(Time.current) + expect(build.artifacts_expire_at).to be_nil + end + end + + context 'when project is undergoing statistics refresh' do + before do + allow(build.project).to receive(:refreshing_build_artifacts_size?).and_return(true) + end + + it 'logs a warning' do + expect(Gitlab::ProjectStatsRefreshConflictsLogger) + .to receive(:warn_artifact_deletion_during_stats_refresh) + .with(method: 'Ci::BuildEraseService#execute', project_id: build.project_id) + + service.execute + end + end + end + + context 'when build is not erasable' do + before do + allow(build).to receive(:erasable?).and_return(false) + end + + it 'is not successful' do + result = service.execute + + expect(result).to be_error + expect(result.http_status).to eq(:unprocessable_entity) + end + + it 'does not erase artifacts' do + service.execute + + expect(build.artifacts_file).to be_present + expect(build.artifacts_metadata).to be_present + end + + it 'does not erase trace' do + service.execute + + expect(build.trace).to exist + end + end + end +end diff --git a/spec/services/ci/compare_reports_base_service_spec.rb b/spec/services/ci/compare_reports_base_service_spec.rb index 9ce58c4972d..20d8cd37553 100644 --- a/spec/services/ci/compare_reports_base_service_spec.rb +++ b/spec/services/ci/compare_reports_base_service_spec.rb @@ -6,13 +6,13 @@ RSpec.describe Ci::CompareReportsBaseService do let(:service) { described_class.new(project) } let(:project) { create(:project, :repository) } + let!(:base_pipeline) { nil } + let!(:head_pipeline) { create(:ci_pipeline, :with_test_reports, project: project) } + let!(:key) { service.send(:key, base_pipeline, head_pipeline) } + describe '#latest?' do subject { service.latest?(base_pipeline, head_pipeline, data) } - let!(:base_pipeline) { nil } - let!(:head_pipeline) { create(:ci_pipeline, :with_test_reports, project: project) } - let!(:key) { service.send(:key, base_pipeline, head_pipeline) } - context 'when cache key is latest' do let(:data) { { key: key } } @@ -35,4 +35,14 @@ RSpec.describe Ci::CompareReportsBaseService do it { is_expected.to be_falsy } end end + + describe '#execute' do + context 'when base_pipeline is running' do + let!(:base_pipeline) { create(:ci_pipeline, :running, project: project) } + + subject { service.execute(base_pipeline, head_pipeline) } + + it { is_expected.to eq(status: :parsing, key: key) } + end + end end diff --git a/spec/services/ci/create_downstream_pipeline_service_spec.rb b/spec/services/ci/create_downstream_pipeline_service_spec.rb index 11fb564b843..9c02c5218f1 100644 --- a/spec/services/ci/create_downstream_pipeline_service_spec.rb +++ b/spec/services/ci/create_downstream_pipeline_service_spec.rb @@ -5,9 +5,12 @@ require 'spec_helper' RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do include Ci::SourcePipelineHelpers - let_it_be(:user) { create(:user) } + # Using let_it_be on user and projects for these specs can cause + # spec-ordering failures due to the project-based permissions + # associating them. They should be recreated every time. + let(:user) { create(:user) } let(:upstream_project) { create(:project, :repository) } - let_it_be(:downstream_project, refind: true) { create(:project, :repository) } + let(:downstream_project) { create(:project, :repository) } let!(:upstream_pipeline) do create(:ci_pipeline, :running, project: upstream_project) @@ -440,10 +443,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do let!(:trigger_project_bridge) do create( - :ci_bridge, status: :pending, - user: user, - options: trigger_downstream_project, - pipeline: child_pipeline + :ci_bridge, status: :pending, user: user, options: trigger_downstream_project, pipeline: child_pipeline ) end @@ -819,5 +819,60 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do end end end + + context 'when a downstream pipeline has sibling pipelines' do + it_behaves_like 'logs downstream pipeline creation' do + let(:expected_root_pipeline) { upstream_pipeline } + let(:expected_downstream_relationship) { :multi_project } + + # New downstream, plus upstream, plus two children of upstream created below + let(:expected_hierarchy_size) { 4 } + + before do + create_list(:ci_pipeline, 2, child_of: upstream_pipeline) + end + end + end + + context 'when the pipeline tree is too large' do + let_it_be(:parent) { create(:ci_pipeline) } + let_it_be(:child) { create(:ci_pipeline, child_of: parent) } + let_it_be(:sibling) { create(:ci_pipeline, child_of: parent) } + + before do + stub_const("#{described_class}::MAX_HIERARCHY_SIZE", 3) + end + + let(:bridge) do + create(:ci_bridge, status: :pending, user: user, options: trigger, pipeline: child) + end + + it 'does not create a new pipeline' do + expect { subject }.not_to change { Ci::Pipeline.count } + end + + it 'drops the trigger job with an explanatory reason' do + subject + + expect(bridge.reload).to be_failed + expect(bridge.failure_reason).to eq('reached_max_pipeline_hierarchy_size') + end + + context 'with :ci_limit_complete_hierarchy_size disabled' do + before do + stub_feature_flags(ci_limit_complete_hierarchy_size: false) + end + + it 'creates a new pipeline' do + expect { subject }.to change { Ci::Pipeline.count }.by(1) + end + + it 'marks the bridge job as successful' do + subject + + expect(bridge.reload).to be_success + end + end + end end end diff --git a/spec/services/ci/create_pipeline_service/artifacts_spec.rb b/spec/services/ci/create_pipeline_service/artifacts_spec.rb index 1ec30d68666..e5e405492a0 100644 --- a/spec/services/ci/create_pipeline_service/artifacts_spec.rb +++ b/spec/services/ci/create_pipeline_service/artifacts_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } diff --git a/spec/services/ci/create_pipeline_service/cache_spec.rb b/spec/services/ci/create_pipeline_service/cache_spec.rb index fe777bc50d9..82c3d374636 100644 --- a/spec/services/ci/create_pipeline_service/cache_spec.rb +++ b/spec/services/ci/create_pipeline_service/cache_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do context 'cache' do let(:project) { create(:project, :custom_repo, files: files) } let(:user) { project.first_owner } diff --git a/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb b/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb index a920b90b97d..0ebcecdd6e6 100644 --- a/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb +++ b/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do describe 'creation errors and warnings' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } diff --git a/spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb b/spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb index e1d60ed57ef..74d3534eb45 100644 --- a/spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb +++ b/spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService, '#execute' do +RSpec.describe Ci::CreatePipelineService, '#execute', :yaml_processor_feature_flag_corectness do let_it_be(:group) { create(:group, name: 'my-organization') } let(:upstream_project) { create(:project, :repository, name: 'upstream', group: group) } diff --git a/spec/services/ci/create_pipeline_service/custom_config_content_spec.rb b/spec/services/ci/create_pipeline_service/custom_config_content_spec.rb index a0cbf14d936..dafa227c4c8 100644 --- a/spec/services/ci/create_pipeline_service/custom_config_content_spec.rb +++ b/spec/services/ci/create_pipeline_service/custom_config_content_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } diff --git a/spec/services/ci/create_pipeline_service/custom_yaml_tags_spec.rb b/spec/services/ci/create_pipeline_service/custom_yaml_tags_spec.rb index 716a929830e..3b042f05fc0 100644 --- a/spec/services/ci/create_pipeline_service/custom_yaml_tags_spec.rb +++ b/spec/services/ci/create_pipeline_service/custom_yaml_tags_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do describe '!reference tags' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } diff --git a/spec/services/ci/create_pipeline_service/dry_run_spec.rb b/spec/services/ci/create_pipeline_service/dry_run_spec.rb index 9a7e97fb12b..de1ed251c82 100644 --- a/spec/services/ci/create_pipeline_service/dry_run_spec.rb +++ b/spec/services/ci/create_pipeline_service/dry_run_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } diff --git a/spec/services/ci/create_pipeline_service/environment_spec.rb b/spec/services/ci/create_pipeline_service/environment_spec.rb index 43b5220334c..438cb6ac895 100644 --- a/spec/services/ci/create_pipeline_service/environment_spec.rb +++ b/spec/services/ci/create_pipeline_service/environment_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do let_it_be(:project) { create(:project, :repository) } let_it_be(:developer) { create(:user) } @@ -45,5 +45,51 @@ RSpec.describe Ci::CreatePipelineService do end end end + + context 'when variables are dependent on stage name' do + let(:config) do + <<~YAML + deploy-review-app-1: + stage: deploy + environment: 'test/$CI_JOB_STAGE/1' + script: + - echo $SCOPED_VARIABLE + rules: + - if: $SCOPED_VARIABLE == 'my-value-1' + + deploy-review-app-2: + stage: deploy + script: + - echo $SCOPED_VARIABLE + environment: 'test/$CI_JOB_STAGE/2' + rules: + - if: $SCOPED_VARIABLE == 'my-value-2' + YAML + end + + before do + create(:ci_variable, key: 'SCOPED_VARIABLE', value: 'my-value-1', environment_scope: '*', project: project) + create(:ci_variable, + key: 'SCOPED_VARIABLE', + value: 'my-value-2', + environment_scope: 'test/deploy/*', + project: project + ) + stub_ci_pipeline_yaml_file(config) + end + + it 'creates the pipeline successfully', :aggregate_failures do + pipeline = subject + build = pipeline.builds.first + + expect(pipeline).to be_created_successfully + expect(Environment.find_by_name('test/deploy/2')).to be_persisted + expect(pipeline.builds.size).to eq(1) + expect(build.persisted_environment.name).to eq('test/deploy/2') + expect(build.name).to eq('deploy-review-app-2') + expect(build.environment).to eq('test/$CI_JOB_STAGE/2') + expect(build.variables.to_hash['SCOPED_VARIABLE']).to eq('my-value-2') + end + end end end diff --git a/spec/services/ci/create_pipeline_service/evaluate_runner_tags_spec.rb b/spec/services/ci/create_pipeline_service/evaluate_runner_tags_spec.rb index 7c698242921..e84726d31f6 100644 --- a/spec/services/ci/create_pipeline_service/evaluate_runner_tags_spec.rb +++ b/spec/services/ci/create_pipeline_service/evaluate_runner_tags_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do let_it_be(:group) { create(:group, :private) } let_it_be(:group_variable) { create(:ci_group_variable, group: group, key: 'RUNNER_TAG', value: 'group') } let_it_be(:project) { create(:project, :repository, group: group) } diff --git a/spec/services/ci/create_pipeline_service/include_spec.rb b/spec/services/ci/create_pipeline_service/include_spec.rb index 849eb5885f6..67d8530525a 100644 --- a/spec/services/ci/create_pipeline_service/include_spec.rb +++ b/spec/services/ci/create_pipeline_service/include_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do context 'include:' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } diff --git a/spec/services/ci/create_pipeline_service/logger_spec.rb b/spec/services/ci/create_pipeline_service/logger_spec.rb index 53e5f0dd7f2..2be23802757 100644 --- a/spec/services/ci/create_pipeline_service/logger_spec.rb +++ b/spec/services/ci/create_pipeline_service/logger_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do context 'pipeline logger' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } @@ -19,9 +19,9 @@ RSpec.describe Ci::CreatePipelineService do let(:counters) do { 'count' => a_kind_of(Numeric), - 'avg' => a_kind_of(Numeric), - 'max' => a_kind_of(Numeric), - 'min' => a_kind_of(Numeric) + 'avg' => a_kind_of(Numeric), + 'max' => a_kind_of(Numeric), + 'min' => a_kind_of(Numeric) } end diff --git a/spec/services/ci/create_pipeline_service/merge_requests_spec.rb b/spec/services/ci/create_pipeline_service/merge_requests_spec.rb index de19ef363fb..80f48451e5c 100644 --- a/spec/services/ci/create_pipeline_service/merge_requests_spec.rb +++ b/spec/services/ci/create_pipeline_service/merge_requests_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do context 'merge requests handling' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } diff --git a/spec/services/ci/create_pipeline_service/needs_spec.rb b/spec/services/ci/create_pipeline_service/needs_spec.rb index abd17ccdd6a..38e330316ea 100644 --- a/spec/services/ci/create_pipeline_service/needs_spec.rb +++ b/spec/services/ci/create_pipeline_service/needs_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do context 'needs' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } diff --git a/spec/services/ci/create_pipeline_service/parallel_spec.rb b/spec/services/ci/create_pipeline_service/parallel_spec.rb index ae28b74fef5..5ee378a9719 100644 --- a/spec/services/ci/create_pipeline_service/parallel_spec.rb +++ b/spec/services/ci/create_pipeline_service/parallel_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } diff --git a/spec/services/ci/create_pipeline_service/parameter_content_spec.rb b/spec/services/ci/create_pipeline_service/parameter_content_spec.rb index f593707f460..cae88bb67cf 100644 --- a/spec/services/ci/create_pipeline_service/parameter_content_spec.rb +++ b/spec/services/ci/create_pipeline_service/parameter_content_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } diff --git a/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb b/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb index 4326fa5533f..513cbbed6cd 100644 --- a/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb +++ b/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService, '#execute' do +RSpec.describe Ci::CreatePipelineService, '#execute', :yaml_processor_feature_flag_corectness do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } @@ -36,7 +36,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do expect(pipeline.statuses).to match_array [test, bridge] expect(bridge.options).to eq(expected_bridge_options) expect(bridge.yaml_variables) - .to include(key: 'CROSS', value: 'downstream', public: true) + .to include(key: 'CROSS', value: 'downstream') end end diff --git a/spec/services/ci/create_pipeline_service/partitioning_spec.rb b/spec/services/ci/create_pipeline_service/partitioning_spec.rb new file mode 100644 index 00000000000..43fbb74ede4 --- /dev/null +++ b/spec/services/ci/create_pipeline_service/partitioning_spec.rb @@ -0,0 +1,146 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness, :aggregate_failures 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(:config) do + <<-YAML + stages: + - build + - test + - deploy + + build: + stage: build + script: make build + + test: + stage: test + trigger: + include: child.yml + + deploy: + stage: deploy + script: make deploy + environment: review/$CI_JOB_NAME + YAML + end + + let(:pipeline) { service.execute(:push).payload } + let(:current_partition_id) { 123 } + + before do + stub_ci_pipeline_yaml_file(config) + allow(Ci::Pipeline).to receive(:current_partition_value) { current_partition_id } + end + + it 'assigns partition_id to pipeline' do + expect(pipeline).to be_created_successfully + expect(pipeline.partition_id).to eq(current_partition_id) + end + + it 'assigns partition_id to stages' do + stage_partition_ids = pipeline.stages.map(&:partition_id).uniq + + expect(stage_partition_ids).to eq([current_partition_id]) + end + + it 'assigns partition_id to processables' do + processables_partition_ids = pipeline.processables.map(&:partition_id).uniq + + expect(processables_partition_ids).to eq([current_partition_id]) + end + + it 'assigns partition_id to metadata' do + metadata_partition_ids = pipeline.processables.map { |job| job.metadata.partition_id }.uniq + + expect(metadata_partition_ids).to eq([current_partition_id]) + end + + it 'correctly assigns partition and environment' do + metadata = find_metadata('deploy') + + expect(metadata.partition_id).to eq(current_partition_id) + expect(metadata.expanded_environment_name).to eq('review/deploy') + end + + context 'with pipeline variables' do + let(:variables_attributes) do + [ + { key: 'SOME_VARIABLE', secret_value: 'SOME_VAL' }, + { key: 'OTHER_VARIABLE', secret_value: 'OTHER_VAL' } + ] + end + + let(:service) do + described_class.new( + project, + user, + { ref: 'master', variables_attributes: variables_attributes }) + end + + it 'assigns partition_id to pipeline' do + expect(pipeline).to be_created_successfully + expect(pipeline.partition_id).to eq(current_partition_id) + end + + it 'assigns partition_id to variables' do + variables_partition_ids = pipeline.variables.map(&:partition_id).uniq + + expect(pipeline.variables.size).to eq(2) + expect(variables_partition_ids).to eq([current_partition_id]) + end + end + + context 'with parent child pipelines' do + before do + allow(Ci::Pipeline) + .to receive(:current_partition_value) + .and_return(current_partition_id, 301, 302) + + allow_next_found_instance_of(Ci::Bridge) do |bridge| + allow(bridge).to receive(:yaml_for_downstream).and_return(child_config) + end + end + + let(:config) do + <<-YAML + test: + trigger: + include: child.yml + YAML + end + + let(:child_config) do + <<-YAML + test: + script: make test + YAML + end + + it 'assigns partition values to child pipelines', :aggregate_failures, :sidekiq_inline do + expect(pipeline).to be_created_successfully + expect(pipeline.child_pipelines).to all be_created_successfully + + child_partition_ids = pipeline.child_pipelines.map(&:partition_id).uniq + child_jobs = CommitStatus.where(commit_id: pipeline.child_pipelines) + + expect(pipeline.partition_id).to eq(current_partition_id) + expect(child_partition_ids).to eq([current_partition_id]) + + expect(child_jobs).to all be_a(Ci::Build) + expect(child_jobs.pluck(:partition_id).uniq).to eq([current_partition_id]) + end + end + + def find_metadata(name) + pipeline + .processables + .find { |job| job.name == name } + .metadata + end +end diff --git a/spec/services/ci/create_pipeline_service/pre_post_stages_spec.rb b/spec/services/ci/create_pipeline_service/pre_post_stages_spec.rb index c6e69862422..db110bdc608 100644 --- a/spec/services/ci/create_pipeline_service/pre_post_stages_spec.rb +++ b/spec/services/ci/create_pipeline_service/pre_post_stages_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do describe '.pre/.post stages' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } diff --git a/spec/services/ci/create_pipeline_service/rate_limit_spec.rb b/spec/services/ci/create_pipeline_service/rate_limit_spec.rb index 0000296230f..dfa74870341 100644 --- a/spec/services/ci/create_pipeline_service/rate_limit_spec.rb +++ b/spec/services/ci/create_pipeline_service/rate_limit_spec.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Ci::CreatePipelineService, :freeze_time, :clean_gitlab_redis_rate_limiting do +RSpec.describe Ci::CreatePipelineService, :freeze_time, + :clean_gitlab_redis_rate_limiting, + :yaml_processor_feature_flag_corectness do describe 'rate limiting' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } diff --git a/spec/services/ci/create_pipeline_service/rules_spec.rb b/spec/services/ci/create_pipeline_service/rules_spec.rb index 6e48141226d..fc57ca66d3a 100644 --- a/spec/services/ci/create_pipeline_service/rules_spec.rb +++ b/spec/services/ci/create_pipeline_service/rules_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do let(:project) { create(:project, :repository) } let(:user) { project.first_owner } let(:ref) { 'refs/heads/master' } diff --git a/spec/services/ci/create_pipeline_service/tags_spec.rb b/spec/services/ci/create_pipeline_service/tags_spec.rb index 0774f9fff2a..7450df11eac 100644 --- a/spec/services/ci/create_pipeline_service/tags_spec.rb +++ b/spec/services/ci/create_pipeline_service/tags_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do describe 'tags:' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } @@ -37,7 +37,7 @@ RSpec.describe Ci::CreatePipelineService do context 'tags persistence' do let(:config) do { - build: { + build: { script: 'ls', stage: 'build', tags: build_tag_list(label: 'build') diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index a9442b0dc68..c2e80316d26 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::CreatePipelineService do +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness, :clean_gitlab_redis_cache do include ProjectForksHelper let_it_be_with_refind(:project) { create(:project, :repository) } @@ -463,7 +463,7 @@ RSpec.describe Ci::CreatePipelineService do it 'pull it from Auto-DevOps' do pipeline = execute_service.payload expect(pipeline).to be_auto_devops_source - expect(pipeline.builds.map(&:name)).to match_array(%w[brakeman-sast build code_quality container_scanning eslint-sast secret_detection semgrep-sast test]) + expect(pipeline.builds.map(&:name)).to match_array(%w[brakeman-sast build code_quality container_scanning secret_detection semgrep-sast test]) 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 7b3f67b192f..a2259f9813b 100644 --- a/spec/services/ci/job_artifacts/create_service_spec.rb +++ b/spec/services/ci/job_artifacts/create_service_spec.rb @@ -151,9 +151,8 @@ RSpec.describe Ci::JobArtifacts::CreateService do expect { subject }.not_to change { Ci::JobArtifact.count } expect(subject).to match( - a_hash_including(http_status: :bad_request, - message: 'another artifact of the same type already exists', - status: :error)) + a_hash_including( + http_status: :bad_request, message: 'another artifact of the same type already exists', status: :error)) end end end @@ -182,6 +181,18 @@ RSpec.describe Ci::JobArtifacts::CreateService do end end + context 'with job partitioning' do + let(:job) { create(:ci_build, project: project, partition_id: 123) } + + it 'sets partition_id on artifacts' do + expect { subject }.to change { Ci::JobArtifact.count } + + artifacts_partitions = job.job_artifacts.map(&:partition_id).uniq + + expect(artifacts_partitions).to eq([123]) + end + end + shared_examples 'rescues object storage error' do |klass, message, expected_message| it "handles #{klass}" do allow_next_instance_of(JobArtifactUploader) do |uploader| diff --git a/spec/services/ci/job_artifacts/delete_service_spec.rb b/spec/services/ci/job_artifacts/delete_service_spec.rb new file mode 100644 index 00000000000..62a755eb44a --- /dev/null +++ b/spec/services/ci/job_artifacts/delete_service_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::JobArtifacts::DeleteService do + let_it_be(:build, reload: true) do + create(:ci_build, :artifacts, :trace_artifact, artifacts_expire_at: 100.days.from_now) + end + + subject(:service) { described_class.new(build) } + + describe '#execute' do + it 'is successful' do + result = service.execute + + expect(result).to be_success + end + + it 'deletes erasable artifacts' do + expect { service.execute }.to change { build.job_artifacts.erasable.count }.from(2).to(0) + end + + it 'does not delete trace' do + expect { service.execute }.not_to change { build.has_trace? }.from(true) + end + + context 'when project is undergoing statistics refresh' do + before do + allow(build.project).to receive(:refreshing_build_artifacts_size?).and_return(true) + end + + it 'logs a warning' do + expect(Gitlab::ProjectStatsRefreshConflictsLogger) + .to receive(:warn_artifact_deletion_during_stats_refresh) + .with(method: 'Ci::JobArtifacts::DeleteService#execute', project_id: build.project_id) + + service.execute + end + end + 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 9ca39d4d32e..54d1cacc068 100644 --- a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb @@ -221,6 +221,15 @@ RSpec.describe Ci::JobArtifacts::DestroyBatchService do end context 'with update_stats: false' do + let_it_be(:extra_artifact_with_file) do + create(:ci_job_artifact, :zip, project: artifact_with_file.project) + end + + let(:artifacts) do + Ci::JobArtifact.where(id: [artifact_with_file.id, extra_artifact_with_file.id, + artifact_without_file.id, trace_artifact.id]) + end + it 'does not update project statistics' do expect(ProjectStatistics).not_to receive(:increment_statistic) @@ -230,7 +239,7 @@ RSpec.describe Ci::JobArtifacts::DestroyBatchService do it 'returns size statistics' do expected_updates = { statistics_updates: { - artifact_with_file.project => -artifact_with_file.file.size, + artifact_with_file.project => -(artifact_with_file.file.size + extra_artifact_with_file.file.size), artifact_without_file.project => 0 } } 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 new file mode 100644 index 00000000000..6d9fc4c8e34 --- /dev/null +++ b/spec/services/ci/job_artifacts/track_artifact_report_service_spec.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::JobArtifacts::TrackArtifactReportService do + describe '#execute', :clean_gitlab_redis_shared_state do + let_it_be(:group) { create(:group, :private) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:user1) { create(:user) } + let_it_be(:user2) { create(:user) } + + let(:test_event_name) { 'i_testing_test_report_uploaded' } + let(:counter) { Gitlab::UsageDataCounters::HLLRedisCounter } + let(:start_time) { 1.week.ago } + let(:end_time) { 1.week.from_now } + + subject(:track_artifact_report) { described_class.new.execute(pipeline) } + + context 'when pipeline has test reports' do + let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user1) } + + before do + 2.times do + pipeline.builds << build(:ci_build, :test_reports, pipeline: pipeline, project: pipeline.project) + end + end + + it 'tracks the event using HLLRedisCounter' do + allow(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event) + .with(test_event_name, values: user1.id) + .and_call_original + + expect { track_artifact_report } + .to change { + counter.unique_events(event_names: test_event_name, + start_date: start_time, + end_date: end_time) + } + .by 1 + end + end + + context 'when pipeline does not have test reports' do + let_it_be(:pipeline) { create(:ci_empty_pipeline) } + + it 'does not track the event' do + track_artifact_report + + expect(Gitlab::UsageDataCounters::HLLRedisCounter) + .not_to receive(:track_event) + .with(anything, test_event_name) + end + end + + context 'when a single user started multiple pipelines with test reports' 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) + .to receive(:track_event) + .with(test_event_name, values: user1.id) + .and_call_original + + allow(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event) + .with(test_event_name, values: user1.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, + start_date: start_time, + end_date: end_time) + } + .by 1 + end + end + + context 'when multiple users started multiple pipelines with test reports' 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) + .to receive(:track_event) + .with(test_event_name, values: user1.id) + .and_call_original + + allow(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event) + .with(test_event_name, values: user1.id) + .and_call_original + + allow(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event) + .with(test_event_name, values: user2.id) + .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, + start_date: start_time, + end_date: end_time) + } + .by 2 + end + end + end +end diff --git a/spec/services/ci/job_token_scope/add_project_service_spec.rb b/spec/services/ci/job_token_scope/add_project_service_spec.rb index ba889465fac..bb6df4268dd 100644 --- a/spec/services/ci/job_token_scope/add_project_service_spec.rb +++ b/spec/services/ci/job_token_scope/add_project_service_spec.rb @@ -8,6 +8,14 @@ RSpec.describe Ci::JobTokenScope::AddProjectService do let_it_be(:target_project) { create(:project) } let_it_be(:current_user) { create(:user) } + shared_examples 'adds project' do |context| + it 'adds the project to the scope' do + expect do + expect(result).to be_success + end.to change { Ci::JobToken::ProjectScopeLink.count }.by(1) + end + end + describe '#execute' do subject(:result) { service.execute(target_project) } @@ -18,10 +26,14 @@ RSpec.describe Ci::JobTokenScope::AddProjectService do target_project.add_developer(current_user) end - it 'adds the project to the scope' do - expect do - expect(result).to be_success - end.to change { Ci::JobToken::ProjectScopeLink.count }.by(1) + it_behaves_like 'adds project' + + context 'when token scope is disabled' do + before do + project.ci_cd_settings.update!(job_token_scope_enabled: false) + end + + it_behaves_like 'adds project' end end diff --git a/spec/services/ci/job_token_scope/remove_project_service_spec.rb b/spec/services/ci/job_token_scope/remove_project_service_spec.rb index 238fc879f54..155e60ac48e 100644 --- a/spec/services/ci/job_token_scope/remove_project_service_spec.rb +++ b/spec/services/ci/job_token_scope/remove_project_service_spec.rb @@ -14,6 +14,14 @@ RSpec.describe Ci::JobTokenScope::RemoveProjectService do target_project: target_project) end + shared_examples 'removes project' do |context| + it 'removes the project from the scope' do + expect do + expect(result).to be_success + end.to change { Ci::JobToken::ProjectScopeLink.count }.by(-1) + end + end + describe '#execute' do subject(:result) { service.execute(target_project) } @@ -24,10 +32,14 @@ RSpec.describe Ci::JobTokenScope::RemoveProjectService do target_project.add_developer(current_user) end - it 'removes the project from the scope' do - expect do - expect(result).to be_success - end.to change { Ci::JobToken::ProjectScopeLink.count }.by(-1) + it_behaves_like 'removes project' + + context 'when token scope is disabled' do + before do + project.ci_cd_settings.update!(job_token_scope_enabled: false) + end + + it_behaves_like 'removes project' end end diff --git a/spec/services/ci/list_config_variables_service_spec.rb b/spec/services/ci/list_config_variables_service_spec.rb index 1735f4cfc97..4953b18bfcc 100644 --- a/spec/services/ci/list_config_variables_service_spec.rb +++ b/spec/services/ci/list_config_variables_service_spec.rb @@ -40,8 +40,8 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac it 'returns variable list' do expect(subject['KEY1']).to eq({ value: 'val 1', description: 'description 1' }) expect(subject['KEY2']).to eq({ value: 'val 2', description: '' }) - expect(subject['KEY3']).to eq({ value: 'val 3', description: nil }) - expect(subject['KEY4']).to eq({ value: 'val 4', description: nil }) + expect(subject['KEY3']).to eq({ value: 'val 3' }) + expect(subject['KEY4']).to eq({ value: 'val 4' }) end end diff --git a/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb b/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb index 31548793bac..6d4dcf28108 100644 --- a/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb +++ b/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb @@ -51,6 +51,30 @@ RSpec.describe Ci::PipelineArtifacts::CoverageReportService do let!(:pipeline) { create(:ci_pipeline, :with_coverage_reports, project: project) } it_behaves_like 'creating or updating a pipeline coverage report' + + context 'when ci_update_unlocked_pipeline_artifacts feature flag is enabled' do + it "artifact has pipeline's locked status" do + subject + + artifact = Ci::PipelineArtifact.first + + expect(artifact.locked).to eq(pipeline.locked) + end + end + + context 'when ci_update_unlocked_pipeline_artifacts is disabled' do + before do + stub_feature_flags(ci_update_unlocked_pipeline_artifacts: false) + end + + it 'artifact has unknown locked status' do + subject + + artifact = Ci::PipelineArtifact.first + + expect(artifact.locked).to eq('unknown') + end + end end context 'when pipeline has coverage report from child pipeline' do diff --git a/spec/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service_spec.rb b/spec/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service_spec.rb index 5568052e346..75233248113 100644 --- a/spec/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service_spec.rb +++ b/spec/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service_spec.rb @@ -51,6 +51,30 @@ RSpec.describe ::Ci::PipelineArtifacts::CreateCodeQualityMrDiffReportService do end end + context 'when ci_update_unlocked_pipeline_artifacts feature flag is enabled' do + it "artifact has pipeline's locked status" do + subject + + artifact = Ci::PipelineArtifact.first + + expect(artifact.locked).to eq(head_pipeline.locked) + end + end + + context 'when ci_update_unlocked_pipeline_artifacts is disabled' do + before do + stub_feature_flags(ci_update_unlocked_pipeline_artifacts: false) + end + + it 'artifact has unknown locked status' do + subject + + artifact = Ci::PipelineArtifact.first + + expect(artifact.locked).to eq('unknown') + end + end + it 'does not persist the same artifact twice' do 2.times { described_class.new(head_pipeline).execute } 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 289e004fcce..7578afa7c50 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 @@ -6,11 +6,28 @@ RSpec.describe Ci::PipelineProcessing::AtomicProcessingService::StatusCollection using RSpec::Parameterized::TableSyntax let_it_be(:pipeline) { create(:ci_pipeline) } - let_it_be(:build_a) { create(:ci_build, :success, name: 'build-a', stage: 'build', stage_idx: 0, pipeline: pipeline) } - let_it_be(:build_b) { create(:ci_build, :failed, name: 'build-b', stage: 'build', stage_idx: 0, pipeline: pipeline) } - let_it_be(:test_a) { create(:ci_build, :running, name: 'test-a', stage: 'test', stage_idx: 1, pipeline: pipeline) } - let_it_be(:test_b) { create(:ci_build, :pending, name: 'test-b', stage: 'test', stage_idx: 1, pipeline: pipeline) } - let_it_be(:deploy) { create(:ci_build, :created, name: 'deploy', stage: 'deploy', stage_idx: 2, pipeline: pipeline) } + let_it_be(:build_stage) { create(:ci_stage, name: 'build', pipeline: pipeline) } + let_it_be(:test_stage) { create(:ci_stage, name: 'test', pipeline: pipeline) } + let_it_be(:deploy_stage) { create(:ci_stage, name: 'deploy', pipeline: pipeline) } + let_it_be(:build_a) do + create(:ci_build, :success, name: 'build-a', ci_stage: build_stage, stage_idx: 0, pipeline: pipeline) + end + + let_it_be(:build_b) do + create(:ci_build, :failed, name: 'build-b', ci_stage: build_stage, stage_idx: 0, pipeline: pipeline) + end + + let_it_be(:test_a) do + create(:ci_build, :running, name: 'test-a', ci_stage: test_stage, stage_idx: 1, pipeline: pipeline) + end + + let_it_be(:test_b) do + create(:ci_build, :pending, name: 'test-b', ci_stage: test_stage, stage_idx: 1, pipeline: pipeline) + end + + let_it_be(:deploy) do + create(:ci_build, :created, name: 'deploy', ci_stage: deploy_stage, stage_idx: 2, pipeline: pipeline) + end let(:collection) { described_class.new(pipeline) } 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 5bc508447c1..06bb6d39fe5 100644 --- a/spec/services/ci/pipeline_processing/atomic_processing_service_spec.rb +++ b/spec/services/ci/pipeline_processing/atomic_processing_service_spec.rb @@ -55,6 +55,8 @@ RSpec.describe Ci::PipelineProcessing::AtomicProcessingService do statuses.each do |status| if event == 'play' status.play(user) + elsif event == 'retry' + ::Ci::RetryJobService.new(project, user).execute(status) else status.public_send("#{event}!") end diff --git a/spec/services/ci/pipeline_processing/test_cases/dag_same_stage_with_fail_and_retry_1.yml b/spec/services/ci/pipeline_processing/test_cases/dag_same_stage_with_fail_and_retry_1.yml new file mode 100644 index 00000000000..b9b8eb2f532 --- /dev/null +++ b/spec/services/ci/pipeline_processing/test_cases/dag_same_stage_with_fail_and_retry_1.yml @@ -0,0 +1,55 @@ +config: + build: + script: exit $(($RANDOM % 2)) + + test: + script: exit 0 + needs: [build] + + deploy: + script: exit 0 + needs: [test] + +init: + expect: + pipeline: pending + stages: + test: pending + jobs: + build: pending + test: created + deploy: created + +transitions: + - event: drop + jobs: [build] + expect: + pipeline: failed + stages: + test: failed + jobs: + build: failed + test: skipped + deploy: skipped + + - event: retry + jobs: [build] + expect: + pipeline: running + stages: + test: pending + jobs: + build: pending + test: created + deploy: created + + - event: success + jobs: [build] + expect: + pipeline: running + stages: + test: running + jobs: + build: success + test: pending + deploy: created diff --git a/spec/services/ci/pipeline_processing/test_cases/dag_same_stage_with_fail_and_retry_2.yml b/spec/services/ci/pipeline_processing/test_cases/dag_same_stage_with_fail_and_retry_2.yml new file mode 100644 index 00000000000..c875ebab3c9 --- /dev/null +++ b/spec/services/ci/pipeline_processing/test_cases/dag_same_stage_with_fail_and_retry_2.yml @@ -0,0 +1,63 @@ +config: + build: + script: exit $(($RANDOM % 2)) + + test1: + script: exit 0 + needs: [build] + + test2: + script: exit 0 + when: manual + + deploy: + script: exit 0 + needs: [test1, test2] + +init: + expect: + pipeline: pending + stages: + test: pending + jobs: + build: pending + test1: created + test2: manual + deploy: skipped + +transitions: + - event: drop + jobs: [build] + expect: + pipeline: failed + stages: + test: failed + jobs: + build: failed + test1: skipped + test2: manual + deploy: skipped + + - event: retry + jobs: [build] + expect: + pipeline: running + stages: + test: pending + jobs: + build: pending + test1: created + test2: manual + deploy: skipped + + - event: success + jobs: [build] + expect: + pipeline: running + stages: + test: running + jobs: + build: success + test1: pending + test2: manual + deploy: skipped diff --git a/spec/services/ci/pipeline_processing/test_cases/dag_same_stage_with_subsequent_manual_jobs.yml b/spec/services/ci/pipeline_processing/test_cases/dag_same_stage_with_subsequent_manual_jobs.yml new file mode 100644 index 00000000000..03ffda1caab --- /dev/null +++ b/spec/services/ci/pipeline_processing/test_cases/dag_same_stage_with_subsequent_manual_jobs.yml @@ -0,0 +1,65 @@ +config: + job1: + script: exit 0 + when: manual + + job2: + script: exit 0 + needs: [job1] + + job3: + script: exit 0 + when: manual + needs: [job2] + + job4: + script: exit 0 + needs: [job3] + +init: + expect: + pipeline: skipped + stages: + test: skipped + jobs: + job1: manual + job2: skipped + job3: skipped + job4: skipped + +transitions: + - event: play + jobs: [job1] + expect: + pipeline: pending + stages: + test: pending + jobs: + job1: pending + job2: created + job3: created + job4: created + + - event: success + jobs: [job1] + expect: + pipeline: running + stages: + test: running + jobs: + job1: success + job2: pending + job3: created + job4: created + + - event: success + jobs: [job2] + expect: + pipeline: success + stages: + test: success + jobs: + job1: success + job2: success + job3: manual + job4: skipped diff --git a/spec/services/ci/pipeline_schedule_service_spec.rb b/spec/services/ci/pipeline_schedule_service_spec.rb index b8e4fb19f5d..2f094583f1a 100644 --- a/spec/services/ci/pipeline_schedule_service_spec.rb +++ b/spec/services/ci/pipeline_schedule_service_spec.rb @@ -3,14 +3,15 @@ require 'spec_helper' RSpec.describe Ci::PipelineScheduleService do - let(:project) { create(:project) } - let(:user) { create(:user) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let(:service) { described_class.new(project, user) } describe '#execute' do subject { service.execute(schedule) } - let(:schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } + let_it_be(:schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } it 'schedules next run' do expect(schedule).to receive(:schedule_next_run!) @@ -34,9 +35,7 @@ RSpec.describe Ci::PipelineScheduleService do end context 'when the project is missing' do - before do - project.delete - end + let(:project) { create(:project).tap(&:delete) } it 'does not raise an exception' do expect { subject }.not_to raise_error diff --git a/spec/services/ci/pipelines/add_job_service_spec.rb b/spec/services/ci/pipelines/add_job_service_spec.rb index 560724a1c6a..e735b2752d9 100644 --- a/spec/services/ci/pipelines/add_job_service_spec.rb +++ b/spec/services/ci/pipelines/add_job_service_spec.rb @@ -34,6 +34,14 @@ RSpec.describe Ci::Pipelines::AddJobService do ).and change { job.metadata.project }.to(pipeline.project) end + it 'assigns partition_id to job and metadata' do + pipeline.partition_id = 123 + + expect { execute } + .to change(job, :partition_id).to(pipeline.partition_id) + .and change { job.metadata.partition_id }.to(pipeline.partition_id) + end + it 'returns a service response with the job as payload' do expect(execute).to be_success expect(execute.payload[:job]).to eq(job) diff --git a/spec/services/ci/pipelines/hook_service_spec.rb b/spec/services/ci/pipelines/hook_service_spec.rb index 0e1ef6afd0d..8d138a3d957 100644 --- a/spec/services/ci/pipelines/hook_service_spec.rb +++ b/spec/services/ci/pipelines/hook_service_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Ci::Pipelines::HookService do describe '#execute_hooks' do let_it_be(:namespace) { create(:namespace) } let_it_be(:project) { create(:project, :repository, namespace: namespace) } - let_it_be(:pipeline) { create(:ci_empty_pipeline, :created, project: project) } + let_it_be(:pipeline, reload: true) { create(:ci_empty_pipeline, :created, project: project) } let(:hook_enabled) { true } let!(:hook) { create(:project_hook, project: project, pipeline_events: hook_enabled) } diff --git a/spec/services/ci/play_manual_stage_service_spec.rb b/spec/services/ci/play_manual_stage_service_spec.rb index b3ae92aa787..24f0a21f3dd 100644 --- a/spec/services/ci/play_manual_stage_service_spec.rb +++ b/spec/services/ci/play_manual_stage_service_spec.rb @@ -75,7 +75,6 @@ RSpec.describe Ci::PlayManualStageService, '#execute' do options.merge!({ when: 'manual', pipeline: pipeline, - stage: stage.name, stage_id: stage.id, user: pipeline.user }) @@ -87,7 +86,6 @@ RSpec.describe Ci::PlayManualStageService, '#execute' do options.merge!({ when: 'manual', pipeline: pipeline, - stage: stage.name, stage_id: stage.id, user: pipeline.user, downstream: downstream_project diff --git a/spec/services/ci/process_sync_events_service_spec.rb b/spec/services/ci/process_sync_events_service_spec.rb index 241ac4995ff..7ab7911e578 100644 --- a/spec/services/ci/process_sync_events_service_spec.rb +++ b/spec/services/ci/process_sync_events_service_spec.rb @@ -120,13 +120,15 @@ RSpec.describe Ci::ProcessSyncEventsService do before do Namespaces::SyncEvent.delete_all + # Creates a sync event for group, and the ProjectNamespace of project1 & project2: 3 in total group.update!(parent: parent_group_2) + # Creates a sync event for parent_group2 and all the children: 4 in total parent_group_2.update!(parent: parent_group_1) end shared_examples 'event consuming' do it 'consumes events' do - expect { execute }.to change(Namespaces::SyncEvent, :count).from(2).to(0) + expect { execute }.to change(Namespaces::SyncEvent, :count).from(7).to(0) expect(group.reload.ci_namespace_mirror).to have_attributes( traversal_ids: [parent_group_1.id, parent_group_2.id, group.id] @@ -134,6 +136,12 @@ RSpec.describe Ci::ProcessSyncEventsService do expect(parent_group_2.reload.ci_namespace_mirror).to have_attributes( traversal_ids: [parent_group_1.id, parent_group_2.id] ) + expect(project1.reload.project_namespace).to have_attributes( + traversal_ids: [parent_group_1.id, parent_group_2.id, group.id, project1.project_namespace.id] + ) + expect(project2.reload.project_namespace).to have_attributes( + traversal_ids: [parent_group_1.id, parent_group_2.id, group.id, project2.project_namespace.id] + ) end end @@ -157,7 +165,7 @@ RSpec.describe Ci::ProcessSyncEventsService do end it 'does not enqueue Namespaces::ProcessSyncEventsWorker if no left' do - stub_const("#{described_class}::BATCH_SIZE", 2) + stub_const("#{described_class}::BATCH_SIZE", 7) expect(Namespaces::ProcessSyncEventsWorker).not_to receive(:perform_async) diff --git a/spec/services/ci/queue/pending_builds_strategy_spec.rb b/spec/services/ci/queue/pending_builds_strategy_spec.rb new file mode 100644 index 00000000000..6f22c256c17 --- /dev/null +++ b/spec/services/ci/queue/pending_builds_strategy_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::Queue::PendingBuildsStrategy do + let_it_be(:group) { create(:group) } + let_it_be(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + + let!(:build_1) { create(:ci_build, :created, pipeline: pipeline) } + let!(:build_2) { create(:ci_build, :created, pipeline: pipeline) } + let!(:build_3) { create(:ci_build, :created, pipeline: pipeline) } + let!(:pending_build_1) { create(:ci_pending_build, build: build_2, project: project) } + let!(:pending_build_2) { create(:ci_pending_build, build: build_3, project: project) } + let!(:pending_build_3) { create(:ci_pending_build, build: build_1, project: project) } + + describe 'builds_for_group_runner' do + it 'returns builds ordered by build ID' do + strategy = described_class.new(group_runner) + expect(strategy.builds_for_group_runner).to eq([pending_build_3, pending_build_1, pending_build_2]) + end + end +end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index cabd60a22d1..e2e760b9812 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -571,10 +571,6 @@ module Ci context 'when artifacts of depended job has been erased' do let!(:pre_stage_job) { create(:ci_build, :success, pipeline: pipeline, name: 'test', stage_idx: 0, erased_at: 1.minute.ago) } - before do - pre_stage_job.erase - end - it_behaves_like 'not pick' end @@ -612,10 +608,6 @@ module Ci context 'when artifacts of depended job has been erased' do let!(:pre_stage_job) { create(:ci_build, :success, pipeline: pipeline, name: 'test', stage_idx: 0, erased_at: 1.minute.ago) } - before do - pre_stage_job.erase - end - it { expect(subject).to eq(pending_job) } end end diff --git a/spec/services/ci/resource_groups/assign_resource_from_resource_group_service_spec.rb b/spec/services/ci/resource_groups/assign_resource_from_resource_group_service_spec.rb index 194203a422c..3d1abe290bc 100644 --- a/spec/services/ci/resource_groups/assign_resource_from_resource_group_service_spec.rb +++ b/spec/services/ci/resource_groups/assign_resource_from_resource_group_service_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Ci::ResourceGroups::AssignResourceFromResourceGroupService do + include ConcurrentHelpers + let_it_be(:project) { create(:project) } let_it_be(:user) { create(:user) } @@ -134,6 +136,19 @@ RSpec.describe Ci::ResourceGroups::AssignResourceFromResourceGroupService do end end end + + context 'when parallel services are running' do + it 'can run the same command in parallel' do + parallel_num = 4 + + blocks = Array.new(parallel_num).map do + -> { subject } + end + + run_parallel(blocks) + expect(build.reload).to be_pending + end + end end context 'when there are no available resources' do diff --git a/spec/services/ci/retry_job_service_spec.rb b/spec/services/ci/retry_job_service_spec.rb index b14e4187c7a..69f19c5acf2 100644 --- a/spec/services/ci/retry_job_service_spec.rb +++ b/spec/services/ci/retry_job_service_spec.rb @@ -7,14 +7,13 @@ RSpec.describe Ci::RetryJobService do let_it_be(:developer) { create(:user) } let_it_be(:project) { create(:project, :repository) } let_it_be(:pipeline) do - create(:ci_pipeline, project: project, - sha: 'b83d6e391c22777fca1ed3012fce84f633d7fed0') + create(:ci_pipeline, project: project, sha: 'b83d6e391c22777fca1ed3012fce84f633d7fed0') end let_it_be(:stage) do create(:ci_stage, project: project, - pipeline: pipeline, - name: 'test') + pipeline: pipeline, + name: 'test') end let(:job_variables_attributes) { [{ key: 'MANUAL_VAR', value: 'manual test var' }] } @@ -31,9 +30,8 @@ RSpec.describe Ci::RetryJobService 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', stage_id: stage.id + create(:ci_bridge, :success, + pipeline: pipeline, downstream: downstream_project, description: 'a trigger job', ci_stage: stage ) end @@ -45,13 +43,13 @@ RSpec.describe Ci::RetryJobService do end shared_context 'retryable build' do - let_it_be_with_refind(:job) { create(:ci_build, :success, pipeline: pipeline, stage_id: stage.id) } + 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 create(:ci_build, :failed, :picked, :expired, :erased, :queued, :coverage, :tags, :allowed_to_fail, :on_tag, :triggered, :teardown_environment, :resource_group, - description: 'my-job', stage: 'test', stage_id: stage.id, + description: 'my-job', ci_stage: stage, pipeline: pipeline, auto_canceled_by: another_pipeline, scheduled_at: 10.seconds.since) end @@ -66,8 +64,7 @@ RSpec.describe Ci::RetryJobService do let(:job) { job_to_clone } before_all do - # Make sure that job has both `stage_id` and `stage` - job_to_clone.update!(stage: 'test', stage_id: stage.id) + job_to_clone.update!(ci_stage: stage) create(:ci_build_need, build: job_to_clone) end @@ -126,16 +123,16 @@ 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, - stage: 'deploy') + ci_stage: stage) end let!(:subsequent_bridge) do - create(:ci_bridge, :skipped, stage_idx: 2, - pipeline: pipeline, - stage: 'deploy') + create(:ci_bridge, :skipped, stage_idx: 2, pipeline: pipeline, ci_stage: stage) end it 'resumes pipeline processing in the subsequent stage' do @@ -156,8 +153,8 @@ RSpec.describe Ci::RetryJobService do 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, stage_id: stage.id ) } - let!(:deploy) { create(:ci_build, pipeline: pipeline, stage_id: stage2.id) } + 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) } context 'when job has a nil scheduling_type' do @@ -227,7 +224,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, stage_id: stage.id, project: project) + pipeline: pipeline, ci_stage: stage, project: project) end it 'creates a new deployment' do @@ -245,10 +242,13 @@ RSpec.describe Ci::RetryJobService do let(:environment_name) { 'review/$CI_COMMIT_REF_SLUG-$GITLAB_USER_ID' } let!(:job) 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) + create(:ci_build, :with_deployment, + environment: environment_name, + options: { environment: { name: environment_name } }, + pipeline: pipeline, + ci_stage: stage, + project: project, + user: other_developer) end it 'creates a new deployment' do @@ -307,22 +307,24 @@ 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: 'deploy') + stage_id: stage.id) end let!(:subsequent_bridge) do create(:ci_bridge, :skipped, stage_idx: 2, pipeline: pipeline, - stage: 'deploy') + stage_id: stage.id) 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: 'deploy') + create_list(:ci_build, 2, :skipped, stage_idx: job.stage_idx + 1, pipeline: pipeline, stage_id: stage.id) expect { service.execute(job) }.not_to exceed_all_query_limit(control_count) end diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index 24272801480..0a1e767539d 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -9,6 +9,9 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do let(: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) } context 'when user has full ability to modify pipeline' do before do @@ -20,8 +23,8 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when there are already retried jobs present' do before do - create_build('rspec', :canceled, 0, retried: true) - create_build('rspec', :failed, 0) + create_build('rspec', :canceled, build_stage, retried: true) + create_build('rspec', :failed, build_stage) end it 'does not retry jobs that has already been retried' do @@ -33,9 +36,9 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when there are failed builds in the last stage' do before do - create_build('rspec 1', :success, 0) - create_build('rspec 2', :failed, 1) - create_build('rspec 3', :canceled, 1) + create_build('rspec 1', :success, build_stage) + create_build('rspec 2', :failed, test_stage) + create_build('rspec 3', :canceled, test_stage) end it 'enqueues all builds in the last stage' do @@ -49,10 +52,10 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when there are failed or canceled builds in the first stage' do before do - create_build('rspec 1', :failed, 0) - create_build('rspec 2', :canceled, 0) - create_build('rspec 3', :canceled, 1) - create_build('spinach 1', :canceled, 2) + create_build('rspec 1', :failed, build_stage) + create_build('rspec 2', :canceled, build_stage) + create_build('rspec 3', :canceled, test_stage) + create_build('spinach 1', :canceled, deploy_stage) end it 'retries builds failed builds and marks subsequent for processing' do @@ -80,10 +83,10 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when there is failed build present which was run on failure' do before do - create_build('rspec 1', :failed, 0) - create_build('rspec 2', :canceled, 0) - create_build('rspec 3', :canceled, 1) - create_build('report 1', :failed, 2) + create_build('rspec 1', :failed, build_stage) + create_build('rspec 2', :canceled, build_stage) + create_build('rspec 3', :canceled, test_stage) + create_build('report 1', :failed, deploy_stage) end it 'retries builds only in the first stage' do @@ -105,9 +108,9 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when there is a failed test in a DAG' do before do - create_build('build', :success, 0) - create_build('build2', :success, 0) - test_build = create_build('test', :failed, 1, scheduling_type: :dag) + create_build('build', :success, build_stage) + create_build('build2', :success, build_stage) + test_build = create_build('test', :failed, test_stage, scheduling_type: :dag) create(:ci_build_need, build: test_build, name: 'build') create(:ci_build_need, build: test_build, name: 'build2') end @@ -123,7 +126,7 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when there is a failed DAG test without needs' do before do - create_build('deploy', :failed, 2, scheduling_type: :dag) + create_build('deploy', :failed, deploy_stage, scheduling_type: :dag) end it 'retries the test' do @@ -139,10 +142,10 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when the last stage was skipped' do before do - create_build('build 1', :success, 0) - create_build('test 2', :failed, 1) - create_build('report 3', :skipped, 2) - create_build('report 4', :skipped, 2) + create_build('build 1', :success, build_stage) + create_build('test 2', :failed, test_stage) + create_build('report 3', :skipped, deploy_stage) + create_build('report 4', :skipped, deploy_stage) end it 'retries builds only in the first stage' do @@ -160,9 +163,9 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when there are optional manual actions only' do context 'when there is a canceled manual action in first stage' do before do - create_build('rspec 1', :failed, 0) - create_build('staging', :canceled, 0, when: :manual, allow_failure: true) - create_build('rspec 2', :canceled, 1) + create_build('rspec 1', :failed, build_stage) + create_build('staging', :canceled, build_stage, when: :manual, allow_failure: true) + create_build('rspec 2', :canceled, test_stage) end it 'retries failed builds and marks subsequent for processing' do @@ -189,9 +192,9 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when pipeline has blocking manual actions defined' do context 'when pipeline retry should enqueue builds' do before do - create_build('test', :failed, 0) - create_build('deploy', :canceled, 0, when: :manual, allow_failure: false) - create_build('verify', :canceled, 1) + create_build('test', :failed, build_stage) + create_build('deploy', :canceled, build_stage, when: :manual, allow_failure: false) + create_build('verify', :canceled, test_stage) end it 'retries failed builds' do @@ -206,10 +209,10 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when pipeline retry should block pipeline immediately' do before do - create_build('test', :success, 0) - create_build('deploy:1', :success, 1, when: :manual, allow_failure: false) - create_build('deploy:2', :failed, 1, when: :manual, allow_failure: false) - create_build('verify', :canceled, 2) + create_build('test', :success, build_stage) + create_build('deploy:1', :success, test_stage, when: :manual, allow_failure: false) + create_build('deploy:2', :failed, test_stage, when: :manual, allow_failure: false) + create_build('verify', :canceled, deploy_stage) end it 'reprocesses blocking manual action and blocks pipeline' do @@ -225,9 +228,9 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when there is a skipped manual action in last stage' do before do - create_build('rspec 1', :canceled, 0) - create_build('rspec 2', :skipped, 0, when: :manual, allow_failure: true) - create_build('staging', :skipped, 1, when: :manual, allow_failure: true) + create_build('rspec 1', :canceled, build_stage) + create_build('rspec 2', :skipped, build_stage, when: :manual, allow_failure: true) + create_build('staging', :skipped, test_stage, when: :manual, allow_failure: true) end it 'retries canceled job and reprocesses manual actions' do @@ -242,8 +245,8 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when there is a created manual action in the last stage' do before do - create_build('rspec 1', :canceled, 0) - create_build('staging', :created, 1, when: :manual, allow_failure: true) + create_build('rspec 1', :canceled, build_stage) + create_build('staging', :created, test_stage, when: :manual, allow_failure: true) end it 'retries canceled job and does not update the manual action' do @@ -257,8 +260,8 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when there is a created manual action in the first stage' do before do - create_build('rspec 1', :canceled, 0) - create_build('staging', :created, 0, when: :manual, allow_failure: true) + create_build('rspec 1', :canceled, build_stage) + create_build('staging', :created, build_stage, when: :manual, allow_failure: true) end it 'retries canceled job and processes the manual action' do @@ -285,9 +288,9 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do end context 'when pipeline has processables with nil scheduling_type' do - let!(:build1) { create_build('build1', :success, 0) } - let!(:build2) { create_build('build2', :failed, 0) } - let!(:build3) { create_build('build3', :failed, 1) } + let!(:build1) { create_build('build1', :success, build_stage) } + let!(:build2) { create_build('build2', :failed, build_stage) } + let!(:build3) { create_build('build3', :failed, test_stage) } let!(:build3_needs_build1) { create(:ci_build_need, build: build3, name: build1.name) } before do @@ -319,10 +322,10 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when there are skipped jobs in later stages' do before do - create_build('build 1', :success, 0) - create_build('test 2', :failed, 1) - create_build('report 3', :skipped, 2) - create_bridge('deploy 4', :skipped, 2) + create_build('build 1', :success, build_stage) + create_build('test 2', :failed, test_stage) + create_build('report 3', :skipped, deploy_stage) + create_bridge('deploy 4', :skipped, deploy_stage) end it 'retries failed jobs and processes skipped jobs' do @@ -374,9 +377,9 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when there is a failed manual action present' do before do - create_build('test', :failed, 0) - create_build('deploy', :failed, 0, when: :manual) - create_build('verify', :canceled, 1) + create_build('test', :failed, build_stage) + create_build('deploy', :failed, build_stage, when: :manual) + create_build('verify', :canceled, test_stage) end it 'returns an error' do @@ -390,9 +393,9 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do context 'when there is a failed manual action in later stage' do before do - create_build('test', :failed, 0) - create_build('deploy', :failed, 1, when: :manual) - create_build('verify', :canceled, 2) + create_build('test', :failed, build_stage) + create_build('deploy', :failed, test_stage, when: :manual) + create_build('verify', :canceled, deploy_stage) end it 'returns an error' do @@ -418,7 +421,7 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do target_project: project, source_branch: 'fixes', allow_collaboration: true) - create_build('rspec 1', :failed, 1) + create_build('rspec 1', :failed, test_stage) end it 'allows to retry failed pipeline' do @@ -441,19 +444,19 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do statuses.latest.find_by(name: name) end - def create_build(name, status, stage_num, **opts) - create_processable(:ci_build, name, status, stage_num, **opts) + def create_build(name, status, stage, **opts) + create_processable(:ci_build, name, status, stage, **opts) end - def create_bridge(name, status, stage_num, **opts) - create_processable(:ci_bridge, name, status, stage_num, **opts) + def create_bridge(name, status, stage, **opts) + create_processable(:ci_bridge, name, status, stage, **opts) end - def create_processable(type, name, status, stage_num, **opts) + def create_processable(type, name, status, stage, **opts) create(type, name: name, status: status, - stage: "stage_#{stage_num}", - stage_idx: stage_num, + ci_stage: stage, + stage_idx: stage.position, pipeline: pipeline, **opts) do |_job| ::Ci::ProcessPipelineService.new(pipeline).execute 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 new file mode 100644 index 00000000000..0d2e237c87b --- /dev/null +++ b/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Ci::Runners::SetRunnerAssociatedProjectsService, '#execute' do + subject(:execute) { described_class.new(runner: runner, current_user: user, project_ids: project_ids).execute } + + let_it_be(:owner_project) { create(:project) } + let_it_be(:project2) { create(:project) } + let_it_be(:original_projects) { [owner_project, project2] } + + let(:runner) { create(:ci_runner, :project, projects: original_projects) } + + context 'without user' do + let(:user) { nil } + let(:project_ids) { [project2.id] } + + it 'does not call assign_to on runner and returns error response', :aggregate_failures do + expect(runner).not_to receive(:assign_to) + + expect(execute).to be_error + expect(execute.message).to eq('user not allowed to assign runner') + end + end + + context 'with unauthorized user' do + let(:user) { build(:user) } + let(:project_ids) { [project2.id] } + + it 'does not call assign_to on runner and returns error message' do + expect(runner).not_to receive(:assign_to) + + expect(execute).to be_error + expect(execute.message).to eq('user not allowed to assign runner') + end + end + + context 'with admin user', :enable_admin_mode do + let_it_be(:user) { create(:user, :admin) } + + let(:project3) { create(:project) } + let(:project4) { create(:project) } + + context 'with successful requests' do + context 'when disassociating a project' do + let(:project_ids) { [project3.id, project4.id] } + + it 'reassigns associated projects and returns success response' do + expect(execute).to be_success + expect(runner.reload.projects.ids).to eq([owner_project.id] + project_ids) + end + end + + context 'when disassociating no projects' do + let(:project_ids) { [project2.id, project3.id] } + + it 'reassigns associated projects and returns success response' do + expect(execute).to be_success + expect(runner.reload.projects.ids).to eq([owner_project.id] + project_ids) + end + end + end + + context 'with failing assign_to requests' do + let(:project_ids) { [project3.id, project4.id] } + + it 'returns error response and rolls back transaction' do + expect(runner).to receive(:assign_to).with(project4, user).once.and_return(false) + + expect(execute).to be_error + expect(runner.reload.projects).to eq(original_projects) + end + end + + context 'with failing destroy calls' do + let(:project_ids) { [project3.id, project4.id] } + + it 'returns error response and rolls back transaction' do + allow_next_found_instance_of(Ci::RunnerProject) do |runner_project| + allow(runner_project).to receive(:destroy).and_return(false) + end + + expect(execute).to be_error + expect(runner.reload.projects).to eq(original_projects) + end + end + end +end diff --git a/spec/services/ci/runners/update_runner_service_spec.rb b/spec/services/ci/runners/update_runner_service_spec.rb index e008fde9982..1f953ac4cbb 100644 --- a/spec/services/ci/runners/update_runner_service_spec.rb +++ b/spec/services/ci/runners/update_runner_service_spec.rb @@ -2,69 +2,65 @@ require 'spec_helper' -RSpec.describe Ci::Runners::UpdateRunnerService do +RSpec.describe Ci::Runners::UpdateRunnerService, '#execute' do + subject(:execute) { described_class.new(runner).execute(params) } + let(:runner) { create(:ci_runner) } - describe '#update' do - before do - allow(runner).to receive(:tick_runner_queue) - end + before do + allow(runner).to receive(:tick_runner_queue) + end - context 'with description params' do - let(:params) { { description: 'new runner' } } + context 'with description params' do + let(:params) { { description: 'new runner' } } - it 'updates the runner and ticking the queue' do - expect(update).to be_truthy + it 'updates the runner and ticking the queue' do + expect(execute).to be_success - runner.reload + runner.reload - expect(runner).to have_received(:tick_runner_queue) - expect(runner.description).to eq('new runner') - end + expect(runner).to have_received(:tick_runner_queue) + expect(runner.description).to eq('new runner') end + end - context 'with paused param' do - let(:params) { { paused: true } } + context 'with paused param' do + let(:params) { { paused: true } } - it 'updates the runner and ticking the queue' do - expect(runner.active).to be_truthy - expect(update).to be_truthy + it 'updates the runner and ticking the queue' do + expect(runner.active).to be_truthy + expect(execute).to be_success - runner.reload + runner.reload - expect(runner).to have_received(:tick_runner_queue) - expect(runner.active).to be_falsey - end + expect(runner).to have_received(:tick_runner_queue) + expect(runner.active).to be_falsey end + end - context 'with cost factor params' do - let(:params) { { public_projects_minutes_cost_factor: 1.1, private_projects_minutes_cost_factor: 2.2 } } + context 'with cost factor params' do + let(:params) { { public_projects_minutes_cost_factor: 1.1, private_projects_minutes_cost_factor: 2.2 } } - it 'updates the runner cost factors' do - expect(update).to be_truthy + it 'updates the runner cost factors' do + expect(execute).to be_success - runner.reload + runner.reload - expect(runner.public_projects_minutes_cost_factor).to eq(1.1) - expect(runner.private_projects_minutes_cost_factor).to eq(2.2) - end + expect(runner.public_projects_minutes_cost_factor).to eq(1.1) + expect(runner.private_projects_minutes_cost_factor).to eq(2.2) end + end - context 'when params are not valid' do - let(:params) { { run_untagged: false } } - - it 'does not update and give false because it is not valid' do - expect(update).to be_falsey + context 'when params are not valid' do + let(:params) { { run_untagged: false } } - runner.reload + it 'does not update and returns error because it is not valid' do + expect(execute).to be_error - expect(runner).not_to have_received(:tick_runner_queue) - expect(runner.run_untagged).to be_truthy - end - end + runner.reload - def update - described_class.new(runner).update(params) # rubocop: disable Rails/SaveBang + expect(runner).not_to have_received(:tick_runner_queue) + expect(runner.run_untagged).to be_truthy end end end diff --git a/spec/services/ci/unlock_artifacts_service_spec.rb b/spec/services/ci/unlock_artifacts_service_spec.rb index 94d39fc9f14..776019f03f8 100644 --- a/spec/services/ci/unlock_artifacts_service_spec.rb +++ b/spec/services/ci/unlock_artifacts_service_spec.rb @@ -5,11 +5,15 @@ require 'spec_helper' RSpec.describe Ci::UnlockArtifactsService do using RSpec::Parameterized::TableSyntax - where(:tag, :ci_update_unlocked_job_artifacts) do - false | false - false | true - true | false - true | true + where(:tag, :ci_update_unlocked_job_artifacts, :ci_update_unlocked_pipeline_artifacts) do + false | false | false + false | true | false + true | false | false + true | true | false + false | false | true + false | true | true + true | false | true + true | true | true end with_them do @@ -22,6 +26,7 @@ RSpec.describe Ci::UnlockArtifactsService do 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!(:code_coverage_pipeline) { create(:ci_pipeline, :with_coverage_report_artifact, 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) } @@ -30,7 +35,8 @@ RSpec.describe Ci::UnlockArtifactsService do before do stub_const("#{described_class}::BATCH_SIZE", 1) - stub_feature_flags(ci_update_unlocked_job_artifacts: ci_update_unlocked_job_artifacts) + stub_feature_flags(ci_update_unlocked_job_artifacts: ci_update_unlocked_job_artifacts, + ci_update_unlocked_pipeline_artifacts: ci_update_unlocked_pipeline_artifacts) end describe '#execute' do @@ -72,6 +78,14 @@ RSpec.describe Ci::UnlockArtifactsService do expect { execute }.to change { ::Ci::JobArtifact.artifact_unlocked.count }.from(0).to(2) end + + it 'unlocks pipeline artifact records' do + if ci_update_unlocked_job_artifacts && ci_update_unlocked_pipeline_artifacts + expect { execute }.to change { ::Ci::PipelineArtifact.artifact_unlocked.count }.from(0).to(1) + else + expect { execute }.not_to change { ::Ci::PipelineArtifact.artifact_unlocked.count } + end + end end context 'when running on just the ref' do @@ -106,6 +120,14 @@ RSpec.describe Ci::UnlockArtifactsService do expect { execute }.to change { ::Ci::JobArtifact.artifact_unlocked.count }.from(0).to(8) end + + it 'unlocks pipeline artifact records' do + if ci_update_unlocked_job_artifacts && ci_update_unlocked_pipeline_artifacts + expect { execute }.to change { ::Ci::PipelineArtifact.artifact_unlocked.count }.from(0).to(1) + else + expect { execute }.not_to change { ::Ci::PipelineArtifact.artifact_unlocked.count } + end + end end end |