Elasticsearch: Simplify configuration of servers
Instead of configuring each server in its own section with separate values for protocol, host and port, i.e.: [elasticsearch "elastic1"] hostname = elastic1 protocol = https port = 1234 [elasticsearch "elastic2"] hostname = elastic2 protocol = https port = 1234 Configure them all under the main "elasticsearch" section as multiple values, i.e. [elasticsearch] server = https://elastic1:1234 server = https://elastic2:1234 Allow the port to be omitted, and default it to 9200. Require at least one server to be explicitly configured and throw a provision exception if there are none. During init prompt for the server, defaulting to http://localhost:9200. Since the init framework doesn't support reading or setting string lists, if there are multiple server values it will prompt the last one. Mention in the documentation that configuration of multiple servers must be done manually. Since it is not expected that Elasticsearch is being used in production anywhere, no backwards compatibility with the previous configuration is provided. Users must adjust their configuration to the new format. Add a new test, ElasticConfigurationTest, which in this initial version only tests the configuration of the elasticsearch.server. More tests may be added in follow-up commits. Bug: Issue 9372 Bug: Issue 9383 Change-Id: Iad2c547ae82ba89b7ede777072e0869bc77d2025
This commit is contained in:
@@ -0,0 +1,90 @@
|
||||
// Copyright (C) 2018 The Android Open Source Project
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package com.google.gerrit.elasticsearch;
|
||||
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static java.util.stream.Collectors.toList;
|
||||
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.inject.ProvisionException;
|
||||
import java.util.Arrays;
|
||||
import org.eclipse.jgit.lib.Config;
|
||||
import org.junit.Rule;
|
||||
import org.junit.Test;
|
||||
import org.junit.rules.ExpectedException;
|
||||
|
||||
public class ElasticConfigurationTest {
|
||||
@Rule public ExpectedException exception = ExpectedException.none();
|
||||
|
||||
@Test
|
||||
public void singleServer() throws Exception {
|
||||
Config cfg = new Config();
|
||||
cfg.setString("elasticsearch", null, "server", "http://elastic:1234");
|
||||
ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
|
||||
assertHosts(esCfg, "http://elastic:1234");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void serverWithoutPortSpecified() throws Exception {
|
||||
Config cfg = new Config();
|
||||
cfg.setString("elasticsearch", null, "server", "http://elastic");
|
||||
ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
|
||||
assertHosts(esCfg, "http://elastic:9200");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void multipleServers() throws Exception {
|
||||
Config cfg = new Config();
|
||||
cfg.setStringList(
|
||||
"elasticsearch",
|
||||
null,
|
||||
"server",
|
||||
ImmutableList.of("http://elastic1:1234", "http://elastic2:1234"));
|
||||
ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
|
||||
assertHosts(esCfg, "http://elastic1:1234", "http://elastic2:1234");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void noServers() throws Exception {
|
||||
assertProvisionException(new Config());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void singleServerInvalid() throws Exception {
|
||||
Config cfg = new Config();
|
||||
cfg.setString("elasticsearch", null, "server", "foo");
|
||||
assertProvisionException(cfg);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void multipleServersIncludingInvalid() throws Exception {
|
||||
Config cfg = new Config();
|
||||
cfg.setStringList(
|
||||
"elasticsearch", null, "server", ImmutableList.of("http://elastic1:1234", "foo"));
|
||||
ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
|
||||
assertHosts(esCfg, "http://elastic1:1234");
|
||||
}
|
||||
|
||||
private void assertHosts(ElasticConfiguration cfg, Object... hostURIs) throws Exception {
|
||||
assertThat(Arrays.asList(cfg.getHosts()).stream().map(h -> h.toURI()).collect(toList()))
|
||||
.containsExactly(hostURIs);
|
||||
}
|
||||
|
||||
private void assertProvisionException(Config cfg) throws Exception {
|
||||
exception.expect(ProvisionException.class);
|
||||
exception.expectMessage("No valid Elasticsearch servers configured");
|
||||
new ElasticConfiguration(cfg);
|
||||
}
|
||||
}
|
Reference in New Issue
Block a user