diff options
author | Rémy Coutable <remy@rymai.me> | 2016-08-11 17:27:42 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-08-11 17:27:42 +0300 |
commit | 7e9b41896d8c2ffae36152401f35479b39297e78 (patch) | |
tree | f488221fceef5952f9d3495cff0026ed67169484 /app/services | |
parent | a081b842f9b4e0205cf08656403e6bd9bf0d367f (diff) | |
parent | 39203f1adfc6fee3eca50f0cab99ffc597865200 (diff) |
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/services')
-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 |
8 files changed, 198 insertions, 168 deletions
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 |