8000 updates for quarkus:dev mode by shawkins · Pull Request #47848 · keycloak/keycloak · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ public static boolean isJavaInFipsMode() {
return false;
}

/**
* Check if dev mode - note that non server commands may switch to non-server mode
* at runtime. See {@link #isNonServerMode()}
*/
public static boolean isDevMode() {
return DEV_PROFILE_VALUE.equalsIgnoreCase(getProfile());
}
Expand All @@ -80,6 +84,10 @@ public static String getProfile() {
return System.getenv(ENV_PROFILE);
}

/**
* Check if running in non-server mode - valid only as a runtime check.
* <br>At build time, we build as prod or dev.
*/
public static boolean isNonServerMode() {
return NON_SERVER_MODE.equalsIgnoreCase(Environment.getProfile());
}
Expand Down
6 changes: 2 additions & 4 deletions quarkus/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,12 @@ For debugging the build steps right after start, you can suspend the JVM by runn

**Expected behavior:** The server will start on **http://localhost:8080**.

When running using `quarkus:dev` you are able to do live coding whenever you change / add code in the `server` module, for example when creating a new custom provider.

There are currently limitations when running in development mode that block us to use all capabilities the Quarkus development mode has to offer.
The main limitations you'll find at the moment are:

* Changes are only automatically reflected at runtime if you are changing resources from the `deployment`, `runtime`, and `server` modules. Other modules, such as `keycloak-services` still rely on Hot Swap in Java debuggers to reload classes.
* Changes are only automatically reflected at runtime if you are changing resources from the quarkus `deployment`, `runtime`, and `server` modules. Non-quarkus modules, such as `keycloak-services`, still rely on Hot Swap in Java debuggers to reload classes.
* There is nothing in the Dev UI related to the server itself, although you can still change some configuration from there.
* There are some limitations when passing some options when running in dev mode. You should expect more improvements in this area.
* quarkus.args does not honor build time options. Those instead must also be set via -D arguments, or environment variables, and thus require rerunning the quarkus:dev command to change.

