diff options
author | Stan Hu <stanhu@gmail.com> | 2019-09-03 23:34:52 -0700 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-09-05 21:42:14 -0700 |
commit | c34240d26fdcf447ee86172a81c0fc56fcaf9cbc (patch) | |
tree | f1d74946d5725d2df09c5845f76debc5851adc36 | |
parent | 13227500f29d8a74c77cba23b7dfdb4169222821 (diff) | |
download | gitlab-ce-c34240d26fdcf447ee86172a81c0fc56fcaf9cbc.tar.gz |
Log errors for failed pipeline creation in PostReceivesh-add-sidekiq-logging-for-bad-ci
When a pipeline fails to create in `PostReceive`, the error is silently
discarded, making it difficult to understand why a pipeline was not
created. We now add a Sidekiq warning message for this. Adding a Sentry
exception when this happens would generate a lot of noise for invalid CI
files.
Relates to https://gitlab.com/gitlab-org/gitlab-ee/issues/14720
-rw-r--r-- | app/services/git/base_hooks_service.rb | 28 | ||||
-rw-r--r-- | changelogs/unreleased/sh-add-sidekiq-logging-for-bad-ci.yml | 5 | ||||
-rw-r--r-- | spec/services/git/branch_push_service_spec.rb | 14 |
3 files changed, 46 insertions, 1 deletions
diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb index 47c308c8280..35a4d2015fa 100644 --- a/app/services/git/base_hooks_service.rb +++ b/app/services/git/base_hooks_service.rb @@ -57,7 +57,9 @@ module Git Ci::CreatePipelineService .new(project, current_user, pipeline_params) - .execute(:push, pipeline_options) + .execute!(:push, pipeline_options) + rescue Ci::CreatePipelineService::CreateError => ex + log_pipeline_errors(ex) end def execute_project_hooks @@ -125,5 +127,29 @@ module Git project.mark_stuck_remote_mirrors_as_failed! project.update_remote_mirrors end + + def log_pipeline_errors(exception) + data = { + class: self.class.name, + correlation_id: Labkit::Correlation::CorrelationId.current_id.to_s, + project_id: project.id, + project_path: project.full_path, + message: "Error creating pipeline", + errors: exception.to_s, + pipeline_params: pipeline_params + } + + logger.warn(data) + end + + def logger + if Sidekiq.server? + Sidekiq.logger + else + # This service runs in Sidekiq, so this shouldn't ever be + # called, but this is included just in case. + Gitlab::ProjectServiceLogger + end + end end end diff --git a/changelogs/unreleased/sh-add-sidekiq-logging-for-bad-ci.yml b/changelogs/unreleased/sh-add-sidekiq-logging-for-bad-ci.yml new file mode 100644 index 00000000000..b334355cab6 --- /dev/null +++ b/changelogs/unreleased/sh-add-sidekiq-logging-for-bad-ci.yml @@ -0,0 +1,5 @@ +--- +title: Log errors for failed pipeline creation in PostReceive +merge_request: 32633 +author: +type: other diff --git a/spec/services/git/branch_push_service_spec.rb b/spec/services/git/branch_push_service_spec.rb index d9e607cd251..c3a4f3dbe3f 100644 --- a/spec/services/git/branch_push_service_spec.rb +++ b/spec/services/git/branch_push_service_spec.rb @@ -99,6 +99,20 @@ describe Git::BranchPushService, services: true do expect(pipeline).to be_push expect(Gitlab::Git::BRANCH_REF_PREFIX + pipeline.ref).to eq(ref) end + + context 'when pipeline has errors' do + before do + config = YAML.dump({ test: { script: 'ls', only: ['feature'] } }) + stub_ci_pipeline_yaml_file(config) + end + + it 'reports an error' do + allow(Sidekiq).to receive(:server?).and_return(true) + expect(Sidekiq.logger).to receive(:warn) + + expect { subject }.not_to change { Ci::Pipeline.count } + end + end end describe "Updates merge requests" do |