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

use '-' as default separator and support of compatibility mode #294

Merged
merged 2 commits into from Jul 29, 2022

Conversation

glours
Copy link
Contributor

@glours glours commented Jul 29, 2022

Change the default separator used between project name and resources to -. This is especially needed for container names to avoid issue with hostname support.

Also add detection of the COMPOSE_COMPATIBILITY env variable to support the compatibility usage of the _ separator

Fix docker/compose#9618

Signed-off-by: Guillaume Lours <guillaume.lours@docker.com>
Copy link
Member

@milas milas left a comment

Choose a reason for hiding this comment

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

LGTM

cli/options_test.go Outdated Show resolved Hide resolved
Co-authored-by: Milas Bowman <milasb@gmail.com>
Signed-off-by: Guillaume Lours <guillaume.lours@docker.com>
@glours glours force-pushed the use-dash-in-resource-names branch from 2f23f04 to aa0ca5e Compare July 29, 2022 13:46
Comment on lines +391 to +393
if utils.StringToBool(options.Environment["COMPOSE_COMPATIBILITY"]) {
o.Separator = loader.CompatibilitySeparator
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please note that this only fixes docker/compose#9618 when passing an environment variable. In the case of Docker Compose getting a --compatibility from the command line I'm not sure that will work (didn't test though).

Given this: https://github.com/docker/compose/blob/v2/cmd/compose/compose.go#L162-L169

Looks like compose just evaluates the command line option after this code is already evaluated.
One solution would be checking the command line option before and setting the EnvVar before calling ProjectFromOptions in the Docker Compose repo for this PR to work properly.

@logopk
Copy link

logopk commented Jul 30, 2022

Stupid breaking change - duplicates all my networks and returns conflicting ip ranges.
"failed to create network step-ca_t-default: Error response from daemon: Pool overlaps with other one on this address space"
Quite a hassle to get this fixed!

reverted to 2.7.0 also because all referred "external" network names changed!

Shouldn't that be announced as major change with necessary migration or the explicit use of "COMPOSE_COMPATIBILITY"?

Thanks.

Peter

@MrNaif2018
Copy link

Compose v2 automatically changed container names for me when just using docker compose up command. When upgrading to 2.8.0 it did create new volumes and network, but containers used old ones, so it requires a full restart, also volumes data isn't migrated automatically but probably should be

@glours
Copy link
Contributor Author

glours commented Jul 31, 2022

If you want to keep the previous behaviour, use the --compatibility flag

docker compose --compatibility up -d
[+] Running 5/5
 ⠿ Network react-express-mongodb_react-express  Created                                                                                                                                                    0.1s
 ⠿ Network react-express-mongodb_express-mongo  Created                                                                                                                                                    0.0s
 ⠿ Container mongo                              Started                                                                                                                                                    4.8s
 ⠿ Container backend                            Started                                                                                                                                                    5.1s
 ⠿ Container frontend                           Started

@logopk
Copy link

logopk commented Jul 31, 2022

???

i need to ediit ALL of my scripts?

@MrNaif2018
Copy link

I think the issue here is that --compatibility flag is for compose v1 -> v2 migration, but breaking change (which should have probably been in v2.0) was introduced in a minor release 2.8.0
We'll try to adapt it and move docker volumes ourselves, docker compose v2 is stable to be used in production, right?

@ulyssessouza
Copy link
Contributor

You can also use an environment variable for that:
export COMPOSE_COMPATIBILITY=true

Like this you don't have to use --compatibility on every call on your scripts

@glours
Copy link
Contributor Author

glours commented Jul 31, 2022

@MrNaif2018 yes you can use it production, End-Of-Life of Compose V1 was announced months ago

@logopk
Copy link

logopk commented Aug 1, 2022

I still do think this is absurd. All running environments need to be rebuilt - even with recreation of all volumes.
Plus all connections to external networks/volumes need to be rewritten!

I do understand that the environment variable fixes this. But what other side effects has this, as I'm on v2 for a long time and I don't need compatibility with v1!

Me don't like this. It's a minor version release! Please reconsider and make this an opt in.

(Even better: if there are existing networks/volumes, leave them alone!)

BTW: I'm not against the new separator. - is much better than _. But I don't want to change existing running environments. Quite a few in my case.

@glours
Copy link
Contributor Author

glours commented Aug 1, 2022

@logopk I will rollback the changes and do releases Compose and compose-go

glours added a commit to glours/compose-go that referenced this pull request Aug 1, 2022
…esource-names"

This reverts commit 56e6c33, reversing
changes made to 0ab97a2.
glours added a commit to glours/compose-go that referenced this pull request Aug 1, 2022
…esource-names"

This reverts commit 56e6c33, reversing
changes made to 0ab97a2.

Signed-off-by: Guillaume Lours <guillaume.lours@docker.com>
@glours
Copy link
Contributor Author

glours commented Aug 1, 2022

@MrNaif2018 FYI we're rolling back the changes, do be to fast in your volumes migration!

@glours
Copy link
Contributor Author

glours commented Aug 1, 2022

A new release of compose-go available without the breaking changes, a version of Docker Compose should follow soon
cc @logopk @MrNaif2018 @AkihiroSuda

edit: Compose v2.9.0 is available

@MrNaif2018
Copy link

@glours Oh I see. I didn't start implementing the migration script luckily. What should my next steps be? I mean I am migrating all deployments from compose v1 to v2 and finished it, but in the meantime when I didn't yet release the changes 2.8.0 release occurred. Ideally in our scripts I would install the latest version of compose v2: https://github.com/bitcartcc/bitcart-docker/blob/dc7de21a1826ede14318701a668956935c64a5d8/helpers.sh#L155-L174

What should my further actions be? Should I prepare that those changes will return back at a later point in compose v2, or they won't? Or they will return but with auto-migration from both compose 1.x and 2.x to the new version like it's done for container re-creation? I'm fine either way, ideally I would migrate all users to v2+ and then no weird bug reports would occur (:

@glours
Copy link
Contributor Author

glours commented Aug 1, 2022

@MrNaif2018 just use v2.9.0 of Compose which was just released.
The value of those changes regarding the impact on existing Compose stacks running is to low, so we won't reintroduce them.

@logopk
Copy link

logopk commented Aug 1, 2022

Thank you for the quick response. Much appreciated!

@jowax
Copy link

jowax commented Aug 11, 2022

@glours Seems like the separator between the project and service name when generating the image name changed from _ to - from compose v2.7.0 to v2.9.0. Is this correct?

Nevermind I just found your response in docker/compose#9700 (comment), sorry for the disturbance.

Compose file:

version: '3.8'

services:
  app:
    build:
      context: .
      dockerfile: Dockerfile

Version 2.9.0

~/projects/foo ❯ ./docker-compose-2.9.0 --version                                                                                                                                                                                                
Docker Compose version v2.9.0
~/projects/foo ❮ ./docker-compose-2.9.0 --project-name foo_devcontainer -f docker-compose.yml build                                                                                                                                              
[+] Building 0.2s (5/5) FINISHED                                                                                                                                                                                                                                                          
 => [internal] load build definition from Dockerfile                                                                                                                                                                                                                                 0.0s
 => => transferring dockerfile: 31B                                                                                                                                                                                                                                                  0.0s
 => [internal] load .dockerignore                                                                                                                                                                                                                                                    0.0s
 => => transferring context: 2B                                                                                                                                                                                                                                                      0.0s
 => [internal] load metadata for docker.io/library/ubuntu:20.04                                                                                                                                                                                                                      0.2s
 => CACHED [1/1] FROM docker.io/library/ubuntu:20.04@sha256:af5efa9c28de78b754777af9b4d850112cad01899a5d37d2617bb94dc63a49aa                                                                                                                                                         0.0s
 => exporting to image                                                                                                                                                                                                                                                               0.0s
 => => exporting layers                                                                                                                                                                                                                                                              0.0s
 => => writing image sha256:b6139e4d50625d70c36fc0232ae66d887f4a58115d9579a989f4490f0a65cf28                                                                                                                                                                                         0.0s
 => => naming to docker.io/library/foo_devcontainer-app                                                                                                                                                                                                                              0.0s

Version 2.8.0

~/projects/foo ❯ ./docker-compose-2.8.0 --version                                                                                                                                                                                               
Docker Compose version v2.8.0
~/projects/foo ❮ ./docker-compose-2.8.0 --project-name foo_devcontainer -f docker-compose.yml build                                                                                                                                              
[+] Building 0.5s (5/5) FINISHED                                                                                                                                                                                                                                                          
 => [internal] load build definition from Dockerfile                                                                                                                                                                                                                                 0.0s
 => => transferring dockerfile: 31B                                                                                                                                                                                                                                                  0.0s
 => [internal] load .dockerignore                                                                                                                                                                                                                                                    0.0s
 => => transferring context: 2B                                                                                                                                                                                                                                                      0.0s
 => [internal] load metadata for docker.io/library/ubuntu:20.04                                                                                                                                                                                                                      0.4s
 => CACHED [1/1] FROM docker.io/library/ubuntu:20.04@sha256:af5efa9c28de78b754777af9b4d850112cad01899a5d37d2617bb94dc63a49aa                                                                                                                                                         0.0s
 => exporting to image                                                                                                                                                                                                                                                               0.0s
 => => exporting layers                                                                                                                                                                                                                                                              0.0s
 => => writing image sha256:b6139e4d50625d70c36fc0232ae66d887f4a58115d9579a989f4490f0a65cf28                                                                                                                                                                                         0.0s
 => => naming to docker.io/library/foo_devcontainer-app                                                                                                                                                                                                                              0.0s

Version 2.7.0

~/projects/foo ❯ ./docker-compose-2.7.0 --version                                                                                                                                                                                                
Docker Compose version v2.7.0
~/projects/foo ❯ ./docker-compose-2.7.0 --project-name foo_devcontainer -f docker-compose.yml build                                                                                                                                              
[+] Building 0.6s (5/5) FINISHED                                                                                                                                                                                                                                                          
 => [internal] load build definition from Dockerfile                                                                                                                                                                                                                                 0.0s
 => => transferring dockerfile: 31B                                                                                                                                                                                                                                                  0.0s
 => [internal] load .dockerignore                                                                                                                                                                                                                                                    0.0s
 => => transferring context: 2B                                                                                                                                                                                                                                                      0.0s
 => [internal] load metadata for docker.io/library/ubuntu:20.04                                                                                                                                                                                                                      0.5s
 => CACHED [1/1] FROM docker.io/library/ubuntu:20.04@sha256:af5efa9c28de78b754777af9b4d850112cad01899a5d37d2617bb94dc63a49aa                                                                                                                                                         0.0s
 => exporting to image                                                                                                                                                                                                                                                               0.0s
 => => exporting layers                                                                                                                                                                                                                                                              0.0s
 => => writing image sha256:b6139e4d50625d70c36fc0232ae66d887f4a58115d9579a989f4490f0a65cf28                                                                                                                                                                                         0.0s
 => => naming to docker.io/library/foo_devcontainer_app                                                                                                                                                                                                                              0.0s

@jamshid
Copy link

jamshid commented Mar 5, 2023

@glours wrote:

The value of those changes regarding the impact on existing Compose stacks running is to low, so we won't reintroduce them.

Eh ok but the network name can end up in a fully qualified dns name:

# nslookup 172.23.0.15
15.0.23.172.in-addr.arpa	name = myproject-myservice-2.myproject_default.

and at least python is very strict about underscores, leading to idna.core.InvalidCodepoint: Codepoint U+005F errors e.g.:

          File "/tmp/build/results/usr/local/lib/python3.9/dist-packages/twisted/python/urlpath.py", line 265, in __str__
          File "/tmp/build/results/usr/local/lib/python3.9/dist-packages/hyperlink/_url.py", line 1668, in to_uri
          File "/tmp/build/results/usr/local/lib/python3.9/dist-packages/idna/core.py", line 360, in encode
          File "/tmp/build/results/usr/local/lib/python3.9/dist-packages/idna/core.py", line 258, in alabel
          File "/tmp/build/results/usr/local/lib/python3.9/dist-packages/idna/core.py", line 297, in ulabel
          File "/tmp/build/results/usr/local/lib/python3.9/dist-packages/idna/core.py", line 250, in check_label
        idna.core.InvalidCodepoint: Codepoint U+005F at position 10 of 'myproject_default' not allowed

Workaround: specify the network name in docker-compose.yml:

networks:
  default:
    name: myproject-default
    driver: bridge
    attachable: true

@AlexMikhalev
Copy link

You don't realise how much work such changes generate for projects. Mine is a medium-complexity project https://github.com/applied-knowledge-systems/the-pattern/ where deployment is now broken in multiple places. I spent time two years ago making all deployments docker and docker-compose based. Going forward, I will ditch docker-compose and only use docker for builds. Try to reach a point where benchmarks work: https://reference-architecture.ai/docs/bert-qa-benchmarking/ and track the effort required to update all docker-compose files.
Looking forward to your PRs.

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 this pull request may close these issues.

Compose using underscore when generating volume and network names
9 participants