-
-
Notifications
You must be signed in to change notification settings - Fork 544
feat(couchbase): adding auth to couchbase initCluster functions to support container reuse #3048
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(couchbase): adding auth to couchbase initCluster functions to support container reuse #3048
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
We had a similar issue in Testcontainers for Java and was fixed by skipping the configuration when reuse is enabled testcontainers/testcontainers-java#9957 |
How would you handle the issue of not knowing which of the calls would be the one to be the first to cause the initialization, as well as returning the container too early in ones where is was skipped (i.e. before the configuration occurred)? |
* main: (76 commits) chore(deps): bump mkdocs-include-markdown-plugin from 6.2.2 to 7.1.5 (testcontainers#3137) chore(deps): bump github.com/shirou/gopsutil/v4 from 4.25.1 to 4.25.4 (testcontainers#3133) chore(deps): bump github.com/docker/docker from 28.0.1+incompatible to 28.1.1+incompatible (testcontainers#3152) feat(memcached): add memcached module (testcontainers#3132) fix(etcd): single node etcd cluster access (testcontainers#3149) feat(valkey): add TLS support for Valkey (testcontainers#3131) fix(dockermodelrunner): wait for the model to be pulled (testcontainers#3125) fix(localstack): remove checksum before parsing version (testcontainers#3130) fix(dockermodelrunner): dependency with socat chore: prepare for next minor development cycle (0.38.0) chore: use new version (v0.37.0) in modules and examples fix: handle stopped containers more gracefully when reuse is enabled (testcontainers#3062) feat(gcloud): add option to run firestore in datastore mode (testcontainers#3009) feat: support for mounting images (testcontainers#3044) chore(ci): close PR if it was sent from main (testcontainers#3123) feat: add `WithReuseByName` for modifying Generic Container Requests (testcontainers#3064) chore(deps): bump github/codeql-action from 3.28.15 to 3.28.16 (testcontainers#3120) chore(deps): bump mkdocs-include-markdown-plugin from 6.2.2 to 7.1.5 (testcontainers#3119) chore(deps): bump github.com/magiconair/properties from 1.8.9 to 1.8.10 (testcontainers#3118) chore(ci): exclude more files for a full-blown build (testcontainers#3122) ...
* main: feat: support adding wait strategies as functional option (testcontainers#3161) fix(etcd): expose ports for the etcd nodes (testcontainers#3162) fix(wait): no port to wait for (testcontainers#3158) feat: add more functional options for customising containers (testcontainers#3156) docs(redpanda): update sasl authentication option to use scram sha 256 (testcontainers#3126)
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, thanks!
@waroir20 I've already approved this PR, although I'd like you to take a look at the current https://github.com/testcontainers/testcontainers-go/blob/main/docker.go#L1367-L1382 // If a container was stopped programmatically, we want to ensure the container
// is running again, but only if it is not paused, as it's not possible to start
// a paused container. The Docker Engine returns the "cannot start a paused container,
// try unpause instead" error.
switch c.State {
case "running":
// cannot re-start a running container, but we still need
// to call the startup hooks.
case "paused":
// TODO: we should unpause the container here.
return nil, fmt.Errorf("cannot start a paused container: %w", errors.ErrUnsupported)
default:
if err := dc.Start(ctx); err != nil {
return dc, fmt.Errorf("start container %s in state %s: %w", req.Name, c.State, err)
}
} Tell me what you think, and we could maybe merge this as is, and work on the refactor as a follow-up Thanks! |
Merged, thanks for your work here! Just in case you have time and are interested, I'd like you to check my comment here 🙏 Cheers! |
What does this PR do?
This PR aims to enable a created couchbase container to be reused between test packages, and accomplishes this by passing the basic auth header with every request made to the container's management ports.
Why is it important?
Since Go's test context creates physically separated binaries between packages, re-using a created container directly using the testcontainer's framework is impossible; so to accomplish the same thing utilizing testcontainer's framework we can configure the GenericContainer with a fixed container name. This fixed container name then allows other test packages within an application to reference the already created container minimizing resource consumption and repeated start up cost.
Currently in applications with lots of tests/slow running tests this singleton couchbase container starts up and runs just fine when tests attempt to create their own container via
Run
but once the Admin user has been enabled by the first test, all subsequent tests fail with "init cluster: context deadline exceeded" because the http requests are failing with 401.Related Issues
context deadline exceeded
when reused #3049Follow-ups
It is likely that other implementations of testcontainers for other common images will have a similar issue when the container is reused.