Commit 817bb190 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'ignore-failed-pipeline-creation-on-pipeline-schedule' into 'master'

Remove auto deactivation when failed to create a pipeline via pipeline schedules

Closes #41231

See merge request gitlab-org/gitlab-ce!22243
parents 9f967ba2 d56d419a
......@@ -4,6 +4,8 @@ module Ci
class CreatePipelineService < BaseService
attr_reader :pipeline
CreateError = Class.new(StandardError)
SEQUENCE = [Gitlab::Ci::Pipeline::Chain::Build,
Gitlab::Ci::Pipeline::Chain::Validate::Abilities,
Gitlab::Ci::Pipeline::Chain::Validate::Repository,
......@@ -47,6 +49,14 @@ module Ci
pipeline
end
def execute!(*args, &block)
execute(*args, &block).tap do |pipeline|
unless pipeline.persisted?
raise CreateError, pipeline.errors.full_messages.join(',')
end
end
end
private
def commit
......
......@@ -9,18 +9,36 @@ class PipelineScheduleWorker
Ci::PipelineSchedule.active.where("next_run_at < ?", Time.now)
.preload(:owner, :project).find_each do |schedule|
begin
pipeline = Ci::CreatePipelineService.new(schedule.project,
Ci::CreatePipelineService.new(schedule.project,
schedule.owner,
ref: schedule.ref)
.execute(:schedule, ignore_skip_ci: true, save_on_errors: false, schedule: schedule)
schedule.deactivate! unless pipeline.persisted?
.execute!(:schedule, ignore_skip_ci: true, save_on_errors: true, schedule: schedule)
rescue => e
Rails.logger.error "#{schedule.id}: Failed to create a scheduled pipeline: #{e.message}"
error(schedule, e)
ensure
schedule.schedule_next_run!
end
end
end
# rubocop: enable CodeReuse/ActiveRecord
private
def error(schedule, error)
failed_creation_counter.increment
Rails.logger.error "Failed to create a scheduled pipeline. " \
"schedule_id: #{schedule.id} message: #{error.message}"
Gitlab::Sentry
.track_exception(error,
issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/41231',
extra: { schedule_id: schedule.id })
end
def failed_creation_counter
@failed_creation_counter ||=
Gitlab::Metrics.counter(:pipeline_schedule_creation_failed_total,
"Counter of failed attempts of pipeline schedule creation")
end
end
---
title: Remove auto deactivation when failed to create a pipeline via pipeline schedules
merge_request: 22243
author:
type: changed
......@@ -83,12 +83,12 @@ The next time a pipeline is scheduled, your credentials will be used.
![Schedules list](img/pipeline_schedules_ownership.png)
> **Note:**
When the owner of the schedule doesn't have the ability to create pipelines
anymore, due to e.g., being blocked or removed from the project, or lacking
the permission to run on protected branches or tags. When this happened, the
schedule is deactivated. Another user can take ownership and activate it, so
the schedule can be run again.
NOTE: **Note:**
If the owner of a pipeline schedule doesn't have the ability to create pipelines
on the target branch, the schedule will stop creating new pipelines. This can
happen if, for example, the owner is blocked or removed from the project, or
the target branch or tag is protected. In this case, someone with sufficient
privileges must take ownership of the schedule.
## Advanced admin configuration
......
......@@ -657,4 +657,37 @@ describe Ci::CreatePipelineService do
end
end
end
describe '#execute!' do
subject { service.execute!(*args) }
let(:service) { described_class.new(project, user, ref: ref_name) }
let(:args) { [:push] }
context 'when user has a permission to create a pipeline' do
let(:user) { create(:user) }
before do
project.add_developer(user)
end
it 'does not raise an error' do
expect { subject }.not_to raise_error
end
it 'creates a pipeline' do
expect { subject }.to change { Ci::Pipeline.count }.by(1)
end
end
context 'when user does not have a permission to create a pipeline' do
let(:user) { create(:user) }
it 'raises an error' do
expect { subject }
.to raise_error(described_class::CreateError)
.with_message('Insufficient permissions to create a new pipeline')
end
end
end
end
......@@ -56,17 +56,89 @@ describe PipelineScheduleWorker do
expect { subject }.not_to change { project.pipelines.count }
end
end
context 'when gitlab-ci.yml is corrupted' do
before do
stub_ci_pipeline_yaml_file(YAML.dump(rspec: { variables: 'rspec' } ))
end
it 'creates a failed pipeline with the reason' do
expect { subject }.to change { project.pipelines.count }.by(1)
expect(Ci::Pipeline.last).to be_config_error
expect(Ci::Pipeline.last.yaml_errors).not_to be_nil
end
end
end
context 'when the schedule is not runnable by the user' do
it 'deactivates the schedule' do
before do
expect(Gitlab::Sentry)
.to receive(:track_exception)
.with(Ci::CreatePipelineService::CreateError,
issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/41231',
extra: { schedule_id: pipeline_schedule.id } ).once
end
it 'does not deactivate the schedule' do
subject
expect(pipeline_schedule.reload.active).to be_falsy
expect(pipeline_schedule.reload.active).to be_truthy
end
it 'increments Prometheus counter' do
expect(Gitlab::Metrics)
.to receive(:counter)
.with(:pipeline_schedule_creation_failed_total, "Counter of failed attempts of pipeline schedule creation")
.and_call_original
subject
end
it 'logging a pipeline error' do
expect(Rails.logger)
.to receive(:error)
.with(a_string_matching("Insufficient permissions to create a new pipeline"))
.and_call_original
subject
end
it 'does not schedule a pipeline' do
it 'does not create a pipeline' do
expect { subject }.not_to change { project.pipelines.count }
end
it 'does not raise an exception' do
expect { subject }.not_to raise_error
end
end
context 'when .gitlab-ci.yml is missing in the project' do
before do
stub_ci_pipeline_yaml_file(nil)
project.add_maintainer(user)
expect(Gitlab::Sentry)
.to receive(:track_exception)
.with(Ci::CreatePipelineService::CreateError,
issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/41231',
extra: { schedule_id: pipeline_schedule.id } ).once
end
it 'logging a pipeline error' do
expect(Rails.logger)
.to receive(:error)
.with(a_string_matching("Missing .gitlab-ci.yml file"))
.and_call_original
subject
end
it 'does not create a pipeline' do
expect { subject }.not_to change { project.pipelines.count }
end
it 'does not raise an exception' do
expect { subject }.not_to raise_error
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment