Use a marker file when removing a plugin

Today when removing a plugin, we attempt to move the plugin directory to
a temporary directory and then delete that directory from the
filesystem. We do this to avoid a plugin being in a half-removed
state. We previously tried an atomic move, and fell back to a non-atomic
move if that failed. Atomic moves can fail on union filesystems when the
plugin directory is not in the top layer of the
filesystem. Interestingly, the regular move can fail as well. This is
because when the JDK is executing such a move, it first tries to rename
the source directory to the target directory and if this fails with
EXDEV (as in the case of an atomic move failing), it falls back to
copying the source to the target, and then attempts to rmdir the
source. The bug here is that the JDK never deleted the contents of the
source so the rmdir will always fail (except in the case of an empty
directory).

Given all this silliness, we were inspired to find a different
strategy. The strategy is simple. We will add a marker file to the
plugin directory that indicates the plugin is in a state of
removal. This file will be the last file out the door during removal. If
this file exists during startup, we fail startup.

Relates #24252

1 files changed, 30 insertions, 0 deletions

diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java
index f4aae5232c..89c65ad2c8 100644
--- a/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java
+++ b/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java
@@ -20,6 +20,7 @@
 package org.elasticsearch.plugins;
 
 import org.apache.lucene.util.LuceneTestCase;
+import org.elasticsearch.Version;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.index.IndexModule;
@@ -30,6 +31,7 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Locale;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasToString;
@@ -121,4 +123,32 @@ public class PluginsServiceTests extends ESTestCase {
         assertThat(e, hasToString(containsString(expected)));
     }
 
+    public void testStartupWithRemovingMarker() throws IOException {
+        final Path home = createTempDir();
+        final Settings settings =
+                Settings.builder()
+                        .put(Environment.PATH_HOME_SETTING.getKey(), home)
+                        .build();
+        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"));
+        PluginTestUtil.writeProperties(
+                fake,
+                "description", "fake",
+                "name", "fake",
+                "version", "1.0.0",
+                "elasticsearch.version", Version.CURRENT.toString(),
+                "java.version", System.getProperty("java.specification.version"),
+                "classname", "Fake",
+                "has.native.controller", "false");
+        final IllegalStateException e = expectThrows(IllegalStateException.class, () -> newPluginsService(settings));
+        final String expected = String.format(
+                Locale.ROOT,
+                "found file [%s] from a failed attempt to remove the plugin [fake]; execute [elasticsearch-plugin remove fake]",
+                removing);
+        assertThat(e, hasToString(containsString(expected)));
+    }
+
 }