diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-20 16:49:51 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-20 16:49:51 +0300 |
commit | 71786ddc8e28fbd3cb3fcc4b3ff15e5962a1c82e (patch) | |
tree | 6a2d93ef3fb2d353bb7739e4b57e6541f51cdd71 /spec/services/ci | |
parent | a7253423e3403b8c08f8a161e5937e1488f5f407 (diff) |
Add latest changes from gitlab-org/gitlab@15-9-stable-eev15.9.0-rc42
Diffstat (limited to 'spec/services/ci')
21 files changed, 1460 insertions, 614 deletions
diff --git a/spec/services/ci/archive_trace_service_spec.rb b/spec/services/ci/archive_trace_service_spec.rb index 359ea0699e4..3fb9d092ae7 100644 --- a/spec/services/ci/archive_trace_service_spec.rb +++ b/spec/services/ci/archive_trace_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::ArchiveTraceService, '#execute' do +RSpec.describe Ci::ArchiveTraceService, '#execute', feature_category: :continuous_integration do subject { described_class.new.execute(job, worker_name: Ci::ArchiveTraceWorker.name) } context 'when job is finished' do @@ -192,4 +192,69 @@ RSpec.describe Ci::ArchiveTraceService, '#execute' do expect(job.trace_metadata.archival_attempts).to eq(1) end end + + describe '#batch_execute' do + subject { described_class.new.batch_execute(worker_name: Ci::ArchiveTraceWorker.name) } + + let_it_be_with_reload(:job) { create(:ci_build, :success, :trace_live, finished_at: 1.day.ago) } + let_it_be_with_reload(:job2) { create(:ci_build, :success, :trace_live, finished_at: 1.day.ago) } + + it 'archives multiple traces' do + expect { subject }.not_to raise_error + + expect(job.reload.job_artifacts_trace).to be_exist + expect(job2.reload.job_artifacts_trace).to be_exist + end + + it 'processes traces independently' do + allow_next_instance_of(Gitlab::Ci::Trace) do |instance| + orig_method = instance.method(:archive!) + allow(instance).to receive(:archive!) do + raise('Unexpected error') if instance.job.id == job.id + + orig_method.call + end + end + + expect { subject }.not_to raise_error + + expect(job.reload.job_artifacts_trace).to be_nil + expect(job2.reload.job_artifacts_trace).to be_exist + end + + context 'when timeout is reached' do + before do + stub_const("#{described_class}::LOOP_TIMEOUT", 0.seconds) + end + + it 'stops executing traces' do + expect { subject }.not_to raise_error + + expect(job.reload.job_artifacts_trace).to be_nil + end + end + + context 'when loop limit is reached' do + before do + stub_const("#{described_class}::LOOP_LIMIT", -1) + end + + it 'skips archiving' do + expect(job.trace).not_to receive(:archive!) + + subject + end + + it 'stops executing traces' do + expect(Sidekiq.logger).to receive(:warn).with( + class: Ci::ArchiveTraceWorker.name, + message: "Loop limit reached.", + job_id: job.id) + + expect { subject }.not_to raise_error + + expect(job.reload.job_artifacts_trace).to be_nil + end + end + end end diff --git a/spec/services/ci/components/fetch_service_spec.rb b/spec/services/ci/components/fetch_service_spec.rb new file mode 100644 index 00000000000..f2eaa8d31b4 --- /dev/null +++ b/spec/services/ci/components/fetch_service_spec.rb @@ -0,0 +1,141 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::Components::FetchService, feature_category: :pipeline_authoring do + let_it_be(:project) { create(:project, :repository, create_tag: 'v1.0') } + let_it_be(:user) { create(:user) } + let_it_be(:current_user) { user } + let_it_be(:current_host) { Gitlab.config.gitlab.host } + + let(:service) do + described_class.new(address: address, current_user: current_user) + end + + before do + project.add_developer(user) + end + + describe '#execute', :aggregate_failures do + subject(:result) { service.execute } + + shared_examples 'an external component' do + shared_examples 'component address' do + context 'when content exists' do + let(:sha) { project.commit(version).id } + + let(:content) do + <<~COMPONENT + job: + script: echo + COMPONENT + end + + before do + stub_project_blob(sha, component_yaml_path, content) + end + + it 'returns the content' do + expect(result).to be_success + expect(result.payload[:content]).to eq(content) + end + end + + context 'when content does not exist' do + it 'returns an error' do + expect(result).to be_error + expect(result.reason).to eq(:content_not_found) + end + end + end + + context 'when user does not have permissions to read the code' do + let(:version) { 'master' } + let(:current_user) { create(:user) } + + it 'returns an error' do + expect(result).to be_error + expect(result.reason).to eq(:not_allowed) + end + end + + context 'when version is a branch name' do + it_behaves_like 'component address' do + let(:version) { project.default_branch } + end + end + + context 'when version is a tag name' do + it_behaves_like 'component address' do + let(:version) { project.repository.tags.first.name } + end + end + + context 'when version is a commit sha' do + it_behaves_like 'component address' do + let(:version) { project.repository.tags.first.id } + end + end + + context 'when version is not provided' do + let(:version) { nil } + + it 'returns an error' do + expect(result).to be_error + expect(result.reason).to eq(:content_not_found) + end + end + + context 'when project does not exist' do + let(:component_path) { 'unknown/component' } + let(:version) { '1.0' } + + it 'returns an error' do + expect(result).to be_error + expect(result.reason).to eq(:content_not_found) + end + end + + context 'when host is different than the current instance host' do + let(:current_host) { 'another-host.com' } + let(:version) { '1.0' } + + it 'returns an error' do + expect(result).to be_error + expect(result.reason).to eq(:unsupported_path) + end + end + end + + context 'when address points to an external component' do + let(:address) { "#{current_host}/#{component_path}@#{version}" } + + context 'when component path is the full path to a project' do + let(:component_path) { project.full_path } + let(:component_yaml_path) { 'template.yml' } + + it_behaves_like 'an external component' + end + + context 'when component path points to a directory in a project' do + let(:component_path) { "#{project.full_path}/my-component" } + let(:component_yaml_path) { 'my-component/template.yml' } + + it_behaves_like 'an external component' + end + + context 'when component path points to a nested directory in a project' do + let(:component_path) { "#{project.full_path}/my-dir/my-component" } + let(:component_yaml_path) { 'my-dir/my-component/template.yml' } + + it_behaves_like 'an external component' + end + end + end + + def stub_project_blob(ref, path, content) + allow_next_instance_of(Repository) do |instance| + allow(instance).to receive(:blob_data_at).with(ref, path).and_return(content) + 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 fd978bffacb..7b576339c61 100644 --- a/spec/services/ci/create_downstream_pipeline_service_spec.rb +++ b/spec/services/ci/create_downstream_pipeline_service_spec.rb @@ -890,23 +890,6 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute', feature_category end end 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) - expect(subject).to be_success - 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/job_artifacts/create_service_spec.rb b/spec/services/ci/job_artifacts/create_service_spec.rb index 711002e28af..47e9e5994ef 100644 --- a/spec/services/ci/job_artifacts/create_service_spec.rb +++ b/spec/services/ci/job_artifacts/create_service_spec.rb @@ -132,6 +132,14 @@ RSpec.describe Ci::JobArtifacts::CreateService do expect(new_artifact).to be_public_accessibility end + it 'logs the created artifact and metadata' do + expect(Gitlab::Ci::Artifacts::Logger) + .to receive(:log_created) + .with(an_instance_of(Ci::JobArtifact)).twice + + subject + end + context 'when accessibility level passed as private' do before do params.merge!('accessibility' => 'private') diff --git a/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb index dd10c0df374..457be67c1ea 100644 --- a/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' -RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_shared_state do +RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_shared_state, +feature_category: :build_artifacts do include ExclusiveLeaseHelpers let(:service) { described_class.new } 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 bf7df3a5595..e6674ee384f 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 @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Ci::JobTokenScope::AddProjectService do +RSpec.describe Ci::JobTokenScope::AddProjectService, feature_category: :continuous_integration do let(:service) { described_class.new(project, current_user) } let_it_be(:project) { create(:project, ci_outbound_job_token_scope_enabled: true).tap(&:save!) } @@ -21,6 +21,8 @@ RSpec.describe Ci::JobTokenScope::AddProjectService do it_behaves_like 'editable job token scope' do context 'when user has permissions on source and target projects' do + let(:resulting_direction) { result.payload.fetch(:project_link)&.direction } + before do project.add_maintainer(current_user) target_project.add_developer(current_user) @@ -34,6 +36,26 @@ RSpec.describe Ci::JobTokenScope::AddProjectService do end it_behaves_like 'adds project' + + it 'creates an outbound link by default' do + expect(resulting_direction).to eq('outbound') + end + + context 'when direction is specified' do + subject(:result) { service.execute(target_project, direction: direction) } + + context 'when the direction is outbound' do + let(:direction) { :outbound } + + specify { expect(resulting_direction).to eq('outbound') } + end + + context 'when the direction is inbound' do + let(:direction) { :inbound } + + specify { expect(resulting_direction).to eq('inbound') } + end + end 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 c3f9081cbd8..5b39f8908f2 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 @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Ci::JobTokenScope::RemoveProjectService do +RSpec.describe Ci::JobTokenScope::RemoveProjectService, feature_category: :continuous_integration do let(:service) { described_class.new(project, current_user) } let_it_be(:project) { create(:project, ci_outbound_job_token_scope_enabled: true).tap(&:save!) } @@ -23,7 +23,7 @@ RSpec.describe Ci::JobTokenScope::RemoveProjectService do end describe '#execute' do - subject(:result) { service.execute(target_project) } + subject(:result) { service.execute(target_project, :outbound) } it_behaves_like 'editable job token scope' do context 'when user has permissions on source and target project' do diff --git a/spec/services/ci/list_config_variables_service_spec.rb b/spec/services/ci/list_config_variables_service_spec.rb index 5b865914d1b..e2bbdefef7f 100644 --- a/spec/services/ci/list_config_variables_service_spec.rb +++ b/spec/services/ci/list_config_variables_service_spec.rb @@ -2,19 +2,21 @@ require 'spec_helper' -RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_caching do +RSpec.describe Ci::ListConfigVariablesService, +:use_clean_rails_memory_store_caching, feature_category: :pipeline_authoring do include ReactiveCachingHelpers let(:ci_config) { {} } let(:files) { { '.gitlab-ci.yml' => YAML.dump(ci_config) } } let(:project) { create(:project, :custom_repo, :auto_devops_disabled, files: files) } let(:user) { project.creator } - let(:sha) { project.default_branch } + let(:ref) { project.default_branch } + let(:sha) { project.commit(ref).sha } let(:service) { described_class.new(project, user) } - subject(:result) { service.execute(sha) } + subject(:result) { service.execute(ref) } - context 'when sending a valid sha' do + context 'when sending a valid ref' do let(:ci_config) do { variables: { @@ -109,8 +111,8 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac end end - context 'when sending an invalid sha' do - let(:sha) { 'invalid-sha' } + context 'when sending an invalid ref' do + let(:ref) { 'invalid-ref' } let(:ci_config) { nil } before do diff --git a/spec/services/ci/parse_dotenv_artifact_service_spec.rb b/spec/services/ci/parse_dotenv_artifact_service_spec.rb index 7b3af33ac72..f720375f05c 100644 --- a/spec/services/ci/parse_dotenv_artifact_service_spec.rb +++ b/spec/services/ci/parse_dotenv_artifact_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::ParseDotenvArtifactService do +RSpec.describe Ci::ParseDotenvArtifactService, feature_category: :build_artifacts do let_it_be(:project) { create(:project) } let_it_be(:pipeline) { create(:ci_pipeline, project: project) } @@ -223,6 +223,18 @@ RSpec.describe Ci::ParseDotenvArtifactService do end end + context 'when blob is encoded in UTF-16 LE' do + let(:blob) { File.read(Rails.root.join('spec/fixtures/build_artifacts/dotenv_utf16_le.txt')) } + + it 'parses the dotenv data' do + subject + + expect(build.job_variables.as_json(only: [:key, :value])).to contain_exactly( + hash_including('key' => 'MY_ENV_VAR', 'value' => 'true'), + hash_including('key' => 'TEST2', 'value' => 'false')) + end + end + context 'when more than limitated variables are specified in dotenv' do let(:blob) do StringIO.new.tap do |s| diff --git a/spec/services/ci/pipeline_creation/cancel_redundant_pipelines_service_spec.rb b/spec/services/ci/pipeline_creation/cancel_redundant_pipelines_service_spec.rb new file mode 100644 index 00000000000..402bc2faa81 --- /dev/null +++ b/spec/services/ci/pipeline_creation/cancel_redundant_pipelines_service_spec.rb @@ -0,0 +1,250 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::PipelineCreation::CancelRedundantPipelinesService, feature_category: :continuous_integration do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + + let(:prev_pipeline) { create(:ci_pipeline, project: project) } + let!(:new_commit) { create(:commit, project: project) } + let(:pipeline) { create(:ci_pipeline, project: project, sha: new_commit.sha) } + + let(:service) { described_class.new(pipeline) } + + before do + create(:ci_build, :interruptible, :running, pipeline: prev_pipeline) + create(:ci_build, :interruptible, :success, pipeline: prev_pipeline) + create(:ci_build, :created, pipeline: prev_pipeline) + + create(:ci_build, :interruptible, pipeline: pipeline) + end + + describe '#execute!' do + subject(:execute) { service.execute } + + context 'when build statuses are set up correctly' do + it 'has builds of all statuses' do + expect(build_statuses(prev_pipeline)).to contain_exactly('running', 'success', 'created') + expect(build_statuses(pipeline)).to contain_exactly('pending') + end + end + + context 'when auto-cancel is enabled' do + before do + project.update!(auto_cancel_pending_pipelines: 'enabled') + end + + it 'cancels only previous interruptible builds' do + execute + + expect(build_statuses(prev_pipeline)).to contain_exactly('canceled', 'success', 'canceled') + expect(build_statuses(pipeline)).to contain_exactly('pending') + end + + it 'logs canceled pipelines' do + allow(Gitlab::AppLogger).to receive(:info) + + execute + + expect(Gitlab::AppLogger).to have_received(:info).with( + class: described_class.name, + message: "Pipeline #{pipeline.id} auto-canceling pipeline #{prev_pipeline.id}", + canceled_pipeline_id: prev_pipeline.id, + canceled_by_pipeline_id: pipeline.id, + canceled_by_pipeline_source: pipeline.source + ) + end + + it 'cancels the builds with 2 queries to avoid query timeout' do + second_query_regex = /WHERE "ci_pipelines"\."id" = \d+ AND \(NOT EXISTS/ + recorder = ActiveRecord::QueryRecorder.new { execute } + second_query = recorder.occurrences.keys.filter { |occ| occ =~ second_query_regex } + + expect(second_query).to be_one + end + + context 'when the previous pipeline has a child pipeline' do + let(:child_pipeline) { create(:ci_pipeline, child_of: prev_pipeline) } + + context 'with another nested child pipeline' do + let(:another_child_pipeline) { create(:ci_pipeline, child_of: child_pipeline) } + + before do + create(:ci_build, :interruptible, :running, pipeline: another_child_pipeline) + create(:ci_build, :interruptible, :running, pipeline: another_child_pipeline) + end + + it 'cancels all nested child pipeline builds' do + expect(build_statuses(another_child_pipeline)).to contain_exactly('running', 'running') + + execute + + expect(build_statuses(another_child_pipeline)).to contain_exactly('canceled', 'canceled') + end + end + + context 'when started after pipeline was finished' do + before do + create(:ci_build, :interruptible, :running, pipeline: child_pipeline) + prev_pipeline.update!(status: "success") + end + + it 'cancels child pipeline builds' do + expect(build_statuses(child_pipeline)).to contain_exactly('running') + + execute + + expect(build_statuses(child_pipeline)).to contain_exactly('canceled') + end + end + + context 'when the child pipeline has interruptible running jobs' do + before do + create(:ci_build, :interruptible, :running, pipeline: child_pipeline) + create(:ci_build, :interruptible, :running, pipeline: child_pipeline) + end + + it 'cancels all child pipeline builds' do + expect(build_statuses(child_pipeline)).to contain_exactly('running', 'running') + + execute + + expect(build_statuses(child_pipeline)).to contain_exactly('canceled', 'canceled') + end + + context 'when the child pipeline includes completed interruptible jobs' do + before do + create(:ci_build, :interruptible, :failed, pipeline: child_pipeline) + create(:ci_build, :interruptible, :success, pipeline: child_pipeline) + end + + it 'cancels all child pipeline builds with a cancelable_status' do + expect(build_statuses(child_pipeline)).to contain_exactly('running', 'running', 'failed', 'success') + + execute + + expect(build_statuses(child_pipeline)).to contain_exactly('canceled', 'canceled', 'failed', 'success') + end + end + end + + context 'when the child pipeline has started non-interruptible job' do + before do + create(:ci_build, :interruptible, :running, pipeline: child_pipeline) + # non-interruptible started + create(:ci_build, :success, pipeline: child_pipeline) + end + + it 'does not cancel any child pipeline builds' do + expect(build_statuses(child_pipeline)).to contain_exactly('running', 'success') + + execute + + expect(build_statuses(child_pipeline)).to contain_exactly('running', 'success') + end + end + + context 'when the child pipeline has non-interruptible non-started job' do + before do + create(:ci_build, :interruptible, :running, pipeline: child_pipeline) + end + + not_started_statuses = Ci::HasStatus::AVAILABLE_STATUSES - Ci::HasStatus::STARTED_STATUSES + context 'when the jobs are cancelable' do + cancelable_not_started_statuses = + Set.new(not_started_statuses).intersection(Ci::HasStatus::CANCELABLE_STATUSES) + cancelable_not_started_statuses.each do |status| + it "cancels all child pipeline builds when build status #{status} included" do + # non-interruptible but non-started + create(:ci_build, status.to_sym, pipeline: child_pipeline) + + expect(build_statuses(child_pipeline)).to contain_exactly('running', status) + + execute + + expect(build_statuses(child_pipeline)).to contain_exactly('canceled', 'canceled') + end + end + end + + context 'when the jobs are not cancelable' do + not_cancelable_not_started_statuses = not_started_statuses - Ci::HasStatus::CANCELABLE_STATUSES + not_cancelable_not_started_statuses.each do |status| + it "does not cancel child pipeline builds when build status #{status} included" do + # non-interruptible but non-started + create(:ci_build, status.to_sym, pipeline: child_pipeline) + + expect(build_statuses(child_pipeline)).to contain_exactly('running', status) + + execute + + expect(build_statuses(child_pipeline)).to contain_exactly('canceled', status) + end + end + end + end + end + + context 'when the pipeline is a child pipeline' do + let!(:parent_pipeline) { create(:ci_pipeline, project: project, sha: new_commit.sha) } + let(:pipeline) { create(:ci_pipeline, child_of: parent_pipeline) } + + before do + create(:ci_build, :interruptible, :running, pipeline: parent_pipeline) + create(:ci_build, :interruptible, :running, pipeline: parent_pipeline) + end + + it 'does not cancel any builds' do + expect(build_statuses(prev_pipeline)).to contain_exactly('running', 'success', 'created') + expect(build_statuses(parent_pipeline)).to contain_exactly('running', 'running') + + execute + + expect(build_statuses(prev_pipeline)).to contain_exactly('running', 'success', 'created') + expect(build_statuses(parent_pipeline)).to contain_exactly('running', 'running') + end + end + + context 'when the previous pipeline source is webide' do + let(:prev_pipeline) { create(:ci_pipeline, :webide, project: project) } + + it 'does not cancel builds of the previous pipeline' do + execute + + expect(build_statuses(prev_pipeline)).to contain_exactly('created', 'running', 'success') + expect(build_statuses(pipeline)).to contain_exactly('pending') + end + end + + it 'does not cancel future pipelines' do + expect(prev_pipeline.id).to be < pipeline.id + expect(build_statuses(pipeline)).to contain_exactly('pending') + expect(build_statuses(prev_pipeline)).to contain_exactly('running', 'success', 'created') + + described_class.new(prev_pipeline).execute + + expect(build_statuses(pipeline.reload)).to contain_exactly('pending') + end + end + + context 'when auto-cancel is disabled' do + before do + project.update!(auto_cancel_pending_pipelines: 'disabled') + end + + it 'does not cancel any build' do + subject + + expect(build_statuses(prev_pipeline)).to contain_exactly('running', 'success', 'created') + expect(build_statuses(pipeline)).to contain_exactly('pending') + end + end + end + + private + + def build_statuses(pipeline) + pipeline.builds.pluck(:status) + end +end diff --git a/spec/services/ci/pipeline_schedule_service_spec.rb b/spec/services/ci/pipeline_schedule_service_spec.rb index 2f094583f1a..8896d8ace30 100644 --- a/spec/services/ci/pipeline_schedule_service_spec.rb +++ b/spec/services/ci/pipeline_schedule_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::PipelineScheduleService do +RSpec.describe Ci::PipelineScheduleService, feature_category: :continuous_integration do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project) } diff --git a/spec/services/ci/pipeline_schedules/update_service_spec.rb b/spec/services/ci/pipeline_schedules/update_service_spec.rb new file mode 100644 index 00000000000..838f49f6dea --- /dev/null +++ b/spec/services/ci/pipeline_schedules/update_service_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuous_integration do + let_it_be(:user) { 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: user) } + + before_all do + project.add_maintainer(user) + 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(_('The current user is not authorized to update the pipeline schedule')) + end + end + + context 'when user has permission' do + let(:params) do + { + description: 'updated_desc', + ref: 'patch-x', + active: false, + cron: '*/1 * * * *' + } + end + + subject(:service) { described_class.new(pipeline_schedule, user, params) } + + it 'updates database values with passed params' do + expect { service.execute } + .to change { pipeline_schedule.description }.from('pipeline schedule').to('updated_desc') + .and change { pipeline_schedule.ref }.from('master').to('patch-x') + .and change { pipeline_schedule.active }.from(true).to(false) + .and change { pipeline_schedule.cron }.from('0 1 * * *').to('*/1 * * * *') + end + + it 'returns ServiceResponse.success' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.success?).to be(true) + expect(result.payload.description).to eq('updated_desc') + end + + context 'when schedule update fails' do + subject(:service) { described_class.new(pipeline_schedule, user, {}) } + + 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 match_array(['An error occurred']) + 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 f40f5cc5a62..9183df359b4 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -3,795 +3,830 @@ require 'spec_helper' module Ci - RSpec.describe RegisterJobService do + RSpec.describe RegisterJobService, feature_category: :continuous_integration do let_it_be(:group) { create(:group) } let_it_be_with_reload(:project) { create(:project, group: group, shared_runners_enabled: false, group_runners_enabled: false) } let_it_be_with_reload(:pipeline) { create(:ci_pipeline, project: project) } - let!(:shared_runner) { create(:ci_runner, :instance) } - let!(:specific_runner) { create(:ci_runner, :project, projects: [project]) } + let_it_be(:shared_runner) { create(:ci_runner, :instance) } + let!(:project_runner) { create(:ci_runner, :project, projects: [project]) } let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) } describe '#execute' do - subject { described_class.new(shared_runner).execute } + subject(:execute) { described_class.new(runner, runner_machine).execute } + + context 'with runner_machine specified' do + let(:runner) { project_runner } + let!(:runner_machine) { create(:ci_runner_machine, runner: project_runner) } - context 'checks database loadbalancing stickiness' do before do - project.update!(shared_runners_enabled: false) + pending_job.update!(tag_list: ["linux"]) + pending_job.reload + pending_job.create_queuing_entry! + project_runner.update!(tag_list: ["linux"]) end - 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 } + it 'sets runner_machine on job' do + expect { execute }.to change { pending_job.reload.runner_machine }.from(nil).to(runner_machine) - expect(subject).to be_valid - expect(subject.build).to be_nil - expect(subject.build_json).to be_nil + expect(execute.build).to eq(pending_job) end + end - 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 } + context 'with no runner machine' do + let(:runner_machine) { nil } - expect(subject).not_to be_valid - expect(subject.build).to be_nil - expect(subject.build_json).to be_nil - end - end + context 'checks database loadbalancing stickiness' do + let(:runner) { shared_runner } - shared_examples 'handles runner assignment' do - context 'runner follow tag list' do - it "picks build with the same tag" do - pending_job.update!(tag_list: ["linux"]) - pending_job.reload - pending_job.create_queuing_entry! - specific_runner.update!(tag_list: ["linux"]) - expect(execute(specific_runner)).to eq(pending_job) + before do + project.update!(shared_runners_enabled: false) end - it "does not pick build with different tag" do - pending_job.update!(tag_list: ["linux"]) - pending_job.reload - pending_job.create_queuing_entry! - specific_runner.update!(tag_list: ["win32"]) - expect(execute(specific_runner)).to be_falsey - end + it 'result is valid if replica did caught-up', :aggregate_failures do + expect(ApplicationRecord.sticking).to receive(:all_caught_up?).with(:runner, runner.id) { true } - it "picks build without tag" do - expect(execute(specific_runner)).to eq(pending_job) + expect(execute).to be_valid + expect(execute.build).to be_nil + expect(execute.build_json).to be_nil end - it "does not pick build with tag" do - pending_job.update!(tag_list: ["linux"]) - pending_job.reload - pending_job.create_queuing_entry! - expect(execute(specific_runner)).to be_falsey - end + 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 } - it "pick build without tag" do - specific_runner.update!(tag_list: ["win32"]) - expect(execute(specific_runner)).to eq(pending_job) + expect(subject).not_to be_valid + expect(subject.build).to be_nil + expect(subject.build_json).to be_nil end end - context 'deleted projects' do - before do - project.update!(pending_delete: true) - end + shared_examples 'handles runner assignment' do + context 'runner follow tag list' do + it "picks build with the same tag" do + pending_job.update!(tag_list: ["linux"]) + pending_job.reload + pending_job.create_queuing_entry! + project_runner.update!(tag_list: ["linux"]) + expect(build_on(project_runner)).to eq(pending_job) + end - context 'for shared runners' do - before do - project.update!(shared_runners_enabled: true) + it "does not pick build with different tag" do + pending_job.update!(tag_list: ["linux"]) + pending_job.reload + pending_job.create_queuing_entry! + project_runner.update!(tag_list: ["win32"]) + expect(build_on(project_runner)).to be_falsey end - it 'does not pick a build' do - expect(execute(shared_runner)).to be_nil + it "picks build without tag" do + expect(build_on(project_runner)).to eq(pending_job) end - end - context 'for specific runner' do - it 'does not pick a build' do - expect(execute(specific_runner)).to be_nil - expect(pending_job.reload).to be_failed - expect(pending_job.queuing_entry).to be_nil + it "does not pick build with tag" do + pending_job.update!(tag_list: ["linux"]) + pending_job.reload + pending_job.create_queuing_entry! + expect(build_on(project_runner)).to be_falsey end - end - end - context 'allow shared runners' do - before do - project.update!(shared_runners_enabled: true) - pipeline.reload - pending_job.reload - pending_job.create_queuing_entry! + it "pick build without tag" do + project_runner.update!(tag_list: ["win32"]) + expect(build_on(project_runner)).to eq(pending_job) + end end - context 'when build owner has been blocked' do - let(:user) { create(:user, :blocked) } - + context 'deleted projects' do before do - pending_job.update!(user: user) + project.update!(pending_delete: true) end - it 'does not pick the build and drops the build' do - expect(execute(shared_runner)).to be_falsey + context 'for shared runners' do + before do + project.update!(shared_runners_enabled: true) + end - expect(pending_job.reload).to be_user_blocked + it 'does not pick a build' do + expect(build_on(shared_runner)).to be_nil + end + end + + context 'for project runner' do + it 'does not pick a build' do + expect(build_on(project_runner)).to be_nil + expect(pending_job.reload).to be_failed + expect(pending_job.queuing_entry).to be_nil + end end end - context 'for multiple builds' do - let!(:project2) { create :project, shared_runners_enabled: true } - let!(:pipeline2) { create :ci_pipeline, project: project2 } - let!(:project3) { create :project, shared_runners_enabled: true } - let!(:pipeline3) { create :ci_pipeline, project: project3 } - let!(:build1_project1) { pending_job } - let!(:build2_project1) { create(:ci_build, :pending, :queued, pipeline: pipeline) } - let!(:build3_project1) { create(:ci_build, :pending, :queued, pipeline: pipeline) } - let!(:build1_project2) { create(:ci_build, :pending, :queued, pipeline: pipeline2) } - let!(:build2_project2) { create(:ci_build, :pending, :queued, pipeline: pipeline2) } - let!(:build1_project3) { create(:ci_build, :pending, :queued, pipeline: pipeline3) } - - it 'picks builds one-by-one' do - expect(Ci::Build).to receive(:find).with(pending_job.id).and_call_original - - expect(execute(shared_runner)).to eq(build1_project1) - end - - context 'when using fair scheduling' do - context 'when all builds are pending' do - it 'prefers projects without builds first' do - # it gets for one build from each of the projects - expect(execute(shared_runner)).to eq(build1_project1) - expect(execute(shared_runner)).to eq(build1_project2) - expect(execute(shared_runner)).to eq(build1_project3) - - # then it gets a second build from each of the projects - expect(execute(shared_runner)).to eq(build2_project1) - expect(execute(shared_runner)).to eq(build2_project2) - - # in the end the third build - expect(execute(shared_runner)).to eq(build3_project1) - end + context 'allow shared runners' do + before do + project.update!(shared_runners_enabled: true) + pipeline.reload + pending_job.reload + pending_job.create_queuing_entry! + end + + context 'when build owner has been blocked' do + let(:user) { create(:user, :blocked) } + + before do + pending_job.update!(user: user) end - context 'when some builds transition to success' do - it 'equalises number of running builds' do - # after finishing the first build for project 1, get a second build from the same project - expect(execute(shared_runner)).to eq(build1_project1) - build1_project1.reload.success - expect(execute(shared_runner)).to eq(build2_project1) + it 'does not pick the build and drops the build' do + expect(build_on(shared_runner)).to be_falsey - expect(execute(shared_runner)).to eq(build1_project2) - build1_project2.reload.success - expect(execute(shared_runner)).to eq(build2_project2) - expect(execute(shared_runner)).to eq(build1_project3) - expect(execute(shared_runner)).to eq(build3_project1) - end + expect(pending_job.reload).to be_user_blocked end end - context 'when using DEFCON mode that disables fair scheduling' do - before do - stub_feature_flags(ci_queueing_disaster_recovery_disable_fair_scheduling: true) - end - - context 'when all builds are pending' do - it 'returns builds in order of creation (FIFO)' do - # it gets for one build from each of the projects - expect(execute(shared_runner)).to eq(build1_project1) - expect(execute(shared_runner)).to eq(build2_project1) - expect(execute(shared_runner)).to eq(build3_project1) - expect(execute(shared_runner)).to eq(build1_project2) - expect(execute(shared_runner)).to eq(build2_project2) - expect(execute(shared_runner)).to eq(build1_project3) + context 'for multiple builds' do + let!(:project2) { create :project, shared_runners_enabled: true } + let!(:pipeline2) { create :ci_pipeline, project: project2 } + let!(:project3) { create :project, shared_runners_enabled: true } + let!(:pipeline3) { create :ci_pipeline, project: project3 } + let!(:build1_project1) { pending_job } + let!(:build2_project1) { create(:ci_build, :pending, :queued, pipeline: pipeline) } + let!(:build3_project1) { create(:ci_build, :pending, :queued, pipeline: pipeline) } + let!(:build1_project2) { create(:ci_build, :pending, :queued, pipeline: pipeline2) } + let!(:build2_project2) { create(:ci_build, :pending, :queued, pipeline: pipeline2) } + let!(:build1_project3) { create(:ci_build, :pending, :queued, pipeline: pipeline3) } + + it 'picks builds one-by-one' do + expect(Ci::Build).to receive(:find).with(pending_job.id).and_call_original + + expect(build_on(shared_runner)).to eq(build1_project1) + end + + context 'when using fair scheduling' do + context 'when all builds are pending' do + it 'prefers projects without builds first' do + # it gets for one build from each of the projects + expect(build_on(shared_runner)).to eq(build1_project1) + expect(build_on(shared_runner)).to eq(build1_project2) + expect(build_on(shared_runner)).to eq(build1_project3) + + # then it gets a second build from each of the projects + expect(build_on(shared_runner)).to eq(build2_project1) + expect(build_on(shared_runner)).to eq(build2_project2) + + # in the end the third build + expect(build_on(shared_runner)).to eq(build3_project1) + end + end + + context 'when some builds transition to success' do + it 'equalises number of running builds' do + # after finishing the first build for project 1, get a second build from the same project + expect(build_on(shared_runner)).to eq(build1_project1) + build1_project1.reload.success + expect(build_on(shared_runner)).to eq(build2_project1) + + expect(build_on(shared_runner)).to eq(build1_project2) + build1_project2.reload.success + expect(build_on(shared_runner)).to eq(build2_project2) + expect(build_on(shared_runner)).to eq(build1_project3) + expect(build_on(shared_runner)).to eq(build3_project1) + end end end - context 'when some builds transition to success' do - it 'returns builds in order of creation (FIFO)' do - expect(execute(shared_runner)).to eq(build1_project1) - build1_project1.reload.success - expect(execute(shared_runner)).to eq(build2_project1) + context 'when using DEFCON mode that disables fair scheduling' do + before do + stub_feature_flags(ci_queueing_disaster_recovery_disable_fair_scheduling: true) + end + + context 'when all builds are pending' do + it 'returns builds in order of creation (FIFO)' do + # it gets for one build from each of the projects + expect(build_on(shared_runner)).to eq(build1_project1) + expect(build_on(shared_runner)).to eq(build2_project1) + expect(build_on(shared_runner)).to eq(build3_project1) + expect(build_on(shared_runner)).to eq(build1_project2) + expect(build_on(shared_runner)).to eq(build2_project2) + expect(build_on(shared_runner)).to eq(build1_project3) + end + end - expect(execute(shared_runner)).to eq(build3_project1) - build2_project1.reload.success - expect(execute(shared_runner)).to eq(build1_project2) - expect(execute(shared_runner)).to eq(build2_project2) - expect(execute(shared_runner)).to eq(build1_project3) + context 'when some builds transition to success' do + it 'returns builds in order of creation (FIFO)' do + expect(build_on(shared_runner)).to eq(build1_project1) + build1_project1.reload.success + expect(build_on(shared_runner)).to eq(build2_project1) + + expect(build_on(shared_runner)).to eq(build3_project1) + build2_project1.reload.success + expect(build_on(shared_runner)).to eq(build1_project2) + expect(build_on(shared_runner)).to eq(build2_project2) + expect(build_on(shared_runner)).to eq(build1_project3) + end end end end - end - context 'shared runner' do - let(:response) { described_class.new(shared_runner).execute } - let(:build) { response.build } + context 'shared runner' do + let(:response) { described_class.new(shared_runner, nil).execute } + let(:build) { response.build } - it { expect(build).to be_kind_of(Build) } - it { expect(build).to be_valid } - it { expect(build).to be_running } - it { expect(build.runner).to eq(shared_runner) } - it { expect(Gitlab::Json.parse(response.build_json)['id']).to eq(build.id) } - end + it { expect(build).to be_kind_of(Build) } + it { expect(build).to be_valid } + it { expect(build).to be_running } + it { expect(build.runner).to eq(shared_runner) } + it { expect(Gitlab::Json.parse(response.build_json)['id']).to eq(build.id) } + end - context 'specific runner' do - let(:build) { execute(specific_runner) } + context 'project runner' do + let(:build) { build_on(project_runner) } - it { expect(build).to be_kind_of(Build) } - it { expect(build).to be_valid } - it { expect(build).to be_running } - it { expect(build.runner).to eq(specific_runner) } + it { expect(build).to be_kind_of(Build) } + it { expect(build).to be_valid } + it { expect(build).to be_running } + it { expect(build.runner).to eq(project_runner) } + end end - end - context 'disallow shared runners' do - before do - project.update!(shared_runners_enabled: false) - end + context 'disallow shared runners' do + before do + project.update!(shared_runners_enabled: false) + end - context 'shared runner' do - let(:build) { execute(shared_runner) } + context 'shared runner' do + let(:build) { build_on(shared_runner) } - it { expect(build).to be_nil } - end + it { expect(build).to be_nil } + end - context 'specific runner' do - let(:build) { execute(specific_runner) } + context 'project runner' do + let(:build) { build_on(project_runner) } - it { expect(build).to be_kind_of(Build) } - it { expect(build).to be_valid } - it { expect(build).to be_running } - it { expect(build.runner).to eq(specific_runner) } + it { expect(build).to be_kind_of(Build) } + it { expect(build).to be_valid } + it { expect(build).to be_running } + it { expect(build.runner).to eq(project_runner) } + end end - end - context 'disallow when builds are disabled' do - before do - project.update!(shared_runners_enabled: true, group_runners_enabled: true) - project.project_feature.update_attribute(:builds_access_level, ProjectFeature::DISABLED) + context 'disallow when builds are disabled' do + before do + project.update!(shared_runners_enabled: true, group_runners_enabled: true) + project.project_feature.update_attribute(:builds_access_level, ProjectFeature::DISABLED) - pending_job.reload.create_queuing_entry! - end + pending_job.reload.create_queuing_entry! + end - context 'and uses shared runner' do - let(:build) { execute(shared_runner) } + context 'and uses shared runner' do + let(:build) { build_on(shared_runner) } - it { expect(build).to be_nil } - end + it { expect(build).to be_nil } + end - context 'and uses group runner' do - let(:build) { execute(group_runner) } + context 'and uses group runner' do + let(:build) { build_on(group_runner) } - it { expect(build).to be_nil } - end + it { expect(build).to be_nil } + end - context 'and uses project runner' do - let(:build) { execute(specific_runner) } + context 'and uses project runner' do + let(:build) { build_on(project_runner) } - it 'does not pick a build' do - expect(build).to be_nil - expect(pending_job.reload).to be_failed - expect(pending_job.queuing_entry).to be_nil + it 'does not pick a build' do + expect(build).to be_nil + expect(pending_job.reload).to be_failed + expect(pending_job.queuing_entry).to be_nil + end end end - end - context 'allow group runners' do - before do - project.update!(group_runners_enabled: true) - end + context 'allow group runners' do + before do + project.update!(group_runners_enabled: true) + end - context 'for multiple builds' do - let!(:project2) { create(:project, group_runners_enabled: true, group: group) } - let!(:pipeline2) { create(:ci_pipeline, project: project2) } - let!(:project3) { create(:project, group_runners_enabled: true, group: group) } - let!(:pipeline3) { create(:ci_pipeline, project: project3) } + context 'for multiple builds' do + let!(:project2) { create(:project, group_runners_enabled: true, group: group) } + let!(:pipeline2) { create(:ci_pipeline, project: project2) } + let!(:project3) { create(:project, group_runners_enabled: true, group: group) } + let!(:pipeline3) { create(:ci_pipeline, project: project3) } - let!(:build1_project1) { pending_job } - let!(:build2_project1) { create(:ci_build, :queued, pipeline: pipeline) } - let!(:build3_project1) { create(:ci_build, :queued, pipeline: pipeline) } - let!(:build1_project2) { create(:ci_build, :queued, pipeline: pipeline2) } - let!(:build2_project2) { create(:ci_build, :queued, pipeline: pipeline2) } - let!(:build1_project3) { create(:ci_build, :queued, pipeline: pipeline3) } + let!(:build1_project1) { pending_job } + let!(:build2_project1) { create(:ci_build, :queued, pipeline: pipeline) } + let!(:build3_project1) { create(:ci_build, :queued, pipeline: pipeline) } + let!(:build1_project2) { create(:ci_build, :queued, pipeline: pipeline2) } + let!(:build2_project2) { create(:ci_build, :queued, pipeline: pipeline2) } + let!(:build1_project3) { create(:ci_build, :queued, pipeline: pipeline3) } - # these shouldn't influence the scheduling - let!(:unrelated_group) { create(:group) } - let!(:unrelated_project) { create(:project, group_runners_enabled: true, group: unrelated_group) } - let!(:unrelated_pipeline) { create(:ci_pipeline, project: unrelated_project) } - let!(:build1_unrelated_project) { create(:ci_build, :pending, :queued, pipeline: unrelated_pipeline) } - let!(:unrelated_group_runner) { create(:ci_runner, :group, groups: [unrelated_group]) } + # these shouldn't influence the scheduling + let!(:unrelated_group) { create(:group) } + let!(:unrelated_project) { create(:project, group_runners_enabled: true, group: unrelated_group) } + let!(:unrelated_pipeline) { create(:ci_pipeline, project: unrelated_project) } + let!(:build1_unrelated_project) { create(:ci_build, :pending, :queued, pipeline: unrelated_pipeline) } + let!(:unrelated_group_runner) { create(:ci_runner, :group, groups: [unrelated_group]) } - it 'does not consider builds from other group runners' do - queue = ::Ci::Queue::BuildQueueService.new(group_runner) + it 'does not consider builds from other group runners' do + queue = ::Ci::Queue::BuildQueueService.new(group_runner) - expect(queue.builds_for_group_runner.size).to eq 6 - execute(group_runner) + expect(queue.builds_for_group_runner.size).to eq 6 + build_on(group_runner) - expect(queue.builds_for_group_runner.size).to eq 5 - execute(group_runner) + expect(queue.builds_for_group_runner.size).to eq 5 + build_on(group_runner) - expect(queue.builds_for_group_runner.size).to eq 4 - execute(group_runner) + expect(queue.builds_for_group_runner.size).to eq 4 + build_on(group_runner) - expect(queue.builds_for_group_runner.size).to eq 3 - execute(group_runner) + expect(queue.builds_for_group_runner.size).to eq 3 + build_on(group_runner) - expect(queue.builds_for_group_runner.size).to eq 2 - execute(group_runner) + expect(queue.builds_for_group_runner.size).to eq 2 + build_on(group_runner) - expect(queue.builds_for_group_runner.size).to eq 1 - execute(group_runner) + expect(queue.builds_for_group_runner.size).to eq 1 + build_on(group_runner) - expect(queue.builds_for_group_runner.size).to eq 0 - expect(execute(group_runner)).to be_nil + expect(queue.builds_for_group_runner.size).to eq 0 + expect(build_on(group_runner)).to be_nil + end end - end - context 'group runner' do - let(:build) { execute(group_runner) } + context 'group runner' do + let(:build) { build_on(group_runner) } - it { expect(build).to be_kind_of(Build) } - it { expect(build).to be_valid } - it { expect(build).to be_running } - it { expect(build.runner).to eq(group_runner) } + it { expect(build).to be_kind_of(Build) } + it { expect(build).to be_valid } + it { expect(build).to be_running } + it { expect(build.runner).to eq(group_runner) } + end end - end - context 'disallow group runners' do - before do - project.update!(group_runners_enabled: false) + context 'disallow group runners' do + before do + project.update!(group_runners_enabled: false) - pending_job.reload.create_queuing_entry! - end + pending_job.reload.create_queuing_entry! + end - context 'group runner' do - let(:build) { execute(group_runner) } + context 'group runner' do + let(:build) { build_on(group_runner) } - it { expect(build).to be_nil } + it { expect(build).to be_nil } + end end - end - context 'when first build is stalled' do - before do - allow_any_instance_of(Ci::RegisterJobService).to receive(:assign_runner!).and_call_original - allow_any_instance_of(Ci::RegisterJobService).to receive(:assign_runner!) - .with(pending_job, anything).and_raise(ActiveRecord::StaleObjectError) - end + context 'when first build is stalled' do + before do + allow_any_instance_of(Ci::RegisterJobService).to receive(:assign_runner!).and_call_original + allow_any_instance_of(Ci::RegisterJobService).to receive(:assign_runner!) + .with(pending_job, anything).and_raise(ActiveRecord::StaleObjectError) + end - subject { described_class.new(specific_runner).execute } + subject { described_class.new(project_runner, nil).execute } - context 'with multiple builds are in queue' do - let!(:other_build) { create(:ci_build, :pending, :queued, pipeline: pipeline) } + context 'with multiple builds are in queue' do + let!(:other_build) { create(:ci_build, :pending, :queued, pipeline: pipeline) } - before do - allow_any_instance_of(::Ci::Queue::BuildQueueService) - .to receive(:execute) - .and_return(Ci::Build.where(id: [pending_job, other_build]).pluck(:id)) - end + before do + allow_any_instance_of(::Ci::Queue::BuildQueueService) + .to receive(:execute) + .and_return(Ci::Build.where(id: [pending_job, other_build]).pluck(:id)) + end - it "receives second build from the queue" do - expect(subject).to be_valid - expect(subject.build).to eq(other_build) + it "receives second build from the queue" do + expect(subject).to be_valid + expect(subject.build).to eq(other_build) + end end - end - context 'when single build is in queue' do - before do - allow_any_instance_of(::Ci::Queue::BuildQueueService) - .to receive(:execute) - .and_return(Ci::Build.where(id: pending_job).pluck(:id)) - end + context 'when single build is in queue' do + before do + allow_any_instance_of(::Ci::Queue::BuildQueueService) + .to receive(:execute) + .and_return(Ci::Build.where(id: pending_job).pluck(:id)) + end - it "does not receive any valid result" do - expect(subject).not_to be_valid + it "does not receive any valid result" do + expect(subject).not_to be_valid + end end - end - context 'when there is no build in queue' do - before do - allow_any_instance_of(::Ci::Queue::BuildQueueService) - .to receive(:execute) - .and_return([]) - end + context 'when there is no build in queue' do + before do + allow_any_instance_of(::Ci::Queue::BuildQueueService) + .to receive(:execute) + .and_return([]) + end - it "does not receive builds but result is valid" do - expect(subject).to be_valid - expect(subject.build).to be_nil + it "does not receive builds but result is valid" do + expect(subject).to be_valid + expect(subject.build).to be_nil + end end end - end - context 'when access_level of runner is not_protected' do - let!(:specific_runner) { create(:ci_runner, :project, projects: [project]) } + context 'when access_level of runner is not_protected' do + let!(:project_runner) { create(:ci_runner, :project, projects: [project]) } - context 'when a job is protected' do - let!(:pending_job) { create(:ci_build, :pending, :queued, :protected, pipeline: pipeline) } + context 'when a job is protected' do + let!(:pending_job) { create(:ci_build, :pending, :queued, :protected, pipeline: pipeline) } - it 'picks the job' do - expect(execute(specific_runner)).to eq(pending_job) + it 'picks the job' do + expect(build_on(project_runner)).to eq(pending_job) + end end - end - context 'when a job is unprotected' do - let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) } + context 'when a job is unprotected' do + let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) } - it 'picks the job' do - expect(execute(specific_runner)).to eq(pending_job) + it 'picks the job' do + expect(build_on(project_runner)).to eq(pending_job) + end end - end - context 'when protected attribute of a job is nil' do - let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) } + context 'when protected attribute of a job is nil' do + let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) } - before do - pending_job.update_attribute(:protected, nil) - end + before do + pending_job.update_attribute(:protected, nil) + end - it 'picks the job' do - expect(execute(specific_runner)).to eq(pending_job) + it 'picks the job' do + expect(build_on(project_runner)).to eq(pending_job) + end end end - end - context 'when access_level of runner is ref_protected' do - let!(:specific_runner) { create(:ci_runner, :project, :ref_protected, projects: [project]) } + context 'when access_level of runner is ref_protected' do + let!(:project_runner) { create(:ci_runner, :project, :ref_protected, projects: [project]) } - context 'when a job is protected' do - let!(:pending_job) { create(:ci_build, :pending, :queued, :protected, pipeline: pipeline) } + context 'when a job is protected' do + let!(:pending_job) { create(:ci_build, :pending, :queued, :protected, pipeline: pipeline) } - it 'picks the job' do - expect(execute(specific_runner)).to eq(pending_job) + it 'picks the job' do + expect(build_on(project_runner)).to eq(pending_job) + end end - end - context 'when a job is unprotected' do - let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) } + context 'when a job is unprotected' do + let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) } - it 'does not pick the job' do - expect(execute(specific_runner)).to be_nil + it 'does not pick the job' do + expect(build_on(project_runner)).to be_nil + end end - end - context 'when protected attribute of a job is nil' do - let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) } + context 'when protected attribute of a job is nil' do + let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) } - before do - pending_job.update_attribute(:protected, nil) - end + before do + pending_job.update_attribute(:protected, nil) + end - it 'does not pick the job' do - expect(execute(specific_runner)).to be_nil + it 'does not pick the job' do + expect(build_on(project_runner)).to be_nil + end end end - end - context 'runner feature set is verified' do - let(:options) { { artifacts: { reports: { junit: "junit.xml" } } } } - let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline, options: options) } + context 'runner feature set is verified' do + let(:options) { { artifacts: { reports: { junit: "junit.xml" } } } } + let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline, options: options) } - subject { execute(specific_runner, params) } + subject { build_on(project_runner, params: params) } - context 'when feature is missing by runner' do - let(:params) { {} } + context 'when feature is missing by runner' do + let(:params) { {} } - it 'does not pick the build and drops the build' do - expect(subject).to be_nil - expect(pending_job.reload).to be_failed - expect(pending_job).to be_runner_unsupported + it 'does not pick the build and drops the build' do + expect(subject).to be_nil + expect(pending_job.reload).to be_failed + expect(pending_job).to be_runner_unsupported + end end - end - context 'when feature is supported by runner' do - let(:params) do - { info: { features: { upload_multiple_artifacts: true } } } - end + context 'when feature is supported by runner' do + let(:params) do + { info: { features: { upload_multiple_artifacts: true } } } + end - it 'does pick job' do - expect(subject).not_to be_nil + it 'does pick job' do + expect(subject).not_to be_nil + end end end - end - - context 'when "dependencies" keyword is specified' do - let!(:pre_stage_job) do - create(:ci_build, :success, :artifacts, pipeline: pipeline, name: 'test', stage_idx: 0) - end - let!(:pending_job) do - create(:ci_build, :pending, :queued, - pipeline: pipeline, stage_idx: 1, - options: { script: ["bash"], dependencies: dependencies }) - end + context 'when "dependencies" keyword is specified' do + let!(:pre_stage_job) do + create(:ci_build, :success, :artifacts, pipeline: pipeline, name: 'test', stage_idx: 0) + end - let(:dependencies) { %w[test] } + let!(:pending_job) do + create(:ci_build, :pending, :queued, + pipeline: pipeline, stage_idx: 1, + options: { script: ["bash"], dependencies: dependencies }) + end - subject { execute(specific_runner) } + let(:dependencies) { %w[test] } - it 'picks a build with a dependency' do - picked_build = execute(specific_runner) + subject { build_on(project_runner) } - expect(picked_build).to be_present - end + it 'picks a build with a dependency' do + picked_build = build_on(project_runner) - context 'when there are multiple dependencies with artifacts' do - let!(:pre_stage_job_second) do - create(:ci_build, :success, :artifacts, pipeline: pipeline, name: 'deploy', stage_idx: 0) + expect(picked_build).to be_present end - let(:dependencies) { %w[test deploy] } - - it 'logs build artifacts size' do - execute(specific_runner) - - artifacts_size = [pre_stage_job, pre_stage_job_second].sum do |job| - job.job_artifacts_archive.size + context 'when there are multiple dependencies with artifacts' do + let!(:pre_stage_job_second) do + create(:ci_build, :success, :artifacts, pipeline: pipeline, name: 'deploy', stage_idx: 0) end - expect(artifacts_size).to eq 107464 * 2 - expect(Gitlab::ApplicationContext.current).to include({ - 'meta.artifacts_dependencies_size' => artifacts_size, - 'meta.artifacts_dependencies_count' => 2 - }) - end - end + let(:dependencies) { %w[test deploy] } - shared_examples 'not pick' do - it 'does not pick the build and drops the build' do - expect(subject).to be_nil - expect(pending_job.reload).to be_failed - expect(pending_job).to be_missing_dependency_failure - end - end + it 'logs build artifacts size' do + build_on(project_runner) - shared_examples 'validation is active' do - context 'when depended job has not been completed yet' do - let!(:pre_stage_job) { create(:ci_build, :pending, :queued, :manual, pipeline: pipeline, name: 'test', stage_idx: 0) } + artifacts_size = [pre_stage_job, pre_stage_job_second].sum do |job| + job.job_artifacts_archive.size + end - it { is_expected.to eq(pending_job) } + expect(artifacts_size).to eq 107464 * 2 + expect(Gitlab::ApplicationContext.current).to include({ + 'meta.artifacts_dependencies_size' => artifacts_size, + 'meta.artifacts_dependencies_count' => 2 + }) + end end - context 'when artifacts of depended job has been expired' do - let!(:pre_stage_job) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) } + shared_examples 'not pick' do + it 'does not pick the build and drops the build' do + expect(subject).to be_nil + expect(pending_job.reload).to be_failed + expect(pending_job).to be_missing_dependency_failure + end + end - context 'when the pipeline is locked' do - before do - pipeline.artifacts_locked! + shared_examples 'validation is active' do + context 'when depended job has not been completed yet' do + let!(:pre_stage_job) do + create(:ci_build, :pending, :queued, :manual, pipeline: pipeline, name: 'test', stage_idx: 0) end it { is_expected.to eq(pending_job) } end - context 'when the pipeline is unlocked' do - before do - pipeline.unlocked! + context 'when artifacts of depended job has been expired' do + let!(:pre_stage_job) do + create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) end - it_behaves_like 'not pick' + context 'when the pipeline is locked' do + before do + pipeline.artifacts_locked! + end + + it { is_expected.to eq(pending_job) } + end + + context 'when the pipeline is unlocked' do + before do + pipeline.unlocked! + end + + it_behaves_like 'not pick' + end end - end - 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) } + context 'when artifacts of depended job has been erased' do + let!(:pre_stage_job) do + create(:ci_build, :success, pipeline: pipeline, name: 'test', stage_idx: 0, erased_at: 1.minute.ago) + end - it_behaves_like 'not pick' - end + it_behaves_like 'not pick' + end - context 'when job object is staled' do - let!(:pre_stage_job) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) } + context 'when job object is staled' do + let!(:pre_stage_job) do + create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) + end - before do - pipeline.unlocked! + before do + pipeline.unlocked! - allow_next_instance_of(Ci::Build) do |build| - expect(build).to receive(:drop!) - .and_raise(ActiveRecord::StaleObjectError.new(pending_job, :drop!)) + allow_next_instance_of(Ci::Build) do |build| + expect(build).to receive(:drop!) + .and_raise(ActiveRecord::StaleObjectError.new(pending_job, :drop!)) + end end - end - it 'does not drop nor pick' do - expect(subject).to be_nil + it 'does not drop nor pick' do + expect(subject).to be_nil + end end end - end - shared_examples 'validation is not active' do - context 'when depended job has not been completed yet' do - let!(:pre_stage_job) { create(:ci_build, :pending, :queued, :manual, pipeline: pipeline, name: 'test', stage_idx: 0) } + shared_examples 'validation is not active' do + context 'when depended job has not been completed yet' do + let!(:pre_stage_job) do + create(:ci_build, :pending, :queued, :manual, pipeline: pipeline, name: 'test', stage_idx: 0) + end - it { expect(subject).to eq(pending_job) } - end + it { expect(subject).to eq(pending_job) } + end - context 'when artifacts of depended job has been expired' do - let!(:pre_stage_job) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) } + context 'when artifacts of depended job has been expired' do + let!(:pre_stage_job) do + create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) + end - it { expect(subject).to eq(pending_job) } - end + it { expect(subject).to eq(pending_job) } + end - 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) } + context 'when artifacts of depended job has been erased' do + let!(:pre_stage_job) do + create(:ci_build, :success, pipeline: pipeline, name: 'test', stage_idx: 0, erased_at: 1.minute.ago) + end - it { expect(subject).to eq(pending_job) } + it { expect(subject).to eq(pending_job) } + end end - end - it_behaves_like 'validation is active' - end + it_behaves_like 'validation is active' + end - context 'when build is degenerated' do - let!(:pending_job) { create(:ci_build, :pending, :queued, :degenerated, pipeline: pipeline) } + context 'when build is degenerated' do + let!(:pending_job) { create(:ci_build, :pending, :queued, :degenerated, pipeline: pipeline) } - subject { execute(specific_runner, {}) } + subject { build_on(project_runner) } - it 'does not pick the build and drops the build' do - expect(subject).to be_nil + it 'does not pick the build and drops the build' do + expect(subject).to be_nil - pending_job.reload - expect(pending_job).to be_failed - expect(pending_job).to be_archived_failure + pending_job.reload + expect(pending_job).to be_failed + expect(pending_job).to be_archived_failure + end end - end - context 'when build has data integrity problem' do - let!(:pending_job) do - create(:ci_build, :pending, :queued, pipeline: pipeline) - end + context 'when build has data integrity problem' do + let!(:pending_job) do + create(:ci_build, :pending, :queued, pipeline: pipeline) + end - before do - pending_job.update_columns(options: "string") - end + before do + pending_job.update_columns(options: "string") + end - subject { execute(specific_runner, {}) } + subject { build_on(project_runner) } - it 'does drop the build and logs both failures' do - expect(Gitlab::ErrorTracking).to receive(:track_exception) - .with(anything, a_hash_including(build_id: pending_job.id)) - .twice - .and_call_original + it 'does drop the build and logs both failures' do + expect(Gitlab::ErrorTracking).to receive(:track_exception) + .with(anything, a_hash_including(build_id: pending_job.id)) + .twice + .and_call_original - expect(subject).to be_nil + expect(subject).to be_nil - pending_job.reload - expect(pending_job).to be_failed - expect(pending_job).to be_data_integrity_failure + pending_job.reload + expect(pending_job).to be_failed + expect(pending_job).to be_data_integrity_failure + end end - end - context 'when build fails to be run!' do - let!(:pending_job) do - create(:ci_build, :pending, :queued, pipeline: pipeline) - end + context 'when build fails to be run!' do + let!(:pending_job) do + create(:ci_build, :pending, :queued, pipeline: pipeline) + end - before do - expect_any_instance_of(Ci::Build).to receive(:run!) - .and_raise(RuntimeError, 'scheduler error') - end + before do + expect_any_instance_of(Ci::Build).to receive(:run!) + .and_raise(RuntimeError, 'scheduler error') + end - subject { execute(specific_runner, {}) } + subject { build_on(project_runner) } - it 'does drop the build and logs failure' do - expect(Gitlab::ErrorTracking).to receive(:track_exception) - .with(anything, a_hash_including(build_id: pending_job.id)) - .once - .and_call_original + it 'does drop the build and logs failure' do + expect(Gitlab::ErrorTracking).to receive(:track_exception) + .with(anything, a_hash_including(build_id: pending_job.id)) + .once + .and_call_original - expect(subject).to be_nil + expect(subject).to be_nil - pending_job.reload - expect(pending_job).to be_failed - expect(pending_job).to be_scheduler_failure + pending_job.reload + expect(pending_job).to be_failed + expect(pending_job).to be_scheduler_failure + end end - end - context 'when an exception is raised during a persistent ref creation' do - before do - allow_any_instance_of(Ci::PersistentRef).to receive(:exist?) { false } - allow_any_instance_of(Ci::PersistentRef).to receive(:create_ref) { raise ArgumentError } - end + context 'when an exception is raised during a persistent ref creation' do + before do + allow_any_instance_of(Ci::PersistentRef).to receive(:exist?) { false } + allow_any_instance_of(Ci::PersistentRef).to receive(:create_ref) { raise ArgumentError } + end - subject { execute(specific_runner, {}) } + subject { build_on(project_runner) } - it 'picks the build' do - expect(subject).to eq(pending_job) + it 'picks the build' do + expect(subject).to eq(pending_job) - pending_job.reload - expect(pending_job).to be_running - end - end - - context 'when only some builds can be matched by runner' do - let!(:specific_runner) { create(:ci_runner, :project, projects: [project], tag_list: %w[matching]) } - let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline, tag_list: %w[matching]) } - - before do - # create additional matching and non-matching jobs - create_list(:ci_build, 2, :pending, :queued, pipeline: pipeline, tag_list: %w[matching]) - create(:ci_build, :pending, :queued, pipeline: pipeline, tag_list: %w[non-matching]) + pending_job.reload + expect(pending_job).to be_running + end end - it 'observes queue size of only matching jobs' do - # pending_job + 2 x matching ones - expect(Gitlab::Ci::Queue::Metrics.queue_size_total).to receive(:observe) - .with({ runner_type: specific_runner.runner_type }, 3) + context 'when only some builds can be matched by runner' do + let!(:project_runner) { create(:ci_runner, :project, projects: [project], tag_list: %w[matching]) } + let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline, tag_list: %w[matching]) } - expect(execute(specific_runner)).to eq(pending_job) - end + before do + # create additional matching and non-matching jobs + create_list(:ci_build, 2, :pending, :queued, pipeline: pipeline, tag_list: %w[matching]) + create(:ci_build, :pending, :queued, pipeline: pipeline, tag_list: %w[non-matching]) + end - it 'observes queue processing time by the runner type' do - expect(Gitlab::Ci::Queue::Metrics.queue_iteration_duration_seconds) - .to receive(:observe) - .with({ runner_type: specific_runner.runner_type }, anything) + it 'observes queue size of only matching jobs' do + # pending_job + 2 x matching ones + expect(Gitlab::Ci::Queue::Metrics.queue_size_total).to receive(:observe) + .with({ runner_type: project_runner.runner_type }, 3) - expect(Gitlab::Ci::Queue::Metrics.queue_retrieval_duration_seconds) - .to receive(:observe) - .with({ runner_type: specific_runner.runner_type }, anything) + expect(build_on(project_runner)).to eq(pending_job) + end - expect(execute(specific_runner)).to eq(pending_job) - end - end + it 'observes queue processing time by the runner type' do + expect(Gitlab::Ci::Queue::Metrics.queue_iteration_duration_seconds) + .to receive(:observe) + .with({ runner_type: project_runner.runner_type }, anything) - context 'when ci_register_job_temporary_lock is enabled' do - before do - stub_feature_flags(ci_register_job_temporary_lock: true) + expect(Gitlab::Ci::Queue::Metrics.queue_retrieval_duration_seconds) + .to receive(:observe) + .with({ runner_type: project_runner.runner_type }, anything) - allow(Gitlab::Ci::Queue::Metrics.queue_operations_total).to receive(:increment) + expect(build_on(project_runner)).to eq(pending_job) + end end - context 'when a build is temporarily locked' do - let(:service) { described_class.new(specific_runner) } - + context 'when ci_register_job_temporary_lock is enabled' do before do - service.send(:acquire_temporary_lock, pending_job.id) - end - - it 'skips this build and marks queue as invalid' do - expect(Gitlab::Ci::Queue::Metrics.queue_operations_total).to receive(:increment) - .with(operation: :queue_iteration) - expect(Gitlab::Ci::Queue::Metrics.queue_operations_total).to receive(:increment) - .with(operation: :build_temporary_locked) + stub_feature_flags(ci_register_job_temporary_lock: true) - expect(service.execute).not_to be_valid + allow(Gitlab::Ci::Queue::Metrics.queue_operations_total).to receive(:increment) end - context 'when there is another build in queue' do - let!(:next_pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) } + context 'when a build is temporarily locked' do + let(:service) { described_class.new(project_runner, nil) } - it 'skips this build and picks another build' do + before do + service.send(:acquire_temporary_lock, pending_job.id) + end + + it 'skips this build and marks queue as invalid' do expect(Gitlab::Ci::Queue::Metrics.queue_operations_total).to receive(:increment) - .with(operation: :queue_iteration).twice + .with(operation: :queue_iteration) expect(Gitlab::Ci::Queue::Metrics.queue_operations_total).to receive(:increment) .with(operation: :build_temporary_locked) - result = service.execute + expect(service.execute).not_to be_valid + end + + context 'when there is another build in queue' do + let!(:next_pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) } + + it 'skips this build and picks another build' do + expect(Gitlab::Ci::Queue::Metrics.queue_operations_total).to receive(:increment) + .with(operation: :queue_iteration).twice + expect(Gitlab::Ci::Queue::Metrics.queue_operations_total).to receive(:increment) + .with(operation: :build_temporary_locked) - expect(result.build).to eq(next_pending_job) - expect(result).to be_valid + result = service.execute + + expect(result.build).to eq(next_pending_job) + expect(result).to be_valid + end end end end end - end - - context 'when using pending builds table' do - include_examples 'handles runner assignment' - context 'when a conflicting data is stored in denormalized table' do - let!(:specific_runner) { create(:ci_runner, :project, projects: [project], tag_list: %w[conflict]) } - let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline, tag_list: %w[conflict]) } + context 'when using pending builds table' do + include_examples 'handles runner assignment' - before do - pending_job.update_column(:status, :running) - end + context 'when a conflicting data is stored in denormalized table' do + let!(:runner) { create(:ci_runner, :project, projects: [project], tag_list: %w[conflict]) } + let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline, tag_list: %w[conflict]) } - it 'removes queuing entry upon build assignment attempt' do - expect(pending_job.reload).to be_running - expect(pending_job.queuing_entry).to be_present + before do + pending_job.update_column(:status, :running) + end - result = described_class.new(specific_runner).execute + it 'removes queuing entry upon build assignment attempt' do + expect(pending_job.reload).to be_running + expect(pending_job.queuing_entry).to be_present - expect(result).not_to be_valid - expect(pending_job.reload.queuing_entry).not_to be_present + expect(execute).not_to be_valid + expect(pending_job.reload.queuing_entry).not_to be_present + end end end end @@ -807,11 +842,11 @@ module Ci # Stub tested metrics allow(Gitlab::Ci::Queue::Metrics) .to receive(:attempt_counter) - .and_return(attempt_counter) + .and_return(attempt_counter) allow(Gitlab::Ci::Queue::Metrics) .to receive(:job_queue_duration_seconds) - .and_return(job_queue_duration_seconds) + .and_return(job_queue_duration_seconds) project.update!(shared_runners_enabled: true) pending_job.update!(created_at: current_time - 3600, queued_at: current_time - 1800) @@ -822,7 +857,7 @@ module Ci allow(job_queue_duration_seconds).to receive(:observe) expect(attempt_counter).to receive(:increment) - execute(runner) + build_on(runner) end end @@ -834,7 +869,7 @@ module Ci jobs_running_for_project: expected_jobs_running_for_project_first_job, shard: expected_shard }, 1800) - execute(runner) + build_on(runner) end context 'when project already has running jobs' do @@ -854,7 +889,7 @@ module Ci jobs_running_for_project: expected_jobs_running_for_project_third_job, shard: expected_shard }, 1800) - execute(runner) + build_on(runner) end end end @@ -913,12 +948,12 @@ module Ci allow(attempt_counter).to receive(:increment) expect(job_queue_duration_seconds).not_to receive(:observe) - execute(runner) + build_on(runner) end end end - context 'when specific runner is used' do + context 'when project runner is used' do let(:runner) { create(:ci_runner, :project, projects: [project], tag_list: %w(tag1 metrics_shard::shard_tag tag2)) } let(:expected_shared_runner) { false } let(:expected_shard) { ::Gitlab::Ci::Queue::Metrics::DEFAULT_METRICS_SHARD } @@ -933,12 +968,12 @@ module Ci it 'present sets runner session configuration in the build' do runner_session_params = { session: { 'url' => 'https://example.com' } } - expect(execute(specific_runner, runner_session_params).runner_session.attributes) + expect(build_on(project_runner, params: runner_session_params).runner_session.attributes) .to include(runner_session_params[:session]) end it 'not present it does not configure the runner session' do - expect(execute(specific_runner).runner_session).to be_nil + expect(build_on(project_runner).runner_session).to be_nil end end @@ -954,7 +989,7 @@ module Ci it 'returns 409 conflict' do expect(Ci::Build.pending.unstarted.count).to eq 3 - result = described_class.new(specific_runner).execute + result = described_class.new(project_runner, nil).execute expect(result).not_to be_valid expect(result.build).to be_nil @@ -962,8 +997,8 @@ module Ci end end - def execute(runner, params = {}) - described_class.new(runner).execute(params).build + def build_on(runner, runner_machine: nil, params: {}) + described_class.new(runner, runner_machine).execute(params).build end end end diff --git a/spec/services/ci/retry_job_service_spec.rb b/spec/services/ci/retry_job_service_spec.rb index c3d80f2cb56..10acf032b1a 100644 --- a/spec/services/ci/retry_job_service_spec.rb +++ b/spec/services/ci/retry_job_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::RetryJobService do +RSpec.describe Ci::RetryJobService, feature_category: :continuous_integration do using RSpec::Parameterized::TableSyntax let_it_be(:reporter) { create(:user) } let_it_be(:developer) { create(:user) } @@ -27,6 +27,22 @@ 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) + end + end + shared_context 'retryable build' do let_it_be_with_reload(:job) do create(:ci_build, :success, pipeline: pipeline, ci_stage: stage) @@ -102,6 +118,14 @@ RSpec.describe Ci::RetryJobService do end end + shared_examples_for 'does not retry the job' do + it 'returns :not_retryable and :unprocessable_entity' do + expect(subject.message).to be('Job cannot be retried') + expect(subject.payload[:reason]).to eq(:not_retryable) + expect(subject.payload[:job]).to eq(job) + end + end + shared_examples_for 'retries the job' do it_behaves_like 'clones the job' @@ -189,6 +213,20 @@ 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' @@ -287,7 +325,33 @@ RSpec.describe Ci::RetryJobService do subject { service.execute(job) } + context 'when the job to be retried is a bridge' do + context 'and it is not retryable' do + let_it_be(:job) { create(:ci_bridge, :failed, :reached_max_descendant_pipelines_depth) } + + it_behaves_like 'does not retry the job' + end + + include_context 'retryable bridge' + + it_behaves_like 'retries 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 retried is a build' do + context 'and it is not retryable' do + let_it_be(:job) { create(:ci_build, :deployment_rejected, pipeline: pipeline) } + + it_behaves_like 'does not retry the job' + end + include_context 'retryable build' it_behaves_like 'retries the job' diff --git a/spec/services/ci/runners/create_runner_service_spec.rb b/spec/services/ci/runners/create_runner_service_spec.rb new file mode 100644 index 00000000000..673bf3ef90e --- /dev/null +++ b/spec/services/ci/runners/create_runner_service_spec.rb @@ -0,0 +1,135 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Ci::Runners::CreateRunnerService, "#execute", feature_category: :runner_fleet do + subject(:execute) { described_class.new(user: current_user, type: type, params: params).execute } + + let(:runner) { execute.payload[:runner] } + + let_it_be(:admin) { create(:admin) } + let_it_be(:non_admin_user) { create(:user) } + let_it_be(:anonymous) { nil } + + shared_context 'when admin user' do + let(:current_user) { admin } + + before do + allow(current_user).to receive(:can?).with(:create_instance_runners).and_return true + end + end + + shared_examples 'it can create a runner' do + it 'creates a runner of the specified type' do + expect(runner.runner_type).to eq expected_type + end + + context 'with default params provided' do + let(:args) do + {} + end + + before do + params.merge!(args) + end + + it { is_expected.to be_success } + + it 'uses default values when none are provided' do + expect(runner).to be_an_instance_of(::Ci::Runner) + expect(runner.persisted?).to be_truthy + expect(runner.run_untagged).to be true + expect(runner.active).to be true + expect(runner.creator).to be current_user + expect(runner.authenticated_user_registration_type?).to be_truthy + expect(runner.runner_type).to eq 'instance_type' + end + end + + context 'with non-default params provided' do + let(:args) do + { + description: 'some description', + maintenance_note: 'a note', + paused: true, + tag_list: %w[tag1 tag2], + access_level: 'ref_protected', + locked: true, + maximum_timeout: 600, + run_untagged: false + } + end + + before do + params.merge!(args) + end + + it { is_expected.to be_success } + + it 'creates runner with specified values', :aggregate_failures do + expect(runner).to be_an_instance_of(::Ci::Runner) + expect(runner.description).to eq 'some description' + expect(runner.maintenance_note).to eq 'a note' + expect(runner.active).to eq !args[:paused] + expect(runner.locked).to eq args[:locked] + expect(runner.run_untagged).to eq args[:run_untagged] + expect(runner.tags).to contain_exactly( + an_object_having_attributes(name: 'tag1'), + an_object_having_attributes(name: 'tag2') + ) + expect(runner.access_level).to eq args[:access_level] + expect(runner.maximum_timeout).to eq args[:maximum_timeout] + + expect(runner.authenticated_user_registration_type?).to be_truthy + expect(runner.runner_type).to eq 'instance_type' + end + end + end + + shared_examples 'it cannot create a runner' do + it 'runner payload is nil' do + expect(runner).to be nil + end + + it { is_expected.to be_error } + end + + shared_examples 'it can return an error' do + let(:group) { create(:group) } + let(:runner_double) { Ci::Runner.new } + + context 'when the runner fails to save' do + before do + allow(Ci::Runner).to receive(:new).and_return runner_double + end + + it_behaves_like 'it cannot create a runner' + + it 'returns error message' do + expect(execute.errors).not_to be_empty + end + end + end + + context 'with type param set to nil' do + let(:expected_type) { 'instance_type' } + let(:type) { nil } + let(:params) { {} } + + it_behaves_like 'it cannot create a runner' do + let(:current_user) { anonymous } + end + + it_behaves_like 'it cannot create a runner' do + let(:current_user) { non_admin_user } + end + + it_behaves_like 'it can create a runner' do + include_context 'when admin user' + end + + it_behaves_like 'it can return an error' do + include_context 'when admin user' + end + end +end diff --git a/spec/services/ci/runners/process_runner_version_update_service_spec.rb b/spec/services/ci/runners/process_runner_version_update_service_spec.rb index d2a7e87b2d5..e62cb1ec3e3 100644 --- a/spec/services/ci/runners/process_runner_version_update_service_spec.rb +++ b/spec/services/ci/runners/process_runner_version_update_service_spec.rb @@ -53,14 +53,14 @@ RSpec.describe Ci::Runners::ProcessRunnerVersionUpdateService, feature_category: end context 'with existing ci_runner_version record' do - let!(:runner_version) { create(:ci_runner_version, version: '1.0.0', status: :not_available) } + let!(:runner_version) { create(:ci_runner_version, version: '1.0.0', status: :unavailable) } it 'updates ci_runner_versions record', :aggregate_failures do expect do expect(execute).to be_success expect(execute.http_status).to eq :ok expect(execute.payload).to eq({ upgrade_status: 'recommended' }) - end.to change { runner_version.reload.status }.from('not_available').to('recommended') + end.to change { runner_version.reload.status }.from('unavailable').to('recommended') end end diff --git a/spec/services/ci/runners/reconcile_existing_runner_versions_service_spec.rb b/spec/services/ci/runners/reconcile_existing_runner_versions_service_spec.rb index 39082b5c0f4..8d7e97e5ea8 100644 --- a/spec/services/ci/runners/reconcile_existing_runner_versions_service_spec.rb +++ b/spec/services/ci/runners/reconcile_existing_runner_versions_service_spec.rb @@ -9,7 +9,7 @@ RSpec.describe ::Ci::Runners::ReconcileExistingRunnerVersionsService, '#execute' let_it_be(:runner_14_0_1) { create(:ci_runner, version: '14.0.1') } let_it_be(:runner_version_14_0_1) do - create(:ci_runner_version, version: '14.0.1', status: :not_available) + create(:ci_runner_version, version: '14.0.1', status: :unavailable) end context 'with RunnerUpgradeCheck recommending 14.0.2' do @@ -23,15 +23,17 @@ RSpec.describe ::Ci::Runners::ReconcileExistingRunnerVersionsService, '#execute' context 'with runner with new version' do let!(:runner_14_0_2) { create(:ci_runner, version: '14.0.2') } - let!(:runner_version_14_0_0) { create(:ci_runner_version, version: '14.0.0', status: :not_available) } let!(:runner_14_0_0) { create(:ci_runner, version: '14.0.0') } + let!(:runner_version_14_0_0) do + create(:ci_runner_version, version: '14.0.0', status: :unavailable) + end before do allow(upgrade_check).to receive(:check_runner_upgrade_suggestion) .and_return([::Gitlab::VersionInfo.new(14, 0, 2), :recommended]) allow(upgrade_check).to receive(:check_runner_upgrade_suggestion) .with('14.0.2') - .and_return([::Gitlab::VersionInfo.new(14, 0, 2), :not_available]) + .and_return([::Gitlab::VersionInfo.new(14, 0, 2), :unavailable]) .once end @@ -43,9 +45,9 @@ RSpec.describe ::Ci::Runners::ReconcileExistingRunnerVersionsService, '#execute' .and_call_original expect { execute } - .to change { runner_version_14_0_0.reload.status }.from('not_available').to('recommended') - .and change { runner_version_14_0_1.reload.status }.from('not_available').to('recommended') - .and change { ::Ci::RunnerVersion.find_by(version: '14.0.2')&.status }.from(nil).to('not_available') + .to change { runner_version_14_0_0.reload.status }.from('unavailable').to('recommended') + .and change { runner_version_14_0_1.reload.status }.from('unavailable').to('recommended') + .and change { ::Ci::RunnerVersion.find_by(version: '14.0.2')&.status }.from(nil).to('unavailable') expect(execute).to be_success expect(execute.payload).to eq({ @@ -57,17 +59,19 @@ RSpec.describe ::Ci::Runners::ReconcileExistingRunnerVersionsService, '#execute' end context 'with orphan ci_runner_version' do - let!(:runner_version_14_0_2) { create(:ci_runner_version, version: '14.0.2', status: :not_available) } + let!(:runner_version_14_0_2) do + create(:ci_runner_version, version: '14.0.2', status: :unavailable) + end before do allow(upgrade_check).to receive(:check_runner_upgrade_suggestion) - .and_return([::Gitlab::VersionInfo.new(14, 0, 2), :not_available]) + .and_return([::Gitlab::VersionInfo.new(14, 0, 2), :unavailable]) end it 'deletes orphan ci_runner_versions entry', :aggregate_failures do expect { execute } - .to change { ::Ci::RunnerVersion.find_by_version('14.0.2')&.status }.from('not_available').to(nil) - .and not_change { runner_version_14_0_1.reload.status }.from('not_available') + .to change { ::Ci::RunnerVersion.find_by_version('14.0.2')&.status }.from('unavailable').to(nil) + .and not_change { runner_version_14_0_1.reload.status }.from('unavailable') expect(execute).to be_success expect(execute.payload).to eq({ @@ -81,11 +85,11 @@ RSpec.describe ::Ci::Runners::ReconcileExistingRunnerVersionsService, '#execute' context 'with no runner version changes' do before do allow(upgrade_check).to receive(:check_runner_upgrade_suggestion) - .and_return([::Gitlab::VersionInfo.new(14, 0, 1), :not_available]) + .and_return([::Gitlab::VersionInfo.new(14, 0, 1), :unavailable]) end it 'does not modify ci_runner_versions entries', :aggregate_failures do - expect { execute }.not_to change { runner_version_14_0_1.reload.status }.from('not_available') + expect { execute }.not_to change { runner_version_14_0_1.reload.status }.from('unavailable') expect(execute).to be_success expect(execute.payload).to eq({ @@ -103,7 +107,7 @@ RSpec.describe ::Ci::Runners::ReconcileExistingRunnerVersionsService, '#execute' end it 'makes no changes to ci_runner_versions', :aggregate_failures do - expect { execute }.not_to change { runner_version_14_0_1.reload.status }.from('not_available') + expect { execute }.not_to change { runner_version_14_0_1.reload.status }.from('unavailable') expect(execute).to be_success expect(execute.payload).to eq({ @@ -121,7 +125,7 @@ RSpec.describe ::Ci::Runners::ReconcileExistingRunnerVersionsService, '#execute' end it 'does not modify ci_runner_versions entries', :aggregate_failures do - expect { execute }.not_to change { runner_version_14_0_1.reload.status }.from('not_available') + expect { execute }.not_to change { runner_version_14_0_1.reload.status }.from('unavailable') expect(execute).to be_success expect(execute.payload).to eq({ diff --git a/spec/services/ci/runners/register_runner_service_spec.rb b/spec/services/ci/runners/register_runner_service_spec.rb index 47d399cb19a..c67040e45eb 100644 --- a/spec/services/ci/runners/register_runner_service_spec.rb +++ b/spec/services/ci/runners/register_runner_service_spec.rb @@ -47,6 +47,7 @@ RSpec.describe ::Ci::Runners::RegisterRunnerService, '#execute', feature_categor expect(runner.run_untagged).to be true expect(runner.active).to be true expect(runner.token).not_to eq(registration_token) + expect(runner.token).not_to start_with(::Ci::Runner::CREATED_RUNNER_TOKEN_PREFIX) expect(runner).to be_instance_type end diff --git a/spec/services/ci/runners/stale_machines_cleanup_service_spec.rb b/spec/services/ci/runners/stale_machines_cleanup_service_spec.rb new file mode 100644 index 00000000000..456dbcebb84 --- /dev/null +++ b/spec/services/ci/runners/stale_machines_cleanup_service_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::Runners::StaleMachinesCleanupService, feature_category: :runner_fleet do + let(:service) { described_class.new } + let!(:runner_machine3) { create(:ci_runner_machine, created_at: 6.months.ago, contacted_at: Time.current) } + + subject(:response) { service.execute } + + context 'with no stale runner machines' do + it 'does not clean any runner machines and returns :success status' do + expect do + expect(response).to be_success + expect(response.payload).to match({ deleted_machines: false }) + end.not_to change { Ci::RunnerMachine.count }.from(1) + end + end + + context 'with some stale runner machines' do + before do + create(:ci_runner_machine, :stale) + create(:ci_runner_machine, :stale, contacted_at: nil) + end + + it 'only leaves non-stale runners' do + expect(response).to be_success + expect(response.payload).to match({ deleted_machines: true }) + expect(Ci::RunnerMachine.all).to contain_exactly(runner_machine3) + end + + context 'with more stale runners than MAX_DELETIONS' do + before do + stub_const("#{described_class}::MAX_DELETIONS", 1) + end + + it 'only leaves non-stale runners' do + expect do + expect(response).to be_success + expect(response.payload).to match({ deleted_machines: true }) + end.to change { Ci::RunnerMachine.count }.by(-Ci::Runners::StaleMachinesCleanupService::MAX_DELETIONS) + end + end + end +end diff --git a/spec/services/ci/update_build_queue_service_spec.rb b/spec/services/ci/update_build_queue_service_spec.rb index d3f537a1aa0..dd26339831c 100644 --- a/spec/services/ci/update_build_queue_service_spec.rb +++ b/spec/services/ci/update_build_queue_service_spec.rb @@ -277,7 +277,7 @@ RSpec.describe Ci::UpdateBuildQueueService do end end - context 'when updating specific runners' do + context 'when updating project runners' do let(:runner) { create(:ci_runner, :project, projects: [project]) } it_behaves_like 'matching build' diff --git a/spec/services/ci/update_build_state_service_spec.rb b/spec/services/ci/update_build_state_service_spec.rb index 90a86e7ae59..f8ecff20728 100644 --- a/spec/services/ci/update_build_state_service_spec.rb +++ b/spec/services/ci/update_build_state_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::UpdateBuildStateService do +RSpec.describe Ci::UpdateBuildStateService, feature_category: :continuous_integration do let_it_be(:project) { create(:project) } let_it_be(:pipeline) { create(:ci_pipeline, project: project) } |