diff options
-rw-r--r-- | app/controllers/projects/pipelines_settings_controller.rb | 2 | ||||
-rw-r--r-- | app/models/ci/pipeline.rb | 8 | ||||
-rw-r--r-- | app/models/concerns/has_status.rb | 1 | ||||
-rw-r--r-- | app/models/project.rb | 2 | ||||
-rw-r--r-- | app/services/ci/create_pipeline_service.rb | 12 | ||||
-rw-r--r-- | app/views/projects/pipelines_settings/_show.html.haml | 17 | ||||
-rw-r--r-- | changelogs/unreleased/8998_skip_pending_commits_if_not_head.yml | 4 | ||||
-rw-r--r-- | db/migrate/20170305180853_add_auto_cancel_pending_pipelines_to_project.rb | 33 | ||||
-rw-r--r-- | db/migrate/20170312114329_add_auto_canceled_by_to_pipeline.rb | 29 | ||||
-rw-r--r-- | db/schema.rb | 6 | ||||
-rw-r--r-- | doc/user/project/pipelines/settings.md | 9 | ||||
-rw-r--r-- | spec/services/ci/create_pipeline_service_spec.rb | 214 |
12 files changed, 232 insertions, 105 deletions
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 49dec770096..0ac12d9c3dc 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -127,6 +127,14 @@ module Ci where.not(duration: nil).sum(:duration) end + def auto_cancelable_pipelines + project.pipelines + .where(ref: ref) + .where.not(id: id) + .where.not(sha: project.repository.sha_from_ref(ref)) + .created_or_pending + end + def stage(name) stage = Ci::Stage.new(self, name: name) stage unless stage.statuses_count.zero? 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 83660d8c431..333b7319ffa 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -256,6 +256,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: { enabled: 1, disabled: 0 } + # 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/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 38a85e9fc42..cb09973081a 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,16 @@ module Ci pipeline.git_commit_message =~ /\[(ci[ _-]skip|skip[ _-]ci)\]/i end + def cancel_pending_pipelines + Gitlab::OptimisticLocking.retry_lock( + pipeline.auto_cancelable_pipelines) do |cancelables| + cancelables.find_each do |cancelable| + cancelable.cancel_running + cancelable.update_attributes(auto_canceled_by: pipeline.id) + end + end + end + def commit @commit ||= project.commit(origin_sha || origin_ref) end 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..48ee9268dea --- /dev/null +++ b/db/migrate/20170305180853_add_auto_cancel_pending_pipelines_to_project.rb @@ -0,0 +1,33 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddAutoCancelPendingPipelinesToProject < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = 'Creating column with default value' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + 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_to_pipeline.rb b/db/migrate/20170312114329_add_auto_canceled_by_to_pipeline.rb new file mode 100644 index 00000000000..344b83fa253 --- /dev/null +++ b/db/migrate/20170312114329_add_auto_canceled_by_to_pipeline.rb @@ -0,0 +1,29 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddAutoCanceledByToPipeline < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + add_column :ci_commits, :auto_canceled_by, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index ccf18d07179..3ec5b08ac40 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -251,6 +251,7 @@ ActiveRecord::Schema.define(version: 20170402231018) do t.integer "duration" t.integer "user_id" t.integer "lock_version" + t.integer "auto_canceled_by" end add_index "ci_pipelines", ["project_id", "ref", "status"], name: "index_ci_pipelines_on_project_id_and_ref_and_status", using: :btree @@ -687,8 +688,8 @@ ActiveRecord::Schema.define(version: 20170402231018) do t.string "avatar" t.boolean "share_with_group_lock", default: false t.integer "visibility_level", default: 20, null: false - t.boolean "request_access_enabled", default: false, null: false t.datetime "deleted_at" + t.boolean "request_access_enabled", default: false, null: false t.text "description_html" t.boolean "lfs_enabled" t.integer "parent_id" @@ -920,6 +921,7 @@ ActiveRecord::Schema.define(version: 20170402231018) do t.text "description_html" t.boolean "only_allow_merge_if_all_discussions_are_resolved" t.boolean "printing_merge_request_link_enabled", default: true, null: false + t.integer "auto_cancel_pending_pipelines", default: 0, null: false end add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree @@ -1239,9 +1241,9 @@ ActiveRecord::Schema.define(version: 20170402231018) do t.boolean "hide_project_limit", default: false t.string "unlock_token" t.datetime "otp_grace_period_started_at" + t.string "incoming_email_token" t.boolean "ldap_email", default: false, null: false t.boolean "external", default: false - t.string "incoming_email_token" t.string "organization" t.boolean "authorized_projects_populated" t.boolean "ghost" 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/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index d2f0337c260..be677facfe2 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -9,72 +9,127 @@ 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" }]) - end - + shared_examples 'a pending pipeline' do 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) } end + context 'valid params' do + let(:pipeline) { execute_service } + + it_behaves_like 'a pending pipeline' + + context 'auto-cancel enabled' do + let(:pipeline_on_previous_commit) do + execute_service( + after: previous_commit_sha_from_ref('master') + ) + end + + def previous_commit_sha_from_ref(ref) + project.repository.find_commits(ref: ref, max_count: 2)[1].id + end + + before do + project.update(auto_cancel_pending_pipelines: 'enabled') + end + + it_behaves_like 'a pending pipeline' + + it 'auto cancel pending non-HEAD pipelines' do + pending_pipeline = pipeline_on_previous_commit + pipeline + + expect(pending_pipeline.reload).to have_attributes(status: 'canceled', auto_canceled_by: pipeline.id) + end + + it 'does not cancel running outdated pipelines' do + running_pipeline = pipeline_on_previous_commit + running_pipeline.run + execute_service + + expect(running_pipeline.reload).to have_attributes(status: 'running', auto_canceled_by: nil) + end + + it 'cancel created outdated pipelines' do + created_pipeline = pipeline_on_previous_commit + created_pipeline.update(status: 'created') + pipeline + + expect(created_pipeline.reload).to have_attributes(status: 'canceled', auto_canceled_by: 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: nil) + end + 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 +152,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 +160,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 skips 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 +197,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 +212,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 +226,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 |