-
-
Notifications
You must be signed in to change notification settings - Fork 512
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: include PostReady hook, defining proper execution order for container lifecycle hooks #1922
feat: include PostReady hook, defining proper execution order for container lifecycle hooks #1922
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
I think this still doesn't make clear how this hook execution order related to the container's readiness. I just found it by looking at the code and realizing the |
You mean in the docs and in this PR description? PTAL at https://deploy-preview-1922--testcontainers-go.netlify.app/features/creating_container/#lifecycle-hooks, the Info section |
In that sense I believe we need to always run the default wait strategies before the user-defined post-start hooks. Otherwise we can see a module trying to run commands even before the readyness (defined by the wait strategies) is not complete. I'm tempted to always prepend the wait strategies to the post-start hooks. Wdyt? @kiview we have already discussed about this today |
This is how this works now and precisely the problem I mentioned in the slack. The changes introduced in this PR change that and allow running a post-strart hook before the wait condition, and then use a wait condition that verifies the effects of such hook. maybe I'm missing something here, but what you propose seems to take us back to the current behavior |
Documentation. In the proposed changes you mention this in a info note regarding the default hooks, but I think the definition of Post-start hook should be more specific, because "started" and "ready" (post wait condition meets) are in my opinion two different things and make create confusion. |
@pablochacin I've updated the PR and description to reflect the new |
* main: feat: support for executing commands in a container with user, workDir and env (testcontainers#1914) fix(modules.kafka): Switch to MaxInt for 32-bit support (testcontainers#1923)
* main: (24 commits) chore(deps): bump github.com/ClickHouse/clickhouse-go/v2 (testcontainers#1982) chore(deps): bump github.com/twmb/franz-go in /modules/redpanda (testcontainers#1973) chore(deps): bump google.golang.org/api from 0.152.0 to 0.153.0, cloud.google.com/go/bigtable from 1.20.0 to 1.21.0 and cloud.google.com/go/spanner from 1.53.0 to 1.53.1 in /modules/gcloud (testcontainers#1983) chore(deps): bump github.com/aws/aws-sdk-go and github.com/aws/aws-sdk-go-v2 in /modules/localstack (testcontainers#1981) chore(deps): bump mkdocs-include-markdown-plugin from 6.0.1 to 6.0.4 (testcontainers#1974) feat: add module to support Microsoft SQL Server (testcontainers#1969) chore(deps): bump github.com/shirou/gopsutil/v3 from 3.23.10 to 3.23.11 (testcontainers#1943) chore(deps): bump golang.org/x/mod in /modules/kafka (testcontainers#1956) chore(deps): bump golang.org/x/sys from 0.13.0 to 0.15.0 (testcontainers#1944) chore(deps): bump golang.org/x/text and golang.org/x/mod from 0.13.0 to 0.14.0 in /modulegen (testcontainers#1968) chore(deps): bump go.mongodb.org/mongo-driver in /modules/mongodb (testcontainers#1960) chore(deps): bump github.com/couchbase/gocb/v2 in /modules/couchbase (testcontainers#1952) chore(deps): bump github.com/elastic/go-elasticsearch/v8 from 8.10.1 to 8.11.1 and golang.org/x/mod from 0.13.0 to 0.14.0 in /modules/elasticsearch (testcontainers#1967) chore(deps): bump github.com/aws/aws-sdk-go and github.com/aws/aws-sdk-go-v2 in /modules/localstack (testcontainers#1953) chore(deps): bump actions/github-script from 6.4.1 to 7.0.1 (testcontainers#1949) chore(deps): bump github.com/IBM/sarama in /modules/kafka (testcontainers#1955) chore(deps): bump github.com/neo4j/neo4j-go-driver/v5 in /modules/neo4j (testcontainers#1961) chore(deps): bump github.com/compose-spec/compose-go from 1.20.0 to 1.20.2 and github.com/docker/compose/v2 from 2.23.0 to 2.23.3 in /modules/compose (testcontainers#1966) chore(deps): bump google.golang.org/api from 0.143.0 to 0.152.0 and cloud.google.com/go/spanner from 1.50.0 to 1.53.0 in /modules/gcloud (testcontainers#1965) chore(deps): bump mkdocs-include-markdown-plugin from 6.0.1 to 6.0.4 (testcontainers#1934) ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
* main: (25 commits) chore(deps): bump github.com/dvsekhvalnov/jose2go in /modules/pulsar (testcontainers#2136) fix: skip-host-cache option removed in latest MySQL 8.3.0 version (testcontainers#2130) chore: skip assertions for Docker Rootless (testcontainers#2135) pin Docker images version (testcontainers#2129) enable golangci-lint for examples (testcontainers#2128) chore(deps): bump github.com/neo4j/neo4j-go-driver/v5 in /modules/neo4j (testcontainers#2098) enable golangci-lint for redis module (testcontainers#2126) Go install gotestsum and golangci-lint (testcontainers#2127) improve OSSF score (testcontainers#2125) chore: run make lint on new modules (testcontainers#2122) enable golangci-lint for pulsar (testcontainers#2121) lint: enable testifylint (testcontainers#2120) chore(deps): bump github.com/nats-io/nats.go in /modules/nats (testcontainers#2094) chore(deps): bump golang.org/x/sys from 0.15.0 to 0.16.0 (testcontainers#2104) Revert "chore(deps): bump actions/upload-artifact from 3.1.3 to 4.0.0 (testcontainers#2088)" chore(deps): bump actions/upload-artifact from 3.1.3 to 4.0.0 (testcontainers#2088) chore(deps): bump cloud.google.com/go/spanner from 1.54.0 to 1.55.0, google.golang.org/api from 0.154.0 to 0.156.0 in /modules/gcloud (testcontainers#2115) chore(deps): bump github.com/aws/aws-sdk-go-v2/config from 1.25.10 to 1.26.3, github.com/aws/aws-sdk-go from 1.48.13 to 1.49.19 in /modules/localstack (testcontainers#2114) chore(deps): bump github.com/docker/go-connections from 0.4.0 to 0.5.0 (testcontainers#2113) Adding mockserver module (testcontainers#2085) ...
* main: (33 commits) feat (postgres): support for creating and restoring Snapshots (testcontainers#2199) fix: apply volume options only to volumes (testcontainers#2201) redpanda/test: add admin client call (testcontainers#2200) chore(deps): bump cloud.google.com/go/spanner from 1.55.0 to 1.56.0 in /modules/gcloud, cloud.google.com/go/pubsub from 1.35.0 to 1.36.1 in /modules/gcloud, cloud.google.com/go/bigquery from 1.57.1 to 1.58.0 in /modules/gcloud (testcontainers#2197) chore(deps): bump github.com/docker/docker from 25.0.1+incompatible to 25.0.2+incompatible (testcontainers#2196) fix: go doc reference broken image (testcontainers#2195) Add Support for WASM Transforms to Redpanda Module (testcontainers#2170) feat(modules.clickhouse): Add zookeeper for clickhouse clusterization (testcontainers#1995) redpanda: allow using SASL and TLS together (testcontainers#2140) chore: do not panic in testable examples (testcontainers#2193) fix: all mounts should contain the testcontainers labels (testcontainers#2191) [redpanda] sasl test for wrong mechanism (testcontainers#2048) fix: deprecate BindMounts correctly (testcontainers#2190) chore(docker_mounts): stop doing misleading logging (testcontainers#2178) fix: Add HTTPStrategy WithForcedIPv4LocalHost To Fix Docker Port Map (testcontainers#1775) chore(deps): bump github.com/docker/compose/v2 in /modules/compose (testcontainers#2162) feat(modules.cockroachdb) Adds cockroachdb module (testcontainers#2131) chore(deps): bump golang.org/x/crypto in /modules/minio (testcontainers#2161) chore(deps): bump golang.org/x/crypto in /modules/openldap (testcontainers#2165) chore(deps): bump github.com/google/uuid from 1.5.0 to 1.6.0 (testcontainers#2169) ...
0b7c18b
to
3fa1e37
Compare
What does this PR do?
This PR internally changes how the container lifecycle hooks are executed. Before these changes:
It could be OK, but it is not allowing users to perform tasks related to the warm-up of a container:
With the changes in this PR, we are updating the execution order to:
With this order, we make sure that users can define their stuff at the right place in the container life cycle.
In order to achieve this order, we are combining default and user-defined hooks into one single hook, and that will be the one finally added to the container request. We have added unit tests to verify that the order is honoured.
To enrich the experience, we are defining a new container lifecycle event: Ready, which happens right after the PostStart event. This
Ready
event definesPostReadies
hooks only. This new event will represent the execution of the wait strategies, which is the Testcontainers representation for Readiness.Finally, given the PR touches the container lifecycles, we are refining the log messages in the default logging hooks, always using ✅ for the default post hooks.
Why is it important?
Define a consistent container life cycl, allowing users to act before the default hooks.
Related issues