Make reaper service a gradle build service (#69535) (#69674)

- Allows creating the service lazy only when required - Removes the need for build finish hook usage as Gradle takes care of cleaning up - This helps us getting closer to be gradle configuration cache compliant
elastic · Mar 1, 2021 · 88b9db3 · 88b9db3
1 parent 5a07d94
commit 88b9db3
Show file tree

Hide file tree

Showing 8 changed files with 91 additions and 59 deletions.
diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/ReaperPlugin.java b/buildSrc/src/main/java/org/elasticsearch/gradle/ReaperPlugin.java
@@ -8,33 +8,45 @@
 
 package org.elasticsearch.gradle;
 
+import org.elasticsearch.gradle.info.BuildParams;
 import org.elasticsearch.gradle.info.GlobalBuildInfoPlugin;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
+import org.gradle.api.file.ProjectLayout;
 
-import java.nio.file.Path;
+import javax.inject.Inject;
+import java.io.File;
 
 /**
  * A plugin to handle reaping external services spawned by a build if Gradle dies.
  */
 public class ReaperPlugin implements Plugin<Project> {
 
+    public static final String REAPER_SERVICE_NAME = "reaper";
+    private final ProjectLayout projectLayout;
+
+    @Inject
+    public ReaperPlugin(ProjectLayout projectLayout) {
+        this.projectLayout = projectLayout;
+    }
+
     @Override
     public void apply(Project project) {
         if (project != project.getRootProject()) {
             throw new IllegalArgumentException("ReaperPlugin can only be applied to the root project of a build");
         }
-
         project.getPlugins().apply(GlobalBuildInfoPlugin.class);
-
-        Path inputDir = project.getRootDir()
-            .toPath()
-            .resolve(".gradle")
-            .resolve("reaper")
-            .resolve("build-" + ProcessHandle.current().pid());
-        ReaperService service = project.getExtensions()
-            .create("reaper", ReaperService.class, project, project.getBuildDir().toPath(), inputDir);
-
-        project.getGradle().buildFinished(result -> service.shutdown());
+        File inputDir = projectLayout.getProjectDirectory()
+            .dir(".gradle")
+            .dir("reaper")
+            .dir("build-" + ProcessHandle.current().pid())
+            .getAsFile();
+        project.getGradle().getSharedServices().registerIfAbsent(REAPER_SERVICE_NAME, ReaperService.class, spec -> {
+            // Provide some parameters
+            spec.getParameters().getInputDir().set(inputDir);
+            spec.getParameters().getBuildDir().set(projectLayout.getBuildDirectory());
+            spec.getParameters().setInternal(BuildParams.isInternal());
+        });
     }
+
 }
diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/ReaperService.java b/buildSrc/src/main/java/org/elasticsearch/gradle/ReaperService.java
@@ -8,13 +8,16 @@
 
 package org.elasticsearch.gradle;
 
-import org.elasticsearch.gradle.info.BuildParams;
 import org.gradle.api.GradleException;
-import org.gradle.api.Project;
+import org.gradle.api.file.DirectoryProperty;
 import org.gradle.api.logging.Logger;
+import org.gradle.api.logging.Logging;
+import org.gradle.api.services.BuildService;
+import org.gradle.api.services.BuildServiceParameters;
 import org.gradle.internal.jvm.Jvm;
 
 import java.io.FileWriter;
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -26,24 +29,12 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-public class ReaperService {
+public abstract class ReaperService implements BuildService<ReaperService.Params>, AutoCloseable {
 
     private static final String REAPER_CLASS = "org/elasticsearch/gradle/reaper/Reaper.class";
     private static final Pattern REAPER_JAR_PATH_PATTERN = Pattern.compile("file:(.*)!/" + REAPER_CLASS);
-    private final Logger logger;
-    private final boolean isInternal;
-    private final Path buildDir;
-    private final Path inputDir;
-    private final Path logFile;
     private volatile Process reaperProcess;
-
-    public ReaperService(Project project, Path buildDir, Path inputDir) {
-        this.logger = project.getLogger();
-        this.isInternal = BuildParams.isInternal();
-        this.buildDir = buildDir;
-        this.inputDir = inputDir;
-        this.logFile = inputDir.resolve("reaper.log");
-    }
+    private final Logger logger = Logging.getLogger(getClass());
 
     /**
      * Register a pid that will be killed by the reaper.
@@ -70,7 +61,7 @@ public void registerCommand(String serviceId, String... command) {
     }
 
     private Path getCmdFile(String serviceId) {
-        return inputDir.resolve(serviceId.replaceAll("[^a-zA-Z0-9]", "-") + ".cmd");
+        return getParameters().getInputDir().get().getAsFile().toPath().resolve(serviceId.replaceAll("[^a-zA-Z0-9]", "-") + ".cmd");
     }
 
     public void unregister(String serviceId) {
@@ -88,6 +79,7 @@ void shutdown() {
                 reaperProcess.getOutputStream().close();
                 logger.info("Waiting for reaper to exit normally");
                 if (reaperProcess.waitFor() != 0) {
+                    Path inputDir = getParameters().getInputDir().get().getAsFile().toPath();
                     throw new GradleException("Reaper process failed. Check log at " + inputDir.resolve("error.log") + " for details");
                 }
             } catch (Exception e) {
@@ -101,10 +93,10 @@ private synchronized void ensureReaperStarted() {
         if (reaperProcess == null) {
             try {
                 Path jarPath = locateReaperJar();
+                Path inputDir = getParameters().getInputDir().get().getAsFile().toPath();
 
                 // ensure the input directory exists
                 Files.createDirectories(inputDir);
-
                 // start the reaper
                 ProcessBuilder builder = new ProcessBuilder(
                     Jvm.current().getJavaExecutable().toString(), // same jvm as gradle
@@ -117,8 +109,9 @@ private synchronized void ensureReaperStarted() {
                 logger.info("Launching reaper: " + String.join(" ", builder.command()));
                 // be explicit for stdin, we use closing of the pipe to signal shutdown to the reaper
                 builder.redirectInput(ProcessBuilder.Redirect.PIPE);
-                builder.redirectOutput(logFile.toFile());
-                builder.redirectError(logFile.toFile());
+                File logFile = logFilePath().toFile();
+                builder.redirectOutput(logFile);
+                builder.redirectError(logFile);
                 reaperProcess = builder.start();
             } catch (Exception e) {
                 throw new RuntimeException(e);
@@ -128,8 +121,12 @@ private synchronized void ensureReaperStarted() {
         }
     }
 
+    private Path logFilePath() {
+        return getParameters().getInputDir().get().getAsFile().toPath().resolve("reaper.log");
+    }
+
     private Path locateReaperJar() {
-        if (isInternal) {
+        if (getParameters().getInternal()) {
             // when running inside the Elasticsearch build just pull find the jar in the runtime classpath
             URL main = this.getClass().getClassLoader().getResource(REAPER_CLASS);
             String mainPath = main.getFile();
@@ -143,7 +140,7 @@ private Path locateReaperJar() {
             }
         } else {
             // copy the reaper jar
-            Path jarPath = buildDir.resolve("reaper").resolve("reaper.jar");
+            Path jarPath = getParameters().getBuildDir().get().getAsFile().toPath().resolve("reaper").resolve("reaper.jar");
             try {
                 Files.createDirectories(jarPath.getParent());
             } catch (IOException e) {
@@ -166,7 +163,22 @@ private Path locateReaperJar() {
 
     private void ensureReaperAlive() {
         if (reaperProcess.isAlive() == false) {
-            throw new IllegalStateException("Reaper process died unexpectedly! Check the log at " + logFile.toString());
+            throw new IllegalStateException("Reaper process died unexpectedly! Check the log at " + logFilePath().toString());
         }
     }
+
+    @Override
+    public void close() throws Exception {
+        shutdown();
+    }
+
+    interface Params extends BuildServiceParameters {
+        Boolean getInternal();
+
+        void setInternal(Boolean internal);
+
+        DirectoryProperty getBuildDir();
+
+        DirectoryProperty getInputDir();
+    }
 }
diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java
@@ -56,7 +56,7 @@ public class ElasticsearchCluster implements TestClusterConfiguration, Named {
     private final File workingDirBase;
     private final LinkedHashMap<String, Predicate<TestClusterConfiguration>> waitConditions = new LinkedHashMap<>();
     private final Project project;
-    private final ReaperService reaper;
+    private final Provider<ReaperService> reaper;
     private final FileSystemOperations fileSystemOperations;
     private final ArchiveOperations archiveOperations;
     private final ExecOperations execOperations;
@@ -65,7 +65,7 @@ public class ElasticsearchCluster implements TestClusterConfiguration, Named {
     public ElasticsearchCluster(
         String clusterName,
         Project project,
-        ReaperService reaper,
+        Provider<ReaperService> reaper,
         File workingDirBase,
         FileSystemOperations fileSystemOperations,
         ArchiveOperations archiveOperations,

diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java
@@ -116,8 +116,8 @@ public class ElasticsearchNode implements TestClusterConfiguration {
     private final String path;
     private final String name;
     private final Project project;
-    private final ReaperService reaper;
     private final Jdk bwcJdk;
+    private final Provider<ReaperService> reaperServiceProvider;
     private final FileSystemOperations fileSystemOperations;
     private final ArchiveOperations archiveOperations;
     private final ExecOperations execOperations;
@@ -166,7 +166,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
         String path,
         String name,
         Project project,
-        ReaperService reaper,
+        Provider<ReaperService> reaperServiceProvider,
         File workingDirBase,
         FileSystemOperations fileSystemOperations,
         ArchiveOperations archiveOperations,
@@ -177,7 +177,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
         this.path = path;
         this.name = name;
         this.project = project;
-        this.reaper = reaper;
+        this.reaperServiceProvider = reaperServiceProvider;
         this.fileSystemOperations = fileSystemOperations;
         this.archiveOperations = archiveOperations;
         this.execOperations = execOperations;
@@ -853,7 +853,7 @@ private void startElasticsearchProcess() {
         } catch (IOException e) {
             throw new TestClustersException("Failed to start ES process for " + this, e);
         }
-        reaper.registerPid(toString(), esProcess.pid());
+        reaperServiceProvider.get().registerPid(toString(), esProcess.pid());
     }
 
     @Internal
@@ -921,7 +921,7 @@ public synchronized void stop(boolean tailLogs) {
         requireNonNull(esProcess, "Can't stop `" + this + "` as it was not started or already stopped.");
         // Test clusters are not reused, don't spend time on a graceful shutdown
         stopHandle(esProcess.toHandle(), true);
-        reaper.unregister(toString());
+        reaperServiceProvider.get().unregister(toString());
         esProcess = null;
         // Clean up the ports file in case this is started again.
         try {

diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java
@@ -70,11 +70,7 @@ public void apply(Project project) {
         project.getRootProject().getPluginManager().apply(GlobalBuildInfoPlugin.class);
         BuildParams.withInternalBuild(() -> project.getPlugins().apply(InternalDistributionDownloadPlugin.class))
             .orElse(() -> project.getPlugins().apply(DistributionDownloadPlugin.class));
-
         project.getRootProject().getPluginManager().apply(ReaperPlugin.class);
-
-        ReaperService reaper = project.getRootProject().getExtensions().getByType(ReaperService.class);
-
         // register legacy jdk distribution for testing pre-7.0 BWC clusters
         Jdk bwcJdk = JdkDownloadPlugin.getContainer(project).create("bwc_jdk", jdk -> {
             jdk.setVendor(LEGACY_JAVA_VENDOR);
@@ -83,8 +79,16 @@ public void apply(Project project) {
             jdk.setArchitecture(Architecture.current().name().toLowerCase());
         });
 
+        Provider<ReaperService> reaperServiceProvider = GradleUtils.getBuildService(
+            project.getGradle().getSharedServices(),
+            ReaperPlugin.REAPER_SERVICE_NAME
+        );
         // enable the DSL to describe clusters
-        NamedDomainObjectContainer<ElasticsearchCluster> container = createTestClustersContainerExtension(project, reaper, bwcJdk);
+        NamedDomainObjectContainer<ElasticsearchCluster> container = createTestClustersContainerExtension(
+            project,
+            reaperServiceProvider,
+            bwcJdk
+        );
 
         // provide a task to be able to list defined clusters.
         createListClustersTask(project, container);
@@ -107,7 +111,7 @@ public void apply(Project project) {
 
     private NamedDomainObjectContainer<ElasticsearchCluster> createTestClustersContainerExtension(
         Project project,
-        ReaperService reaper,
+        Provider<ReaperService> reaper,
         Jdk bwcJdk
     ) {
         // Create an extensions that allows describing clusters

diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/vagrant/VagrantBasePlugin.java b/buildSrc/src/main/java/org/elasticsearch/gradle/vagrant/VagrantBasePlugin.java
@@ -9,7 +9,7 @@
 package org.elasticsearch.gradle.vagrant;
 
 import org.elasticsearch.gradle.ReaperPlugin;
-import org.elasticsearch.gradle.ReaperService;
+import org.elasticsearch.gradle.util.GradleUtils;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
 import org.gradle.api.Task;
@@ -34,9 +34,9 @@ public void apply(Project project) {
         project.getRootProject().getPluginManager().apply(VagrantManagerPlugin.class);
         project.getRootProject().getPluginManager().apply(ReaperPlugin.class);
 
-        ReaperService reaper = project.getRootProject().getExtensions().getByType(ReaperService.class);
-        VagrantExtension extension = project.getExtensions().create("vagrant", VagrantExtension.class, project);
-        VagrantMachine service = project.getExtensions().create("vagrantService", VagrantMachine.class, project, extension, reaper);
+        var reaperServiceProvider = GradleUtils.getBuildService(project.getGradle().getSharedServices(), ReaperPlugin.REAPER_SERVICE_NAME);
+        var extension = project.getExtensions().create("vagrant", VagrantExtension.class, project);
+        var service = project.getExtensions().create("vagrantService", VagrantMachine.class, extension, reaperServiceProvider);
 
         project.getGradle()
             .getTaskGraph()

diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/vagrant/VagrantMachine.java b/buildSrc/src/main/java/org/elasticsearch/gradle/vagrant/VagrantMachine.java
@@ -15,6 +15,7 @@
 import org.elasticsearch.gradle.util.Util;
 import org.gradle.api.Action;
 import org.gradle.api.Project;
+import org.gradle.api.provider.Provider;
 import org.gradle.internal.logging.progress.ProgressLogger;
 import org.gradle.internal.logging.progress.ProgressLoggerFactory;
 import org.gradle.process.ExecOperations;
@@ -35,17 +36,16 @@
  */
 public class VagrantMachine {
 
-    private final Project project;
     private final VagrantExtension extension;
-    private final ReaperService reaper;
+    private final Provider<ReaperService> reaperServiceProvider;
+    private ReaperService reaper;
     // pkg private so plugin can set this after construction
     long refs;
     private boolean isVMStarted = false;
 
-    public VagrantMachine(Project project, VagrantExtension extension, ReaperService reaper) {
-        this.project = project;
+    public VagrantMachine(VagrantExtension extension, Provider<ReaperService> reaperServiceProvider) {
         this.extension = extension;
-        this.reaper = reaper;
+        this.reaperServiceProvider = reaperServiceProvider;
     }
 
     @Inject
@@ -97,7 +97,6 @@ void maybeStartVM() {
         if (isVMStarted) {
             return;
         }
-
         execute(spec -> {
             spec.setCommand("box");
             spec.setSubcommand("update");
@@ -114,6 +113,7 @@ void maybeStartVM() {
         }
 
         // register box to be shutdown if gradle dies
+        reaper = reaperServiceProvider.get();
         reaper.registerCommand(extension.getBox(), "vagrant", "halt", "-f", extension.getBox());
 
         // We lock the provider to virtualbox because the Vagrantfile specifies lots of boxes that only work

diff --git a/buildSrc/src/testKit/reaper/build.gradle b/buildSrc/src/testKit/reaper/build.gradle
@@ -2,9 +2,13 @@ plugins {
   id 'elasticsearch.reaper'
 }
 
+import org.elasticsearch.gradle.ReaperPlugin;
+import org.elasticsearch.gradle.util.GradleUtils;
+
 tasks.register("launchReaper") {
   doLast {
-    def reaper = project.extensions.getByName('reaper')
+    def serviceProvider = GradleUtils.getBuildService(project.getGradle().getSharedServices(), ReaperPlugin.REAPER_SERVICE_NAME);
+    def reaper = serviceProvider.get()
     reaper.registerCommand('test', 'true')
     reaper.unregister('test')
   }