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

Switch to Docker Compose v2 #3876

Closed
zackkrida opened this issue Mar 5, 2024 · 12 comments
Closed

Switch to Docker Compose v2 #3876

zackkrida opened this issue Mar 5, 2024 · 12 comments
Labels
🤖 aspect: dx Concerns developers' experience with the codebase ✨ goal: improvement Improvement to an existing user-facing feature 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: mgmt Related to repo management and automations

Comments

@zackkrida
Copy link
Member

zackkrida commented Mar 5, 2024

Problem

Docker recommends users use docker compose instead of docker-compose as of July 2023, when version 1 of the compose api stopped being supported.

Important

Docker's documentation refers to and describes Compose V2 functionality.

Effective July 2023, Compose V1 stopped receiving updates and is no longer in new Docker Desktop releases. Compose V2 has replaced it and is now integrated into all current Docker Desktop versions. For more information, see Migrate to Compose V2.

-- https://docs.docker.com/compose/

Most fresh installs of Docker no longer include the docker-compose alias.

Description

Update all of our justfiles and build steps to use docker compose instead of docker-compose.
As a bonus, this would allow for predictability in service container names, which use a - instead of _ separator in Compose V21.

Alternatives

Keep using old software 😢

Footnotes

  1. https://docs.docker.com/compose/migrate/#service-container-names

@zackkrida zackkrida added 🟨 priority: medium Not blocking but should be addressed soon ✨ goal: improvement Improvement to an existing user-facing feature 🤖 aspect: dx Concerns developers' experience with the codebase 🧱 stack: mgmt Related to repo management and automations labels Mar 5, 2024
@sarayourfriend
Copy link
Contributor

Duplicate issue #3973 has some additional information that could be helpful for completing this and cleaning up our code (e.g., massively reduce the complexity of just ps script)

@obulat obulat added 🟥 priority: critical Must be addressed ASAP and removed 🟨 priority: medium Not blocking but should be addressed soon labels Apr 3, 2024
@obulat
Copy link
Contributor

obulat commented Apr 3, 2024

I updated the priority to critical because the CI is intermittently failing due to docker-compose not being available in GitHub CI runners.

@AetherUnbound
Copy link
Contributor

Resolved by #4017

@AetherUnbound
Copy link
Contributor

Ah, I was mistaken, there's a bit more work here that's necessary.

@AetherUnbound AetherUnbound reopened this Apr 3, 2024
@sarayourfriend
Copy link
Contributor

@AetherUnbound can you clarify what work that is? The basic part is done, separate issues could be helpful to break down further tasks.

@AetherUnbound
Copy link
Contributor

Specifically this piece from @obulat's PR description:

This PR only replaces docker-compose in justfiles and documentation with docker compose.
It does not simplify the docker ps script.
@zackkrida mentions the replacement of underscores with dashes for container names in the issue: should this be done manually? Would it also affect production?

If this piece is not considered important then we can close the issue.

@sarayourfriend
Copy link
Contributor

I think they are important, but should be separate issues. We're not using compose v1 anymore, so at the very least the title of the issue and the main portion of it is complete. The others are far less important "nice to have" clean ups, right?

@dhruvkb
Copy link
Member

dhruvkb commented Apr 4, 2024

I'm reducing the priority back down to medium because the critical part of this issue has been resolved by #4017.


As a bonus, this would allow for predictability in service container names, which use a - instead of _ separator in Compose V21

To be honest, this makes service names more confusing, not less, because the project-service separator is now the same as our separator used within the service name.

Footnotes

  1. https://docs.docker.com/compose/migrate/#service-container-names

@dhruvkb dhruvkb added 🟨 priority: medium Not blocking but should be addressed soon and removed 🟥 priority: critical Must be addressed ASAP labels Apr 4, 2024
@sarayourfriend
Copy link
Contributor

To be honest, this makes service names "more" confusing, not "less", because the project-service separator is now the same as our separator used within the service name.

+1, I was going to comment that on the eventual issue, if we made a new one :)

@AetherUnbound
Copy link
Contributor

I think, in that case, we can probably close this issue! Any objections?

@dhruvkb
Copy link
Member

dhruvkb commented Apr 5, 2024

I think that would be good @AetherUnbound as it would help to clarify the exact changes remaining to be made and if scoped sufficiently small, it could even be a good first issue for introducing contributors to our Docker development stack.

@sarayourfriend
Copy link
Contributor

Here are follow up issues. Closing this one in favour of them.

#4045
#4046

I did not create an issue for changing the service separator, it won't be relevant after the just ps script change and seems likely to be more of a headache than any potential benefits that I could think of (I really can't think of any specific ones). If anyone else has a strong feeling and especially justification for it, please write an issue for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 aspect: dx Concerns developers' experience with the codebase ✨ goal: improvement Improvement to an existing user-facing feature 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: mgmt Related to repo management and automations
Projects
Archived in project
Development

No branches or pull requests

5 participants