Skip to content
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

[Bug]: Postgres snapshot creation silently fails if a username different to "postgres" is provided while creating the container #2474

Closed
MarioRomanDono opened this issue Apr 8, 2024 · 2 comments · Fixed by #2489
Labels
bug An issue with the library

Comments

@MarioRomanDono
Copy link

MarioRomanDono commented Apr 8, 2024

Testcontainers version

0.30.0

Using the latest Testcontainers version?

Yes

Host OS

Linux

Host arch

x86

Go version

1.22.1

Docker version

Client:
 Cloud integration: v1.0.35+desktop.10
 Version:           25.0.2
 API version:       1.44
 Go version:        go1.21.6
 Git commit:        29cf629
 Built:             Thu Feb  1 00:22:06 2024
 OS/Arch:           linux/amd64
 Context:           default

Server: Docker Desktop
 Engine:
  Version:          25.0.2
  API version:      1.44 (minimum version 1.24)
  Go version:       go1.21.6
  Git commit:       fce6e0c
  Built:            Thu Feb  1 00:23:17 2024
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.6.28
  GitCommit:        ae07eda36dd25f8a1b98dfbf587313b99c0190bb
 runc:
  Version:          1.1.12
  GitCommit:        v1.1.12-0-g51d5e94
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad

Docker info

Client:
 Version:    25.0.2
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.12.1-desktop.4
    Path:     /usr/local/lib/docker/cli-plugins/docker-buildx
  compose: Docker Compose (Docker Inc.)
    Version:  v2.24.3-desktop.1
    Path:     /usr/local/lib/docker/cli-plugins/docker-compose
  debug: Get a shell into any image or container. (Docker Inc.)
    Version:  0.0.22
    Path:     /usr/local/lib/docker/cli-plugins/docker-debug
  dev: Docker Dev Environments (Docker Inc.)
    Version:  v0.1.0
    Path:     /usr/local/lib/docker/cli-plugins/docker-dev
  extension: Manages Docker extensions (Docker Inc.)
    Version:  v0.2.21
    Path:     /usr/local/lib/docker/cli-plugins/docker-extension
  feedback: Provide feedback, right in your terminal! (Docker Inc.)
    Version:  v1.0.4
    Path:     /usr/local/lib/docker/cli-plugins/docker-feedback
  init: Creates Docker-related starter files for your project (Docker Inc.)
    Version:  v1.0.0
    Path:     /usr/local/lib/docker/cli-plugins/docker-init
  sbom: View the packaged-based Software Bill Of Materials (SBOM) for an image (Anchore Inc.)
    Version:  0.6.0
    Path:     /usr/local/lib/docker/cli-plugins/docker-sbom
  scout: Docker Scout (Docker Inc.)
    Version:  v1.3.0
    Path:     /usr/local/lib/docker/cli-plugins/docker-scout

Server:
 Containers: 5
  Running: 0
  Paused: 0
  Stopped: 5
 Images: 13
 Server Version: 25.0.2
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 1
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: ae07eda36dd25f8a1b98dfbf587313b99c0190bb
 runc version: v1.1.12-0-g51d5e94
 init version: de40ad0
 Security Options:
  seccomp
   Profile: unconfined
 Kernel Version: 5.15.146.1-microsoft-standard-WSL2
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 12
 Total Memory: 15.19GiB
 Name: docker-desktop
 ID: 7cfafb33-158f-42d6-ba06-c092bc51f0f4
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 HTTP Proxy: http.docker.internal:3128
 HTTPS Proxy: http.docker.internal:3128
 No Proxy: hubproxy.docker.internal
 Experimental: false
 Insecure Registries:
  hubproxy.docker.internal:5555
  127.0.0.0/8
 Live Restore Enabled: false

WARNING: No blkio throttle.read_bps_device support
WARNING: No blkio throttle.write_bps_device support
WARNING: No blkio throttle.read_iops_device support
WARNING: No blkio throttle.write_iops_device support
WARNING: daemon is not using the default seccomp profile

What happened?

When creating a snapshot for a container which has been created specifying an username different to the default "postgres", the process silently fails and the snapshot is not created, which causes problems when calling container.Restore(). I believe the problem is in this line:

_, _, err := c.Exec(ctx, []string{"psql", "-U", c.user, "-c", cmd})

I think '-d', c.dbName should be added in order to correctly execute the commands. I have attached a video where the issue is reproduced. You can also reproduce the bug if you change

to a different value and you try to run TestSnapshot. If you think this is a valid issue, I would be happy to submit a pull request with the fix.

Also, as a sidenote, I think there should be a better error handling in this case. If one of the command executions fails and the snapshot is not created or restored, the error should be correctly returned to the user instead of silently failing. I can help with that too if you want :)

Thanks!

Relevant log output

No response

Additional information

Testcontainers-Go.snapshot.mp4
@MarioRomanDono MarioRomanDono added the bug An issue with the library label Apr 8, 2024
@MarioRomanDono MarioRomanDono changed the title [Bug]: Postgres snapshot creation silently fails if a database name is provided while creating the container [Bug]: Postgres snapshot creation silently fails if a username different to "postgres" is provided while creating the container Apr 8, 2024
@mdelapenya
Copy link
Member

@Minivera, as contributor of this feature, could you take a look at this?

Minivera added a commit to Minivera/testcontainers-go that referenced this issue Apr 18, 2024
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
@Minivera
Copy link
Contributor

For sure 🙂 I think this should cover both asks? #2489

mdelapenya pushed a commit that referenced this issue Apr 18, 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 #2474

* Document the postgres dbname issue in the docs
mdelapenya pushed a commit to mdelapenya/testcontainers-go that referenced this issue 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 that referenced this issue 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue with the library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants