Skip to content

Commit

Permalink
Deprecate setting 'cluster.initial_master_nodes' and introduce the al…
Browse files Browse the repository at this point in the history
…ternative setting 'cluster.initial_cluster_manager_nodes' (#2463)

* Deprecate setting cluster.initial_master_nodes, and add setting cluster.initial_cluster_manager_nodes

Signed-off-by: Tianli Feng <[email protected]>
  • Loading branch information
Tianli Feng authored Mar 18, 2022
1 parent 9c4d7d9 commit 19eadb4
Show file tree
Hide file tree
Showing 20 changed files with 202 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ class ClusterFormationTasks {
}
boolean supportsInitialMasterNodes = hasBwcNodes == false || config.bwcVersion.onOrAfter("7.0.0")
if (esConfig['discovery.type'] == null && config.getAutoSetInitialMasterNodes() && supportsInitialMasterNodes) {
esConfig['cluster.initial_master_nodes'] = nodes.stream().map({ n ->
// To promote inclusive language, the old setting name is deprecated in 2.0.0
esConfig[node.nodeVersion.onOrAfter("2.0.0") ? 'cluster.initial_cluster_manager_nodes' : 'cluster.initial_master_nodes'] = nodes.stream().map({ n ->
if (n.config.settings['node.name'] == null) {
return "node-" + n.nodeNum
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,12 @@ private void commonNodeConfig(OpenSearchNode node, String nodeNames, OpenSearchN
.collect(Collectors.toList())
.forEach(node.defaultConfig::remove);
if (nodeNames != null && node.settings.getOrDefault("discovery.type", "anything").equals("single-node") == false) {
node.defaultConfig.put("cluster.initial_master_nodes", "[" + nodeNames + "]");
// To promote inclusive language, the old setting name is deprecated n 2.0.0
if (node.getVersion().onOrAfter("2.0.0")) {
node.defaultConfig.put("cluster.initial_cluster_manager_nodes", "[" + nodeNames + "]");
} else {
node.defaultConfig.put("cluster.initial_master_nodes", "[" + nodeNames + "]");
}
}
node.defaultConfig.put("discovery.seed_providers", "file");
node.defaultConfig.put("discovery.seed_hosts", "[]");
Expand Down
4 changes: 2 additions & 2 deletions distribution/docker/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ services:
image: opensearch:test
environment:
- node.name=opensearch-1
- cluster.initial_master_nodes=opensearch-1,opensearch-2
- cluster.initial_cluster_manager_nodes=opensearch-1,opensearch-2
- discovery.seed_hosts=opensearch-2:9300
- cluster.name=opensearch
- bootstrap.memory_lock=true
Expand All @@ -29,7 +29,7 @@ services:
image: opensearch:test
environment:
- node.name=opensearch-2
- cluster.initial_master_nodes=opensearch-1,opensearch-2
- cluster.initial_cluster_manager_nodes=opensearch-1,opensearch-2
- discovery.seed_hosts=opensearch-1:9300
- cluster.name=opensearch
- bootstrap.memory_lock=true
Expand Down
4 changes: 2 additions & 2 deletions distribution/src/config/opensearch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ ${path.logs}
#
#discovery.seed_hosts: ["host1", "host2"]
#
# Bootstrap the cluster using an initial set of master-eligible nodes:
# Bootstrap the cluster using an initial set of cluster-manager-eligible nodes:
#
#cluster.initial_master_nodes: ["node-1", "node-2"]
#cluster.initial_cluster_manager_nodes: ["node-1", "node-2"]
#
# For more information, consult the discovery and cluster formation module documentation.
#
Expand Down
4 changes: 2 additions & 2 deletions qa/remote-clusters/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ services:
image: opensearch:test
environment:
- node.name=opensearch-1
- cluster.initial_master_nodes=opensearch-1
- cluster.initial_cluster_manager_nodes=opensearch-1
- cluster.name=opensearch-1
- bootstrap.memory_lock=true
- network.publish_host=127.0.0.1
Expand Down Expand Up @@ -39,7 +39,7 @@ services:
image: opensearch:test
environment:
- node.name=opensearch-2
- cluster.initial_master_nodes=opensearch-2
- cluster.initial_cluster_manager_nodes=opensearch-2
- cluster.name=opensearch-2
- bootstrap.memory_lock=true
- network.publish_host=127.0.0.1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ public boolean clearData(String nodeName) {
@Override
public Settings onNodeStopped(String nodeName) {
return Settings.builder()
.put(ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING.getKey(), nodeName)
.put(ClusterBootstrapService.INITIAL_CLUSTER_MANAGER_NODES_SETTING.getKey(), nodeName)
/*
* the data node might join while the master is still not fully established as master just yet and bypasses the join
* validation that is done before adding the node to the cluster. Only the join validation when handling the publish
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
import java.util.Set;
import java.util.stream.IntStream;

import static org.opensearch.cluster.coordination.ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING;
import static org.opensearch.cluster.coordination.ClusterBootstrapService.INITIAL_CLUSTER_MANAGER_NODES_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder;
Expand Down Expand Up @@ -383,7 +383,7 @@ public void testTwoNodeFirstNodeCleared() throws Exception {
public Settings onNodeStopped(String nodeName) {
return Settings.builder()
.put(RECOVER_AFTER_NODES_SETTING.getKey(), 2)
.putList(INITIAL_MASTER_NODES_SETTING.getKey()) // disable bootstrapping
.putList(INITIAL_CLUSTER_MANAGER_NODES_SETTING.getKey()) // disable bootstrapping
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.opensearch.cluster.coordination.ClusterBootstrapService.INITIAL_CLUSTER_MANAGER_NODES_SETTING;
import static org.opensearch.cluster.coordination.ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING;
import static org.opensearch.discovery.DiscoveryModule.DISCOVERY_SEED_PROVIDERS_SETTING;
import static org.opensearch.discovery.SettingsBasedSeedHostsProvider.DISCOVERY_SEED_HOSTS_SETTING;
Expand Down Expand Up @@ -773,10 +774,12 @@ public BootstrapCheckResult check(BootstrapContext context) {
return BootstrapCheckResult.failure(
String.format(
Locale.ROOT,
"the default discovery settings are unsuitable for production use; at least one of [%s] must be configured",
Stream.of(DISCOVERY_SEED_HOSTS_SETTING, DISCOVERY_SEED_PROVIDERS_SETTING, INITIAL_MASTER_NODES_SETTING)
// TODO: Remove ' / %s' from the error message after removing MASTER_ROLE, and update unit test.
"the default discovery settings are unsuitable for production use; at least one of [%s / %s] must be configured",
Stream.of(DISCOVERY_SEED_HOSTS_SETTING, DISCOVERY_SEED_PROVIDERS_SETTING, INITIAL_CLUSTER_MANAGER_NODES_SETTING)
.map(Setting::getKey)
.collect(Collectors.joining(", "))
.collect(Collectors.joining(", ")),
INITIAL_MASTER_NODES_SETTING.getKey()
)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,15 @@ public class ClusterBootstrapService {
"cluster.initial_master_nodes",
emptyList(),
Function.identity(),
Property.NodeScope,
Property.Deprecated
);
// The setting below is going to replace the above.
// To keep backwards compatibility, the old usage is remained, and it's also used as the fallback for the new usage.
public static final Setting<List<String>> INITIAL_CLUSTER_MANAGER_NODES_SETTING = Setting.listSetting(
"cluster.initial_cluster_manager_nodes",
INITIAL_MASTER_NODES_SETTING,
Function.identity(),
Property.NodeScope
);

Expand Down Expand Up @@ -105,10 +114,14 @@ public ClusterBootstrapService(
Consumer<VotingConfiguration> votingConfigurationConsumer
) {
if (DiscoveryModule.isSingleNodeDiscovery(settings)) {
if (INITIAL_MASTER_NODES_SETTING.exists(settings)) {
if (INITIAL_CLUSTER_MANAGER_NODES_SETTING.existsOrFallbackExists(settings)) {
// TODO: Remove variable 'initialClusterManagerSettingName' after removing MASTER_ROLE.
String initialClusterManagerSettingName = INITIAL_CLUSTER_MANAGER_NODES_SETTING.exists(settings)
? INITIAL_CLUSTER_MANAGER_NODES_SETTING.getKey()
: INITIAL_MASTER_NODES_SETTING.getKey();
throw new IllegalArgumentException(
"setting ["
+ INITIAL_MASTER_NODES_SETTING.getKey()
+ initialClusterManagerSettingName
+ "] is not allowed when ["
+ DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey()
+ "] is set to ["
Expand All @@ -128,11 +141,11 @@ public ClusterBootstrapService(
bootstrapRequirements = Collections.singleton(Node.NODE_NAME_SETTING.get(settings));
unconfiguredBootstrapTimeout = null;
} else {
final List<String> initialMasterNodes = INITIAL_MASTER_NODES_SETTING.get(settings);
final List<String> initialMasterNodes = INITIAL_CLUSTER_MANAGER_NODES_SETTING.get(settings);
bootstrapRequirements = unmodifiableSet(new LinkedHashSet<>(initialMasterNodes));
if (bootstrapRequirements.size() != initialMasterNodes.size()) {
throw new IllegalArgumentException(
"setting [" + INITIAL_MASTER_NODES_SETTING.getKey() + "] contains duplicates: " + initialMasterNodes
"setting [" + INITIAL_CLUSTER_MANAGER_NODES_SETTING.getKey() + "] contains duplicates: " + initialMasterNodes
);
}
unconfiguredBootstrapTimeout = discoveryIsConfigured(settings) ? null : UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING.get(settings);
Expand All @@ -150,7 +163,7 @@ public static boolean discoveryIsConfigured(Settings settings) {
LEGACY_DISCOVERY_HOSTS_PROVIDER_SETTING,
DISCOVERY_SEED_HOSTS_SETTING,
LEGACY_DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING,
INITIAL_MASTER_NODES_SETTING
INITIAL_CLUSTER_MANAGER_NODES_SETTING
).anyMatch(s -> s.exists(settings));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;

import static org.opensearch.cluster.coordination.ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING;
import static org.opensearch.cluster.coordination.ClusterBootstrapService.INITIAL_CLUSTER_MANAGER_NODES_SETTING;
import static org.opensearch.monitor.StatusInfo.Status.UNHEALTHY;

public class ClusterFormationFailureHelper {
Expand Down Expand Up @@ -198,13 +198,13 @@ String getDescription() {
if (clusterState.getLastAcceptedConfiguration().isEmpty()) {
final String bootstrappingDescription;

if (INITIAL_MASTER_NODES_SETTING.get(Settings.EMPTY).equals(INITIAL_MASTER_NODES_SETTING.get(settings))) {
bootstrappingDescription = "[" + INITIAL_MASTER_NODES_SETTING.getKey() + "] is empty on this node";
if (INITIAL_CLUSTER_MANAGER_NODES_SETTING.get(Settings.EMPTY).equals(INITIAL_CLUSTER_MANAGER_NODES_SETTING.get(settings))) {
bootstrappingDescription = "[" + INITIAL_CLUSTER_MANAGER_NODES_SETTING.getKey() + "] is empty on this node";
} else {
bootstrappingDescription = String.format(
Locale.ROOT,
"this node must discover master-eligible nodes %s to bootstrap a cluster",
INITIAL_MASTER_NODES_SETTING.get(settings)
INITIAL_CLUSTER_MANAGER_NODES_SETTING.get(settings)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,8 @@ public void apply(Settings value, Settings current, Settings previous) {
LeaderChecker.LEADER_CHECK_RETRY_COUNT_SETTING,
Reconfigurator.CLUSTER_AUTO_SHRINK_VOTING_CONFIGURATION,
TransportAddVotingConfigExclusionsAction.MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING,
ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING,
ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING, // deprecated
ClusterBootstrapService.INITIAL_CLUSTER_MANAGER_NODES_SETTING,
ClusterBootstrapService.UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING,
LagDetector.CLUSTER_FOLLOWER_LAG_TIMEOUT_SETTING,
HandshakingTransportAddressConnector.PROBE_CONNECT_TIMEOUT_SETTING,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,7 @@ public void testDiscoveryConfiguredCheck() throws NodeValidationException {
hasToString(
containsString(
"the default discovery settings are unsuitable for production use; at least one "
+ "of [discovery.seed_hosts, discovery.seed_providers, cluster.initial_master_nodes] must be configured"
+ "of [discovery.seed_hosts, discovery.seed_providers, cluster.initial_cluster_manager_nodes / cluster.initial_master_nodes] must be configured"
)
)
);
Expand All @@ -815,7 +815,7 @@ public void testDiscoveryConfiguredCheck() throws NodeValidationException {
BootstrapChecks.check(context, true, checks);
};

ensureChecksPass.accept(Settings.builder().putList(ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING.getKey()));
ensureChecksPass.accept(Settings.builder().putList(ClusterBootstrapService.INITIAL_CLUSTER_MANAGER_NODES_SETTING.getKey()));
ensureChecksPass.accept(Settings.builder().putList(DiscoveryModule.DISCOVERY_SEED_PROVIDERS_SETTING.getKey()));
ensureChecksPass.accept(Settings.builder().putList(SettingsBasedSeedHostsProvider.DISCOVERY_SEED_HOSTS_SETTING.getKey()));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* 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.cluster.coordination;

import org.opensearch.common.settings.ClusterSettings;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
import org.opensearch.test.OpenSearchTestCase;

import java.util.Arrays;
import java.util.Set;

/**
* A unit test to validate the former name of the setting 'cluster.initial_cluster_manager_nodes' still take effect,
* after it is deprecated, so that the backwards compatibility is maintained.
* The test can be removed along with removing support of the deprecated setting.
*/
public class ClusterBootstrapServiceRenamedSettingTests extends OpenSearchTestCase {
/**
* Validate the both settings are known and supported.
*/
public void testReindexSettingsExist() {
Set<Setting<?>> settings = ClusterSettings.BUILT_IN_CLUSTER_SETTINGS;
assertTrue(
"Both 'cluster.initial_cluster_manager_nodes' and its predecessor should be supported built-in settings.",
settings.containsAll(
Arrays.asList(
ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING,
ClusterBootstrapService.INITIAL_CLUSTER_MANAGER_NODES_SETTING
)
)
);
}

/**
* Validate the default value of the both settings is the same.
*/
public void testSettingFallback() {
assertEquals(
ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING.get(Settings.EMPTY),
ClusterBootstrapService.INITIAL_CLUSTER_MANAGER_NODES_SETTING.get(Settings.EMPTY)
);
}

/**
* Validate the new setting can be configured correctly, and it doesn't impact the old setting.
*/
public void testSettingGetValue() {
Settings settings = Settings.builder().put("cluster.initial_cluster_manager_nodes", "node-a").build();
assertEquals(Arrays.asList("node-a"), ClusterBootstrapService.INITIAL_CLUSTER_MANAGER_NODES_SETTING.get(settings));
assertEquals(
ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING.getDefault(Settings.EMPTY),
ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING.get(settings)
);
}

/**
* Validate the value of the old setting will be applied to the new setting, if the new setting is not configured.
*/
public void testSettingGetValueWithFallback() {
Settings settings = Settings.builder().put("cluster.initial_master_nodes", "node-a").build();
assertEquals(Arrays.asList("node-a"), ClusterBootstrapService.INITIAL_CLUSTER_MANAGER_NODES_SETTING.get(settings));
assertSettingDeprecationsAndWarnings(new Setting<?>[] { ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING });
}

/**
* Validate the value of the old setting will be ignored, if the new setting is configured.
*/
public void testSettingGetValueWhenBothAreConfigured() {
Settings settings = Settings.builder()
.put("cluster.initial_cluster_manager_nodes", "node-a")
.put("cluster.initial_master_nodes", "node-a, node-b")
.build();
assertEquals(Arrays.asList("node-a"), ClusterBootstrapService.INITIAL_CLUSTER_MANAGER_NODES_SETTING.get(settings));
assertEquals(Arrays.asList("node-a", "node-b"), ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING.get(settings));
assertSettingDeprecationsAndWarnings(new Setting<?>[] { ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING });
}
}
Loading

0 comments on commit 19eadb4

Please sign in to comment.