fix(postgres): Fix the non-default dbname error #2489
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The linked issue described in great detail an issue where we assumed everyone would use the default database user, whose home DB defaults to the postgres database. When that was not the case, the snapshots would fail silently as the user would not connect to the right database to take the commands. This PR fixes the issue by adding the dbname by default in the command, and adds a test to validate this works as intended. In addition, it also adds some logic to handle any error that does not cause the exec command to fail, such as database access failures. Run the added test to test this works as intended. Closes testcontainers#2474
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
mdelapenya
reviewed
Apr 18, 2024
There was a problem hiding this comment.
The changes LGTM, thanks for the quick fix! I was seeing this behaviour too in other internal projects, so very happy we have it fixed here.
BTW, would you mind adding docs regarding the behaviours described here: e.g., not using the default postgres db for snapshots.
Thank you!
On it, it was something I noticed while writing the new tests. You can't drop the postgres DB and we use it as the connected DB when restoring the snapshot (can't drop the DB you're connected to!), so I thought I'd add a better error message for that specific error. |
mdelapenya
approved these changes
Apr 18, 2024
mdelapenya
changed the title
mdelapenya
added a commit
to mdelapenya/testcontainers-go
that referenced
this pull request
Apr 18, 2024
* main: fix(postgres): Fix the non-default dbname error (testcontainers#2489)
mdelapenya
pushed a commit
to mdelapenya/testcontainers-go
that referenced
this pull request
Apr 23, 2024
* Fix the non-default dbname error The linked issue described in great detail an issue where we assumed everyone would use the default database user, whose home DB defaults to the postgres database. When that was not the case, the snapshots would fail silently as the user would not connect to the right database to take the commands. This PR fixes the issue by adding the dbname by default in the command, and adds a test to validate this works as intended. In addition, it also adds some logic to handle any error that does not cause the exec command to fail, such as database access failures. Run the added test to test this works as intended. Closes testcontainers#2474 * Document the postgres dbname issue in the docs
mdelapenya
added a commit
to mdelapenya/testcontainers-go
that referenced
this pull request
Apr 23, 2024
* main: fix: don't retry on permanent APIClient errors (testcontainers#2506) feat: support overriding the default recreate options for compose (testcontainers#2511) feat: support passing io.Reader for compose files when creating a compose instance (testcontainers#2509) chore: add funding button for testcontainers (testcontainers#2510) feat: support Ryuk for the compose module (testcontainers#2485) chore(deps): bump golang.org/x/net in modules (minio, gcloud, weaviate, compose, qdrant, couchbase, k3s, milvus, mockserver, pulsar, kafka) (testcontainers#2505) fix: fallback to URL-path when parsing auth config URL without scheme (testcontainers#2488) fix(postgres): Fix the non-default dbname error (testcontainers#2489) feat: Bump default postgres version (testcontainers#2481) support Dolt (testcontainers#2177) chore: create TLS certs in a consistent manner (testcontainers#2478) chore(deps): bump idna from 3.6 to 3.7 (testcontainers#2480) Elasticsearch disable CA retrieval when ssl is disabled (testcontainers#2475) fix: handle dockerignore exclusions properly (testcontainers#2476)
mdelapenya
added a commit
that referenced
this pull request
Apr 24, 2024
* chore: start a foundational package for interacting with Docker networks * feat: add an SSH tunnel forwarding a host port to a container * fix: rename struct * chore: pass the original context to the exposeHostPorts function * chore: start tunnel using context * chore: push goroutines to the method where they are used * fix: proper eval of first network * fix: handle dockerignore exclusions properly (#2476) * chore: only include the dockerignore if it contains ignore files * fix: the inclusions must be relative to the context * docs: document the dockerignore feature * chore: only include the dockerignore file if it exists * Elasticsearch disable CA retrieval when ssl is disabled (#2475) * skip search for CACert if ssl has been turned off * add tests with and without ssl enabled * add all config keys that disable CA gen, restrict check to version 8 * rename test to match content * chore(deps): bump idna from 3.6 to 3.7 (#2480) Bumps [idna](https://github.com/kjd/idna) from 3.6 to 3.7. - [Release notes](https://github.com/kjd/idna/releases) - [Changelog](https://github.com/kjd/idna/blob/master/HISTORY.rst) - [Commits](kjd/idna@v3.6...v3.7) --- updated-dependencies: - dependency-name: idna dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: create TLS certs in a consistent manner (#2478) * fix: remove suspicious filepath.Join * chore: fix lint * fix: handle error * chore: reverse assertion for lint * feat: support generating TLS certificates on the fly * chore: apply to cockroachdb * chore: support saving the cert and priv key files to disk * chore: apply to rabbitmq * chore: simplify * chore: use in redpanda module * chore: lint * chore: set validFrom internally * fix: properly use the new API in redpanda * docs: document the TLS helpers * chore: simplify WithParent to accept the struct directly * chore: use tlscert package instead * fix: use non-deprecated API * docs: update * docs: fix examples * chore: use released version of tlscert * fix: add common name for the node cert * support Dolt (#2177) * /modules/dolt: wip, kinda working * /modules/dolt: get tests passing * /{.github,.vscode,docs,mkdocs,modules,sonar-project}: use modulegen tool * /modules/dolt/{dolt.go,examples_test.go}: run linter * /modules/dolt/{dolt.go,examples_test.go}: add methods for cloning * /{docs, modules}: add with creds file * /{docs,modules}: pr feedback, cleanup * /modules/dolt/examples_test.go: remove panics, lint * chore: run mod tidy * chore: include MustConnectionString method * chore: do not use named returns * chore: perform initialisation before the container has started --------- Co-authored-by: Manuel de la Peña <mdelapenya@gmail.com> * feat: Bump default postgres version (#2481) * Bump default postgres version * Bump to use latest pg * Bump version from non-ancient version --------- Co-authored-by: bstrausser <bstrausser@locusrobotics.com> * fix(postgres): Fix the non-default dbname error (#2489) * Fix the non-default dbname error The linked issue described in great detail an issue where we assumed everyone would use the default database user, whose home DB defaults to the postgres database. When that was not the case, the snapshots would fail silently as the user would not connect to the right database to take the commands. This PR fixes the issue by adding the dbname by default in the command, and adds a test to validate this works as intended. In addition, it also adds some logic to handle any error that does not cause the exec command to fail, such as database access failures. Run the added test to test this works as intended. Closes #2474 * Document the postgres dbname issue in the docs * fix: fallback to URL-path when parsing auth config URL without scheme (#2488) * chore(deps): bump golang.org/x/net in modules (minio, gcloud, weaviate, compose, qdrant, couchbase, k3s, milvus, mockserver, pulsar, kafka) (#2505) * chore(deps): bump golang.org/x/net in /modules/kafka Bumps [golang.org/x/net](https://github.com/golang/net) from 0.17.0 to 0.23.0. - [Commits](golang/net@v0.17.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump golang.org/x/net in /modules/pulsar Bumps [golang.org/x/net](https://github.com/golang/net) from 0.17.0 to 0.23.0. - [Commits](golang/net@v0.17.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump golang.org/x/net in /modules/mockserver Bumps [golang.org/x/net](https://github.com/golang/net) from 0.17.0 to 0.23.0. - [Commits](golang/net@v0.17.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump golang.org/x/net in /modules/milvus Bumps [golang.org/x/net](https://github.com/golang/net) from 0.17.0 to 0.23.0. - [Commits](golang/net@v0.17.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump golang.org/x/net from 0.19.0 to 0.23.0 in /modules/k3s Bumps [golang.org/x/net](https://github.com/golang/net) from 0.19.0 to 0.23.0. - [Commits](golang/net@v0.19.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump golang.org/x/net in /modules/couchbase Bumps [golang.org/x/net](https://github.com/golang/net) from 0.20.0 to 0.23.0. - [Commits](golang/net@v0.20.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump golang.org/x/net in /modules/qdrant Bumps [golang.org/x/net](https://github.com/golang/net) from 0.20.0 to 0.23.0. - [Commits](golang/net@v0.20.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump golang.org/x/net in /modules/compose Bumps [golang.org/x/net](https://github.com/golang/net) from 0.20.0 to 0.23.0. - [Commits](golang/net@v0.20.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump golang.org/x/net in /modules/weaviate Bumps [golang.org/x/net](https://github.com/golang/net) from 0.20.0 to 0.23.0. - [Commits](golang/net@v0.20.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump golang.org/x/net in /modules/gcloud Bumps [golang.org/x/net](https://github.com/golang/net) from 0.21.0 to 0.23.0. - [Commits](golang/net@v0.21.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump golang.org/x/net in /modules/minio Bumps [golang.org/x/net](https://github.com/golang/net) from 0.21.0 to 0.23.0. - [Commits](golang/net@v0.21.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * feat: support Ryuk for the compose module (#2485) * feat: add testcontainers labels to compose containers * feat: support reaper for compose * chore: increase ryuk reconnection timeout on CI * chore: cache containers on UP * chore: more tuning for compose * chore: more consistent assertion * chore: the compose stack asks for the reaper, but each container then connects to it * chore: use different error groups the first time wait is called, the context is cancelled * chore: the lookup method include cache checks * chore: update tests to make them deterministic * chore: rename local compose testss * chore: support returning the dynamic port in the helper function * chore: try with default reconnection timeout * feat: support removing networks from compose * chore: support naming test services with local and api It will allow the tests to be more deterministic, as there could be service containers started from the local test suite with the same name as in the API test suite. * Revert "chore: try with default reconnection timeout" This reverts commit 336760c. * fix: typo * chore: add funding button for testcontainers (#2510) * feat: support passing io.Reader for compose files when creating a compose instance (#2509) * feat: support passing io.Reader when creating a compose instance * docs: change title * feat: support overriding the default recreate options for compose (#2511) * feat: support overriding the default recreate options for compose * chore: validate recreation values * fix: don't retry on permanent APIClient errors (#2506) * fix: don't retry on permanent APIClient errors * fix: add more tests for un-retryable scenarios * chore: run mod tidy * chore: implement the port-forwarding correctly * chore: use new sshd image * chore: simplify channel creation to avoid allocations * fix: do not leak goroutines Detected with go.uber.org/goleak * chore: expose host internal constant * fix: update variables --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Laurent Saint-Félix <laurent.saintfelix@elastic.co> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Dustin Brown <dustin@dolthub.com> Co-authored-by: Barrett Strausser <bearrito@users.noreply.github.com> Co-authored-by: bstrausser <bstrausser@locusrobotics.com> Co-authored-by: Guillaume St-Pierre <guillaume.stpierre@docker.com> Co-authored-by: Patrick Jahn <33724206+p-jahn@users.noreply.github.com>
mdelapenya
added a commit
to stevenh/testcontainers-go
that referenced
this pull request
Apr 24, 2024
* main: (22 commits) feat: forward host ports to a container using an SSH tunnel (testcontainers#2471) Update follow_logs.md with adding missing package (testcontainers#2513) fix: don't retry on permanent APIClient errors (testcontainers#2506) feat: support overriding the default recreate options for compose (testcontainers#2511) feat: support passing io.Reader for compose files when creating a compose instance (testcontainers#2509) chore: add funding button for testcontainers (testcontainers#2510) feat: support Ryuk for the compose module (testcontainers#2485) chore(deps): bump golang.org/x/net in modules (minio, gcloud, weaviate, compose, qdrant, couchbase, k3s, milvus, mockserver, pulsar, kafka) (testcontainers#2505) fix: fallback to URL-path when parsing auth config URL without scheme (testcontainers#2488) fix(postgres): Fix the non-default dbname error (testcontainers#2489) feat: Bump default postgres version (testcontainers#2481) support Dolt (testcontainers#2177) chore: create TLS certs in a consistent manner (testcontainers#2478) chore(deps): bump idna from 3.6 to 3.7 (testcontainers#2480) Elasticsearch disable CA retrieval when ssl is disabled (testcontainers#2475) fix: handle dockerignore exclusions properly (testcontainers#2476) chore: prepare for next minor development cycle (0.31.0) chore: use new version (v0.30.0) in modules and examples Fix url creation to handle query params when using HTTP wait strategy (testcontainers#2466) fix: data race on container run (testcontainers#2345) ...
mdelapenya
added a commit
to mdelapenya/testcontainers-go
that referenced
this pull request
Apr 26, 2024
* main: (34 commits) break: return error from Customize request option (testcontainers#2267) fix: wrong copy paste (testcontainers#2515) docs: add documentation for Exec method (testcontainers#2451) docs: document the SSHd tunnel (testcontainers#2514) fix: enhance host configuration port binding (testcontainers#2512) feat: forward host ports to a container using an SSH tunnel (testcontainers#2471) Update follow_logs.md with adding missing package (testcontainers#2513) fix: don't retry on permanent APIClient errors (testcontainers#2506) feat: support overriding the default recreate options for compose (testcontainers#2511) feat: support passing io.Reader for compose files when creating a compose instance (testcontainers#2509) chore: add funding button for testcontainers (testcontainers#2510) feat: support Ryuk for the compose module (testcontainers#2485) chore(deps): bump golang.org/x/net in modules (minio, gcloud, weaviate, compose, qdrant, couchbase, k3s, milvus, mockserver, pulsar, kafka) (testcontainers#2505) fix: fallback to URL-path when parsing auth config URL without scheme (testcontainers#2488) fix(postgres): Fix the non-default dbname error (testcontainers#2489) feat: Bump default postgres version (testcontainers#2481) support Dolt (testcontainers#2177) chore: create TLS certs in a consistent manner (testcontainers#2478) chore(deps): bump idna from 3.6 to 3.7 (testcontainers#2480) Elasticsearch disable CA retrieval when ssl is disabled (testcontainers#2475) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR fixes the dbuser/dbname issue by adding the dbname by default in the command, and adds a test to validate this works as intended. In addition, it also adds some logic to handle any error that does not cause the exec command to fail, such as database access failures.
Why is it important?
The linked issue described in great detail an issue where we assumed everyone would use the default database user, whose home DB defaults to the postgres database. When that was not the case, the snapshots would fail silently as the user would not connect to the right database to take the commands.
Related issues
How to test this PR
Run the added test to test this works as intended.