Commit 27aaf1aa authored by Bob Van Landuyt's avatar Bob Van Landuyt

Add context to the pages domain cron workers

They all look very similar, so this adds context to them avoiding
extra queries.
parent dd5500f1
...@@ -62,6 +62,8 @@ class PagesDomain < ApplicationRecord ...@@ -62,6 +62,8 @@ class PagesDomain < ApplicationRecord
scope :for_removal, -> { where("remove_at < ?", Time.now) } scope :for_removal, -> { where("remove_at < ?", Time.now) }
scope :with_logging_info, -> { includes(project: [:namespace, :route]) }
def verified? def verified?
!!verified_at !!verified_at
end end
...@@ -285,3 +287,5 @@ class PagesDomain < ApplicationRecord ...@@ -285,3 +287,5 @@ class PagesDomain < ApplicationRecord
!auto_ssl_enabled? && project&.pages_https_only? !auto_ssl_enabled? && project&.pages_https_only?
end end
end end
PagesDomain.prepend_if_ee('::EE::PagesDomain')
...@@ -2,14 +2,14 @@ ...@@ -2,14 +2,14 @@
class PagesDomainRemovalCronWorker class PagesDomainRemovalCronWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue # rubocop:disable Scalability/CronWorkerContext include CronjobQueue
feature_category :pages feature_category :pages
worker_resource_boundary :cpu worker_resource_boundary :cpu
def perform def perform
PagesDomain.for_removal.find_each do |domain| PagesDomain.for_removal.with_logging_info.find_each do |domain|
domain.destroy! with_context(project: domain.project) { domain.destroy! }
rescue => e rescue => e
Gitlab::ErrorTracking.track_exception(e) Gitlab::ErrorTracking.track_exception(e)
end end
......
...@@ -2,15 +2,17 @@ ...@@ -2,15 +2,17 @@
class PagesDomainSslRenewalCronWorker class PagesDomainSslRenewalCronWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue # rubocop:disable Scalability/CronWorkerContext include CronjobQueue
feature_category :pages feature_category :pages
def perform def perform
return unless ::Gitlab::LetsEncrypt.enabled? return unless ::Gitlab::LetsEncrypt.enabled?
PagesDomain.need_auto_ssl_renewal.find_each do |domain| PagesDomain.need_auto_ssl_renewal.with_logging_info.find_each do |domain|
with_context(project: domain.project) do
PagesDomainSslRenewalWorker.perform_async(domain.id) PagesDomainSslRenewalWorker.perform_async(domain.id)
end end
end end
end
end end
...@@ -2,15 +2,17 @@ ...@@ -2,15 +2,17 @@
class PagesDomainVerificationCronWorker class PagesDomainVerificationCronWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue # rubocop:disable Scalability/CronWorkerContext include CronjobQueue
feature_category :pages feature_category :pages
def perform def perform
return if Gitlab::Database.read_only? return if Gitlab::Database.read_only?
PagesDomain.needs_verification.find_each do |domain| PagesDomain.needs_verification.with_logging_info.find_each do |domain|
with_context(project: domain.project) do
PagesDomainVerificationWorker.perform_async(domain.id) PagesDomainVerificationWorker.perform_async(domain.id)
end end
end end
end
end end
# frozen_string_literal: true
module EE
module PagesDomain
extend ActiveSupport::Concern
prepended do
scope :with_logging_info, -> { includes(project: [:route, { namespace: [:plan, :gitlab_subscription] }]) }
end
end
end
...@@ -380,5 +380,9 @@ x6zG6WoibsbsJMj70nwseUnPTBQNDP+j61RJjC/r ...@@ -380,5 +380,9 @@ x6zG6WoibsbsJMj70nwseUnPTBQNDP+j61RJjC/r
scope { :instance } scope { :instance }
usage { :serverless } usage { :serverless }
end end
trait :with_project do
association :project
end
end end
end end
# frozen_string_literal: true
RSpec.shared_examples 'a pages cronjob scheduling jobs with context' do |scheduled_worker_class|
let(:worker) { described_class.new }
it 'does not cause extra queries for multiple domains' do
control = ActiveRecord::QueryRecorder.new { worker.perform }
extra_domain
expect { worker.perform }.not_to exceed_query_limit(control)
end
it 'schedules the renewal with a context' do
extra_domain
worker.perform
expect(scheduled_worker_class.jobs.last).to include("meta.project" => extra_domain.project.full_path)
end
end
...@@ -12,7 +12,7 @@ describe PagesDomainSslRenewalCronWorker do ...@@ -12,7 +12,7 @@ describe PagesDomainSslRenewalCronWorker do
end end
describe '#perform' do describe '#perform' do
let(:project) { create :project } let_it_be(:project) { create :project }
let!(:domain) { create(:pages_domain, project: project, auto_ssl_enabled: false) } let!(:domain) { create(:pages_domain, project: project, auto_ssl_enabled: false) }
let!(:domain_with_enabled_auto_ssl) { create(:pages_domain, project: project, auto_ssl_enabled: true) } let!(:domain_with_enabled_auto_ssl) { create(:pages_domain, project: project, auto_ssl_enabled: true) }
let!(:domain_with_obtained_letsencrypt) do let!(:domain_with_obtained_letsencrypt) do
...@@ -35,12 +35,16 @@ describe PagesDomainSslRenewalCronWorker do ...@@ -35,12 +35,16 @@ describe PagesDomainSslRenewalCronWorker do
[domain, [domain,
domain_with_obtained_letsencrypt].each do |domain| domain_with_obtained_letsencrypt].each do |domain|
expect(PagesDomainVerificationWorker).not_to receive(:perform_async).with(domain.id) expect(PagesDomainSslRenewalWorker).not_to receive(:perform_async).with(domain.id)
end end
worker.perform worker.perform
end end
it_behaves_like 'a pages cronjob scheduling jobs with context', PagesDomainSslRenewalWorker do
let(:extra_domain) { create(:pages_domain, :with_project, auto_ssl_enabled: true) }
end
shared_examples 'does nothing' do shared_examples 'does nothing' do
it 'does nothing' do it 'does nothing' do
expect(PagesDomainSslRenewalWorker).not_to receive(:perform_async) expect(PagesDomainSslRenewalWorker).not_to receive(:perform_async)
......
...@@ -5,9 +5,9 @@ require 'spec_helper' ...@@ -5,9 +5,9 @@ require 'spec_helper'
describe PagesDomainVerificationCronWorker do describe PagesDomainVerificationCronWorker do
subject(:worker) { described_class.new } subject(:worker) { described_class.new }
describe '#perform' do describe '#perform', :sidekiq do
let!(:verified) { create(:pages_domain) } let!(:verified) { create(:pages_domain) }
let!(:reverify) { create(:pages_domain, :reverify) } let!(:reverify) { create(:pages_domain, :reverify, :with_project) }
let!(:disabled) { create(:pages_domain, :disabled) } let!(:disabled) { create(:pages_domain, :disabled) }
it 'does nothing if the database is read-only' do it 'does nothing if the database is read-only' do
...@@ -26,5 +26,9 @@ describe PagesDomainVerificationCronWorker do ...@@ -26,5 +26,9 @@ describe PagesDomainVerificationCronWorker do
worker.perform worker.perform
end end
it_behaves_like 'a pages cronjob scheduling jobs with context', PagesDomainVerificationWorker do
let(:extra_domain) { create(:pages_domain, :reverify, :with_project) }
end
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