Do not start redis dev services in augmentation (plus supporting framework)

bom/application/pom.xml

@@ -839,6 +839,11 @@
                 <artifactId>quarkus-devservices-common</artifactId>
                 <version>${project.version}</version>
             </dependency>
+            <dependency>
+                <groupId>io.quarkus</groupId>
+                <artifactId>quarkus-devservices</artifactId>
+                <version>${project.version}</version>
+            </dependency>
             <dependency>
                 <groupId>io.quarkus</groupId>
                 <artifactId>quarkus-devservices-deployment</artifactId>

core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java

@@ -2,27 +2,35 @@
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.function.Supplier;
+
+import org.jboss.logging.Logger;
 
 import io.quarkus.builder.item.MultiBuildItem;
 
 /**
  * BuildItem for running dev services.
  * Combines injected configs to the application with container id (if it exists).
- *
+ * <p>
  * Processors are expected to return this build item not only when the dev service first starts,
  * but also if a running dev service already exists.
- *
+ * <p>
  * {@link RunningDevService} helps to manage the lifecycle of the running dev service.
  */
 public final class DevServicesResultBuildItem extends MultiBuildItem {
 
+    private static final Logger log = Logger.getLogger(DevServicesResultBuildItem.class);
+
     private final String name;
     private final String description;
     private final String containerId;
-    private final Map<String, String> config;
+    protected final Map<String, String> config;
+    protected RunnableDevService runnableDevService;
 
     public DevServicesResultBuildItem(String name, String containerId, Map<String, String> config) {
         this(name, null, containerId, config);
@@ -35,6 +43,12 @@ public DevServicesResultBuildItem(String name, String description, String contai
         this.config = config;
     }
 
+    public DevServicesResultBuildItem(String name, String description, String containerId, Map<String, String> config,
+            RunnableDevService runnableDevService) {
+        this(name, description, containerId, config);
+        this.runnableDevService = runnableDevService;
+    }
+
     public String getName() {
         return name;
     }
@@ -51,13 +65,22 @@ public Map<String, String> getConfig() {
         return config;
     }
 
+    public void start() {
+        if (runnableDevService != null) {
+            runnableDevService.start();
+        } else {
+            log.debugf("Not starting %s because runnable dev service is null (it is probably a running dev service.", name);
+        }
+    }
+
     public static class RunningDevService implements Closeable {
 
-        private final String name;
-        private final String description;
-        private final String containerId;
-        private final Map<String, String> config;
-        private final Closeable closeable;
+        protected final String name;
+        protected final String description;
+        protected final String containerId;
+        protected final Map<String, String> config;
+        protected final Closeable closeable;
+        protected volatile boolean isRunning = true;
 
         private static Map<String, String> mapOf(String key, String value) {
             Map<String, String> map = new HashMap<>();
@@ -109,6 +132,9 @@ public Closeable getCloseable() {
             return closeable;
         }
 
+        // This method should be on RunningDevService, but not on RunnableDevService, where we use different logic to
+        // decide when it's time to close a container. For now, leave it where it is and hope it doesn't get called when it shouldn't.
+        // We can either make a common parent class or throw unsupported when this is called from Runnable.
         public boolean isOwner() {
             return closeable != null;
         }
@@ -117,11 +143,84 @@ public boolean isOwner() {
         public void close() throws IOException {
             if (this.closeable != null) {
                 this.closeable.close();
+                isRunning = false;
             }
         }
 
         public DevServicesResultBuildItem toBuildItem() {
             return new DevServicesResultBuildItem(name, description, containerId, config);
         }
     }
+
+    public static class RunnableDevService extends RunningDevService implements Supplier<Map<String, String>> {
+        final DevServicesTrackerBuildItem tracker;
+
+        private final Startable container;
+        private Map<String, Supplier> lazyConfig;
+
+        public RunnableDevService(String name, String containerId, Startable container, Map config,
+                Map lazyConfig, DevServicesTrackerBuildItem tracker) {
+            super(name, containerId, container::close, config);
+
+            this.container = container;
+            this.tracker = tracker;
+            isRunning = false;
+            this.lazyConfig = lazyConfig;
+        }
+
+        public boolean isRunning() {
+            return isRunning;
+        }
+
+        public void start() {
+            // We want to do two things; find things with the same config as us to reuse them, and find things with different config to close them
+            // We figure out if we need to shut down existing redis containers that might have been started in previous profiles or restarts
+
+            List<RunnableDevService> matchedDevServices = tracker.getRunningServices(name, config);
+            // if the redis containers have already started we just return; if we wanted to be very cautious we could check the entries for an isRunningStatus, but they might be in the wrong classloader, so that's hard work
+
+            if (matchedDevServices == null || matchedDevServices.size() > 0) {
+                // There isn't a running container that has the right config, we need to do work
+                // Let's get all the running dev services associated with this feature, so we can close them
+                Collection<Closeable> unusableDevServices = tracker.getAllServices(name);
+                if (unusableDevServices != null) {
+                    for (Closeable closeable : unusableDevServices) {
+                        try {
+                            closeable.close();
+                        } catch (IOException e) {
+                            throw new RuntimeException(e);
+                        }
+                    }
+                }
+
+                if (container != null) {
+                    container.start();
+                    //  tell the tracker that we started
+                    tracker.addRunningService(name, config, this);
+                    isRunning = true;
+                }
+            }
+        }
+
+        @Override
+        public void close() throws IOException {
+            super.close();
+            tracker.removeRunningService(name, config, this);
+        }
+
+        public DevServicesResultBuildItem toBuildItem() {
+            return new DevServicesResultBuildItem(name, description, containerId, config, this);
+        }
+
+        @Override
+        public Map<String, String> get() {
+            // TODO printlns show this gets called way too often - does specifying the properties cut that down?
+            Map config = getConfig();
+            Map newConfig = new HashMap<>(config);
+            for (Map.Entry<String, Supplier> entry : lazyConfig.entrySet()) {
+                newConfig.put(entry.getKey(), entry.getValue().get());
+            }
+            return newConfig;
+        }
+    }
 }

core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java

@@ -0,0 +1,104 @@
+package io.quarkus.deployment.builditem;
+
+import java.io.Closeable;
+import java.lang.reflect.InvocationTargetException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+import io.quarkus.builder.item.SimpleBuildItem;
+import io.quarkus.devservices.runtime.DevServicesConfigTracker;
+
+// Ideally we would have a unique build item for each processor/feature, but that would need a new KeyedBuildItem or FeatureBuildItem type
+// Needs to be in core because DevServicesResultBuildItem is in core
+public final class DevServicesTrackerBuildItem extends SimpleBuildItem {
+
+    // A map of a map of a list? What?!?
+    // The reason this is like this, rather than being broken out into types, is because this map gets shared between classloaders, so language-level constructs work best
+    private static Map<String, Map<Map, List<Closeable>>> systemInstance = null;
+    private final DevServicesConfigTracker configTracker;
+
+    public DevServicesTrackerBuildItem() {
+
+        if (systemInstance == null) {
+            try {
+                Class s = ClassLoader.getSystemClassLoader().loadClass(DevServicesTrackerBuildItem.class.getName());
+                systemInstance = (Map<String, Map<Map, List<Closeable>>>) s.getMethod("getBackingMap")
+                        .invoke(null);
+            } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
+                throw new RuntimeException(e);
+            }
+        }
+        configTracker = new DevServicesConfigTracker();
+
+    }
+
+    /**
+     * Public so it can be used across classloaders. Should not be used except by this class.
+     * Invoked reflectively.
+     */
+    public static Map getBackingMap() {
+        if (systemInstance == null) {
+            systemInstance = new ConcurrentHashMap<>();
+        }
+        return systemInstance;
+    }
+
+    public List getRunningServices(String featureName,
+            Map config) {
+        Map<Map, List<Closeable>> services = systemInstance.get(featureName);
+        if (services != null) {
+            return services.get(config);
+        }
+        return null;
+    }
+
+    public Set<Closeable> getAllServices(String featureName) {
+        Map<Map, List<Closeable>> services = systemInstance.get(featureName);
+        if (services == null) {
+            return Set.of();
+        } else {
+            // Flatten
+            Set<Closeable> ls = new HashSet<>();
+            services.values().forEach(ls::addAll);
+            return ls;
+        }
+    }
+
+    public void addRunningService(String name, Map<String, String> identifyingConfig,
+            DevServicesResultBuildItem.RunnableDevService service) {
+        Map<Map, List<Closeable>> services = systemInstance.get(name);
+
+        if (services == null) {
+            services = new HashMap<>();
+            systemInstance.put(name, services);
+        }
+
+        // Make a list so that we can add and remove to it
+        List<Closeable> list = new ArrayList<>();
+        list.add(service);
+        services.put(identifyingConfig, list);
+
+        configTracker.addRunningService(service);
+    }
+
+    public void removeRunningService(String name, Map<String, String> identifyingConfig,
+            DevServicesResultBuildItem.RunnableDevService service) {
+        Map<Map, List<Closeable>> services = systemInstance.get(name);
+
+        if (services != null) {
+            List servicesForConfig = services.get(identifyingConfig);
+            if (servicesForConfig != null) {
+                servicesForConfig.remove(service);
+            }
+        }
+
+        configTracker.removeRunningService(service);
+
+    }
+
+}

core/deployment/src/main/java/io/quarkus/deployment/builditem/Startable.java

@@ -0,0 +1,7 @@
+package io.quarkus.deployment.builditem;
+
+import java.io.Closeable;
+
+public interface Startable extends Closeable {
+    void start();
+}

core/deployment/src/main/java/io/quarkus/runner/bootstrap/StartupActionImpl.java

@@ -33,6 +33,7 @@
 import io.quarkus.deployment.builditem.ApplicationClassNameBuildItem;
 import io.quarkus.deployment.builditem.DevServicesLauncherConfigResultBuildItem;
 import io.quarkus.deployment.builditem.DevServicesNetworkIdBuildItem;
+import io.quarkus.deployment.builditem.DevServicesResultBuildItem;
 import io.quarkus.deployment.builditem.GeneratedClassBuildItem;
 import io.quarkus.deployment.builditem.GeneratedResourceBuildItem;
 import io.quarkus.deployment.builditem.MainClassBuildItem;
@@ -54,6 +55,7 @@ public class StartupActionImpl implements StartupAction {
     private final String mainClassName;
     private final String applicationClassName;
     private final Map<String, String> devServicesProperties;
+    private final List<DevServicesResultBuildItem> devServicesResultBuildItems;
     private final String devServicesNetworkId;
     private final List<RuntimeApplicationShutdownBuildItem> runtimeApplicationShutdownBuildItems;
     private final List<Closeable> runtimeCloseTasks = new ArrayList<>();
@@ -67,6 +69,9 @@ public StartupActionImpl(CuratedApplication curatedApplication, BuildResult buil
         this.devServicesNetworkId = extractDevServicesNetworkId(buildResult);
         this.runtimeApplicationShutdownBuildItems = buildResult.consumeMulti(RuntimeApplicationShutdownBuildItem.class);
 
+        devServicesResultBuildItems = buildResult
+                .consumeMulti(DevServicesResultBuildItem.class);
+
         Map<String, byte[]> transformedClasses = extractTransformedClasses(buildResult);
         QuarkusClassLoader baseClassLoader = curatedApplication.getOrCreateBaseRuntimeClassLoader();
         QuarkusClassLoader runtimeClassLoader;
@@ -99,6 +104,8 @@ public StartupActionImpl(CuratedApplication curatedApplication, BuildResult buil
      * of the JVM will exit when the app stops.
      */
     public RunningQuarkusApplication runMainClass(String... args) throws Exception {
+        // Start dev services that weren't started in the augmentation phase
+        startDevServices(devServicesResultBuildItems);
 
         //first we hack around class loading in the fork join pool
         ForkJoinClassLoading.setForkJoinClassLoader(runtimeClassLoader);
@@ -206,6 +213,9 @@ private void doClose() {
 
     @Override
     public int runMainClassBlocking(String... args) throws Exception {
+        // Start dev services that weren't started in the augmentation phase
+        startDevServices(devServicesResultBuildItems);
+
         //first we hack around class loading in the fork join pool
         ForkJoinClassLoading.setForkJoinClassLoader(runtimeClassLoader);
 
@@ -276,10 +286,19 @@ public void overrideConfig(Map<String, String> config) {
         RuntimeOverrideConfigSource.setConfig(runtimeClassLoader, config);
     }
 
+    private static void startDevServices(List<DevServicesResultBuildItem> devServicesResultBuildItems) {
+        for (DevServicesResultBuildItem devServicesResultBuildItem : devServicesResultBuildItems) {
+            devServicesResultBuildItem.start();
+        }
+    }
+
     /**
      * Runs the application, and returns a handle that can be used to shut it down.
      */
     public RunningQuarkusApplication run(String... args) throws Exception {
+        // Start dev services that weren't started in the augmentation phase
+        startDevServices(devServicesResultBuildItems);
+
         //first we hack around class loading in the fork join pool
         ForkJoinClassLoading.setForkJoinClassLoader(runtimeClassLoader);
 

core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigBuilder.java

@@ -0,0 +1,21 @@
+package io.quarkus.devservice.runtime.config;
+
+import io.quarkus.runtime.configuration.ConfigBuilder;
+import io.smallrye.config.SmallRyeConfigBuilder;
+
+// This should live in the devservices/runtime module, but that module doesn't exist, and adding it is a breaking change
+public class DevServicesConfigBuilder implements ConfigBuilder {
+
+    @Override
+    public SmallRyeConfigBuilder configBuilder(SmallRyeConfigBuilder builder) {
+        return builder.withSources(new DevServicesConfigSource());
+    }
+
+    @Override
+    public int priority() {
+        // We feel more important than config in environment variables and application.properties files, but dev services should be looking at those sources and not doing anything if there's existing config,
+        // so a very low priority is also arguably correct.
+
+        return 500;
+    }
+}