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

feat: Support docker compose CLI #1116

Merged
merged 18 commits into from
Oct 15, 2021
Merged

feat: Support docker compose CLI #1116

merged 18 commits into from
Oct 15, 2021

Conversation

EricsonMacedo
Copy link
Contributor

Check if docker compose v2, CLI, is available and get semantic version
from it, or fallback to get semantic version out of docker-compose v1
when checking minimum requirements during install.sh script

Fixes #962

Check if docker compose v2, CLI, is available and get semantic version
from it, or fallback to get semantic version out of docker-compose v1
when checking minimum requirements during install.sh script

Fixes #962
Copy link
Member

@BYK BYK left a comment

Choose a reason for hiding this comment

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

Hi @EricsonMacedo thanks a lot for the patch!

I think we need the following to get this patch landed:

  1. Fix the version check logic so we don't fail on 2.0.0+
  2. Make sure we use docker compose instead of docker-compose by replacing $dc accordingly
  3. Add a test case into the test matrix for Docker Compose 2.0

install/check-minimum-requirements.sh Outdated Show resolved Hide resolved
Fix getting semantic version of docker compose version 2.0.0 by properly
cleaning up possible `v` prefix in version output

Assign docker compose CLI if available to $dc alias or fallback to
docker-compose

Add integration test to check support of docker compose v2

Update install wrap-up message to output the docker compose command
based on $dc alias
@BYK BYK enabled auto-merge (squash) October 15, 2021 16:41
@BYK BYK merged commit 78a5c3c into getsentry:master Oct 15, 2021
@EricsonMacedo EricsonMacedo deleted the feat/support-docker-compose-cli branch October 15, 2021 19:38
@PMExtra
Copy link

PMExtra commented Oct 15, 2021

Please note that: I met some trouble in docker compose with sentry-onpremise.
The first, the install output format will be wrong. They cannot wrap line correctly.
Then when I used docker compose to make a clean install, I cannot make any interaction with the initialization script (It won't take any response, just like without -i / --interactive flag.

I have tested in several weeks ago with my own patch, with Docker version 20.10.8 and Docker Compose version v2.0.0-rc2 in a SSH session of CentOS 8.

@BYK
Copy link
Member

BYK commented Oct 15, 2021

@PMExtra does this patch address some of the issues you had? Btw they released v2.0.1 recently.

@aminvakil
Copy link
Collaborator

v2.0.1 can be used like before (docker-compose) and there is no need to specifically use docker compose.

So do you think this PR should be reverted (most parts) and using two different approaches for versions < 2 and >=2 would bring confusion for maintaining this repository or not?

@EricsonMacedo
Copy link
Contributor Author

EricsonMacedo commented Oct 16, 2021

v2.0.1 can be used like before (docker-compose) and there is no need to specifically use docker compose.

So do you think this PR should be reverted (most parts) and using two different approaches for versions < 2 and >=2 would bring confusion for maintaining this repository or not?

@aminvakil Setting docker-compose binary isn't mandatory, it's an additional compatibility bin/symlink for transition period https://github.com/docker/compose-switch/. In v2 the command to output version isn't docker-compose --version as before and the output pattern is different as well.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support docker compose CLI.
4 participants