diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2017-04-07 08:30:32 +0000 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2017-04-07 08:30:32 +0000 |
commit | 15e87cea3a374e8d929c28f6843170b182fe159a (patch) | |
tree | 628d14a453f307c229296b508b37d8d56e93a1b4 | |
parent | c970fa973d482c50820db75d08d74bd5c1e9d475 (diff) | |
parent | 38796f56173caa2ed6575a4f60f7bf07ed90af55 (diff) | |
download | gitlab-ce-15e87cea3a374e8d929c28f6843170b182fe159a.tar.gz |
Merge branch '8998_skip_pending_commits_if_not_head' into 'master'
Add auto-cancel for pending pipelines on branch, if they are not HEAD
See merge request !9362
35 files changed, 550 insertions, 160 deletions
diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 5de8301f74c..1780cc0233c 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -116,7 +116,7 @@ class Projects::PipelinesController < Projects::ApplicationController end def pipeline - @pipeline ||= project.pipelines.find_by!(id: params[:id]) + @pipeline ||= project.pipelines.find_by!(id: params[:id]).present(current_user: current_user) end def commit diff --git a/app/controllers/projects/pipelines_settings_controller.rb b/app/controllers/projects/pipelines_settings_controller.rb index c8c80551ac9..ff50602831c 100644 --- a/app/controllers/projects/pipelines_settings_controller.rb +++ b/app/controllers/projects/pipelines_settings_controller.rb @@ -23,7 +23,7 @@ class Projects::PipelinesSettingsController < Projects::ApplicationController def update_params params.require(:project).permit( :runners_token, :builds_enabled, :build_allow_git_fetch, :build_timeout_in_minutes, :build_coverage_regex, - :public_builds + :public_builds, :auto_cancel_pending_pipelines ) end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index c197e50ed69..37a81fa7781 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -4,9 +4,14 @@ module Ci include HasStatus include Importable include AfterCommitQueue + include Presentable belongs_to :project belongs_to :user + belongs_to :auto_canceled_by, class_name: 'Ci::Pipeline' + + has_many :auto_canceled_pipelines, class_name: 'Ci::Pipeline', foreign_key: 'auto_canceled_by_id' + has_many :auto_canceled_jobs, class_name: 'CommitStatus', foreign_key: 'auto_canceled_by_id' has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id has_many :builds, foreign_key: :commit_id @@ -71,6 +76,10 @@ module Ci pipeline.update_duration end + before_transition canceled: any - [:canceled] do |pipeline| + pipeline.auto_canceled_by = nil + end + after_transition [:created, :pending] => :running do |pipeline| pipeline.run_after_commit { PipelineMetricsWorker.perform_async(id) } end @@ -216,9 +225,24 @@ module Ci cancelable_statuses.any? end + def auto_canceled? + canceled? && auto_canceled_by_id? + end + def cancel_running Gitlab::OptimisticLocking.retry_lock(cancelable_statuses) do |cancelable| - cancelable.find_each(&:cancel) + cancelable.find_each do |job| + yield(job) if block_given? + job.cancel + end + end + end + + def auto_cancel_running(pipeline) + update(auto_canceled_by: pipeline) + + cancel_running do |job| + job.auto_canceled_by = pipeline end end diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 17b322b5ae3..2c4033146bf 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -7,6 +7,7 @@ class CommitStatus < ActiveRecord::Base belongs_to :project belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :commit_id + belongs_to :auto_canceled_by, class_name: 'Ci::Pipeline' belongs_to :user delegate :commit, to: :pipeline @@ -137,6 +138,10 @@ class CommitStatus < ActiveRecord::Base false end + def auto_canceled? + canceled? && auto_canceled_by_id? + end + # Added in 9.0 to keep backward compatibility for projects exported in 8.17 # and prior. def gl_project_id diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb index 0a1a65da05a..2f61709110c 100644 --- a/app/models/concerns/has_status.rb +++ b/app/models/concerns/has_status.rb @@ -76,6 +76,7 @@ module HasStatus scope :canceled, -> { where(status: 'canceled') } scope :skipped, -> { where(status: 'skipped') } scope :manual, -> { where(status: 'manual') } + scope :created_or_pending, -> { where(status: [:created, :pending]) } scope :running_or_pending, -> { where(status: [:running, :pending]) } scope :finished, -> { where(status: [:success, :failed, :canceled]) } scope :failed_or_canceled, -> { where(status: [:failed, :canceled]) } diff --git a/app/models/project.rb b/app/models/project.rb index 2b467f95087..639615b91a2 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -262,6 +262,8 @@ class Project < ActiveRecord::Base scope :with_builds_enabled, -> { with_feature_enabled(:builds) } scope :with_issues_enabled, -> { with_feature_enabled(:issues) } + enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 } + # project features may be "disabled", "internal" or "enabled". If "internal", # they are only available to team members. This scope returns projects where # the feature is either enabled, or internal with permission for the user. diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb index ed72ed14d72..c495c3f39bb 100644 --- a/app/presenters/ci/build_presenter.rb +++ b/app/presenters/ci/build_presenter.rb @@ -11,5 +11,11 @@ module Ci def erased_by_name erased_by.name if erased_by_user? end + + def status_title + if auto_canceled? + "Job is redundant and is auto-canceled by Pipeline ##{auto_canceled_by_id}" + end + end end end diff --git a/app/presenters/ci/pipeline_presenter.rb b/app/presenters/ci/pipeline_presenter.rb new file mode 100644 index 00000000000..a542bdd8295 --- /dev/null +++ b/app/presenters/ci/pipeline_presenter.rb @@ -0,0 +1,11 @@ +module Ci + class PipelinePresenter < Gitlab::View::Presenter::Delegated + presents :pipeline + + def status_title + if auto_canceled? + "Pipeline is redundant and is auto-canceled by Pipeline ##{auto_canceled_by_id}" + end + end + end +end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 38a85e9fc42..21350be5557 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -53,6 +53,8 @@ module Ci .execute(pipeline) end + cancel_pending_pipelines if project.auto_cancel_pending_pipelines? + pipeline.tap(&:process!) end @@ -63,6 +65,22 @@ module Ci pipeline.git_commit_message =~ /\[(ci[ _-]skip|skip[ _-]ci)\]/i end + def cancel_pending_pipelines + Gitlab::OptimisticLocking.retry_lock(auto_cancelable_pipelines) do |cancelables| + cancelables.find_each do |cancelable| + cancelable.auto_cancel_running(pipeline) + end + end + end + + def auto_cancelable_pipelines + project.pipelines + .where(ref: pipeline.ref) + .where.not(id: pipeline.id) + .where.not(sha: project.repository.sha_from_ref(pipeline.ref)) + .created_or_pending + end + def commit @commit ||= project.commit(origin_sha || origin_ref) end diff --git a/app/views/ci/status/_badge.html.haml b/app/views/ci/status/_badge.html.haml index c00c7f7407e..39c7fb0eba2 100644 --- a/app/views/ci/status/_badge.html.haml +++ b/app/views/ci/status/_badge.html.haml @@ -1,12 +1,13 @@ - status = local_assigns.fetch(:status) -- link = local_assigns.fetch(:link, true) -- css_classes = "ci-status ci-#{status.group}" +- link = local_assigns.fetch(:link, true) +- title = local_assigns.fetch(:title, nil) +- css_classes = "ci-status ci-#{status.group} #{'has-tooltip' if title.present?}" - if link && status.has_details? - = link_to status.details_path, class: css_classes do + = link_to status.details_path, class: css_classes, title: title do = custom_icon(status.icon) = status.text - else - %span{ class: css_classes } + %span{ class: css_classes, title: title } = custom_icon(status.icon) = status.text diff --git a/app/views/projects/builds/_header.html.haml b/app/views/projects/builds/_header.html.haml index 7eb17e887e7..104db85809c 100644 --- a/app/views/projects/builds/_header.html.haml +++ b/app/views/projects/builds/_header.html.haml @@ -1,14 +1,16 @@ +- pipeline = @build.pipeline + .content-block.build-header.top-area .header-content - = render 'ci/status/badge', status: @build.detailed_status(current_user), link: false + = render 'ci/status/badge', status: @build.detailed_status(current_user), link: false, title: @build.status_title Job %strong.js-build-id ##{@build.id} in pipeline - = link_to pipeline_path(@build.pipeline) do - %strong ##{@build.pipeline.id} + = link_to pipeline_path(pipeline) do + %strong ##{pipeline.id} for commit - = link_to namespace_project_commit_path(@project.namespace, @project, @build.pipeline.sha) do - %strong= @build.pipeline.short_sha + = link_to namespace_project_commit_path(@project.namespace, @project, pipeline.sha) do + %strong= pipeline.short_sha from = link_to namespace_project_commits_path(@project.namespace, @project, @build.ref) do %code diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 508465cbfb7..4700b7a9a45 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -1,3 +1,5 @@ +- job = build.present(current_user: current_user) +- pipeline = job.pipeline - admin = local_assigns.fetch(:admin, false) - ref = local_assigns.fetch(:ref, nil) - commit_sha = local_assigns.fetch(:commit_sha, nil) @@ -8,101 +10,101 @@ %tr.build.commit{ class: ('retried' if retried) } %td.status - = render "ci/status/badge", status: build.detailed_status(current_user) + = render "ci/status/badge", status: job.detailed_status(current_user), title: job.status_title %td.branch-commit - - if can?(current_user, :read_build, build) - = link_to namespace_project_build_url(build.project.namespace, build.project, build) do - %span.build-link ##{build.id} + - if can?(current_user, :read_build, job) + = link_to namespace_project_build_url(job.project.namespace, job.project, job) do + %span.build-link ##{job.id} - else - %span.build-link ##{build.id} + %span.build-link ##{job.id} - if ref - - if build.ref + - if job.ref .icon-container - = build.tag? ? icon('tag') : icon('code-fork') - = link_to build.ref, namespace_project_commits_path(build.project.namespace, build.project, build.ref), class: "monospace branch-name" + = job.tag? ? icon('tag') : icon('code-fork') + = link_to job.ref, namespace_project_commits_path(job.project.namespace, job.project, job.ref), class: "monospace branch-name" - else .light none .icon-container.commit-icon = custom_icon("icon_commit") - if commit_sha - = link_to build.short_sha, namespace_project_commit_path(build.project.namespace, build.project, build.sha), class: "commit-id monospace" + = link_to job.short_sha, namespace_project_commit_path(job.project.namespace, job.project, job.sha), class: "commit-id monospace" - - if build.stuck? + - if job.stuck? = icon('warning', class: 'text-warning has-tooltip', title: 'Job is stuck. Check runners.') - if retried = icon('refresh', class: 'text-warning has-tooltip', title: 'Job was retried') .label-container - - if build.tags.any? - - build.tags.each do |tag| + - if job.tags.any? + - job.tags.each do |tag| %span.label.label-primary = tag - - if build.try(:trigger_request) + - if job.try(:trigger_request) %span.label.label-info triggered - - if build.try(:allow_failure) + - if job.try(:allow_failure) %span.label.label-danger allowed to fail - - if build.action? + - if job.action? %span.label.label-info manual - if pipeline_link %td - = link_to pipeline_path(build.pipeline) do - %span.pipeline-id ##{build.pipeline.id} + = link_to pipeline_path(pipeline) do + %span.pipeline-id ##{pipeline.id} %span by - - if build.pipeline.user - = user_avatar(user: build.pipeline.user, size: 20) + - if pipeline.user + = user_avatar(user: pipeline.user, size: 20) - else %span.monospace API - if admin %td - - if build.project - = link_to build.project.name_with_namespace, admin_namespace_project_path(build.project.namespace, build.project) + - if job.project + = link_to job.project.name_with_namespace, admin_namespace_project_path(job.project.namespace, job.project) %td - - if build.try(:runner) - = runner_link(build.runner) + - if job.try(:runner) + = runner_link(job.runner) - else .light none - if stage %td - = build.stage + = job.stage %td - = build.name + = job.name %td - - if build.duration + - if job.duration %p.duration = custom_icon("icon_timer") - = duration_in_numbers(build.duration) + = duration_in_numbers(job.duration) - - if build.finished_at + - if job.finished_at %p.finished-at = icon("calendar") - %span= time_ago_with_tooltip(build.finished_at) + %span= time_ago_with_tooltip(job.finished_at) %td.coverage - - if build.try(:coverage) - #{build.coverage}% + - if job.try(:coverage) + #{job.coverage}% %td .pull-right - - if can?(current_user, :read_build, build) && build.artifacts? - = link_to download_namespace_project_build_artifacts_path(build.project.namespace, build.project, build), rel: 'nofollow', download: '', title: 'Download artifacts', class: 'btn btn-build' do + - if can?(current_user, :read_build, job) && job.artifacts? + = link_to download_namespace_project_build_artifacts_path(job.project.namespace, job.project, job), rel: 'nofollow', download: '', title: 'Download artifacts', class: 'btn btn-build' do = icon('download') - - if can?(current_user, :update_build, build) - - if build.active? - = link_to cancel_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Cancel', class: 'btn btn-build' do + - if can?(current_user, :update_build, job) + - if job.active? + = link_to cancel_namespace_project_build_path(job.project.namespace, job.project, job, return_to: request.original_url), method: :post, title: 'Cancel', class: 'btn btn-build' do = icon('remove', class: 'cred') - elsif allow_retry - - if build.playable? && !admin - = link_to play_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Play', class: 'btn btn-build' do + - if job.playable? && !admin + = link_to play_namespace_project_build_path(job.project.namespace, job.project, job, return_to: request.original_url), method: :post, title: 'Play', class: 'btn btn-build' do = custom_icon('icon_play') - - elsif build.retryable? - = link_to retry_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Retry', class: 'btn btn-build' do + - elsif job.retryable? + = link_to retry_namespace_project_build_path(job.project.namespace, job.project, job, return_to: request.original_url), method: :post, title: 'Retry', class: 'btn btn-build' do = icon('repeat') diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index fc1d0989787..ab6baaf35b6 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -1,6 +1,6 @@ .page-content-header .header-main-content - = render 'ci/status/badge', status: @pipeline.detailed_status(current_user) + = render 'ci/status/badge', status: @pipeline.detailed_status(current_user), title: @pipeline.status_title %strong Pipeline ##{@pipeline.id} triggered #{time_ago_with_tooltip(@pipeline.created_at)} - if @pipeline.user diff --git a/app/views/projects/pipelines_settings/_show.html.haml b/app/views/projects/pipelines_settings/_show.html.haml index 132f6372e40..a3f84476dea 100644 --- a/app/views/projects/pipelines_settings/_show.html.haml +++ b/app/views/projects/pipelines_settings/_show.html.haml @@ -21,7 +21,7 @@ Git strategy for pipelines %p Choose between <code>clone</code> or <code>fetch</code> to get the recent application code - = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'git-strategy') + = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'git-strategy'), target: '_blank' .radio = f.label :build_allow_git_fetch_false do = f.radio_button :build_allow_git_fetch, 'false' @@ -43,7 +43,7 @@ = f.number_field :build_timeout_in_minutes, class: 'form-control', min: '0' %p.help-block Per job in minutes. If a job passes this threshold, it will be marked as failed. - = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'timeout') + = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'timeout'), target: '_blank' %hr .form-group @@ -53,7 +53,16 @@ %strong Public pipelines .help-block Allow everyone to access pipelines for public and internal projects - = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'visibility-of-pipelines') + = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'visibility-of-pipelines'), target: '_blank' + %hr + .form-group + .checkbox + = f.label :auto_cancel_pending_pipelines do + = f.check_box :auto_cancel_pending_pipelines, {}, 'enabled', 'disabled' + %strong Auto-cancel redundant, pending pipelines + .help-block + New pipelines will cancel older, pending pipelines on the same branch + = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'auto-cancel-pending-pipelines'), target: '_blank' %hr .form-group @@ -65,7 +74,7 @@ %p.help-block A regular expression that will be used to find the test coverage output in the job trace. Leave blank to disable - = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'test-coverage-parsing') + = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'test-coverage-parsing'), target: '_blank' .bs-callout.bs-callout-info %p Below are examples of regex for existing tools: %ul diff --git a/changelogs/unreleased/8998_skip_pending_commits_if_not_head.yml b/changelogs/unreleased/8998_skip_pending_commits_if_not_head.yml new file mode 100644 index 00000000000..9852cd6e4ff --- /dev/null +++ b/changelogs/unreleased/8998_skip_pending_commits_if_not_head.yml @@ -0,0 +1,4 @@ +--- +title: Cancel pending pipelines if commits not HEAD +merge_request: 9362 +author: Rydkin Maxim diff --git a/db/migrate/20170305180853_add_auto_cancel_pending_pipelines_to_project.rb b/db/migrate/20170305180853_add_auto_cancel_pending_pipelines_to_project.rb new file mode 100644 index 00000000000..aa64f2dddca --- /dev/null +++ b/db/migrate/20170305180853_add_auto_cancel_pending_pipelines_to_project.rb @@ -0,0 +1,15 @@ +class AddAutoCancelPendingPipelinesToProject < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:projects, :auto_cancel_pending_pipelines, :integer, default: 0) + end + + def down + remove_column(:projects, :auto_cancel_pending_pipelines) + end +end diff --git a/db/migrate/20170312114329_add_auto_canceled_by_id_to_pipeline.rb b/db/migrate/20170312114329_add_auto_canceled_by_id_to_pipeline.rb new file mode 100644 index 00000000000..1690ce90564 --- /dev/null +++ b/db/migrate/20170312114329_add_auto_canceled_by_id_to_pipeline.rb @@ -0,0 +1,9 @@ +class AddAutoCanceledByIdToPipeline < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :ci_pipelines, :auto_canceled_by_id, :integer + end +end diff --git a/db/migrate/20170312114529_add_auto_canceled_by_id_foreign_key_to_pipeline.rb b/db/migrate/20170312114529_add_auto_canceled_by_id_foreign_key_to_pipeline.rb new file mode 100644 index 00000000000..1e7b02ecf0e --- /dev/null +++ b/db/migrate/20170312114529_add_auto_canceled_by_id_foreign_key_to_pipeline.rb @@ -0,0 +1,22 @@ +class AddAutoCanceledByIdForeignKeyToPipeline < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + on_delete = + if Gitlab::Database.mysql? + :nullify + else + 'SET NULL' + end + + add_concurrent_foreign_key :ci_pipelines, :ci_pipelines, column: :auto_canceled_by_id, on_delete: on_delete + end + + def down + remove_foreign_key :ci_pipelines, column: :auto_canceled_by_id + end +end diff --git a/db/migrate/20170402231018_remove_index_for_users_current_sign_in_at.rb b/db/migrate/20170402231018_remove_index_for_users_current_sign_in_at.rb index 0237c3189a5..9d4380ef960 100644 --- a/db/migrate/20170402231018_remove_index_for_users_current_sign_in_at.rb +++ b/db/migrate/20170402231018_remove_index_for_users_current_sign_in_at.rb @@ -15,7 +15,7 @@ class RemoveIndexForUsersCurrentSignInAt < ActiveRecord::Migration if Gitlab::Database.postgresql? execute 'DROP INDEX CONCURRENTLY index_users_on_current_sign_in_at;' else - remove_index :users, :current_sign_in_at + remove_concurrent_index :users, :current_sign_in_at end end end diff --git a/db/migrate/20170406114958_add_auto_canceled_by_id_to_ci_builds.rb b/db/migrate/20170406114958_add_auto_canceled_by_id_to_ci_builds.rb new file mode 100644 index 00000000000..c1d803b4308 --- /dev/null +++ b/db/migrate/20170406114958_add_auto_canceled_by_id_to_ci_builds.rb @@ -0,0 +1,9 @@ +class AddAutoCanceledByIdToCiBuilds < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :ci_builds, :auto_canceled_by_id, :integer + end +end diff --git a/db/migrate/20170406115029_add_auto_canceled_by_id_foreign_key_to_ci_builds.rb b/db/migrate/20170406115029_add_auto_canceled_by_id_foreign_key_to_ci_builds.rb new file mode 100644 index 00000000000..3004683933b --- /dev/null +++ b/db/migrate/20170406115029_add_auto_canceled_by_id_foreign_key_to_ci_builds.rb @@ -0,0 +1,22 @@ +class AddAutoCanceledByIdForeignKeyToCiBuilds < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + on_delete = + if Gitlab::Database.mysql? + :nullify + else + 'SET NULL' + end + + add_concurrent_foreign_key :ci_builds, :ci_pipelines, column: :auto_canceled_by_id, on_delete: on_delete + end + + def down + remove_foreign_key :ci_builds, column: :auto_canceled_by_id + end +end diff --git a/db/schema.rb b/db/schema.rb index a7a8f5bd38f..16f3f293079 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170405080720) do +ActiveRecord::Schema.define(version: 20170406115029) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -223,6 +223,7 @@ ActiveRecord::Schema.define(version: 20170405080720) do t.string "token" t.integer "lock_version" t.string "coverage_regex" + t.integer "auto_canceled_by_id" end add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree @@ -251,6 +252,7 @@ ActiveRecord::Schema.define(version: 20170405080720) do t.integer "duration" t.integer "user_id" t.integer "lock_version" + t.integer "auto_canceled_by_id" end add_index "ci_pipelines", ["project_id", "ref", "status"], name: "index_ci_pipelines_on_project_id_and_ref_and_status", using: :btree @@ -947,6 +949,7 @@ ActiveRecord::Schema.define(version: 20170405080720) do t.boolean "lfs_enabled" t.text "description_html" t.boolean "only_allow_merge_if_all_discussions_are_resolved" + t.integer "auto_cancel_pending_pipelines", default: 0, null: false t.boolean "printing_merge_request_link_enabled", default: true, null: false t.string "import_jid" end @@ -1328,6 +1331,8 @@ ActiveRecord::Schema.define(version: 20170405080720) do add_foreign_key "boards", "projects" add_foreign_key "chat_teams", "namespaces", on_delete: :cascade + add_foreign_key "ci_builds", "ci_pipelines", column: "auto_canceled_by_id", name: "fk_a2141b1522", on_delete: :nullify + add_foreign_key "ci_pipelines", "ci_pipelines", column: "auto_canceled_by_id", name: "fk_262d4c2d19", on_delete: :nullify add_foreign_key "ci_trigger_schedules", "ci_triggers", column: "trigger_id", name: "fk_90a406cc94", on_delete: :cascade add_foreign_key "ci_triggers", "users", column: "owner_id", name: "fk_e8e10d1964", on_delete: :cascade add_foreign_key "container_repositories", "projects" diff --git a/doc/user/project/pipelines/settings.md b/doc/user/project/pipelines/settings.md index c398ac2eb25..88246e22391 100644 --- a/doc/user/project/pipelines/settings.md +++ b/doc/user/project/pipelines/settings.md @@ -60,6 +60,14 @@ anyone and those logged in respectively. If you wish to hide it so that only the members of the project or group have access to it, uncheck the **Public pipelines** checkbox and save the changes. +## Auto-cancel pending pipelines + +> [Introduced][ce-9362] in GitLab 9.1. + +If you want to auto-cancel all pending non-HEAD pipelines on branch, when +new pipeline will be created (after your git push or manually from UI), +check **Auto-cancel pending pipelines** checkbox and save the changes. + ## Badges In the pipelines settings page you can find pipeline status and test coverage @@ -111,3 +119,4 @@ into your `README.md`: [var]: ../../../ci/yaml/README.md#git-strategy [coverage report]: #test-coverage-parsing +[ce-9362]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9362 diff --git a/spec/features/projects/settings/pipelines_settings_spec.rb b/spec/features/projects/settings/pipelines_settings_spec.rb index 76cb240ea98..035c57eaa47 100644 --- a/spec/features/projects/settings/pipelines_settings_spec.rb +++ b/spec/features/projects/settings/pipelines_settings_spec.rb @@ -32,5 +32,16 @@ feature "Pipelines settings", feature: true do expect(page).to have_button('Save changes', disabled: false) expect(page).to have_field('Test coverage parsing', with: 'coverage_regex') end + + scenario 'updates auto_cancel_pending_pipelines' do + page.check('Auto-cancel redundant, pending pipelines') + click_on 'Save changes' + + expect(page.status_code).to eq(200) + expect(page).to have_button('Save changes', disabled: false) + + checkbox = find_field('project_auto_cancel_pending_pipelines') + expect(checkbox).to be_checked + end end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 259c97f936a..9770d2e3b13 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -89,6 +89,9 @@ pipelines: - statuses - builds - trigger_requests +- auto_canceled_by +- auto_canceled_pipelines +- auto_canceled_jobs - pending_builds - retryable_builds - cancelable_statuses @@ -98,6 +101,7 @@ statuses: - project - pipeline - user +- auto_canceled_by variables: - project triggers: diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 0c43c5662e8..04b5c80f22d 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -183,6 +183,7 @@ Ci::Pipeline: - duration - user_id - lock_version +- auto_canceled_by_id CommitStatus: - id - project_id @@ -223,6 +224,7 @@ CommitStatus: - token - lock_version - coverage_regex +- auto_canceled_by_id Ci::Variable: - id - project_id diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 02c47b3efc4..32aa2f4b336 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -12,10 +12,13 @@ describe Ci::Pipeline, models: true do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:auto_canceled_by) } it { is_expected.to have_many(:statuses) } it { is_expected.to have_many(:trigger_requests) } it { is_expected.to have_many(:builds) } + it { is_expected.to have_many(:auto_canceled_pipelines) } + it { is_expected.to have_many(:auto_canceled_jobs) } it { is_expected.to validate_presence_of :sha } it { is_expected.to validate_presence_of :status } @@ -134,6 +137,43 @@ describe Ci::Pipeline, models: true do end end + describe '#auto_canceled?' do + subject { pipeline.auto_canceled? } + + context 'when it is canceled' do + before do + pipeline.cancel + end + + context 'when there is auto_canceled_by' do + before do + pipeline.update(auto_canceled_by: create(:ci_empty_pipeline)) + end + + it 'is auto canceled' do + is_expected.to be_truthy + end + end + + context 'when there is no auto_canceled_by' do + it 'is not auto canceled' do + is_expected.to be_falsey + end + end + + context 'when it is retried and canceled manually' do + before do + pipeline.enqueue + pipeline.cancel + end + + it 'is not auto canceled' do + is_expected.to be_falsey + end + end + end + end + describe 'pipeline stages' do before do create(:commit_status, pipeline: pipeline, diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 7343b735a74..0ee85489574 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -16,6 +16,7 @@ describe CommitStatus, :models do it { is_expected.to belong_to(:pipeline) } it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:auto_canceled_by) } it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_inclusion_of(:status).in_array(%w(pending running failed success canceled)) } @@ -101,6 +102,32 @@ describe CommitStatus, :models do end end + describe '#auto_canceled?' do + subject { commit_status.auto_canceled? } + + context 'when it is canceled' do + before do + commit_status.update(status: 'canceled') + end + + context 'when there is auto_canceled_by' do + before do + commit_status.update(auto_canceled_by: create(:ci_empty_pipeline)) + end + + it 'is auto canceled' do + is_expected.to be_truthy + end + end + + context 'when there is no auto_canceled_by' do + it 'is not auto canceled' do + is_expected.to be_falsey + end + end + end + end + describe '#duration' do subject { commit_status.duration } diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index 82abad0e2f6..67dae7cf4c0 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -231,6 +231,18 @@ describe HasStatus do end end + describe '.created_or_pending' do + subject { CommitStatus.created_or_pending } + + %i[created pending].each do |status| + it_behaves_like 'containing the job', status + end + + %i[running failed success].each do |status| + it_behaves_like 'not containing the job', status + end + end + describe '.finished' do subject { CommitStatus.finished } diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb index 7a35da38b2b..2190ab0e82e 100644 --- a/spec/presenters/ci/build_presenter_spec.rb +++ b/spec/presenters/ci/build_presenter_spec.rb @@ -57,6 +57,32 @@ describe Ci::BuildPresenter do end end + describe '#status_title' do + context 'when build is auto-canceled' do + before do + expect(build).to receive(:auto_canceled?).and_return(true) + expect(build).to receive(:auto_canceled_by_id).and_return(1) + end + + it 'shows that the build is auto-canceled' do + status_title = presenter.status_title + + expect(status_title).to include('auto-canceled') + expect(status_title).to include('Pipeline #1') + end + end + + context 'when build is not auto-canceled' do + before do + expect(build).to receive(:auto_canceled?).and_return(false) + end + + it 'does not have a status title' do + expect(presenter.status_title).to be_nil + end + end + end + describe 'quack like a Ci::Build permission-wise' do context 'user is not allowed' do let(:project) { build_stubbed(:empty_project, public_builds: false) } diff --git a/spec/presenters/ci/pipeline_presenter_spec.rb b/spec/presenters/ci/pipeline_presenter_spec.rb new file mode 100644 index 00000000000..9134d1cc31c --- /dev/null +++ b/spec/presenters/ci/pipeline_presenter_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe Ci::PipelinePresenter do + let(:project) { create(:empty_project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + + subject(:presenter) do + described_class.new(pipeline) + end + + it 'inherits from Gitlab::View::Presenter::Delegated' do + expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated) + end + + describe '#initialize' do + it 'takes a pipeline and optional params' do + expect { presenter }.not_to raise_error + end + + it 'exposes pipeline' do + expect(presenter.pipeline).to eq(pipeline) + end + + it 'forwards missing methods to pipeline' do + expect(presenter.ref).to eq(pipeline.ref) + end + end + + describe '#status_title' do + context 'when pipeline is auto-canceled' do + before do + expect(pipeline).to receive(:auto_canceled?).and_return(true) + expect(pipeline).to receive(:auto_canceled_by_id).and_return(1) + end + + it 'shows that the pipeline is auto-canceled' do + status_title = presenter.status_title + + expect(status_title).to include('auto-canceled') + expect(status_title).to include('Pipeline #1') + end + end + + context 'when pipeline is not auto-canceled' do + before do + expect(pipeline).to receive(:auto_canceled?).and_return(false) + end + + it 'does not have a status title' do + expect(presenter.status_title).to be_nil + end + end + end +end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index d2f0337c260..fa5014cee07 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -9,72 +9,140 @@ describe Ci::CreatePipelineService, services: true do end describe '#execute' do - def execute(params) + def execute_service(after: project.commit.id, message: 'Message', ref: 'refs/heads/master') + params = { ref: ref, + before: '00000000', + after: after, + commits: [{ message: message }] } + described_class.new(project, user, params).execute end context 'valid params' do - let(:pipeline) do - execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: "Message" }]) + let(:pipeline) { execute_service } + + let(:pipeline_on_previous_commit) do + execute_service( + after: previous_commit_sha_from_ref('master') + ) end it { expect(pipeline).to be_kind_of(Ci::Pipeline) } it { expect(pipeline).to be_valid } - it { expect(pipeline).to be_persisted } it { expect(pipeline).to eq(project.pipelines.last) } it { expect(pipeline).to have_attributes(user: user) } + it { expect(pipeline).to have_attributes(status: 'pending') } it { expect(pipeline.builds.first).to be_kind_of(Ci::Build) } + + context 'auto-cancel enabled' do + before do + project.update(auto_cancel_pending_pipelines: 'enabled') + end + + it 'does not cancel HEAD pipeline' do + pipeline + pipeline_on_previous_commit + + expect(pipeline.reload).to have_attributes(status: 'pending', auto_canceled_by_id: nil) + end + + it 'auto cancel pending non-HEAD pipelines' do + pipeline_on_previous_commit + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) + end + + it 'does not cancel running outdated pipelines' do + pipeline_on_previous_commit.run + execute_service + + expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'running', auto_canceled_by_id: nil) + end + + it 'cancel created outdated pipelines' do + pipeline_on_previous_commit.update(status: 'created') + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) + end + + it 'does not cancel pipelines from the other branches' do + pending_pipeline = execute_service( + ref: 'refs/heads/feature', + after: previous_commit_sha_from_ref('feature') + ) + pipeline + + expect(pending_pipeline.reload).to have_attributes(status: 'pending', auto_canceled_by_id: nil) + end + end + + context 'auto-cancel disabled' do + before do + project.update(auto_cancel_pending_pipelines: 'disabled') + end + + it 'does not auto cancel pending non-HEAD pipelines' do + pipeline_on_previous_commit + pipeline + + expect(pipeline_on_previous_commit.reload) + .to have_attributes(status: 'pending', auto_canceled_by_id: nil) + end + end + + def previous_commit_sha_from_ref(ref) + project.commit(ref).parent.sha + end end context "skip tag if there is no build for it" do it "creates commit if there is appropriate job" do - result = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: "Message" }]) - expect(result).to be_persisted + expect(execute_service).to be_persisted end it "creates commit if there is no appropriate job but deploy job has right ref setting" do config = YAML.dump({ deploy: { script: "ls", only: ["master"] } }) stub_ci_pipeline_yaml_file(config) - result = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: "Message" }]) - expect(result).to be_persisted + expect(execute_service).to be_persisted end end it 'skips creating pipeline for refs without .gitlab-ci.yml' do stub_ci_pipeline_yaml_file(nil) - result = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: 'Message' }]) - expect(result).not_to be_persisted + expect(execute_service).not_to be_persisted expect(Ci::Pipeline.count).to eq(0) end - it 'fails commits if yaml is invalid' do - message = 'message' - allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { message } - stub_ci_pipeline_yaml_file('invalid: file: file') - commits = [{ message: message }] - pipeline = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: commits) - - expect(pipeline).to be_persisted - expect(pipeline.builds.any?).to be false - expect(pipeline.status).to eq('failed') - expect(pipeline.yaml_errors).not_to be_nil + shared_examples 'a failed pipeline' do + it 'creates failed pipeline' do + stub_ci_pipeline_yaml_file(ci_yaml) + + pipeline = execute_service(message: message) + + expect(pipeline).to be_persisted + expect(pipeline.builds.any?).to be false + expect(pipeline.status).to eq('failed') + expect(pipeline.yaml_errors).not_to be_nil + end + end + + context 'when yaml is invalid' do + let(:ci_yaml) { 'invalid: file: fiile' } + let(:message) { 'Message' } + + it_behaves_like 'a failed pipeline' + + context 'when receive git commit' do + before do + allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { message } + end + + it_behaves_like 'a failed pipeline' + end end context 'when commit contains a [ci skip] directive' do @@ -97,11 +165,7 @@ describe Ci::CreatePipelineService, services: true do ci_messages.each do |ci_message| it "skips builds creation if the commit message is #{ci_message}" do - commits = [{ message: ci_message }] - pipeline = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: commits) + pipeline = execute_service(message: ci_message) expect(pipeline).to be_persisted expect(pipeline.builds.any?).to be false @@ -109,58 +173,34 @@ describe Ci::CreatePipelineService, services: true do end end - it "does not skips builds creation if there is no [ci skip] or [skip ci] tag in commit message" do - allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { "some message" } + shared_examples 'creating a pipeline' do + it 'does not skip pipeline creation' do + allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { commit_message } - commits = [{ message: "some message" }] - pipeline = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: commits) + pipeline = execute_service(message: commit_message) - expect(pipeline).to be_persisted - expect(pipeline.builds.first.name).to eq("rspec") + expect(pipeline).to be_persisted + expect(pipeline.builds.first.name).to eq("rspec") + end end - it "does not skip builds creation if the commit message is nil" do - allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { nil } - - commits = [{ message: nil }] - pipeline = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: commits) + context 'when commit message does not contain [ci skip] nor [skip ci]' do + let(:commit_message) { 'some message' } - expect(pipeline).to be_persisted - expect(pipeline.builds.first.name).to eq("rspec") + it_behaves_like 'creating a pipeline' end - it "fails builds creation if there is [ci skip] tag in commit message and yaml is invalid" do - stub_ci_pipeline_yaml_file('invalid: file: fiile') - commits = [{ message: message }] - pipeline = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: commits) + context 'when commit message is nil' do + let(:commit_message) { nil } - expect(pipeline).to be_persisted - expect(pipeline.builds.any?).to be false - expect(pipeline.status).to eq("failed") - expect(pipeline.yaml_errors).not_to be_nil + it_behaves_like 'creating a pipeline' end - end - it "creates commit with failed status if yaml is invalid" do - stub_ci_pipeline_yaml_file('invalid: file') - commits = [{ message: "some message" }] - pipeline = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: commits) - - expect(pipeline).to be_persisted - expect(pipeline.status).to eq("failed") - expect(pipeline.builds.any?).to be false + context 'when there is [ci skip] tag in commit message and yaml is invalid' do + let(:ci_yaml) { 'invalid: file: fiile' } + + it_behaves_like 'a failed pipeline' + end end context 'when there are no jobs for this pipeline' do @@ -170,10 +210,7 @@ describe Ci::CreatePipelineService, services: true do end it 'does not create a new pipeline' do - result = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: 'some msg' }]) + result = execute_service expect(result).not_to be_persisted expect(Ci::Build.all).to be_empty @@ -188,10 +225,7 @@ describe Ci::CreatePipelineService, services: true do end it 'does not create a new pipeline' do - result = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: 'some msg' }]) + result = execute_service expect(result).to be_persisted expect(result.manual_actions).not_to be_empty @@ -205,10 +239,7 @@ describe Ci::CreatePipelineService, services: true do end it 'creates the environment' do - result = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: 'some msg' }]) + result = execute_service expect(result).to be_persisted expect(Environment.find_by(name: "review/master")).not_to be_nil diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 8567817147b..b2d37657770 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -16,20 +16,21 @@ describe Ci::RetryBuildService, :services do %i[id status user token coverage trace runner artifacts_expire_at artifacts_file artifacts_metadata artifacts_size created_at updated_at started_at finished_at queued_at erased_by - erased_at].freeze + erased_at auto_canceled_by].freeze IGNORE_ACCESSORS = %i[type lock_version target_url base_tags commit_id deployments erased_by_id last_deployment project_id runner_id tag_taggings taggings tags trigger_request_id - user_id].freeze + user_id auto_canceled_by_id].freeze shared_examples 'build duplication' do let(:build) do create(:ci_build, :failed, :artifacts_expired, :erased, :queued, :coverage, :tags, :allowed_to_fail, :on_tag, :teardown_environment, :triggered, :trace, - description: 'some build', pipeline: pipeline) + description: 'some build', pipeline: pipeline, + auto_canceled_by: create(:ci_empty_pipeline)) end describe 'clone accessors' do diff --git a/spec/views/projects/builds/show.html.haml_spec.rb b/spec/views/projects/builds/show.html.haml_spec.rb index 55b64808fb3..0f39df0f250 100644 --- a/spec/views/projects/builds/show.html.haml_spec.rb +++ b/spec/views/projects/builds/show.html.haml_spec.rb @@ -9,7 +9,7 @@ describe 'projects/builds/show', :view do end before do - assign(:build, build) + assign(:build, build.present) assign(:project, project) allow(view).to receive(:can?).and_return(true) diff --git a/spec/views/projects/pipelines/show.html.haml_spec.rb b/spec/views/projects/pipelines/show.html.haml_spec.rb index dca78dec6df..bb39ec8efbf 100644 --- a/spec/views/projects/pipelines/show.html.haml_spec.rb +++ b/spec/views/projects/pipelines/show.html.haml_spec.rb @@ -5,7 +5,13 @@ describe 'projects/pipelines/show' do let(:user) { create(:user) } let(:project) { create(:project, :repository) } - let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.id, user: user) } + + let(:pipeline) do + create(:ci_empty_pipeline, + project: project, + sha: project.commit.id, + user: user) + end before do controller.prepend_view_path('app/views/projects') @@ -21,7 +27,7 @@ describe 'projects/pipelines/show' do create(:generic_commit_status, pipeline: pipeline, stage: 'external', name: 'jenkins', stage_idx: 3) assign(:project, project) - assign(:pipeline, pipeline) + assign(:pipeline, pipeline.present(current_user: user)) assign(:commit, project.commit) allow(view).to receive(:can?).and_return(true) |