Browse Source

Convert node shutdown system property feature flag to setting (#74267)

This converts the system property feature flag 'es.shutdown_feature_flag_enabled' to a regular
non-dynamic node setting. This setting can only be set to 'true' on a snapshot build of
Elasticsearch (not a release build).

Relates to #70338
Lee Hinman 4 years ago
parent
commit
b566abc8f4

+ 4 - 1
build-tools-internal/src/main/groovy/elasticsearch.run.gradle

@@ -6,6 +6,7 @@
  * Side Public License, v 1.
  */
 
+import org.elasticsearch.gradle.VersionProperties
 import org.elasticsearch.gradle.testclusters.RunTask
 
 // gradle has an open issue of failing applying plugins in
@@ -26,9 +27,11 @@ testClusters {
         throw new IllegalArgumentException("Unsupported self-generated license type: [" + licenseType + "[basic] or [trial].")
       }
       setting 'xpack.security.enabled', 'true'
+      if (VersionProperties.elasticsearch.toString().endsWith('-SNAPSHOT')) {
+        setting 'es.shutdown_feature_flag_enabled', 'true'
+      }
       keystore 'bootstrap.password', 'password'
       user username: 'elastic-admin', password: 'elastic-password', role: 'superuser'
-      systemProperty 'es.shutdown_feature_flag_enabled', 'true'
     }
   }
 }

+ 4 - 1
docs/build.gradle

@@ -1,3 +1,4 @@
+import org.elasticsearch.gradle.VersionProperties
 import org.elasticsearch.gradle.internal.info.BuildParams
 
 import static org.elasticsearch.gradle.testclusters.TestDistribution.DEFAULT
@@ -63,8 +64,10 @@ testClusters.matching { it.name == "integTest"}.configureEach {
     setting 'xpack.license.self_generated.type', 'trial'
     setting 'indices.lifecycle.history_index_enabled', 'false'
     setting 'ingest.geoip.downloader.enabled', 'false'
+    if (VersionProperties.elasticsearch.toString().endsWith('-SNAPSHOT')) {
+      setting 'es.shutdown_feature_flag_enabled', 'true'
+    }
     systemProperty 'es.geoip_v2_feature_flag_enabled', 'true'
-    systemProperty 'es.shutdown_feature_flag_enabled', 'true'
     keystorePassword 'keystore-password'
   }
 

+ 5 - 1
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java

@@ -35,7 +35,11 @@ public class OperatorOnlyRegistry {
         // Repository analysis actions are not mentioned in core, literal strings are needed.
         "cluster:admin/repository/analyze",
         "cluster:admin/repository/analyze/blob",
-        "cluster:admin/repository/analyze/blob/read"
+        "cluster:admin/repository/analyze/blob/read",
+        // Node shutdown APIs are operator only
+        "cluster:admin/shutdown/create",
+        "cluster:admin/shutdown/get",
+        "cluster:admin/shutdown/delete"
         );
 
     private final ClusterSettings clusterSettings;

+ 5 - 1
x-pack/plugin/shutdown/build.gradle

@@ -1,3 +1,5 @@
+import org.elasticsearch.gradle.VersionProperties
+
 apply plugin: 'elasticsearch.internal-es-plugin'
 apply plugin: 'elasticsearch.internal-cluster-test'
 
@@ -20,7 +22,9 @@ testClusters.all {
   testDistribution = 'default'
   setting 'xpack.security.enabled', 'true'
   setting 'xpack.license.self_generated.type', 'trial'
+  if (VersionProperties.elasticsearch.toString().endsWith('-SNAPSHOT')) {
+    setting 'es.shutdown_feature_flag_enabled', 'true'
+  }
   keystore 'bootstrap.password', 'x-pack-test-password'
   user username: "x_pack_rest_user", password: "x-pack-test-password"
-  systemProperty 'es.shutdown_feature_flag_enabled', 'true'
 }

+ 5 - 1
x-pack/plugin/shutdown/qa/multi-node/build.gradle

@@ -1,3 +1,5 @@
+import org.elasticsearch.gradle.VersionProperties
+
 apply plugin: 'elasticsearch.java-rest-test'
 
 dependencies {
@@ -16,7 +18,9 @@ testClusters.all {
   testDistribution = 'DEFAULT'
   numberOfNodes = 4
 
-  systemProperty 'es.shutdown_feature_flag_enabled', 'true'
+  if (VersionProperties.elasticsearch.toString().endsWith('-SNAPSHOT')) {
+    setting 'es.shutdown_feature_flag_enabled', 'true'
+  }
   setting 'xpack.security.enabled', 'true'
   user username: clusterCredentials.username, password: clusterCredentials.password, role: 'superuser'
 }

+ 6 - 0
x-pack/plugin/shutdown/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/shutdown/NodeShutdownIT.java

@@ -7,6 +7,7 @@
 
 package org.elasticsearch.xpack.shutdown;
 
+import org.elasticsearch.Build;
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.Response;
 import org.elasticsearch.common.settings.SecureString;
@@ -35,6 +36,7 @@ public class NodeShutdownIT extends ESRestTestCase {
 
     @SuppressWarnings("unchecked")
     public void testCRUD() throws Exception {
+        assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
         String nodeIdToShutdown = getRandomNodeId();
         String type = randomFrom("RESTART", "REMOVE");
 
@@ -65,6 +67,7 @@ public class NodeShutdownIT extends ESRestTestCase {
      */
     @SuppressWarnings("unchecked")
     public void testAllocationPreventedForRemoval() throws Exception {
+        assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
         String nodeIdToShutdown = getRandomNodeId();
         putNodeShutdown(nodeIdToShutdown, "REMOVE");
 
@@ -111,6 +114,7 @@ public class NodeShutdownIT extends ESRestTestCase {
      */
     @SuppressWarnings("unchecked")
     public void testShardsMoveOffRemovingNode() throws Exception {
+        assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
         String nodeIdToShutdown = getRandomNodeId();
 
         final String indexName = "test-idx";
@@ -162,6 +166,7 @@ public class NodeShutdownIT extends ESRestTestCase {
     }
 
     public void testShardsCanBeAllocatedAfterShutdownDeleted() throws Exception {
+        assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
         String nodeIdToShutdown = getRandomNodeId();
         putNodeShutdown(nodeIdToShutdown, "REMOVE");
 
@@ -184,6 +189,7 @@ public class NodeShutdownIT extends ESRestTestCase {
     }
 
     public void testStalledShardMigrationProperlyDetected() throws Exception {
+        assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
         String nodeIdToShutdown = getRandomNodeId();
         int numberOfShards = randomIntBetween(1,5);
 

+ 6 - 10
x-pack/plugin/shutdown/src/internalClusterTest/java/org/elasticsearch/xpack/shutdown/NodeShutdownPluginsIT.java

@@ -9,6 +9,7 @@ package org.elasticsearch.xpack.shutdown;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.elasticsearch.Build;
 import org.elasticsearch.action.admin.cluster.node.info.NodeInfo;
 import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse;
 import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata;
@@ -36,13 +37,15 @@ public class NodeShutdownPluginsIT extends ESIntegTestCase {
 
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
-        return Arrays.asList(ShutdownEnabledPlugin.class, TestShutdownAwarePlugin.class);
+        return Arrays.asList(ShutdownPlugin.class, TestShutdownAwarePlugin.class);
     }
 
     public void testShutdownAwarePlugin() throws Exception {
+        assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
         // Start two nodes, one will be marked as shutting down
-        final String node1 = internalCluster().startNode(Settings.EMPTY);
-        final String node2 = internalCluster().startNode(Settings.EMPTY);
+        Settings enabledSettings = Settings.builder().put(ShutdownPlugin.SHUTDOWN_FEATURE_ENABLED_FLAG, true).build();
+        final String node1 = internalCluster().startNode(enabledSettings);
+        final String node2 = internalCluster().startNode(enabledSettings);
 
         final String shutdownNode;
         final String remainNode;
@@ -110,13 +113,6 @@ public class NodeShutdownPluginsIT extends ESIntegTestCase {
         assertThat(triggeredNodes.get(), empty());
     }
 
-    public static class ShutdownEnabledPlugin extends ShutdownPlugin {
-        @Override
-        public boolean isEnabled() {
-            return true;
-        }
-    }
-
     public static class TestShutdownAwarePlugin extends Plugin implements ShutdownAwarePlugin {
 
         @Override

+ 6 - 10
x-pack/plugin/shutdown/src/internalClusterTest/java/org/elasticsearch/xpack/shutdown/NodeShutdownTasksIT.java

@@ -9,6 +9,7 @@ package org.elasticsearch.xpack.shutdown;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.elasticsearch.Build;
 import org.elasticsearch.ResourceAlreadyExistsException;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
@@ -72,13 +73,15 @@ public class NodeShutdownTasksIT extends ESIntegTestCase {
 
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
-        return Arrays.asList(ShutdownEnabledPlugin.class, TaskPlugin.class);
+        return Arrays.asList(ShutdownPlugin.class, TaskPlugin.class);
     }
 
     public void testTasksAreNotAssignedToShuttingDownNode() throws Exception {
+        assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
         // Start two nodes, one will be marked as shutting down
-        final String node1 = internalCluster().startNode(Settings.EMPTY);
-        final String node2 = internalCluster().startNode(Settings.EMPTY);
+        Settings enabledSettings = Settings.builder().put(ShutdownPlugin.SHUTDOWN_FEATURE_ENABLED_FLAG, true).build();
+        final String node1 = internalCluster().startNode(enabledSettings);
+        final String node2 = internalCluster().startNode(enabledSettings);
 
         final String shutdownNode;
         final String candidateNode;
@@ -124,13 +127,6 @@ public class NodeShutdownTasksIT extends ESIntegTestCase {
         assertThat(candidates.get().stream().map(DiscoveryNode::getId).collect(Collectors.toSet()), not(contains(shutdownNode)));
     }
 
-    public static class ShutdownEnabledPlugin extends ShutdownPlugin {
-        @Override
-        public boolean isEnabled() {
-            return true;
-        }
-    }
-
     public static class TaskPlugin extends Plugin implements PersistentTaskPlugin {
 
         TaskExecutor taskExecutor;

+ 21 - 7
x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/ShutdownPlugin.java

@@ -6,6 +6,7 @@
  */
 package org.elasticsearch.xpack.shutdown;
 
+import org.elasticsearch.Build;
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
@@ -13,6 +14,7 @@ import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.node.DiscoveryNodes;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.IndexScopedSettings;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsFilter;
 import org.elasticsearch.plugins.ActionPlugin;
@@ -27,17 +29,24 @@ import java.util.function.Supplier;
 
 public class ShutdownPlugin extends Plugin implements ActionPlugin {
 
-    public static final boolean SHUTDOWN_FEATURE_FLAG_ENABLED = "true".equals(System.getProperty("es.shutdown_feature_flag_enabled"));
+    public static final String SHUTDOWN_FEATURE_ENABLED_FLAG = "es.shutdown_feature_flag_enabled";
+    public static final Setting<Boolean> SHUTDOWN_FEATURE_ENABLED_FLAG_SETTING = Setting.boolSetting(
+        SHUTDOWN_FEATURE_ENABLED_FLAG,
+        false,
+        enabled -> {
+            if (enabled != null && enabled && Build.CURRENT.isSnapshot() == false) {
+                throw new IllegalArgumentException("shutdown plugin may not be enabled on a non-snapshot build");
+            }
+        },
+        Setting.Property.NodeScope
+    );
 
-    public boolean isEnabled() {
-        return SHUTDOWN_FEATURE_FLAG_ENABLED;
+    public boolean isEnabled(Settings settings) {
+        return SHUTDOWN_FEATURE_ENABLED_FLAG_SETTING.get(settings);
     }
 
     @Override
     public List<ActionHandler<? extends ActionRequest, ? extends ActionResponse>> getActions() {
-        if (isEnabled() == false) {
-            return Collections.emptyList();
-        }
         ActionHandler<PutShutdownNodeAction.Request, AcknowledgedResponse> putShutdown = new ActionHandler<>(
             PutShutdownNodeAction.INSTANCE,
             TransportPutShutdownNodeAction.class
@@ -63,9 +72,14 @@ public class ShutdownPlugin extends Plugin implements ActionPlugin {
         IndexNameExpressionResolver indexNameExpressionResolver,
         Supplier<DiscoveryNodes> nodesInCluster
     ) {
-        if (isEnabled() == false) {
+        if (isEnabled(settings) == false) {
             return Collections.emptyList();
         }
         return Arrays.asList(new RestPutShutdownNodeAction(), new RestDeleteShutdownNodeAction(), new RestGetShutdownStatusAction());
     }
+
+    @Override
+    public List<Setting<?>> getSettings() {
+        return Collections.singletonList(SHUTDOWN_FEATURE_ENABLED_FLAG_SETTING);
+    }
 }