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

Should devcontainers/cli uses docker compose always instead docker-compose? #826

Closed
duduribeiro opened this issue May 11, 2024 · 3 comments · Fixed by #848
Closed

Should devcontainers/cli uses docker compose always instead docker-compose? #826

duduribeiro opened this issue May 11, 2024 · 3 comments · Fixed by #848
Assignees

Comments

@duduribeiro
Copy link

Hey devcontainers/cli folks 👋

I have a CI that uses devcontainers/ci action (which uses the devcontainers/cli) to validate a devcontainer generated for the project.

I'm trying to add the top-level name property into the compose file rails/rails#51791

but it is failing on CI with the error of:

he Compose file .. is invalid because:
  'name' does not match any of the regexes: '^x-'

https://github.com/rails/rails/actions/runs/9044514158/job/24853256908?pr=51791

I also tried to specify the version 3.8 on the compose but without success.

I see that devcontainers/cli code base uses docker-compose instead docker compose. should we replace the codebase to use docker compose instead so we avoid to use the old compose which is deprecate since jul 23? https://docs.docker.com/compose/migrate/

I also see a test on devcontainers/cli codebase https://github.com/devcontainers/cli/blob/main/src/test/configs/compose-with-name/.devcontainer/docker-compose.yml which tests the exact same scenario but in my case it is not working.

thoughts?

@samruddhikhandale
Copy link
Member

Hi 👋

Looking at 👇 , I think the request makes sense for the CLI to switch to the newer Docker compose V2 syntax (ie docker compose) given https://docs.docker.com/compose/migrate/

const dockerComposePath = options.dockerComposePath || 'docker-compose';

@gauravsaini04 Can you help take a look at this request? Thanks!

// cc @chrmarti Let us know if there are any concerns, thanks!

@chrmarti
Copy link
Contributor

Sounds good! We currently check for docker-compose and if that doesn't exist we use docker compose. To minimize breakage, we could turn this around. Similar code exists in the Dev Containers extension which will also need updating to stay consistent.

The check is here:

export function dockerComposeCLIConfig(params: Omit<PartialExecParameters, 'cmd'>, dockerCLICmd: string, dockerComposeCLICmd: string) {

@gauravsaini04
Copy link
Contributor

pr #848 was merged to the codebase. closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants