Remove Node Shutdown API feature flag (#76588)

* Remove Node Shutdown API feature flag

This PR removes the Node Shutdown API feature flag.

The Node Shutdown API will now always be available.

* Check if xpack is enabled in cleanup

When I removed the feature flag, I assumed that we would always have the
Node Shutdown APIs, but that turns out not to be the case if xpack isn't
enabled. This case was caught by the logic to handle the case where the
feature flag wasn't enabled by accident.

This commit adds the check we always should have had.

* Also check version before tyring cleanup

Author: gwbrown

test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

@@ -729,21 +729,19 @@ private void wipeCluster() throws Exception {
      */
     @SuppressWarnings("unchecked")
     protected void deleteAllNodeShutdownMetadata() throws IOException {
-        Request getShutdownStatus = new Request("GET", "_nodes/shutdown");
-        Map<String, Object> statusResponse = responseAsMap(adminClient().performRequest(getShutdownStatus));
-        if (statusResponse.containsKey("_nodes") && statusResponse.containsKey("cluster_name")) {
-            // If the response contains these two keys, the feature flag isn't enabled on this cluster, so skip out now.
-            // We can't check the system property directly because it only gets set for the cluster under test's JVM, not for the test
-            // runner's JVM.
+        if (hasXPack() == false || minimumNodeVersion().before(Version.V_7_15_0)) {
+            // Node shutdown APIs are only present in xpack
             return;
         }
-            List<Map<String, Object>> nodesArray = (List<Map<String, Object>>) statusResponse.get("nodes");
-            List<String> nodeIds = nodesArray.stream()
-                .map(nodeShutdownMetadata -> (String) nodeShutdownMetadata.get("node_id"))
-                .collect(Collectors.toUnmodifiableList());
-            for (String nodeId : nodeIds) {
-                Request deleteRequest = new Request("DELETE", "_nodes/" + nodeId + "/shutdown");
-                assertOK(adminClient().performRequest(deleteRequest));
+        Request getShutdownStatus = new Request("GET", "_nodes/shutdown");
+        Map<String, Object> statusResponse = responseAsMap(adminClient().performRequest(getShutdownStatus));
+        List<Map<String, Object>> nodesArray = (List<Map<String, Object>>) statusResponse.get("nodes");
+        List<String> nodeIds = nodesArray.stream()
+            .map(nodeShutdownMetadata -> (String) nodeShutdownMetadata.get("node_id"))
+            .collect(Collectors.toUnmodifiableList());
+        for (String nodeId : nodeIds) {
+            Request deleteRequest = new Request("DELETE", "_nodes/" + nodeId + "/shutdown");
+            assertOK(adminClient().performRequest(deleteRequest));
         }
     }
 

x-pack/plugin/ml/src/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/MlNodeShutdownIT.java

@@ -8,7 +8,6 @@
 package org.elasticsearch.xpack.ml.integration;
 
 import org.apache.lucene.util.SetOnce;
-import org.elasticsearch.Build;
 import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata;
 import org.elasticsearch.common.unit.ByteSizeValue;
@@ -37,9 +36,6 @@ public class MlNodeShutdownIT extends BaseMlIntegTestCase {
 
     public void testJobsVacateShuttingDownNode() throws Exception {
 
-        // TODO: delete this condition when the shutdown API is always available
-        assumeTrue("shutdown API is behind a snapshot-only feature flag", Build.CURRENT.isSnapshot());
-
         internalCluster().ensureAtLeastNumDataNodes(3);
         ensureStableCluster();
 
@@ -105,9 +101,6 @@ public void testJobsVacateShuttingDownNode() throws Exception {
 
     public void testCloseJobVacatingShuttingDownNode() throws Exception {
 
-        // TODO: delete this condition when the shutdown API is always available
-        assumeTrue("shutdown API is behind a snapshot-only feature flag", Build.CURRENT.isSnapshot());
-
         internalCluster().ensureAtLeastNumDataNodes(3);
         ensureStableCluster();
 

x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/support/BaseMlIntegTestCase.java

@@ -7,7 +7,6 @@
 package org.elasticsearch.xpack.ml.support;
 
 import org.apache.logging.log4j.Logger;
-import org.elasticsearch.Build;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.DocWriteRequest;
 import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksAction;
@@ -131,10 +130,6 @@ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
         settings.put(MonitoringService.ENABLED.getKey(), false);
         settings.put(MonitoringService.ELASTICSEARCH_COLLECTION_ENABLED.getKey(), false);
         settings.put(LifecycleSettings.LIFECYCLE_HISTORY_INDEX_ENABLED_SETTING.getKey(), false);
-        // TODO: put this setting unconditionally once the shutdown API is not protected by a feature flag
-        if (Build.CURRENT.isSnapshot()) {
-            settings.put(ShutdownPlugin.SHUTDOWN_FEATURE_ENABLED_FLAG, true);
-        }
         return settings.build();
     }
 

x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesIT.java

@@ -49,12 +49,6 @@ protected void deleteAllNodeShutdownMetadata() throws IOException {
         Request getShutdownStatus = new Request("GET", "_nodes/shutdown");
         getShutdownStatus.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", OPERATOR_AUTH_HEADER));
         Map<String, Object> statusResponse = responseAsMap(adminClient().performRequest(getShutdownStatus));
-        if (statusResponse.containsKey("_nodes") && statusResponse.containsKey("cluster_name")) {
-            // If the response contains these two keys, the feature flag isn't enabled on this cluster, so skip out now.
-            // We can't check the system property directly because it only gets set for the cluster under test's JVM, not for the test
-            // runner's JVM.
-            return;
-        }
         List<Map<String, Object>> nodesArray = (List<Map<String, Object>>) statusResponse.get("nodes");
         List<String> nodeIds = nodesArray.stream()
             .map(nodeShutdownMetadata -> (String) nodeShutdownMetadata.get("node_id"))

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

@@ -7,7 +7,6 @@
 
 package org.elasticsearch.xpack.shutdown;
 
-import org.elasticsearch.Build;
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.ResponseException;
@@ -47,7 +46,6 @@ public void testRemoveCRUD() throws Exception {
 
     @SuppressWarnings("unchecked")
     public void checkCRUD(String type, String allocationDelay) 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();
 
         // Ensure if we do a GET before the cluster metadata is set up, we don't get an error
@@ -83,7 +81,6 @@ public void testPutShutdownIsIdempotentForRemove() throws Exception {
 
     @SuppressWarnings("unchecked")
     private void checkPutShutdownIdempotency(String type) 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();
 
         // PUT the shutdown once
@@ -120,7 +117,6 @@ public void testPutShutdownCanChangeTypeFromRemoveToRestart() throws Exception {
 
     @SuppressWarnings("unchecked")
     public void checkTypeChange(String fromType, String toType) 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 = fromType;
 
@@ -153,7 +149,6 @@ public void checkTypeChange(String fromType, String toType) throws Exception {
      */
     @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");
 
@@ -200,7 +195,6 @@ public void testAllocationPreventedForRemoval() throws Exception {
      */
     @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";
@@ -252,7 +246,6 @@ public void testShardsMoveOffRemovingNode() throws Exception {
     }
 
     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");
 
@@ -275,7 +268,6 @@ public void testShardsCanBeAllocatedAfterShutdownDeleted() throws Exception {
     }
 
     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);
 
@@ -328,7 +320,6 @@ public void testStalledShardMigrationProperlyDetected() throws Exception {
      * Ensures that attempting to delete the status of a node that is not registered for shutdown gives a 404 response code.
      */
     public void testDeleteNodeNotRegisteredForShutdown() 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());
         Request deleteReq = new Request("DELETE", "_nodes/this-node-doesnt-exist/shutdown");
         ResponseException ex = expectThrows(ResponseException.class, () -> client().performRequest(deleteReq));
         assertThat(ex.getResponse().getStatusLine().getStatusCode(), is(404));

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

@@ -9,12 +9,10 @@
 
 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;
 import org.elasticsearch.cluster.node.DiscoveryNode;
-import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.plugins.ShutdownAwarePlugin;
 import org.elasticsearch.test.ESIntegTestCase;
@@ -41,11 +39,8 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
     }
 
     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
-        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 node1 = internalCluster().startNode();
+        final String node2 = internalCluster().startNode();
 
         final String shutdownNode;
         final String remainNode;

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

@@ -9,7 +9,6 @@
 
 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;
@@ -26,7 +25,6 @@
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsModule;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -77,11 +75,9 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
     }
 
     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
-        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 node1 = internalCluster().startNode();
+        final String node2 = internalCluster().startNode();
 
         final String shutdownNode;
         final String candidateNode;

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

@@ -6,7 +6,6 @@
  */
 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;
@@ -17,7 +16,6 @@
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 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.common.xcontent.NamedXContentRegistry;
@@ -39,34 +37,6 @@
 import java.util.function.Supplier;
 
 public class ShutdownPlugin extends Plugin implements ActionPlugin {
-
-    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,
-        (settings) -> {
-            final String enabled = settings.get(SHUTDOWN_FEATURE_ENABLED_FLAG);
-            // Enabled by default on snapshot builds, disabled on release builds
-            if (Build.CURRENT.isSnapshot()) {
-                if (enabled != null && enabled.equalsIgnoreCase("false")) {
-                    return "false";
-                } else {
-                    return "true";
-                }
-            } else {
-                if (enabled != null && enabled.equalsIgnoreCase("true")) {
-                    throw new IllegalArgumentException("shutdown plugin may not be enabled on a non-snapshot build");
-                } else {
-                    return "false";
-                }
-            }
-        },
-        Setting.Property.NodeScope
-    );
-
-    public boolean isEnabled(Settings settings) {
-        return SHUTDOWN_FEATURE_ENABLED_FLAG_SETTING.get(settings);
-    }
-
     @Override
     public Collection<Object> createComponents(
         Client client,
@@ -114,14 +84,6 @@ public List<RestHandler> getRestHandlers(
         IndexNameExpressionResolver indexNameExpressionResolver,
         Supplier<DiscoveryNodes> nodesInCluster
     ) {
-        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);
-    }
 }