Skip to content

Commit bb1ca00

Browse files
jasontedorkcm
jasontedor
authored and
kcm
committed
Fix remote cluster seeds fallback (#34090)
Recently we introduced the settings cluster.remote to take the place of search.remote for configuring remote cluster connections. We made this change due to the fact that we have generalized the remote cluster infrastructure to also be used within cross-cluster replication and not only cross-cluster search. For backwards compatibility, when we made this change, we allowed that cluster.remote would fallback to search.remote. Alas, the initial change for this contained a bug for handling the proxy and seeds settings. The bug for the seeds settings arose because we were manually iterating over the concrete settings only for cluster.remote seeds but not for search.remote seeds. This commit addresses this by iterating over both cluster.remote seeds and search.remote seeds. Additionally, when checking for existence of proxy settings, we have to not only check cluster.remote proxy settings, but also fallback to search.remote proxy settings. This commit addresses both issues, and adds tests for these situations.
1 parent b80763b commit bb1ca00

File tree

2 files changed

+83
-15
lines changed

2 files changed

+83
-15
lines changed

‎server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,16 @@
3636
import java.net.InetSocketAddress;
3737
import java.net.UnknownHostException;
3838
import java.util.ArrayList;
39+
import java.util.Arrays;
3940
import java.util.Collections;
4041
import java.util.EnumSet;
4142
import java.util.HashMap;
4243
import java.util.List;
44+
import java.util.Locale;
4345
import java.util.Map;
46+
import java.util.NavigableSet;
4447
import java.util.Set;
48+
import java.util.TreeSet;
4549
import java.util.function.Predicate;
4650
import java.util.function.Supplier;
4751
import java.util.stream.Collectors;
@@ -181,12 +185,36 @@ protected RemoteClusterAware(Settings settings) {
181185
* {@link TransportAddress#META_ADDRESS} and their configured address will be used as the hostname for the generated discovery node.
182186
*/
183187
protected static Map<String, Tuple<String, List<Supplier<DiscoveryNode>>>> buildRemoteClustersDynamicConfig(Settings settings) {
184-
Stream<Setting<List<String>>> allConcreteSettings = REMOTE_CLUSTERS_SEEDS.getAllConcreteSettings(settings);
188+
final Map<String, Tuple<String, List<Supplier<DiscoveryNode>>>> remoteSeeds =
189+
buildRemoteClustersDynamicConfig(settings, REMOTE_CLUSTERS_SEEDS);
190+
final Map<String, Tuple<String, List<Supplier<DiscoveryNode>>>> searchRemoteSeeds =
191+
buildRemoteClustersDynamicConfig(settings, SEARCH_REMOTE_CLUSTERS_SEEDS);
192+
// sort the intersection for predictable output order
193+
final NavigableSet<String> intersection =
194+
new TreeSet<>(Arrays.asList(
195+
searchRemoteSeeds.keySet().stream().filter(s -> remoteSeeds.keySet().contains(s)).sorted().toArray(String[]::new)));
196+
if (intersection.isEmpty() == false) {
197+
final String message = String.format(
198+
Locale.ROOT,
199+
"found duplicate remote cluster configurations for cluster alias%s [%s]",
200+
intersection.size() == 1 ? "" : "es",
201+
String.join(",", intersection));
202+
throw new IllegalArgumentException(message);
203+
}
204+
return Stream
205+
.concat(remoteSeeds.entrySet().stream(), searchRemoteSeeds.entrySet().stream())
206+
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
207+
}
208+
209+
private static Map<String, Tuple<String, List<Supplier<DiscoveryNode>>>> buildRemoteClustersDynamicConfig(
210+
final Settings settings, final Setting.AffixSetting<List<String>> seedsSetting) {
211+
final Stream<Setting<List<String>>> allConcreteSettings = seedsSetting.getAllConcreteSettings(settings);
185212
return allConcreteSettings.collect(
186-
Collectors.toMap(REMOTE_CLUSTERS_SEEDS::getNamespace, concreteSetting -> {
187-
String clusterName = REMOTE_CLUSTERS_SEEDS.getNamespace(concreteSetting);
213+
Collectors.toMap(seedsSetting::getNamespace, concreteSetting -> {
214+
String clusterName = seedsSetting.getNamespace(concreteSetting);
188215
List<String> addresses = concreteSetting.get(settings);
189-
final boolean proxyMode = REMOTE_CLUSTERS_PROXY.getConcreteSettingForNamespace(clusterName).exists(settings);
216+
final boolean proxyMode =
217+
REMOTE_CLUSTERS_PROXY.getConcreteSettingForNamespace(clusterName).existsOrFallbackExists(settings);
190218
List<Supplier<DiscoveryNode>> nodes = new ArrayList<>(addresses.size());
191219
for (String address : addresses) {
192220
nodes.add(() -> buildSeedNode(clusterName, address, proxyMode));

‎server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java‎

Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,10 @@
6262
import static org.hamcrest.CoreMatchers.containsString;
6363
import static org.hamcrest.CoreMatchers.instanceOf;
6464
import static org.hamcrest.Matchers.anyOf;
65+
import static org.hamcrest.Matchers.containsInAnyOrder;
6566
import static org.hamcrest.Matchers.equalTo;
67+
import static org.hamcrest.Matchers.hasSize;
68+
import static org.hamcrest.Matchers.hasToString;
6669
import static org.hamcrest.Matchers.startsWith;
6770

6871
public class RemoteClusterServiceTests extends ESTestCase {
@@ -120,17 +123,19 @@ public void testRemoteClusterSeedSetting() {
120123

121124
public void testBuildRemoteClustersDynamicConfig() throws Exception {
122125
Map<String, Tuple<String, List<Supplier<DiscoveryNode>>>> map = RemoteClusterService.buildRemoteClustersDynamicConfig(
123-
Settings.builder().put("cluster.remote.foo.seeds", "192.168.0.1:8080")
124-
.put("cluster.remote.bar.seeds", "[::1]:9090")
125-
.put("cluster.remote.boom.seeds", "boom-node1.internal:1000")
126-
.put("cluster.remote.boom.proxy", "foo.bar.com:1234").build());
127-
assertEquals(3, map.size());
128-
assertTrue(map.containsKey("foo"));
129-
assertTrue(map.containsKey("bar"));
130-
assertTrue(map.containsKey("boom"));
131-
assertEquals(1, map.get("foo").v2().size());
132-
assertEquals(1, map.get("bar").v2().size());
133-
assertEquals(1, map.get("boom").v2().size());
126+
Settings.builder()
127+
.put("cluster.remote.foo.seeds", "192.168.0.1:8080")
128+
.put("cluster.remote.bar.seeds", "[::1]:9090")
129+
.put("cluster.remote.boom.seeds", "boom-node1.internal:1000")
130+
.put("cluster.remote.boom.proxy", "foo.bar.com:1234")
131+
.put("search.remote.quux.seeds", "quux:9300")
132+
.put("search.remote.quux.proxy", "quux-proxy:19300")
133+
.build());
134+
assertThat(map.keySet(), containsInAnyOrder(equalTo("foo"), equalTo("bar"), equalTo("boom"), equalTo("quux")));
135+
assertThat(map.get("foo").v2(), hasSize(1));
136+
assertThat(map.get("bar").v2(), hasSize(1));
137+
assertThat(map.get("boom").v2(), hasSize(1));
138+
assertThat(map.get("quux").v2(), hasSize(1));
134139

135140
DiscoveryNode foo = map.get("foo").v2().get(0).get();
136141
assertEquals("", map.get("foo").v1());
@@ -150,6 +155,41 @@ public void testBuildRemoteClustersDynamicConfig() throws Exception {
150155
assertEquals(boom.getId(), "boom#boom-node1.internal:1000");
151156
assertEquals("foo.bar.com:1234", map.get("boom").v1());
152157
assertEquals(boom.getVersion(), Version.CURRENT.minimumCompatibilityVersion());
158+
159+
DiscoveryNode quux = map.get("quux").v2().get(0).get();
160+
assertEquals(quux.getAddress(), new TransportAddress(TransportAddress.META_ADDRESS, 0));
161+
assertEquals("quux", quux.getHostName());
162+
assertEquals(quux.getId(), "quux#quux:9300");
163+
assertEquals("quux-proxy:19300", map.get("quux").v1());
164+
assertEquals(quux.getVersion(), Version.CURRENT.minimumCompatibilityVersion());
165+
166+
assertSettingDeprecationsAndWarnings(new String[]{"search.remote.quux.seeds", "search.remote.quux.proxy"});
167+
}
168+
169+
public void testBuildRemoteClustersDynamicConfigWithDuplicate() {
170+
final IllegalArgumentException e = expectThrows(
171+
IllegalArgumentException.class,
172+
() -> RemoteClusterService.buildRemoteClustersDynamicConfig(
173+
Settings.builder()
174+
.put("cluster.remote.foo.seeds", "192.168.0.1:8080")
175+
.put("search.remote.foo.seeds", "192.168.0.1:8080")
176+
.build()));
177+
assertThat(e, hasToString(containsString("found duplicate remote cluster configurations for cluster alias [foo]")));
178+
assertSettingDeprecationsAndWarnings(new String[]{"search.remote.foo.seeds"});
179+
}
180+
181+
public void testBuildRemoteClustersDynamicConfigWithDuplicates() {
182+
final IllegalArgumentException e = expectThrows(
183+
IllegalArgumentException.class,
184+
() -> RemoteClusterService.buildRemoteClustersDynamicConfig(
185+
Settings.builder()
186+
.put("cluster.remote.foo.seeds", "192.168.0.1:8080")
187+
.put("search.remote.foo.seeds", "192.168.0.1:8080")
188+
.put("cluster.remote.bar.seeds", "192.168.0.1:8080")
189+
.put("search.remote.bar.seeds", "192.168.0.1:8080")
190+
.build()));
191+
assertThat(e, hasToString(containsString("found duplicate remote cluster configurations for cluster aliases [bar,foo]")));
192+
assertSettingDeprecationsAndWarnings(new String[]{"search.remote.bar.seeds", "search.remote.foo.seeds"});
153193
}
154194

155195
public void testGroupClusterIndices() throws IOException {

0 commit comments

Comments
 (0)