Skip to content

Commit b566abc

Browse files
dakronedakrone
authored
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
1 parent 03de065 commit b566abc

File tree

9 files changed

+62
-32
lines changed

9 files changed

+62
-32
lines changed

‎build-tools-internal/src/main/groovy/elasticsearch.run.gradle‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* Side Public License, v 1.
77
*/
88

9+
import org.elasticsearch.gradle.VersionProperties
910
import org.elasticsearch.gradle.testclusters.RunTask
1011

1112
// gradle has an open issue of failing applying plugins in
@@ -26,9 +27,11 @@ testClusters {
2627
throw new IllegalArgumentException("Unsupported self-generated license type: [" + licenseType + "[basic] or [trial].")
2728
}
2829
setting 'xpack.security.enabled', 'true'
30+
if (VersionProperties.elasticsearch.toString().endsWith('-SNAPSHOT')) {
31+
setting 'es.shutdown_feature_flag_enabled', 'true'
32+
}
2933
keystore 'bootstrap.password', 'password'
3034
user username: 'elastic-admin', password: 'elastic-password', role: 'superuser'
31-
systemProperty 'es.shutdown_feature_flag_enabled', 'true'
3235
}
3336
}
3437
}

‎docs/build.gradle‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import org.elasticsearch.gradle.VersionProperties
12
import org.elasticsearch.gradle.internal.info.BuildParams
23

34
import static org.elasticsearch.gradle.testclusters.TestDistribution.DEFAULT
@@ -63,8 +64,10 @@ testClusters.matching { it.name == "integTest"}.configureEach {
6364
setting 'xpack.license.self_generated.type', 'trial'
6465
setting 'indices.lifecycle.history_index_enabled', 'false'
6566
setting 'ingest.geoip.downloader.enabled', 'false'
67+
if (VersionProperties.elasticsearch.toString().endsWith('-SNAPSHOT')) {
68+
setting 'es.shutdown_feature_flag_enabled', 'true'
69+
}
6670
systemProperty 'es.geoip_v2_feature_flag_enabled', 'true'
67-
systemProperty 'es.shutdown_feature_flag_enabled', 'true'
6871
keystorePassword 'keystore-password'
6972
}
7073

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,11 @@ public class OperatorOnlyRegistry {
3535
// Repository analysis actions are not mentioned in core, literal strings are needed.
3636
"cluster:admin/repository/analyze",
3737
"cluster:admin/repository/analyze/blob",
38-
"cluster:admin/repository/analyze/blob/read"
38+
"cluster:admin/repository/analyze/blob/read",
39+
// Node shutdown APIs are operator only
40+
"cluster:admin/shutdown/create",
41+
"cluster:admin/shutdown/get",
42+
"cluster:admin/shutdown/delete"
3943
);
4044

4145
private final ClusterSettings clusterSettings;

‎x-pack/plugin/shutdown/build.gradle‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import org.elasticsearch.gradle.VersionProperties
2+
13
apply plugin: 'elasticsearch.internal-es-plugin'
24
apply plugin: 'elasticsearch.internal-cluster-test'
35

@@ -20,7 +22,9 @@ testClusters.all {
2022
testDistribution = 'default'
2123
setting 'xpack.security.enabled', 'true'
2224
setting 'xpack.license.self_generated.type', 'trial'
25+
if (VersionProperties.elasticsearch.toString().endsWith('-SNAPSHOT')) {
26+
setting 'es.shutdown_feature_flag_enabled', 'true'
27+
}
2328
keystore 'bootstrap.password', 'x-pack-test-password'
2429
user username: "x_pack_rest_user", password: "x-pack-test-password"
25-
systemProperty 'es.shutdown_feature_flag_enabled', 'true'
2630
}

‎x-pack/plugin/shutdown/qa/multi-node/build.gradle‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import org.elasticsearch.gradle.VersionProperties
2+
13
apply plugin: 'elasticsearch.java-rest-test'
24

