dimitri / pgcopydb

Copy a Postgres database to a target Postgres server (pg_dump | pg_restore on steroids)
Other
1.19k stars 78 forks source link

Improve Docker and compose definitions #861

Closed hanefi closed 3 months ago

hanefi commented 3 months ago

This is a PR that contains several improvements of our Docker infra.

Below is a detailed overview of all the changes covered in sections:

  1. Remove versions of compose files

We are moving away from the legacy version of compose files. This commit removes the version key from the compose files as it is obsolete.

See https://docs.docker.com/compose/compose-file/04-version-and-name/


  1. Rename compose files to compose.yaml

Excerpt from Docker documentation states:

The default path for a Compose file is compose.yaml (preferred) or compose.yml that is placed in the working directory. Compose also supports docker-compose.yaml and docker-compose.yml for backwards compatibility of earlier versions. If both files exist, Compose prefers the canonical compose.yaml.


  1. Use exec form of Dockerfile CMD instructions

This change updates the Dockerfile CMD instructions to use the exec form instead of the shell form. This is a best practice for Dockerfiles as it avoids the overhead of running a shell process.

One downside of shell form is that it does not pass signals to the process. This can cause the process to not shut down cleanly when stopping the container. The exec form does not have this issue.

hanefi commented 3 months ago

Summary: I used to have 4 commits here. I am removing one as it does not look ready.

Details:

Now that the shared volumes are managed by docker, host OS can not directly set file mode bits to fix permission errors.

The fact that we need to do a chmod a+rwx tests/*/workdir in our test suite indicate that we do a poor job managing users in the docker containers. To be able to fix shared volumes, I need to first fix the users and privileges in the containers.

For the time being, I am removing the fix on docker volumes from this PR. I hope I can fix that in a future PR instead.