diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java index ae2f330b717..b2cea4c0ad2 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -52,6 +52,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; @@ -60,6 +61,7 @@ import java.util.Objects; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.elasticsearch.common.io.FileSystemUtils.isAccessibleDirectory; @@ -285,6 +287,27 @@ public class PluginsService extends AbstractComponent { return bundles; } + static void checkForFailedPluginRemovals(final Path pluginsDirectory) throws IOException { + /* + * Check for the existence of a marker file that indicates any plugins are in a garbage state from a failed attempt to remove the + * plugin. + */ + try (DirectoryStream stream = Files.newDirectoryStream(pluginsDirectory, ".removing-*")) { + final Iterator iterator = stream.iterator(); + if (iterator.hasNext()) { + final Path removing = iterator.next(); + final String fileName = removing.getFileName().toString(); + final String name = fileName.substring(1 + fileName.indexOf("-")); + final String message = String.format( + Locale.ROOT, + "found file [%s] from a failed attempt to remove the plugin [%s]; execute [elasticsearch-plugin remove %2$s]", + removing, + name); + throw new IllegalStateException(message); + } + } + } + static Set getPluginBundles(Path pluginsDirectory) throws IOException { Logger logger = Loggers.getLogger(PluginsService.class); @@ -295,6 +318,8 @@ public class PluginsService extends AbstractComponent { Set bundles = new LinkedHashSet<>(); + checkForFailedPluginRemovals(pluginsDirectory); + try (DirectoryStream stream = Files.newDirectoryStream(pluginsDirectory)) { for (Path plugin : stream) { logger.trace("--- adding plugin [{}]", plugin.toAbsolutePath()); @@ -305,19 +330,6 @@ public class PluginsService extends AbstractComponent { throw new IllegalStateException("Could not load plugin descriptor for existing plugin [" + plugin.getFileName() + "]. Was the plugin built before 2.0?", e); } - /* - * Check for the existence of a marker file that indicates the plugin is in a garbage state from a failed attempt to remove - * the plugin. - */ - final Path removing = plugin.resolve(".removing-" + info.getName()); - if (Files.exists(removing)) { - final String message = String.format( - Locale.ROOT, - "found file [%s] from a failed attempt to remove the plugin [%s]; execute [elasticsearch-plugin remove %2$s]", - removing, - info.getName()); - throw new IllegalStateException(message); - } Set urls = new LinkedHashSet<>(); try (DirectoryStream jarStream = Files.newDirectoryStream(plugin, "*.jar")) { diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java index 89c65ad2c8d..e980081479b 100644 --- a/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java +++ b/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java @@ -132,8 +132,8 @@ public class PluginsServiceTests extends ESTestCase { final Path fake = home.resolve("plugins").resolve("fake"); Files.createDirectories(fake); Files.createFile(fake.resolve("plugin.jar")); - final Path removing = fake.resolve(".removing-fake"); - Files.createFile(fake.resolve(".removing-fake")); + final Path removing = home.resolve("plugins").resolve(".removing-fake"); + Files.createFile(removing); PluginTestUtil.writeProperties( fake, "description", "fake", diff --git a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index 1e44d0522d3..04aa2660e32 100644 --- a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -481,6 +481,8 @@ class InstallPluginCommand extends EnvironmentAwareCommand { throw new UserException(PLUGIN_EXISTS, message); } + PluginsService.checkForFailedPluginRemovals(env.pluginsFile()); + terminal.println(VERBOSE, info.toString()); // don't let user install plugin as a module... diff --git a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java index 1653d663a2f..3cd5a329043 100644 --- a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java +++ b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java @@ -26,7 +26,6 @@ import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.UserException; -import org.elasticsearch.common.Strings; import org.elasticsearch.env.Environment; import java.io.IOException; @@ -34,6 +33,7 @@ import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.stream.Collectors; @@ -46,75 +46,113 @@ import static org.elasticsearch.cli.Terminal.Verbosity.VERBOSE; */ class RemovePluginCommand extends EnvironmentAwareCommand { + private final OptionSpec purgeOption; private final OptionSpec arguments; RemovePluginCommand() { super("removes a plugin from Elasticsearch"); + this.purgeOption = parser.acceptsAll(Arrays.asList("p", "purge"), "Purge plugin configuration files"); this.arguments = parser.nonOptions("plugin name"); } @Override - protected void execute(final Terminal terminal, final OptionSet options, final Environment env) - throws Exception { + protected void execute(final Terminal terminal, final OptionSet options, final Environment env) throws Exception { final String pluginName = arguments.value(options); - execute(terminal, pluginName, env); + final boolean purge = options.has(purgeOption); + execute(terminal, env, pluginName, purge); } /** * Remove the plugin specified by {@code pluginName}. * * @param terminal the terminal to use for input/output - * @param pluginName the name of the plugin to remove * @param env the environment for the local node + * @param pluginName the name of the plugin to remove + * @param purge if true, plugin configuration files will be removed but otherwise preserved * @throws IOException if any I/O exception occurs while performing a file operation * @throws UserException if plugin name is null * @throws UserException if plugin directory does not exist * @throws UserException if the plugin bin directory is not a directory */ - void execute(final Terminal terminal, final String pluginName, final Environment env) - throws IOException, UserException { + void execute( + final Terminal terminal, + final Environment env, + final String pluginName, + final boolean purge) throws IOException, UserException { if (pluginName == null) { throw new UserException(ExitCodes.USAGE, "plugin name is required"); } - terminal.println("-> removing [" + Strings.coalesceToEmpty(pluginName) + "]..."); + terminal.println("-> removing [" + pluginName + "]..."); final Path pluginDir = env.pluginsFile().resolve(pluginName); - if (Files.exists(pluginDir) == false) { + final Path pluginConfigDir = env.configFile().resolve(pluginName); + final Path removing = env.pluginsFile().resolve(".removing-" + pluginName); + /* + * If the plugin does not exist and the plugin config does not exist, fail to the user that the plugin is not found, unless there's + * a marker file left from a previously failed attempt in which case we proceed to clean up the marker file. Or, if the plugin does + * not exist, the plugin config does, and we are not purging, again fail to the user that the plugin is not found. + */ + if ((!Files.exists(pluginDir) && !Files.exists(pluginConfigDir) && !Files.exists(removing)) + || (!Files.exists(pluginDir) && Files.exists(pluginConfigDir) && !purge)) { final String message = String.format( - Locale.ROOT, - "plugin [%s] not found; " - + "run 'elasticsearch-plugin list' to get list of installed plugins", - pluginName); + Locale.ROOT, "plugin [%s] not found; run 'elasticsearch-plugin list' to get list of installed plugins", pluginName); throw new UserException(ExitCodes.CONFIG, message); } final List pluginPaths = new ArrayList<>(); + /* + * Add the contents of the plugin directory before creating the marker file and adding it to the list of paths to be deleted so + * that the marker file is the last file to be deleted. + */ + if (Files.exists(pluginDir)) { + try (Stream paths = Files.list(pluginDir)) { + pluginPaths.addAll(paths.collect(Collectors.toList())); + } + terminal.println(VERBOSE, "removing [" + pluginDir + "]"); + } + final Path pluginBinDir = env.binFile().resolve(pluginName); if (Files.exists(pluginBinDir)) { - if (Files.isDirectory(pluginBinDir) == false) { - throw new UserException( - ExitCodes.IO_ERROR, "bin dir for " + pluginName + " is not a directory"); + if (!Files.isDirectory(pluginBinDir)) { + throw new UserException(ExitCodes.IO_ERROR, "bin dir for " + pluginName + " is not a directory"); + } + try (Stream paths = Files.list(pluginBinDir)) { + pluginPaths.addAll(paths.collect(Collectors.toList())); } pluginPaths.add(pluginBinDir); terminal.println(VERBOSE, "removing [" + pluginBinDir + "]"); } - terminal.println(VERBOSE, "removing [" + pluginDir + "]"); - /* + if (Files.exists(pluginConfigDir)) { + if (purge) { + try (Stream paths = Files.list(pluginConfigDir)) { + pluginPaths.addAll(paths.collect(Collectors.toList())); + } + pluginPaths.add(pluginConfigDir); + terminal.println(VERBOSE, "removing [" + pluginConfigDir + "]"); + } else { + /* + * By default we preserve the config files in case the user is upgrading the plugin, but we print a message so the user + * knows in case they want to remove manually. + */ + final String message = String.format( + Locale.ROOT, + "-> preserving plugin config files [%s] in case of upgrade; use --purge if not needed", + pluginConfigDir); + terminal.println(message); + } + } + + /* * We are going to create a marker file in the plugin directory that indicates that this plugin is a state of removal. If the * removal fails, the existence of this marker file indicates that the plugin is in a garbage state. We check for existence of this - * marker file during startup so that we do not startup with plugins in such a garbage state. + * marker file during startup so that we do not startup with plugins in such a garbage state. Up to this point, we have not done + * anything destructive, so we create the marker file as the last action before executing destructive operations. We place this + * marker file in the root plugin directory (not the specific plugin directory) so that we do not have to create the specific plugin + * directory if it does not exist (we are purging configuration files). */ - final Path removing = pluginDir.resolve(".removing-" + pluginName); - /* - * Add the contents of the plugin directory before creating the marker file and adding it to the list of paths to be deleted so - * that the marker file is the last file to be deleted. - */ - try (Stream paths = Files.list(pluginDir)) { - pluginPaths.addAll(paths.collect(Collectors.toList())); - } try { Files.createFile(removing); } catch (final FileAlreadyExistsException e) { @@ -124,24 +162,14 @@ class RemovePluginCommand extends EnvironmentAwareCommand { */ terminal.println(VERBOSE, "marker file [" + removing + "] already exists"); } - // now add the marker file - pluginPaths.add(removing); - // finally, add the plugin directory - pluginPaths.add(pluginDir); - IOUtils.rm(pluginPaths.toArray(new Path[pluginPaths.size()])); - /* - * We preserve the config files in case the user is upgrading the plugin, but we print a - * message so the user knows in case they want to remove manually. - */ - final Path pluginConfigDir = env.configFile().resolve(pluginName); - if (Files.exists(pluginConfigDir)) { - final String message = String.format( - Locale.ROOT, - "-> preserving plugin config files [%s] in case of upgrade; delete manually if not needed", - pluginConfigDir); - terminal.println(message); - } + // add the plugin directory + pluginPaths.add(pluginDir); + + // finally, add the marker file + pluginPaths.add(removing); + + IOUtils.rm(pluginPaths.toArray(new Path[pluginPaths.size()])); } } diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java index e631ee31a24..8ad432059d1 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java @@ -37,6 +37,8 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.PosixPermissionsResetter; import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; import java.io.BufferedReader; import java.io.IOException; @@ -52,7 +54,6 @@ import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; -import java.nio.file.Paths; import java.nio.file.SimpleFileVisitor; import java.nio.file.StandardCopyOption; import java.nio.file.attribute.BasicFileAttributes; @@ -64,6 +65,7 @@ import java.nio.file.attribute.UserPrincipal; import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -73,18 +75,14 @@ import java.util.zip.ZipOutputStream; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.not; @LuceneTestCase.SuppressFileSystems("*") public class InstallPluginCommandTests extends ESTestCase { - private static InstallPluginCommand SKIP_JARHELL_COMMAND = new InstallPluginCommand() { - @Override - void jarHellCheck(Path candidate, Path pluginsDir) throws Exception { - // no jarhell check - } - }; - private static InstallPluginCommand DEFAULT_COMMAND = new InstallPluginCommand(); + private InstallPluginCommand skipJarHellCommand; + private InstallPluginCommand defaultCommand; private final Function temp; @@ -104,9 +102,23 @@ public class InstallPluginCommandTests extends ESTestCase { System.setProperty("java.io.tmpdir", temp.apply("tmpdir").toString()); } + @Before + public void setUp() throws Exception { + super.setUp(); + skipJarHellCommand = new InstallPluginCommand() { + @Override + void jarHellCheck(Path candidate, Path pluginsDir) throws Exception { + // no jarhell check + } + }; + defaultCommand = new InstallPluginCommand(); + } + @After @SuppressForbidden(reason = "resets java.io.tmpdir") public void tearDown() throws Exception { + defaultCommand.close(); + skipJarHellCommand.close(); System.setProperty("java.io.tmpdir", javaIoTmpdir); PathUtilsForTesting.teardown(); super.tearDown(); @@ -210,11 +222,11 @@ public class InstallPluginCommandTests extends ESTestCase { return writeZip(structure, "elasticsearch"); } - static MockTerminal installPlugin(String pluginUrl, Path home) throws Exception { - return installPlugin(pluginUrl, home, SKIP_JARHELL_COMMAND); + MockTerminal installPlugin(String pluginUrl, Path home) throws Exception { + return installPlugin(pluginUrl, home, skipJarHellCommand); } - static MockTerminal installPlugin(String pluginUrl, Path home, InstallPluginCommand command) throws Exception { + MockTerminal installPlugin(String pluginUrl, Path home, InstallPluginCommand command) throws Exception { Environment env = new Environment(Settings.builder().put("path.home", home).build()); MockTerminal terminal = new MockTerminal(); command.execute(terminal, pluginUrl, true, env); @@ -322,6 +334,20 @@ public class InstallPluginCommandTests extends ESTestCase { assertPlugin("fake", pluginDir, env.v2()); } + public void testInstallFailsIfPreviouslyRemovedPluginFailed() throws Exception { + Tuple env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); + String pluginZip = createPluginUrl("fake", pluginDir); + final Path removing = env.v2().pluginsFile().resolve(".removing-failed"); + Files.createDirectory(removing); + final IllegalStateException e = expectThrows(IllegalStateException.class, () -> installPlugin(pluginZip, env.v1())); + final String expected = String.format( + Locale.ROOT, + "found file [%s] from a failed attempt to remove the plugin [failed]; execute [elasticsearch-plugin remove failed]", + removing); + assertThat(e, hasToString(containsString(expected))); + } + public void testSpaceInUrl() throws Exception { Tuple env = createEnv(fs, temp); Path pluginDir = createPluginDir(temp); @@ -377,7 +403,7 @@ public class InstallPluginCommandTests extends ESTestCase { writeJar(pluginDirectory.resolve("other.jar"), "FakePlugin"); String pluginZip = createPluginUrl("fake", pluginDirectory); // adds plugin.jar with FakePlugin IllegalStateException e = expectThrows(IllegalStateException.class, - () -> installPlugin(pluginZip, environment.v1(), DEFAULT_COMMAND)); + () -> installPlugin(pluginZip, environment.v1(), defaultCommand)); assertTrue(e.getMessage(), e.getMessage().contains("jar hell")); assertInstallCleaned(environment.v2()); } @@ -705,7 +731,7 @@ public class InstallPluginCommandTests extends ESTestCase { String pluginZip = createPluginUrl("fake", pluginDir); installPlugin(pluginZip, env.v1()); final UserException e = expectThrows(UserException.class, - () -> installPlugin(pluginZip, env.v1(), randomFrom(SKIP_JARHELL_COMMAND, DEFAULT_COMMAND))); + () -> installPlugin(pluginZip, env.v1(), randomFrom(skipJarHellCommand, defaultCommand))); assertThat( e.getMessage(), equalTo("plugin directory [" + env.v2().pluginsFile().resolve("fake") + "] already exists; " + @@ -717,7 +743,7 @@ public class InstallPluginCommandTests extends ESTestCase { Path pluginDir = createPluginDir(temp); // if batch is enabled, we also want to add a security policy String pluginZip = createPlugin("fake", pluginDir, isBatch).toUri().toURL().toString(); - SKIP_JARHELL_COMMAND.execute(terminal, pluginZip, isBatch, env.v2()); + skipJarHellCommand.execute(terminal, pluginZip, isBatch, env.v2()); } public void assertInstallPluginFromUrl(String pluginId, String name, String url, String stagingHash) throws Exception { diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java index 25fbf60fa4e..5879f2821d9 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java @@ -37,6 +37,7 @@ import java.nio.file.Path; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.Matchers.hasToString; @LuceneTestCase.SuppressFileSystems("*") public class RemovePluginCommandTests extends ESTestCase { @@ -58,10 +59,10 @@ public class RemovePluginCommandTests extends ESTestCase { env = new Environment(settings); } - static MockTerminal removePlugin(String name, Path home) throws Exception { + static MockTerminal removePlugin(String name, Path home, boolean purge) throws Exception { Environment env = new Environment(Settings.builder().put("path.home", home).build()); MockTerminal terminal = new MockTerminal(); - new RemovePluginCommand().execute(terminal, name, env); + new RemovePluginCommand().execute(terminal, env, name, purge); return terminal; } @@ -76,7 +77,7 @@ public class RemovePluginCommandTests extends ESTestCase { } public void testMissing() throws Exception { - UserException e = expectThrows(UserException.class, () -> removePlugin("dne", home)); + UserException e = expectThrows(UserException.class, () -> removePlugin("dne", home, randomBoolean())); assertTrue(e.getMessage(), e.getMessage().contains("plugin [dne] not found")); assertRemoveCleaned(env); } @@ -86,7 +87,7 @@ public class RemovePluginCommandTests extends ESTestCase { Files.createFile(env.pluginsFile().resolve("fake").resolve("plugin.jar")); Files.createDirectory(env.pluginsFile().resolve("fake").resolve("subdir")); Files.createDirectory(env.pluginsFile().resolve("other")); - removePlugin("fake", home); + removePlugin("fake", home, randomBoolean()); assertFalse(Files.exists(env.pluginsFile().resolve("fake"))); assertTrue(Files.exists(env.pluginsFile().resolve("other"))); assertRemoveCleaned(env); @@ -97,7 +98,7 @@ public class RemovePluginCommandTests extends ESTestCase { Path binDir = env.binFile().resolve("fake"); Files.createDirectories(binDir); Files.createFile(binDir.resolve("somescript")); - removePlugin("fake", home); + removePlugin("fake", home, randomBoolean()); assertFalse(Files.exists(env.pluginsFile().resolve("fake"))); assertTrue(Files.exists(env.binFile().resolve("elasticsearch"))); assertFalse(Files.exists(binDir)); @@ -106,7 +107,7 @@ public class RemovePluginCommandTests extends ESTestCase { public void testBinNotDir() throws Exception { Files.createDirectories(env.pluginsFile().resolve("elasticsearch")); - UserException e = expectThrows(UserException.class, () -> removePlugin("elasticsearch", home)); + UserException e = expectThrows(UserException.class, () -> removePlugin("elasticsearch", home, randomBoolean())); assertTrue(e.getMessage(), e.getMessage().contains("not a directory")); assertTrue(Files.exists(env.pluginsFile().resolve("elasticsearch"))); // did not remove assertTrue(Files.exists(env.binFile().resolve("elasticsearch"))); @@ -118,21 +119,58 @@ public class RemovePluginCommandTests extends ESTestCase { final Path configDir = env.configFile().resolve("fake"); Files.createDirectories(configDir); Files.createFile(configDir.resolve("fake.yml")); - final MockTerminal terminal = removePlugin("fake", home); + final MockTerminal terminal = removePlugin("fake", home, false); assertTrue(Files.exists(env.configFile().resolve("fake"))); assertThat(terminal.getOutput(), containsString(expectedConfigDirPreservedMessage(configDir))); assertRemoveCleaned(env); } + public void testPurgePluginExists() throws Exception { + Files.createDirectories(env.pluginsFile().resolve("fake")); + final Path configDir = env.configFile().resolve("fake"); + if (randomBoolean()) { + Files.createDirectories(configDir); + Files.createFile(configDir.resolve("fake.yml")); + } + final MockTerminal terminal = removePlugin("fake", home, true); + assertFalse(Files.exists(env.configFile().resolve("fake"))); + assertThat(terminal.getOutput(), not(containsString(expectedConfigDirPreservedMessage(configDir)))); + assertRemoveCleaned(env); + } + + public void testPurgePluginDoesNotExist() throws Exception { + final Path configDir = env.configFile().resolve("fake"); + Files.createDirectories(configDir); + Files.createFile(configDir.resolve("fake.yml")); + final MockTerminal terminal = removePlugin("fake", home, true); + assertFalse(Files.exists(env.configFile().resolve("fake"))); + assertThat(terminal.getOutput(), not(containsString(expectedConfigDirPreservedMessage(configDir)))); + assertRemoveCleaned(env); + } + + public void testPurgeNothingExists() throws Exception { + final UserException e = expectThrows(UserException.class, () -> removePlugin("fake", home, true)); + assertThat(e, hasToString(containsString("plugin [fake] not found"))); + } + + public void testPurgeOnlyMarkerFileExists() throws Exception { + final Path configDir = env.configFile().resolve("fake"); + final Path removing = env.pluginsFile().resolve(".removing-fake"); + Files.createFile(removing); + final MockTerminal terminal = removePlugin("fake", home, randomBoolean()); + assertFalse(Files.exists(removing)); + assertThat(terminal.getOutput(), not(containsString(expectedConfigDirPreservedMessage(configDir)))); + } + public void testNoConfigDirPreserved() throws Exception { Files.createDirectories(env.pluginsFile().resolve("fake")); final Path configDir = env.configFile().resolve("fake"); - final MockTerminal terminal = removePlugin("fake", home); + final MockTerminal terminal = removePlugin("fake", home, randomBoolean()); assertThat(terminal.getOutput(), not(containsString(expectedConfigDirPreservedMessage(configDir)))); } public void testRemoveUninstalledPluginErrors() throws Exception { - UserException e = expectThrows(UserException.class, () -> removePlugin("fake", home)); + UserException e = expectThrows(UserException.class, () -> removePlugin("fake", home, randomBoolean())); assertEquals(ExitCodes.CONFIG, e.exitCode); assertEquals("plugin [fake] not found; run 'elasticsearch-plugin list' to get list of installed plugins", e.getMessage()); @@ -152,7 +190,7 @@ public class RemovePluginCommandTests extends ESTestCase { } public void testMissingPluginName() throws Exception { - UserException e = expectThrows(UserException.class, () -> removePlugin(null, home)); + UserException e = expectThrows(UserException.class, () -> removePlugin(null, home, randomBoolean())); assertEquals(ExitCodes.USAGE, e.exitCode); assertEquals("plugin name is required", e.getMessage()); } @@ -160,12 +198,12 @@ public class RemovePluginCommandTests extends ESTestCase { public void testRemoveWhenRemovingMarker() throws Exception { Files.createDirectory(env.pluginsFile().resolve("fake")); Files.createFile(env.pluginsFile().resolve("fake").resolve("plugin.jar")); - Files.createFile(env.pluginsFile().resolve("fake").resolve(".removing-fake")); - removePlugin("fake", home); + Files.createFile(env.pluginsFile().resolve(".removing-fake")); + removePlugin("fake", home, randomBoolean()); } private String expectedConfigDirPreservedMessage(final Path configDir) { - return "-> preserving plugin config files [" + configDir + "] in case of upgrade; delete manually if not needed"; + return "-> preserving plugin config files [" + configDir + "] in case of upgrade; use --purge if not needed"; } } diff --git a/docs/plugins/plugin-script.asciidoc b/docs/plugins/plugin-script.asciidoc index e89766f3c1f..8b72509c057 100644 --- a/docs/plugins/plugin-script.asciidoc +++ b/docs/plugins/plugin-script.asciidoc @@ -107,7 +107,14 @@ Plugins can be removed manually, by deleting the appropriate directory under sudo bin/elasticsearch-plugin remove [pluginname] ----------------------------------- -After a Java plugin has been removed, you will need to restart the node to complete the removal process. +After a Java plugin has been removed, you will need to restart the node to +complete the removal process. + +By default, plugin configuration files (if any) are preserved on disk; this is +so that configuration is not lost while upgrading a plugin. If you wish to +purge the configuration files while removing a plugin, use `-p` or `--purge`. +This can option can be used after a plugin is removed to remove any lingering +configuration files. [float] === Updating plugins