Commit 55675684 authored by Shinya Maeda's avatar Shinya Maeda

Improve error handling in OlderDeploymentsDropService

This commit improves the error handling in order to
reduce the noise in Sentry
parent 222ae11b
...@@ -12,7 +12,9 @@ module Deployments ...@@ -12,7 +12,9 @@ module Deployments
return unless @deployment&.running? return unless @deployment&.running?
older_deployments.find_each do |older_deployment| older_deployments.find_each do |older_deployment|
older_deployment.deployable&.drop!(:forward_deployment_failure) Gitlab::OptimisticLocking.retry_lock(older_deployment.deployable) do |deployable|
deployable.drop(:forward_deployment_failure)
end
rescue => e rescue => e
Gitlab::ErrorTracking.track_exception(e, subject_id: @deployment.id, deployment_id: older_deployment.id) Gitlab::ErrorTracking.track_exception(e, subject_id: @deployment.id, deployment_id: older_deployment.id)
end end
......
...@@ -66,6 +66,43 @@ describe Deployments::OlderDeploymentsDropService do ...@@ -66,6 +66,43 @@ describe Deployments::OlderDeploymentsDropService do
expect(deployable.reload.failed?).to be_truthy expect(deployable.reload.failed?).to be_truthy
end end
context 'when older deployable is a manual job' do
let(:older_deployment) { create(:deployment, :created, environment: environment, deployable: build) }
let(:build) { create(:ci_build, :manual) }
it 'does not drop any builds nor track the exception' do
expect(Gitlab::ErrorTracking).not_to receive(:track_exception)
expect { subject }.not_to change { Ci::Build.failed.count }
end
end
context 'when deployable.drop raises RuntimeError' do
before do
allow_any_instance_of(Ci::Build).to receive(:drop).and_raise(RuntimeError)
end
it 'does not drop an older deployment and tracks the exception' do
expect(Gitlab::ErrorTracking).to receive(:track_exception)
.with(kind_of(RuntimeError), subject_id: deployment.id, deployment_id: older_deployment.id)
expect { subject }.not_to change { Ci::Build.failed.count }
end
end
context 'when ActiveRecord::StaleObjectError is raised' do
before do
allow_any_instance_of(Ci::Build)
.to receive(:drop).and_raise(ActiveRecord::StaleObjectError)
end
it 'resets the object via Gitlab::OptimisticLocking' do
allow_any_instance_of(Ci::Build).to receive(:reset).at_least(:once)
subject
end
end
context 'and there is no deployable for that older deployment' do context 'and there is no deployable for that older deployment' do
let(:older_deployment) { create(:deployment, :running, environment: environment, deployable: nil) } let(:older_deployment) { create(:deployment, :running, environment: environment, deployable: nil) }
......
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