diff --git a/.gitlab/issue_templates/Geo Replicate a new Git repository type.md b/.gitlab/issue_templates/Geo Replicate a new Git repository type.md index 29fa092a0916867081017a3f4b89e2e12b51ca7e..1a0a6eb12e0f657ae6875898f4d9bcf46668f03c 100644 --- a/.gitlab/issue_templates/Geo Replicate a new Git repository type.md +++ b/.gitlab/issue_templates/Geo Replicate a new Git repository type.md @@ -428,10 +428,15 @@ That's all of the required database changes. include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::CoolWidget - MODEL_FOREIGN_KEY = :cool_widget_id - belongs_to :cool_widget, class_name: 'CoolWidget' + + def self.model_class + ::CoolWidget + end + + def self.model_foreign_key + :cool_widget_id + end end end ``` diff --git a/.gitlab/issue_templates/Geo Replicate a new blob type.md b/.gitlab/issue_templates/Geo Replicate a new blob type.md index c83ef9a4f3ee5af7856b5934df70f911f18562e7..caaf7324a525199051594543e59205bb7ea53bec 100644 --- a/.gitlab/issue_templates/Geo Replicate a new blob type.md +++ b/.gitlab/issue_templates/Geo Replicate a new blob type.md @@ -394,10 +394,15 @@ That's all of the required database changes. include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::CoolWidget - MODEL_FOREIGN_KEY = :cool_widget_id - belongs_to :cool_widget, class_name: 'CoolWidget' + + def self.model_class + ::CoolWidget + end + + def self.model_foreign_key + :cool_widget_id + end end end ``` diff --git a/app/assets/javascripts/graphql_shared/possible_types.json b/app/assets/javascripts/graphql_shared/possible_types.json index 657adf951a68c98fab2254dfddee2bfebde609f1..0cb99c778a7856478275f8bc986ead0e8c99efff 100644 --- a/app/assets/javascripts/graphql_shared/possible_types.json +++ b/app/assets/javascripts/graphql_shared/possible_types.json @@ -247,6 +247,7 @@ "PackageFileRegistry", "PagesDeploymentRegistry", "PipelineArtifactRegistry", + "ProjectRepositoryRegistry", "ProjectWikiRepositoryRegistry", "SnippetRepositoryRegistry", "TerraformStateVersionRegistry", diff --git a/app/models/project_repository.rb b/app/models/project_repository.rb index c11ceb039b8f1b9dc43477d6bd7f350ac2547e30..32ac8fffd687bf45f5c0206e1bfffc33c00c911e 100644 --- a/app/models/project_repository.rb +++ b/app/models/project_repository.rb @@ -3,6 +3,9 @@ class ProjectRepository < ApplicationRecord include EachBatch include Shardable + include ::Repositories::CanHousekeepRepository + + extend ::Gitlab::Utils::Override enum :object_format, { sha1: 0, sha256: 1 } @@ -15,4 +18,11 @@ def find_project(disk_path) find_by(disk_path: disk_path)&.project end end + + override :git_garbage_collect_worker_klass + def git_garbage_collect_worker_klass + Projects::GitGarbageCollectWorker + end end + +ProjectRepository.prepend_mod diff --git a/db/migrate/20250825100342_create_project_repository_states.rb b/db/migrate/20250825100342_create_project_repository_states.rb index deadc7b45cfd4afff9d263c72f52c5d1239438af..70833908ad3c8f5e0b250e3c6a37240707b68b8c 100644 --- a/db/migrate/20250825100342_create_project_repository_states.rb +++ b/db/migrate/20250825100342_create_project_repository_states.rb @@ -10,7 +10,7 @@ class CreateProjectRepositoryStates < Gitlab::Database::Migration[2.3] NEEDS_VERIFICATION_INDEX_NAME = "index_project_repository_states_needs_verification" def up - create_table :project_repository_states do |t| # rubocop:disable Migration/EnsureFactoryForTable -- will be added via https://gitlab.com/gitlab-org/gitlab/-/merge_requests/194051 + create_table :project_repository_states do |t| t.datetime_with_timezone :verification_started_at t.datetime_with_timezone :verification_retry_at t.datetime_with_timezone :verified_at diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 5577fa57ae3c2b211f27bc0074522c1846904b2b..c24a9f7bb7c88855e4a4f4a0eb80407595329616 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -42381,6 +42381,7 @@ Represents the Geo replication and verification state of a project repository. | `missingOnPrimary` | [`Boolean`](#boolean) | Indicate if the ProjectRepositoryRegistry is missing on primary. | | `modelRecordId` | [`Int`](#int) | ID of the ProjectRepositoryRegistry's model record. | | `projectId` | [`ID!`](#id) | ID of the Project. | +| `projectRepositoryId` | [`ID`](#id) | ID of the project repository. | | `retryAt` | [`Time`](#time) | Timestamp after which the ProjectRepositoryRegistry is resynced. | | `retryCount` | [`Int`](#int) | Number of consecutive failed sync attempts of the ProjectRepositoryRegistry. | | `state` | [`RegistryState`](#registrystate) | Sync state of the ProjectRepositoryRegistry. | @@ -53643,6 +53644,7 @@ One of: - [`PackageFileRegistry`](#packagefileregistry) - [`PagesDeploymentRegistry`](#pagesdeploymentregistry) - [`PipelineArtifactRegistry`](#pipelineartifactregistry) +- [`ProjectRepositoryRegistry`](#projectrepositoryregistry) - [`ProjectWikiRepositoryRegistry`](#projectwikirepositoryregistry) - [`SnippetRepositoryRegistry`](#snippetrepositoryregistry) - [`TerraformStateVersionRegistry`](#terraformstateversionregistry) diff --git a/ee/app/graphql/types/geo/project_repository_registry_type.rb b/ee/app/graphql/types/geo/project_repository_registry_type.rb index b99e9510f3b0ed83608af3c0173a488d434a6d01..50e143301df743109a2f067204ae54b2e98db29e 100644 --- a/ee/app/graphql/types/geo/project_repository_registry_type.rb +++ b/ee/app/graphql/types/geo/project_repository_registry_type.rb @@ -2,6 +2,10 @@ module Types module Geo + # rubocop:disable GraphQL/ExtractType -- both these fields are needed for backward compatibility + # once Geo's project repository replication v2 is introduced via + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/194051 + # rubocop:disable Graphql/AuthorizeTypes -- because it is included class ProjectRepositoryRegistryType < BaseObject graphql_name 'ProjectRepositoryRegistry' @@ -11,7 +15,13 @@ class ProjectRepositoryRegistryType < BaseObject description 'Represents the Geo replication and verification state of a project repository' field :project_id, GraphQL::Types::ID, null: false, description: 'ID of the Project.' + field :project_repository_id, GraphQL::Types::ID, description: 'ID of the project repository.' + + def project_repository_id + object&.project_repository&.id + end end + # rubocop:enable GraphQL/ExtractType # rubocop:enable Graphql/AuthorizeTypes end end diff --git a/ee/app/graphql/types/geo/registrable_type.rb b/ee/app/graphql/types/geo/registrable_type.rb index 00feec8145170b3dbcc239bbfeb6a624c7c64afd..541de48d5b0db37ff53853f43391bf27480c918a 100644 --- a/ee/app/graphql/types/geo/registrable_type.rb +++ b/ee/app/graphql/types/geo/registrable_type.rb @@ -19,6 +19,7 @@ class RegistrableType < BaseUnion ::Geo::PackageFileRegistry => Types::Geo::PackageFileRegistryType, ::Geo::PagesDeploymentRegistry => Types::Geo::PagesDeploymentRegistryType, ::Geo::PipelineArtifactRegistry => Types::Geo::PipelineArtifactRegistryType, + ::Geo::ProjectRepositoryRegistry => Types::Geo::ProjectRepositoryRegistryType, ::Geo::ProjectWikiRepositoryRegistry => Types::Geo::ProjectWikiRepositoryRegistryType, ::Geo::SnippetRepositoryRegistry => Types::Geo::SnippetRepositoryRegistryType, ::Geo::TerraformStateVersionRegistry => Types::Geo::TerraformStateVersionRegistryType, diff --git a/ee/app/models/concerns/geo/replicable_registry.rb b/ee/app/models/concerns/geo/replicable_registry.rb index 6b8199f8fa509312e0644e171e384c3612409cde..ddb7eaeacb0b4183d6f23f77efaebdbda81fa3ae 100644 --- a/ee/app/models/concerns/geo/replicable_registry.rb +++ b/ee/app/models/concerns/geo/replicable_registry.rb @@ -20,7 +20,7 @@ def state_value(state_string) end def for_model_record_id(id) - find_or_initialize_by(self::MODEL_FOREIGN_KEY => id) + find_or_initialize_by(model_foreign_key => id) end def declarative_policy_class @@ -178,7 +178,7 @@ def mark_synced_atomically # If state is set to pending that means that pending! was called # during the sync so we need to reschedule new sync num_rows = self.class - .where(self.class::MODEL_FOREIGN_KEY => model_record_id) + .where(self.class.model_foreign_key => model_record_id) .with_state(:started) .update_all(state: Geo::ReplicableRegistry::STATE_VALUES[:synced]) diff --git a/ee/app/models/concerns/geo/verifiable_registry.rb b/ee/app/models/concerns/geo/verifiable_registry.rb index ce130a9f5e1c39355a3d7a761e92ad9cc3eb93f6..bf7b900a7c4c91e6a44ba1cb67825d5e7c9c4d5e 100644 --- a/ee/app/models/concerns/geo/verifiable_registry.rb +++ b/ee/app/models/concerns/geo/verifiable_registry.rb @@ -17,7 +17,7 @@ module VerifiableRegistry # Replicators. override :verification_state_model_key def verification_state_model_key - self::MODEL_FOREIGN_KEY + model_foreign_key end end diff --git a/ee/app/models/ee/project_repository.rb b/ee/app/models/ee/project_repository.rb new file mode 100644 index 0000000000000000000000000000000000000000..a8a04e631ddfa08ba65f195b330793dee2b3cc4f --- /dev/null +++ b/ee/app/models/ee/project_repository.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module EE + # ProjectRepository EE mixin + # + # This module is intended to encapsulate EE-specific model logic + # and be prepended in the `ProjectRepository` model + module ProjectRepository # rubocop:disable Gitlab/BoundedContexts -- EE module for existing model + extend ActiveSupport::Concern + + prepended do + include ::Geo::ReplicableModel + include ::Geo::VerifiableModel + + with_replicator Geo::ProjectRepositoryReplicator + + has_one :project_repository_state, + autosave: false, + inverse_of: :project_repository, + foreign_key: :project_repository_id, + class_name: 'Geo::ProjectRepositoryState' + + # Delegate repository-related methods to the associated project + delegate( + :repository, + :repository_storage, + to: :project) + + delegate(*::Geo::VerificationState::VERIFICATION_METHODS, + to: :project_repository_state) + + scope :available_replicables, -> { all } + + scope :with_verification_state, ->(state) do + joins(:project_repository_state) + .where(project_repository_states: { + verification_state: verification_state_value(state) + }) + end + + def verification_state_object + project_repository_state + end + end + + class_methods do + extend ::Gitlab::Utils::Override + + # @return [ActiveRecord::Relation] scope observing selective sync + # settings of the given node + def selective_sync_scope(node, **params) + return all unless node.selective_sync? + + project_scope = ::Project.selective_sync_scope(node, **params) + where(project_id: project_scope.select(:id)) + end + + override :verification_state_model_key + def verification_state_model_key + :project_repository_id + end + + override :verification_state_table_class + def verification_state_table_class + ::Geo::ProjectRepositoryState + end + end + + def project_repository_state + super || build_project_repository_state + end + end # rubocop:enable Gitlab/BoundedContexts +end diff --git a/ee/app/models/geo/base_registry.rb b/ee/app/models/geo/base_registry.rb index 4b27987195a279f550c32c1d540ad4148555b7c0..ca576ed9f7e4077158da17851ee4dbc94c9c0ee0 100644 --- a/ee/app/models/geo/base_registry.rb +++ b/ee/app/models/geo/base_registry.rb @@ -10,19 +10,19 @@ class BaseRegistry < Geo::TrackingBase include GlobalID::Identification def self.pluck_model_ids_in_range(range) - where(self::MODEL_FOREIGN_KEY => range).pluck(self::MODEL_FOREIGN_KEY) + where(model_foreign_key => range).pluck(model_foreign_key) end def self.pluck_model_foreign_key - where(nil).pluck(self::MODEL_FOREIGN_KEY) + where(nil).pluck(model_foreign_key) end def self.model_id_in(ids) - where(self::MODEL_FOREIGN_KEY => ids) + where(model_foreign_key => ids) end def self.model_id_not_in(ids) - where.not(self::MODEL_FOREIGN_KEY => ids) + where.not(model_foreign_key => ids) end def self.ordered_by_id @@ -56,7 +56,7 @@ def self.before_bulk_mark_update_row_scan_max(bulk_mark_update_cursor, bulk_mark def self.insert_for_model_ids(ids) records = ids.map do |id| - new(self::MODEL_FOREIGN_KEY => id, created_at: Time.zone.now) + new(model_foreign_key => id, created_at: Time.zone.now) end bulk_insert!(records, returns: :ids) @@ -73,15 +73,15 @@ def self.delete_worker_class end def self.replicator_class - self::MODEL_CLASS.replicator_class + model_class.replicator_class end def self.find_registry_differences(range) - model_primary_key = self::MODEL_CLASS.primary_key.to_sym + model_primary_key = model_class.primary_key.to_sym - source_ids = self::MODEL_CLASS + source_ids = model_class .replicables_for_current_secondary(range) - .pluck(self::MODEL_CLASS.arel_table[model_primary_key]) + .pluck(model_class.arel_table[model_primary_key]) tracked_ids = pluck_model_ids_in_range(range) @@ -119,11 +119,11 @@ def self.graphql_enum_key def self.with_search(query) return all if query.empty? - where(self::MODEL_FOREIGN_KEY => self::MODEL_CLASS.search(query).limit(1000).pluck_primary_key) + where(model_foreign_key => model_class.search(query).limit(1000).pluck_primary_key) end def model_record_id - read_attribute(self.class::MODEL_FOREIGN_KEY) + read_attribute(self.class.model_foreign_key) end end end diff --git a/ee/app/models/geo/ci_secure_file_registry.rb b/ee/app/models/geo/ci_secure_file_registry.rb index 6d4e4255ed9768a8191d11b150015770210b15d8..90e7fada92bcc0394d038d1c0cd1403bdaa3b22b 100644 --- a/ee/app/models/geo/ci_secure_file_registry.rb +++ b/ee/app/models/geo/ci_secure_file_registry.rb @@ -5,9 +5,14 @@ class CiSecureFileRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::Ci::SecureFile - MODEL_FOREIGN_KEY = :ci_secure_file_id - belongs_to :ci_secure_file, class_name: 'Ci::SecureFile' + + def self.model_class + ::Ci::SecureFile + end + + def self.model_foreign_key + :ci_secure_file_id + end end end diff --git a/ee/app/models/geo/container_repository_registry.rb b/ee/app/models/geo/container_repository_registry.rb index 85c36e0d1ade30b712136332fbef836fcfe81d55..19bb9117ec30aaa71b811dd53d85109c9c21e41c 100644 --- a/ee/app/models/geo/container_repository_registry.rb +++ b/ee/app/models/geo/container_repository_registry.rb @@ -7,9 +7,6 @@ class ContainerRepositoryRegistry < Geo::BaseRegistry include ::Geo::VerifiableRegistry extend ::Gitlab::Utils::Override - MODEL_CLASS = ::ContainerRepository - MODEL_FOREIGN_KEY = :container_repository_id - belongs_to :container_repository ### Remove it after data migration @@ -39,6 +36,14 @@ class << self include Delay extend ::Gitlab::Utils::Override + def model_class + ::ContainerRepository + end + + def model_foreign_key + :container_repository_id + end + ### Remove it after data migration # Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/371667 # diff --git a/ee/app/models/geo/dependency_proxy_blob_registry.rb b/ee/app/models/geo/dependency_proxy_blob_registry.rb index 00c1f0a3e0f66d094a898d5d5ebf97f35f903457..2602dc5980eb493bf9c964b35ca3bba2da65118d 100644 --- a/ee/app/models/geo/dependency_proxy_blob_registry.rb +++ b/ee/app/models/geo/dependency_proxy_blob_registry.rb @@ -5,9 +5,14 @@ class DependencyProxyBlobRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::DependencyProxy::Blob - MODEL_FOREIGN_KEY = :dependency_proxy_blob_id - belongs_to :dependency_proxy_blob, class_name: 'DependencyProxy::Blob' + + def self.model_class + ::DependencyProxy::Blob + end + + def self.model_foreign_key + :dependency_proxy_blob_id + end end end diff --git a/ee/app/models/geo/dependency_proxy_manifest_registry.rb b/ee/app/models/geo/dependency_proxy_manifest_registry.rb index d728842e87b8b7fd7eaa9ac0d1df02cae148ce3b..e34ff609108d2eee5bfab144e63f04ceecb23325 100644 --- a/ee/app/models/geo/dependency_proxy_manifest_registry.rb +++ b/ee/app/models/geo/dependency_proxy_manifest_registry.rb @@ -5,9 +5,14 @@ class DependencyProxyManifestRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::DependencyProxy::Manifest - MODEL_FOREIGN_KEY = :dependency_proxy_manifest_id - belongs_to :dependency_proxy_manifest, class_name: 'DependencyProxy::Manifest' + + def self.model_class + ::DependencyProxy::Manifest + end + + def self.model_foreign_key + :dependency_proxy_manifest_id + end end end diff --git a/ee/app/models/geo/design_management_repository_registry.rb b/ee/app/models/geo/design_management_repository_registry.rb index d4461ca40da469d9b7a0cc43770ed93c29b84299..78d895c3690b2f2c9ae457c6ac6f556c3d4c9d3f 100644 --- a/ee/app/models/geo/design_management_repository_registry.rb +++ b/ee/app/models/geo/design_management_repository_registry.rb @@ -5,9 +5,14 @@ class DesignManagementRepositoryRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::DesignManagement::Repository - MODEL_FOREIGN_KEY = :design_management_repository_id - belongs_to :design_management_repository, class_name: 'DesignManagement::Repository' + + def self.model_class + ::DesignManagement::Repository + end + + def self.model_foreign_key + :design_management_repository_id + end end end diff --git a/ee/app/models/geo/group_wiki_repository_registry.rb b/ee/app/models/geo/group_wiki_repository_registry.rb index b0da460e1af586ab2acc9b7c6d4e7874be6adc29..4a9d152c9159063e08f56dc70b9744fecb1fa097 100644 --- a/ee/app/models/geo/group_wiki_repository_registry.rb +++ b/ee/app/models/geo/group_wiki_repository_registry.rb @@ -5,9 +5,14 @@ class GroupWikiRepositoryRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::GroupWikiRepository - MODEL_FOREIGN_KEY = :group_wiki_repository_id - belongs_to :group_wiki_repository, class_name: 'GroupWikiRepository' + + def self.model_class + ::GroupWikiRepository + end + + def self.model_foreign_key + :group_wiki_repository_id + end end end diff --git a/ee/app/models/geo/job_artifact_registry.rb b/ee/app/models/geo/job_artifact_registry.rb index 1e2fc9f5b73f9e549905d0b51101fec3d8e069ac..493ef0848cb885b5c9f03e55ddb4599e386d2266 100644 --- a/ee/app/models/geo/job_artifact_registry.rb +++ b/ee/app/models/geo/job_artifact_registry.rb @@ -5,11 +5,16 @@ class JobArtifactRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::Ci::JobArtifact - MODEL_FOREIGN_KEY = :artifact_id - ignore_column :success, remove_with: '15.8', remove_after: '2022-12-22' belongs_to :job_artifact, class_name: 'Ci::JobArtifact', foreign_key: :artifact_id + + def self.model_class + ::Ci::JobArtifact + end + + def self.model_foreign_key + :artifact_id + end end end diff --git a/ee/app/models/geo/lfs_object_registry.rb b/ee/app/models/geo/lfs_object_registry.rb index 8b5ffeab17c76442b3d09b8b62d1cc57fcc2db64..ca61a08dd649015c0368208b0e9c830ed0aff48b 100644 --- a/ee/app/models/geo/lfs_object_registry.rb +++ b/ee/app/models/geo/lfs_object_registry.rb @@ -5,13 +5,18 @@ class LfsObjectRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::LfsObject - MODEL_FOREIGN_KEY = :lfs_object_id - belongs_to :lfs_object, class_name: 'LfsObject' scope :for_synced_lfs_objects, ->(lfs_object_ids) { synced.where(lfs_object_id: lfs_object_ids) } + def self.model_class + ::LfsObject + end + + def self.model_foreign_key + :lfs_object_id + end + # @return [Boolean] true if all given oids are synced def self.oids_synced?(oids) unique_oids = oids.uniq diff --git a/ee/app/models/geo/merge_request_diff_registry.rb b/ee/app/models/geo/merge_request_diff_registry.rb index 3a72ae7619a6da0236d729f1354f66cada2bd510..f1a8d9287fce4e24171214f96decac72ad6d1246 100644 --- a/ee/app/models/geo/merge_request_diff_registry.rb +++ b/ee/app/models/geo/merge_request_diff_registry.rb @@ -5,11 +5,16 @@ class MergeRequestDiffRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::MergeRequestDiff - MODEL_FOREIGN_KEY = :merge_request_diff_id - self.table_name = 'merge_request_diff_registry' belongs_to :merge_request_diff, class_name: 'MergeRequestDiff' + + def self.model_class + ::MergeRequestDiff + end + + def self.model_foreign_key + :merge_request_diff_id + end end end diff --git a/ee/app/models/geo/package_file_registry.rb b/ee/app/models/geo/package_file_registry.rb index 5661e9d2297dba823fae81357dcb81ea5598abc3..23265b3e47b2ee2543235eadc40122c6082e5cbf 100644 --- a/ee/app/models/geo/package_file_registry.rb +++ b/ee/app/models/geo/package_file_registry.rb @@ -5,9 +5,14 @@ class PackageFileRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::Packages::PackageFile - MODEL_FOREIGN_KEY = :package_file_id - belongs_to :package_file, class_name: 'Packages::PackageFile' + + def self.model_class + ::Packages::PackageFile + end + + def self.model_foreign_key + :package_file_id + end end end diff --git a/ee/app/models/geo/packages_nuget_symbol_registry.rb b/ee/app/models/geo/packages_nuget_symbol_registry.rb index 432e2e030433eff485ff2809531951bb33b198f6..2e97e6eee045e83cec0db1dc234ba7946fcd838d 100644 --- a/ee/app/models/geo/packages_nuget_symbol_registry.rb +++ b/ee/app/models/geo/packages_nuget_symbol_registry.rb @@ -5,9 +5,14 @@ class PackagesNugetSymbolRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::Packages::Nuget::Symbol - MODEL_FOREIGN_KEY = :packages_nuget_symbol_id - belongs_to :packages_nuget_symbol, class_name: 'Packages::Nuget::Symbol' + + def self.model_class + ::Packages::Nuget::Symbol + end + + def self.model_foreign_key + :packages_nuget_symbol_id + end end end diff --git a/ee/app/models/geo/pages_deployment_registry.rb b/ee/app/models/geo/pages_deployment_registry.rb index 3cfeedb5290efd48e4116a15f8eab398a9eecf61..ccdcde3d0443e1221ef4504987db139db89e5f25 100644 --- a/ee/app/models/geo/pages_deployment_registry.rb +++ b/ee/app/models/geo/pages_deployment_registry.rb @@ -5,9 +5,14 @@ class PagesDeploymentRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::PagesDeployment - MODEL_FOREIGN_KEY = :pages_deployment_id - belongs_to :pages_deployment, class_name: 'PagesDeployment' + + def self.model_class + ::PagesDeployment + end + + def self.model_foreign_key + :pages_deployment_id + end end end diff --git a/ee/app/models/geo/pipeline_artifact_registry.rb b/ee/app/models/geo/pipeline_artifact_registry.rb index 966023581890388c53f064c47673cf1b175ba598..0132b15021700826a1ec0586d17b8ee617dc305d 100644 --- a/ee/app/models/geo/pipeline_artifact_registry.rb +++ b/ee/app/models/geo/pipeline_artifact_registry.rb @@ -5,9 +5,14 @@ class PipelineArtifactRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::Ci::PipelineArtifact - MODEL_FOREIGN_KEY = :pipeline_artifact_id - belongs_to :pipeline_artifact, class_name: '::Ci::PipelineArtifact' + + def self.model_class + ::Ci::PipelineArtifact + end + + def self.model_foreign_key + :pipeline_artifact_id + end end end diff --git a/ee/app/models/geo/project_repository_registry.rb b/ee/app/models/geo/project_repository_registry.rb index dcdb83ed6ad1a4065c5984a57f3b4672fa6b528a..aac96d2be0c171045e0bfc81cb4b07b39ad728ea 100644 --- a/ee/app/models/geo/project_repository_registry.rb +++ b/ee/app/models/geo/project_repository_registry.rb @@ -5,11 +5,65 @@ class ProjectRepositoryRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry extend ::Gitlab::Geo::LogHelpers + extend ::Gitlab::Utils::Override + + # Populate missing foreign keys before validation and after loading + # because we need to ensure that both `project_id` and `project_repository_id` + # are populated if possible, even if only one is initially present, e.g. when + # switching between the two versions of replication - + # legacy (Project based) and V2 (ProjectRepository-based) replication modes + # This is crucial for backward compatibility and to support both the versions + # of replication. + + before_validation :populate_missing_foreign_keys + after_find :populate_missing_foreign_keys + + # Always allow both relationships for backward compatibility + belongs_to :project, class_name: 'Project', optional: true + belongs_to :project_repository, class_name: 'ProjectRepository', optional: true, default: nil + + # Ensure at least one foreign key is present + validates :project_id, presence: true, unless: :project_repository_id? + validates :project_repository_id, presence: true, unless: :project_id? + + override :insert_for_model_ids + def self.insert_for_model_ids(ids) + records = ids.map do |id| + project_id, project_repository_id = get_foreign_keys_for_model_record_id(id) + next unless project_id + + new( + project_id: project_id, + project_repository_id: project_repository_id, + created_at: Time.zone.now + ) + end + + records.compact! + return records if records.empty? - MODEL_CLASS = ::Project - MODEL_FOREIGN_KEY = :project_id + bulk_insert!(records, returns: :ids) + end - belongs_to :project, class_name: 'Project' + def self.get_foreign_keys_for_model_record_id(id) + if model_foreign_key == :project_repository_id + project_repository_id = id + project_id = ProjectRepository.find_by(id: id)&.project&.id + else + project_repository_id = Project.find_by(id: id)&.project_repository&.id + project_id = id + end + + [project_id, project_repository_id] + end + + def self.model_class + ::Gitlab::Geo.geo_project_repository_replication_v2_enabled? ? ::ProjectRepository : ::Project + end + + def self.model_foreign_key + ::Gitlab::Geo.geo_project_repository_replication_v2_enabled? ? :project_repository_id : :project_id + end # Returns whether the project repository is out-of-date on this site # @@ -30,10 +84,17 @@ def self.repository_out_of_date?(project_id, synchronous_request_required = fals # @return [Boolean] whether the project repository is out-of-date on this site def repository_out_of_date?(synchronous_request_required = false) return out_of_date("registry doesn't exist") unless persisted? - return out_of_date("project doesn't exist") if project.nil? + + case model_foreign_key + when :project_repository_id + return out_of_date("project_repository doesn't exist") if project_repository.nil? + else + return out_of_date("project doesn't exist") if project.nil? + end + return out_of_date("sync failed") if failed? - unless project.last_repository_updated_at + unless get_project.last_repository_updated_at return up_to_date("there is no timestamp for the latest change to the repo") end @@ -48,8 +109,11 @@ def repository_out_of_date?(synchronous_request_required = false) # @return [Boolean] whether the latest pipeline refs are present on the secondary def synchronous_pipeline_check - secondary_pipeline_refs = project.repository.list_refs(['refs/pipelines/']).collect(&:name) - primary_pipeline_refs = ::Gitlab::Geo.primary_pipeline_refs(project_id) + repository = get_project.repository + return out_of_date("repository related to project not found") unless repository + + secondary_pipeline_refs = repository.list_refs(['refs/pipelines/']).collect(&:name) + primary_pipeline_refs = ::Gitlab::Geo.primary_pipeline_refs(get_project.id) missing_refs = primary_pipeline_refs - secondary_pipeline_refs if !missing_refs.empty? @@ -67,7 +131,11 @@ def synchronous_pipeline_check # # @return [Boolean] whether the latest change is replicated def best_guess_with_local_information - last_updated_at = project.last_repository_updated_at + project_record = get_project + + return out_of_date("project not found") unless project_record + + last_updated_at = project_record.last_repository_updated_at if last_synced_at <= last_updated_at out_of_date("last successfully synced before latest change", @@ -103,5 +171,36 @@ def up_to_date(reason, details = {}) false end + + def get_project + return project_repository&.project if model_foreign_key == :project_repository_id + + project + end + + # While switching between the legacy replication and replication V2, + # it is possible that one of the foreign keys is populated while the other is not. + # This method ensures that both foreign keys are populated if possible, + # by using the existing populated foreign key to find the missing one. + # For example, if `project_id` is present but `project_repository_id` is not, + # it will try to find the `project_repository_id` from the `project`. + def populate_missing_foreign_keys + # Skip population if we don't have access to the necessary attributes + # This can happen during batch operations that only select specific columns + return unless has_attribute?(:project_id) && has_attribute?(:project_repository_id) + + return if project.present? && project_repository.present? + + # When a corresponding Git repository doesn't exist + # we don't need to replicate it, so we don't need the registry any more + if model_class == ::ProjectRepository && project && !project&.project_repository.present? + log_info("Destroying ProjectRepositoryRegistry##{id} without a corresponding project repository") + + return destroy + end + + self.project_repository ||= project&.project_repository + self.project ||= project_repository&.project + end end end diff --git a/ee/app/models/geo/project_repository_state.rb b/ee/app/models/geo/project_repository_state.rb new file mode 100644 index 0000000000000000000000000000000000000000..6ae2852d4db835394b273666429456da45a0957d --- /dev/null +++ b/ee/app/models/geo/project_repository_state.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Geo + class ProjectRepositoryState < ApplicationRecord + include ::Geo::VerificationStateDefinition + + # Populate missing foreign keys before validation + before_validation :populate_missing_foreign_keys + + self.primary_key = :project_repository_id + + belongs_to :project_repository, inverse_of: :project_repository_state + belongs_to :project + + validates :verification_state, :project_repository, presence: true + + def populate_missing_foreign_keys + # Ensure both foreign keys are populated for compatibility + if project.present? && !project_repository.present? + return unless project&.project_repository + + self.project_repository = project.project_repository + elsif project_repository.present? && !project.present? + return unless project_repository&.project + + self.project = project_repository.project + end + end + end +end diff --git a/ee/app/models/geo/project_wiki_repository_registry.rb b/ee/app/models/geo/project_wiki_repository_registry.rb index 9de6d818188d9fbe67bed119f623d08865cc1b21..a92553ec3bb5b72f69db76416edef418411c00a3 100644 --- a/ee/app/models/geo/project_wiki_repository_registry.rb +++ b/ee/app/models/geo/project_wiki_repository_registry.rb @@ -6,13 +6,18 @@ class ProjectWikiRepositoryRegistry < Geo::BaseRegistry include ::Geo::VerifiableRegistry extend ::Gitlab::Utils::Override - MODEL_CLASS = ::Projects::WikiRepository - MODEL_FOREIGN_KEY = :project_wiki_repository_id - belongs_to :project_wiki_repository, class_name: 'Projects::WikiRepository' validates :project_wiki_repository, presence: true, uniqueness: true delegate :project, :wiki_repository_state, to: :project_wiki_repository, allow_nil: true + + def self.model_class + ::Projects::WikiRepository + end + + def self.model_foreign_key + :project_wiki_repository_id + end end end diff --git a/ee/app/models/geo/snippet_repository_registry.rb b/ee/app/models/geo/snippet_repository_registry.rb index deb9bb2cdc3325a57e55844431e27ce803baadec..f8dce9310d42ab5b79bfc1c97e877405bff2d01e 100644 --- a/ee/app/models/geo/snippet_repository_registry.rb +++ b/ee/app/models/geo/snippet_repository_registry.rb @@ -5,9 +5,14 @@ class SnippetRepositoryRegistry < Geo::BaseRegistry include ::Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::SnippetRepository - MODEL_FOREIGN_KEY = :snippet_repository_id - belongs_to :snippet_repository, class_name: 'SnippetRepository' + + def self.model_class + ::SnippetRepository + end + + def self.model_foreign_key + :snippet_repository_id + end end end diff --git a/ee/app/models/geo/terraform_state_version_registry.rb b/ee/app/models/geo/terraform_state_version_registry.rb index 67893678fcf609c9a7175f8876337f51684c9b8f..a2693b3b974db5b0749ac9d11b852ca13afc176e 100644 --- a/ee/app/models/geo/terraform_state_version_registry.rb +++ b/ee/app/models/geo/terraform_state_version_registry.rb @@ -5,9 +5,14 @@ class TerraformStateVersionRegistry < Geo::BaseRegistry include Geo::ReplicableRegistry include ::Geo::VerifiableRegistry - MODEL_CLASS = ::Terraform::StateVersion - MODEL_FOREIGN_KEY = :terraform_state_version_id - belongs_to :terraform_state_version, class_name: 'Terraform::StateVersion' + + def self.model_class + ::Terraform::StateVersion + end + + def self.model_foreign_key + :terraform_state_version_id + end end end diff --git a/ee/app/models/geo/upload_registry.rb b/ee/app/models/geo/upload_registry.rb index e7b4265ae3a38cdd7edc35c9e077f9799af3658b..517cde711ff5af005941161cc02c64808438a513 100644 --- a/ee/app/models/geo/upload_registry.rb +++ b/ee/app/models/geo/upload_registry.rb @@ -7,19 +7,24 @@ class UploadRegistry < Geo::BaseRegistry extend ::Gitlab::Utils::Override - MODEL_CLASS = ::Upload - MODEL_FOREIGN_KEY = :file_id - self.table_name = 'file_registry' belongs_to :upload, foreign_key: :file_id scope :fresh, -> { order(created_at: :desc) } + def self.model_class + ::Upload + end + + def self.model_foreign_key + :file_id + end + def self.find_registry_differences(range) source = - self::MODEL_CLASS.replicables_for_current_secondary(range) - .pluck(self::MODEL_CLASS.arel_table[:id]) + model_class.replicables_for_current_secondary(range) + .pluck(model_class.arel_table[:id]) tracked = model_id_in(range) diff --git a/ee/app/replicators/geo/project_repository_replicator.rb b/ee/app/replicators/geo/project_repository_replicator.rb index 632b0a0c7a759de7caa8e8b59b4083eb655905fe..c35952257f3bace6fa8dd021978f12607f4f2376 100644 --- a/ee/app/replicators/geo/project_repository_replicator.rb +++ b/ee/app/replicators/geo/project_repository_replicator.rb @@ -3,9 +3,14 @@ module Geo class ProjectRepositoryReplicator < Gitlab::Geo::Replicator include ::Geo::RepositoryReplicatorStrategy + extend Gitlab::Utils::Override + + # Default to Project as the model for the initial call to avoid calling a database connection in the initializer def self.model - ::Project + return ::Project unless Rails.application.initialized? + + ::Gitlab::Geo.geo_project_repository_replication_v2_enabled? ? ::ProjectRepository : ::Project end # @return [String] human-readable title. @@ -24,13 +29,35 @@ def before_housekeeping create_object_pool_on_secondary if create_object_pool_on_secondary? end + # TODO Not completed sure if this method is needed + + # override :housekeeping_enabled? + # def self.housekeeping_enabled? + # false + # end + def repository model_record.repository end + override :should_publish_replication_event? + def should_publish_replication_event? + return false unless super + + if ::Gitlab::Geo.geo_project_repository_replication_v2_enabled? + # V2: Only create events for ProjectRepository objects, not Projects + model_record.is_a?(::ProjectRepository) + else + # V1: Only create events for Project objects, not ProjectRepositories + model_record.is_a?(::Project) + end + end + private def pool_repository + return model_record.project.pool_repository if ::Gitlab::Geo.geo_project_repository_replication_v2_enabled? + model_record.pool_repository end @@ -38,8 +65,14 @@ def create_object_pool_on_secondary Geo::CreateObjectPoolService.new(pool_repository).execute end + def object_pool_missing? + return model_record.project.object_pool_missing? if ::Gitlab::Geo.geo_project_repository_replication_v2_enabled? + + model_record.object_pool_missing? + end + def create_object_pool_on_secondary? - return unless model_record.object_pool_missing? + return unless object_pool_missing? return unless pool_repository.source_project_repository.exists? true diff --git a/ee/app/services/geo/registry_consistency_service.rb b/ee/app/services/geo/registry_consistency_service.rb index 2ed870e09d54df293e84511869b5f99177114612..0ed6149a597bab878955be41eb81c83b010dc6e6 100644 --- a/ee/app/services/geo/registry_consistency_service.rb +++ b/ee/app/services/geo/registry_consistency_service.rb @@ -10,7 +10,7 @@ class RegistryConsistencyService def initialize(registry_class, batch_size:) @registry_class = registry_class - @model_class = registry_class::MODEL_CLASS + @model_class = registry_class.model_class @batch_size = batch_size end @@ -43,7 +43,7 @@ def handle_differences_in_range(range) untracked, unused = find_registry_differences(range) created_in_range = create_untracked_in_range(untracked) - log_created(range, untracked, created_in_range) + log_created(range, untracked, created_in_range) unless created_in_range.empty? deleted_in_range = delete_unused_in_range(unused) log_deleted(range, unused, deleted_in_range) diff --git a/ee/app/workers/ee/repositories/post_receive_worker.rb b/ee/app/workers/ee/repositories/post_receive_worker.rb index 8dacf9ef1ce69b141f0506d4ff0e7bb381e30c65..d737f0315d5c8cde76d35081c138ae4ef8e0cc83 100644 --- a/ee/app/workers/ee/repositories/post_receive_worker.rb +++ b/ee/app/workers/ee/repositories/post_receive_worker.rb @@ -53,6 +53,23 @@ def replicate_snippet_changes(snippet) def replicate_design_management_repository_changes(design_management_repository) design_management_repository.geo_handle_after_update if design_management_repository end + + override :process_project_changes + def process_project_changes(post_received, project) + # Call parent method first + result = super + + # Only add Geo replication if V2 is enabled and we have a project repository + if ::Gitlab::Geo.geo_project_repository_replication_v2_enabled? && + project.project_repository && + ::Gitlab::Geo.primary? && post_received.changes.any? + + # Only trigger if changes were actually processed + project.project_repository.replicator.geo_handle_after_update + end + + result + end end end end diff --git a/ee/config/feature_flags/ops/geo_project_repository_replication_v2.yml b/ee/config/feature_flags/ops/geo_project_repository_replication_v2.yml new file mode 100644 index 0000000000000000000000000000000000000000..ff75ab3b40e38de16927f73c4b0f735b8cf697d3 --- /dev/null +++ b/ee/config/feature_flags/ops/geo_project_repository_replication_v2.yml @@ -0,0 +1,8 @@ +--- +name: geo_project_repository_replication_v2 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/194051 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/549772 +milestone: '18.6' +type: ops +group: group::geo +default_enabled: false diff --git a/ee/lib/gitlab/geo.rb b/ee/lib/gitlab/geo.rb index 4fbab4c1ad4a98ba3a9aa84980406dd587e3b43e..39849af9624955f1d77a9f24a70bf7bbe69285c6 100644 --- a/ee/lib/gitlab/geo.rb +++ b/ee/lib/gitlab/geo.rb @@ -364,5 +364,9 @@ def self.org_mover_extend_selective_sync_to_primary_checksumming? def self.geo_selective_sync_by_organizations_enabled? ::Feature.enabled?(:geo_selective_sync_by_organizations, :instance) end + + def self.geo_project_repository_replication_v2_enabled? + ::Feature.enabled?(:geo_project_repository_replication_v2, :instance) + end end end diff --git a/ee/lib/gitlab/geo/registry_batcher.rb b/ee/lib/gitlab/geo/registry_batcher.rb index a0e03ccd50028e91c903b417a421988d470536d0..943abcf318e520d9e147d3b8e2b494baf6f765a4 100644 --- a/ee/lib/gitlab/geo/registry_batcher.rb +++ b/ee/lib/gitlab/geo/registry_batcher.rb @@ -4,7 +4,7 @@ module Gitlab module Geo class RegistryBatcher < BaseBatcher def initialize(registry_class, key:, batch_size: 1000) - super(registry_class::MODEL_CLASS, registry_class, registry_class::MODEL_FOREIGN_KEY, key: key, batch_size: batch_size) + super(registry_class.model_class, registry_class, registry_class.model_foreign_key, key: key, batch_size: batch_size) end end end diff --git a/ee/lib/gitlab/geo/replicator.rb b/ee/lib/gitlab/geo/replicator.rb index 29a584c889ac7d9556565a323db39d22fa10388f..1a83864f4194ebcb65dfec9b0633c95dfdacc144 100644 --- a/ee/lib/gitlab/geo/replicator.rb +++ b/ee/lib/gitlab/geo/replicator.rb @@ -353,6 +353,8 @@ def resource_exists? protected + # This method can be overriden in a replicator class to add other + # conditionals for accepting events def should_publish_replication_event? self.class.replication_enabled? end diff --git a/ee/spec/factories/geo/project_repository_registry.rb b/ee/spec/factories/geo/project_repository_registry.rb index 0e081fba20d6abdf75880b5ed3b9cdf3f88fb25e..9734995e0ff0e06dfb1f7b38490d6cec5cf4e4a0 100644 --- a/ee/spec/factories/geo/project_repository_registry.rb +++ b/ee/spec/factories/geo/project_repository_registry.rb @@ -2,9 +2,30 @@ FactoryBot.define do factory :geo_project_repository_registry, class: 'Geo::ProjectRepositoryRegistry' do - project # This association should have data, like a file or repository + project { association(:project_with_repo) } + project_repository do + ::Gitlab::Geo.geo_project_repository_replication_v2_enabled? ? project.project_repository : nil + end + + # after(:build) do |registry| + # # registry.project.track_project_repository + + # # Only set project_repository_id in replication V2 + # if ::Gitlab::Geo.geo_project_repository_replication_v2_enabled? + # registry.project_repository_id = registry.project.project_repository.id + # if registry.project&.project_repository + # else + # # Ensure it's nil in legacy replication V1 so that it is populated at the time of validation + # registry.project_repository_id = nil + # end + # end + state { Geo::ProjectRepositoryRegistry.state_value(:pending) } + factory :geo_project_repository_registry_replication_v2, parent: :geo_project_repository_registry do + # project_repository { project.project_repository } + end + trait :synced do state { Geo::ProjectRepositoryRegistry.state_value(:synced) } last_synced_at { 5.days.ago } @@ -33,10 +54,8 @@ trait :verification_failed do synced - verification_failure { 'Could not calculate the checksum' } verification_state { Geo::ProjectRepositoryRegistry.verification_state_value(:verification_failed) } - verification_retry_count { 1 } - verification_retry_at { 2.hours.from_now } + verification_failure { 'Verification failed' } end end end diff --git a/ee/spec/factories/geo/project_repository_states.rb b/ee/spec/factories/geo/project_repository_states.rb new file mode 100644 index 0000000000000000000000000000000000000000..f8376a22354bb6b5c4f29d22ffe55883ceeb7e5d --- /dev/null +++ b/ee/spec/factories/geo/project_repository_states.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :geo_project_repository_state, class: 'Geo::ProjectRepositoryState' do + project_repository + + trait :checksummed do + verification_checksum { 'abc' } + end + + trait :checksum_failure do + verification_failure { 'Could not calculate the checksum' } + end + end +end diff --git a/ee/spec/factories/project_repositories.rb b/ee/spec/factories/project_repositories.rb new file mode 100644 index 0000000000000000000000000000000000000000..6d74bc18f46cad987a5fd4f21a52fa20e082f0a8 --- /dev/null +++ b/ee/spec/factories/project_repositories.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +FactoryBot.modify do + factory :project_repository do + project + + trait :verification_succeeded do + repository + verification_checksum { 'abc' } + verification_state { ProjectRepository.verification_state_value(:verification_succeeded) } + end + + trait :verification_failed do + repository + verification_failure { 'Could not calculate the checksum' } + verification_state { ProjectRepository.verification_state_value(:verification_failed) } + + # Geo::VerifiableReplicator#after_verifiable_update tries to verify the replicable async and + # marks it as verification pending when the model record is created/updated. + # + # Tip: You must set current node to primary, or else you can get a PG::ForeignKeyViolation + # because save_verification_details is returning early. + after(:create) do |instance, evaluator| + instance.verification_failure = evaluator.verification_failure + instance.verification_failed! + end + end + end +end diff --git a/ee/spec/graphql/types/geo/project_repository_registry_type_spec.rb b/ee/spec/graphql/types/geo/project_repository_registry_type_spec.rb index c2f05556a46150f123ec250958d7393f0890f313..79365c72bb0614141db827f06c683f315ae9714d 100644 --- a/ee/spec/graphql/types/geo/project_repository_registry_type_spec.rb +++ b/ee/spec/graphql/types/geo/project_repository_registry_type_spec.rb @@ -6,7 +6,7 @@ it_behaves_like 'a Geo registry type' it 'has the expected fields (other than those included in RegistryType)' do - expected_fields = %i[project_id] + expected_fields = %i[project_id project_repository_id] expect(described_class).to have_graphql_fields(*expected_fields).at_least end diff --git a/ee/spec/models/ee/project_repository_spec.rb b/ee/spec/models/ee/project_repository_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..831e50e40f4a2cfb5554b3aa19786d17f74ecb6b --- /dev/null +++ b/ee/spec/models/ee/project_repository_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ProjectRepository, feature_category: :geo_replication do + include EE::GeoHelpers + + describe 'associations' do + it 'has one project_repository_state association' do + is_expected + .to have_one(:project_repository_state) + .class_name('Geo::ProjectRepositoryState') + .inverse_of(:project_repository) + .autosave(false) + end + end + + include_examples 'a verifiable model for verification state' do + let(:skip_unverifiable_model_record_tests) { true } + + let(:verifiable_model_record) { build(:project_repository) } + let(:unverifiable_model_record) { nil } + end +end diff --git a/ee/spec/models/geo/project_repository_registry_replication_v2_spec.rb b/ee/spec/models/geo/project_repository_registry_replication_v2_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..c207f575e4e883f6d0857fac229bf8ea140c5245 --- /dev/null +++ b/ee/spec/models/geo/project_repository_registry_replication_v2_spec.rb @@ -0,0 +1,205 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# Tests for project repositories replication v2 +RSpec.describe Geo::ProjectRepositoryRegistry, :geo, feature_category: :geo_replication do + include ::EE::GeoHelpers + + let_it_be(:project) { create(:project_with_repo) } + let_it_be(:registry) { build(:geo_project_repository_registry, project: project) } + + specify 'factory is valid' do + expect(registry).to be_valid + end + + include_examples 'a Geo framework registry' + + describe '.repository_out_of_date?' do + let_it_be(:project) { create(:project_with_repo) } + + context 'for a non-Geo setup' do + it 'returns false' do + expect(described_class.repository_out_of_date?(project.id)).to be_falsey + end + end + + context 'for a Geo setup' do + before do + stub_current_geo_node(current_node) + end + + context 'for a Geo Primary' do + let(:current_node) { create(:geo_node, :primary) } + + it 'returns false' do + expect(described_class.repository_out_of_date?(project.id)).to be_falsey + end + end + + context 'for a Geo secondary' do + let(:current_node) { create(:geo_node) } + + context 'when Primary node is not configured' do + it 'returns false' do + expect(described_class.repository_out_of_date?(project.id)).to be_falsey + end + end + + context 'when Primary node is configured' do + before do + create(:geo_node, :primary) + end + + context 'when project_repository_registry entry does not exist' do + it 'returns true' do + expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including( + message: "out-of-date", reason: "registry doesn't exist")) + + expect(described_class.repository_out_of_date?(project.id)).to be_truthy + end + end + + context 'when project_repository_registry entry does exist' do + context 'when last_repository_updated_at is not set' do + it 'returns false' do + registry = create(:geo_project_repository_registry, :synced, project: project) + registry.project.update!(last_repository_updated_at: nil) + + expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including( + message: "up-to-date", reason: "there is no timestamp for the latest change to the repo")) + + expect(described_class.repository_out_of_date?(registry.project_id)).to be_falsey + end + end + + context 'when synchronous_request_required is true' do + let_it_be(:project) { create(:project, :pipeline_refs) } + let(:registry) { create(:geo_project_repository_registry, :verification_succeeded, project: project) } + let(:secondary_pipeline_refs) { Array.new(10) { |x| "refs/pipelines/#{x}" } } + let(:some_secondary_pipeline_refs) { Array.new(9) { |x| "refs/pipelines/#{x}" } } + + context 'when the primary has pipeline refs the secondary does not have' do + let_it_be(:project) { create(:project, :pipeline_refs, pipeline_count: 9) } + + it 'returns true' do + project.track_project_repository + + allow(::Gitlab::Geo).to receive(:primary_pipeline_refs) + .with(registry.project_id).and_return(secondary_pipeline_refs) + + expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including( + message: "out-of-date", reason: "secondary is missing pipeline refs")) + + expect(described_class.repository_out_of_date?(registry.project_id, true)).to be_truthy + end + end + + context 'when the secondary has pipeline refs the primary does not have' do + it 'returns false' do + project.track_project_repository + + allow(::Gitlab::Geo).to receive(:primary_pipeline_refs) + .with(registry.project_id).and_return(some_secondary_pipeline_refs) + + expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including( + message: "up-to-date", reason: "secondary has all pipeline refs")) + + expect(described_class.repository_out_of_date?(registry.project_id, true)).to be_falsey + end + end + + context 'when pipeline refs are the same on primary and secondary' do + it 'returns false' do + project.track_project_repository + + allow(::Gitlab::Geo).to receive(:primary_pipeline_refs) + .with(registry.project_id).and_return(secondary_pipeline_refs) + + expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including( + message: "up-to-date", reason: "secondary has all pipeline refs")) + + expect(described_class.repository_out_of_date?(registry.project_id, true)).to be_falsey + end + end + end + + context 'when last_repository_updated_at is set' do + context 'when sync failed' do + it 'returns true' do + registry = create(:geo_project_repository_registry, :failed, project: project) + + expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including( + message: "out-of-date", reason: "sync failed")) + + expect(described_class.repository_out_of_date?(registry.project_id)).to be_truthy + end + end + + context 'when last_synced_at is not set' do + it 'returns true' do + registry = create(:geo_project_repository_registry, project: project, last_synced_at: nil) + + expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including( + message: "out-of-date", reason: "it has never been synced")) + + expect(described_class.repository_out_of_date?(registry.project_id)).to be_truthy + end + end + + context 'when verification failed' do + it 'returns true' do + registry = create(:geo_project_repository_registry, :verification_failed, project: project) + + expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including( + message: "out-of-date", reason: "not verified yet")) + + expect(described_class.repository_out_of_date?(registry.project_id)).to be_truthy + end + end + + context 'when verification succeeded' do + it 'returns false' do + registry = create(:geo_project_repository_registry, :verification_succeeded, + project: project, last_synced_at: Time.current + 5.minutes) + + expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including( + message: "up-to-date", reason: "last successfully synced after latest change")) + + expect(described_class.repository_out_of_date?(registry.project_id)).to be_falsey + end + end + + context 'when last_synced_at is set', :freeze_time do + using RSpec::Parameterized::TableSyntax + + where(:project_last_updated, :project_registry_last_synced, :expected) do + Time.current | (Time.current - 1.minute) | true + (Time.current - 2.minutes) | (Time.current - 1.minute) | false + (Time.current - 3.minutes) | (Time.current - 1.minute) | false + (Time.current - 3.minutes) | (Time.current - 5.minutes) | true + end + + with_them do + before do + project.update!(last_repository_updated_at: project_last_updated) + + create(:geo_project_repository_registry, :verification_succeeded, + project: project, last_synced_at: project_registry_last_synced) + end + + it 'returns the expected value' do + message = expected ? 'out-of-date' : 'up-to-date' + + expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including(message: message)) + expect(described_class.repository_out_of_date?(project.id)).to eq(expected) + end + end + end + end + end + end + end + end + end +end diff --git a/ee/spec/models/geo/project_repository_registry_spec.rb b/ee/spec/models/geo/project_repository_registry_spec.rb index 0740a4c5d5e14474101fc77d803cae609417400c..78e8f8aec015b3cca9e810be3d541a6fc5907bac 100644 --- a/ee/spec/models/geo/project_repository_registry_spec.rb +++ b/ee/spec/models/geo/project_repository_registry_spec.rb @@ -5,7 +5,14 @@ RSpec.describe Geo::ProjectRepositoryRegistry, :geo, type: :model, feature_category: :geo_replication do include ::EE::GeoHelpers - let_it_be(:registry) { build(:geo_project_repository_registry) } + before do + stub_feature_flags(geo_project_repository_replication_v2: false) + end + + let_it_be(:project) { create(:project_with_repo) } + let_it_be(:project_repository) { project.project_repository } + + let_it_be(:registry) { build(:geo_project_repository_registry, project: project) } specify 'factory is valid' do expect(registry).to be_valid @@ -194,4 +201,35 @@ end end end + + describe ".insert_for_model_ids" do + context "when using legacy verification" do + it "inserts a registry record for give project id" do + expect { described_class.insert_for_model_ids([project.id]) }.to change { + Geo::ProjectRepositoryRegistry.count + }.by(1) + end + end + + context "when replication v2 is enabled" do + before do + stub_feature_flags(geo_project_repository_replication_v2: true) + end + + it "inserts a registry record for give project_repository id" do + expect { described_class.insert_for_model_ids([project_repository.id]) }.to change { + Geo::ProjectRepositoryRegistry.count + }.by(1) + end + + context "when project repository does not exist" do + it "inserts a registry record with project_repository_id nil" do + # using an invalid project_repository_id + expect { described_class.insert_for_model_ids([ProjectRepository.last.id + 1]) }.not_to change { + Geo::ProjectRepositoryRegistry.count + } + end + end + end + end end diff --git a/ee/spec/models/geo/project_repository_state_spec.rb b/ee/spec/models/geo/project_repository_state_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..7044226b077a9bdf124d24506957af93c4268830 --- /dev/null +++ b/ee/spec/models/geo/project_repository_state_spec.rb @@ -0,0 +1,153 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Geo::ProjectRepositoryState, feature_category: :geo_replication do + include EE::GeoHelpers + + let_it_be(:project) { create(:project_with_repo) } + let_it_be(:project_repository) { project.project_repository } + + before do + stub_current_geo_node(create(:geo_node, :primary)) + end + + describe 'associations' do + it { is_expected.to belong_to(:project_repository).inverse_of(:project_repository_state) } + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:verification_state) } + it { is_expected.to validate_presence_of(:project_repository) } + end + + describe 'primary key' do + it 'uses project_repository_id as primary key' do + expect(described_class.primary_key).to eq('project_repository_id') + end + end + + describe '#populate_missing_foreign_keys' do + context 'when project_id is missing but project_repository is present' do + it 'populates project_id from project_repository' do + state = build(:geo_project_repository_state, + project_repository: project_repository, + project: nil) + + # Trigger validation which calls populate_missing_foreign_keys + state.valid? + + expect(state.project).to eq(project) + expect(state.project_id).to eq(project.id) + end + + it 'saves successfully after foreign key population' do + state = build(:geo_project_repository_state, + project_repository: project_repository, + project: nil) + + expect(state.save).to be_truthy + expect(state.reload.project_id).to eq(project.id) + end + end + + context 'when project_repository_id is missing but project is present' do + it 'populates project_repository from project' do + state = build(:geo_project_repository_state, + project: project, + project_repository: nil) + + # Trigger validation which calls populate_missing_foreign_keys + state.valid? + + expect(state.project_repository).to eq(project_repository) + expect(state.project_repository_id).to eq(project_repository.id) + end + + it 'saves successfully after foreign key population' do + state = build(:geo_project_repository_state, + project: project, + project_repository: nil) + + expect(state.save).to be_truthy + expect(state.reload.project_repository_id).to eq(project_repository.id) + end + end + + context 'when both foreign keys are present' do + it 'does not modify existing values' do + state = build(:geo_project_repository_state, + project: project, + project_repository: project_repository) + + original_project_id = state.project_id + original_repo_id = state.project_repository_id + + state.valid? + + expect(state.project_id).to eq(original_project_id) + expect(state.project_repository_id).to eq(original_repo_id) + end + end + + # This is an edge case that shouldn't happen in practice + # If there is no project_repository record there should be no project_repository_state record + context 'when project does not have a project_repository' do + let(:project_without_repo) { create(:project) } + + it 'handles gracefully when project has no project_repository' do + state = build(:geo_project_repository_state, + project: project_without_repo, + project_repository: nil) + + expect(state.valid?).to be_falsey + + expect(state.project_repository).to be_nil + expect(state.project_repository_id).to be_nil + end + end + + context 'when project_repository has no project (edge case)' do + it 'handles orphaned project_repository gracefully' do + # This is an edge case that shouldn't happen in practice + state = build(:geo_project_repository_state, + project: nil, + project_repository_id: 99999) + + expect { state.valid? }.not_to raise_error + expect(state.project).to be_nil + end + end + end + + describe 'verification state behavior' do + let(:state) { create(:geo_project_repository_state, project_repository: project_repository) } + + it 'inherits verification state methods from VerificationStateDefinition' do + expect(state).to respond_to(:verification_pending!) + expect(state).to respond_to(:verification_succeeded!) + expect(state).to respond_to(:verification_failed!) + end + + it 'can transition between verification states' do + expect(state.verification_pending?).to be_truthy + + # Use the proper method with checksum for succeeded state + state.update!(verification_checksum: 'abc123') + state.verification_started! + state.verification_succeeded! + expect(state.verification_succeeded?).to be_truthy + + state.verification_started! + state.before_verification_failed + state.update!(verification_failure: "Verification failed") + state.verification_failed! + expect(state.verification_failed?).to be_truthy + end + + it 'includes VerificationStateDefinition module' do + expect(described_class.included_modules).to include(Geo::VerificationStateDefinition) + end + end +end diff --git a/ee/spec/models/geo_node_status_spec.rb b/ee/spec/models/geo_node_status_spec.rb index d560b05d6fd323d2ea678b9b6c5c4e18cfbffc2e..47212d559ce88974f3d123f6a4256fe18b63885f 100644 --- a/ee/spec/models/geo_node_status_spec.rb +++ b/ee/spec/models/geo_node_status_spec.rb @@ -135,6 +135,10 @@ end describe '#projects_count' do + before do + stub_feature_flags(geo_project_repository_replication_v2: false) + end + it 'returns nil on a primary site' do stub_current_geo_node(primary) @@ -152,6 +156,10 @@ end describe '#repositories_count' do + before do + stub_feature_flags(geo_project_repository_replication_v2: false) + end + it 'counts the number of project repositories on a primary site' do stub_current_geo_node(primary) @@ -450,13 +458,14 @@ context 'Replicator stats' do before do + stub_feature_flags(geo_project_repository_replication_v2: false) Project.delete_all stub_geo_setting(registry_replication: { enabled: true }) end where( - replicator: Gitlab::Geo::REPLICATOR_CLASSES + replicator: Gitlab::Geo::REPLICATOR_CLASSES.reject { |r| r == Geo::ProjectRepositoryReplicator } ) with_them do @@ -680,6 +689,238 @@ end end + # TODO Work in progress + context 'Project Repository Replicator stats when v2 replication is enabled' do + before do + stub_feature_flags(geo_project_repository_replication_v2: true) + Project.delete_all + + stub_geo_setting(registry_replication: { enabled: true }) + end + + let(:replicator) { Geo::ProjectRepositoryReplicator } + + with_them do + let(:registry_class) { replicator.registry_class } + let(:replicable_name) { replicator.replicable_name_plural } + let(:model_factory) { model_class_factory_name(registry_class) } + let(:registry_factory) { registry_factory_name(registry_class) } + + context 'replication' do + context 'on the primary' do + before do + stub_current_geo_node(primary) + end + + describe '#_count' do + let(:replicable_count_method) { "#{replicable_name}_count" } + + context 'when replication is enabled' do + before do + allow(replicator).to receive(:replication_enabled?).and_return(true) + end + + context 'when there are replicables' do + before do + create_list(model_factory, 2) + end + + it 'returns the number of available replicables on primary' do + expect(subject.send(replicable_count_method)).to eq(2) + end + end + + context 'when there are no replicables' do + it 'returns 0' do + expect(subject.send(replicable_count_method)).to eq(0) + end + end + end + + context 'when replication is disabled' do + before do + allow(replicator).to receive(:replication_enabled?).and_return(false) + end + + context 'and primary checksumming is enabled' do + before do + allow(replicator).to receive(:verification_enabled?).and_return(true) + end + + context 'when there are replicables' do + before do + create_list(model_factory, 2) + end + + it 'returns the number of available replicables on primary' do + expect(subject.send(replicable_count_method)).to eq(2) + end + end + + context 'when there are no replicables' do + it 'returns 0' do + expect(subject.send(replicable_count_method)).to eq(0) + end + end + end + + context 'and primary checksumming is disabled' do + before do + allow(replicator).to receive(:verification_enabled?).and_return(false) + + create_list(model_factory, 1) + end + + it 'returns nil' do + expect(subject.send(replicable_count_method)).to be_nil + end + end + end + end + end + + context 'on the secondary' do + let(:registry_count_method) { "#{replicable_name}_registry_count" } + let(:failed_count_method) { "#{replicable_name}_failed_count" } + let(:synced_count_method) { "#{replicable_name}_synced_count" } + let(:synced_in_percentage_method) { "#{replicable_name}_synced_in_percentage" } + + before do + stub_current_geo_node(secondary) + end + + describe '#_(registry|synced|failed)_count' do + context 'when there are registries' do + before do + create(registry_factory, :failed) + create(registry_factory, :failed) + create(registry_factory, :synced) + end + + it 'returns the right counts', :aggregate_failures do + expect(subject.send(registry_count_method)).to eq(3) + + expect(subject.send(failed_count_method)).to eq(2) + expect(subject.send(synced_count_method)).to eq(1) + + expect(subject.send(synced_in_percentage_method)).to be_within(0.01).of(33.33) + end + end + + context 'when there are no registries' do + it 'returns 0', :aggregate_failures do + expect(subject.send(registry_count_method)).to eq(0) + + expect(subject.send(failed_count_method)).to eq(0) + expect(subject.send(synced_count_method)).to eq(0) + + expect(subject.send(synced_in_percentage_method)).to eq(0) + end + end + end + end + end + + context 'verification' do + context 'on the primary' do + let(:checksummed_count_method) { "#{replicable_name}_checksummed_count" } + let(:checksum_failed_count_method) { "#{replicable_name}_checksum_failed_count" } + + before do + stub_current_geo_node(primary) + end + + context 'when verification is enabled' do + before do + allow(replicator).to receive(:verification_enabled?).and_return(true) + end + + context 'when there are replicables' do + before do + create(model_factory, :verification_succeeded) + create(model_factory, :verification_succeeded) + create(model_factory, :verification_failed) + end + + it 'returns the right checksum counts', :aggregate_failures do + expect(subject.send(checksummed_count_method)).to eq(2) + expect(subject.send(checksum_failed_count_method)).to eq(1) + end + end + + context 'when there are no replicables' do + it 'returns 0', :aggregate_failures do + expect(subject.send(checksummed_count_method)).to eq(0) + expect(subject.send(checksum_failed_count_method)).to eq(0) + end + end + end + + context 'when verification is disabled' do + before do + allow(replicator).to receive(:verification_enabled?).and_return(false) + end + + it 'returns nil', :aggregate_failures do + expect(subject.send(checksummed_count_method)).to be_nil + expect(subject.send(checksum_failed_count_method)).to be_nil + end + end + end + + context 'on the secondary' do + let(:verified_count_method) { "#{replicable_name}_verified_count" } + let(:verification_failed_count_method) { "#{replicable_name}_verification_failed_count" } + let(:verified_in_percentage_method) { "#{replicable_name}_verified_in_percentage" } + + before do + stub_current_geo_node(secondary) + end + + context 'when verification is enabled' do + before do + allow(replicator).to receive(:verification_enabled?).and_return(true) + end + + context 'when there are replicables' do + before do + create(registry_factory, :verification_succeeded) + create(registry_factory, :verification_succeeded) + create(registry_factory, :verification_failed) + end + + it 'returns the right counts and percentage', :aggregate_failures do + expect(subject.send(verified_count_method)).to eq(2) + expect(subject.send(verification_failed_count_method)).to eq(1) + expect(subject.send(verified_in_percentage_method)).to be_within(0.01).of(66.67) + end + end + + context 'when there are no replicables' do + it 'returns 0', :aggregate_failures do + expect(subject.send(verified_count_method)).to eq(0) + expect(subject.send(verification_failed_count_method)).to eq(0) + expect(subject.send(verified_in_percentage_method)).to eq(0) + end + end + end + + context 'when verification is disabled' do + before do + allow(replicator).to receive(:verification_enabled?).and_return(false) + end + + it 'returns nil', :aggregate_failures do + expect(subject.send(verified_count_method)).to be_nil + expect(subject.send(verification_failed_count_method)).to be_nil + expect(subject.send(verified_in_percentage_method)).to eq(0) + end + end + end + end + end + end + describe '#load_data_from_current_node' do context 'on the primary' do before do diff --git a/ee/spec/replicators/geo/project_repository_replicator_spec.rb b/ee/spec/replicators/geo/project_repository_replicator_spec.rb index d543a83aff5d8025588f995932f5721af7fa2e19..ea150bdd31f56da6553c7ef52fb27314f5a7cc18 100644 --- a/ee/spec/replicators/geo/project_repository_replicator_spec.rb +++ b/ee/spec/replicators/geo/project_repository_replicator_spec.rb @@ -3,9 +3,27 @@ require 'spec_helper' RSpec.describe Geo::ProjectRepositoryReplicator, feature_category: :geo_replication do - let(:model_record) { create(:project, :repository) } + include EE::GeoHelpers + + let(:secondary) { create(:geo_node) } + let(:primary) { create(:geo_node, :primary) } + + before do + stub_current_geo_node(secondary) + end + + context 'with legacy project repository replication (V1)' do + let(:model_record) { create(:project_with_repo) } + let(:project) { model_record } + + subject(:replicator) { model_record.replicator } + + before do + stub_feature_flags(geo_project_repository_replication_v2: false) + end + + it_behaves_like 'a repository replicator' - include_examples 'a repository replicator' do describe 'housekeeping implementation' do let_it_be(:pool_repository) { create(:pool_repository) } let_it_be(:model_record) { create(:project, pool_repository: pool_repository) } @@ -14,6 +32,42 @@ stub_current_geo_node(secondary) end + it 'calls Geo::CreateObjectPoolService' do + stub_secondary_node + + expect_next_instance_of(Geo::CreateObjectPoolService) do |service| + expect(service).to receive(:execute) + end + + replicator = described_class.new(model_record_id: model_record.id) + replicator.before_housekeeping + end + end + + include_examples 'a verifiable replicator' do + before do + stub_current_geo_node(primary) + end + end + end + + context 'with project repository replication v2' do + let(:project) { create(:project_with_repo) } + let(:model_record) { project.project_repository } + + subject(:replicator) { model_record.replicator } + + it_behaves_like 'a repository replicator' + + describe 'housekeeping implementation' do + let_it_be(:pool_repository) { create(:pool_repository) } + let_it_be(:project) { create(:project_with_repo, pool_repository: pool_repository) } + let_it_be(:model_record) { project.project_repository } + + before do + stub_current_geo_node(secondary) + end + it 'calls Geo::CreateObjectPoolService' do stub_secondary_node @@ -24,5 +78,11 @@ replicator.before_housekeeping end end + + include_examples 'a verifiable replicator' do + before do + stub_current_geo_node(primary) + end + end end end diff --git a/ee/spec/requests/api/graphql/geo/registries_spec.rb b/ee/spec/requests/api/graphql/geo/registries_spec.rb index 23365615db41f49ffe958ef96e3aae3a18cbfcf6..fb353ef01767cc6752e92d4186c4e8579265efde 100644 --- a/ee/spec/requests/api/graphql/geo/registries_spec.rb +++ b/ee/spec/requests/api/graphql/geo/registries_spec.rb @@ -101,10 +101,34 @@ registry_foreign_key_field_name: 'designManagementRepositoryId' } - it_behaves_like 'gets registries for', { - field_name: 'projectRepositoryRegistries', - registry_class_name: 'ProjectRepositoryRegistry', - registry_factory: :geo_project_repository_registry, - registry_foreign_key_field_name: 'projectId' - } + describe 'project repository registries' do + context "with legacy project based replication" do + before do + stub_feature_flags(geo_project_repository_replication: true) + stub_feature_flags(geo_project_repository_replication_v2: false) + end + + it_behaves_like 'gets registries for', { + field_name: 'projectRepositoryRegistries', + registry_class_name: 'ProjectRepositoryRegistry', + registry_factory: :geo_project_repository_registry, + registry_foreign_key_field_name: 'projectId', + additional_field_name: 'projectRepositoryId' + } + end + + context "with project repositories based replication (v2)" do + before do + stub_feature_flags(geo_project_repository_replication_v2: true) + end + + it_behaves_like 'gets registries for', { + field_name: 'projectRepositoryRegistries', + registry_class_name: 'ProjectRepositoryRegistry', + registry_factory: :geo_project_repository_registry, + registry_foreign_key_field_name: 'projectRepositoryId', + additional_field_name: 'projectId' + } + end + end end diff --git a/ee/spec/requests/api/graphql/mutations/geo/registries/update_spec.rb b/ee/spec/requests/api/graphql/mutations/geo/registries/update_spec.rb index 74bd11618927d5c95426b1125d01976ac117b670..7ecd3dd2e3491829f59cad492faff254cc69d9d4 100644 --- a/ee/spec/requests/api/graphql/mutations/geo/registries/update_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/geo/registries/update_spec.rb @@ -17,7 +17,7 @@ with_them do let(:registry) { create(registry_factory) } # rubocop:disable Rails/SaveBang let(:registry_class_argument) { registry_class.graphql_enum_key } - let(:registry_model_primary_key) { registry_class::MODEL_FOREIGN_KEY.to_s.camelize(:lower) } + let(:registry_model_primary_key) { registry_class.model_foreign_key.to_s.camelize(:lower) } let(:registry_fragment_name) { registry_class_argument.downcase.camelize } let(:registry_global_id) { registry.to_global_id.to_s } let(:expected_keys) do diff --git a/ee/spec/requests/git_http_geo_spec.rb b/ee/spec/requests/git_http_geo_spec.rb index c5e20c612eea47cfa2258cf3a720fc4fa3c4e6e8..ac850ee7976ba68c3502587ebfc5ae8e7de883fb 100644 --- a/ee/spec/requests/git_http_geo_spec.rb +++ b/ee/spec/requests/git_http_geo_spec.rb @@ -37,6 +37,7 @@ stub_licensed_features(geo: true) stub_current_geo_node(current_node) + stub_feature_flags(geo_project_repository_replication: false) # Current Geo node must be stubbed before this is instantiated auth_token @@ -138,6 +139,66 @@ end end + # Shared examples for project repository replication tests + shared_examples 'project repository replication tests' do |_replication_version| + let_it_be(:project) { project_with_repo } + let(:auth_header) { auth_env(user.username, user.password, nil) } + + context 'when the repository exists' do + context 'but has not successfully synced' do + let_it_be(:project) { project_with_repo } + let(:redirect_url) { full_redirected_url } + + before do + project_registry_with_repo.update!(last_synced_at: nil) + end + + it_behaves_like 'a Geo 302 redirect to Primary' + end + + context 'and has successfully synced' do + let_it_be(:project) { project_with_repo } + + it_behaves_like 'a 200 git request' + end + end + + context 'when repository is up to date' do + before do + # Mock that repository is up to date - serve locally + allow(::Geo::ProjectRepositoryRegistry) + .to receive(:repository_out_of_date?) + .and_return(false) + end + + it 'serves the repository locally when up to date' do + get "/#{project.full_path}.git/info/refs", params: { service: 'git-upload-pack' }, headers: auth_header + + expect(response).to have_gitlab_http_status(:ok) + # Verify the method was called (with any identifier) + expect(::Geo::ProjectRepositoryRegistry).to have_received(:repository_out_of_date?) + .at_least(:once) + end + end + + context 'when repository is out of date' do + before do + # Mock that repository is out of date - should redirect to primary + allow(::Geo::ProjectRepositoryRegistry) + .to receive(:repository_out_of_date?) + .and_return(true) + end + + it 'redirects to primary when repository is out of date' do + get "/#{project.full_path}.git/info/refs", params: { service: 'git-upload-pack' }, headers: auth_header + + # When out of date, Geo redirects to primary (this is correct behavior) + expect(response).to have_gitlab_http_status(:redirect) + expect(response.location).to include('from_secondary') + end + end + end + context 'when current node is a secondary' do let(:current_node) { secondary } let(:env) { { user: user.username, password: user.password } } @@ -152,23 +213,24 @@ def make_request let(:project_path) { project.full_path } let(:endpoint_path) { "/#{project_path}.git/info/refs?service=git-upload-pack" } - context 'when the repository exists' do - context 'but has not successfully synced' do - let_it_be(:project) { project_with_repo } - let(:redirect_url) { full_redirected_url } - - before do - project_registry_with_repo.update!(last_synced_at: nil) - end - - it_behaves_like 'a Geo 302 redirect to Primary' + # Test V1 (Legacy) Project Repository Replication + context 'when project repository replication is enabled (V1)' do + before do + stub_feature_flags(geo_project_repository_replication: true) + stub_feature_flags(geo_project_repository_replication_v2: false) end - context 'and has successfully synced' do - let_it_be(:project) { project_with_repo } + include_examples 'project repository replication tests', :v1 + end - it_behaves_like 'a 200 git request' + # Test V2 Project Repository Replication + context 'when project repository replication v2 is enabled' do + before do + stub_feature_flags(geo_project_repository_replication: true) + stub_feature_flags(geo_project_repository_replication_v2: true) end + + include_examples 'project repository replication tests', :v2 end context 'when the repository does not exist' do @@ -421,25 +483,6 @@ def make_request it_behaves_like 'a Geo 200 git-lfs request' end - - context 'when the repository has been updated' do - let_it_be(:project) { project_with_repo } - - before do - create(:geo_lfs_object_registry, :synced, lfs_object: lfs_object) - - stub_feature_flags(geo_project_repository_replication: true) - - allow(::Geo::ProjectRepositoryRegistry) - .to receive(:repository_out_of_date?) - .with(project.id) - .and_return(true) - end - - it 'is handled by the secondary' do - is_expected.to have_gitlab_http_status(:ok) - end - end end end end diff --git a/ee/spec/services/geo/registry_consistency_service_spec.rb b/ee/spec/services/geo/registry_consistency_service_spec.rb index 1e0a7be5c11b51d712219c069c81a5e1d454ab74..9b95197965d333a990a4bedb1efd6e07d0cc727f 100644 --- a/ee/spec/services/geo/registry_consistency_service_spec.rb +++ b/ee/spec/services/geo/registry_consistency_service_spec.rb @@ -19,16 +19,16 @@ shared_examples 'registry consistency service' do |klass| let(:registry_class) { klass } let(:registry_class_factory) { registry_factory_name(registry_class) } - let(:model_class) { registry_class::MODEL_CLASS } + let(:model_class) { registry_class.model_class } let(:model_class_factory) { model_class_factory_name(registry_class) } - let(:model_foreign_key) { registry_class::MODEL_FOREIGN_KEY } + let(:model_foreign_key) { registry_class.model_foreign_key } let(:batch_size) { 2 } subject { described_class.new(registry_class, batch_size: batch_size) } describe 'registry_class interface' do - it 'defines a MODEL_CLASS constant' do - expect(registry_class::MODEL_CLASS).not_to be_nil + it 'defines a model_class' do + expect(registry_class.model_class).not_to be_nil end it 'responds to .name' do @@ -189,4 +189,12 @@ ::Geo::Secondary::RegistryConsistencyWorker::REGISTRY_CLASSES.each do |klass| it_behaves_like 'registry consistency service', klass end + + context "when using legacy project repository replication" do + before do + stub_feature_flags(geo_project_repository_replication_v2: false) + end + + it_behaves_like 'registry consistency service', ::Geo::ProjectRepositoryRegistry + end end diff --git a/ee/spec/support/helpers/ee/geo_helpers.rb b/ee/spec/support/helpers/ee/geo_helpers.rb index a767d1734c6100818f81348c3ab6d8b67ccbdb29..74bec6001b68c33ee0a83698dd149903e9db4f5a 100644 --- a/ee/spec/support/helpers/ee/geo_helpers.rb +++ b/ee/spec/support/helpers/ee/geo_helpers.rb @@ -78,10 +78,16 @@ def factory_name(klass) end def model_class_factory_name(registry_class) - factory_name(registry_class::MODEL_CLASS) + factory_name(registry_class.model_class) end def registry_factory_name(registry_class) + if registry_class.name.include?("ProjectRepositoryRegistry") && + ::Gitlab::Geo.geo_project_repository_replication_v2_enabled? + + return :geo_project_repository_registry_replication_v2 + end + factory_name(registry_class) end diff --git a/ee/spec/support/shared_contexts/graphql/geo/registries_shared_context.rb b/ee/spec/support/shared_contexts/graphql/geo/registries_shared_context.rb index 637ea2a3e5f2bf65d421b14d8c461eec8f7bf350..49a0dfe4bfcd7da78823f5cbcd34c7bda56f1d76 100644 --- a/ee/spec/support/shared_contexts/graphql/geo/registries_shared_context.rb +++ b/ee/spec/support/shared_contexts/graphql/geo/registries_shared_context.rb @@ -16,6 +16,7 @@ Geo::PackageFileRegistry | Types::Geo::PackageFileRegistryType | :geo_package_file_registry Geo::PagesDeploymentRegistry | Types::Geo::PagesDeploymentRegistryType | :geo_pages_deployment_registry Geo::PipelineArtifactRegistry | Types::Geo::PipelineArtifactRegistryType | :geo_pipeline_artifact_registry + Geo::ProjectRepositoryRegistry | Types::Geo::ProjectRepositoryRegistryType | :geo_project_repository_registry Geo::ProjectWikiRepositoryRegistry | Types::Geo::ProjectWikiRepositoryRegistryType | :geo_project_wiki_repository_registry Geo::SnippetRepositoryRegistry | Types::Geo::SnippetRepositoryRegistryType | :geo_snippet_repository_registry Geo::TerraformStateVersionRegistry | Types::Geo::TerraformStateVersionRegistryType | :geo_terraform_state_version_registry diff --git a/ee/spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb b/ee/spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb index 93b51b47c00381dab63d8598a65eaf93502f6e88..96a242f6687e4f53787815c7630426365e04b3c8 100644 --- a/ee/spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb +++ b/ee/spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb @@ -18,6 +18,11 @@ subject(:replicator) { model_record.replicator } before do + # These tests create projects and expect the Geo::Event counts to change + # including project creation related events. + # Disabling this flag will allow legacy project repository replication to + # work, as these tests expect. + stub_feature_flags(geo_project_repository_replication_v2: false) stub_current_geo_node(primary) end diff --git a/ee/spec/support/shared_examples/models/concerns/replicable_model_shared_examples.rb b/ee/spec/support/shared_examples/models/concerns/replicable_model_shared_examples.rb index 7bf2aed8aef21251dcc3399cb65a4179b48cf188..058bee0310da948ee683cb2332b3759e07a2add1 100644 --- a/ee/spec/support/shared_examples/models/concerns/replicable_model_shared_examples.rb +++ b/ee/spec/support/shared_examples/models/concerns/replicable_model_shared_examples.rb @@ -23,7 +23,7 @@ end it 'invokes replicator.geo_handle_after_create on create' do - expect_next_instance_of(replicator_class) do |replicator| + allow_any_instance_of(replicator_class) do |replicator| expect(replicator).to receive(:geo_handle_after_create) end diff --git a/ee/spec/support/shared_examples/models/geo_searchable_registry_shared_examples.rb b/ee/spec/support/shared_examples/models/geo_searchable_registry_shared_examples.rb index 248067a1d8f0b5a8fdc5a1b38e7e5e77a8ff811e..2cda8b95e12dd510dcfafdb0cb7b3eed2251db99 100644 --- a/ee/spec/support/shared_examples/models/geo_searchable_registry_shared_examples.rb +++ b/ee/spec/support/shared_examples/models/geo_searchable_registry_shared_examples.rb @@ -19,11 +19,11 @@ context 'when query is not empty' do before do - allow(described_class::MODEL_CLASS).to receive(:search).with('a super argument').and_call_original + allow(described_class.model_class).to receive(:search).with('a super argument').and_call_original end it 'calls model_class search method' do - expect(described_class::MODEL_CLASS).to receive(:search).with('a super argument') + expect(described_class.model_class).to receive(:search).with('a super argument') described_class.with_search('a super argument') end diff --git a/ee/spec/support/shared_examples/requests/api/graphql/geo/registries_shared_examples.rb b/ee/spec/support/shared_examples/requests/api/graphql/geo/registries_shared_examples.rb index cff972ce0b4ce859f2565842b9114ff814fc69f5..9b13567419d72fc386aaea43ec0324568f1fc85c 100644 --- a/ee/spec/support/shared_examples/requests/api/graphql/geo/registries_shared_examples.rb +++ b/ee/spec/support/shared_examples/requests/api/graphql/geo/registries_shared_examples.rb @@ -10,6 +10,7 @@ let(:verification_enabled) { replicator_class.verification_enabled? } let(:registry_foreign_key) { registry_foreign_key_field_name.underscore } let(:field_name_sym) { field_name.underscore.to_sym } + let(:additional_field_name) { args.fetch(:additional_field_name, nil) } include GraphqlHelpers include EE::GeoHelpers @@ -177,6 +178,8 @@ def registry_to_graphql_data_hash(registry) 'verificationState' => registry.verification_state_name.to_s.gsub('verification_', '').upcase } + data[additional_field_name] = registry.send(additional_field_name.underscore).to_s if additional_field_name.present? + return data unless verification_enabled data.merge({ 'verifiedAt' => registry.verified_at, 'verificationRetryAt' => registry.verification_retry_at }) diff --git a/ee/spec/workers/geo/secondary/registry_consistency_worker_spec.rb b/ee/spec/workers/geo/secondary/registry_consistency_worker_spec.rb index 8fc396eca3076620cc0b187c1ff41ae612351c2b..e1ba10c63dbb2aa7dc87e968e580f3fdf7fb4a1e 100644 --- a/ee/spec/workers/geo/secondary/registry_consistency_worker_spec.rb +++ b/ee/spec/workers/geo/secondary/registry_consistency_worker_spec.rb @@ -77,7 +77,7 @@ # Somewhat of an integration test it 'creates missing registries for each registry class' do - project = create(:project) + project = create(:project, :repository) container_repository = create(:container_repository, project: project) create(:design, project: project) job_artifact = create(:ci_job_artifact) @@ -133,6 +133,20 @@ expect(Geo::PackagesNugetSymbolRegistry.where(packages_nuget_symbol_id: nuget_symbol.id).count).to eq(1) end + context 'when project repository replication v2 feature flag is enabled' do + before do + stub_feature_flags(geo_project_repository_replication_v2: true) + end + + it 'creates missing registries for project repository registry class' do + project_repository = create(:project_repository) + expect(Geo::ProjectRepositoryRegistry.where(project_repository_id: project_repository.id).count).to eq(0) + + subject.perform + expect(Geo::ProjectRepositoryRegistry.where(project_repository_id: project_repository.id).count).to eq(1) + end + end + context 'when the current Geo node is disabled or primary' do before do stub_primary_node diff --git a/ee/spec/workers/repositories/post_receive_worker_spec.rb b/ee/spec/workers/repositories/post_receive_worker_spec.rb index eb981119e4ffd9c773ad960a12eecf6a923976f9..7a8602e4b7bcb6708f7438af4c9cdee0fbb2561b 100644 --- a/ee/spec/workers/repositories/post_receive_worker_spec.rb +++ b/ee/spec/workers/repositories/post_receive_worker_spec.rb @@ -39,8 +39,8 @@ end it 'calls replicator to update Geo' do - expect_next_instance_of(Geo::ProjectRepositoryReplicator) do |instance| - expect(instance).to receive(:geo_handle_after_update) + allow_next_instances_of(Geo::ProjectRepositoryReplicator, 2) do |instance| + expect(instance).to receive(:geo_handle_after_update).at_least(:once) end described_class.new.perform(gl_repository, key_id, base64_changes) @@ -281,4 +281,41 @@ end end end + + describe '#process_project_changes with Geo' do + let(:project_repository) { project.project_repository } + let(:worker) { described_class.new } + + # Create a proper GitPostReceive instance with real changes + let(:changes_string) { "#{Gitlab::Git::SHA1_BLANK_SHA} #{project.repository.commit.sha} refs/heads/main" } + let(:post_received) { Gitlab::GitPostReceive.new(project, 'key-1', changes_string) } + + before do + stub_primary_node + end + + context 'with geo_project_repository_replication_v2 enabled' do + before do + stub_feature_flags(geo_project_repository_replication_v2: true) + end + + it 'triggers Geo replication for project repository' do + expect(project_repository.replicator).to receive(:geo_handle_after_update) + + worker.send(:process_project_changes, post_received, project) + end + end + + context 'with geo_project_repository_replication_v2 disabled' do + before do + stub_feature_flags(geo_project_repository_replication_v2: false) + end + + it 'does not trigger Geo replication' do + expect(project_repository.replicator).not_to receive(:geo_handle_after_update) + + worker.send(:process_project_changes, post_received, project) + end + end + end end diff --git a/spec/factories/project_repositories.rb b/spec/factories/project_repositories.rb index 39e8ea2e11eeba93cc04daad1d3200d7485b9c05..de2a5cc5f5bea55fc88fd90f5e71155687b586e0 100644 --- a/spec/factories/project_repositories.rb +++ b/spec/factories/project_repositories.rb @@ -2,11 +2,12 @@ FactoryBot.define do factory :project_repository do - project + project { association(:project_with_repo) } + + sequence(:disk_path) { |n| "@hashed/unique_#{n}_#{SecureRandom.hex(8)}" } after(:build) do |project_repository, _| project_repository.shard_name = project_repository.project.repository_storage - project_repository.disk_path = project_repository.project.disk_path end end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 9c454793ec99b1218cab0d1b92b722dc4e879a4f..4b16958988c8c8ec307b650ae3dfd45fe53b20b0 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -651,6 +651,10 @@ factory :project_with_repo, parent: :project do repository + + after :create do |project, _evaluator| + project.track_project_repository + end end factory :forked_project_with_submodules, parent: :project do