### Debugging the server distribution

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@
import org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider;
import org.keycloak.quarkus.runtime.configuration.PersistedConfigSource;
import org.keycloak.quarkus.runtime.configuration.PropertyMappingInterceptor;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
import org.keycloak.quarkus.runtime.configuration.mappers.WildcardPropertyMapper;
import org.keycloak.quarkus.runtime.integration.QuarkusKeycloakSessionFactory;
Expand Down Expand Up @@ -130,7 +129,6 @@
import io.quarkus.arc.deployment.BuildTimeConditionBuildItem;
import io.quarkus.arc.deployment.SyntheticBeanBuildItem;
import io.quarkus.bootstrap.logging.InitialConfigurator;
import io.quarkus.datasource.deployment.spi.DevServicesDatasourceResultBuildItem;
import io.quarkus.datasource.runtime.DataSourcesBuildTimeConfig;
import io.quarkus.deployment.IsDevelopment;
import io.quarkus.deployment.annotations.BuildProducer;
Expand Down Expand Up @@ -749,40 +747,11 @@ private void configureUserDefinedPersistenceUnits(List<PersistenceXmlDescriptorB
*
* @param configSources
*/
@BuildStep( )
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes configureConfigSources unused and never even called by Quarkus?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected.

@BuildStep
void configureConfigSources(BuildProducer<StaticInitConfigBuilderBuildItem> configSources) {
configSources.produce(new StaticInitConfigBuilderBuildItem(KeycloakConfigSourceProvider.class.getName()));
}

@BuildStep(>
void prepareTestEnvironment(BuildProducer< StaticInitConfigBuilderBuildItem> configSources, DevServicesDatasourceResultBuildItem dbConfig) {
configSources.produce(new StaticInitConfigBuilderBuildItem("org.keycloak.quarkus.runtime.configuration.test.TestKeycloakConfigSourceProvider"));

// we do not enable dev services by default and the DevServicesDatasourceResultBuildItem might not be available when discovering build steps
// Quarkus seems to allow that when the DevServicesDatasourceResultBuildItem is not the only parameter to the build step
// this might be too sensitive and break if Quarkus changes the behavior
if (dbConfig != null && dbConfig.getDefaultDatasource() != null) {
Map<String, String> configProperties = dbConfig.getDefaultDatasource().getConfigProperties();

for (Entry<String, String> dbConfigProperty : configProperties.entrySet()) {
PropertyMapper<?> mapper = PropertyMappers.getMapper(dbConfigProperty.getKey());

if (mapper == null) {
continue;
}

String kcProperty = mapper.getFrom();

if (kcProperty.endsWith("db")) {
// db kind set when running tests
continue;
}

System.setProperty(kcProperty, dbConfigProperty.getValue());
}
}
}

/**
* <p>Make the build time configuration available at runtime so that the server can run without having to specify some of
* the properties again.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ public static void setProfile(String profile) {
}
}

/**
* Check if the we're currently in or built as dev mode.
*/
public static boolean isDevMode() {
if (org.keycloak.common.util.Environment.isDevMode()) {
return true;
Expand All @@ -100,7 +103,7 @@ public static boolean isDevMode() {
}

public static boolean isDevProfile(){
return Optional.ofNullable(org.keycloak.common.util.Environment.getProfile()).orElse("").equalsIgnoreCase(org.keycloak.common.util.Environment.DEV_PROFILE_VALUE);
return org.keycloak.common.util.Environment.isDevMode();
}

public static boolean isWindows() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ public static void main(String[] args) {
ensureForkJoinPoolThreadFactoryHasBeenSetToQuarkus();
InfinispanUtils.ensureVirtualThreadsParallelism();

System.setProperty("kc.version", Version.VERSION);

Picocli picocli;
Properties clonedProps = null;
if (!(Thread.currentThread().getContextClassLoader() instanceof RunnerClassLoader)) {
picocli = new Picocli() { // embedded launch case, avoid System.exit
clonedProps = (Properties) System.getProperties().clone();
picocli = new Picocli() { // non-script launch case, avoid System.exit
@Override
public void exit(int exitCode) {
Quarkus.asyncExit(exitCode);
Expand All @@ -79,7 +79,16 @@ public void exit(int exitCode) {
} else {
picocli = new Picocli();
}
main(args, picocli);

System.setProperty("kc.version", Version.VERSION);

try {
main(args, picocli);
} finally {
if (clonedProps != null) {
reset(clonedProps);
}
}
}

public static void reset(Properties systemProperties) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ public void testCleanStartDev() {
NonRunningPicocli nonRunningPicocli = pseudoLaunch("start-dev");
assertFalse(nonRunningPicocli.getOutString(), nonRunningPicocli.getOutString().toUpperCase().contains("WARN"));
assertFalse(nonRunningPicocli.getOutString(), nonRunningPicocli.getOutString().toUpperCase().contains("ERROR"));
onAfter();
assertFalse(Environment.isDevProfile());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Properties;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;

Expand Down Expand Up @@ -178,6 +179,7 @@ public static Builder builder() {
private List<Dependency> dependencies;
private boolean fipsEnabled;
private Properties systemProperties;
private CountDownLatch closed;

public Keycloak() {
this(null, Version.VERSION, List.of(), false);
Expand Down Expand Up @@ -212,6 +214,8 @@ private Keycloak start(List<String> args) {
return this;
}
StartupAction startupAction = action.createInitialRuntimeApplication();
closed = new CountDownLatch(1);
startupAction.addRuntimeCloseTask(closed::countDown);
application = startupAction.runMainClass(args.toArray(new String[0]));

return this;
Expand Down Expand Up @@ -304,6 +308,11 @@ private void closeApplication() {
} catch (Exception cause) {
cause.printStackTrace();
}
try {
closed.await(); // wait for an orderly completion of all cleanup in the other thread
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is a test framework class, it almost doesn't matter, but in general, I can't make sense of this, can you elaborate what it solves? Because the comment says:

// wait for an orderly completion of all cleanup in the other thread

However, the stack should be: org.keycloak.Keycloak#closeApplication -> io.quarkus.bootstrap.app.RunningQuarkusApplication#close -> closeable from io.quarkus.runner.bootstrap.StartupActionImpl#runMainClass which runs these close tasks where you added CountDownLatch. And they are not strictly speaking the last to run (e.g. there is RuntimeApplicationShutdownBuildItem and few other actions). That may be irrelevant since we shouldn't rely on the impl. details anyway.

So this only makes sense if the io.quarkus.bootstrap.app.RunningQuarkusApplication#close has some asynchronous tasks, otherwise regardless of the thread switching it should only end the close invocation when all the tasks are done. If it doesn't work this way, I think it is dangerous considering all the class loader switching they are doing.

@shawkins shawkins Apr 9, 2026 •
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is a test framework class, it almost doesn't matter, but in general, I can't make sense of this, can you elaborate what it solves? Because the comment says:

It comes down to the cleanup I'm trying to do here: https://github.com/keycloak/keycloak/pull/47848/changes#diff-c1907932c84285e376fbc3a66277c655f30b91476db47834c3f5a81a7a45b319R89 - that is within the quarkus classloader after the application has been stopped. The quarkus classloader seems to be reused for dev mode reload - for example editing the command line.

That logic also restores the previous system properties - we unfortunately make extensive use of setting those.

So this only makes sense if the io.quarkus.bootstrap.app.RunningQuarkusApplication#close has some asynchronous tasks

application.close() completes asynchronously from the cleanup being done in https://github.com/keycloak/keycloak/pull/47848/changes#diff-c1907932c84285e376fbc3a66277c655f30b91476db47834c3f5a81a7a45b319R89

There is further cleanup performed by the test logic when running in embedded mode - because it also modifies static state, including system properties. So I don't want these to trample each other.

An alternative would be to only do the https://github.com/keycloak/keycloak/pull/47848/changes#diff-c1907932c84285e376fbc3a66277c655f30b91476db47834c3f5a81a7a45b319R89 cleanup if running in dev mode, but I was thinking that having a more sequential startup and shutdown for embedded may be good in general.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case it makes sense to add the startup task you are adding, I didn't realize there was the async exit. It doesn't eliminate every side-effect, but hopefully good enough for tests.

} catch (InterruptedException e) {
e.printStackTrace();
}
}

QuarkusConfigFactory.setConfig(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.stream.Stream;

Expand All @@ -34,16 +33,13 @@
import org.keycloak.quarkus.runtime.cli.command.DryRunMixin;
import org.keycloak.quarkus.runtime.cli.command.Start;
import org.keycloak.quarkus.runtime.cli.command.StartDev;
import org.keycloak.quarkus.runtime.configuration.Configuration;

import io.quarkus.deployment.util.FileUtil;
import io.quarkus.runtime.configuration.QuarkusConfigFactory;
import io.quarkus.test.junit.QuarkusMainTestExtension;
import io.quarkus.test.junit.main.Launch;
import io.quarkus.test.junit.main.LaunchResult;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ExtensionContext.Namespace;
import org.junit.jupiter.api.extension.ExtensionContext.Store;
import org.junit.jupiter.api.extension.ParameterContext;
import org.junit.jupiter.api.extension.ParameterResolutionException;
import org.junit.jupiter.api.extension.ReflectiveInvocationContext;
Expand All @@ -56,7 +52,6 @@

public class CLITestExtension extends QuarkusMainTestExtension {

private static final String SYS_PROPS = "sys-props";
private KeycloakDistribution dist;
private DatabaseContainer databaseContainer;
private InfinispanContainer infinispanContainer;
Expand All @@ -66,7 +61,6 @@ public class CLITestExtension extends QuarkusMainTestExtension {
public void beforeEach(ExtensionContext context) throws Exception {
DistributionTest distConfig = getDistributionConfig(context);
Launch launch = context.getRequiredTestMethod().getAnnotation(Launch.class);
getStore(context).put(SYS_PROPS, new HashMap<>(System.getProperties()));

if (launch != null && distConfig == null) {
Stream.of(launch.value()).forEach(arg -> {
Expand Down Expand Up @@ -117,10 +111,6 @@ public void beforeEach(ExtensionContext context) throws Exception {
}
}

private Store getStore(ExtensionContext context) {
return context.getStore(Namespace.create(context.getRequiredTestClass(), context.getRequiredTestMethod()));
}

private static Storage getStoreConfig(ExtensionContext context) {
return context.getTestClass().get().getDeclaredAnnotation(Storage.class);
}
Expand Down Expand Up @@ -193,12 +183,6 @@ public void afterEach(ExtensionContext context) throws Exception {

private void reset(DistributionTest distConfig, ExtensionContext context) {
QuarkusConfigFactory.setConfig(null);
HashMap props = getStore(context).remove(SYS_PROPS, HashMap.class);
System.getProperties().clear();
System.getProperties().putAll(props);
// TODO: for in-vm tests this is not all that it takes to reset static state
// may want to call AbstractConfigurationTest.resetConfiguration
Configuration.resetConfig();
if (databaseContainer != null && databaseContainer.isRunning()) {
databaseContainer.stop();
databaseContainer = null;
Expand Down
Loading
0