Added support for feature flags in opensearch.yml

This change introduces a static store "settings" in FeatureFlags.java file to enable isEnabled method to fetch flag settings defined in opensearch.yml. Signed-off-by: Nagaraj Tantri <nagarajtantri@gmail.com>
opensearch-project · Dec 8, 2022 · 5ee23a4 · 5ee23a4
1 parent 110e409
commit 5ee23a4
Show file tree

Hide file tree

Showing 8 changed files with 141 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - [Test] Add IAE test for deprecated edgeNGram analyzer name ([#5040](https://github.com/opensearch-project/OpenSearch/pull/5040))
 - Allow mmap to use new JDK-19 preview APIs in Apache Lucene 9.4+ ([#5151](https://github.com/opensearch-project/OpenSearch/pull/5151))
 - Add feature flag for extensions ([#5211](https://github.com/opensearch-project/OpenSearch/pull/5211))
+- Added support for feature flags in opensearch.yml ([#4959](https://github.com/opensearch-project/OpenSearch/pull/4959))
 
 ### Dependencies
 - Bumps `log4j-core` from 2.18.0 to 2.19.0

diff --git a/distribution/src/config/opensearch.yml b/distribution/src/config/opensearch.yml
@@ -86,3 +86,26 @@ ${path.logs}
 # Require explicit names when deleting indices:
 #
 #action.destructive_requires_name: true
+#
+# ---------------------------------- Experimental Features -----------------------------------
+#
+# Gates the visibility of the index setting that allows changing of replication type.
+# Once the feature is ready for production release, this feature flag can be removed.
+#
+#opensearch.experimental.feature.replication_type.enabled: false
+#
+#
+# Gates the visibility of the index setting that allows persisting data to remote store along with local disk.
+# Once the feature is ready for production release, this feature flag can be removed.
+#
+#opensearch.experimental.feature.remote_store.enabled: false
+#
+#
+# Gates the functionality of a new parameter to the snapshot restore API
+# that allows for creation of a new index type that searches a snapshot
+# directly in a remote repository without restoring all index data to disk
+# ahead of time.
+#
+#opensearch.experimental.feature.searchable_snapshot.enabled: false
+#
+#
diff --git a/...src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationIT.java b/...src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationIT.java
@@ -65,11 +65,6 @@ public class SegmentReplicationIT extends OpenSearchIntegTestCase {
     private static final int SHARD_COUNT = 1;
     private static final int REPLICA_COUNT = 1;
 
-    @BeforeClass
-    public static void assumeFeatureFlag() {
-        assumeTrue("Segment replication Feature flag is enabled", Boolean.parseBoolean(System.getProperty(FeatureFlags.REPLICATION_TYPE)));
-    }
-
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
         return Arrays.asList(MockTransportService.TestPlugin.class);
@@ -91,11 +86,19 @@ protected boolean addMockInternalEngine() {
         return false;
     }
 
+    @Override
+    protected Settings featureFlagSettings() {
+        return Settings.builder()
+            .put(super.featureFlagSettings())
+            .put(FeatureFlags.REPLICATION_TYPE, "true")
+            .build();
+    }
+
     public void testPrimaryStopped_ReplicaPromoted() throws Exception {
-        final String primary = internalCluster().startNode();
+        final String primary = internalCluster().startNode(featureFlagSettings());
         createIndex(INDEX_NAME);
         ensureYellowAndNoInitializingShards(INDEX_NAME);
-        final String replica = internalCluster().startNode();
+        final String replica = internalCluster().startNode(featureFlagSettings());
         ensureGreen(INDEX_NAME);
 
         client().prepareIndex(INDEX_NAME).setId("1").setSource("foo", "bar").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).get();
@@ -122,7 +125,7 @@ public void testPrimaryStopped_ReplicaPromoted() throws Exception {
         assertHitCount(client(replica).prepareSearch(INDEX_NAME).setSize(0).setPreference("_only_local").get(), 3);
 
         // start another node, index another doc and replicate.
-        String nodeC = internalCluster().startNode();
+        String nodeC = internalCluster().startNode(featureFlagSettings());
         ensureGreen(INDEX_NAME);
         client().prepareIndex(INDEX_NAME).setId("4").setSource("baz", "baz").get();
         refresh(INDEX_NAME);
@@ -162,10 +165,10 @@ public void testRestartPrimary() throws Exception {
 
     public void testCancelPrimaryAllocation() throws Exception {
         // this test cancels allocation on the primary - promoting the new replica and recreating the former primary as a replica.
-        final String primary = internalCluster().startNode();
+        final String primary = internalCluster().startNode(featureFlagSettings());
         createIndex(INDEX_NAME);
         ensureYellowAndNoInitializingShards(INDEX_NAME);
-        final String replica = internalCluster().startNode();
+        final String replica = internalCluster().startNode(featureFlagSettings());
         ensureGreen(INDEX_NAME);
 
         final int initialDocCount = 1;

diff --git a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java
@@ -0,0 +1,34 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.common.settings;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import org.opensearch.common.settings.Setting.Property;
+import org.opensearch.common.util.FeatureFlags;
+
+public class FeatureFlagSettings extends AbstractScopedSettings {
+
+  protected FeatureFlagSettings(Settings settings, Set<Setting<?>> settingsSet,
+      Set<SettingUpgrader<?>> settingUpgraders, Property scope) {
+    super(settings, settingsSet, settingUpgraders, scope);
+  }
+
+  public static final Set<Setting<?>> BUILT_IN_FEATURE_FLAGS = Collections.unmodifiableSet(
+      new HashSet<>(
+          Arrays.asList(
+              FeatureFlags.REPLICATION_TYPE_SETTING,
+              FeatureFlags.REMOTE_STORE_SETTING,
+              FeatureFlags.SEARCHABLE_SNAPSHOT_SETTING,
+              FeatureFlags.EXTENSIONS_SETTING
+          )
+      ));
+}
diff --git a/server/src/main/java/org/opensearch/common/settings/SettingsModule.java b/server/src/main/java/org/opensearch/common/settings/SettingsModule.java
@@ -87,6 +87,9 @@ public SettingsModule(
         for (Setting<?> setting : IndexScopedSettings.BUILT_IN_INDEX_SETTINGS) {
             registerSetting(setting);
         }
+        for (Setting<?> setting : FeatureFlagSettings.BUILT_IN_FEATURE_FLAGS) {
+            registerSetting(setting);
+        }
 
         for (Map.Entry<String, List<Setting>> featureFlaggedSetting : IndexScopedSettings.FEATURE_FLAGGED_INDEX_SETTINGS.entrySet()) {
             if (FeatureFlags.isEnabled(featureFlaggedSetting.getKey())) {

diff --git a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java
@@ -8,6 +8,10 @@
 
 package org.opensearch.common.util;
 
+import org.opensearch.common.settings.Setting;
+import org.opensearch.common.settings.Setting.Property;
+import org.opensearch.common.settings.Settings;
+
 /**
  * Utility class to manage feature flags. Feature flags are system properties that must be set on the JVM.
  * These are used to gate the visibility/availability of incomplete features. Fore more information, see
@@ -43,12 +47,55 @@ public class FeatureFlags {
      */
     public static final String EXTENSIONS = "opensearch.experimental.feature.extensions.enabled";
 
+    /**
+     * Should store the settings from opensearch.yml.
+     */
+    private static Settings settings;
+
+    /**
+     * This method is responsible to map settings from opensearch.yml to local stored
+     * settings value. That is used for the existing isEnabled method.
+     *
+     * @param openSearchSettings The settings stored in opensearch.yml.
+     */
+    public static void initialiseFeatureFlags(Settings openSearchSettings) {
+        settings = openSearchSettings;
+    }
+
     /**
      * Used to test feature flags whose values are expected to be booleans.
      * This method returns true if the value is "true" (case-insensitive),
      * and false otherwise.
      */
     public static boolean isEnabled(String featureFlagName) {
-        return "true".equalsIgnoreCase(System.getProperty(featureFlagName));
+        if ("true".equalsIgnoreCase(System.getProperty(featureFlagName))) {
+            // TODO: Remove the if condition once FeatureFlags are only supported via opensearch.yml
+            return true;
+        }
+        return settings != null && settings.getAsBoolean(featureFlagName, false);
     }
+
+    public static final Setting<Boolean> REPLICATION_TYPE_SETTING = Setting.boolSetting(
+        REPLICATION_TYPE,
+        false,
+        Property.NodeScope
+    );
+
+    public static final Setting<Boolean> REMOTE_STORE_SETTING = Setting.boolSetting(
+        REMOTE_STORE,
+        false,
+        Property.NodeScope
+    );
+
+    public static final Setting<Boolean> SEARCHABLE_SNAPSHOT_SETTING = Setting.boolSetting(
+        SEARCHABLE_SNAPSHOT,
+        false,
+        Property.NodeScope
+    );
+
+    public static final Setting<Boolean> EXTENSIONS_SETTING = Setting.boolSetting(
+        EXTENSIONS,
+        false,
+        Property.NodeScope
+    );
 }
diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java
@@ -426,6 +426,9 @@ protected Node(
             );
             final Settings settings = pluginsService.updatedSettings();
 
+            // Ensure to initialise Feature Flags via the settings from opensearch.yml
+            FeatureFlags.initialiseFeatureFlags(settings);
+
             final Set<DiscoveryNodeRole> additionalRoles = pluginsService.filterPlugins(Plugin.class)
                 .stream()
                 .map(Plugin::getRoles)

diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java
@@ -109,6 +109,7 @@
 import org.opensearch.common.unit.ByteSizeUnit;
 import org.opensearch.common.unit.ByteSizeValue;
 import org.opensearch.common.unit.TimeValue;
+import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.common.util.concurrent.OpenSearchRejectedExecutionException;
 import org.opensearch.common.util.concurrent.ThreadContext;
 import org.opensearch.common.xcontent.NamedXContentRegistry;
@@ -763,6 +764,21 @@ public Settings indexSettings() {
         return builder.build();
     }
 
+    /**
+     * Setting all feature flag settings at base IT, which can be overridden later by individual
+     * IT classes.
+     *
+     * @return Feature flag settings.
+     */
+    protected Settings featureFlagSettings() {
+        return Settings.builder()
+            .put(FeatureFlags.REPLICATION_TYPE, "true")
+            .put(FeatureFlags.REMOTE_STORE, "false")
+            .put(FeatureFlags.SEARCHABLE_SNAPSHOT, "false")
+            .put(FeatureFlags.EXTENSIONS, "false")
+            .build();
+    }
+
     /**
      * Creates one or more indices and asserts that the indices are acknowledged. If one of the indices
      * already exists this method will fail and wipe all the indices created so far.