Repository navigation
Replies: 1 comment
|
Even as we're started to refine our CLI handling, our legacy handling introduces a subtly that needs highlighted:
means the start will run with --metrics-enabled=true, even though the default is false. only if I explicitly add --metrics-enabled=false, will it error. Note this isn't specific to the cli. The same behavior exists for ENV or keycloak.conf. If I remove an entry after building, that may still get used by start.
Actually I misremembered this behavior - we do check the full build time state for a non optimized start. For --optimized if we want to support what users are currently required to do - that is remove all the build-time options, then we need to retain the legacy behavior. cc @keycloak/cloud-native |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There are a couple of issues that may be confusing users with build, auto-builds, and --optimized:
We somewhat interchangably use the terms build and (re-)augmentation, but refer to the output as optimized, not built nor augmented.
When you specify --optimized build-time cli options no longer show up - however under the covers we still pick up any build-time options specified in other config sources and must validate that they aren't changing. It seems unnecessary to continue to make this distinction specifically for cli options.
For those in dev scenarios, or trying to use the same keycloak deployment location to run all commands, it may not be clear what the persisted / augmented state is. Users should not use --optimized in this case.
--optimized is supposed to allow a fast start mode that skips a couple of milliseconds of picocli handling - however it is very fragile. Any other option, such as --verbose or a runtime option on the cli will prevent a fast start - see --verbose causes --optimized fast start to not be used #38790
Alternatives:
The most aggressive is to do away with --optimized altogether. There are two paths this could take:
We'll run the built-time check on every applicable command, and rebuild when needed. The downside of this approach is there is no "safety" for those using optimized images - they will have to check the logs to ensure that reaugmentation is happening only when expected.
If you run a build (or alternatively named command), then the build time state for each subsequent applicable command will be checked against the persisted state (we already persist an optimized boolean to track if a formal build was run). If build-time state does not match, then we'll throw an exception indicating a build is needed. We simply won't worry about the fast start behavior for skipping picocli - or will look at alternative stratigies for that optimization. This may also require the introdution of something like "build --clear" to remove the persisted / augmented state - if we were clever about this, it would not run through the QuarkusEntryPoint and/or directly delete our persisted properties and lib/quarkus state - so that it would be run even if classloading would be otherwise corrupted Keycloak Fails to Start: Unable to Update or Pick Latest Custom Providers in quarkus-application.dat File #38221 (comment)
This still may not work for all users with optimized images as it bakes in that it must be used in an optimized fashion. I'm not sure however how many are in this case - there could be a subset of those using the optimized=false flag on the Keycloak CR who want behavior like this. If that's the case we can consider whether it makes sense to have the operator and other logic issue a separate build command, or simply require the user rebuild their image with the build time update instead of putting it in the CR.
The alternative to "build --clear" and how to support the optimized=false behavior described above is to instead have an "--auto-build" flag.
cc @keycloak/cloud-native
All reactions