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

v2.3.x config yaml output contains ports as strings #9306

Closed
jsoriano opened this issue Mar 23, 2022 · 26 comments
Closed

v2.3.x config yaml output contains ports as strings #9306

jsoriano opened this issue Mar 23, 2022 · 26 comments

Comments

@jsoriano
Copy link
Contributor

jsoriano commented Mar 23, 2022

Description

docker-compose config formats the published port as a string, this can be a problem when parsing this output.

This starts to happen in 2.3.0.

In documentation ports appear as numbers.

Steps to reproduce the issue:

  1. Define a project with exposed ports.
  2. Run docker-compose config.
  3. published port is a string, while target port is a number.

Can be reproduced for example with this minimal docker-compose.yml:

version: "2.3"
services:
  web:
    image: "nginx"
    ports:
      - "8080:8080"

Describe the results you received:

$ docker-compose config
name: foo
services:
  web:
    image: nginx
    networks:
      default: null
    ports:
    - mode: ingress
      target: 8080
      published: "8080"
      protocol: tcp
networks:
  default:
    name: foo_default

Describe the results you expected:

$ docker-compose config
name: foo
services:
  web:
    image: nginx
    networks:
      default: null
    ports:
    - mode: ingress
      target: 8080
      published: 8080
      protocol: tcp
networks:
  default:
    name: foo_default

Additional information you deem important (e.g. issue happens only occasionally):

It seems to happen since 2.3.0, in 2.2.3 the result is the expected one.

Output of docker compose version:

Docker Compose version v2.3.3

Output of docker info:

Client:
 Context:    default
 Debug Mode: false
 Plugins:
  app: Docker App (Docker Inc., v0.9.1-beta3)
  buildx: Build with BuildKit (Docker Inc., v0.5.1-docker)
  scan: Docker Scan (Docker Inc., v0.8.0)

Server:
 Containers: 1
  Running: 1
  Paused: 0
  Stopped: 0
 Images: 327
 Server Version: 20.10.7
 Storage Driver: btrfs
  Build Version: Btrfs v4.4
  Library Version: 101
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 1
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runtime.v1.linux runc io.containerd.runc.v2
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: d71fcd7d8303cbf684402823e425e9dd2e99285d
 runc version: b9ee9c6314599f1b4a7f497e1f1f856fe433d3b7
 init version: de40ad0
 Security Options:
  apparmor
  seccomp
   Profile: default
 Kernel Version: 4.15.0-171-generic
 Operating System: Ubuntu 18.04.6 LTS
 OSType: linux
 Architecture: x86_64
 CPUs: 4
 Total Memory: 15.44GiB
 Name: voyager
 ID: R7WQ:DRKJ:F2ZK:EC7J:V3RF:4GRC:GPI6:AEDN:6GL4:VU3V:LU2U:4YE5
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false

Additional environment details:

@jsoriano jsoriano changed the title v2.3.3 config yaml contains ports as strings v2.3.x config yaml output contains ports as strings Mar 23, 2022
@designermonkey
Copy link

This has broken our build system. We run:

(echo "version: '3.8'"; docker compose -f traefik/docker-compose.yaml convert) | docker stack deploy --with-registry-auth --compose-file - traefik
services.server.ports.0.published must be a integer

Our docker desktop version:

Version
4.6.1 (76265)
Engine: 20.10.13
Compose: v2.3.3
Credential Helper: 0.6.4
Kubernetes: v1.22.5
Snyk: v1.827.0

@designermonkey
Copy link

Docker compose is now on version 2.4.1, and Docker Desktop at 4.7.1

I am stuck at holding docker desktop at v4.5.0 to prevent this from happening.

I'm not complaining but would love to know if there's any idea on when this will be addressed?

@trajano
Copy link

trajano commented May 18, 2022

Docker compose is now on version 2.4.1, and Docker Desktop at 4.7.1

I am stuck at holding docker desktop at v4.5.0 to prevent this from happening.

