diff options
-rw-r--r-- | app/models/ci/build.rb | 24 | ||||
-rw-r--r-- | app/models/commit_status.rb | 9 | ||||
-rw-r--r-- | app/models/commit_status_enums.rb | 3 | ||||
-rw-r--r-- | app/presenters/commit_status_presenter.rb | 3 | ||||
-rw-r--r-- | app/services/ci/prepare_build_service.rb | 25 | ||||
-rw-r--r-- | app/workers/all_queues.yml | 1 | ||||
-rw-r--r-- | app/workers/ci/build_prepare_worker.rb | 16 | ||||
-rw-r--r-- | lib/gitlab/ci/build/prerequisite/base.rb | 27 | ||||
-rw-r--r-- | lib/gitlab/ci/build/prerequisite/factory.rb | 33 | ||||
-rw-r--r-- | lib/gitlab/ci/status/build/failed.rb | 3 | ||||
-rw-r--r-- | spec/features/projects/badges/pipeline_badge_spec.rb | 3 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb | 34 | ||||
-rw-r--r-- | spec/models/ci/build_spec.rb | 57 | ||||
-rw-r--r-- | spec/models/ci/pipeline_spec.rb | 6 | ||||
-rw-r--r-- | spec/models/commit_status_spec.rb | 6 | ||||
-rw-r--r-- | spec/requests/api/runner_spec.rb | 9 | ||||
-rw-r--r-- | spec/services/ci/prepare_build_service_spec.rb | 54 | ||||
-rw-r--r-- | spec/workers/ci/build_prepare_worker_spec.rb | 30 |
18 files changed, 336 insertions, 7 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index a629db82c19..a293afaed08 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -172,6 +172,10 @@ module Ci end state_machine :status do + event :enqueue do + transition [:created, :skipped, :manual, :scheduled] => :preparing, if: :any_unmet_prerequisites? + end + event :actionize do transition created: :manual end @@ -185,8 +189,12 @@ module Ci end event :enqueue_scheduled do + transition scheduled: :preparing, if: ->(build) do + build.scheduled_at&.past? && build.any_unmet_prerequisites? + end + transition scheduled: :pending, if: ->(build) do - build.scheduled_at && build.scheduled_at < Time.now + build.scheduled_at&.past? && !build.any_unmet_prerequisites? end end @@ -204,6 +212,12 @@ module Ci end end + after_transition any => [:preparing] do |build| + build.run_after_commit do + Ci::BuildPrepareWorker.perform_async(id) + end + end + after_transition any => [:pending] do |build| build.run_after_commit do BuildQueueWorker.perform_async(id) @@ -355,6 +369,14 @@ module Ci !retried? end + def any_unmet_prerequisites? + prerequisites.present? + end + + def prerequisites + Gitlab::Ci::Build::Prerequisite::Factory.new(self).unmet + end + def expanded_environment_name return unless has_environment? diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index bfb13fa0ce8..5f66a661324 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -66,7 +66,10 @@ class CommitStatus < ActiveRecord::Base end event :enqueue do - transition [:created, :preparing, :skipped, :manual, :scheduled] => :pending + # A CommitStatus will never have prerequisites, but this event + # is shared by Ci::Build, which cannot progress unless prerequisites + # are satisfied. + transition [:created, :preparing, :skipped, :manual, :scheduled] => :pending, unless: :any_unmet_prerequisites? end event :run do @@ -180,6 +183,10 @@ class CommitStatus < ActiveRecord::Base false end + def any_unmet_prerequisites? + false + end + def auto_canceled? canceled? && auto_canceled_by_id? end diff --git a/app/models/commit_status_enums.rb b/app/models/commit_status_enums.rb index 152105d9429..45e08fa18fe 100644 --- a/app/models/commit_status_enums.rb +++ b/app/models/commit_status_enums.rb @@ -14,7 +14,8 @@ module CommitStatusEnums runner_unsupported: 6, stale_schedule: 7, job_execution_timeout: 8, - archived_failure: 9 + archived_failure: 9, + unmet_prerequisites: 10 } end end diff --git a/app/presenters/commit_status_presenter.rb b/app/presenters/commit_status_presenter.rb index 0cd77da6303..28a25c8b7a3 100644 --- a/app/presenters/commit_status_presenter.rb +++ b/app/presenters/commit_status_presenter.rb @@ -11,7 +11,8 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated runner_unsupported: 'Your runner is outdated, please upgrade your runner', stale_schedule: 'Delayed job could not be executed by some reason, please try again', job_execution_timeout: 'The script exceeded the maximum execution time set for the job', - archived_failure: 'The job is archived and cannot be run' + archived_failure: 'The job is archived and cannot be run', + unmet_prerequisites: 'The job failed to complete prerequisite tasks' }.freeze private_constant :CALLOUT_FAILURE_MESSAGES diff --git a/app/services/ci/prepare_build_service.rb b/app/services/ci/prepare_build_service.rb new file mode 100644 index 00000000000..32f11438b79 --- /dev/null +++ b/app/services/ci/prepare_build_service.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Ci + class PrepareBuildService + attr_reader :build + + def initialize(build) + @build = build + end + + def execute + prerequisites.each(&:complete!) + + unless build.enqueue + build.drop!(:unmet_prerequisites) + end + end + + private + + def prerequisites + build.prerequisites + end + end +end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index b2d88567e0e..6ebd756d3da 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -71,6 +71,7 @@ - pipeline_hooks:build_hooks - pipeline_hooks:pipeline_hooks - pipeline_processing:build_finished +- pipeline_processing:ci_build_prepare - pipeline_processing:build_queue - pipeline_processing:build_success - pipeline_processing:pipeline_process diff --git a/app/workers/ci/build_prepare_worker.rb b/app/workers/ci/build_prepare_worker.rb new file mode 100644 index 00000000000..1a35a74ae53 --- /dev/null +++ b/app/workers/ci/build_prepare_worker.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Ci + class BuildPrepareWorker + include ApplicationWorker + include PipelineQueue + + queue_namespace :pipeline_processing + + def perform(build_id) + Ci::Build.find_by_id(build_id).try do |build| + Ci::PrepareBuildService.new(build).execute + end + end + end +end diff --git a/lib/gitlab/ci/build/prerequisite/base.rb b/lib/gitlab/ci/build/prerequisite/base.rb new file mode 100644 index 00000000000..156aa22d95b --- /dev/null +++ b/lib/gitlab/ci/build/prerequisite/base.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Build + module Prerequisite + class Base + include Utils::StrongMemoize + + attr_reader :build + + def initialize(build) + @build = build + end + + def unmet? + raise NotImplementedError + end + + def complete! + raise NotImplementedError + end + end + end + end + end +end diff --git a/lib/gitlab/ci/build/prerequisite/factory.rb b/lib/gitlab/ci/build/prerequisite/factory.rb new file mode 100644 index 00000000000..6b9c17bba07 --- /dev/null +++ b/lib/gitlab/ci/build/prerequisite/factory.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Build + module Prerequisite + class Factory + attr_reader :build + + def self.prerequisites + [] + end + + def initialize(build) + @build = build + end + + def unmet + build_prerequisites.select(&:unmet?) + end + + private + + def build_prerequisites + self.class.prerequisites.map do |prerequisite| + prerequisite.new(build) + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb index d40454df737..76dfe7b7639 100644 --- a/lib/gitlab/ci/status/build/failed.rb +++ b/lib/gitlab/ci/status/build/failed.rb @@ -15,7 +15,8 @@ module Gitlab runner_unsupported: 'unsupported runner', stale_schedule: 'stale schedule', job_execution_timeout: 'job execution timeout', - archived_failure: 'archived failure' + archived_failure: 'archived failure', + unmet_prerequisites: 'unmet prerequisites' }.freeze private_constant :REASONS diff --git a/spec/features/projects/badges/pipeline_badge_spec.rb b/spec/features/projects/badges/pipeline_badge_spec.rb index 96efa06d843..4ac4e8f0fcb 100644 --- a/spec/features/projects/badges/pipeline_badge_spec.rb +++ b/spec/features/projects/badges/pipeline_badge_spec.rb @@ -47,10 +47,11 @@ describe 'Pipeline Badge' do before do # Prevent skipping directly to 'pending' allow(Ci::BuildPrepareWorker).to receive(:perform_async) + allow(job).to receive(:prerequisites).and_return([double]) end it 'displays the preparing badge' do - job.prepare + job.enqueue visit pipeline_project_badges_path(project, ref: ref, format: :svg) diff --git a/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb new file mode 100644 index 00000000000..5187f99a441 --- /dev/null +++ b/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::Build::Prerequisite::Factory do + let(:build) { create(:ci_build) } + + describe '.for_build' do + let(:kubernetes_namespace) do + instance_double( + Gitlab::Ci::Build::Prerequisite::KubernetesNamespace, + unmet?: unmet) + end + + subject { described_class.new(build).unmet } + + before do + expect(Gitlab::Ci::Build::Prerequisite::KubernetesNamespace) + .to receive(:new).with(build).and_return(kubernetes_namespace) + end + + context 'prerequisite is unmet' do + let(:unmet) { true } + + it { is_expected.to eq [kubernetes_namespace] } + end + + context 'prerequisite is met' do + let(:unmet) { false } + + it { is_expected.to be_empty } + end + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 9ca4241d7d8..b31c4fcceb3 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -186,6 +186,37 @@ describe Ci::Build do end end + describe '#enqueue' do + let(:build) { create(:ci_build, :created) } + + subject { build.enqueue } + + before do + allow(build).to receive(:any_unmet_prerequisites?).and_return(has_prerequisites) + allow(Ci::PrepareBuildService).to receive(:perform_async) + end + + context 'build has unmet prerequisites' do + let(:has_prerequisites) { true } + + it 'transitions to preparing' do + subject + + expect(build).to be_preparing + end + end + + context 'build has no prerequisites' do + let(:has_prerequisites) { false } + + it 'transitions to pending' do + subject + + expect(build).to be_pending + end + end + end + describe '#actionize' do context 'when build is a created' do before do @@ -344,6 +375,18 @@ describe Ci::Build do expect(build).to be_pending end + + context 'build has unmet prerequisites' do + before do + allow(build).to receive(:prerequisites).and_return([double]) + end + + it 'transits to preparing' do + subject + + expect(build).to be_preparing + end + end end end @@ -2928,6 +2971,20 @@ describe Ci::Build do end end + describe 'state transition: any => [:preparing]' do + let(:build) { create(:ci_build, :created) } + + before do + allow(build).to receive(:prerequisites).and_return([double]) + end + + it 'queues BuildPrepareWorker' do + expect(Ci::BuildPrepareWorker).to receive(:perform_async).with(build.id) + + build.enqueue + end + end + describe 'state transition: any => [:pending]' do let(:build) { create(:ci_build, :created) } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index d35caac33dc..2ac056f63b2 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1804,7 +1804,11 @@ describe Ci::Pipeline, :mailer do context 'on prepare' do before do - build.prepare + # Prevent skipping directly to 'pending' + allow(build).to receive(:prerequisites).and_return([double]) + allow(Ci::BuildPrepareWorker).to receive(:perform_async) + + build.enqueue end it { is_expected.to eq('preparing') } diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 1d241bf6000..e2b7f5c6ee2 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -489,6 +489,12 @@ describe CommitStatus do it { is_expected.to be_script_failure } end + + context 'when failure_reason is unmet_prerequisites' do + let(:reason) { :unmet_prerequisites } + + it { is_expected.to be_unmet_prerequisites } + end end describe 'ensure stage assignment' do diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 9087cccb759..3ccedd8dd06 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -918,6 +918,15 @@ describe API::Runner, :clean_gitlab_redis_shared_state do it { expect(job).to be_job_execution_timeout } end + + context 'when failure_reason is unmet_prerequisites' do + before do + update_job(state: 'failed', failure_reason: 'unmet_prerequisites') + job.reload + end + + it { expect(job).to be_unmet_prerequisites } + end end context 'when trace is given' do diff --git a/spec/services/ci/prepare_build_service_spec.rb b/spec/services/ci/prepare_build_service_spec.rb new file mode 100644 index 00000000000..1797f8f964f --- /dev/null +++ b/spec/services/ci/prepare_build_service_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Ci::PrepareBuildService do + describe '#execute' do + let(:build) { create(:ci_build, :preparing) } + + subject { described_class.new(build).execute } + + before do + allow(build).to receive(:prerequisites).and_return(prerequisites) + end + + shared_examples 'build enqueueing' do + it 'enqueues the build' do + expect(build).to receive(:enqueue).once + + subject + end + end + + context 'build has unmet prerequisites' do + let(:prerequisite) { double(complete!: true) } + let(:prerequisites) { [prerequisite] } + + it 'completes each prerequisite' do + expect(prerequisites).to all(receive(:complete!)) + + subject + end + + include_examples 'build enqueueing' + + context 'prerequisites fail to complete' do + before do + allow(build).to receive(:enqueue).and_return(false) + end + + it 'drops the build' do + expect(build).to receive(:drop!).with(:unmet_prerequisites).once + + subject + end + end + end + + context 'build has no prerequisites' do + let(:prerequisites) { [] } + + include_examples 'build enqueueing' + end + end +end diff --git a/spec/workers/ci/build_prepare_worker_spec.rb b/spec/workers/ci/build_prepare_worker_spec.rb new file mode 100644 index 00000000000..9f76696ee66 --- /dev/null +++ b/spec/workers/ci/build_prepare_worker_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Ci::BuildPrepareWorker do + subject { described_class.new.perform(build_id) } + + context 'build exists' do + let(:build) { create(:ci_build) } + let(:build_id) { build.id } + let(:service) { double(execute: true) } + + it 'calls the prepare build service' do + expect(Ci::PrepareBuildService).to receive(:new).with(build).and_return(service) + expect(service).to receive(:execute).once + + subject + end + end + + context 'build does not exist' do + let(:build_id) { -1 } + + it 'does not attempt to prepare the build' do + expect(Ci::PrepareBuildService).not_to receive(:new) + + subject + end + end +end |