Merge branch 'gitaly-feature-toggles-development-opt-out' into 'master'

Gitaly feature toggles are on by default in development environments

Closes gitaly#485

See merge request !13802

3 files changed, 49 insertions, 8 deletions

diff --git a/changelogs/unreleased/gitaly-feature-toggles-development-opt-out.yml b/changelogs/unreleased/gitaly-feature-toggles-development-opt-out.yml
new file mode 100644
index 00000000000..90c25d782c8
--- /dev/null
+++ b/changelogs/unreleased/gitaly-feature-toggles-development-opt-out.yml
@@ -0,0 +1,5 @@
+---
+title: Gitaly feature toggles are on by default in development
+merge_request: 13802
+author: 
+type: other
diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb
index 9a5f4f598b2..a3dc2cd0b60 100644
--- a/lib/gitlab/gitaly_client.rb
+++ b/lib/gitlab/gitaly_client.rb
@@ -70,21 +70,41 @@ module Gitlab
       params['gitaly_token'].presence || Gitlab.config.gitaly['token']
     end
 
-    def self.feature_enabled?(feature, status: MigrationStatus::OPT_IN)
+    # Evaluates whether a feature toggle is on or off
+    def self.feature_enabled?(feature_name, status: MigrationStatus::OPT_IN)
+      # Disabled features are always off!
       return false if status == MigrationStatus::DISABLED
 
-      feature = Feature.get("gitaly_#{feature}")
+      feature = Feature.get("gitaly_#{feature_name}")
 
-      # If the feature hasn't been set, turn it on if it's opt-out
-      return status == MigrationStatus::OPT_OUT unless Feature.persisted?(feature)
+      # If the feature has been set, always evaluate
+      if Feature.persisted?(feature)
+        if feature.percentage_of_time_value > 0
+          # Probabilistically enable this feature
+          return Random.rand() * 100 < feature.percentage_of_time_value
+        end
+
+        return feature.enabled?
+      end
 
-      if feature.percentage_of_time_value > 0
-        # Probabilistically enable this feature
-        return Random.rand() * 100 < feature.percentage_of_time_value
+      # If the feature has not been set, the default depends
+      # on it's status
+      case status
+      when MigrationStatus::OPT_OUT
+        true
+      when MigrationStatus::OPT_IN
+        opt_into_all_features?
+      else
+        false
       end
+    end
 
-      feature.enabled?
+    # opt_into_all_features? returns true when the current environment
+    # is one in which we opt into features automatically
+    def self.opt_into_all_features?
+      Rails.env.development? || ENV["GITALY_FEATURE_DEFAULT_ON"] == "1"
     end
+    private_class_method :opt_into_all_features?
 
     def self.migrate(feature, status: MigrationStatus::OPT_IN)
       is_enabled  = feature_enabled?(feature, status: status)
diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb
index 921e786a55c..a9b861fcff2 100644
--- a/spec/lib/gitlab/gitaly_client_spec.rb
+++ b/spec/lib/gitlab/gitaly_client_spec.rb
@@ -102,6 +102,22 @@ describe Gitlab::GitalyClient, skip_gitaly_mock: true do
           expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false)
         end
       end
+
+      context "when a feature is not persisted" do
+        it 'returns false when opt_into_all_features is off' do
+          allow(Feature).to receive(:persisted?).and_return(false)
+          allow(described_class).to receive(:opt_into_all_features?).and_return(false)
+
+          expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false)
+        end
+
+        it 'returns true when the override is on' do
+          allow(Feature).to receive(:persisted?).and_return(false)
+          allow(described_class).to receive(:opt_into_all_features?).and_return(true)
+
+          expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(true)
+        end
+      end
     end
 
     context 'when the feature_status is OPT_OUT' do