Commit 910dd9e9 authored by Steve Abrams's avatar Steve Abrams Committed by David Fernandez

Skip registry import after too many retries

Skip container registry import after it has
been aborted more than the number of maxiumum retries.
parent c98c7465
...@@ -14,6 +14,7 @@ class ContainerRepository < ApplicationRecord ...@@ -14,6 +14,7 @@ class ContainerRepository < ApplicationRecord
ACTIVE_MIGRATION_STATES = %w[pre_importing importing].freeze ACTIVE_MIGRATION_STATES = %w[pre_importing importing].freeze
MIGRATION_STATES = (IDLE_MIGRATION_STATES + ACTIVE_MIGRATION_STATES).freeze MIGRATION_STATES = (IDLE_MIGRATION_STATES + ACTIVE_MIGRATION_STATES).freeze
ABORTABLE_MIGRATION_STATES = (ACTIVE_MIGRATION_STATES + %w[pre_import_done default]).freeze ABORTABLE_MIGRATION_STATES = (ACTIVE_MIGRATION_STATES + %w[pre_import_done default]).freeze
SKIPPABLE_MIGRATION_STATES = (ABORTABLE_MIGRATION_STATES + %w[import_aborted]).freeze
IRRECONCILABLE_MIGRATIONS_STATUSES = %w[import_in_progress pre_import_in_progress pre_import_canceled import_canceled].freeze IRRECONCILABLE_MIGRATIONS_STATUSES = %w[import_in_progress pre_import_in_progress pre_import_canceled import_canceled].freeze
...@@ -137,7 +138,7 @@ class ContainerRepository < ApplicationRecord ...@@ -137,7 +138,7 @@ class ContainerRepository < ApplicationRecord
end end
event :skip_import do event :skip_import do
transition ABORTABLE_MIGRATION_STATES.map(&:to_sym) => :import_skipped transition SKIPPABLE_MIGRATION_STATES.map(&:to_sym) => :import_skipped
end end
event :retry_pre_import do event :retry_pre_import do
...@@ -184,6 +185,12 @@ class ContainerRepository < ApplicationRecord ...@@ -184,6 +185,12 @@ class ContainerRepository < ApplicationRecord
container_repository.migration_retries_count += 1 container_repository.migration_retries_count += 1
end end
after_transition any => :import_aborted do |container_repository|
if container_repository.retried_too_many_times?
container_repository.skip_import(reason: :too_many_retries)
end
end
before_transition import_aborted: any do |container_repository| before_transition import_aborted: any do |container_repository|
container_repository.migration_aborted_at = nil container_repository.migration_aborted_at = nil
container_repository.migration_aborted_in_state = nil container_repository.migration_aborted_in_state = nil
...@@ -325,6 +332,10 @@ class ContainerRepository < ApplicationRecord ...@@ -325,6 +332,10 @@ class ContainerRepository < ApplicationRecord
end end
end end
def retried_too_many_times?
migration_retries_count >= ContainerRegistry::Migration.max_retries
end
def last_import_step_done_at def last_import_step_done_at
[migration_pre_import_done_at, migration_import_done_at, migration_aborted_at].compact.max [migration_pre_import_done_at, migration_import_done_at, migration_aborted_at].compact.max
end end
......
...@@ -302,6 +302,19 @@ RSpec.describe ContainerRepository, :aggregate_failures do ...@@ -302,6 +302,19 @@ RSpec.describe ContainerRepository, :aggregate_failures do
expect(repository.migration_aborted_in_state).to eq('importing') expect(repository.migration_aborted_in_state).to eq('importing')
expect(repository).to be_import_aborted expect(repository).to be_import_aborted
end end
context 'above the max retry limit' do
before do
stub_application_setting(container_registry_import_max_retries: 1)
end
it 'skips the migration' do
expect { subject }.to change { repository.migration_skipped_at }
expect(repository.reload).to be_import_skipped
expect(repository.migration_skipped_reason).to eq('too_many_retries')
end
end
end end
describe '#skip_import' do describe '#skip_import' do
...@@ -309,7 +322,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do ...@@ -309,7 +322,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do
subject { repository.skip_import(reason: :too_many_retries) } subject { repository.skip_import(reason: :too_many_retries) }
it_behaves_like 'transitioning from allowed states', ContainerRepository::ABORTABLE_MIGRATION_STATES it_behaves_like 'transitioning from allowed states', ContainerRepository::SKIPPABLE_MIGRATION_STATES
it 'sets migration_skipped_at and migration_skipped_reason' do it 'sets migration_skipped_at and migration_skipped_reason' do
expect { subject }.to change { repository.reload.migration_skipped_at } expect { subject }.to change { repository.reload.migration_skipped_at }
...@@ -1136,6 +1149,30 @@ RSpec.describe ContainerRepository, :aggregate_failures do ...@@ -1136,6 +1149,30 @@ RSpec.describe ContainerRepository, :aggregate_failures do
end end
end end
describe '#retried_too_many_times?' do
subject { repository.retried_too_many_times? }
before do
stub_application_setting(container_registry_import_max_retries: 3)
end
context 'migration_retries_count is equal or greater than max_retries' do
before do
repository.update_column(:migration_retries_count, 3)
end
it { is_expected.to eq(true) }
end
context 'migration_retries_count is lower than max_retries' do
before do
repository.update_column(:migration_retries_count, 2)
end
it { is_expected.to eq(false) }
end
end
context 'with repositories' do context 'with repositories' do
let_it_be_with_reload(:repository) { create(:container_repository, :cleanup_unscheduled) } let_it_be_with_reload(:repository) { create(:container_repository, :cleanup_unscheduled) }
let_it_be(:other_repository) { create(:container_repository, :cleanup_unscheduled) } let_it_be(:other_repository) { create(:container_repository, :cleanup_unscheduled) }
......
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