spring-boot icon indicating copy to clipboard operation
spring-boot copied to clipboard

Contributing Dynamic Properties at Development Time is not working with @RestartScope

Open eddumelendez opened this issue 1 year ago • 8 comments

I’ve been playing around with several examples using Spring Boot Testcontainers at Development time and last night I noticed those are not working as expected with @RestartScope. IIRC this example used to work in the past, download the image, start the container and so on. I also tried something similar to what’s mentioned in the docs but the image is not downloaded at all in this example if @ServiceConnection is removed. It works as expected without @RestartScope

eddumelendez avatar Jun 08 '23 10:06 eddumelendez

Hello @philwebb, i would like to fix this. Please assign it to me.

nedimAT avatar Jun 23 '23 07:06 nedimAT

Thanks @nedimAT, I've assigned the issue to you. I've no idea if it's an easy or difficult fix so feel free to comment here if you don't get anywhere.

philwebb avatar Jun 23 '23 21:06 philwebb

Hello @eddumelendez, for me the rabbitmq example works as expected. Removing the @ServiceConnection Annotation means you have to setup the properties yourself, see docs dynamic-properties The Pulsar example i can not get to run on my machine, caused by this bug Can you provide further information about your issue and probably some logs? Your logs should contain some insights about what testcontainers is up to, similar to the following tc.rabbitmq:3.11-alpine : Pulling docker image: rabbitmq:3.11-alpine. Please be patient; this may take some time but only needs to be done once.

nedimAT avatar Jun 27 '23 06:06 nedimAT

I have another example. See https://github.com/eddumelendez/spring-boot-sqs-testcontainers-reusable-mode

In summary, @RestartScope works with existing supported ServiceConnections but not when used along with DynamicProperties. Not close at the computer right now but in those scenarios is like Testcontainers is ignored and not picked up. You should be able to reproduce with this last example. If needed I can attach logs later.

eddumelendez avatar Jun 27 '23 11:06 eddumelendez

Thanks, with that example i can reproduce it. I will start looking into it.

nedimAT avatar Jun 27 '23 14:06 nedimAT

Hello @philwebb could you provide me with some hint where i should start. What i know so far is, that @RestartScope on its own is not enough for the Container to be started. One needs additionally @Scope or @ServiceConnection, but then the Container will restart with the Application. I searched now for some time in the DefaultListableBeanFactory for clues but i think i am stuck.

nedimAT avatar Jul 11 '23 05:07 nedimAT

I did another check and wonder if just changing the following lines

https://github.com/spring-projects/spring-boot/blob/b8c4fb6b9a20bad8ffa6d781a8bf38681562ffed/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/lifecycle/TestcontainersLifecycleBeanPostProcessor.java#L84-L85

to

beanNames.addAll(List.of(this.beanFactory.getBeanNamesForType(ContainerState.class, true, false)));
beanNames.addAll(List.of(this.beanFactory.getBeanNamesForType(Startable.class, true, false)));

due to the scope is not singleton anymore but restart.

eddumelendez avatar Jul 15 '23 08:07 eddumelendez

I have another example. See https://github.com/eddumelendez/spring-boot-sqs-testcontainers-reusable-mode

Hi, @eddumelendez, @philwebb this example is DynamicPropertyRegistry at Development Time is not working with @RestartScope. DynamicPropertyRegistry need init property before SqsAsyncClient.

But when add @RestartScope, scope change to restart and skip code DefaultListableBeanFactory.preInstantiateSingletons , so DynamicPropertyRegistry init property after SqsAsyncClient.

Is it reasonable to add a tip to the documentation?

Wzy19930507 avatar Jan 22 '24 09:01 Wzy19930507

Thanks for the hint @eddumelendez. Hopefully fixed in the next snapshot.

philwebb avatar Jun 26 '24 05:06 philwebb

Thanks @philwebb ! It's working :)

eddumelendez avatar Jun 26 '24 05:06 eddumelendez

I've been following this ticket since before it got renamed and with the latest 3.3.2-SNAPSHOT The combination of @RestartScope and DynamicPropertyRegistry does work on startup but all the properties get unset after dev-tools reloads the app. Previously the properties didn't get set at all so it's definitely an improvement but I'm not sure this fully fixes the issue.

shawnweeks avatar Jul 17 '24 20:07 shawnweeks

@shawnweeks Does the discussion in #35200 and the documentation changes made in this related commit help with your use case?

scottfrederick avatar Jul 17 '24 21:07 scottfrederick

@scottfrederick They don't appear to. This issue was originally about Contributing Dynamic Properties at Development Time is not working with @RestartScope which is what still doesn't seem to be working. This issue seems to resolve part of the original issue but dynamic properties still don't fully work with RestartScope. Don't mind starting another ticket just don't want it to get closed as duplicate since this ticket appeared to be trying to resolve the same issue.

shawnweeks avatar Jul 17 '24 21:07 shawnweeks