diff --git a/ee/app/helpers/ee/geo_helper.rb b/ee/app/helpers/ee/geo_helper.rb index 8f872882eb4e452efbdcbe675afdb9ef179538b1..c5385720b1a0c9fb1530562652a0ec6c008fe151 100644 --- a/ee/app/helpers/ee/geo_helper.rb +++ b/ee/app/helpers/ee/geo_helper.rb @@ -59,9 +59,17 @@ def replicable_types end def enabled_replicator_classes + replication_enabled_replicator_classes | verification_enabled_replicator_classes + end + + def replication_enabled_replicator_classes ::Gitlab::Geo.enabled_replicator_classes end + def verification_enabled_replicator_classes + ::Gitlab::Geo.verification_enabled_replicator_classes + end + def geo_filter_nav_options(replicable_controller, replicable_name) [ { diff --git a/ee/app/models/concerns/geo/replicable_registry.rb b/ee/app/models/concerns/geo/replicable_registry.rb index d8bff21a5b975291c48d6ed7d30419f36e06acc3..641435ba3ed7fdb6a8edc42cca90ac81baa6dfac 100644 --- a/ee/app/models/concerns/geo/replicable_registry.rb +++ b/ee/app/models/concerns/geo/replicable_registry.rb @@ -26,7 +26,7 @@ def declarative_policy_class end def registry_consistency_worker_enabled? - replicator_class.enabled? + replicator_class.replication_enabled? end # Fail syncs for records which started syncing a long time ago diff --git a/ee/app/models/concerns/geo/verifiable_replicator.rb b/ee/app/models/concerns/geo/verifiable_replicator.rb index 084eecfd227e753dd4930a2743a58567a9978e66..c3d4f16552a4a4f5e57f368c4b9da83aca41c6df 100644 --- a/ee/app/models/concerns/geo/verifiable_replicator.rb +++ b/ee/app/models/concerns/geo/verifiable_replicator.rb @@ -30,14 +30,11 @@ module VerifiableReplicator :reverify_batch, to: :verification_query_class - # If replication is disabled, then so is verification. override :verification_enabled? def verification_enabled? - enabled? && verification_feature_flag_enabled? - end + return primary_checksumming_enabled? if ::Gitlab::Geo.primary? + return secondary_verification_enabled? if ::Gitlab::Geo.secondary? - # Override this to check a feature flag - def verification_feature_flag_enabled? false end @@ -46,7 +43,6 @@ def trigger_background_verification return false unless verification_enabled? ::Geo::VerificationBatchWorker.perform_with_capacity(replicable_name) - ::Geo::VerificationTimeoutWorker.perform_async(replicable_name) # Secondaries don't need to run this since they will receive an event for each @@ -214,10 +210,36 @@ def verification_total_count registry_class.verification_not_disabled.count end end + + private + + def primary_checksumming_enabled? + # If replication is enabled on the primary, then the + # verification must be enabled. This way, we can verify + # the data on the secondary sites. + return true if replication_enabled? + + force_primary_checksumming_enabled? + end + + def secondary_verification_enabled? + # If replication is disabled on the primary, then the + # verification is also disabled on the secondary since + # it will not have the data to verify. + replication_enabled? + end + + def force_primary_checksumming_enabled? + Feature.enabled?(force_primary_checksumming_feature_key, type: :ops) # rubocop:disable Gitlab/FeatureFlagWithoutActor -- Verification is instance wide + end + + def force_primary_checksumming_feature_key + :"geo_#{replicable_name}_force_primary_checksumming" + end end - def handle_after_checksum_succeeded - return false unless Gitlab::Geo.primary? + def geo_handle_after_checksum_succeeded + return unless Gitlab::Geo.primary? return unless self.class.verification_enabled? publish(:checksum_succeeded, **event_params) diff --git a/ee/app/models/concerns/geo/verification_state.rb b/ee/app/models/concerns/geo/verification_state.rb index b221ae4a8c899f817bcf8fc6a696b2f3a901147c..be0e3085fc26f899a2fb8b8c05333b5e10f6817f 100644 --- a/ee/app/models/concerns/geo/verification_state.rb +++ b/ee/app/models/concerns/geo/verification_state.rb @@ -284,7 +284,7 @@ def verification_succeeded_with_checksum!(checksum, calculation_started_at) return unless Gitlab::Geo.primary? - self.replicator.handle_after_checksum_succeeded + self.replicator.geo_handle_after_checksum_succeeded end # Convenience method to update failure message and transition to failed diff --git a/ee/app/models/geo/base_registry.rb b/ee/app/models/geo/base_registry.rb index 9d019b080d800d46c81edd9d53463c35a43594d8..263a06ede4484ba263be81ac4f32ec287c13212f 100644 --- a/ee/app/models/geo/base_registry.rb +++ b/ee/app/models/geo/base_registry.rb @@ -24,10 +24,6 @@ def self.model_id_not_in(ids) where.not(self::MODEL_FOREIGN_KEY => ids) end - def self.replication_enabled? - true - end - def self.ordered_by_id order(:id) end diff --git a/ee/app/models/geo_node.rb b/ee/app/models/geo_node.rb index b505ecae5dac3019791f2afa7b2394ed28ab4bb1..34d9abff25b971bc4aee07fadfc6f412a8bdc1b8 100644 --- a/ee/app/models/geo_node.rb +++ b/ee/app/models/geo_node.rb @@ -221,7 +221,8 @@ def oauth_logout_url(state) def geo_replication_details_url return unless self.secondary? - replicator_class = ::Gitlab::Geo.enabled_replicator_classes.first + replicator_class = + ::Gitlab::Geo.enabled_replicator_classes.first || ::Gitlab::Geo.verification_enabled_replicator_classes.first # redirect to the replicables for the first SSF data type Gitlab::Routing.url_helpers.site_replicables_admin_geo_node_url( diff --git a/ee/app/models/geo_node_status.rb b/ee/app/models/geo_node_status.rb index f4126ec0e8a67cd62fd34caa3f5089d7e082b624..fd677cd6367660978ac455f64d92ae99f079c35e 100644 --- a/ee/app/models/geo_node_status.rb +++ b/ee/app/models/geo_node_status.rb @@ -374,7 +374,7 @@ def load_verification_data def load_primary_verification_data Gitlab::Geo::REPLICATOR_CLASSES.each do |replicator| - next unless replicator.verification_feature_flag_enabled? + next unless replicator.verification_enabled? public_send("#{replicator.replicable_name_plural}_checksummed_count=", replicator.checksummed_count) # rubocop:disable GitlabSecurity/PublicSend public_send("#{replicator.replicable_name_plural}_checksum_failed_count=", replicator.checksum_failed_count) # rubocop:disable GitlabSecurity/PublicSend diff --git a/ee/app/replicators/geo/ci_secure_file_replicator.rb b/ee/app/replicators/geo/ci_secure_file_replicator.rb index e66354664c5755393602a5019730338c4de26c07..4e0664bb5f378131875efc63869e7fdde9834eb4 100644 --- a/ee/app/replicators/geo/ci_secure_file_replicator.rb +++ b/ee/app/replicators/geo/ci_secure_file_replicator.rb @@ -11,14 +11,5 @@ def self.model def carrierwave_uploader model_record.file end - - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - # We are adding verification at the same time as replication, so we - # don't need to toggle verification separately from replication. When - # the replication feature flag is off, then verification is also off - # (see `VerifiableReplicator.verification_enabled?`) - true - end end end diff --git a/ee/app/replicators/geo/container_repository_replicator.rb b/ee/app/replicators/geo/container_repository_replicator.rb index 2ba695c1a06af6b2fa00fd62da66766da5d7eabd..b50a8f57f9b878b8adbe1db447294994a1a215c3 100644 --- a/ee/app/replicators/geo/container_repository_replicator.rb +++ b/ee/app/replicators/geo/container_repository_replicator.rb @@ -17,11 +17,6 @@ class ContainerRepositoryReplicator < Gitlab::Geo::Replicator class << self extend ::Gitlab::Utils::Override - override :verification_feature_flag_enabled? - def verification_feature_flag_enabled? - true - end - def model ::ContainerRepository end @@ -31,7 +26,8 @@ def model # it's enabled in the config file Gitlab.config.geo.registry_replication.enabled # # rubocop:disable Style/IfUnlessModifier - def enabled? + override :replication_enabled? + def replication_enabled? if ::Gitlab::Geo.secondary? return super && Geo::ContainerRepositoryRegistry.replication_enabled? end diff --git a/ee/app/replicators/geo/dependency_proxy_blob_replicator.rb b/ee/app/replicators/geo/dependency_proxy_blob_replicator.rb index 415e261af36202295483a4dfa6a79f88ae905912..3d54a8a655df3af98e8047767e98b9244c658cee 100644 --- a/ee/app/replicators/geo/dependency_proxy_blob_replicator.rb +++ b/ee/app/replicators/geo/dependency_proxy_blob_replicator.rb @@ -11,14 +11,5 @@ def self.model def carrierwave_uploader model_record.file end - - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - # We are adding verification at the same time as replication, so we - # don't need to toggle verification separately from replication. When - # the replication feature flag is off, then verification is also off - # (see `VerifiableReplicator.verification_enabled?`) - true - end end end diff --git a/ee/app/replicators/geo/dependency_proxy_manifest_replicator.rb b/ee/app/replicators/geo/dependency_proxy_manifest_replicator.rb index ae1af48c16de6f15d15d99b5e11acf74a11c2b71..165007417236089b4efa66be89df30b03e266863 100644 --- a/ee/app/replicators/geo/dependency_proxy_manifest_replicator.rb +++ b/ee/app/replicators/geo/dependency_proxy_manifest_replicator.rb @@ -11,14 +11,5 @@ def self.model def carrierwave_uploader model_record.file end - - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - # We are adding verification at the same time as replication, so we - # don't need to toggle verification separately from replication. When - # the replication feature flag is off, then verification is also off - # (see `VerifiableReplicator.verification_enabled?`) - true - end end end diff --git a/ee/app/replicators/geo/design_management_repository_replicator.rb b/ee/app/replicators/geo/design_management_repository_replicator.rb index 3bd0162bf68f090417d980d51c83f0b5e3f839f3..b75d25991b9ed5b00fa2d2e0b72ce154c1a790c7 100644 --- a/ee/app/replicators/geo/design_management_repository_replicator.rb +++ b/ee/app/replicators/geo/design_management_repository_replicator.rb @@ -16,11 +16,6 @@ def self.no_repo_message git_access_class.error_message(:no_repo) end - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - true - end - override :housekeeping_enabled? def self.housekeeping_enabled? false diff --git a/ee/app/replicators/geo/group_wiki_repository_replicator.rb b/ee/app/replicators/geo/group_wiki_repository_replicator.rb index 0799d854f8d7cae22b72a7c6f2aa8fe9ab764c00..1610d3a08ca7aa14393dedb85001efe656f727b0 100644 --- a/ee/app/replicators/geo/group_wiki_repository_replicator.rb +++ b/ee/app/replicators/geo/group_wiki_repository_replicator.rb @@ -16,11 +16,6 @@ def self.no_repo_message git_access_class.error_message(:no_group_repo) end - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - true - end - override :housekeeping_enabled? def self.housekeeping_enabled? false diff --git a/ee/app/replicators/geo/job_artifact_replicator.rb b/ee/app/replicators/geo/job_artifact_replicator.rb index 4c00494570e50678e118c812463b440cb724aaf7..9fa382ee60850f923e0dfb45b878ac475f13851a 100644 --- a/ee/app/replicators/geo/job_artifact_replicator.rb +++ b/ee/app/replicators/geo/job_artifact_replicator.rb @@ -11,14 +11,5 @@ def self.model def carrierwave_uploader model_record.file end - - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - # We are adding verification at the same time as replication, so we - # don't need to toggle verification separately from replication. When - # the replication feature flag is off, then verification is also off - # (see `VerifiableReplicator.verification_enabled?`) - true - end end end diff --git a/ee/app/replicators/geo/lfs_object_replicator.rb b/ee/app/replicators/geo/lfs_object_replicator.rb index c9426c2fed8c390ffa003dac61123d3dcee85af6..fc52e438da5aad5397242eb400a4a7c9966c5a5a 100644 --- a/ee/app/replicators/geo/lfs_object_replicator.rb +++ b/ee/app/replicators/geo/lfs_object_replicator.rb @@ -11,10 +11,5 @@ def carrierwave_uploader def self.model ::LfsObject end - - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - true - end end end diff --git a/ee/app/replicators/geo/merge_request_diff_replicator.rb b/ee/app/replicators/geo/merge_request_diff_replicator.rb index 201b64e65ecc16a39a2488b9fc02bfceea308926..48346623b2ade97b070b738161338fac405abb53 100644 --- a/ee/app/replicators/geo/merge_request_diff_replicator.rb +++ b/ee/app/replicators/geo/merge_request_diff_replicator.rb @@ -19,10 +19,5 @@ def carrierwave_uploader def checksummable? model_record.stored_externally? && super end - - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - true - end end end diff --git a/ee/app/replicators/geo/package_file_replicator.rb b/ee/app/replicators/geo/package_file_replicator.rb index 1894660e9a86df66d1b6cd62cb63959c5b0669a9..53a6c37e05062fe1f8925addc05ae307e3be30be 100644 --- a/ee/app/replicators/geo/package_file_replicator.rb +++ b/ee/app/replicators/geo/package_file_replicator.rb @@ -8,11 +8,6 @@ def self.model ::Packages::PackageFile end - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - true - end - def carrierwave_uploader model_record.file end diff --git a/ee/app/replicators/geo/pages_deployment_replicator.rb b/ee/app/replicators/geo/pages_deployment_replicator.rb index af6fd561114006eae8663b09afcc82b66fb761ec..bc72eabc08e9591c4331b0c7ece0eeeb3a369e42 100644 --- a/ee/app/replicators/geo/pages_deployment_replicator.rb +++ b/ee/app/replicators/geo/pages_deployment_replicator.rb @@ -11,10 +11,5 @@ def self.model def carrierwave_uploader model_record.file end - - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - true - end end end diff --git a/ee/app/replicators/geo/pipeline_artifact_replicator.rb b/ee/app/replicators/geo/pipeline_artifact_replicator.rb index 75eb30caa28b16e261880907f577ee97a113f174..1feb092ce6c67f50ac45d733c18e22df2c833a3d 100644 --- a/ee/app/replicators/geo/pipeline_artifact_replicator.rb +++ b/ee/app/replicators/geo/pipeline_artifact_replicator.rb @@ -11,14 +11,5 @@ def self.model def carrierwave_uploader model_record.file end - - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - # We are adding verification at the same time as replication, so we don't - # need to toggle verification separately from replication. When the - # replication feature flag is off, then verification is also off (see - # `VerifiableReplicator.verification_enabled?`) - true - end end end diff --git a/ee/app/replicators/geo/project_repository_replicator.rb b/ee/app/replicators/geo/project_repository_replicator.rb index b9d34028b8a3bbd67443c76f801bd1a2c17213e1..f60017f86e0ead79895aafc6e20ed975ebd3c7b8 100644 --- a/ee/app/replicators/geo/project_repository_replicator.rb +++ b/ee/app/replicators/geo/project_repository_replicator.rb @@ -16,15 +16,6 @@ def self.no_repo_message git_access_class.error_message(:no_repo) end - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - # We are adding verification at the same time as replication, so we - # don't need to toggle verification separately from replication. When - # the replication feature flag is off, then verification is also off - # (see `VerifiableReplicator.verification_enabled?`) - true - end - def before_housekeeping return unless ::Gitlab::Geo.secondary? diff --git a/ee/app/replicators/geo/project_wiki_repository_replicator.rb b/ee/app/replicators/geo/project_wiki_repository_replicator.rb index d6145e776f71dc7c943997b9ffe8d29d9b568a78..7ef5aa57703bbcc3f537d259c5fe244e583c130d 100644 --- a/ee/app/replicators/geo/project_wiki_repository_replicator.rb +++ b/ee/app/replicators/geo/project_wiki_repository_replicator.rb @@ -16,15 +16,6 @@ def self.no_repo_message git_access_class.error_message(:no_repo) end - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - # We are adding verification at the same time as replication, so we - # don't need to toggle verification separately from replication. When - # the replication feature flag is off, then verification is also off - # (see `VerifiableReplicator.verification_enabled?`) - true - end - override :housekeeping_model_record def housekeeping_model_record # The Repositories::HousekeepingService and Wikis::GitGarbageCollectWorker diff --git a/ee/app/replicators/geo/snippet_repository_replicator.rb b/ee/app/replicators/geo/snippet_repository_replicator.rb index 42e1d1826ca6c099e8da3cfbe852212a80760fde..1cb7d63945dd95ee5dd5cacd8fa86135d6203be6 100644 --- a/ee/app/replicators/geo/snippet_repository_replicator.rb +++ b/ee/app/replicators/geo/snippet_repository_replicator.rb @@ -16,11 +16,6 @@ def self.no_repo_message git_access_class.error_message(:no_repo) end - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - true - end - override :housekeeping_enabled? def self.housekeeping_enabled? false diff --git a/ee/app/replicators/geo/terraform_state_version_replicator.rb b/ee/app/replicators/geo/terraform_state_version_replicator.rb index 6854828e9bdfe806b2dd5c21774e19e58df51008..30f3daaf17021f6e7633bfdf5af629bfae14db25 100644 --- a/ee/app/replicators/geo/terraform_state_version_replicator.rb +++ b/ee/app/replicators/geo/terraform_state_version_replicator.rb @@ -11,10 +11,5 @@ def carrierwave_uploader def self.model ::Terraform::StateVersion end - - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - true - end end end diff --git a/ee/app/replicators/geo/upload_replicator.rb b/ee/app/replicators/geo/upload_replicator.rb index 7b228f930fdcabc2520a8fe6a10edb785cc8d8a1..94bea0ac0eb83fa6d90cb113144995fa026b2869 100644 --- a/ee/app/replicators/geo/upload_replicator.rb +++ b/ee/app/replicators/geo/upload_replicator.rb @@ -8,11 +8,6 @@ def self.model ::Upload end - override :verification_feature_flag_enabled? - def self.verification_feature_flag_enabled? - true - end - def carrierwave_uploader model_record.retrieve_uploader end diff --git a/ee/app/workers/geo/repository_registry_sync_worker.rb b/ee/app/workers/geo/repository_registry_sync_worker.rb index 7f1a0f45439cb0d540210f01adc26c74a43983c3..4453cb356fc5ddfff3adf14fdf4612be8a1cdb24 100644 --- a/ee/app/workers/geo/repository_registry_sync_worker.rb +++ b/ee/app/workers/geo/repository_registry_sync_worker.rb @@ -13,7 +13,9 @@ def max_capacity # Transition-period-solution, see # https://gitlab.com/gitlab-org/gitlab/-/issues/372444#note_1087132645c - capacity += current_node.container_repositories_max_capacity if ::Geo::ContainerRepositoryReplicator.enabled? + if ::Geo::ContainerRepositoryReplicator.replication_enabled? + capacity += current_node.container_repositories_max_capacity + end capacity end diff --git a/ee/config/feature_flags/ops/geo_ci_secure_file_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_ci_secure_file_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..2286f5a0214e69dcecc154f6b25f327a2b131163 --- /dev/null +++ b/ee/config/feature_flags/ops/geo_ci_secure_file_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_ci_secure_file_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_container_repository_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_container_repository_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..8fd5af53cc270faaa37b4223e66790a9ce8bf2ff --- /dev/null +++ b/ee/config/feature_flags/ops/geo_container_repository_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_container_repository_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_dependency_proxy_blob_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_dependency_proxy_blob_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..9659e0fbaba56ea0b08e422a7dcd0b509bcf3d92 --- /dev/null +++ b/ee/config/feature_flags/ops/geo_dependency_proxy_blob_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_dependency_proxy_blob_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_dependency_proxy_manifest_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_dependency_proxy_manifest_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..e40698bf942a427c241693bb7e33417a94907345 --- /dev/null +++ b/ee/config/feature_flags/ops/geo_dependency_proxy_manifest_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_dependency_proxy_manifest_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_design_management_repository_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_design_management_repository_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..3ba730b962329a8b32b16b2d1f1069ed89e5aab4 --- /dev/null +++ b/ee/config/feature_flags/ops/geo_design_management_repository_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_design_management_repository_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_group_wiki_repository_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_group_wiki_repository_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..644a13c82e3e33440e0da669f73cfcab74490855 --- /dev/null +++ b/ee/config/feature_flags/ops/geo_group_wiki_repository_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_group_wiki_repository_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_job_artifact_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_job_artifact_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..02252125f588358461166e32be9fe2fa02950179 --- /dev/null +++ b/ee/config/feature_flags/ops/geo_job_artifact_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_job_artifact_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_lfs_object_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_lfs_object_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..88553ac03ba32f06994e939946ca5fbd2d0ad0be --- /dev/null +++ b/ee/config/feature_flags/ops/geo_lfs_object_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_lfs_object_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_merge_request_diff_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_merge_request_diff_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..311592307bcd9c4ca54b93130304346553075b62 --- /dev/null +++ b/ee/config/feature_flags/ops/geo_merge_request_diff_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_merge_request_diff_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_package_file_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_package_file_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..de116dd630bd68a758d5cfeea03f86b076639a7f --- /dev/null +++ b/ee/config/feature_flags/ops/geo_package_file_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_package_file_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_pages_deployment_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_pages_deployment_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..c5fda6563ed95e75f37a7988fc2d52e955557517 --- /dev/null +++ b/ee/config/feature_flags/ops/geo_pages_deployment_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_pages_deployment_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_pipeline_artifact_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_pipeline_artifact_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..82a81abc63107a2ec16ac7c7648153ab2bfc545b --- /dev/null +++ b/ee/config/feature_flags/ops/geo_pipeline_artifact_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_pipeline_artifact_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_project_repository_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_project_repository_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..a7b78ca58a41d0c8a6875b01268af1bd4be409fd --- /dev/null +++ b/ee/config/feature_flags/ops/geo_project_repository_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_project_repository_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_project_wiki_repository_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_project_wiki_repository_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..02eb36b24d2f14d60b13862631304025bb3152cc --- /dev/null +++ b/ee/config/feature_flags/ops/geo_project_wiki_repository_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_project_wiki_repository_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_snippet_repository_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_snippet_repository_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..01773da6c2c2645d91a83906fe8c8333c5f3f921 --- /dev/null +++ b/ee/config/feature_flags/ops/geo_snippet_repository_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_snippet_repository_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_terraform_state_version_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_terraform_state_version_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..f726cac9975990ee213312bd5228fa5b537bdab5 --- /dev/null +++ b/ee/config/feature_flags/ops/geo_terraform_state_version_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_terraform_state_version_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/config/feature_flags/ops/geo_upload_force_primary_checksumming.yml b/ee/config/feature_flags/ops/geo_upload_force_primary_checksumming.yml new file mode 100644 index 0000000000000000000000000000000000000000..ea9ef1674354a9c9c2a424a23793b4ce1abd0664 --- /dev/null +++ b/ee/config/feature_flags/ops/geo_upload_force_primary_checksumming.yml @@ -0,0 +1,8 @@ +--- +name: geo_upload_force_primary_checksumming +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157697 +rollout_issue_url: +milestone: '17.2' +type: ops +group: group::geo +default_enabled: true diff --git a/ee/lib/gitlab/geo.rb b/ee/lib/gitlab/geo.rb index 7cf298e10155b6cbbb7081e9ee0800ba87fe4ab1..5eb95b4e9ad3cb23456b842b4724d16b1d0ac1b9 100644 --- a/ee/lib/gitlab/geo.rb +++ b/ee/lib/gitlab/geo.rb @@ -278,7 +278,7 @@ def self.interacting_with_primary_message(url) end def self.enabled_replicator_classes - REPLICATOR_CLASSES.select(&:enabled?) + REPLICATOR_CLASSES.select(&:replication_enabled?) end def self.blob_replicator_classes diff --git a/ee/lib/gitlab/geo/replicator.rb b/ee/lib/gitlab/geo/replicator.rb index a55bd98d01b612002cf8587015a519b2ce19a2ff..2de73d4ab987d398fd594f2de6c59a3fa2a7ae4a 100644 --- a/ee/lib/gitlab/geo/replicator.rb +++ b/ee/lib/gitlab/geo/replicator.rb @@ -168,14 +168,10 @@ def self.failed_count end end - def self.enabled? + def self.replication_enabled? Feature.enabled?(replication_enabled_feature_key, type: :ops) end - def self.disabled? - !enabled? - end - # @example Given `Geo::PackageFileRegistryFinder`, this returns # `::Geo::PackageFileReplicator` # @example Given `Resolver::Geo::PackageFileRegistriesResolver`, this @@ -246,6 +242,7 @@ def model_record # @param [Symbol] event_name # @param [Hash] event_data def publish(event_name, **event_data) + return unless should_publish_replication_event? raise ArgumentError, "Unsupported event: '#{event_name}'" unless self.class.event_supported?(event_name) create_event_with( @@ -300,29 +297,23 @@ def replicable_params end def geo_handle_after_create - return false unless Gitlab::Geo.primary? - return unless self.class.enabled? + return unless Gitlab::Geo.primary? publish(:created, **created_params) - after_verifiable_update if respond_to?(:after_verifiable_update) end def geo_handle_after_destroy - return false unless Gitlab::Geo.primary? - return unless self.class.enabled? + return unless Gitlab::Geo.primary? publish(:deleted, **deleted_params) end def geo_handle_after_update - return false unless Gitlab::Geo.primary? - return unless self.class.enabled? + return unless Gitlab::Geo.primary? before_verifiable_update if respond_to?(:before_verifiable_update) - publish(:updated, **updated_params) - after_verifiable_update if respond_to?(:after_verifiable_update) end @@ -357,6 +348,10 @@ def resource_exists? protected + def should_publish_replication_event? + self.class.replication_enabled? + end + # Store an event on the database # # @example Create an event diff --git a/ee/spec/lib/gitlab/geo/replicator_spec.rb b/ee/spec/lib/gitlab/geo/replicator_spec.rb index 4961c5cd9b2f4d599febf0c4baaf9c4186e27f62..0ac0f764d8865114c93ca72f7bc611f951258f94 100644 --- a/ee/spec/lib/gitlab/geo/replicator_spec.rb +++ b/ee/spec/lib/gitlab/geo/replicator_spec.rb @@ -18,6 +18,7 @@ before do stub_dummy_replicator_class + stub_dummy_replication_feature_flag end context 'event DSL' do diff --git a/ee/spec/lib/gitlab/geo_spec.rb b/ee/spec/lib/gitlab/geo_spec.rb index ba80885dcfd0d3bfb0bbe6de7724aa7c1c423895..159b87917107ea84e0dc250b9c309b3d2cb7e568 100644 --- a/ee/spec/lib/gitlab/geo_spec.rb +++ b/ee/spec/lib/gitlab/geo_spec.rb @@ -687,27 +687,124 @@ end describe '.verification_enabled_replicator_classes' do - it 'returns an Array of replicator classes' do - result = described_class.verification_enabled_replicator_classes + context 'on a Geo primary site' do + before do + stub_primary_site + end - expect(result).to be_an(Array) - expect(result).to include(Geo::PackageFileReplicator) + it 'returns an Array of replicator classes' do + result = described_class.verification_enabled_replicator_classes + + expect(result).to be_an(Array) + expect(result).to include(Geo::PackageFileReplicator) + end + + context 'when replication feature flag is enabled' do + before do + stub_feature_flags(geo_package_file_replication: true) + end + + context 'when force primary checksumming feature flag is enabled' do + it 'returns the replicator class' do + stub_feature_flags(geo_package_file_force_primary_checksumming: true) + + expect(described_class.verification_enabled_replicator_classes).to include(Geo::PackageFileReplicator) + end + end + + context 'when force primary checksumming feature flag is disabled' do + it 'returns the replicator class' do + stub_feature_flags(geo_package_file_force_primary_checksumming: false) + + expect(described_class.verification_enabled_replicator_classes).to include(Geo::PackageFileReplicator) + end + end + end + + context 'when replication feature flag is disabled' do + before do + stub_feature_flags(geo_package_file_replication: false) + end + + context 'when force primary checksumming feature flag is enabled' do + it 'returns the replicator class' do + stub_feature_flags(geo_package_file_force_primary_checksumming: true) + + expect(described_class.verification_enabled_replicator_classes).to include(Geo::PackageFileReplicator) + end + end + + context 'when force primary checksumming feature flag is disabled' do + it 'does not return the replicator class' do + stub_feature_flags(geo_package_file_force_primary_checksumming: false) + + expect(described_class.verification_enabled_replicator_classes).not_to include(Geo::PackageFileReplicator) + end + end + end end - context 'when replication is disabled' do + context 'on a Geo secondary site' do before do - stub_feature_flags(geo_package_file_replication: false) + stub_secondary_site end - it 'does not return the replicator class' do - expect(described_class.verification_enabled_replicator_classes).not_to include(Geo::PackageFileReplicator) + it 'returns an Array of replicator classes' do + result = described_class.verification_enabled_replicator_classes + + expect(result).to be_an(Array) + expect(result).to include(Geo::PackageFileReplicator) + end + + context 'when replication feature flag is enabled' do + before do + stub_feature_flags(geo_package_file_replication: true) + end + + context 'when force primary checksumming feature flag is enabled' do + it 'returns the replicator class' do + stub_feature_flags(geo_package_file_force_primary_checksumming: true) + + expect(described_class.verification_enabled_replicator_classes).to include(Geo::PackageFileReplicator) + end + end + + context 'when force primary checksumming feature flag is disabled' do + it 'returns the replicator class' do + stub_feature_flags(geo_package_file_force_primary_checksumming: true) + + expect(described_class.verification_enabled_replicator_classes).to include(Geo::PackageFileReplicator) + end + end + end + + context 'when replication feature flag is disabled' do + before do + stub_feature_flags(geo_package_file_replication: false) + end + + context 'when force primary checksumming feature flag is enabled' do + it 'does not return the replicator class' do + stub_feature_flags(geo_package_file_force_primary_checksumming: false) + + expect(described_class.verification_enabled_replicator_classes).not_to include(Geo::PackageFileReplicator) + end + end + + context 'when force primary checksumming feature flag is disabled' do + it 'does not return the replicator class' do + stub_feature_flags(geo_package_file_force_primary_checksumming: false) + + expect(described_class.verification_enabled_replicator_classes).not_to include(Geo::PackageFileReplicator) + end + end end end end describe '.verification_max_capacity_per_replicator_class' do let(:verification_max_capacity) { 12 } - let(:node) { double('node', verification_max_capacity: verification_max_capacity, secondary?: true) } + let(:node) { double('node', verification_max_capacity: verification_max_capacity, primary?: false, secondary?: true) } before do stub_current_geo_node(node) diff --git a/ee/spec/models/concerns/geo/verification_state_spec.rb b/ee/spec/models/concerns/geo/verification_state_spec.rb index 30bbbe952625098df8a2f2b83c676b8193a6f175..ce409cc79bf6f61b6595552d070b46f466ebf0ed 100644 --- a/ee/spec/models/concerns/geo/verification_state_spec.rb +++ b/ee/spec/models/concerns/geo/verification_state_spec.rb @@ -396,20 +396,20 @@ end context 'primary node' do - it 'calls replicator.handle_after_checksum_succeeded' do + it 'calls replicator.geo_handle_after_checksum_succeeded' do stub_current_geo_node(primary_node) - expect(subject.replicator).to receive(:handle_after_checksum_succeeded) + expect(subject.replicator).to receive(:geo_handle_after_checksum_succeeded) subject.verification_succeeded_with_checksum!('abc123', Time.current) end end context 'secondary node' do - it 'does not call replicator.handle_after_checksum_succeeded' do + it 'does not call replicator.geo_handle_after_checksum_succeeded' do stub_current_geo_node(secondary_node) - expect(subject.replicator).not_to receive(:handle_after_checksum_succeeded) + expect(subject.replicator).not_to receive(:geo_handle_after_checksum_succeeded) subject.verification_succeeded_with_checksum!('abc123', Time.current) end diff --git a/ee/spec/replicators/geo/container_repository_replicator_spec.rb b/ee/spec/replicators/geo/container_repository_replicator_spec.rb index 25e722ff52e5822f46e15e3e134721319707c2c2..c471f0897296d587ab93fde466b9fcb404eacd96 100644 --- a/ee/spec/replicators/geo/container_repository_replicator_spec.rb +++ b/ee/spec/replicators/geo/container_repository_replicator_spec.rb @@ -28,56 +28,92 @@ end describe '#geo_handle_after_update' do - it 'creates a Geo::Event' do - model_record.save! + context 'on a Geo primary' do + before do + stub_current_geo_node(primary) + end - expect do - replicator.geo_handle_after_update - end.to change(::Geo::Event, :count).by(1) + it 'creates a Geo::Event' do + model_record.save! + + expect do + replicator.geo_handle_after_update + end.to change(::Geo::Event, :count).by(1) + + expect(::Geo::Event.last.attributes).to include( + "replicable_name" => replicator.replicable_name, + "event_name" => ::Geo::ContainerRepositoryReplicator::EVENT_UPDATED, + "payload" => { "model_record_id" => replicator.model_record.id }) + end + + context 'when replication feature flag is disabled' do + before do + stub_feature_flags(replicator.replication_enabled_feature_key => false) + end - expect(::Geo::Event.last.attributes).to include( - "replicable_name" => replicator.replicable_name, - "event_name" => ::Geo::ContainerRepositoryReplicator::EVENT_UPDATED, - "payload" => { "model_record_id" => replicator.model_record.id }) + it 'does not publish' do + expect do + replicator.geo_handle_after_update + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } + end + end end - context 'when replication feature flag is disabled' do + context 'on a Geo secondary' do before do - stub_feature_flags(replicator.replication_enabled_feature_key => false) + stub_current_geo_node(secondary) end - it 'does not publish' do - expect(replicator).not_to receive(:publish) - - replicator.geo_handle_after_update + it 'does not create an event' do + expect do + replicator.geo_handle_after_update + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } end end end describe '#geo_handle_after_destroy' do - it 'creates a Geo::Event' do - expect do - replicator.geo_handle_after_destroy - end.to change(::Geo::Event, :count).by(1) - - expect(::Geo::Event.last.attributes).to include( - "replicable_name" => replicator.replicable_name, - "event_name" => ::Geo::ContainerRepositoryReplicator::EVENT_DELETED, - "payload" => { - "model_record_id" => replicator.model_record.id, - "path" => replicator.model_record.path - }) + context 'on a Geo primary' do + before do + stub_current_geo_node(primary) + end + + it 'creates a Geo::Event' do + expect do + replicator.geo_handle_after_destroy + end.to change(::Geo::Event, :count).by(1) + + expect(::Geo::Event.last.attributes).to include( + "replicable_name" => replicator.replicable_name, + "event_name" => ::Geo::ContainerRepositoryReplicator::EVENT_DELETED, + "payload" => { + "model_record_id" => replicator.model_record.id, + "path" => replicator.model_record.path + }) + end + + context 'when replication feature flag is disabled' do + before do + stub_feature_flags(replicator.replication_enabled_feature_key => false) + end + + it 'does not publish' do + expect do + replicator.geo_handle_after_destroy + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } + end + end end - context 'when replication feature flag is disabled' do + context 'on a Geo secondary' do before do - stub_feature_flags(replicator.replication_enabled_feature_key => false) + stub_current_geo_node(secondary) end - it 'does not publish' do - expect(replicator).not_to receive(:publish) - - replicator.geo_handle_after_destroy + it 'does not create an event' do + expect do + replicator.geo_handle_after_destroy + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } end end end @@ -166,6 +202,8 @@ subject(:replicator) { model_record.replicator } before do + allow(Geo::ContainerRepositoryRegistry).to receive(:replication_enabled?).and_return(true) + stub_container_registry_config(enabled: true, api_url: api_url) stub_request(:get, "#{repository_url}/tags/list?n=#{::ContainerRegistry::Client::DEFAULT_TAGS_PAGE_SIZE}") diff --git a/ee/spec/support/helpers/ee/geo_helpers.rb b/ee/spec/support/helpers/ee/geo_helpers.rb index f2d98437d2963da903d44b970b1978acd1960b68..c00528474870131fdde545789395f24743dcaea3 100644 --- a/ee/spec/support/helpers/ee/geo_helpers.rb +++ b/ee/spec/support/helpers/ee/geo_helpers.rb @@ -15,15 +15,17 @@ def stub_current_node_name(name) allow(GeoNode).to receive(:current_node_name).and_return(name) end - def stub_primary_node + def stub_primary_site allow(::Gitlab::Geo).to receive(:primary?).and_return(true) allow(::Gitlab::Geo).to receive(:secondary?).and_return(false) end + alias_method :stub_primary_node, :stub_primary_site - def stub_secondary_node + def stub_secondary_site allow(::Gitlab::Geo).to receive(:primary?).and_return(false) allow(::Gitlab::Geo).to receive(:secondary?).and_return(true) end + alias_method :stub_secondary_node, :stub_secondary_site def stub_proxied_request allow(::Gitlab::Geo).to receive(:proxied_request?).and_return(true) @@ -95,11 +97,7 @@ def self.model model_class.constantize end - def handle_after_create_commit - true - end - - def handle_after_checksum_succeeded + def geo_handle_after_checksum_succeeded true end @@ -137,7 +135,7 @@ def self.replicables_for_current_secondary(primary_key_in) def stub_dummy_replication_feature_flag(replicator_class: 'Geo::DummyReplicator', enabled: true) replicator = Object.const_get(replicator_class, false) - allow(replicator).to receive(:enabled?).and_return(enabled) + allow(replicator).to receive(:replication_enabled?).and_return(enabled) end # Example: diff --git a/ee/spec/support/shared_examples/graphql/geo/geo_registries_resolver_shared_examples.rb b/ee/spec/support/shared_examples/graphql/geo/geo_registries_resolver_shared_examples.rb index d6b9326bf552c3d7c20ea6857c474ce2f7612bb1..88726d80fc94eca8eef9c6c9addfc0fb712f3da7 100644 --- a/ee/spec/support/shared_examples/graphql/geo/geo_registries_resolver_shared_examples.rb +++ b/ee/spec/support/shared_examples/graphql/geo/geo_registries_resolver_shared_examples.rb @@ -4,9 +4,13 @@ include GraphqlHelpers include EE::GeoHelpers - describe '#resolve' do - let_it_be(:secondary) { create(:geo_node) } + let_it_be(:secondary) { create(:geo_node) } + + before do + stub_current_geo_node(secondary) + end + describe '#resolve' do # rubocop:disable Rails/SaveBang let(:replicator_class) { Gitlab::Geo::Replicator.for_class_name(described_class.name) } let(:factory_traits) { replicator_class.verification_enabled? ? [:synced, :verification_succeeded] : [:synced] } @@ -20,10 +24,6 @@ let(:gql_context) { { current_user: current_user } } context 'when the parent object is the current node' do - before do - stub_current_geo_node(secondary) - end - context 'when the user has permission to view Geo data' do let_it_be(:current_user) { create(:admin) } 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 fd77b4029014480e0a4b847b2cf312896b826acb..71f2fc7519d23fb22b845b892319a1e05fcaf5c2 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 @@ -75,75 +75,105 @@ end describe '#geo_handle_after_create' do - it 'creates a Geo::Event' do - model_record.save! - - expect do - replicator.geo_handle_after_create - end.to change { ::Geo::Event.count }.by(1) - - expect(::Geo::Event.last.attributes).to include( - "replicable_name" => replicator.replicable_name, - "event_name" => ::Geo::BlobReplicatorStrategy::EVENT_CREATED, - "payload" => { - "model_record_id" => replicator.model_record.id - } - ) - end + context 'on a Geo primary' do + before do + stub_current_geo_node(primary) + end - it 'calls #after_verifiable_update' do - expect(replicator).to receive(:after_verifiable_update) + it 'creates a Geo::Event' do + model_record.save! - replicator.geo_handle_after_create - end + expect do + replicator.geo_handle_after_create + end.to change { ::Geo::Event.count }.by(1) - context 'when replication feature flag is disabled' do - before do - stub_feature_flags(replicator.replication_enabled_feature_key => false) + expect(::Geo::Event.last.attributes).to include( + "replicable_name" => replicator.replicable_name, + "event_name" => ::Geo::BlobReplicatorStrategy::EVENT_CREATED, + "payload" => { + "model_record_id" => replicator.model_record.id + } + ) end - it 'does not call #after_verifiable_update' do - expect(replicator).not_to receive(:after_verifiable_update) + it 'calls #after_verifiable_update' do + expect(replicator).to receive(:after_verifiable_update) replicator.geo_handle_after_create end - it 'does not publish' do - expect(replicator).not_to receive(:publish) + context 'when replication feature flag is disabled' do + before do + stub_feature_flags(replicator.replication_enabled_feature_key => false) + end - replicator.geo_handle_after_create + it 'does not publish' do + expect do + replicator.geo_handle_after_create + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } + end + end + end + + context 'on a Geo secondary' do + before do + stub_current_geo_node(secondary) + end + + it 'does not create an event' do + expect do + replicator.geo_handle_after_create + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } end end end describe '#geo_handle_after_destroy' do - it 'creates a Geo::Event' do - model_record + context 'on a Geo primary' do + before do + stub_current_geo_node(primary) + end - expect do - replicator.geo_handle_after_destroy - end.to change { ::Geo::Event.count }.by(1) - - expect(::Geo::Event.last.attributes).to include( - "replicable_name" => replicator.replicable_name, - "event_name" => ::Geo::BlobReplicatorStrategy::EVENT_DELETED, - "payload" => { - "model_record_id" => replicator.model_record.id, - "uploader_class" => replicator.carrierwave_uploader.class.to_s, - "blob_path" => replicator.carrierwave_uploader.relative_path.to_s - } - ) + it 'creates a Geo::Event' do + model_record + + expect do + replicator.geo_handle_after_destroy + end.to change { ::Geo::Event.count }.by(1) + + expect(::Geo::Event.last.attributes).to include( + "replicable_name" => replicator.replicable_name, + "event_name" => ::Geo::BlobReplicatorStrategy::EVENT_DELETED, + "payload" => { + "model_record_id" => replicator.model_record.id, + "uploader_class" => replicator.carrierwave_uploader.class.to_s, + "blob_path" => replicator.carrierwave_uploader.relative_path.to_s + } + ) + end + + context 'when replication feature flag is disabled' do + before do + stub_feature_flags(replicator.replication_enabled_feature_key => false) + end + + it 'does not publish' do + expect do + replicator.geo_handle_after_destroy + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } + end + end end - context 'when replication feature flag is disabled' do + context 'on a Geo secondary' do before do - stub_feature_flags(replicator.replication_enabled_feature_key => false) + stub_current_geo_node(secondary) end - it 'does not publish' do - expect(replicator).not_to receive(:publish) - - replicator.geo_handle_after_destroy + it 'does not create an event' do + expect do + replicator.geo_handle_after_destroy + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } end end end diff --git a/ee/spec/support/shared_examples/models/concerns/repository_replicator_strategy_shared_examples.rb b/ee/spec/support/shared_examples/models/concerns/repository_replicator_strategy_shared_examples.rb index e6cd273385f7fad54fd316160f9a63a5c967b05e..69266242d2b9fd826a6cd127ad49e5c171dd0d54 100644 --- a/ee/spec/support/shared_examples/models/concerns/repository_replicator_strategy_shared_examples.rb +++ b/ee/spec/support/shared_examples/models/concerns/repository_replicator_strategy_shared_examples.rb @@ -35,68 +35,74 @@ end describe '#geo_handle_after_update' do - it 'creates a Geo::Event' do - model_record - - expect do - replicator.geo_handle_after_update - end.to change { ::Geo::Event.count }.by(1) - - expect(::Geo::Event.last.attributes).to include( - "replicable_name" => replicator.replicable_name, "event_name" => ::Geo::RepositoryReplicatorStrategy::EVENT_UPDATED, "payload" => { "model_record_id" => replicator.model_record.id }) - end + context 'on a Geo primary' do + before do + stub_current_geo_node(primary) + end - it 'calls #before_verifiable_update' do - expect(replicator).to receive(:before_verifiable_update) + it 'creates a Geo::Event' do + model_record - replicator.geo_handle_after_update - end + expect do + replicator.geo_handle_after_update + end.to change { ::Geo::Event.count }.by(1) - context 'when replication feature flag is disabled' do - before do - stub_feature_flags(replicator.replication_enabled_feature_key => false) + expect(::Geo::Event.last.attributes).to include( + "replicable_name" => replicator.replicable_name, "event_name" => ::Geo::RepositoryReplicatorStrategy::EVENT_UPDATED, "payload" => { "model_record_id" => replicator.model_record.id }) end - it 'does not call #before_verifiable_update' do - expect(replicator).not_to receive(:before_verifiable_update) + it 'calls #before_verifiable_update' do + expect(replicator).to receive(:before_verifiable_update) replicator.geo_handle_after_update end - it 'does not publish' do - expect(replicator).not_to receive(:publish) + context 'when replication feature flag is disabled' do + before do + stub_feature_flags(replicator.replication_enabled_feature_key => false) + end - replicator.geo_handle_after_update + it 'does not publish' do + expect do + replicator.geo_handle_after_update + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } + end end end - end - describe '#geo_handle_after_create' do - it 'creates a Geo::Event' do - model_record.save! + context 'on a Geo secondary' do + before do + stub_current_geo_node(secondary) + end - expect do - replicator.geo_handle_after_create - end.to change { ::Geo::Event.count }.by(1) - - expect(::Geo::Event.last.attributes).to include( - "replicable_name" => replicator.replicable_name, - "event_name" => ::Geo::RepositoryReplicatorStrategy::EVENT_CREATED, - "payload" => { - "model_record_id" => replicator.model_record.id - } - ) + it 'does not create an event' do + expect do + replicator.geo_handle_after_update + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } + end end + end - it 'does not call #before_verifiable_update' do - expect(replicator).not_to receive(:before_verifiable_update) + describe '#geo_handle_after_create' do + context 'on a Geo primary' do + before do + stub_current_geo_node(primary) + end - replicator.geo_handle_after_create - end + it 'creates a Geo::Event' do + model_record.save! - context 'when replication feature flag is disabled' do - before do - stub_feature_flags(replicator.replication_enabled_feature_key => false) + expect do + replicator.geo_handle_after_create + end.to change { ::Geo::Event.count }.by(1) + + expect(::Geo::Event.last.attributes).to include( + "replicable_name" => replicator.replicable_name, + "event_name" => ::Geo::RepositoryReplicatorStrategy::EVENT_CREATED, + "payload" => { + "model_record_id" => replicator.model_record.id + } + ) end it 'does not call #before_verifiable_update' do @@ -105,36 +111,72 @@ replicator.geo_handle_after_create end - it 'does not publish' do - expect(replicator).not_to receive(:publish) + context 'when replication feature flag is disabled' do + before do + stub_feature_flags(replicator.replication_enabled_feature_key => false) + end - replicator.geo_handle_after_create + it 'does not publish' do + expect do + replicator.geo_handle_after_create + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } + end + end + end + + context 'on a Geo secondary' do + before do + stub_current_geo_node(secondary) + end + + it 'does not create an event' do + expect do + replicator.geo_handle_after_create + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } end end end describe '#geo_handle_after_destroy' do - it 'creates a Geo::Event' do - model_record + context 'on a Geo primary' do + before do + stub_current_geo_node(primary) + end - expect do - replicator.geo_handle_after_destroy - end.to change { ::Geo::Event.count }.by(1) + it 'creates a Geo::Event' do + model_record + + expect do + replicator.geo_handle_after_destroy + end.to change { ::Geo::Event.count }.by(1) + + expect(::Geo::Event.last.attributes).to include( + "replicable_name" => replicator.replicable_name, "event_name" => ::Geo::RepositoryReplicatorStrategy::EVENT_DELETED) + expect(::Geo::Event.last.payload).to include({ "model_record_id" => replicator.model_record.id }) + end - expect(::Geo::Event.last.attributes).to include( - "replicable_name" => replicator.replicable_name, "event_name" => ::Geo::RepositoryReplicatorStrategy::EVENT_DELETED) - expect(::Geo::Event.last.payload).to include({ "model_record_id" => replicator.model_record.id }) + context 'when replication feature flag is disabled' do + before do + stub_feature_flags("geo_#{replicator.replicable_name}_replication": false) + end + + it 'does not publish' do + expect do + replicator.geo_handle_after_destroy + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } + end + end end - context 'when replication feature flag is disabled' do + context 'on a Geo secondary' do before do - stub_feature_flags("geo_#{replicator.replicable_name}_replication": false) + stub_current_geo_node(secondary) end - it 'does not publish' do - expect(replicator).not_to receive(:publish) - - replicator.geo_handle_after_destroy + it 'does not create an event' do + expect do + replicator.geo_handle_after_destroy + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } end end end diff --git a/ee/spec/support/shared_examples/models/concerns/verifiable_replicator_shared_examples.rb b/ee/spec/support/shared_examples/models/concerns/verifiable_replicator_shared_examples.rb index 1dfdeecb2052a4bebb8f19d5b06894c677f5eec0..44c454b25b87f57b5749b17394ead190aad8ad64 100644 --- a/ee/spec/support/shared_examples/models/concerns/verifiable_replicator_shared_examples.rb +++ b/ee/spec/support/shared_examples/models/concerns/verifiable_replicator_shared_examples.rb @@ -71,35 +71,105 @@ end describe '.verification_enabled?' do - context 'when replication is enabled' do + let(:replicable_name) { described_class.replicable_name } + + context 'on a Geo primary site' do before do - expect(described_class).to receive(:enabled?).and_return(true) + stub_primary_site end - context 'when verification_feature_flag_enabled? returns true' do - it 'returns true' do - allow(described_class).to receive(:verification_feature_flag_enabled?).and_return(true) + context 'when replication feature flag is enabled' do + before do + stub_feature_flags("geo_#{replicable_name}_replication" => true) + end + + context 'when force primary checksumming feature flag is enabled' do + it 'returns true' do + stub_feature_flags("geo_#{replicable_name}_force_primary_checksumming" => true) + + expect(described_class.verification_enabled?).to be_truthy + end + end + + context 'when the force primary checksumming feature flag is disabled' do + it 'returns true' do + stub_feature_flags("geo_#{replicable_name}_force_primary_checksumming" => false) - expect(described_class.verification_enabled?).to be_truthy + expect(described_class.verification_enabled?).to be_truthy + end end end - context 'when verification_feature_flag_enabled? returns false' do - it 'returns false' do - allow(described_class).to receive(:verification_feature_flag_enabled?).and_return(false) + context 'when replication feature flag is disabled' do + before do + stub_feature_flags("geo_#{replicable_name}_replication" => false) + end + + context 'when force primary checksumming feature flag is enabled' do + it 'returns true' do + stub_feature_flags("geo_#{replicable_name}_force_primary_checksumming" => true) - expect(described_class.verification_enabled?).to be_falsey + expect(described_class.verification_enabled?).to be_truthy + end + end + + context 'when the force primary checksumming feature flag is disabled' do + it 'returns false' do + stub_feature_flags("geo_#{replicable_name}_force_primary_checksumming" => false) + + expect(described_class.verification_enabled?).to be_falsey + end end end end - context 'when replication is disabled' do + context 'on a Geo secondary site' do before do - expect(described_class).to receive(:enabled?).and_return(false) + stub_secondary_site end - it 'returns false' do - expect(described_class.verification_enabled?).to be_falsey + context 'when replication feature flag is enabled' do + before do + stub_feature_flags("geo_#{replicable_name}_replication" => true) + end + + context 'when force primary checksumming feature flag is enabled' do + it 'returns true' do + stub_feature_flags("geo_#{replicable_name}_force_primary_checksumming" => true) + + expect(described_class.verification_enabled?).to be_truthy + end + end + + context 'when the force primary checksumming feature flag is disabled' do + it 'returns true' do + stub_feature_flags("geo_#{replicable_name}_force_primary_checksumming" => false) + + expect(described_class.verification_enabled?).to be_truthy + end + end + end + + context 'when replication feature flag is disabled' do + before do + stub_feature_flags("geo_#{replicable_name}_replication" => false) + end + + context 'when force primary checksumming feature flag is enabled' do + it 'returns false' do + stub_feature_flags("geo_#{replicable_name}_force_primary_checksumming" => true) + + expect(described_class.verification_enabled?).to be_falsey + end + end + + context 'when the force primary checksumming feature flag is disabled' do + it 'returns false' do + stub_feature_flags("geo_#{replicable_name}_force_primary_checksumming" => false) + + expect(described_class.verification_enabled?).to be_falsey + end + end end end end @@ -577,7 +647,7 @@ end end - describe '#handle_after_checksum_succeeded' do + describe '#geo_handle_after_checksum_succeeded' do context 'on a Geo primary' do before do stub_primary_node @@ -586,7 +656,7 @@ it 'creates checksum_succeeded event' do model_record - expect { replicator.handle_after_checksum_succeeded }.to change { ::Geo::Event.count }.by(1) + expect { replicator.geo_handle_after_checksum_succeeded }.to change { ::Geo::Event.count }.by(1) expect(::Geo::Event.last.event_name).to eq 'checksum_succeeded' end @@ -596,6 +666,18 @@ expect { model_record.verification_succeeded_with_checksum!('abc123', Time.current) }.to change { ::Geo::Event.count }.by(1) expect(::Geo::Event.last.event_name).to eq 'checksum_succeeded' end + + context 'when replication feature flag is disabled' do + before do + stub_feature_flags("geo_#{replicator.replicable_name}_replication": false) + end + + it 'does not publish' do + expect do + replicator.geo_handle_after_checksum_succeeded + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } + end + end end context 'on a Geo secondary' do @@ -604,7 +686,9 @@ end it 'does not create an event' do - expect { replicator.handle_after_checksum_succeeded }.not_to change { ::Geo::Event.count } + expect do + replicator.geo_handle_after_checksum_succeeded + end.not_to change { ::Geo::Event.where("replicable_name" => replicator.replicable_name).count } end end end diff --git a/ee/spec/workers/geo/repository_registry_sync_worker_spec.rb b/ee/spec/workers/geo/repository_registry_sync_worker_spec.rb index 7c8e2af6fb1fd7dc651ce37e728bc2f45766fdb9..54e06318d4647ec86bffd715b1dd633f7c292cc2 100644 --- a/ee/spec/workers/geo/repository_registry_sync_worker_spec.rb +++ b/ee/spec/workers/geo/repository_registry_sync_worker_spec.rb @@ -37,6 +37,10 @@ end context 'when container repository replication is disabled' do + before do + stub_registry_replication_config(enabled: false) + end + it 'returns 1' do expect(described_class.new.send(:max_capacity)).to eq(1) end @@ -59,6 +63,10 @@ end context 'when container repository replication is disabled' do + before do + stub_registry_replication_config(enabled: false) + end + it 'returns only 1/10 of repos_max_capacity based capacity' do expect(described_class.new.send(:max_capacity)).to eq(2) end diff --git a/scripts/feature_flags/used-feature-flags b/scripts/feature_flags/used-feature-flags index b3005c3cba5c50fc96b1249da0512946fcb60723..8d9f82304acc025fdb027642b05cf8f09d6499aa 100755 --- a/scripts/feature_flags/used-feature-flags +++ b/scripts/feature_flags/used-feature-flags @@ -37,11 +37,15 @@ def mark_replicator_flags_as_used(edition) # Geo feature flags are constructed dynamically and there's no explicit checks in the codebase so we mark all # the replicators' derived feature flags as used. # See https://gitlab.com/gitlab-org/gitlab/-/blob/54e802e8fe76b6f93656d75ef9b566bf57b60f41/ee/lib/gitlab/geo/replicator.rb#L183-185 + geo_feature_flag_suffixes = %w[replication force_primary_checksumming] + Dir.glob("#{edition}/app/replicators/geo/*_replicator.rb").each do |path| replicator_name = File.basename(path, '.rb') - feature_flag_name = "geo_#{replicator_name.delete_suffix('_replicator')}_replication" - FileUtils.touch(File.join('tmp', 'feature_flags', "#{feature_flag_name}.used")) + geo_feature_flag_suffixes.each do |suffix| + feature_flag_name = "geo_#{replicator_name.delete_suffix('_replicator')}_#{suffix}" + FileUtils.touch(File.join('tmp', 'feature_flags', "#{feature_flag_name}.used")) + end end end