Skip to content

Commit 28b22df

Browse files
probakowskiprobakowski
committed
Enable GeoIP downloader by default (elastic#71505)
This change enables GeoIP downloader by default. It removes feature flag but adds flag that is used by tests to disable it again (as we don't want to hammer GeoIP database service with every test cluster we spin up). Relates to elastic#68920
1 parent 976cf4b commit 28b22df

File tree

16 files changed

+53
-43
lines changed

16 files changed

+53
-43
lines changed

‎build.gradle‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import org.gradle.util.DistributionLocator
2121
import org.gradle.util.GradleVersion
2222
import static org.elasticsearch.gradle.util.GradleUtils.maybeConfigure
2323
import org.gradle.plugins.ide.eclipse.model.ProjectDependency
24+
import org.elasticsearch.gradle.testclusters.TestClustersPlugin
2425

2526
plugins {
2627
id 'lifecycle-base'
@@ -511,3 +512,11 @@ subprojects {
511512
}
512513
}
513514
}
515+
516+
subprojects { Project subproj ->
517+
plugins.withType(TestClustersPlugin).whenPluginAdded {
518+
testClusters.all {
519+
systemProperty "geoip.downloader.enabled.default", "false"
520+
}
521+
}
522+
}

‎client/rest-high-level/build.gradle‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,6 @@ tasks.named("check").configure {
8585
testClusters.all {
8686
testDistribution = 'DEFAULT'
8787
systemProperty 'es.scripting.update.ctx_in_params', 'false'
88-
systemProperty 'es.geoip_v2_feature_flag_enabled', 'true'
89-
setting 'geoip.downloader.enabled', 'false'
9088
setting 'reindex.remote.whitelist', '[ "[::1]:*", "127.0.0.1:*" ]'
9189
setting 'xpack.license.self_generated.type', 'trial'
9290
setting 'xpack.security.enabled', 'true'

‎distribution/docker/docker-compose.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ services:
1616
- cluster.routing.allocation.disk.watermark.high=1b
1717
- cluster.routing.allocation.disk.watermark.flood_stage=1b
1818
- node.store.allow_mmap=false
19+
- geoip.downloader.enabled=false
1920
- xpack.security.enabled=true
2021
- xpack.security.transport.ssl.enabled=true
2122
- xpack.security.http.ssl.enabled=true
@@ -59,6 +60,7 @@ services:
5960
- cluster.routing.allocation.disk.watermark.high=1b
6061
- cluster.routing.allocation.disk.watermark.flood_stage=1b
6162
- node.store.allow_mmap=false
63+
- geoip.downloader.enabled=false
6264
- xpack.security.enabled=true
6365
- xpack.security.transport.ssl.enabled=true
6466
- xpack.security.http.ssl.enabled=true

‎modules/ingest-geoip/build.gradle‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,11 @@ if (useFixture) {
5151
}
5252

5353
tasks.named("internalClusterTest").configure {
54-
systemProperty "es.geoip_v2_feature_flag_enabled", "true"
5554
if (useFixture) {
5655
nonInputProperties.systemProperty "geoip_endpoint", "${-> fixtureAddress()}"
5756
}
5857
}
5958

60-
testClusters.all {
61-
systemProperty "es.geoip_v2_feature_flag_enabled", "true"
62-
setting "geoip.downloader.enabled", "false"
63-
}
64-
6559
tasks.register("copyDefaultGeoIp2DatabaseFiles", Copy) {
6660
from { zipTree(configurations.testCompileClasspath.files.find { it.name.contains('geolite2-databases') }) }
6761
into "${project.buildDir}/ingest-geoip"

‎modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpDownloader.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,6 @@ public class GeoIpDownloader extends AllocatedPersistentTask {
5858

5959
private static final Logger logger = LogManager.getLogger(GeoIpDownloader.class);
6060

61-
public static final boolean GEOIP_V2_FEATURE_FLAG_ENABLED = "true".equals(System.getProperty("es.geoip_v2_feature_flag_enabled"));
62-
6361
public static final Setting<TimeValue> POLL_INTERVAL_SETTING = Setting.timeSetting("geoip.downloader.poll.interval",
6462
TimeValue.timeValueDays(3), TimeValue.timeValueDays(1), Property.Dynamic, Property.NodeScope);
6563
public static final Setting<String> ENDPOINT_SETTING = Setting.simpleString("geoip.downloader.endpoint",

‎modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderTaskExecutor.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,15 @@
3030
import java.util.concurrent.atomic.AtomicReference;
3131

3232
import static org.elasticsearch.ingest.geoip.GeoIpDownloader.GEOIP_DOWNLOADER;
33-
import static org.elasticsearch.ingest.geoip.GeoIpDownloader.GEOIP_V2_FEATURE_FLAG_ENABLED;
3433

3534
/**
3635
* Persistent task executor that is responsible for starting {@link GeoIpDownloader} after task is allocated by master node.
3736
* Also bootstraps GeoIP download task on clean cluster and handles changes to the 'geoip.downloader.enabled' setting
3837
*/
3938
public final class GeoIpDownloaderTaskExecutor extends PersistentTasksExecutor<GeoIpTaskParams> implements ClusterStateListener {
4039

41-
public static final Setting<Boolean> ENABLED_SETTING = Setting.boolSetting("geoip.downloader.enabled", GEOIP_V2_FEATURE_FLAG_ENABLED,
40+
private static final boolean ENABLED_DEFAULT = "false".equals(System.getProperty("geoip.downloader.enabled.default")) == false;
41+
public static final Setting<Boolean> ENABLED_SETTING = Setting.boolSetting("geoip.downloader.enabled", ENABLED_DEFAULT,
4242
Setting.Property.Dynamic, Setting.Property.NodeScope);
4343

4444
private static final Logger logger = LogManager.getLogger(GeoIpDownloader.class);

‎modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/IngestGeoIpPlugin.java‎

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@
5555
import java.io.Closeable;
5656
import java.io.IOException;
5757
import java.io.UncheckedIOException;
58-
import java.util.ArrayList;
5958
import java.util.Arrays;
6059
import java.util.Collection;
6160
import java.util.Collections;
@@ -67,7 +66,6 @@
6766
import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME;
6867
import static org.elasticsearch.ingest.geoip.GeoIpDownloader.DATABASES_INDEX;
6968
import static org.elasticsearch.ingest.geoip.GeoIpDownloader.GEOIP_DOWNLOADER;
70-
import static org.elasticsearch.ingest.geoip.GeoIpDownloader.GEOIP_V2_FEATURE_FLAG_ENABLED;
7169

7270
public class IngestGeoIpPlugin extends Plugin implements IngestPlugin, SystemIndexPlugin, Closeable, PersistentTaskPlugin, ActionPlugin {
7371
public static final Setting<Long> CACHE_SIZE =
@@ -81,13 +79,8 @@ public class IngestGeoIpPlugin extends Plugin implements IngestPlugin, SystemInd
8179

8280
@Override
8381
public List<Setting<?>> getSettings() {
84-
List<Setting<?>> settings = new ArrayList<>(Arrays.asList(CACHE_SIZE,
85-
GeoIpDownloader.ENDPOINT_SETTING,
86-
GeoIpDownloader.POLL_INTERVAL_SETTING));
87-
if (GEOIP_V2_FEATURE_FLAG_ENABLED) {
88-
settings.add(GeoIpDownloaderTaskExecutor.ENABLED_SETTING);
89-
}
90-
return settings;
82+
return Arrays.asList(CACHE_SIZE, GeoIpDownloader.ENDPOINT_SETTING, GeoIpDownloader.POLL_INTERVAL_SETTING,
83+
GeoIpDownloaderTaskExecutor.ENABLED_SETTING);
9184
}
9285

9386
@Override
@@ -119,11 +112,9 @@ public Collection<Object> createComponents(Client client,
119112
} catch (IOException e) {
120113
throw new UncheckedIOException(e);
121114
}
122-
if (GEOIP_V2_FEATURE_FLAG_ENABLED) {
123-
geoIpDownloaderTaskExecutor = new GeoIpDownloaderTaskExecutor(client, new HttpClient(), clusterService, threadPool);
124-
return Arrays.asList(databaseRegistry.get(), geoIpDownloaderTaskExecutor);
125-
}
126-
return Collections.singletonList(databaseRegistry.get());
115+
116+
geoIpDownloaderTaskExecutor = new GeoIpDownloaderTaskExecutor(client, new HttpClient(), clusterService, threadPool);
117+
return Arrays.asList(databaseRegistry.get(), geoIpDownloaderTaskExecutor);
127118
}
128119

129120
@Override
@@ -135,31 +126,21 @@ public void close() throws IOException {
135126
public List<PersistentTasksExecutor<?>> getPersistentTasksExecutor(ClusterService clusterService, ThreadPool threadPool,
136127
Client client, SettingsModule settingsModule,
137128
IndexNameExpressionResolver expressionResolver) {
138-
if (GEOIP_V2_FEATURE_FLAG_ENABLED) {
139-
return Collections.singletonList(geoIpDownloaderTaskExecutor);
140-
} else {
141-
return Collections.emptyList();
142-
}
129+
return Collections.singletonList(geoIpDownloaderTaskExecutor);
143130
}
144131

145132
@Override
146133
public List<ActionHandler<? extends ActionRequest, ? extends ActionResponse>> getActions() {
147-
if (GEOIP_V2_FEATURE_FLAG_ENABLED) {
148-
return Collections.singletonList(new ActionHandler<>(GeoIpDownloaderStatsAction.INSTANCE,
149-
GeoIpDownloaderStatsTransportAction.class));
150-
}
151-
return Collections.emptyList();
134+
return Collections.singletonList(new ActionHandler<>(GeoIpDownloaderStatsAction.INSTANCE,
135+
GeoIpDownloaderStatsTransportAction.class));
152136
}
153137

154138
@Override
155139
public List<RestHandler> getRestHandlers(Settings settings, RestController restController, ClusterSettings clusterSettings,
156140
IndexScopedSettings indexScopedSettings, SettingsFilter settingsFilter,
157141
IndexNameExpressionResolver indexNameExpressionResolver,
158142
Supplier<DiscoveryNodes> nodesInCluster) {
159-
if (GEOIP_V2_FEATURE_FLAG_ENABLED) {
160-
return Collections.singletonList(new RestGeoIpDownloaderStatsAction());
161-
}
162-
return Collections.emptyList();
143+
return Collections.singletonList(new RestGeoIpDownloaderStatsAction());
163144
}
164145

165146
@Override

‎qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import static org.elasticsearch.packaging.util.FileUtils.append;
3131
import static org.elasticsearch.packaging.util.FileUtils.mv;
3232
import static org.elasticsearch.packaging.util.FileUtils.rm;
33+
import static org.elasticsearch.packaging.util.ServerUtils.disableGeoIpDownloader;
3334
import static org.elasticsearch.packaging.util.ServerUtils.makeRequest;
3435
import static org.hamcrest.CoreMatchers.containsString;
3536
import static org.hamcrest.CoreMatchers.equalTo;
@@ -51,6 +52,7 @@ public static void filterDistros() {
5152
public void test10Install() throws Exception {
5253
installation = installArchive(sh, distribution());
5354
verifyArchiveInstallation(installation, distribution());
55+
disableGeoIpDownloader(installation);
5456
}
5557

5658
public void test20PluginsListWithNoPlugins() throws Exception {

‎qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public static void filterDistros() {
9292

9393
@Before
9494
public void setupTest() throws IOException {
95-
installation = runContainer(distribution());
95+
installation = runContainer(distribution(), builder().envVars(Map.of("geoip.downloader.enabled", "false")));
9696
tempDir = createTempDir(DockerTests.class.getSimpleName());
9797
}
9898

‎qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import static org.elasticsearch.packaging.util.Packages.assertRemoved;
5050
import static org.elasticsearch.packaging.util.Packages.installPackage;
5151
import static org.elasticsearch.packaging.util.Packages.verifyPackageInstallation;
52+
import static org.elasticsearch.packaging.util.ServerUtils.disableGeoIpDownloader;
5253
import static org.hamcrest.CoreMatchers.containsString;
5354
import static org.hamcrest.CoreMatchers.is;
5455
import static org.hamcrest.CoreMatchers.notNullValue;
@@ -71,6 +72,8 @@ public void test10InstallArchiveDistribution() throws Exception {
7172
installation = installArchive(sh, distribution);
7273
verifyArchiveInstallation(installation, distribution());
7374

75+
disableGeoIpDownloader(installation);
76+
7477
final Installation.Executables bin = installation.executables();
7578
Shell.Result r = sh.runIgnoreExitCode(bin.keystoreTool.toString() + " has-passwd");
7679
assertFalse("has-passwd should fail", r.isSuccess());
@@ -85,6 +88,7 @@ public void test11InstallPackageDistribution() throws Exception {
8588
installation = installPackage(sh, distribution);
8689
assertInstalled(distribution);
8790
verifyPackageInstallation(installation, distribution, sh);
91+
disableGeoIpDownloader(installation);
8892

8993
final Installation.Executables bin = installation.executables();
9094
Shell.Result r = sh.runIgnoreExitCode(bin.keystoreTool.toString() + " has-passwd");
@@ -98,7 +102,7 @@ public void test11InstallPackageDistribution() throws Exception {
98102
public void test12InstallDockerDistribution() throws Exception {
99103
assumeTrue(distribution().isDocker());
100104

101-
installation = Docker.runContainer(distribution());
105+
installation = Docker.runContainer(distribution(), builder().envVars(Map.of("geoip.downloader.enabled", "false")));
102106

103107
try {
104108
waitForPathToExist(installation.config("elasticsearch.keystore"));
@@ -300,7 +304,9 @@ public void test60DockerEnvironmentVariablePassword() throws Exception {
300304

301305
// restart ES with password and mounted keystore
302306
Map<Path, Path> volumes = singletonMap(localKeystoreFile, dockerKeystore);
303-
Map<String, String> envVars = singletonMap("KEYSTORE_PASSWORD", password);
307+
Map<String, String> envVars = new HashMap<>();
308+
envVars.put("KEYSTORE_PASSWORD", password);
309+
envVars.put("geoip.downloader.enabled", "false");
304310
runContainer(distribution(), builder().volumes(volumes).envVars(envVars));
305311
waitForElasticsearch(installation);
306312
ServerUtils.runElasticsearchTests();
@@ -333,6 +339,7 @@ public void test61DockerEnvironmentVariablePasswordFromFile() throws Exception {
333339

334340
Map<String, String> envVars = new HashMap<>();
335341
envVars.put("KEYSTORE_PASSWORD_FILE", "/run/secrets/" + passwordFilename);
342+
envVars.put("geoip.downloader.enabled", "false");
336343

337344
runContainer(distribution(), builder().volumes(volumes).envVars(envVars));
338345

0 commit comments

Comments
 (0)