diff --git a/app/models/container_registry/data_repair_detail.rb b/app/models/container_registry/data_repair_detail.rb index 09e617e69f51acd5604602d4d179f51f42ae6d1f..a2616490905aa1d746bd1e804580278f647ec379 100644 --- a/app/models/container_registry/data_repair_detail.rb +++ b/app/models/container_registry/data_repair_detail.rb @@ -2,9 +2,15 @@ module ContainerRegistry class DataRepairDetail < ApplicationRecord + include EachBatch + self.table_name = 'container_registry_data_repair_details' self.primary_key = :project_id belongs_to :project, optional: false + + enum status: { ongoing: 0, completed: 1, failed: 2 } + + scope :ongoing_since, ->(threshold) { where(status: :ongoing).where('updated_at < ?', threshold) } end end diff --git a/app/models/project.rb b/app/models/project.rb index d8818d9c174e3d95fef96e1fd584f82e3ceea7d9..224193fba0830554c355d199c5c130af5a47abf5 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -188,6 +188,7 @@ def self.integration_association_name(name) has_one :confluence_integration, class_name: 'Integrations::Confluence' has_one :custom_issue_tracker_integration, class_name: 'Integrations::CustomIssueTracker' has_one :datadog_integration, class_name: 'Integrations::Datadog' + has_one :container_registry_data_repair_detail, class_name: 'ContainerRegistry::DataRepairDetail' has_one :discord_integration, class_name: 'Integrations::Discord' has_one :drone_ci_integration, class_name: 'Integrations::DroneCi' has_one :emails_on_push_integration, class_name: 'Integrations::EmailsOnPush' @@ -740,6 +741,11 @@ def self.integration_association_name(name) topic ? with_topic(topic) : none end + scope :pending_data_repair_analysis, -> do + left_outer_joins(:container_registry_data_repair_detail) + .where(container_registry_data_repair_details: { project_id: nil }) + end + enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 } chronic_duration_attr :build_timeout_human_readable, :build_timeout, diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 3eb80292a2f0be18a4752ea1e65ac8904c150244..1149f64314e23c60a349b45a173fbc4ed0fb584d 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -165,6 +165,15 @@ :weight: 1 :idempotent: true :tags: [] +- :name: container_repository:container_registry_record_data_repair_detail + :worker_name: ContainerRegistry::RecordDataRepairDetailWorker + :feature_category: :container_registry + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: container_repository:delete_container_repository :worker_name: DeleteContainerRepositoryWorker :feature_category: :container_registry diff --git a/app/workers/container_registry/cleanup_worker.rb b/app/workers/container_registry/cleanup_worker.rb index a838b97b35d4be7e318da3f203c9c76975b1f4fc..448a16ad30990509fd02253fe79fba9d5180c93a 100644 --- a/app/workers/container_registry/cleanup_worker.rb +++ b/app/workers/container_registry/cleanup_worker.rb @@ -12,14 +12,17 @@ class CleanupWorker feature_category :container_registry STALE_DELETE_THRESHOLD = 30.minutes.freeze + STALE_REPAIR_DETAIL_THRESHOLD = 2.hours.freeze BATCH_SIZE = 200 def perform log_counts reset_stale_deletes + delete_stale_ongoing_repair_details enqueue_delete_container_repository_jobs if ContainerRepository.delete_scheduled.exists? + enqueue_record_repair_detail_jobs if should_enqueue_record_detail_jobs? end private @@ -33,10 +36,31 @@ def reset_stale_deletes end end + def delete_stale_ongoing_repair_details + # Deleting stale ongoing repair details would put the project back to the analysis pool + ContainerRegistry::DataRepairDetail + .ongoing_since(STALE_REPAIR_DETAIL_THRESHOLD.ago) + .each_batch(of: BATCH_SIZE) do |batch| # rubocop:disable Style/SymbolProc + batch.delete_all + end + end + def enqueue_delete_container_repository_jobs ContainerRegistry::DeleteContainerRepositoryWorker.perform_with_capacity end + def enqueue_record_repair_detail_jobs + ContainerRegistry::RecordDataRepairDetailWorker.perform_with_capacity + end + + def should_enqueue_record_detail_jobs? + return false unless Gitlab.com? + return false unless Feature.enabled?(:registry_data_repair_worker) + return false unless ContainerRegistry::GitlabApiClient.supports_gitlab_api? + + Project.pending_data_repair_analysis.exists? + end + def log_counts ::Gitlab::Database::LoadBalancing::Session.current.use_replicas_for_read_queries do log_extra_metadata_on_done( diff --git a/app/workers/container_registry/record_data_repair_detail_worker.rb b/app/workers/container_registry/record_data_repair_detail_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..f400568a3ef7045a1062c9f484bee4c77785c789 --- /dev/null +++ b/app/workers/container_registry/record_data_repair_detail_worker.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +module ContainerRegistry + class RecordDataRepairDetailWorker + include ApplicationWorker + include ExclusiveLeaseGuard + include LimitedCapacity::Worker + include Gitlab::Utils::StrongMemoize + + data_consistency :always # rubocop:disable SidekiqLoadBalancing/WorkerDataConsistency + queue_namespace :container_repository + feature_category :container_registry + urgency :low + worker_resource_boundary :unknown + idempotent! + + MAX_CAPACITY = 2 + LEASE_TIMEOUT = 1.hour.to_i + + def perform_work + return unless Gitlab.com? + return unless next_project + return if next_project.container_registry_data_repair_detail + + missing_count = 0 + + try_obtain_lease do + detail = create_data_repair_detail + + GitlabApiClient.each_sub_repositories_with_tag_page(path: next_project.full_path, + page_size: 50) do |repositories| + next if repositories.empty? + + paths = repositories.map { |repo| ContainerRegistry::Path.new(repo["path"]) } + paths, invalid_paths = paths.partition(&:valid?) + unless invalid_paths.empty? + log_extra_metadata_on_done( + :invalid_paths_parsed_in_container_repository_repair, + invalid_paths.join(' ,') + ) + end + + found_repositories = next_project.container_repositories.where(name: paths.map(&:repository_name)) # rubocop:disable CodeReuse/ActiveRecord + + missing_count += repositories.count - found_repositories.count + end + detail.update!(missing_count: missing_count, status: :completed) + end + rescue StandardError => exception + next_project.reset.container_registry_data_repair_detail&.update(status: :failed) + Gitlab::ErrorTracking.log_exception(exception, class: self.class.name) + end + + def remaining_work_count + return 0 unless Gitlab.com? + return 0 unless Feature.enabled?(:registry_data_repair_worker) + return 0 unless ContainerRegistry::GitlabApiClient.supports_gitlab_api? + + Project.pending_data_repair_analysis.limit(max_running_jobs + 1).count + end + + def max_running_jobs + MAX_CAPACITY + end + + private + + def next_project + Project.pending_data_repair_analysis.first + end + strong_memoize_attr :next_project + + def create_data_repair_detail + ContainerRegistry::DataRepairDetail.create!(project: next_project, status: :ongoing) + end + + # Used by ExclusiveLeaseGuard + def lease_key + "container_registry_data_repair_detail_worker:#{next_project.id}" + end + + # Used by ExclusiveLeaseGuard + def lease_timeout + LEASE_TIMEOUT + end + end +end diff --git a/config/feature_flags/development/registry_data_repair_worker.yml b/config/feature_flags/development/registry_data_repair_worker.yml new file mode 100644 index 0000000000000000000000000000000000000000..3d8982c3e81b7833bde91fdfe5f1a323f037a6d1 --- /dev/null +++ b/config/feature_flags/development/registry_data_repair_worker.yml @@ -0,0 +1,8 @@ +--- +name: registry_data_repair_worker +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115156 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/397505 +milestone: '16.0' +type: development +group: group::container registry +default_enabled: false diff --git a/db/migrate/20230404160131_add_status_to_data_repair_details.rb b/db/migrate/20230404160131_add_status_to_data_repair_details.rb new file mode 100644 index 0000000000000000000000000000000000000000..564b0450752e42a807eb0341f2d3403c9db0f454 --- /dev/null +++ b/db/migrate/20230404160131_add_status_to_data_repair_details.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddStatusToDataRepairDetails < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = 'index_container_registry_data_repair_details_on_status' + + def up + unless column_exists?(:container_registry_data_repair_details, :status) + add_column(:container_registry_data_repair_details, :status, :integer, default: 0, null: false, limit: 2) + end + + add_concurrent_index :container_registry_data_repair_details, :status, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :container_registry_data_repair_details, name: INDEX_NAME + remove_column :container_registry_data_repair_details, :status + end +end diff --git a/db/schema_migrations/20230404160131 b/db/schema_migrations/20230404160131 new file mode 100644 index 0000000000000000000000000000000000000000..089788a185f479c266bae2a3ed83319062278caa --- /dev/null +++ b/db/schema_migrations/20230404160131 @@ -0,0 +1 @@ +d49adae338b04deee154fd2fcb9bebf4f746eb5e7d1d39402a617ca511a06bfe \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 7f360c5c6f4c884f382c8884cfb2e221f8cd3f18..75087462e3a52ac294090df2ab03803ffbc82fa5 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14486,7 +14486,8 @@ CREATE TABLE container_registry_data_repair_details ( missing_count integer DEFAULT 0, project_id bigint NOT NULL, created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL + updated_at timestamp with time zone NOT NULL, + status smallint DEFAULT 0 NOT NULL ); CREATE TABLE container_repositories ( @@ -30345,6 +30346,8 @@ CREATE INDEX index_composer_cache_files_where_namespace_id_is_null ON packages_c CREATE INDEX index_container_expiration_policies_on_next_run_at_and_enabled ON container_expiration_policies USING btree (next_run_at, enabled); +CREATE INDEX index_container_registry_data_repair_details_on_status ON container_registry_data_repair_details USING btree (status); + CREATE INDEX index_container_repositories_on_greatest_completed_at ON container_repositories USING btree (GREATEST(migration_pre_import_done_at, migration_import_done_at, migration_aborted_at, migration_skipped_at)) WHERE (migration_state = ANY (ARRAY['import_done'::text, 'pre_import_done'::text, 'import_aborted'::text, 'import_skipped'::text])); CREATE INDEX index_container_repositories_on_migration_state_import_done_at ON container_repositories USING btree (migration_state, migration_import_done_at); diff --git a/lib/container_registry/gitlab_api_client.rb b/lib/container_registry/gitlab_api_client.rb index d3c0ec03983ef930550592e022c3a1e7a1d25c69..00877bb5a4825bc209fd87843c072e5f2a0ac218 100644 --- a/lib/container_registry/gitlab_api_client.rb +++ b/lib/container_registry/gitlab_api_client.rb @@ -54,6 +54,33 @@ def self.one_project_with_container_registry_tag(path) end end + def self.each_sub_repositories_with_tag_page(path:, page_size: 100, &block) + raise ArgumentError, 'block not given' unless block + + # dummy uri to initialize the loop + next_page_uri = URI('') + page_count = 0 + + with_dummy_client(token_config: { type: :nested_repositories_token, path: path&.downcase }) do |client| + while next_page_uri + last = Rack::Utils.parse_nested_query(next_page_uri.query)['last'] + current_page = client.sub_repositories_with_tag(path&.downcase, page_size: page_size, last: last) + + if current_page&.key?(:response_body) + yield (current_page[:response_body] || []) + next_page_uri = current_page.dig(:pagination, :next, :uri) + else + # no current page. Break the loop + next_page_uri = nil + end + + page_count += 1 + + raise 'too many pages requested' if page_count >= MAX_REPOSITORIES_PAGE_SIZE + end + end + end + # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#compliance-check def supports_gitlab_api? strong_memoize(:supports_gitlab_api) do diff --git a/spec/factories/container_registry/data_repair_detail.rb b/spec/factories/container_registry/data_repair_detail.rb new file mode 100644 index 0000000000000000000000000000000000000000..79467c464db55dfa9b2f75baf5b349c327413180 --- /dev/null +++ b/spec/factories/container_registry/data_repair_detail.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :container_registry_data_repair_detail, class: 'ContainerRegistry::DataRepairDetail' do + project + updated_at { 1.hour.ago } + + trait :ongoing do + status { :ongoing } + end + + trait :completed do + status { :completed } + end + + trait :failed do + status { :failed } + end + end +end diff --git a/spec/lib/container_registry/gitlab_api_client_spec.rb b/spec/lib/container_registry/gitlab_api_client_spec.rb index ac15048e4b548b440cc24511a68ecd14682ccf9f..c70dd265073490ba6006c45316dcc306f0f50d21 100644 --- a/spec/lib/container_registry/gitlab_api_client_spec.rb +++ b/spec/lib/container_registry/gitlab_api_client_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ContainerRegistry::GitlabApiClient do +RSpec.describe ContainerRegistry::GitlabApiClient, feature_category: :container_registry do using RSpec::Parameterized::TableSyntax include_context 'container registry client' @@ -615,6 +615,159 @@ end end + describe '#each_sub_repositories_with_tag_page' do + let(:page_size) { 100 } + let(:project_path) { 'repo/project' } + + shared_examples 'iterating through a page' do |expected_tags: true| + it 'iterates through one page' do + expect_next_instance_of(described_class) do |client| + expect(client).to receive(:sub_repositories_with_tag).with(project_path, page_size: page_size, last: nil).and_return(client_response) + end + + expect { |b| described_class.each_sub_repositories_with_tag_page(path: project_path, page_size: page_size, &b) } + .to yield_with_args(expected_tags ? client_response_repositories : []) + end + end + + context 'when no block is given' do + it 'raises an Argument error' do + expect do + described_class.each_sub_repositories_with_tag_page(path: project_path, page_size: page_size) + end.to raise_error(ArgumentError, 'block not given') + end + end + + context 'when a block is given' do + before do + expect(Auth::ContainerRegistryAuthenticationService).to receive(:pull_nested_repositories_access_token).with(project_path).and_return(token) + stub_container_registry_config(enabled: true, api_url: registry_api_url, key: 'spec/fixtures/x509_certificate_pk.key') + end + + context 'with an empty page' do + let(:client_response) { { pagination: {}, response_body: [] } } + + it_behaves_like 'iterating through a page', expected_tags: false + end + + context 'with one page' do + let(:client_response) { { pagination: {}, response_body: client_response_repositories } } + let(:client_response_repositories) do + [ + { + "name": "docker-alpine", + "path": "gitlab-org/build/cng/docker-alpine", + "created_at": "2022-06-07T12:11:13.633+00:00", + "updated_at": "2022-06-07T14:37:49.251+00:00" + }, + { + "name": "git-base", + "path": "gitlab-org/build/cng/git-base", + "created_at": "2022-06-07T12:11:13.633+00:00", + "updated_at": "2022-06-07T14:37:49.251+00:00" + } + ] + end + + it_behaves_like 'iterating through a page' + end + + context 'with two pages' do + let(:client_response1) { { pagination: { next: { uri: URI('http://localhost/next?last=latest') } }, response_body: client_response_repositories1 } } + let(:client_response_repositories1) do + [ + { + "name": "docker-alpine", + "path": "gitlab-org/build/cng/docker-alpine", + "created_at": "2022-06-07T12:11:13.633+00:00", + "updated_at": "2022-06-07T14:37:49.251+00:00" + }, + { + "name": "git-base", + "path": "gitlab-org/build/cng/git-base", + "created_at": "2022-06-07T12:11:13.633+00:00", + "updated_at": "2022-06-07T14:37:49.251+00:00" + } + ] + end + + let(:client_response2) { { pagination: {}, response_body: client_response_repositories2 } } + let(:client_response_repositories2) do + [ + { + "name": "docker-alpine1", + "path": "gitlab-org/build/cng/docker-alpine", + "created_at": "2022-06-07T12:11:13.633+00:00", + "updated_at": "2022-06-07T14:37:49.251+00:00" + }, + { + "name": "git-base1", + "path": "gitlab-org/build/cng/git-base", + "created_at": "2022-06-07T12:11:13.633+00:00", + "updated_at": "2022-06-07T14:37:49.251+00:00" + } + ] + end + + it 'iterates through two pages' do + expect_next_instance_of(described_class) do |client| + expect(client).to receive(:sub_repositories_with_tag).with(project_path, page_size: page_size, last: nil).and_return(client_response1) + expect(client).to receive(:sub_repositories_with_tag).with(project_path, page_size: page_size, last: 'latest').and_return(client_response2) + end + + expect { |b| described_class.each_sub_repositories_with_tag_page(path: project_path, page_size: page_size, &b) } + .to yield_successive_args(client_response_repositories1, client_response_repositories2) + end + end + + context 'when max pages is reached' do + let(:client_response) { { pagination: {}, response_body: [] } } + + before do + stub_const('ContainerRegistry::GitlabApiClient::MAX_REPOSITORIES_PAGE_SIZE', 0) + expect_next_instance_of(described_class) do |client| + expect(client).to receive(:sub_repositories_with_tag).with(project_path, page_size: page_size, last: nil).and_return(client_response) + end + end + + it 'raises an error' do + expect { described_class.each_sub_repositories_with_tag_page(path: project_path, page_size: page_size) {} } # rubocop:disable Lint/EmptyBlock + .to raise_error(StandardError, 'too many pages requested') + end + end + + context 'without a page size set' do + let(:client_response) { { pagination: {}, response_body: [] } } + + it 'uses a default size' do + expect_next_instance_of(described_class) do |client| + expect(client).to receive(:sub_repositories_with_tag).with(project_path, page_size: page_size, last: nil).and_return(client_response) + end + + expect { |b| described_class.each_sub_repositories_with_tag_page(path: project_path, &b) }.to yield_with_args([]) + end + end + + context 'with an empty client response' do + let(:client_response) { {} } + + it 'breaks the loop' do + expect_next_instance_of(described_class) do |client| + expect(client).to receive(:sub_repositories_with_tag).with(project_path, page_size: page_size, last: nil).and_return(client_response) + end + + expect { |b| described_class.each_sub_repositories_with_tag_page(path: project_path, page_size: page_size, &b) }.not_to yield_control + end + end + + context 'with a nil page' do + let(:client_response) { { pagination: {}, response_body: nil } } + + it_behaves_like 'iterating through a page', expected_tags: false + end + end + end + def stub_pre_import(path, status_code, pre:) import_type = pre ? 'pre' : 'final' stub_request(:put, "#{registry_api_url}/gitlab/v1/import/#{path}/?import_type=#{import_type}") diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 8a2602ea9f6296dbc613bb239c5d3bbedd778116..34f9948b9dc97cc23cca12057813f2c9ba558482 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -647,6 +647,7 @@ project: - redirect_routes - statistics - container_repositories +- container_registry_data_repair_detail - uploads - file_uploads - import_state diff --git a/spec/models/container_registry/data_repair_detail_spec.rb b/spec/models/container_registry/data_repair_detail_spec.rb index 92833553a1e2261de3f4c74032ba8678de4ac4b6..4d2ac5fff42cf778f88f9ecb6bae896160e5aa6c 100644 --- a/spec/models/container_registry/data_repair_detail_spec.rb +++ b/spec/models/container_registry/data_repair_detail_spec.rb @@ -8,4 +8,22 @@ subject { described_class.new(project: project) } it { is_expected.to belong_to(:project).required } + + it_behaves_like 'having unique enum values' + + describe '.ongoing_since' do + let_it_be(:repair_detail1) { create(:container_registry_data_repair_detail, :ongoing, updated_at: 1.day.ago) } + let_it_be(:repair_detail2) { create(:container_registry_data_repair_detail, :ongoing, updated_at: 20.minutes.ago) } + let_it_be(:repair_detail3) do + create(:container_registry_data_repair_detail, :completed, updated_at: 20.minutes.ago) + end + + let_it_be(:repair_detail4) do + create(:container_registry_data_repair_detail, :completed, updated_at: 31.minutes.ago) + end + + subject { described_class.ongoing_since(30.minutes.ago) } + + it { is_expected.to contain_exactly(repair_detail1) } + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 855c5f66554ccda25d8fbbd4f2e6791e6b4876aa..e9bb01f4b230302f31acf5a392878b479f437cc8 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -95,6 +95,7 @@ it { is_expected.to have_one(:mock_ci_integration) } it { is_expected.to have_one(:mock_monitoring_integration) } it { is_expected.to have_one(:service_desk_custom_email_verification).class_name('ServiceDesk::CustomEmailVerification') } + it { is_expected.to have_one(:container_registry_data_repair_detail).class_name('ContainerRegistry::DataRepairDetail') } it { is_expected.to have_many(:commit_statuses) } it { is_expected.to have_many(:ci_pipelines) } it { is_expected.to have_many(:ci_refs) } @@ -6800,6 +6801,19 @@ def has_external_wiki end end + describe '.pending_data_repair_analysis' do + it 'returns projects that are not in ContainerRegistry::DataRepairDetail' do + project_1 = create(:project) + project_2 = create(:project) + + expect(described_class.pending_data_repair_analysis).to match_array([project_1, project_2]) + + create(:container_registry_data_repair_detail, project: project_1) + + expect(described_class.pending_data_repair_analysis).to match_array([project_2]) + end + end + describe '.deployments' do subject { project.deployments } diff --git a/spec/workers/container_registry/cleanup_worker_spec.rb b/spec/workers/container_registry/cleanup_worker_spec.rb index 72e1243ccb5d69cc06fcb2b3e8f0740a76a98b41..955d2175085b3a462793ca04f4e87dbc308029ad 100644 --- a/spec/workers/container_registry/cleanup_worker_spec.rb +++ b/spec/workers/container_registry/cleanup_worker_spec.rb @@ -46,6 +46,77 @@ end end + context 'with stale ongoing repair details' do + let_it_be(:stale_updated_at) { (described_class::STALE_REPAIR_DETAIL_THRESHOLD + 5.minutes).ago } + let_it_be(:recent_updated_at) { (described_class::STALE_REPAIR_DETAIL_THRESHOLD - 5.minutes).ago } + let_it_be(:old_repair_detail) { create(:container_registry_data_repair_detail, updated_at: stale_updated_at) } + let_it_be(:new_repair_detail) { create(:container_registry_data_repair_detail, updated_at: recent_updated_at) } + + it 'deletes them' do + expect { perform }.to change { ContainerRegistry::DataRepairDetail.count }.from(2).to(1) + expect(ContainerRegistry::DataRepairDetail.all).to contain_exactly(new_repair_detail) + end + end + + shared_examples 'does not enqueue record repair detail jobs' do + it 'does not enqueue record repair detail jobs' do + expect(ContainerRegistry::RecordDataRepairDetailWorker).not_to receive(:perform_with_capacity) + + perform + end + end + + context 'when on gitlab.com', :saas do + context 'when the gitlab api is supported' do + let(:relation) { instance_double(ActiveRecord::Relation) } + + before do + allow(ContainerRegistry::GitlabApiClient).to receive(:supports_gitlab_api?).and_return(true) + allow(Project).to receive(:pending_data_repair_analysis).and_return(relation) + end + + context 'when there are pending projects to analyze' do + before do + allow(relation).to receive(:exists?).and_return(true) + end + + it "enqueues record repair detail jobs" do + expect(ContainerRegistry::RecordDataRepairDetailWorker).to receive(:perform_with_capacity) + + perform + end + end + + context 'when there are no pending projects to analyze' do + before do + allow(relation).to receive(:exists?).and_return(false) + end + + it_behaves_like 'does not enqueue record repair detail jobs' + end + end + + context 'when the Gitlab API is not supported' do + before do + allow(ContainerRegistry::GitlabApiClient).to receive(:supports_gitlab_api?).and_return(false) + end + + it_behaves_like 'does not enqueue record repair detail jobs' + end + end + + context 'when not on Gitlab.com' do + it_behaves_like 'does not enqueue record repair detail jobs' + end + + context 'when registry_data_repair_worker feature is disabled' do + before do + stub_feature_flags(registry_data_repair_worker: false) + end + + it_behaves_like 'does not enqueue record repair detail jobs' + end + context 'for counts logging' do let_it_be(:delete_started_at) { (described_class::STALE_DELETE_THRESHOLD + 5.minutes).ago } let_it_be(:stale_delete_container_repository) do diff --git a/spec/workers/container_registry/record_data_repair_detail_worker_spec.rb b/spec/workers/container_registry/record_data_repair_detail_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..f107144d397b89c29615f0b4cbd4cb93be634206 --- /dev/null +++ b/spec/workers/container_registry/record_data_repair_detail_worker_spec.rb @@ -0,0 +1,191 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ContainerRegistry::RecordDataRepairDetailWorker, :aggregate_failures, :clean_gitlab_redis_shared_state, + feature_category: :container_registry do + include ExclusiveLeaseHelpers + + let(:worker) { described_class.new } + + describe '#perform_work' do + subject(:perform_work) { worker.perform_work } + + context 'with no work to do - no projects pending analysis' do + it 'will not try to get an exclusive lease and connect to the endpoint' do + allow(Project).to receive(:pending_data_repair_analysis).and_return([]) + expect(::Gitlab::ExclusiveLease).not_to receive(:new) + + expect(::ContainerRegistry::GitlabApiClient).not_to receive(:each_sub_repositories_with_tag_page) + + perform_work + end + end + + context 'with work to do' do + let_it_be(:path) { 'build/cng/docker-alpine' } + let_it_be(:group) { create(:group, path: 'build') } + let_it_be(:project) { create(:project, name: 'cng', namespace: group) } + + let_it_be(:container_repository) { create(:container_repository, project: project, name: "docker-alpine") } + let_it_be(:lease_key) { "container_registry_data_repair_detail_worker:#{project.id}" } + + before do + allow(ContainerRegistry::GitlabApiClient).to receive(:each_sub_repositories_with_tag_page) + allow(ContainerRegistry::GitlabApiClient).to receive(:supports_gitlab_api?).and_return(true) + end + + context 'when on Gitlab.com', :saas do + it 'obtains exclusive lease on the project' do + expect(Project).to receive(:pending_data_repair_analysis).and_call_original + expect_to_obtain_exclusive_lease("container_registry_data_repair_detail_worker:#{project.id}", + timeout: described_class::LEASE_TIMEOUT) + expect_to_cancel_exclusive_lease("container_registry_data_repair_detail_worker:#{project.id}", 'uuid') + + perform_work + end + + it 'queries how many are existing repositories and counts the missing ones' do + stub_exclusive_lease("container_registry_data_repair_detail_worker:#{project.id}", + timeout: described_class::LEASE_TIMEOUT) + allow(ContainerRegistry::GitlabApiClient).to receive(:each_sub_repositories_with_tag_page) + .with(path: project.full_path, page_size: 50).and_yield( + [ + { "path" => container_repository.path }, + { "path" => 'missing1/repository' }, + { "path" => 'missing2/repository' } + ] + ) + + expect(worker).not_to receive(:log_extra_metadata_on_done) + expect { perform_work }.to change { ContainerRegistry::DataRepairDetail.count }.from(0).to(1) + expect(ContainerRegistry::DataRepairDetail.first).to have_attributes(project: project, missing_count: 2) + end + + it 'logs invalid paths' do + stub_exclusive_lease("container_registry_data_repair_detail_worker:#{project.id}", + timeout: described_class::LEASE_TIMEOUT) + valid_path = ContainerRegistry::Path.new('valid/path') + invalid_path = ContainerRegistry::Path.new('invalid/path') + allow(valid_path).to receive(:valid?).and_return(true) + allow(invalid_path).to receive(:valid?).and_return(false) + + allow(ContainerRegistry::GitlabApiClient).to receive(:each_sub_repositories_with_tag_page) + .with(path: project.full_path, page_size: 50).and_yield( + [ + { "path" => valid_path.to_s }, + { "path" => invalid_path.to_s } + ] + ) + + allow(ContainerRegistry::Path).to receive(:new).with(valid_path.to_s).and_return(valid_path) + allow(ContainerRegistry::Path).to receive(:new).with(invalid_path.to_s).and_return(invalid_path) + + expect(worker).to receive(:log_extra_metadata_on_done).with( + :invalid_paths_parsed_in_container_repository_repair, + "invalid/path" + ) + perform_work + end + + it_behaves_like 'an idempotent worker' do + it 'creates a data repair detail' do + expect { perform_work }.to change { ContainerRegistry::DataRepairDetail.count }.from(0).to(1) + expect(project.container_registry_data_repair_detail).to be_present + end + end + + context 'when the lease cannot be obtained' do + before do + stub_exclusive_lease_taken(lease_key, timeout: described_class::LEASE_TIMEOUT) + end + + it 'logs an error and does not proceed' do + expect(worker).to receive(:log_lease_taken) + expect(ContainerRegistry::GitlabApiClient).not_to receive(:each_sub_repositories_with_tag_page) + + perform_work + end + + it 'does not create the data repair detail' do + perform_work + + expect(project.reload.container_registry_data_repair_detail).to be_nil + end + end + + context 'when an error occurs' do + before do + stub_exclusive_lease("container_registry_data_repair_detail_worker:#{project.id}", + timeout: described_class::LEASE_TIMEOUT) + allow(ContainerRegistry::GitlabApiClient).to receive(:each_sub_repositories_with_tag_page) + .with(path: project.full_path, page_size: 50).and_raise(RuntimeError) + end + + it 'logs the error' do + expect(::Gitlab::ErrorTracking).to receive(:log_exception) + .with(instance_of(RuntimeError), class: described_class.name) + + perform_work + end + + it 'sets the status of the repair detail to failed' do + expect { perform_work }.to change { ContainerRegistry::DataRepairDetail.failed.count }.from(0).to(1) + expect(project.reload.container_registry_data_repair_detail.failed?).to eq(true) + end + end + end + + context 'when not on Gitlab.com' do + it 'will not do anything' do + expect(::Gitlab::ExclusiveLease).not_to receive(:new) + expect(::ContainerRegistry::GitlabApiClient).not_to receive(:each_sub_repositories_with_tag_page) + + perform_work + end + end + end + end + + describe '#max_running_jobs' do + subject { worker.max_running_jobs } + + it { is_expected.to eq(described_class::MAX_CAPACITY) } + end + + describe '#remaining_work_count' do + let_it_be(:pending_projects) do + create_list(:project, described_class::MAX_CAPACITY + 2) + end + + subject { worker.remaining_work_count } + + context 'when on Gitlab.com', :saas do + before do + allow(ContainerRegistry::GitlabApiClient).to receive(:supports_gitlab_api?).and_return(true) + end + + it { is_expected.to eq(described_class::MAX_CAPACITY + 1) } + + context 'when the Gitlab API is not supported' do + before do + allow(ContainerRegistry::GitlabApiClient).to receive(:supports_gitlab_api?).and_return(false) + end + + it { is_expected.to eq(0) } + end + end + + context 'when not on Gitlab.com' do + it { is_expected.to eq(0) } + end + + context 'when registry_data_repair_worker feature is disabled' do + before do + stub_feature_flags(registry_data_repair_worker: false) + end + + it { is_expected.to eq(0) } + end + end +end diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index 4309ec24a9ef21228352b38763082f34cda1a25c..2c2cd7e9960a26a9bec5c249d77e467184166b41 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -191,6 +191,7 @@ 'Clusters::Cleanup::ServiceAccountWorker' => 3, 'ContainerExpirationPolicies::CleanupContainerRepositoryWorker' => 0, 'ContainerRegistry::DeleteContainerRepositoryWorker' => 0, + 'ContainerRegistry::RecordDataRepairDetailWorker' => 0, 'CreateCommitSignatureWorker' => 3, 'CreateGithubWebhookWorker' => 3, 'CreateNoteDiffFileWorker' => 3,