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

Add name to DevContainer compose file #51791

Merged

Conversation

duduribeiro
Copy link
Contributor

Motivation / Background

Currently when you generate a project the compose.yaml generated file does not include the top-level name property. Not having this top-level name property makes all containers / volumes started by devcontainer to be prepended with the folder name, which is devcontainer in the case of the .devcontainer/compose.yml` file.

docker compose -f .devcontainer/compose.yaml up -d

[+] Running 4/4
 ✔ Container devcontainer-postgres-1   Started                                                                                                   0.0s
 ✔ Container devcontainer-selenium-1   Started                                                                                                   0.0s
 ✔ Container devcontainer-rails-app-1  Started

This is ok if you run only one project with devcontainer but starts to get problem if you run multiple projects. If you have one project that runs on postgresql 15 and a new one that runs on postgresql 16 it will fail to boot the postgresql container because both devcontainers is using the same volume: devcontainer_postgres-data.

docker logs PGCONTAINERID

PostgreSQL Database directory appears to contain a database; Skipping initialization

2024-05-11 04:15:34.246 UTC [1] FATAL:  database files are incompatible with server
2024-05-11 04:15:34.246 UTC [1] DETAIL:  The data directory was initialized by PostgreSQL version 15, which is not compatible with this version 16.1 (Debian 16.1-1.pgdg120+1).

Detail

This commit fixes this by setting the top-lavel name property with the project name, which will make the containers and volumes to be prepended with the project name and not with devcontainer.

With the top name property:

➜  my_app git:(main) ✗ docker compose -f .devcontainer/compose.yaml up
[+] Running 4/4
 ✔ Container my_app-selenium-1   Created                                                                                                       0.0s
 ✔ Container my_app-postgres-1   Created                                                                                                       0.0s
 ✔ Container my_app-rails-app-1  Recreated

docker inspect -f '{{ .Mounts }}' PGCONTAINERID

[{volume my_app_postgres-data /var/lib/docker/volumes/my_app_postgres-data/_data /var/lib/postgresql/data local z true }]

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • n/a CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot bot added the railties label May 11, 2024
@duduribeiro
Copy link
Contributor Author

Devcontainer smoke test container is failing because devcontainers/ci is using the old compose version that is deprecated since jul 23 https://docs.docker.com/compose/migrate/

checking if we can run the CI with the newer compose version

Currently when you generate a project the compose.yaml generated
file does not include the top-level name property. Not having this
top-level name property makes all containers / volumes started by
devcontainer to be prepended with the folder name, which is
`devcontainer` in the case of the .devcontainer/compose.yml` file.

This is ok if you run only one project with devcontainer but starts
to get problem if you run multiple projects. If you have one project
that runs on postgresql 15 and a new one that runs on postgresql 16 it
will fail to boot the postgresql container because both devcontainers is
using the same volume: `devcontainer_postgres-data`.

This commit fixes this by setting the top-lavel name property with
the project name, which will make the containers and volumes to be
prepended with the project name and not with `devcontainer`.
@duduribeiro duduribeiro force-pushed the add-name-to-devcontainer-compose branch from 37d68c4 to f63d344 Compare May 13, 2024 17:26
Comment on lines +36 to +38
- name: Remove old deprecated docker-compose
run: |
sudo rm /usr/local/bin/docker-compose
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the old and deprecated docker-compose 1.29.1 (from may 10, 2021), so it uses the compose v2 https://docs.docker.com/compose/migrate/

@rafaelfranca rafaelfranca merged commit 5c933a3 into rails:main May 13, 2024
2 of 3 checks passed
rafaelfranca added a commit that referenced this pull request May 13, 2024
…ompose

Add name to DevContainer compose file
@duduribeiro duduribeiro deleted the add-name-to-devcontainer-compose branch May 13, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants