Skip to content

G
gitlab-ce
Commit d9b99c31 authored by Bob Van Landuyt's avatar Bob Van Landuyt
Browse files
Options

Merge branch '325273-remove-feature-flag' into 'master' 

Remove the container expiration loopless feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!63873
parents 8c7c7a7d 5aca511a
Hide whitespace changes
Inline Side-by-side
Showing with 367 additions and 645 deletions
app/services/container_expiration_policies/cleanup_service.rb
View file @ d9b99c31
......@@ -49,7 +49,6 @@ module ContainerExpirationPolicies
private
def schedule_next_run_if_needed
return unless Feature.enabled?(:container_registry_expiration_policies_loopless)
return if policy.next_run_at.future?
repos_before_next_run = ::ContainerRepository.for_project_id(policy.project_id)
......
app/workers/container_expiration_policies/cleanup_container_repository_worker.rb
View file @ d9b99c31
......@@ -65,19 +65,9 @@ module ContainerExpirationPolicies
def container_repository
strong_memoize(:container_repository) do
ContainerRepository.transaction do
# rubocop: disable CodeReuse/ActiveRecord
# We need a lock to prevent two workers from picking up the same row
container_repository = if loopless_enabled?
next_container_repository
else
ContainerRepository.waiting_for_cleanup
.order(:expiration_policy_cleanup_status, :expiration_policy_started_at)
.limit(1)
.lock('FOR UPDATE SKIP LOCKED')
.first
end
# rubocop: enable CodeReuse/ActiveRecord
container_repository = next_container_repository
container_repository&.tap(&:cleanup_ongoing!)
end
end
......@@ -102,28 +92,20 @@ module ContainerExpirationPolicies
def cleanup_scheduled_count
strong_memoize(:cleanup_scheduled_count) do
if loopless_enabled?
limit = max_running_jobs + 1
ContainerExpirationPolicy.with_container_repositories
.runnable_schedules
.limit(limit)
.count
else
ContainerRepository.cleanup_scheduled.count
end
limit = max_running_jobs + 1
ContainerExpirationPolicy.with_container_repositories
.runnable_schedules
.limit(limit)
.count
end
end
def cleanup_unfinished_count
strong_memoize(:cleanup_unfinished_count) do
if loopless_enabled?
limit = max_running_jobs + 1
ContainerRepository.with_unfinished_cleanup
.limit(limit)
.count
else
ContainerRepository.cleanup_unfinished.count
end
limit = max_running_jobs + 1
ContainerRepository.with_unfinished_cleanup
.limit(limit)
.count
end
end
......@@ -132,21 +114,13 @@ module ContainerExpirationPolicies
now = Time.zone.now
if loopless_enabled?
policy.next_run_at < now || (now + max_cleanup_execution_time.seconds < policy.next_run_at)
else
now + max_cleanup_execution_time.seconds < policy.next_run_at
end
policy.next_run_at < now || (now + max_cleanup_execution_time.seconds < policy.next_run_at)
end
def throttling_enabled?
Feature.enabled?(:container_registry_expiration_policies_throttling)
end
def loopless_enabled?
Feature.enabled?(:container_registry_expiration_policies_loopless)
end
def max_cleanup_execution_time
::Gitlab::CurrentSettings.container_registry_delete_tags_service_timeout
end
......
app/workers/container_expiration_policy_worker.rb
View file @ d9b99c31
......@@ -38,18 +38,6 @@ class ContainerExpirationPolicyWorker # rubocop:disable Scalability/IdempotentWo
def perform_throttled
try_obtain_lease do
unless loopless_enabled?
with_runnable_policy do |policy|
ContainerExpirationPolicy.transaction do
policy.schedule_next_run!
ContainerRepository.for_project_id(policy.id)
.each_batch do |relation|
relation.update_all(expiration_policy_cleanup_status: :cleanup_scheduled)
end
end
end
end
ContainerExpirationPolicies::CleanupContainerRepositoryWorker.perform_with_capacity
end
end
......@@ -86,10 +74,6 @@ class ContainerExpirationPolicyWorker # rubocop:disable Scalability/IdempotentWo
Feature.enabled?(:container_registry_expiration_policies_throttling)
end
def loopless_enabled?
Feature.enabled?(:container_registry_expiration_policies_loopless)
end
def lease_timeout
5.hours
end
......
config/feature_flags/development/container_registry_expiration_policies_loopless.yml deleted 100644 → 0
View file @ 8c7c7a7d
---
name: container_registry_expiration_policies_loopless
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56962
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/325273
milestone: '13.11'
type: development
group: group::package
default_enabled: false
spec/services/container_expiration_policies/cleanup_service_spec.rb
View file @ d9b99c31
......@@ -9,37 +9,68 @@ RSpec.describe ContainerExpirationPolicies::CleanupService do
let(:service) { described_class.new(repository) }
describe '#execute' do
let(:policy) { repository.project.container_expiration_policy }
subject { service.execute }
shared_examples 'cleaning up a container repository' do
context 'with a successful cleanup tags service execution' do
let(:cleanup_tags_service_params) { project.container_expiration_policy.policy_params.merge('container_expiration_policy' => true) }
let(:cleanup_tags_service) { instance_double(Projects::ContainerRepository::CleanupTagsService) }
before do
policy.update!(enabled: true)
policy.update_column(:next_run_at, 5.minutes.ago)
end
it 'completely clean up the repository' do
expect(Projects::ContainerRepository::CleanupTagsService)
.to receive(:new).with(project, nil, cleanup_tags_service_params).and_return(cleanup_tags_service)
expect(cleanup_tags_service).to receive(:execute).with(repository).and_return(status: :success)
context 'with a successful cleanup tags service execution' do
let(:cleanup_tags_service_params) { project.container_expiration_policy.policy_params.merge('container_expiration_policy' => true) }
let(:cleanup_tags_service) { instance_double(Projects::ContainerRepository::CleanupTagsService) }
response = subject
it 'completely clean up the repository' do
expect(Projects::ContainerRepository::CleanupTagsService)
.to receive(:new).with(project, nil, cleanup_tags_service_params).and_return(cleanup_tags_service)
expect(cleanup_tags_service).to receive(:execute).with(repository).and_return(status: :success)
aggregate_failures "checking the response and container repositories" do
expect(response.success?).to eq(true)
expect(response.payload).to include(cleanup_status: :finished, container_repository_id: repository.id)
expect(ContainerRepository.waiting_for_cleanup.count).to eq(0)
expect(repository.reload.cleanup_unscheduled?).to be_truthy
expect(repository.expiration_policy_completed_at).not_to eq(nil)
expect(repository.expiration_policy_started_at).not_to eq(nil)
end
response = subject
aggregate_failures "checking the response and container repositories" do
expect(response.success?).to eq(true)
expect(response.payload).to include(cleanup_status: :finished, container_repository_id: repository.id)
expect(ContainerRepository.waiting_for_cleanup.count).to eq(0)
expect(repository.reload.cleanup_unscheduled?).to be_truthy
expect(repository.expiration_policy_completed_at).not_to eq(nil)
expect(repository.expiration_policy_started_at).not_to eq(nil)
end
end
end
context 'without a successful cleanup tags service execution' do
let(:cleanup_tags_service_response) { { status: :error, message: 'timeout' } }
context 'without a successful cleanup tags service execution' do
let(:cleanup_tags_service_response) { { status: :error, message: 'timeout' } }
before do
expect(Projects::ContainerRepository::CleanupTagsService)
.to receive(:new).and_return(double(execute: cleanup_tags_service_response))
before do
expect(Projects::ContainerRepository::CleanupTagsService)
.to receive(:new).and_return(double(execute: cleanup_tags_service_response))
end
it 'partially clean up the repository' do
response = subject
aggregate_failures "checking the response and container repositories" do
expect(response.success?).to eq(true)
expect(response.payload).to include(cleanup_status: :unfinished, container_repository_id: repository.id)
expect(ContainerRepository.waiting_for_cleanup.count).to eq(1)
expect(repository.reload.cleanup_unfinished?).to be_truthy
expect(repository.expiration_policy_started_at).not_to eq(nil)
expect(repository.expiration_policy_completed_at).to eq(nil)
end
end
context 'with a truncated cleanup tags service response' do
let(:cleanup_tags_service_response) do
{
status: :error,
original_size: 1000,
before_truncate_size: 800,
after_truncate_size: 200,
before_delete_size: 100,
deleted_size: 100
}
end
it 'partially clean up the repository' do
......@@ -47,179 +78,134 @@ RSpec.describe ContainerExpirationPolicies::CleanupService do
aggregate_failures "checking the response and container repositories" do
expect(response.success?).to eq(true)
expect(response.payload).to include(cleanup_status: :unfinished, container_repository_id: repository.id)
expect(response.payload)
.to include(
cleanup_status: :unfinished,
container_repository_id: repository.id,
cleanup_tags_service_original_size: 1000,
cleanup_tags_service_before_truncate_size: 800,
cleanup_tags_service_after_truncate_size: 200,
cleanup_tags_service_before_delete_size: 100,
cleanup_tags_service_deleted_size: 100
)
expect(ContainerRepository.waiting_for_cleanup.count).to eq(1)
expect(repository.reload.cleanup_unfinished?).to be_truthy
expect(repository.expiration_policy_started_at).not_to eq(nil)
expect(repository.expiration_policy_completed_at).to eq(nil)
end
end
context 'with a truncated cleanup tags service response' do
let(:cleanup_tags_service_response) do
{
status: :error,
original_size: 1000,
before_truncate_size: 800,
after_truncate_size: 200,
before_delete_size: 100,
deleted_size: 100
}
end
it 'partially clean up the repository' do
response = subject
aggregate_failures "checking the response and container repositories" do
expect(response.success?).to eq(true)
expect(response.payload)
.to include(
cleanup_status: :unfinished,
container_repository_id: repository.id,
cleanup_tags_service_original_size: 1000,
cleanup_tags_service_before_truncate_size: 800,
cleanup_tags_service_after_truncate_size: 200,
cleanup_tags_service_before_delete_size: 100,
cleanup_tags_service_deleted_size: 100
)
expect(ContainerRepository.waiting_for_cleanup.count).to eq(1)
expect(repository.reload.cleanup_unfinished?).to be_truthy
expect(repository.expiration_policy_started_at).not_to eq(nil)
expect(repository.expiration_policy_completed_at).to eq(nil)
end
end
end
end
end
context 'with no repository' do
let(:service) { described_class.new(nil) }
context 'with no repository' do
let(:service) { described_class.new(nil) }
it 'returns an error response' do
expect(subject.success?).to eq(false)
expect(subject.message).to eq('no repository')
end
it 'returns an error response' do
expect(subject.success?).to eq(false)
expect(subject.message).to eq('no repository')
end
end
context 'with an invalid policy' do
let(:policy) { repository.project.container_expiration_policy }
context 'with an invalid policy' do
let(:policy) { repository.project.container_expiration_policy }
before do
policy.name_regex = nil
policy.enabled = true
repository.expiration_policy_cleanup_status = :cleanup_ongoing
end
before do
policy.name_regex = nil
policy.enabled = true
repository.expiration_policy_cleanup_status = :cleanup_ongoing
end
it 'returns an error response' do
expect { subject }.to change { repository.expiration_policy_cleanup_status }.from('cleanup_ongoing').to('cleanup_unscheduled')
expect(subject.success?).to eq(false)
expect(subject.message).to eq('invalid policy')
expect(policy).not_to be_enabled
end
it 'returns an error response' do
expect { subject }.to change { repository.expiration_policy_cleanup_status }.from('cleanup_ongoing').to('cleanup_unscheduled')
expect(subject.success?).to eq(false)
expect(subject.message).to eq('invalid policy')
expect(policy).not_to be_enabled
end
end
context 'with a network error' do
before do
expect(Projects::ContainerRepository::CleanupTagsService)
.to receive(:new).and_raise(Faraday::TimeoutError)
end
context 'with a network error' do
before do
expect(Projects::ContainerRepository::CleanupTagsService)
.to receive(:new).and_raise(Faraday::TimeoutError)
end
it 'raises an error' do
expect { subject }.to raise_error(Faraday::TimeoutError)
it 'raises an error' do
expect { subject }.to raise_error(Faraday::TimeoutError)
expect(ContainerRepository.waiting_for_cleanup.count).to eq(1)
expect(repository.reload.cleanup_unfinished?).to be_truthy
expect(repository.expiration_policy_started_at).not_to eq(nil)
expect(repository.expiration_policy_completed_at).to eq(nil)
end
expect(ContainerRepository.waiting_for_cleanup.count).to eq(1)
expect(repository.reload.cleanup_unfinished?).to be_truthy
expect(repository.expiration_policy_started_at).not_to eq(nil)
expect(repository.expiration_policy_completed_at).to eq(nil)
end
end
context 'with loopless enabled' do
let(:policy) { repository.project.container_expiration_policy }
context 'next run scheduling' do
let_it_be_with_reload(:repository2) { create(:container_repository, project: project) }
let_it_be_with_reload(:repository3) { create(:container_repository, project: project) }
before do
policy.update!(enabled: true)
policy.update_column(:next_run_at, 5.minutes.ago)
cleanup_tags_service = instance_double(Projects::ContainerRepository::CleanupTagsService)
allow(Projects::ContainerRepository::CleanupTagsService)
.to receive(:new).and_return(cleanup_tags_service)
allow(cleanup_tags_service).to receive(:execute).and_return(status: :success)
end
it_behaves_like 'cleaning up a container repository'
context 'next run scheduling' do
let_it_be_with_reload(:repository2) { create(:container_repository, project: project) }
let_it_be_with_reload(:repository3) { create(:container_repository, project: project) }
shared_examples 'not scheduling the next run' do
it 'does not scheduled the next run' do
expect(policy).not_to receive(:schedule_next_run!)
before do
cleanup_tags_service = instance_double(Projects::ContainerRepository::CleanupTagsService)
allow(Projects::ContainerRepository::CleanupTagsService)
.to receive(:new).and_return(cleanup_tags_service)
allow(cleanup_tags_service).to receive(:execute).and_return(status: :success)
end
shared_examples 'not scheduling the next run' do
it 'does not scheduled the next run' do
expect(policy).not_to receive(:schedule_next_run!)
expect { subject }.not_to change { policy.reload.next_run_at }
end
expect { subject }.not_to change { policy.reload.next_run_at }
end
end
shared_examples 'scheduling the next run' do
it 'schedules the next run' do
expect(policy).to receive(:schedule_next_run!).and_call_original
shared_examples 'scheduling the next run' do
it 'schedules the next run' do
expect(policy).to receive(:schedule_next_run!).and_call_original
expect { subject }.to change { policy.reload.next_run_at }
end
expect { subject }.to change { policy.reload.next_run_at }
end
end
context 'with cleanups started_at before policy next_run_at' do
before do
ContainerRepository.update_all(expiration_policy_started_at: 10.minutes.ago)
end
it_behaves_like 'not scheduling the next run'
context 'with cleanups started_at before policy next_run_at' do
before do
ContainerRepository.update_all(expiration_policy_started_at: 10.minutes.ago)
end
context 'with cleanups started_at around policy next_run_at' do
before do
repository3.update!(expiration_policy_started_at: policy.next_run_at + 10.minutes.ago)
end
it_behaves_like 'not scheduling the next run'
end
it_behaves_like 'not scheduling the next run'
context 'with cleanups started_at around policy next_run_at' do
before do
repository3.update!(expiration_policy_started_at: policy.next_run_at + 10.minutes.ago)
end
context 'with only the current repository started_at before the policy next_run_at' do
before do
repository2.update!(expiration_policy_started_at: policy.next_run_at + 10.minutes)
repository3.update!(expiration_policy_started_at: policy.next_run_at + 12.minutes)
end
it_behaves_like 'not scheduling the next run'
end
it_behaves_like 'scheduling the next run'
context 'with only the current repository started_at before the policy next_run_at' do
before do
repository2.update!(expiration_policy_started_at: policy.next_run_at + 10.minutes)
repository3.update!(expiration_policy_started_at: policy.next_run_at + 12.minutes)
end
context 'with cleanups started_at after policy next_run_at' do
before do
ContainerRepository.update_all(expiration_policy_started_at: policy.next_run_at + 10.minutes)
end
it_behaves_like 'scheduling the next run'
end
it_behaves_like 'scheduling the next run'
context 'with cleanups started_at after policy next_run_at' do
before do
ContainerRepository.update_all(expiration_policy_started_at: policy.next_run_at + 10.minutes)
end
context 'with a future policy next_run_at' do
before do
policy.update_column(:next_run_at, 5.minutes.from_now)
end
it_behaves_like 'scheduling the next run'
end
it_behaves_like 'not scheduling the next run'
context 'with a future policy next_run_at' do
before do
policy.update_column(:next_run_at, 5.minutes.from_now)
end
end
end
context 'with loopless disabled' do
before do
stub_feature_flags(container_registry_expiration_policies_loopless: false)
it_behaves_like 'not scheduling the next run'
end
it_behaves_like 'cleaning up a container repository'
end
end
end
spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb
View file @ d9b99c31
......@@ -85,7 +85,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
context 'with policy running shortly' do
before do
repository.cleanup_unfinished! if loopless_enabled?
repository.cleanup_unfinished!
policy.update_column(:next_run_at, 1.minute.from_now)
end
......@@ -108,371 +108,261 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
it 'skips the repository' do
expect(ContainerExpirationPolicies::CleanupService).not_to receive(:new)
if loopless_enabled?
expect { subject }
.to not_change { ContainerRepository.waiting_for_cleanup.count }
.and not_change { repository.reload.expiration_policy_cleanup_status }
else
expect { subject }.to change { ContainerRepository.waiting_for_cleanup.count }.from(1).to(0)
expect(repository.reload.cleanup_unscheduled?).to be_truthy
end
expect { subject }
.to not_change { ContainerRepository.waiting_for_cleanup.count }
.and not_change { repository.reload.expiration_policy_cleanup_status }
end
end
end
context 'with loopless enabled' do
context 'with repository in cleanup unscheduled state' do
before do
stub_feature_flags(container_registry_expiration_policies_loopless: true)
policy.update_column(:next_run_at, 5.minutes.ago)
end
context 'with repository in cleanup unscheduled state' do
before do
policy.update_column(:next_run_at, 5.minutes.ago)
end
it_behaves_like 'handling all repository conditions'
end
it_behaves_like 'handling all repository conditions'
context 'with repository in cleanup unfinished state' do
before do
repository.cleanup_unfinished!
end
context 'with repository in cleanup unfinished state' do
before do
repository.cleanup_unfinished!
end
it_behaves_like 'handling all repository conditions'
end
it_behaves_like 'handling all repository conditions'
end
context 'container repository selection' do
where(:repository_cleanup_status, :repository_policy_status, :other_repository_cleanup_status, :other_repository_policy_status, :expected_selected_repository) do
:unscheduled | :disabled | :unscheduled | :disabled | :none
:unscheduled | :disabled | :unscheduled | :runnable | :other_repository
:unscheduled | :disabled | :unscheduled | :not_runnable | :none
context 'container repository selection' do
where(:repository_cleanup_status, :repository_policy_status, :other_repository_cleanup_status, :other_repository_policy_status, :expected_selected_repository) do
:unscheduled | :disabled | :unscheduled | :disabled | :none
:unscheduled | :disabled | :unscheduled | :runnable | :other_repository
:unscheduled | :disabled | :unscheduled | :not_runnable | :none
:unscheduled | :disabled | :scheduled | :disabled | :none
:unscheduled | :disabled | :scheduled | :runnable | :other_repository
:unscheduled | :disabled | :scheduled | :not_runnable | :none
:unscheduled | :disabled | :scheduled | :disabled | :none
:unscheduled | :disabled | :scheduled | :runnable | :other_repository
:unscheduled | :disabled | :scheduled | :not_runnable | :none
:unscheduled | :disabled | :unfinished | :disabled | :none
:unscheduled | :disabled | :unfinished | :runnable | :other_repository
:unscheduled | :disabled | :unfinished | :not_runnable | :other_repository
:unscheduled | :disabled | :unfinished | :disabled | :none
:unscheduled | :disabled | :unfinished | :runnable | :other_repository
:unscheduled | :disabled | :unfinished | :not_runnable | :other_repository
:unscheduled | :disabled | :ongoing | :disabled | :none
:unscheduled | :disabled | :ongoing | :runnable | :none
:unscheduled | :disabled | :ongoing | :not_runnable | :none
:unscheduled | :disabled | :ongoing | :disabled | :none
:unscheduled | :disabled | :ongoing | :runnable | :none
:unscheduled | :disabled | :ongoing | :not_runnable | :none
:unscheduled | :runnable | :unscheduled | :disabled | :repository
:unscheduled | :runnable | :unscheduled | :runnable | :repository
:unscheduled | :runnable | :unscheduled | :not_runnable | :repository
:unscheduled | :runnable | :unscheduled | :disabled | :repository
:unscheduled | :runnable | :unscheduled | :runnable | :repository
:unscheduled | :runnable | :unscheduled | :not_runnable | :repository
:unscheduled | :runnable | :scheduled | :disabled | :repository
:unscheduled | :runnable | :scheduled | :runnable | :repository
:unscheduled | :runnable | :scheduled | :not_runnable | :repository
:unscheduled | :runnable | :scheduled | :disabled | :repository
:unscheduled | :runnable | :scheduled | :runnable | :repository
:unscheduled | :runnable | :scheduled | :not_runnable | :repository
:unscheduled | :runnable | :unfinished | :disabled | :repository
:unscheduled | :runnable | :unfinished | :runnable | :repository
:unscheduled | :runnable | :unfinished | :not_runnable | :repository
:unscheduled | :runnable | :unfinished | :disabled | :repository
:unscheduled | :runnable | :unfinished | :runnable | :repository
:unscheduled | :runnable | :unfinished | :not_runnable | :repository
:unscheduled | :runnable | :ongoing | :disabled | :repository
:unscheduled | :runnable | :ongoing | :runnable | :repository
:unscheduled | :runnable | :ongoing | :not_runnable | :repository
:unscheduled | :runnable | :ongoing | :disabled | :repository
:unscheduled | :runnable | :ongoing | :runnable | :repository
:unscheduled | :runnable | :ongoing | :not_runnable | :repository
:scheduled | :disabled | :unscheduled | :disabled | :none
:scheduled | :disabled | :unscheduled | :runnable | :other_repository
:scheduled | :disabled | :unscheduled | :not_runnable | :none
:scheduled | :disabled | :unscheduled | :disabled | :none
:scheduled | :disabled | :unscheduled | :runnable | :other_repository
:scheduled | :disabled | :unscheduled | :not_runnable | :none
:scheduled | :disabled | :scheduled | :disabled | :none
:scheduled | :disabled | :scheduled | :runnable | :other_repository
:scheduled | :disabled | :scheduled | :not_runnable | :none
:scheduled | :disabled | :scheduled | :disabled | :none
:scheduled | :disabled | :scheduled | :runnable | :other_repository
:scheduled | :disabled | :scheduled | :not_runnable | :none
:scheduled | :disabled | :unfinished | :disabled | :none
:scheduled | :disabled | :unfinished | :runnable | :other_repository
:scheduled | :disabled | :unfinished | :not_runnable | :other_repository
:scheduled | :disabled | :unfinished | :disabled | :none
:scheduled | :disabled | :unfinished | :runnable | :other_repository
:scheduled | :disabled | :unfinished | :not_runnable | :other_repository
:scheduled | :disabled | :ongoing | :disabled | :none
:scheduled | :disabled | :ongoing | :runnable | :none
:scheduled | :disabled | :ongoing | :not_runnable | :none
:scheduled | :disabled | :ongoing | :disabled | :none
:scheduled | :disabled | :ongoing | :runnable | :none
:scheduled | :disabled | :ongoing | :not_runnable | :none
:scheduled | :runnable | :unscheduled | :disabled | :repository
:scheduled | :runnable | :unscheduled | :runnable | :other_repository
:scheduled | :runnable | :unscheduled | :not_runnable | :repository
:scheduled | :runnable | :unscheduled | :disabled | :repository
:scheduled | :runnable | :unscheduled | :runnable | :other_repository
:scheduled | :runnable | :unscheduled | :not_runnable | :repository
:scheduled | :runnable | :scheduled | :disabled | :repository
:scheduled | :runnable | :scheduled | :runnable | :repository
:scheduled | :runnable | :scheduled | :not_runnable | :repository
:scheduled | :runnable | :scheduled | :disabled | :repository
:scheduled | :runnable | :scheduled | :runnable | :repository
:scheduled | :runnable | :scheduled | :not_runnable | :repository
:scheduled | :runnable | :unfinished | :disabled | :repository
:scheduled | :runnable | :unfinished | :runnable | :repository
:scheduled | :runnable | :unfinished | :not_runnable | :repository
:scheduled | :runnable | :unfinished | :disabled | :repository
:scheduled | :runnable | :unfinished | :runnable | :repository
:scheduled | :runnable | :unfinished | :not_runnable | :repository
:scheduled | :runnable | :ongoing | :disabled | :repository
:scheduled | :runnable | :ongoing | :runnable | :repository
:scheduled | :runnable | :ongoing | :not_runnable | :repository
:scheduled | :runnable | :ongoing | :disabled | :repository
:scheduled | :runnable | :ongoing | :runnable | :repository
:scheduled | :runnable | :ongoing | :not_runnable | :repository
:scheduled | :not_runnable | :unscheduled | :disabled | :none
:scheduled | :not_runnable | :unscheduled | :runnable | :other_repository
:scheduled | :not_runnable | :unscheduled | :not_runnable | :none
:scheduled | :not_runnable | :unscheduled | :disabled | :none
:scheduled | :not_runnable | :unscheduled | :runnable | :other_repository
:scheduled | :not_runnable | :unscheduled | :not_runnable | :none
:scheduled | :not_runnable | :scheduled | :disabled | :none
:scheduled | :not_runnable | :scheduled | :runnable | :other_repository
:scheduled | :not_runnable | :scheduled | :not_runnable | :none
:scheduled | :not_runnable | :scheduled | :disabled | :none
:scheduled | :not_runnable | :scheduled | :runnable | :other_repository
:scheduled | :not_runnable | :scheduled | :not_runnable | :none
:scheduled | :not_runnable | :unfinished | :disabled | :none
:scheduled | :not_runnable | :unfinished | :runnable | :other_repository
:scheduled | :not_runnable | :unfinished | :not_runnable | :other_repository
:scheduled | :not_runnable | :unfinished | :disabled | :none
:scheduled | :not_runnable | :unfinished | :runnable | :other_repository
:scheduled | :not_runnable | :unfinished | :not_runnable | :other_repository
:scheduled | :not_runnable | :ongoing | :disabled | :none
:scheduled | :not_runnable | :ongoing | :runnable | :none
:scheduled | :not_runnable | :ongoing | :not_runnable | :none
:scheduled | :not_runnable | :ongoing | :disabled | :none
:scheduled | :not_runnable | :ongoing | :runnable | :none
:scheduled | :not_runnable | :ongoing | :not_runnable | :none
:unfinished | :disabled | :unscheduled | :disabled | :none
:unfinished | :disabled | :unscheduled | :runnable | :other_repository
:unfinished | :disabled | :unscheduled | :not_runnable | :none
:unfinished | :disabled | :unscheduled | :disabled | :none
:unfinished | :disabled | :unscheduled | :runnable | :other_repository
:unfinished | :disabled | :unscheduled | :not_runnable | :none
:unfinished | :disabled | :scheduled | :disabled | :none
:unfinished | :disabled | :scheduled | :runnable | :other_repository
:unfinished | :disabled | :scheduled | :not_runnable | :none
:unfinished | :disabled | :scheduled | :disabled | :none
:unfinished | :disabled | :scheduled | :runnable | :other_repository
:unfinished | :disabled | :scheduled | :not_runnable | :none
:unfinished | :disabled | :unfinished | :disabled | :none
:unfinished | :disabled | :unfinished | :runnable | :other_repository
:unfinished | :disabled | :unfinished | :not_runnable | :other_repository
:unfinished | :disabled | :unfinished | :disabled | :none
:unfinished | :disabled | :unfinished | :runnable | :other_repository
:unfinished | :disabled | :unfinished | :not_runnable | :other_repository
:unfinished | :disabled | :ongoing | :disabled | :none
:unfinished | :disabled | :ongoing | :runnable | :none
:unfinished | :disabled | :ongoing | :not_runnable | :none
:unfinished | :disabled | :ongoing | :disabled | :none
:unfinished | :disabled | :ongoing | :runnable | :none
:unfinished | :disabled | :ongoing | :not_runnable | :none
:unfinished | :runnable | :unscheduled | :disabled | :repository
:unfinished | :runnable | :unscheduled | :runnable | :other_repository
:unfinished | :runnable | :unscheduled | :not_runnable | :repository
:unfinished | :runnable | :scheduled | :disabled | :repository
:unfinished | :runnable | :scheduled | :runnable | :other_repository
:unfinished | :runnable | :scheduled | :not_runnable | :repository
:unfinished | :runnable | :unscheduled | :disabled | :repository
:unfinished | :runnable | :unscheduled | :runnable | :other_repository
:unfinished | :runnable | :unscheduled | :not_runnable | :repository
:unfinished | :runnable | :scheduled | :disabled | :repository
:unfinished | :runnable | :scheduled | :runnable | :other_repository
:unfinished | :runnable | :scheduled | :not_runnable | :repository
:unfinished | :runnable | :unfinished | :disabled | :repository
:unfinished | :runnable | :unfinished | :runnable | :repository
:unfinished | :runnable | :unfinished | :not_runnable | :repository
:unfinished | :runnable | :unfinished | :disabled | :repository
:unfinished | :runnable | :unfinished | :runnable | :repository
:unfinished | :runnable | :unfinished | :not_runnable | :repository
:unfinished | :runnable | :ongoing | :disabled | :repository
:unfinished | :runnable | :ongoing | :runnable | :repository
:unfinished | :runnable | :ongoing | :not_runnable | :repository
:unfinished | :runnable | :ongoing | :disabled | :repository
:unfinished | :runnable | :ongoing | :runnable | :repository
:unfinished | :runnable | :ongoing | :not_runnable | :repository
:unfinished | :not_runnable | :unscheduled | :disabled | :repository
:unfinished | :not_runnable | :unscheduled | :runnable | :other_repository
:unfinished | :not_runnable | :unscheduled | :not_runnable | :repository
:unfinished | :not_runnable | :unscheduled | :disabled | :repository
:unfinished | :not_runnable | :unscheduled | :runnable | :other_repository
:unfinished | :not_runnable | :unscheduled | :not_runnable | :repository
:unfinished | :not_runnable | :scheduled | :disabled | :repository
:unfinished | :not_runnable | :scheduled | :runnable | :other_repository
:unfinished | :not_runnable | :scheduled | :not_runnable | :repository
:unfinished | :not_runnable | :scheduled | :disabled | :repository
:unfinished | :not_runnable | :scheduled | :runnable | :other_repository
:unfinished | :not_runnable | :scheduled | :not_runnable | :repository
:unfinished | :not_runnable | :unfinished | :disabled | :repository
:unfinished | :not_runnable | :unfinished | :runnable | :repository
:unfinished | :not_runnable | :unfinished | :not_runnable | :repository
:unfinished | :not_runnable | :unfinished | :disabled | :repository
:unfinished | :not_runnable | :unfinished | :runnable | :repository
:unfinished | :not_runnable | :unfinished | :not_runnable | :repository
:unfinished | :not_runnable | :ongoing | :disabled | :repository
:unfinished | :not_runnable | :ongoing | :runnable | :repository
:unfinished | :not_runnable | :ongoing | :not_runnable | :repository
:unfinished | :not_runnable | :ongoing | :disabled | :repository
:unfinished | :not_runnable | :ongoing | :runnable | :repository
:unfinished | :not_runnable | :ongoing | :not_runnable | :repository
:ongoing | :disabled | :unscheduled | :disabled | :none
:ongoing | :disabled | :unscheduled | :runnable | :other_repository
:ongoing | :disabled | :unscheduled | :not_runnable | :none
:ongoing | :disabled | :unscheduled | :disabled | :none
:ongoing | :disabled | :unscheduled | :runnable | :other_repository
:ongoing | :disabled | :unscheduled | :not_runnable | :none
:ongoing | :disabled | :scheduled | :disabled | :none
:ongoing | :disabled | :scheduled | :runnable | :other_repository
:ongoing | :disabled | :scheduled | :not_runnable | :none
:ongoing | :disabled | :scheduled | :disabled | :none
:ongoing | :disabled | :scheduled | :runnable | :other_repository
:ongoing | :disabled | :scheduled | :not_runnable | :none
:ongoing | :disabled | :unfinished | :disabled | :none
:ongoing | :disabled | :unfinished | :runnable | :other_repository
:ongoing | :disabled | :unfinished | :not_runnable | :other_repository
:ongoing | :disabled | :unfinished | :disabled | :none
:ongoing | :disabled | :unfinished | :runnable | :other_repository
:ongoing | :disabled | :unfinished | :not_runnable | :other_repository
:ongoing | :disabled | :ongoing | :disabled | :none
:ongoing | :disabled | :ongoing | :runnable | :none
:ongoing | :disabled | :ongoing | :not_runnable | :none
:ongoing | :disabled | :ongoing | :disabled | :none
:ongoing | :disabled | :ongoing | :runnable | :none
:ongoing | :disabled | :ongoing | :not_runnable | :none
:ongoing | :runnable | :unscheduled | :disabled | :none
:ongoing | :runnable | :unscheduled | :runnable | :other_repository
:ongoing | :runnable | :unscheduled | :not_runnable | :none
:ongoing | :runnable | :unscheduled | :disabled | :none
:ongoing | :runnable | :unscheduled | :runnable | :other_repository
:ongoing | :runnable | :unscheduled | :not_runnable | :none
:ongoing | :runnable | :scheduled | :disabled | :none
:ongoing | :runnable | :scheduled | :runnable | :other_repository
:ongoing | :runnable | :scheduled | :not_runnable | :none
:ongoing | :runnable | :scheduled | :disabled | :none
:ongoing | :runnable | :scheduled | :runnable | :other_repository
:ongoing | :runnable | :scheduled | :not_runnable | :none
:ongoing | :runnable | :unfinished | :disabled | :none
:ongoing | :runnable | :unfinished | :runnable | :other_repository
:ongoing | :runnable | :unfinished | :not_runnable | :other_repository
:ongoing | :runnable | :unfinished | :disabled | :none
:ongoing | :runnable | :unfinished | :runnable | :other_repository
:ongoing | :runnable | :unfinished | :not_runnable | :other_repository
:ongoing | :runnable | :ongoing | :disabled | :none
:ongoing | :runnable | :ongoing | :runnable | :none
:ongoing | :runnable | :ongoing | :not_runnable | :none
:ongoing | :runnable | :ongoing | :disabled | :none
:ongoing | :runnable | :ongoing | :runnable | :none
:ongoing | :runnable | :ongoing | :not_runnable | :none
:ongoing | :not_runnable | :unscheduled | :disabled | :none
:ongoing | :not_runnable | :unscheduled | :runnable | :other_repository
:ongoing | :not_runnable | :unscheduled | :not_runnable | :none
:ongoing | :not_runnable | :unscheduled | :disabled | :none
:ongoing | :not_runnable | :unscheduled | :runnable | :other_repository
:ongoing | :not_runnable | :unscheduled | :not_runnable | :none
:ongoing | :not_runnable | :scheduled | :disabled | :none
:ongoing | :not_runnable | :scheduled | :runnable | :other_repository
:ongoing | :not_runnable | :scheduled | :not_runnable | :none
:ongoing | :not_runnable | :scheduled | :disabled | :none
:ongoing | :not_runnable | :scheduled | :runnable | :other_repository
:ongoing | :not_runnable | :scheduled | :not_runnable | :none
:ongoing | :not_runnable | :unfinished | :disabled | :none
:ongoing | :not_runnable | :unfinished | :runnable | :other_repository
:ongoing | :not_runnable | :unfinished | :not_runnable | :other_repository
:ongoing | :not_runnable | :unfinished | :disabled | :none
:ongoing | :not_runnable | :unfinished | :runnable | :other_repository
:ongoing | :not_runnable | :unfinished | :not_runnable | :other_repository
:ongoing | :not_runnable | :ongoing | :disabled | :none
:ongoing | :not_runnable | :ongoing | :runnable | :none
:ongoing | :not_runnable | :ongoing | :not_runnable | :none
end
:ongoing | :not_runnable | :ongoing | :disabled | :none
:ongoing | :not_runnable | :ongoing | :runnable | :none
:ongoing | :not_runnable | :ongoing | :not_runnable | :none
with_them do
before do
update_container_repository(repository, repository_cleanup_status, repository_policy_status)
update_container_repository(other_repository, other_repository_cleanup_status, other_repository_policy_status)
end
with_them do
before do
update_container_repository(repository, repository_cleanup_status, repository_policy_status)
update_container_repository(other_repository, other_repository_cleanup_status, other_repository_policy_status)
end
subject { worker.send(:container_repository) }
if params[:expected_selected_repository] == :none
it 'does not select any repository' do
expect(subject).to eq(nil)
end
else
it 'does select a repository' do
selected_repository = expected_selected_repository == :repository ? repository : other_repository
subject { worker.send(:container_repository) }
expect(subject).to eq(selected_repository)
end
if params[:expected_selected_repository] == :none
it 'does not select any repository' do
expect(subject).to eq(nil)
end
else
it 'does select a repository' do
selected_repository = expected_selected_repository == :repository ? repository : other_repository
def update_container_repository(container_repository, cleanup_status, policy_status)
container_repository.update_column(:expiration_policy_cleanup_status, "cleanup_#{cleanup_status}")
policy = container_repository.project.container_expiration_policy
case policy_status
when :disabled
policy.update!(enabled: false)
when :runnable
policy.update!(enabled: true)
policy.update_column(:next_run_at, 5.minutes.ago)
when :not_runnable
policy.update!(enabled: true)
policy.update_column(:next_run_at, 5.minutes.from_now)
end
expect(subject).to eq(selected_repository)
end
end
end
context 'with another repository in cleanup unfinished state' do
let_it_be(:another_repository) { create(:container_repository, :cleanup_unfinished) }
def update_container_repository(container_repository, cleanup_status, policy_status)
container_repository.update_column(:expiration_policy_cleanup_status, "cleanup_#{cleanup_status}")
before do
policy.update_column(:next_run_at, 5.minutes.ago)
end
policy = container_repository.project.container_expiration_policy
it 'process the cleanup scheduled repository first' do
service_response = cleanup_service_response(repository: repository)
expect(ContainerExpirationPolicies::CleanupService)
.to receive(:new).with(repository).and_return(double(execute: service_response))
expect_log_extra_metadata(service_response: service_response)
subject
case policy_status
when :disabled
policy.update!(enabled: false)
when :runnable
policy.update!(enabled: true)
policy.update_column(:next_run_at, 5.minutes.ago)
when :not_runnable
policy.update!(enabled: true)
policy.update_column(:next_run_at, 5.minutes.from_now)
end
end
end
end
context 'with loopless disabled' do
before do
stub_feature_flags(container_registry_expiration_policies_loopless: false)
end
context 'with repository in cleanup scheduled state' do
it_behaves_like 'handling all repository conditions'
end
context 'with repository in cleanup unfinished state' do
before do
repository.cleanup_unfinished!
end
it_behaves_like 'handling all repository conditions'
end
context 'with another repository in cleanup unfinished state' do
let_it_be(:another_repository) { create(:container_repository, :cleanup_unfinished) }
it 'process the cleanup scheduled repository first' do
service_response = cleanup_service_response(repository: repository)
expect(ContainerExpirationPolicies::CleanupService)
.to receive(:new).with(repository).and_return(double(execute: service_response))
expect_log_extra_metadata(service_response: service_response)
subject
end
end
context 'with multiple repositories in cleanup unfinished state' do
let_it_be(:repository2) { create(:container_repository, :cleanup_unfinished, expiration_policy_started_at: 20.minutes.ago) }
let_it_be(:repository3) { create(:container_repository, :cleanup_unfinished, expiration_policy_started_at: 10.minutes.ago) }
before do
repository.update!(expiration_policy_cleanup_status: :cleanup_unfinished, expiration_policy_started_at: 30.minutes.ago)
end
context 'with another repository in cleanup unfinished state' do
let_it_be(:another_repository) { create(:container_repository, :cleanup_unfinished) }
it 'process the repository with the oldest expiration_policy_started_at' do
service_response = cleanup_service_response(repository: repository)
expect(ContainerExpirationPolicies::CleanupService)
.to receive(:new).with(repository).and_return(double(execute: service_response))
expect_log_extra_metadata(service_response: service_response)
subject
end
end
context 'with repository in cleanup ongoing state' do
before do
repository.cleanup_ongoing!
end
it 'does not process it' do
expect(Projects::ContainerRepository::CleanupTagsService).not_to receive(:new)
expect { subject }.not_to change { ContainerRepository.waiting_for_cleanup.count }
expect(repository.cleanup_ongoing?).to be_truthy
end
end
context 'with no repository in any cleanup state' do
before do
repository.cleanup_unscheduled!
end
it 'does not process it' do
expect(Projects::ContainerRepository::CleanupTagsService).not_to receive(:new)
expect { subject }.not_to change { ContainerRepository.waiting_for_cleanup.count }
expect(repository.cleanup_unscheduled?).to be_truthy
end
end
context 'with no container repository waiting' do
before do
repository.destroy!
end
it 'does not execute the cleanup tags service' do
expect(Projects::ContainerRepository::CleanupTagsService).not_to receive(:new)
expect { subject }.not_to change { ContainerRepository.waiting_for_cleanup.count }
end
before do
policy.update_column(:next_run_at, 5.minutes.ago)
end
context 'with feature flag disabled' do
before do
stub_feature_flags(container_registry_expiration_policies_throttling: false)
end
it 'is a no-op' do
expect(Projects::ContainerRepository::CleanupTagsService).not_to receive(:new)
it 'process the cleanup scheduled repository first' do
service_response = cleanup_service_response(repository: repository)
expect(ContainerExpirationPolicies::CleanupService)
.to receive(:new).with(repository).and_return(double(execute: service_response))
expect_log_extra_metadata(service_response: service_response)
expect { subject }.not_to change { ContainerRepository.waiting_for_cleanup.count }
end
subject
end
end
......@@ -509,69 +399,53 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
end
describe '#remaining_work_count' do
subject { worker.remaining_work_count }
let_it_be(:disabled_repository) { create(:container_repository, :cleanup_scheduled) }
shared_examples 'handling all conditions' do
context 'with container repositories waiting for cleanup' do
let_it_be(:unfinished_repositories) { create_list(:container_repository, 2, :cleanup_unfinished) }
let(:capacity) { 10 }
it { is_expected.to eq(3) }
subject { worker.remaining_work_count }
it 'logs the work count' do
expect_log_info(
cleanup_scheduled_count: 1,
cleanup_unfinished_count: 2,
cleanup_total_count: 3
)
before do
stub_application_setting(container_registry_expiration_policies_worker_capacity: capacity)
subject
end
end
ContainerExpirationPolicy.update_all(enabled: true)
repository.project.container_expiration_policy.update_column(:next_run_at, 5.minutes.ago)
disabled_repository.project.container_expiration_policy.update_column(:enabled, false)
end
context 'with no container repositories waiting for cleanup' do
before do
repository.cleanup_ongoing!
policy.update_column(:next_run_at, 5.minutes.from_now)
end
context 'with container repositories waiting for cleanup' do
let_it_be(:unfinished_repositories) { create_list(:container_repository, 2, :cleanup_unfinished) }
it { is_expected.to eq(0) }
it { is_expected.to eq(3) }
it 'logs 0 work count' do
expect_log_info(
cleanup_scheduled_count: 0,
cleanup_unfinished_count: 0,
cleanup_total_count: 0
)
it 'logs the work count' do
expect_log_info(
cleanup_scheduled_count: 1,
cleanup_unfinished_count: 2,
cleanup_total_count: 3
)
subject
end
subject
end
end
context 'with loopless enabled' do
let_it_be(:disabled_repository) { create(:container_repository, :cleanup_scheduled) }
let(:capacity) { 10 }
context 'with no container repositories waiting for cleanup' do
before do
stub_feature_flags(container_registry_expiration_policies_loopless: true)
stub_application_setting(container_registry_expiration_policies_worker_capacity: capacity)
# loopless mode is more accurate that non loopless: policies need to be enabled
ContainerExpirationPolicy.update_all(enabled: true)
repository.project.container_expiration_policy.update_column(:next_run_at, 5.minutes.ago)
disabled_repository.project.container_expiration_policy.update_column(:enabled, false)
repository.cleanup_ongoing!
policy.update_column(:next_run_at, 5.minutes.from_now)
end
it_behaves_like 'handling all conditions'
end
it { is_expected.to eq(0) }
context 'with loopless disabled' do
before do
stub_feature_flags(container_registry_expiration_policies_loopless: false)
end
it 'logs 0 work count' do
expect_log_info(
cleanup_scheduled_count: 0,
cleanup_unfinished_count: 0,
cleanup_total_count: 0
)
it_behaves_like 'handling all conditions'
subject
end
end
end
......@@ -599,8 +473,4 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do
expect(worker.logger)
.to receive(:info).with(worker.structured_payload(structure))
end
def loopless_enabled?
Feature.enabled?(:container_registry_expiration_policies_loopless)
end
end
spec/workers/container_expiration_policy_worker_spec.rb
View file @ d9b99c31
......@@ -34,101 +34,18 @@ RSpec.describe ContainerExpirationPolicyWorker do
end
end
context 'With no container expiration policies' do
context 'with loopless disabled' do
before do
stub_feature_flags(container_registry_expiration_policies_loopless: false)
end
it 'does not execute any policies' do
expect(ContainerRepository).not_to receive(:for_project_id)
expect { subject }.not_to change { ContainerRepository.cleanup_scheduled.count }
end
end
end
context 'with throttling enabled' do
before do
stub_feature_flags(container_registry_expiration_policies_throttling: true)
end
context 'with loopless disabled' do
before do
stub_feature_flags(container_registry_expiration_policies_loopless: false)
end
context 'with container expiration policies' do
let_it_be(:container_expiration_policy) { create(:container_expiration_policy, :runnable) }
let_it_be(:container_repository) { create(:container_repository, project: container_expiration_policy.project) }
before do
expect(worker).to receive(:with_runnable_policy).and_call_original
end
context 'with a valid container expiration policy' do
it 'schedules the next run' do
expect { subject }.to change { container_expiration_policy.reload.next_run_at }
end
it 'marks the container repository as scheduled for cleanup' do
expect { subject }.to change { container_repository.reload.cleanup_scheduled? }.from(false).to(true)
expect(ContainerRepository.cleanup_scheduled.count).to eq(1)
end
it 'calls the limited capacity worker' do
expect(ContainerExpirationPolicies::CleanupContainerRepositoryWorker).to receive(:perform_with_capacity)
subject
end
end
context 'with a disabled container expiration policy' do
before do
container_expiration_policy.disable!
end
it 'does not run the policy' do
expect(ContainerRepository).not_to receive(:for_project_id)
expect { subject }.not_to change { ContainerRepository.cleanup_scheduled.count }
end
end
it 'calls the limited capacity worker' do
expect(ContainerExpirationPolicies::CleanupContainerRepositoryWorker).to receive(:perform_with_capacity)
context 'with an invalid container expiration policy' do
let(:user) { container_expiration_policy.project.owner }
before do
container_expiration_policy.update_column(:name_regex, '*production')
end
it 'disables the policy and tracks an error' do
expect(ContainerRepository).not_to receive(:for_project_id)
expect(Gitlab::ErrorTracking).to receive(:log_exception).with(instance_of(described_class::InvalidPolicyError), container_expiration_policy_id: container_expiration_policy.id)
expect { subject }.to change { container_expiration_policy.reload.enabled }.from(true).to(false)
expect(ContainerRepository.cleanup_scheduled).to be_empty
end
end
end
it_behaves_like 'handling a taken exclusive lease'
subject
end
context 'with loopless enabled' do
before do
stub_feature_flags(container_registry_expiration_policies_loopless: true)
expect(worker).not_to receive(:with_runnable_policy)
end
it 'calls the limited capacity worker' do
expect(ContainerExpirationPolicies::CleanupContainerRepositoryWorker).to receive(:perform_with_capacity)
subject
end
it_behaves_like 'handling a taken exclusive lease'
end
it_behaves_like 'handling a taken exclusive lease'
end
context 'with throttling disabled' do
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7