diff options
| author | Rémy Coutable <remy@rymai.me> | 2016-08-11 14:27:42 +0000 |
|---|---|---|
| committer | Rémy Coutable <remy@rymai.me> | 2016-08-11 14:27:42 +0000 |
| commit | 7e9b41896d8c2ffae36152401f35479b39297e78 (patch) | |
| tree | f488221fceef5952f9d3495cff0026ed67169484 /app | |
| parent | a081b842f9b4e0205cf08656403e6bd9bf0d367f (diff) | |
| parent | 39203f1adfc6fee3eca50f0cab99ffc597865200 (diff) | |
| download | gitlab-ce-7e9b41896d8c2ffae36152401f35479b39297e78.tar.gz | |
Merge branch 'refactor-builds-creation-service' into 'master'
Refactor pipeline creation service
## What does this MR do?
This refactors GitLab CI build processing: all builds for pipeline are pre-created when a pipeline object is created.
The builds are created with a new introduced status `created`.
The builds are then automatically promoted to `pending` when a previous stage do succeed.
This significantly simplifies pipeline processing code solving a lot of problems of lazily initialisation of previous approach (builds were created on-demand).
## Why was this MR needed?
The previous mechanism had a lot of flows (shown in related issues) in how it work, but also in code design. Removing cross model-service-library dependencies.
The current approach moves a build creation to single place `CreatePipelineService` and removes a dynamic dependency on `config_processor` significantly simplifying a build creation and pipeline processing. Pipeline processing is implemented in `ProcessPipelineService`.
This also allows to easily extend GitLab with Manual Actions which is part of 8.10 direction issue.
## Migration problem
~~This MR removes the a on-demand creation of builds in pipelines.
Pipelines that are running and are in mid-stage (some stages started, but not all) will not be fully evaluated after application restart.
This happens, because the code responsible for on-demand creation is removed.
There's no easy way to migrate existing pipelines, other than doing offline migration and putting pipeline processing in migration code (which seems to be a really bad idea).~~
To support old pipelines I added a lazy initialization of builds if none is found.
## What are the relevant issue numbers?
Fixes: https://gitlab.com/gitlab-org/gitlab-ce/issues/12839
Solves: https://gitlab.com/gitlab-org/gitlab-ce/issues/18644 https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/issues/289
Allows to easily implement: https://gitlab.com/gitlab-org/gitlab-ce/issues/17010
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- Tests
- [x] Added for this feature/bug
- [ ] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
See merge request !5295
Diffstat (limited to 'app')
| -rw-r--r-- | app/controllers/projects/builds_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/projects/commit_controller.rb | 4 | ||||
| -rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 4 | ||||
| -rw-r--r-- | app/controllers/projects/pipelines_controller.rb | 2 | ||||
| -rw-r--r-- | app/models/ci/build.rb | 12 | ||||
| -rw-r--r-- | app/models/ci/pipeline.rb | 84 | ||||
| -rw-r--r-- | app/models/commit_status.rb | 31 | ||||
| -rw-r--r-- | app/models/concerns/statuseable.rb | 31 | ||||
| -rw-r--r-- | app/services/ci/create_builds_service.rb | 62 | ||||
| -rw-r--r-- | app/services/ci/create_pipeline_builds_service.rb | 42 | ||||
| -rw-r--r-- | app/services/ci/create_pipeline_service.rb | 95 | ||||
| -rw-r--r-- | app/services/ci/create_trigger_request_service.rb | 17 | ||||
| -rw-r--r-- | app/services/ci/process_pipeline_service.rb | 77 | ||||
| -rw-r--r-- | app/services/create_commit_builds_service.rb | 69 | ||||
| -rw-r--r-- | app/services/git_push_service.rb | 2 | ||||
| -rw-r--r-- | app/services/git_tag_push_service.rb | 2 | ||||
| -rw-r--r-- | app/views/projects/ci/pipelines/_pipeline.html.haml | 2 | ||||
| -rw-r--r-- | app/views/projects/commit/_pipeline.html.haml | 4 |
18 files changed, 279 insertions, 263 deletions
diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb index 553b62741a5..12195c3cbb8 100644 --- a/app/controllers/projects/builds_controller.rb +++ b/app/controllers/projects/builds_controller.rb @@ -6,7 +6,7 @@ class Projects::BuildsController < Projects::ApplicationController def index @scope = params[:scope] - @all_builds = project.builds + @all_builds = project.builds.relevant @builds = @all_builds.order('created_at DESC') @builds = case @scope diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index fdfe7c65b7b..f44e9bb3fd7 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -134,8 +134,8 @@ class Projects::CommitController < Projects::ApplicationController end def define_status_vars - @statuses = CommitStatus.where(pipeline: pipelines) - @builds = Ci::Build.where(pipeline: pipelines) + @statuses = CommitStatus.where(pipeline: pipelines).relevant + @builds = Ci::Build.where(pipeline: pipelines).relevant end def assign_change_commit_vars(mr_source_branch) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 2cf6a2dd1b3..139680d2df9 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -160,7 +160,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @diff_notes_disabled = true @pipeline = @merge_request.pipeline - @statuses = @pipeline.statuses if @pipeline + @statuses = @pipeline.statuses.relevant if @pipeline @note_counts = Note.where(commit_id: @commits.map(&:id)). group(:commit_id).count @@ -362,7 +362,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @commits_count = @merge_request.commits.count @pipeline = @merge_request.pipeline - @statuses = @pipeline.statuses if @pipeline + @statuses = @pipeline.statuses.relevant if @pipeline if @merge_request.locked_long_ago? @merge_request.unlock_mr diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 487963fdcd7..b0c72cfe4b4 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -19,7 +19,7 @@ class Projects::PipelinesController < Projects::ApplicationController end def create - @pipeline = Ci::CreatePipelineService.new(project, current_user, create_params).execute + @pipeline = Ci::CreatePipelineService.new(project, current_user, create_params).execute(ignore_skip_ci: true, save_on_errors: false) unless @pipeline.persisted? render 'new' return diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 08f396210c9..88a340379b8 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -16,7 +16,7 @@ module Ci scope :with_artifacts_not_expired, ->() { with_artifacts.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) } scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) } scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) } - scope :manual_actions, ->() { where(when: :manual) } + scope :manual_actions, ->() { where(when: :manual).relevant } mount_uploader :artifacts_file, ArtifactUploader mount_uploader :artifacts_metadata, ArtifactUploader @@ -65,17 +65,11 @@ module Ci end end - state_machine :status, initial: :pending do + state_machine :status do after_transition pending: :running do |build| build.execute_hooks end - # We use around_transition to create builds for next stage as soon as possible, before the `after_*` is executed - around_transition any => [:success, :failed, :canceled] do |build, block| - block.call - build.pipeline.create_next_builds(build) if build.pipeline - end - after_transition any => [:success, :failed, :canceled] do |build| build.update_coverage build.execute_hooks @@ -461,7 +455,7 @@ module Ci def build_attributes_from_config return {} unless pipeline.config_processor - + pipeline.config_processor.build_attributes(name) end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index bce6a992af6..718fe3290c1 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -13,11 +13,10 @@ module Ci has_many :trigger_requests, dependent: :destroy, class_name: 'Ci::TriggerRequest', foreign_key: :commit_id validates_presence_of :sha + validates_presence_of :ref validates_presence_of :status validate :valid_commit_sha - # Invalidate object and save if when touched - after_touch :update_state after_save :keep_around_commits # ref can't be HEAD or SHA, can only be branch/tag name @@ -90,12 +89,16 @@ module Ci def cancel_running builds.running_or_pending.each(&:cancel) + + reload_status! end def retry_failed(user) builds.latest.failed.select(&:retryable?).each do |build| Ci::Build.retry(build, user) end + + reload_status! end def latest? @@ -109,37 +112,6 @@ module Ci trigger_requests.any? end - def create_builds(user, trigger_request = nil) - ## - # We persist pipeline only if there are builds available - # - return unless config_processor - - build_builds_for_stages(config_processor.stages, user, - 'success', trigger_request) && save - end - - def create_next_builds(build) - return unless config_processor - - # don't create other builds if this one is retried - latest_builds = builds.latest - return unless latest_builds.exists?(build.id) - - # get list of stages after this build - next_stages = config_processor.stages.drop_while { |stage| stage != build.stage } - next_stages.delete(build.stage) - - # get status for all prior builds - prior_builds = latest_builds.where.not(stage: next_stages) - prior_status = prior_builds.status - - # build builds for next stage that has builds available - # and save pipeline if we have builds - build_builds_for_stages(next_stages, build.user, prior_status, - build.trigger_request) && save - end - def retried @retried ||= (statuses.order(id: :desc) - statuses.latest) end @@ -151,6 +123,14 @@ module Ci end end + def config_builds_attributes + return [] unless config_processor + + config_processor. + builds_for_ref(ref, tag?, trigger_requests.first). + sort_by { |build| build[:stage_idx] } + end + def has_warnings? builds.latest.ignored.any? end @@ -182,10 +162,6 @@ module Ci end end - def skip_ci? - git_commit_message =~ /\[(ci skip|skip ci)\]/i if git_commit_message - end - def environments builds.where.not(environment: nil).success.pluck(:environment).uniq end @@ -207,39 +183,33 @@ module Ci Note.for_commit_id(sha) end + def process! + Ci::ProcessPipelineService.new(project, user).execute(self) + reload_status! + end + def predefined_variables [ { key: 'CI_PIPELINE_ID', value: id.to_s, public: true } ] end - private - - def build_builds_for_stages(stages, user, status, trigger_request) - ## - # Note that `Array#any?` implements a short circuit evaluation, so we - # build builds only for the first stage that has builds available. - # - stages.any? do |stage| - CreateBuildsService.new(self). - execute(stage, user, status, trigger_request). - any?(&:active?) - end - end - - def update_state + def reload_status! statuses.reload - self.status = if yaml_errors.blank? - statuses.latest.status || 'skipped' - else - 'failed' - end + self.status = + if yaml_errors.blank? + statuses.latest.status || 'skipped' + else + 'failed' + end self.started_at = statuses.started_at self.finished_at = statuses.finished_at self.duration = statuses.latest.duration save end + private + def keep_around_commits return unless project diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 2d185c28809..20713314a25 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -5,7 +5,7 @@ class CommitStatus < ActiveRecord::Base self.table_name = 'ci_builds' belongs_to :project, class_name: '::Project', foreign_key: :gl_project_id - belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :commit_id, touch: true + belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :commit_id belongs_to :user delegate :commit, to: :pipeline @@ -25,28 +25,36 @@ class CommitStatus < ActiveRecord::Base scope :ordered, -> { order(:name) } scope :ignored, -> { where(allow_failure: true, status: [:failed, :canceled]) } - state_machine :status, initial: :pending do + state_machine :status do event :queue do - transition skipped: :pending + transition [:created, :skipped] => :pending end event :run do transition pending: :running end + event :skip do + transition [:created, :pending] => :skipped + end + event :drop do - transition [:pending, :running] => :failed + transition [:created, :pending, :running] => :failed end event :success do - transition [:pending, :running] => :success + transition [:created, :pending, :running] => :success end event :cancel do - transition [:pending, :running] => :canceled + transition [:created, :pending, :running] => :canceled + end + + after_transition created: [:pending, :running] do |commit_status| + commit_status.update_attributes queued_at: Time.now end - after_transition pending: :running do |commit_status| + after_transition [:created, :pending] => :running do |commit_status| commit_status.update_attributes started_at: Time.now end @@ -54,13 +62,20 @@ class CommitStatus < ActiveRecord::Base commit_status.update_attributes finished_at: Time.now end - after_transition [:pending, :running] => :success do |commit_status| + after_transition [:created, :pending, :running] => :success do |commit_status| MergeRequests::MergeWhenBuildSucceedsService.new(commit_status.pipeline.project, nil).trigger(commit_status) end after_transition any => :failed do |commit_status| MergeRequests::AddTodoWhenBuildFailsService.new(commit_status.pipeline.project, nil).execute(commit_status) end + + # We use around_transition to process pipeline on next stages as soon as possible, before the `after_*` is executed + around_transition any => [:success, :failed, :canceled] do |commit_status, block| + block.call + + commit_status.pipeline.process! if commit_status.pipeline + end end delegate :sha, :short_sha, to: :pipeline diff --git a/app/models/concerns/statuseable.rb b/app/models/concerns/statuseable.rb index 44c6b30f278..5d4b0a86899 100644 --- a/app/models/concerns/statuseable.rb +++ b/app/models/concerns/statuseable.rb @@ -1,18 +1,22 @@ module Statuseable extend ActiveSupport::Concern - AVAILABLE_STATUSES = %w(pending running success failed canceled skipped) + AVAILABLE_STATUSES = %w[created pending running success failed canceled skipped] + STARTED_STATUSES = %w[running success failed skipped] + ACTIVE_STATUSES = %w[pending running] + COMPLETED_STATUSES = %w[success failed canceled] class_methods do def status_sql - builds = all.select('count(*)').to_sql - success = all.success.select('count(*)').to_sql - ignored = all.ignored.select('count(*)').to_sql if all.respond_to?(:ignored) + scope = all.relevant + builds = scope.select('count(*)').to_sql + success = scope.success.select('count(*)').to_sql + ignored = scope.ignored.select('count(*)').to_sql if scope.respond_to?(:ignored) ignored ||= '0' - pending = all.pending.select('count(*)').to_sql - running = all.running.select('count(*)').to_sql - canceled = all.canceled.select('count(*)').to_sql - skipped = all.skipped.select('count(*)').to_sql + pending = scope.pending.select('count(*)').to_sql + running = scope.running.select('count(*)').to_sql + canceled = scope.canceled.select('count(*)').to_sql + skipped = scope.skipped.select('count(*)').to_sql deduce_status = "(CASE WHEN (#{builds})=0 THEN NULL @@ -48,7 +52,8 @@ module Statuseable included do validates :status, inclusion: { in: AVAILABLE_STATUSES } - state_machine :status, initial: :pending do + state_machine :status, initial: :created do + state :created, value: 'created' state :pending, value: 'pending' state :running, value: 'running' state :failed, value: 'failed' @@ -57,6 +62,8 @@ module Statuseable state :skipped, value: 'skipped' end + scope :created, -> { where(status: 'created') } + scope :relevant, -> { where.not(status: 'created') } scope :running, -> { where(status: 'running') } scope :pending, -> { where(status: 'pending') } scope :success, -> { where(status: 'success') } @@ -68,14 +75,14 @@ module Statuseable end def started? - !pending? && !canceled? && started_at + STARTED_STATUSES.include?(status) && started_at end def active? - running? || pending? + ACTIVE_STATUSES.include?(status) end def complete? - canceled? || success? || failed? + COMPLETED_STATUSES.include?(status) end end diff --git a/app/services/ci/create_builds_service.rb b/app/services/ci/create_builds_service.rb deleted file mode 100644 index 4946f7076fd..00000000000 --- a/app/services/ci/create_builds_service.rb +++ /dev/null @@ -1,62 +0,0 @@ -module Ci - class CreateBuildsService - def initialize(pipeline) - @pipeline = pipeline - @config = pipeline.config_processor - end - - def execute(stage, user, status, trigger_request = nil) - builds_attrs = @config.builds_for_stage_and_ref(stage, @pipeline.ref, @pipeline.tag, trigger_request) - - # check when to create next build - builds_attrs = builds_attrs.select do |build_attrs| - case build_attrs[:when] - when 'on_success' - status == 'success' - when 'on_failure' - status == 'failed' - when 'always', 'manual' - %w(success failed).include?(status) - end - end - - # don't create the same build twice - builds_attrs.reject! do |build_attrs| - @pipeline.builds.find_by(ref: @pipeline.ref, - tag: @pipeline.tag, - trigger_request: trigger_request, - name: build_attrs[:name]) - end - - builds_attrs.map do |build_attrs| - build_attrs.slice!(:name, - :commands, - :tag_list, - :options, - :allow_failure, - :stage, - :stage_idx, - :environment, - :when, - :yaml_variables) - - build_attrs.merge!(pipeline: @pipeline, - ref: @pipeline.ref, - tag: @pipeline.tag, - trigger_request: trigger_request, - user: user, - project: @pipeline.project) - - # TODO: The proper implementation for this is in - # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5295 - build_attrs[:status] = 'skipped' if build_attrs[:when] == 'manual' - - ## - # We do not persist new builds here. - # Those will be persisted when @pipeline is saved. - # - @pipeline.builds.new(build_attrs) - end - end - end -end diff --git a/app/services/ci/create_pipeline_builds_service.rb b/app/services/ci/create_pipeline_builds_service.rb new file mode 100644 index 00000000000..005014fa1de --- /dev/null +++ b/app/services/ci/create_pipeline_builds_service.rb @@ -0,0 +1,42 @@ +module Ci + class CreatePipelineBuildsService < BaseService + attr_reader :pipeline + + def execute(pipeline) + @pipeline = pipeline + + new_builds.map do |build_attributes| + create_build(build_attributes) + end + end + + private + + def create_build(build_attributes) + build_attributes = build_attributes.merge( + pipeline: pipeline, + project: pipeline.project, + ref: pipeline.ref, + tag: pipeline.tag, + user: current_user, + trigger_request: trigger_request + ) + pipeline.builds.create(build_attributes) + end + + def new_builds + @new_builds ||= pipeline.config_builds_attributes. + reject { |build| existing_build_names.include?(build[:name]) } + end + + def existing_build_names + @existing_build_names ||= pipeline.builds.pluck(:name) + end + + def trigger_request + return @trigger_request if defined?(@trigger_request) + + @trigger_request ||= pipeline.trigger_requests.first + end + end +end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index be91bf0db85..7398fd8e10a 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -1,49 +1,100 @@ module Ci class CreatePipelineService < BaseService - def execute - pipeline = project.pipelines.new(params) - pipeline.user = current_user + attr_reader :pipeline - unless ref_names.include?(params[:ref]) - pipeline.errors.add(:base, 'Reference not found') - return pipeline + def execute(ignore_skip_ci: false, save_on_errors: true, trigger_request: nil) + @pipeline = Ci::Pipeline.new( + project: project, + ref: ref, + sha: sha, + before_sha: before_sha, + tag: tag?, + trigger_requests: Array(trigger_request), + user: current_user + ) + + unless project.builds_enabled? + return error('Pipeline is disabled') + end + + unless trigger_request || can?(current_user, :create_pipeline, project) + return error('Insufficient permissions to create a new pipeline') end - if commit - pipeline.sha = commit.id - else - pipeline.errors.add(:base, 'Commit not found') - return pipeline + unless branch? || tag? + return error('Reference not found') end - unless can?(current_user, :create_pipeline, project) - pipeline.errors.add(:base, 'Insufficient permissions to create a new pipeline') - return pipeline + unless commit + return error('Commit not found') end unless pipeline.config_processor - pipeline.errors.add(:base, pipeline.yaml_errors || 'Missing .gitlab-ci.yml file') - return pipeline + unless pipeline.ci_yaml_file + return error('Missing .gitlab-ci.yml file') + end + return error(pipeline.yaml_errors, save: save_on_errors) end - pipeline.save! + if !ignore_skip_ci && skip_ci? + return error('Creation of pipeline is skipped', save: save_on_errors) + end - unless pipeline.create_builds(current_user) - pipeline.errors.add(:base, 'No builds for this pipeline.') + unless pipeline.config_builds_attributes.present? + return error('No builds for this pipeline.') end pipeline.save + pipeline.process! pipeline end private - def ref_names - @ref_names ||= project.repository.ref_names + def skip_ci? + pipeline.git_commit_message =~ /\[(ci skip|skip ci)\]/i if pipeline.git_commit_message end def commit - @commit ||= project.commit(params[:ref]) + @commit ||= project.commit(origin_sha || origin_ref) + end + + def sha + commit.try(:id) + end + + def before_sha + params[:checkout_sha] || params[:before] || Gitlab::Git::BLANK_SHA + end + + def origin_sha + params[:checkout_sha] || params[:after] + end + + def origin_ref + params[:ref] + end + + def branch? + project.repository.ref_exists?(Gitlab::Git::BRANCH_REF_PREFIX + ref) + end + + def tag? + project.repository.ref_exists?(Gitlab::Git::TAG_REF_PREFIX + ref) + end + + def ref + Gitlab::Git.ref_name(origin_ref) + end + + def valid_sha? + origin_sha && origin_sha != Gitlab::Git::BLANK_SHA + end + + def error(message, save: false) + pipeline.errors.add(:base, message) + pipeline.reload_status! if save + pipeline end end end diff --git a/app/services/ci/create_trigger_request_service.rb b/app/services/ci/create_trigger_request_service.rb index 1e629cf119a..6af3c1ca5b1 100644 --- a/app/services/ci/create_trigger_request_service.rb +++ b/app/services/ci/create_trigger_request_service.rb @@ -1,20 +1,11 @@ module Ci class CreateTriggerRequestService def execute(project, trigger, ref, variables = nil) - commit = project.commit(ref) - return unless commit + trigger_request = trigger.trigger_requests.create(variables: variables) - # check if ref is tag - tag = project.repository.find_tag(ref).present? - - pipeline = project.pipelines.create(sha: commit.sha, ref: ref, tag: tag) - - trigger_request = trigger.trigger_requests.create!( - variables: variables, - pipeline: pipeline, - ) - - if pipeline.create_builds(nil, trigger_request) + pipeline = Ci::CreatePipelineService.new(project, nil, ref: ref). + execute(ignore_skip_ci: true, trigger_request: trigger_request) + if pipeline.persisted? trigger_request end end diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb new file mode 100644 index 00000000000..86c4823d18a --- /dev/null +++ b/app/services/ci/process_pipeline_service.rb @@ -0,0 +1,77 @@ +module Ci + class ProcessPipelineService < BaseService + attr_reader :pipeline + + def execute(pipeline) + @pipeline = pipeline + + # This method will ensure that our pipeline does have all builds for all stages created + if created_builds.empty? + create_builds! + end + + new_builds = + stage_indexes_of_created_builds.map do |index| + process_stage(index) + end + + # Return a flag if a when builds got enqueued + new_builds.flatten.any? + end + + private + + def create_builds! + Ci::CreatePipelineBuildsService.new(project, current_user).execute(pipeline) + end + + def process_stage(index) + current_status = status_for_prior_stages(index) + + created_builds_in_stage(index).select do |build| + process_build(build, current_status) + end + end + + def process_build(build, current_status) + return false unless Statuseable::COMPLETED_STATUSES.include?(current_status) + + if valid_statuses_for_when(build.when).include?(current_status) + build.queue + true + else + build.skip + false + end + end + + def valid_statuses_for_when(value) + case value + when 'on_success' + %w[success] + when 'on_failure' + %w[failed] + when 'always' + %w[success failed] + else + [] + end + end + + def status_for_prior_stages(index) + pipeline.builds.where('stage_idx < ?', index).latest.status || 'success' + end + + def stage_indexes_of_created_builds + created_builds.order(:stage_idx).pluck('distinct stage_idx') + end + + def created_builds_in_stage(index) + created_builds.where(stage_idx: index) + end + + def created_builds + pipeline.builds.created + end + end +end diff --git a/app/services/create_commit_builds_service.rb b/app/services/create_commit_builds_service.rb deleted file mode 100644 index 0b66b854dea..00000000000 --- a/app/services/create_commit_builds_service.rb +++ /dev/null @@ -1,69 +0,0 @@ -class CreateCommitBuildsService - def execute(project, user, params) - return unless project.builds_enabled? - - before_sha = params[:checkout_sha] || params[:before] - sha = params[:checkout_sha] || params[:after] - origin_ref = params[:ref] - - ref = Gitlab::Git.ref_name(origin_ref) - tag = Gitlab::Git.tag_ref?(origin_ref) - - # Skip branch removal - if sha == Gitlab::Git::BLANK_SHA - return false - end - - @pipeline = Ci::Pipeline.new( - project: project, - sha: sha, - ref: ref, - before_sha: before_sha, - tag: tag, - user: user) - - ## - # Skip creating pipeline if no gitlab-ci.yml is found - # - unless @pipeline.ci_yaml_file - return false - end - - ## - # Skip creating builds for commits that have [ci skip] - # but save pipeline object - # - if @pipeline.skip_ci? - return save_pipeline! - end - - ## - # Skip creating builds when CI config is invalid - # but save pipeline object - # - unless @pipeline.config_processor - return save_pipeline! - end - - ## - # Skip creating pipeline object if there are no builds for it. - # - unless @pipeline.create_builds(user) - @pipeline.errors.add(:base, 'No builds created') - return false - end - - save_pipeline! - end - - private - - ## - # Create a new pipeline and touch object to calculate status - # - def save_pipeline! - @pipeline.save! - @pipeline.touch - @pipeline - end -end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 3f6a177bf3a..6f521462cf3 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -69,7 +69,7 @@ class GitPushService < BaseService SystemHooksService.new.execute_hooks(build_push_data_system_hook.dup, :push_hooks) @project.execute_hooks(build_push_data.dup, :push_hooks) @project.execute_services(build_push_data.dup, :push_hooks) - CreateCommitBuildsService.new.execute(@project, current_user, build_push_data) + Ci::CreatePipelineService.new(project, current_user, build_push_data).execute ProjectCacheWorker.perform_async(@project.id) end diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 969530c4fdc..d2b52f16fa8 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -11,7 +11,7 @@ class GitTagPushService < BaseService SystemHooksService.new.execute_hooks(build_system_push_data.dup, :tag_push_hooks) project.execute_hooks(@push_data.dup, :tag_push_hooks) project.execute_services(@push_data.dup, :tag_push_hooks) - CreateCommitBuildsService.new.execute(project, current_user, @push_data) + Ci::CreatePipelineService.new(project, current_user, @push_data).execute ProjectCacheWorker.perform_async(project.id) true diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml index 9a594877803..78709a92aed 100644 --- a/app/views/projects/ci/pipelines/_pipeline.html.haml +++ b/app/views/projects/ci/pipelines/_pipeline.html.haml @@ -33,7 +33,7 @@ Cant find HEAD commit for this branch - - stages_status = pipeline.statuses.latest.stages_status + - stages_status = pipeline.statuses.relevant.latest.stages_status - stages.each do |stage| %td.stage-cell - status = stages_status[stage] diff --git a/app/views/projects/commit/_pipeline.html.haml b/app/views/projects/commit/_pipeline.html.haml index 540689f4a61..640abdb993f 100644 --- a/app/views/projects/commit/_pipeline.html.haml +++ b/app/views/projects/commit/_pipeline.html.haml @@ -46,5 +46,5 @@ - if pipeline.project.build_coverage_enabled? %th Coverage %th - - pipeline.statuses.stages.each do |stage| - = render 'projects/commit/ci_stage', stage: stage, statuses: pipeline.statuses.where(stage: stage) + - pipeline.statuses.relevant.stages.each do |stage| + = render 'projects/commit/ci_stage', stage: stage, statuses: pipeline.statuses.relevant.where(stage: stage) |
