Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for reusing Testcontainers-backed “dev services” containers across API server restarts, aiming to speed up local development by keeping stateful containers (e.g., PostgreSQL) running.
Changes:
- Introduces a new configuration flag to enable/disable dev-services container reuse.
- Configures PostgreSQL/Kafka/frontend dev-service containers with labels and
withReuse(...). - Adjusts Kafka topic creation to tolerate existing topics when reusing containers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| apiserver/src/main/resources/application.properties | Adds dev.services.container-reuse-enabled configuration and docs for container reuse. |
| apiserver/src/main/java/org/dependencytrack/dev/DevServicesInitializer.java | Enables container reuse/labels, tolerates “topic already exists”, and conditionally skips stopping containers on shutdown. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Defines whether dev services containers should be reused. | ||
| # <br/><br/> | ||
| # Refer to <https://java.testcontainers.org/features/reuse/> for details. | ||
| # | ||
| # @category: Development | ||
| # @type: boolean | ||
| dev.services.container-reuse-enabled=true |
There was a problem hiding this comment.
The new dev.services.container-reuse-enabled setting is documented, but it doesn’t mention two important behavioral constraints of Testcontainers reuse: (1) reuse is ignored unless TESTCONTAINERS_REUSE_ENABLE=true or testcontainers.reuse.enable=true is set, and (2) when reuse is enabled the containers are intentionally not disposed on shutdown. As written, this also conflicts with the earlier dev-services description that says containers are disposed when Dependency-Track stops. Please update the property docs (and/or the dev-services section above) to reflect the prerequisite and the changed shutdown behavior so users don’t end up with unexpected lingering containers.
| public void contextDestroyed(final ServletContextEvent event) { | ||
| if (postgresContainer != null) { | ||
| if (postgresContainer != null && !isContainerReuseEnabled) { | ||
| LOGGER.info("Stopping postgres container"); | ||
| try { | ||
| postgresContainer.close(); | ||
| } catch (Exception e) { | ||
| LOGGER.error("Failed to stop PostgreSQL container", e); | ||
| } | ||
| } | ||
| if (kafkaContainer != null) { | ||
| if (kafkaContainer != null && !isContainerReuseEnabled) { | ||
| LOGGER.info("Stopping Kafka container"); | ||
| try { | ||
| kafkaContainer.close(); | ||
| } catch (Exception e) { | ||
| LOGGER.error("Failed to stop Kafka container", e); | ||
| } | ||
| } | ||
| if (frontendContainer != null) { | ||
| if (frontendContainer != null && !isContainerReuseEnabled) { | ||
| LOGGER.info("Stopping frontend container"); |
There was a problem hiding this comment.
contextDestroyed() skips closing containers solely based on dev.services.container-reuse-enabled. In Testcontainers, withReuse(true) has no effect unless reuse is enabled globally (e.g. testcontainers.reuse.enable=true), so this can leave non-reusable containers running (and keep host ports bound) when the webapp is stopped/redeployed without a JVM exit. Consider only skipping close() when Testcontainers reports that the environment supports reuse, otherwise always close containers on shutdown.
Allows database state to be reused across restarts, and more generally makes startup faster. Signed-off-by: nscuro <nscuro@protonmail.com>
617dfcf to
0a7997f
Compare
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
Description
Supports reuse of dev containers.
Allows database state to be reused across restarts, and more generally makes startup faster.
Addressed Issue
N/A
Additional Details
N/A
Checklist
This PR fixes a defect, and I have provided tests to verify that the fix is effectiveThis PR implements an enhancement, and I have provided tests to verify that it works as intendedThis PR introduces changes to the database model, and I have updated the migration changelog accordinglyThis PR introduces new or alters existing behavior, and I have updated the documentation accordingly