I thinking it may be best to use docker-compose until 21.x.x of Docker comes out , it will also remove that (echo "version:..." hack you had in your build system (nice idea BTW)

@rafipiccolo
Copy link

FYI

This is my script to get a swarm compatible compose file from a "docker compose config"

It is now in production and working with latest docker compose (2.6.0)
(It's a shame I had to create a script but now it works nice)

Docker compose -f swarm.yml config | node reparecompose.js | docker stack deploy --prune --with-registry-auth -c - swarm

The reparecompose.js :

import fs from 'fs';

let s = fs.readFileSync(0, 'utf8');
s += `version: "3.7"\n`;
s = s.replace(/published: "(\d+)"/g, "published: $1");
s = s.replace(/^name: .*\n/, "");

console.log(s);

It does only 3 changes in the file.
Did you notice that other changes were necessary to have a working stack. for me that was enough.

@danielporto
Copy link

This is really annoying. Any clean fix available or must I use an older version?

@designermonkey
Copy link

designermonkey commented Aug 11, 2022

How do we get this to be a priority for the docker dev team?

I upgraded today to 4.11.1 and it is still an issue, so again, reverting to 4.5.0.

I'm getting closer and closer to giving up with Docker.

@glours
Copy link
Contributor

glours commented Aug 11, 2022

Hey 👋
Compose is strictly following the Compose Specification here.
The published port(s) can be a port range that's why a string is used instead of a uint32 for the target attribute
Mirantis now owns Swarm and the stack CLI, the best option here is to ask them:

  • if they using compose-go and are up to date.
  • if they can support a string value for the published port

Docker Compose and Stack have 2 different purposes and should not be use together, even if they have been sharing the same file format at one point.
There is an open issue for the support of the Compose specification in the stack command

@glours glours closed this as completed Aug 11, 2022
@rafipiccolo
Copy link

rafipiccolo commented Aug 11, 2022

@designermonkey, @danielporto i provided a script in this thread to "make docker config great again"

The other viable way is to set your env from your .env file before deploying.
This completely allows you to get rid of docker compose config.
From my understanding, this is the expected way for docker people.
found it here : moby/moby#29133

# put all .env in the real environment
export $(cat .env) > /dev/null 2>&1; 
# run stack, knowing that all variables are going to be populated by current env
docker stack deploy -c docker-compose.yml STACK_NAME

@designermonkey
Copy link

Being a simple end user, I didn't know that different aspects of the same bundled software are owned by different people. Thanks @glours for letting us know that.

Docker Compose and Stack have 2 different purposes and should not be used together

I can't agree with this however as up until very recently they were completely compatible and just ignored each other's differences. It's very stressful to be using what people consider a single application 'docker' in production and it not even be compatible with itself.

Also, thanks @rafipiccolo I will have to investigate that idea more.

@jsoriano
Copy link
Contributor Author

Hey @glours, thanks for your reply.

Mirantis now owns Swarm and the stack CLI, the best option here is to ask them:

I don't use swarm. The issue happens with docker-compose as downloaded from https://github.com/docker/compose/releases, starting on 2.3.0.

Compose is strictly following the Compose Specification here.

I find this a bit confusing. This specification doesn't say anything about the type of published, and even in these examples they appear as numbers (an unquoted number in yaml is a number, not a string). These discrepancies make difficult to build software or CI systems that rely on different versions of docker-compose.

If current one is the expected behaviour, I think there are still some inconsistencies:

  • In the long form, docker-compose config returns target as number, but published as string. Is target supposed to be a number while published is a string?
  • In most documents I have been able to find, including the Compose Specification, published appears as number.

Using port ranges would be a reason to use strings instead of numbers, but in this case docker-compose decomposes the configuration in multiple ports, with the same discrepancies. For example for a port definition like 8080-8081:8080-8081, docker-compose config returns this:

    ports:
    - mode: ingress
      target: 8080
      published: "8080"
      protocol: tcp
    - mode: ingress
      target: 8081
      published: "8081"
      protocol: tcp

@designermonkey
Copy link

I want to throw another one in as it's technically the same issue:

    deploy:
      resources:
        limits:
          cpus: '0.50'
          memory: 1024M
        reservations:
          cpus: '0.25'
          memory: 512M

Again it's a swarm specific property yet compose breaks it and converts cpus to a float/number which swarm expects to be a string.

If these properties aren't part of the compose spec and compose doesn't care about them, surely leaving them well alone would be best practice?

@ndeloof
Copy link
Contributor

ndeloof commented Aug 12, 2022

Compose specification says:

Can be set as a range using syntax start-end, then actual port SHOULD be assigned within this range based on available ports.

While arguably poorly documented, this means published can be set by start-end as a string, so the type need to be a string in json schema. yaml make it possible to write a string without quotes, so the examples with a simpler port syntax looks like those are numbers, but they are not within the model.

The issue happens with docker-compose

Need to double check then, that's weird

@glours
Copy link
Contributor

glours commented Aug 12, 2022

@jsoriano I just tested with your output, and I can't reproduce the problem with Compose

> bat compose.yaml
───────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: compose.yaml
       │ Size: 212 B
───────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ services:
   2   │   test:
   3   │     image: nginx
   4   │     ports:
   5   │     - mode: ingress
   6   │       target: 8080
   7   │       published: "8080"
   8   │       protocol: tcp
   9   │     - mode: ingress
  10   │       target: 8081
  11   │       published: "8081"
  12   │       protocol: tcp
───────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
> docker compose up -d
[+] Running 1/1
 ⠿ Container issue-9306-test-1  Started                                                                                                                                                                                                  0.2s
> docker compose ps
NAME                COMMAND                  SERVICE             STATUS              PORTS
issue-9306-test-1   "/docker-entrypoint.…"   test                running             80/tcp, 0.0.0.0:8080-8081->8080-8081/tcp
> docker compose version
Docker Compose version v2.9.0

@jsoriano
Copy link
Contributor Author

5 │ - mode: ingress
6 │ target: 8080
7 │ published: "8080"
8 │ protocol: tcp

target is a number, but published is a string. This is different to what happened in v2.2.3, and to what is documented.

@glours
Copy link
Contributor

glours commented Aug 12, 2022

@jsoriano I just opened 2 PRs to update both Docker documentation and the Compose specification

Anyway I'm not sure to understand your issue.
If you use a Compose version >= 2.3.0 with the new format of the port.published attribute everything should work as expected, same if you use port.published as an int with the version 2.2.3
So you mean you're using a compose file generated with a >=2.3.0 docker compose config command with a Compose runtime 2.2.3, that's it?
If this is the case, why not just update your compose runtime version?

@jsoriano
Copy link
Contributor Author

Anyway I'm not sure to understand your issue.
If you use a Compose version >= 2.3.0 with the new format of the port.published attribute everything should work as expected, same if you use port.published as an int with the version 2.2.3
So you mean you're using a compose file generated with a >=2.3.0 docker compose config command with a Compose runtime 2.2.3, that's it?
If this is the case, why not just update your compose runtime version?

My issue was that the format of docker-compose config unexpectedly changed on 2.3.0. As described, this can be an issue when parsing this generated output.

In our case we parse the output of docker-compose config, and we don't have control on what version of docker-compose the user has installed. We ended up supporting both formats for ports (in elastic/elastic-package#748).

@glours
Copy link
Contributor

glours commented Aug 12, 2022

Ok it's clear now, sorry for the inconvenience 😞
A that time, we didn't publish the changes of compose-go in the Compose releases and that was a mistake. We now try to improve the visibility of this kind of changes directly in the Compose release description to avoid this kind of situation.
We also try to keep the documentation up to date as much as possible (and as you demonstrate we failed 😬 )

I don't try to find excuses, we mess up in our communication in that case.
I think supporting the both format in your side is the best solution for now until your customers update to newer versions, soon I hope

@jsoriano
Copy link
Contributor Author

Yep, at this point the best is to support both formats. Thanks for updating the docs!

@danielporto
Copy link

For those that need to translate variables and don't want to use the translation with the node script suggested by @rafipiccolo nor copy it to a .env file, a simple "sed" script does the job:
docker-compose -env-file .envfile-dev -f docker-compose.dev.yml config | sed "/published:/s/"//g"
the sed script remove all chars " whenever the word 'published:' is found.

Then I can come back using the newer docker version while this issue gets properly fixed.

To feed docker stack command:
(echo "version: '3.8'"; docker compose -f traefik/docker-compose.yaml convert) | sed "/published:/s/"//g" | docker stack deploy --with-registry-auth --compose-file - traefik

@ajlouie
Copy link

ajlouie commented Mar 10, 2023

@danielporto's sed solution worked for me after I figured out that the quote character needed to be escaped:

sed "/published:/s/\"//g"

However, it exposed the next problem, where the name field is automatically added by docker compose config (see this issue), which is not supported by docker stack deploy. I fixed that by adding another sed command to delete any line that starts with "name:":

sed "/^name:/d"

Putting it all together in one command:

docker stack deploy -c <(echo -e "version: '3.9'"; docker compose config | (sed "/published:/s/\"//g") | (sed "/^name:/d")) "stack-name-goes-here"

@membersound
Copy link

If any (like me) has the same problem: while the sed solution works, it is better to use docker stack config -c... nowadays: docker/cli#3544

@rafipiccolo
Copy link

rafipiccolo commented Aug 22, 2023

I tried it. and it's not a drop in replacement for docker compose config. (although it may solve some bugs like ports format)
In this example i can see that env vars are not replaced.

# cat .env
DOMAIN=test.fr
# cat swarm.yml
version: "3.7"
services:
    whoami:
        image: whoami:latest
        working_dir: /usr/app
        deploy:
            labels:
                - traefik.http.routers.chrome.rule=Host(`whoami.${DOMAIN}`)
        environment:
            - "HOSTNAME=whoami.${DOMAIN}"
            - "TZ=Europe/Paris"
        ports:
            - "8080:8080"
# docker stack config -c /tmp/test.yml
version: "3.7"
services:
  whoami:
    deploy:
      labels:
        traefik.http.routers.whoami.rule: Host(`whoami.`)
    environment:
      HOSTNAME: whoami.
      TZ: Europe/Paris
    image: whoami:latest
    working_dir: /usr/app
    ports:
    - mode: ingress
      target: 8080
      published: 8080
      protocol: tcp

Docker Compose version v2.20.3
Client: Docker Engine - Community
Version: 24.0.5
API version: 1.43
Go version: go1.20.6
Git commit: ced0996
Built: Fri Jul 21 20:35:18 2023
OS/Arch: linux/amd64
Context: default

@membersound
Copy link

My bad, you're absolutely correct: docker stack config seems to ignore .env file. What a pitty. So we still have to stick to the sed solution?

@rafipiccolo
Copy link

FWIW docker seems to believe that the real solution is to source .env in the real env
#9306 (comment)

when you do this can get rid completely of "docker compose config" / "docker stack config".
but ofc depending on your tooling it's may be easier to just sed.

@membersound
Copy link

The then the following would be our command:

export $(cat .env) && docker stack config -c docker-compose.base.yml -c docker-compose.yml --skip-interpolation | docker stack deploy --compose-file - $APPLICATION_NAME

Question: is docker stack config using docker compose v2 under the hood? That's the reason why I'm rewriting, to switch from docker-compose v1 away...

@ndeloof
Copy link
Contributor

ndeloof commented Aug 23, 2023

is docker stack config using docker compose v2 under the hood

Nope, docker stack still relies on legacy docker/cli codebase to parse compose.yaml and didn't adopted the compose-specification (see docker/cli#2527)

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

No branches or pull requests

9 participants