Merge branch 'ag-feature-flags-for-replicables' into 'master'

Geo: Introduce feature flags for replicables

See merge request gitlab-org/gitlab!37112

doc/administration/geo/replication/datatypes.md

@@ -126,6 +126,33 @@ these epics/issues:
 - [Unreplicated Data Types](https://gitlab.com/groups/gitlab-org/-/epics/893)
 - [Verify all replicated data](https://gitlab.com/groups/gitlab-org/-/epics/1430)
 
+### Replicated data types behind a feature flag
+
+The replication for some data types is behind a corresponding feature flag:
+
+> - They're deployed behind a feature flag, enabled by default.
+> - They're enabled on GitLab.com.
+> - They can't be enabled or disabled per-project.
+> - They are recommended for production use.
+> - For GitLab self-managed instances, GitLab administrators can opt to [disable them](#enable-or-disable-replication-for-some-data-types-core-only). **(CORE ONLY)**
+
+#### Enable or disable replication (for some data types) **(CORE ONLY)**
+
+Replication for some data types are released behind feature flags that are **enabled by default**.
+[GitLab administrators with access to the GitLab Rails console](../../feature_flags.md) can opt to disable it for your instance. You can find feature flag names of each of those data types in the notes column of the table below.
+
+To disable, such as for package file replication:
+
+```ruby
+Feature.disable(:geo_package_file_replication)
+```
+
+To enable, such as for package file replication:
+
+```ruby
+Feature.enable(:geo_package_file_replication)
+```
+
 DANGER: **Danger:**
 Features not on this list, or with **No** in the **Replicated** column,
 are not replicated on the **secondary** node. Failing over without manually
@@ -151,12 +178,12 @@ successfully, you must replicate their data using some other means.
 | [Elasticsearch integration](../../../integration/elasticsearch.md)   | [No](https://gitlab.com/gitlab-org/gitlab/-/issues/1186) | No                                                      |                                                                                                            |
 | [GitLab Pages](../../pages/index.md)                                 | [No](https://gitlab.com/groups/gitlab-org/-/epics/589)   | No                                                      |                                                                                                            |
 | [Container Registry](../../packages/container_registry.md)           | **Yes** (12.3)                                                 | No                                                      |                                                                                                            |
-| [NPM Registry](../../../user/packages/npm_registry/index.md)         | **Yes** (13.2)                                                 | No                                                      |                                                                                                            |
-| [Maven Repository](../../../user/packages/maven_repository/index.md) | **Yes** (13.2)                                                 | No                                                      |                                                                                                            |
-| [Conan Repository](../../../user/packages/conan_repository/index.md) | **Yes** (13.2)                                                 | No                                                      |                                                                                                            |
-| [NuGet Repository](../../../user/packages/nuget_repository/index.md) | **Yes** (13.2)                                                 | No                                                      |                                                                                                            |
-| [PyPi Repository](../../../user/packages/pypi_repository/index.md)   | **Yes** (13.2)                                                 | No                                                      |                                                                                                            |
-| [Composer Repository](../../../user/packages/composer_repository/index.md) | **Yes** (13.2)                                             | No                                                      |                                                                                                            |
+| [NPM Registry](../../../user/packages/npm_registry/index.md)         | **Yes** (13.2)                                                 | No                                                      | Behind feature flag `geo_package_file_replication`, enabled by default |                                                                                                            |
+| [Maven Repository](../../../user/packages/maven_repository/index.md) | **Yes** (13.2)                                                 | No                                                      |  Behind feature flag `geo_package_file_replication`, enabled by default |                                                                                                           |
+| [Conan Repository](../../../user/packages/conan_repository/index.md) | **Yes** (13.2)                                                 | No                                                      |  Behind feature flag `geo_package_file_replication`, enabled by default |                                                                                                           |
+| [NuGet Repository](../../../user/packages/nuget_repository/index.md) | **Yes** (13.2)                                                 | No                                                      |  Behind feature flag `geo_package_file_replication`, enabled by default |                                                                                                           |
+| [PyPi Repository](../../../user/packages/pypi_repository/index.md)   | **Yes** (13.2)                                                 | No                                                      |  Behind feature flag `geo_package_file_replication`, enabled by default |                                                                                                           |
+| [Composer Repository](../../../user/packages/composer_repository/index.md) | **Yes** (13.2)                                             | No                                                      |  Behind feature flag `geo_package_file_replication`, enabled by default |                                                                                                           |
 | [External merge request diffs](../../merge_request_diffs.md)         | [No](https://gitlab.com/gitlab-org/gitlab/-/issues/33817)  | No                                                      |                                           |
 | [Terraform State](../../terraform_state.md)                                                                  | [No](https://gitlab.com/groups/gitlab-org/-/epics/3112)(*3*) | No                                                        |                                                                                                            |
 | [Vulnerability Export](../../../user/application_security/security_dashboard/#export-vulnerabilities)      | [No](https://gitlab.com/groups/gitlab-org/-/epics/3111)(*3*) | No                                                        |                                                                                                            |                                                                                                            |

doc/development/geo/framework.md

@@ -90,6 +90,14 @@ module Geo
     def self.model
       ::Packages::PackageFile
     end
+
+    # Change this to `true` to release replication of this model. Then remove
+    # this override in the next release.
+    # The feature flag follows the format `geo_#{replicable_name}_replication`,
+    # so here it would be `geo_package_file_replication`
+    def self.replication_enabled_by_default?
+      false
+    end
   end
 end
 ```

ee/app/models/concerns/geo/blob_replicator_strategy.rb

@@ -14,9 +14,6 @@ module Geo
 
     def handle_after_create_commit
       publish(:created, **created_params)
-
-      return unless Feature.enabled?(:geo_self_service_framework_replication, default_enabled: true)
-
       schedule_checksum_calculation if needs_checksum?
     end
 
@@ -117,6 +114,7 @@ module Geo
     end
 
     def needs_checksum?
+      return false unless self.class.enabled?
       return true unless model_record.respond_to?(:needs_checksum?)
 
       model_record.needs_checksum?

ee/app/models/concerns/geo/replicable_registry.rb

@@ -24,6 +24,14 @@ module Geo::ReplicableRegistry
     end
   end
 
+  def registry_consistency_worker_enabled?
+    replicator_class.enabled?
+  end
+
+  def replicator_class
+    Gitlab::Geo::Replicator.for_class_name(self)
+  end
+
   included do
     include ::Delay
 

ee/changelogs/unreleased/ag-feature-flags-for-replicables.yml

+---
+title: 'Geo: Replace geo_self_service_framework_replication feature flag with replicable specific flags'
+merge_request: 37112
+author:
+type: changed

ee/lib/gitlab/geo.rb

@@ -162,7 +162,9 @@ module Gitlab
     # solutions can be found at
     # https://gitlab.com/gitlab-org/gitlab/-/issues/227693
     def self.replicator_classes
-      [::Geo::PackageFileReplicator]
+      classes = [::Geo::PackageFileReplicator]
+
+      classes.select(&:enabled?)
     end
   end
 end

ee/lib/gitlab/geo/replicator.rb

@@ -160,6 +160,18 @@ module Gitlab
         registry_class.failed.count
       end
 
+      def self.enabled?
+        Feature.enabled?(
+          replication_enabled_feature_key,
+          default_enabled: replication_enabled_by_default?)
+      end
+
+      # Replication is set behind a feature flag, which is enabled by default.
+      # If you want it disabled by default, override this method.
+      def self.replication_enabled_by_default?
+        true
+      end
+
       # @example Given `Geo::PackageFileRegistryFinder`, this returns
       #   `::Geo::PackageFileReplicator`
       # @example Given `Resolver::Geo::PackageFileRegistriesResolver`, this
@@ -201,7 +213,7 @@ module Gitlab
       # @param [Symbol] event_name
       # @param [Hash] event_data
       def publish(event_name, **event_data)
-        return unless Feature.enabled?(:geo_self_service_framework_replication, default_enabled: true)
+        return unless self.class.enabled?
 
         raise ArgumentError, "Unsupported event: '#{event_name}'" unless self.class.event_supported?(event_name)
 
@@ -295,6 +307,10 @@ module Gitlab
 
       protected
 
+      def self.replication_enabled_feature_key
+        :"geo_#{replicable_name}_replication"
+      end
+
       # Store an event on the database
       #
       # @example Create an event

ee/spec/lib/gitlab/geo/replicator_spec.rb

@@ -8,6 +8,18 @@ RSpec.describe Gitlab::Geo::Replicator do
   let_it_be(:primary_node) { create(:geo_node, :primary) }
   let_it_be(:secondary_node) { create(:geo_node) }
 
+  shared_examples 'does not replicate' do
+    it 'returns nil' do
+      expect(subject.publish(:test, other: true)).to be_nil
+    end
+
+    it 'does not call create_event' do
+      expect(subject).not_to receive(:create_event_with)
+
+      subject.publish(:test, other: true)
+    end
+  end
+
   before(:all) do
     ActiveRecord::Schema.define do
       create_table :dummy_models, force: true do |t|
@@ -94,20 +106,12 @@ RSpec.describe Gitlab::Geo::Replicator do
     describe '#publish' do
       subject { Geo::DummyReplicator.new }
 
-      context 'when geo_self_service_framework_replication feature is disabled' do
+      context 'when replication is disabled' do
         before do
-          stub_feature_flags(geo_self_service_framework_replication: false)
-        end
-
-        it 'returns nil' do
-          expect(subject.publish(:test, other: true)).to be_nil
+          stub_feature_flags(geo_dummy_replication: false)
         end
 
-        it 'does not call create_event' do
-          expect(subject).not_to receive(:create_event_with)
-
-          subject.publish(:test, other: true)
-        end
+        it_behaves_like 'does not replicate'
       end
 
       context 'when publishing a supported events with required params' do

ee/spec/lib/gitlab/geo_spec.rb

@@ -343,5 +343,15 @@ RSpec.describe Gitlab::Geo, :geo, :request_store do
       expect(result).to be_an(Array)
       expect(result).to include(Geo::PackageFileReplicator)
     end
+
+    context 'when replication is disabled' do
+      before do
+        stub_feature_flags(geo_package_file_replication: false)
+      end
+
+      it 'does not return the replicator class' do
+        expect(described_class.replicator_classes).not_to include(Geo::PackageFileReplicator)
+      end
+    end
   end
 end

ee/spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb

@@ -14,6 +14,14 @@ RSpec.shared_examples 'a blob replicator' do
 
   subject(:replicator) { model_record.replicator }
 
+  shared_examples 'does not schedule the checksum calculation' do
+    it do
+      expect(Geo::BlobVerificationPrimaryWorker).not_to receive(:perform_async)
+
+      replicator.handle_after_create_commit
+    end
+  end
+
   before do
     stub_current_geo_node(primary)
   end
@@ -37,13 +45,12 @@ RSpec.shared_examples 'a blob replicator' do
       replicator.handle_after_create_commit
     end
 
-    it 'does not schedule the checksum calculation if feature flag is disabled' do
-      stub_feature_flags(geo_self_service_framework_replication: false)
-
-      expect(Geo::BlobVerificationPrimaryWorker).not_to receive(:perform_async)
-      allow(replicator).to receive(:needs_checksum?).and_return(true)
+    context 'when replication feature flag is disabled' do
+      before do
+        stub_feature_flags("geo_#{replicator.replicable_name}_replication": false)
+      end
 
-      replicator.handle_after_create_commit
+      it_behaves_like 'does not schedule the checksum calculation'
     end
   end