diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb index a9bc1b9c258f74c8fb1cbabd476b8487cd6187ed..40b1a760e04dbc8772dbb132b35fbe305acbe0f0 100644 --- a/app/models/container_repository.rb +++ b/app/models/container_repository.rb @@ -258,30 +258,22 @@ def last_published_at def set_delete_ongoing_status now = Time.zone.now - values = { + update_columns( status: :delete_ongoing, delete_started_at: now, - status_updated_at: now - } - - values[:next_delete_attempt_at] = nil if Feature.enabled?(:set_delete_failed_container_repository, project) - - update_columns(values) + status_updated_at: now, + next_delete_attempt_at: nil + ) end def set_delete_scheduled_status - values = { + update_columns( status: :delete_scheduled, delete_started_at: nil, - status_updated_at: Time.zone.now - } - - if Feature.enabled?(:set_delete_failed_container_repository, project) - values[:failed_deletion_count] = failed_deletion_count + 1 - values[:next_delete_attempt_at] = next_delete_attempt_with_delay - end - - update_columns(values) + status_updated_at: Time.zone.now, + failed_deletion_count: failed_deletion_count + 1, + next_delete_attempt_at: next_delete_attempt_with_delay + ) end def set_delete_failed_status diff --git a/app/workers/container_registry/delete_container_repository_worker.rb b/app/workers/container_registry/delete_container_repository_worker.rb index 8c54401d4e85a2df81c4c7c847bfbc2ebbf22686..9452a7160d2bee45ebb5188359a69e7025992352 100644 --- a/app/workers/container_registry/delete_container_repository_worker.rb +++ b/app/workers/container_registry/delete_container_repository_worker.rb @@ -53,8 +53,7 @@ def max_running_jobs def update_next_container_repository_status return unless next_container_repository - if next_container_repository.failed_deletion_count >= ContainerRepository::MAX_DELETION_FAILURES && - Feature.enabled?(:set_delete_failed_container_repository, next_container_repository.project) + if next_container_repository.failed_deletion_count >= ContainerRepository::MAX_DELETION_FAILURES next_container_repository.set_delete_failed_status else next_container_repository.set_delete_scheduled_status diff --git a/config/feature_flags/gitlab_com_derisk/set_delete_failed_container_repository.yml b/config/feature_flags/gitlab_com_derisk/set_delete_failed_container_repository.yml deleted file mode 100644 index 8c168fd9810f85a8ee472c2156c6bf82942f04ba..0000000000000000000000000000000000000000 --- a/config/feature_flags/gitlab_com_derisk/set_delete_failed_container_repository.yml +++ /dev/null @@ -1,9 +0,0 @@ ---- -name: set_delete_failed_container_repository -feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/480652 -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/166119 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/490354 -milestone: '17.5' -group: group::container registry -type: gitlab_com_derisk -default_enabled: false diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index 858719f77744d921108883324f90fcbc7b341918..33e08603d11acfd80d539781b729583ee759cdfc 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -792,21 +792,6 @@ def expected_tags_from(client_tags) .and change(repository, :status_updated_at).from(nil).to(Time.zone.now) .and change(repository, :next_delete_attempt_at).to(nil) end - - context 'when the feature set_delete_failed_container_repository is disabled' do - before do - stub_feature_flags(set_delete_failed_container_repository: false) - end - - it 'updates deletion status attributes', :freeze_time do - expect { subject }.to change(repository, :status).from(nil).to('delete_ongoing') - .and change(repository, :delete_started_at).from(nil).to(Time.zone.now) - .and change(repository, :status_updated_at).from(nil).to(Time.zone.now) - .and not_change(repository, :next_delete_attempt_at) - - expect(repository.updated_at).to eq(Time.zone.now) - end - end end describe '#set_delete_scheduled_status', :freeze_time do @@ -842,21 +827,6 @@ def expected_tags_from(client_tags) expect(repository.status_updated_at).to eq(Time.zone.now) end - - context 'when the feature set_delete_failed_container_repository is disabled' do - before do - stub_feature_flags(set_delete_failed_container_repository: false) - end - - it 'updates delete attributes' do - expect { subject }.to change(repository, :status).from('delete_ongoing').to('delete_scheduled') - .and change(repository, :delete_started_at).to(nil) - .and not_change(repository, :failed_deletion_count) - .and not_change(repository, :next_delete_attempt_at) - - expect(repository.status_updated_at).to eq(Time.zone.now) - end - end end end diff --git a/spec/workers/container_registry/delete_container_repository_worker_spec.rb b/spec/workers/container_registry/delete_container_repository_worker_spec.rb index 58a1c5f96b6001228a30c9fe59a6153665257278..e7ce487d2163ae9b504487e09b142cc6d9f0a0ef 100644 --- a/spec/workers/container_registry/delete_container_repository_worker_spec.rb +++ b/spec/workers/container_registry/delete_container_repository_worker_spec.rb @@ -58,27 +58,6 @@ end end - shared_examples 'setting the status to delete_scheduled regardless of failed_deletion_count' do - let(:set_status_method) { :set_delete_scheduled_status } - let(:status_after_execution) { 'delete_scheduled' } - - context 'when the failed_deletion_count is less than the max' do - before do - container_repository.update!(failed_deletion_count: ContainerRepository::MAX_DELETION_FAILURES - 1) - end - - it_behaves_like 'not deleting the repository and setting the correct status' - end - - context 'when the failed_deletion_count has reached the max' do - before do - container_repository.update!(failed_deletion_count: ContainerRepository::MAX_DELETION_FAILURES) - end - - it_behaves_like 'not deleting the repository and setting the correct status' - end - end - it 'picks and destroys the next container repository for destruction' do expect_next_pending_destruction_container_repository do |repo| expect_logs_on(repo, tags_size_before_delete: 100, deleted_tags_size: 0) @@ -108,28 +87,12 @@ let(:cleanup_tags_service_response) { { status: :error, original_size: 100, deleted_size: 0 } } it_behaves_like 'setting the correct status based on failed_deletion_count' - - context 'when the feature set_delete_failed_container_repository is disabled' do - before do - stub_feature_flags(set_delete_failed_container_repository: false) - end - - it_behaves_like 'setting the status to delete_scheduled regardless of failed_deletion_count' - end end context 'with tags left to destroy' do let(:tags_count) { 10 } it_behaves_like 'setting the correct status based on failed_deletion_count' - - context 'when the feature set_delete_failed_container_repository is disabled' do - before do - stub_feature_flags(set_delete_failed_container_repository: false) - end - - it_behaves_like 'setting the status to delete_scheduled regardless of failed_deletion_count' - end end end @@ -153,14 +116,6 @@ end it_behaves_like 'setting the correct status based on failed_deletion_count' - - context 'when the feature set_delete_failed_container_repository is disabled' do - before do - stub_feature_flags(set_delete_failed_container_repository: false) - end - - it_behaves_like 'setting the status to delete_scheduled regardless of failed_deletion_count' - end end context 'with no tags on the container repository' do