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

[BUG] v2.24.7 multiple changes in config output #11589

Closed
Re4zOon opened this issue Mar 7, 2024 · 26 comments
Closed

[BUG] v2.24.7 multiple changes in config output #11589

Re4zOon opened this issue Mar 7, 2024 · 26 comments

Comments

@Re4zOon
Copy link

Re4zOon commented Mar 7, 2024

Description

Hi,

Between Docker Compose version v2.24.6 and v2.24.7 some kind of change screwed up our stack build.
So far "extra_hosts" were a list separated by "=", not they are separated by ":" and labels changed from ":" to "=".
I guess its not intentional, as now our scripts (that loads the stack yaml to grab values for ansible) of course can't do that.

Also for some reason one of the services environment variables were switched to a list. I'm still trying to find the difference, why only that service gets parsed differently.

Steps To Reproduce

No response

Compose Version

No response

Docker Environment

No response

Anything else?

No response

@Re4zOon
Copy link
Author

Re4zOon commented Mar 7, 2024

I'll try to gather and share some simple outputs from both versions illustrating the issues (our stack yaml is about 6000 lines now).

@stasadev
Copy link

stasadev commented Mar 7, 2024

We see the same in DDEV (all our tests are failing)

extra_hosts:
    - host.docker.internal=host-gateway

became

extra_hosts:
    - host.docker.internal:host-gateway

I tracked it down to:

@rfay
Copy link
Contributor

rfay commented Mar 7, 2024

@Re4zOon could you please retitle this to something like "[BUG] v2.24.7 extra_hosts is output with colon instead of equals sign"

@milas milas self-assigned this Mar 8, 2024
@milas milas changed the title [BUG] Compose output drastically changed [BUG] v2.24.7 extra_hosts uses colon instead of equals sign in config output Mar 8, 2024
@milas milas pinned this issue Mar 8, 2024
@Re4zOon
Copy link
Author

Re4zOon commented Mar 8, 2024

Here you go:
test.zip

What I found:

  • a new default network is created, even tho nothing asked for it
  • if you add an "override", the config becomes a list, instead of a dictionary
  • labels are using equal sign instead of colon
  • extra_hosts are using colon instead of equal sign

@Re4zOon Re4zOon changed the title [BUG] v2.24.7 extra_hosts uses colon instead of equals sign in config output [BUG] v2.24.7 multiple changes in config output Mar 8, 2024
@luciangabor
Copy link

@Re4zOon
Copy link
Author

Re4zOon commented Mar 10, 2024

Well, today is a weekend day after all: https://github.com/dwmkerr/hacker-laws?tab=readme-ov-file#hyrums-law-the-law-of-implicit-interfaces

Yes, thats true. Just like: https://xkcd.com/1172/
However breaking compatibility still shouldn't result in a "patch" version according to semver.

@luciangabor
Copy link

luciangabor commented Mar 10, 2024

