-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Package ingest-geoip as a module #36898
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4708fa1
a283a66
7c52703
147e7e5
503b055
1b37051
1c10cd7
b77e36b
029ff49
ce5320e
30d92d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -222,6 +222,10 @@ void execute(Terminal terminal, String pluginId, boolean isBatch, Environment en | |
throw new UserException(ExitCodes.USAGE, "plugin id is required"); | ||
} | ||
|
||
if ("ingest-geoip".equals(pluginId)) { | ||
handleInstallIngestGeoIp(); | ||
} | ||
|
||
if ("x-pack".equals(pluginId)) { | ||
handleInstallXPack(buildFlavor()); | ||
} | ||
|
@@ -231,6 +235,12 @@ void execute(Terminal terminal, String pluginId, boolean isBatch, Environment en | |
install(terminal, isBatch, extractedZip, env); | ||
} | ||
|
||
private static void handleInstallIngestGeoIp() throws UserException { | ||
throw new UserException( | ||
ExitCodes.OK, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is deliberately status code zero so as to avoid failing automation that relies on installing this plugin during deployment. |
||
"ingest-geoip is no longer a plugin but instead a module packaged with this distribution of Elasticsearch"); | ||
} | ||
|
||
Build.Flavor buildFlavor() { | ||
return Build.CURRENT.flavor(); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ | |
import org.elasticsearch.env.TestEnvironment; | ||
import org.elasticsearch.test.ESTestCase; | ||
import org.elasticsearch.test.VersionUtils; | ||
import org.hamcrest.Matchers; | ||
import org.junit.Before; | ||
|
||
import java.io.BufferedReader; | ||
|
@@ -251,6 +252,33 @@ public void testMissingPluginName() throws Exception { | |
assertEquals("plugin name is required", e.getMessage()); | ||
} | ||
|
||
/** | ||
* The ingest-geoip plugin receives special handling because we have re-packaged it as a module; this test ensures that we are still | ||
* able to uninstall an old installation of ingest-geoip. | ||
* | ||
* @throws Exception if an exception is thrown creating or removing the plugin | ||
*/ | ||
public void testRemoveIngestGeoIp() throws Exception { | ||
createPlugin( | ||
"ingest-geoip", | ||
VersionUtils.randomVersionBetween( | ||
random(), | ||
Version.CURRENT.minimumIndexCompatibilityVersion(), | ||
Version.V_6_6_0)); | ||
removePlugin("ingest-geoip", home, randomBoolean()); | ||
assertThat(Files.exists(env.pluginsFile().resolve("ingest-geoip")), equalTo(false)); | ||
assertRemoveCleaned(env); | ||
} | ||
|
||
public void testRemoveIngestGeoIpWhenNotInstalled() { | ||
final UserException e = expectThrows(UserException.class, () -> removePlugin("ingest-geoip", home, randomBoolean())); | ||
assertThat(e.exitCode, equalTo(ExitCodes.OK)); | ||
assertThat( | ||
e, | ||
hasToString(Matchers.containsString( | ||
"ingest-geoip is no longer a plugin but instead a module packaged with this distribution of Elasticsearch"))); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please add a test here for the case the plugin is installed and it still gets removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I pushed 503b055. |
||
|
||
public void testRemoveWhenRemovingMarker() throws Exception { | ||
createPlugin("fake"); | ||
Files.createFile(env.pluginsFile().resolve("fake").resolve("plugin.jar")); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change will be backported to 6.x to avoid breaking users there, and a note will be added to the migration guide. A follow-up will remove this handling in master so that 7.x will indeed report the usual error message.