Remove disabled pages domains

Domain will be removed by verification worker after 1 week
of being disabled
This commit is contained in:
Vladimir Shushlin 2019-04-30 12:05:54 +00:00 committed by Nick Thomas
parent 9e2958aed9
commit ac744fd4fc
10 changed files with 112 additions and 10 deletions

View file

@ -38,6 +38,8 @@ class PagesDomain < ApplicationRecord
where(verified_at.eq(nil).or(enabled_until.eq(nil).or(enabled_until.lt(threshold))))
end
scope :for_removal, -> { where("remove_at < ?", Time.now) }
def verified?
!!verified_at
end

View file

@ -6,6 +6,7 @@
- cronjob:gitlab_usage_ping
- cronjob:import_export_project_cleanup
- cronjob:pages_domain_verification_cron
- cronjob:pages_domain_removal_cron
- cronjob:pipeline_schedule
- cronjob:prune_old_events
- cronjob:remove_expired_group_links

View file

@ -0,0 +1,16 @@
# frozen_string_literal: true
class PagesDomainRemovalCronWorker
include ApplicationWorker
include CronjobQueue
def perform
return unless Feature.enabled?(:remove_disabled_domains)
PagesDomain.for_removal.find_each do |domain|
domain.destroy!
rescue => e
Raven.capture_exception(e)
end
end
end

View file

@ -0,0 +1,5 @@
---
title: Remove pages domains if they weren't verified for 1 week
merge_request: 26227
author:
type: added

View file

@ -338,6 +338,10 @@ Settings.cron_jobs['pages_domain_verification_cron_worker'] ||= Settingslogic.ne
Settings.cron_jobs['pages_domain_verification_cron_worker']['cron'] ||= '*/15 * * * *'
Settings.cron_jobs['pages_domain_verification_cron_worker']['job_class'] = 'PagesDomainVerificationCronWorker'
Settings.cron_jobs['pages_domain_removal_cron_worker'] ||= Settingslogic.new({})
Settings.cron_jobs['pages_domain_removal_cron_worker']['cron'] ||= '47 0 * * *'
Settings.cron_jobs['pages_domain_removal_cron_worker']['job_class'] = 'PagesDomainRemovalCronWorker'
Settings.cron_jobs['issue_due_scheduler_worker'] ||= Settingslogic.new({})
Settings.cron_jobs['issue_due_scheduler_worker']['cron'] ||= '50 00 * * *'
Settings.cron_jobs['issue_due_scheduler_worker']['job_class'] = 'IssueDueSchedulerWorker'

View file

@ -45,6 +45,10 @@ nNp/xedE1YxutQ==
remove_at { 1.day.from_now }
end
trait :should_be_removed do
remove_at { 1.day.ago }
end
trait :unverified do
verified_at nil
end

View file

@ -344,4 +344,32 @@ describe PagesDomain do
end
end
end
describe '.for_removal' do
subject { described_class.for_removal }
context 'when domain is not schedule for removal' do
let!(:domain) { create :pages_domain }
it 'does not return domain' do
is_expected.to be_empty
end
end
context 'when domain is scheduled for removal yesterday' do
let!(:domain) { create :pages_domain, remove_at: 1.day.ago }
it 'returns domain' do
is_expected.to eq([domain])
end
end
context 'when domain is scheduled for removal tomorrow' do
let!(:domain) { create :pages_domain, remove_at: 1.day.from_now }
it 'does not return domain' do
is_expected.to be_empty
end
end
end
end

View file

@ -57,12 +57,12 @@ describe VerifyPagesDomainService do
expect(domain).not_to be_verified
end
it 'disables domain and shedules it for removal' do
Timecop.freeze do
service.execute
expect(domain).not_to be_enabled
expect(domain.remove_at).to be_within(1.second).of(1.week.from_now)
end
it 'disables domain and shedules it for removal in 1 week' do
service.execute
expect(domain).not_to be_enabled
expect(domain.remove_at).to be_like_time(7.days.from_now)
end
end

View file

@ -0,0 +1,42 @@
# frozen_string_literal: true
require 'spec_helper'
describe PagesDomainRemovalCronWorker do
subject(:worker) { described_class.new }
describe '#perform' do
context 'when there is domain which should be removed' do
let!(:domain_for_removal) { create(:pages_domain, :should_be_removed) }
before do
stub_feature_flags(remove_disabled_domains: true)
end
it 'removes domain' do
expect { worker.perform }.to change { PagesDomain.count }.by(-1)
expect(PagesDomain.exists?).to eq(false)
end
context 'when domain removal is disabled' do
before do
stub_feature_flags(remove_disabled_domains: false)
end
it 'does not remove pages domain' do
expect { worker.perform }.not_to change { PagesDomain.count }
expect(PagesDomain.find_by(domain: domain_for_removal.domain)).to be_present
end
end
end
context 'where there is a domain which scheduled for removal in the future' do
let!(:domain_for_removal) { create(:pages_domain, :scheduled_for_removal) }
it 'does not remove pages domain' do
expect { worker.perform }.not_to change { PagesDomain.count }
expect(PagesDomain.find_by(domain: domain_for_removal.domain)).to be_present
end
end
end
end

View file

@ -6,11 +6,11 @@ describe PagesDomainVerificationCronWorker do
subject(:worker) { described_class.new }
describe '#perform' do
it 'enqueues a PagesDomainVerificationWorker for domains needing verification' do
verified = create(:pages_domain)
reverify = create(:pages_domain, :reverify)
disabled = create(:pages_domain, :disabled)
let!(:verified) { create(:pages_domain) }
let!(:reverify) { create(:pages_domain, :reverify) }
let!(:disabled) { create(:pages_domain, :disabled) }
it 'enqueues a PagesDomainVerificationWorker for domains needing verification' do
[reverify, disabled].each do |domain|
expect(PagesDomainVerificationWorker).to receive(:perform_async).with(domain.id)
end