Commit 7014abd8 authored by Thong Kuah's avatar Thong Kuah

Merge branch '196243-add-exclusive-guard' into 'master'

Add exclusive guard to ClusterUpdateAppWorker

See merge request gitlab-org/gitlab!24228
parents 20792230 5d9b8b23
---
title: Fix race condition bug in Prometheus managed app update process
merge_request: 24228
author:
type: fixed
......@@ -6,11 +6,24 @@ class ClusterUpdateAppWorker
include ApplicationWorker
include ClusterQueue
include ClusterApplications
include ExclusiveLeaseGuard
sidekiq_options retry: 3, dead: false
# rubocop: disable CodeReuse/ActiveRecord
LEASE_TIMEOUT = 10.minutes.to_i
def perform(app_name, app_id, project_id, scheduled_time)
@app_id = app_id
try_obtain_lease do
execute(app_name, app_id, project_id, scheduled_time)
end
end
private
# rubocop: disable CodeReuse/ActiveRecord
def execute(app_name, app_id, project_id, scheduled_time)
project = Project.find_by(id: project_id)
return unless project
......@@ -22,4 +35,12 @@ class ClusterUpdateAppWorker
end
end
# rubocop: enable CodeReuse/ActiveRecord
def lease_key
@lease_key ||= "#{self.class.name.underscore}-#{@app_id}"
end
def lease_timeout
LEASE_TIMEOUT
end
end
......@@ -3,7 +3,10 @@
require 'spec_helper'
describe ClusterUpdateAppWorker do
let(:project) { create(:project) }
include ExclusiveLeaseHelpers
let_it_be(:project) { create(:project) }
let(:prometheus_update_service) { spy }
subject { described_class.new }
......@@ -42,5 +45,54 @@ describe ClusterUpdateAppWorker do
subject.perform(application.name, application.id, project.id, Time.now)
end
context 'with exclusive lease' do
let(:application) { create(:clusters_applications_prometheus, :installed) }
let(:lease_key) { "#{described_class.name.underscore}-#{application.id}" }
before do
allow(Gitlab::ExclusiveLease).to receive(:new)
stub_exclusive_lease_taken(lease_key)
end
it 'does not allow same app to be updated concurrently by same project' do
expect(Clusters::Applications::PrometheusUpdateService).not_to receive(:new)
subject.perform(application.name, application.id, project.id, Time.now)
end
it 'does not allow same app to be updated concurrently by different project' do
project1 = create(:project)
expect(Clusters::Applications::PrometheusUpdateService).not_to receive(:new)
subject.perform(application.name, application.id, project1.id, Time.now)
end
it 'allows different app to be updated concurrently by same project' do
application2 = create(:clusters_applications_prometheus, :installed)
lease_key2 = "#{described_class.name.underscore}-#{application2.id}"
stub_exclusive_lease(lease_key2)
expect(Clusters::Applications::PrometheusUpdateService).to receive(:new)
.with(application2, project)
subject.perform(application2.name, application2.id, project.id, Time.now)
end
it 'allows different app to be updated by different project' do
application2 = create(:clusters_applications_prometheus, :installed)
lease_key2 = "#{described_class.name.underscore}-#{application2.id}"
project2 = create(:project)
stub_exclusive_lease(lease_key2)
expect(Clusters::Applications::PrometheusUpdateService).to receive(:new)
.with(application2, project2)
subject.perform(application2.name, application2.id, project2.id, Time.now)
end
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