35
dependencies {
@@ -16,7 +18,9 @@ testClusters.all {
1618
testDistribution = 'DEFAULT'
1719
numberOfNodes = 4
1820

19-
systemProperty 'es.shutdown_feature_flag_enabled', 'true'
21+
if (VersionProperties.elasticsearch.toString().endsWith('-SNAPSHOT')) {
22+
setting 'es.shutdown_feature_flag_enabled', 'true'
23+
}
2024
setting 'xpack.security.enabled', 'true'
2125
user username: clusterCredentials.username, password: clusterCredentials.password, role: 'superuser'
2226
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package org.elasticsearch.xpack.shutdown;
99

10+
import org.elasticsearch.Build;
1011
import org.elasticsearch.client.Request;
1112
import org.elasticsearch.client.Response;
1213
import org.elasticsearch.common.settings.SecureString;
@@ -35,6 +36,7 @@ public class NodeShutdownIT extends ESRestTestCase {
3536

3637
@SuppressWarnings("unchecked")
3738
public void testCRUD() throws Exception {
39+
assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
3840
String nodeIdToShutdown = getRandomNodeId();
3941
String type = randomFrom("RESTART", "REMOVE");
4042

@@ -65,6 +67,7 @@ public void testCRUD() throws Exception {
6567
*/
6668
@SuppressWarnings("unchecked")
6769
public void testAllocationPreventedForRemoval() throws Exception {
70+
assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
6871
String nodeIdToShutdown = getRandomNodeId();
6972
putNodeShutdown(nodeIdToShutdown, "REMOVE");
7073

@@ -111,6 +114,7 @@ public void testAllocationPreventedForRemoval() throws Exception {
111114
*/
112115
@SuppressWarnings("unchecked")
113116
public void testShardsMoveOffRemovingNode() throws Exception {
117+
assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
114118
String nodeIdToShutdown = getRandomNodeId();
115119

116120
final String indexName = "test-idx";
@@ -162,6 +166,7 @@ public void testShardsMoveOffRemovingNode() throws Exception {
162166
}
163167

164168
public void testShardsCanBeAllocatedAfterShutdownDeleted() throws Exception {
169+
assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
165170
String nodeIdToShutdown = getRandomNodeId();
166171
putNodeShutdown(nodeIdToShutdown, "REMOVE");
167172

@@ -184,6 +189,7 @@ public void testShardsCanBeAllocatedAfterShutdownDeleted() throws Exception {
184189
}
185190

186191
public void testStalledShardMigrationProperlyDetected() throws Exception {
192+
assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
187193
String nodeIdToShutdown = getRandomNodeId();
188194
int numberOfShards = randomIntBetween(1,5);
189195

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import org.apache.logging.log4j.LogManager;
1111
import org.apache.logging.log4j.Logger;
12+
import org.elasticsearch.Build;
1213
import org.elasticsearch.action.admin.cluster.node.info.NodeInfo;
1314
import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse;
1415
import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata;
@@ -36,13 +37,15 @@ public class NodeShutdownPluginsIT extends ESIntegTestCase {
3637

3738
@Override
3839
protected Collection<Class<? extends Plugin>> nodePlugins() {
39-
return Arrays.asList(ShutdownEnabledPlugin.class, TestShutdownAwarePlugin.class);
40+
return Arrays.asList(ShutdownPlugin.class, TestShutdownAwarePlugin.class);
4041
}
4142

4243
public void testShutdownAwarePlugin() throws Exception {
44+
assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
4345
// Start two nodes, one will be marked as shutting down
44-
final String node1 = internalCluster().startNode(Settings.EMPTY);
45-
final String node2 = internalCluster().startNode(Settings.EMPTY);
46+
Settings enabledSettings = Settings.builder().put(ShutdownPlugin.SHUTDOWN_FEATURE_ENABLED_FLAG, true).build();
47+
final String node1 = internalCluster().startNode(enabledSettings);
48+
final String node2 = internalCluster().startNode(enabledSettings);
4649

4750
final String shutdownNode;
4851
final String remainNode;
@@ -110,13 +113,6 @@ public void testShutdownAwarePlugin() throws Exception {
110113
assertThat(triggeredNodes.get(), empty());
111114
}
112115

113-
public static class ShutdownEnabledPlugin extends ShutdownPlugin {
114-
@Override
115-
public boolean isEnabled() {
116-
return true;
117-
}
118-
}
119-
120116
public static class TestShutdownAwarePlugin extends Plugin implements ShutdownAwarePlugin {
121117

122118
@Override

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import org.apache.logging.log4j.LogManager;
1111
import org.apache.logging.log4j.Logger;
12+
import org.elasticsearch.Build;
1213
import org.elasticsearch.ResourceAlreadyExistsException;
1314
import org.elasticsearch.Version;
1415
import org.elasticsearch.action.ActionListener;
@@ -72,13 +73,15 @@ public class NodeShutdownTasksIT extends ESIntegTestCase {
7273

7374
@Override
7475
protected Collection<Class<? extends Plugin>> nodePlugins() {
75-
return Arrays.asList(ShutdownEnabledPlugin.class, TaskPlugin.class);
76+
return Arrays.asList(ShutdownPlugin.class, TaskPlugin.class);
7677
}
7778

7879
public void testTasksAreNotAssignedToShuttingDownNode() throws Exception {
80+
assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
7981
// Start two nodes, one will be marked as shutting down
80-
final String node1 = internalCluster().startNode(Settings.EMPTY);
81-
final String node2 = internalCluster().startNode(Settings.EMPTY);
82+
Settings enabledSettings = Settings.builder().put(ShutdownPlugin.SHUTDOWN_FEATURE_ENABLED_FLAG, true).build();
83+
final String node1 = internalCluster().startNode(enabledSettings);
84+
final String node2 = internalCluster().startNode(enabledSettings);
8285

8386
final String shutdownNode;
8487
final String candidateNode;
@@ -124,13 +127,6 @@ public void testTasksAreNotAssignedToShuttingDownNode() throws Exception {
124127
assertThat(candidates.get().stream().map(DiscoveryNode::getId).collect(Collectors.toSet()), not(contains(shutdownNode)));
125128
}
126129

127-
public static class ShutdownEnabledPlugin extends ShutdownPlugin {
128-
@Override
129-
public boolean isEnabled() {
130-
return true;
131-
}
132-
}
133-
134130
public static class TaskPlugin extends Plugin implements PersistentTaskPlugin {
135131

136132
TaskExecutor taskExecutor;

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

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@
66
*/
77
package org.elasticsearch.xpack.shutdown;
88

9+
import org.elasticsearch.Build;
910
import org.elasticsearch.action.ActionRequest;
1011
import org.elasticsearch.action.ActionResponse;
1112
import org.elasticsearch.action.support.master.AcknowledgedResponse;
1213
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
1314
import org.elasticsearch.cluster.node.DiscoveryNodes;
1415
import org.elasticsearch.common.settings.ClusterSettings;
1516
import org.elasticsearch.common.settings.IndexScopedSettings;
17+
import org.elasticsearch.common.settings.Setting;
1618
import org.elasticsearch.common.settings.Settings;
1719
import org.elasticsearch.common.settings.SettingsFilter;
1820
import org.elasticsearch.plugins.ActionPlugin;
@@ -27,17 +29,24 @@
2729

2830
public class ShutdownPlugin extends Plugin implements ActionPlugin {
2931

30-
public static final boolean SHUTDOWN_FEATURE_FLAG_ENABLED = "true".equals(System.getProperty("es.shutdown_feature_flag_enabled"));
32+
public static final String SHUTDOWN_FEATURE_ENABLED_FLAG = "es.shutdown_feature_flag_enabled";
33+
public static final Setting<Boolean> SHUTDOWN_FEATURE_ENABLED_FLAG_SETTING = Setting.boolSetting(
34+
SHUTDOWN_FEATURE_ENABLED_FLAG,
35+
false,
36+
enabled -> {
37+
if (enabled != null && enabled && Build.CURRENT.isSnapshot() == false) {
38+
throw new IllegalArgumentException("shutdown plugin may not be enabled on a non-snapshot build");
39+
}
40+
},
41+
Setting.Property.NodeScope
42+
);
3143

32-
public boolean isEnabled() {
33-
return SHUTDOWN_FEATURE_FLAG_ENABLED;
44+
public boolean isEnabled(Settings settings) {
45+
return SHUTDOWN_FEATURE_ENABLED_FLAG_SETTING.get(settings);
3446
}
3547

3648
@Override
3749
public List<ActionHandler<? extends ActionRequest, ? extends ActionResponse>> getActions() {
38-
if (isEnabled() == false) {
39-
return Collections.emptyList();
40-
}
4150
ActionHandler<PutShutdownNodeAction.Request, AcknowledgedResponse> putShutdown = new ActionHandler<>(
4251
PutShutdownNodeAction.INSTANCE,
4352
TransportPutShutdownNodeAction.class
@@ -63,9 +72,14 @@ public List<RestHandler> getRestHandlers(
6372
IndexNameExpressionResolver indexNameExpressionResolver,
6473
Supplier<DiscoveryNodes> nodesInCluster
6574
) {
66-
if (isEnabled() == false) {
75+
if (isEnabled(settings) == false) {
6776
return Collections.emptyList();
6877
}
6978
return Arrays.asList(new RestPutShutdownNodeAction(), new RestDeleteShutdownNodeAction(), new RestGetShutdownStatusAction());
7079
}
80+
81+
@Override
82+
public List<Setting<?>> getSettings() {
83+
return Collections.singletonList(SHUTDOWN_FEATURE_ENABLED_FLAG_SETTING);
84+
}
7185
}

0 commit comments

Comments
 (0)