Ok, but what compatibility is broken? If I pipe its output through itself (with varius 2.2*.* versions that I keep around) again everything seems alright: docker compose config | docker compose -f - config -q (using the docker-compose.yml from test.zip I get a deprecation warning, but that's all) ...

@ndeloof
Copy link
Contributor

ndeloof commented Mar 11, 2024

This change indeed took place to follow docker engine preferred format after compose-spec/compose-go#499

services environment variables were switched to a list
this was required to allows resurrection of the --no-interpolate support.

in both case, this is not "breaking compatibility": config render a compose-spec compliant output, the fact you wrote your own parser without considering the many aspects of the spec is unfortunately an issue you have to manage on your own.

I'm closing this issue as "won't fix"

@ndeloof ndeloof closed this as not planned Won't fix, can't repro, duplicate, stale Mar 11, 2024
@Re4zOon
Copy link
Author

Re4zOon commented Mar 11, 2024

I can understand the other point, but:

environment variables were switched to a list

Its either a list, or a dictionary and its dependent on other things.
Why do we have a spec, where it can be either with no clear setting as to which to use?
config just decides it wants to use a list for only ONE service out of 70+, even tho the compose version did not change in our config (still on 3.5).

This means I get random behaviour and my parser will have to check if its a list, or a dictionary, as I can't anticipate which one it will be. Fun.

@luciangabor
Copy link

There's this:

a new default network is created, even tho nothing asked for it

@Re4zOon
Copy link
Author

Re4zOon commented Mar 11, 2024

There's this:

a new default network is created, even tho nothing asked for it

As I said, I can understand all other points. My main two problems:

  • schema changed without any notification from one patch version to the next (even tho we still use version 3.5 )
  • env variable is either a list or a dictionary, based on... something.

My parser gets a 70+ service yaml with 6000+ lines. ONE of those services has a list, all other is dictionary due to a patch.
Its the inconsistency, that bothers me.

@luciangabor
Copy link

even tho we still use version 3.5

maybe I'm wrong and this is not about swarm/stack, but if that's the case, why don't you just use docker stack config ..
(I keep seeing this and I don't get it - why pipe compose's config output, which is like an "ng" variant, to tools that expect the "classic" variant ... and expect it to keep working)

@Re4zOon
Copy link
Author

Re4zOon commented Mar 11, 2024

oh. docker stack config didn't cross my mind...

@luciangabor
Copy link

luciangabor commented Mar 11, 2024

(there's also docker buildx bake to build images without compose)
((neither load .env nor set COMPOSE_* variables, but ...))

@ndeloof
Copy link
Contributor

ndeloof commented Mar 11, 2024

Why do we have a spec, where it can be either with no clear setting as to which to use?

compose file format initially evolved organically, and offered flexibility to users. The spec defines the many ways things can be set, but there's no "single way", compared to kube.yaml which just reflect the internal API structs.

config just decides it wants to use a list for only ONE service out of 70+, even tho the compose version did not change in our config (still on 3.5).

version attribute is obsolete and should be removed, it doesn't guarantee anything and is just a legacy from the time compose was implemented in python.

This means I get random behaviour and my parser will have to check if its a list, or a dictionary, as I can't anticipate which one it will be. Fun.

Indeed, this is the price to pay with the flexibility we offer in the compose file format. You should not try to parse such a file with a shell script or naive parser expecting a fixed format. If you need to transform the config output, you should better rely on compose-go library and write your own transformation as actual code

@luciangabor
Copy link

There's this:

a new default network is created, even tho nothing asked for it

@ndeloof did you notice that?

$ docker compose -f - config <<-'EOF'
name: oh-no
services:
  service:
    image: image
    networks:
      net:
networks:
  net:
EOF
name: oh-no
networks:
  default:
    name: oh-no_default
  net:
    name: oh-no_net
services:
  service:
    image: image
    networks:
      net: null

(up doesn't create it)

@ndeloof
Copy link
Contributor

ndeloof commented Mar 11, 2024

indeed, we miss removal for unused resources - but that's a distinct issue then

@Re4zOon
Copy link
Author

Re4zOon commented Mar 11, 2024

I've switched to docker stack config, so from my side this issue can be closed.
We hope we can dismantle swarm in the "not-so-near" future anyways.

@luciangabor
Copy link

indeed, we miss removal for unused resources - but that's a distinct issue then

and to take into account profiles: #11587

$ docker compose -f - config <<-'EOF'
name: hidden
services:
  service:
    image: image
    profiles:
      - hidden
EOF
name: hidden
networks:
  default:
    name: hidden_default
services:
  service:
    image: image
    networks:
      default: null
    profiles:
      - hidden

# although --services works
$ docker compose -f - config --services <<-'EOF'
name: hidden
services:
  service:
    image: image
    profiles:
      - hidden
EOF

@luciangabor
Copy link

I've switched to docker stack config, so from my side this issue can be closed. We hope we can dismantle swarm in the "not-so-near" future anyways.

https://github.com/search?q=repo%3Adocker%2Fcompose+mirantis&type=issues
https://lwn.net/Articles/905164/

@rfay
Copy link
Contributor

rfay commented Mar 15, 2024

Should have been noted here that the problem behavior here was reverted in v2.25.0.

DDEV's tests can now pass with v2.25.0

@luciangabor
Copy link

Well, but why rely on that?

The relevant part of the change was to restore a case where config did "more" - to inline the content of env files (when found) ... and this change is already subject to ... change: #11627

Who knows ... maybe config will convert all lists to maps where possible ... that's kinda' more aligned with doing "more" (plain lists are mostly for "short" forms and "long" forms, where they exist, are ... kinda' preferred and that's what I meant by doing "more" - to have config "show" you what you meant writing those files).

@ndeloof
Copy link
Contributor

ndeloof commented Mar 16, 2024

actually, the docker engine API uses key=value to set environment variables, so the mapping syntax for those only exists in compose for legacy reasons

@luciangabor
Copy link

maps also "just work":

services:
  s1:
    image: image
    environment:
      - VAR=foo

  s2:
    extends: s1
    environment:
      - VAR=bar

But, being a weekend day after all, when it comes to extra hosts I find it funny that the long syntax is actually shorter that the short one :D

@arcanisgk
Copy link

How can I validate if this works or not?

extra_hosts:
      - "host.docker.internal:host-gateway"

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

No branches or pull requests

8 participants
@rfay @ndeloof @milas @arcanisgk @Re4zOon @luciangabor @stasadev and others