docker / compose

Define and run multi-container applications with Docker
https://docs.docker.com/compose/
Apache License 2.0
33.74k stars 5.19k forks source link

`compose` doesn't correctly merge `exposes`, `heathchecks.test`, and other arrays #9756

Closed ben-page closed 1 year ago

ben-page commented 2 years ago

Description

docker compose doesn't properly merge arrays. This can lead to an invalid docker compose file being generated. And it's a breaking change between v1 and v2.

Steps to reproduce the issue:

  1. Create yml files

a.yml

services:
  test:
    image: fake
    expose:
    - "1001"
    healthcheck:
      test: ["CMD", "curl", "-f", "http://localhost"]

b.yml

services:
  test:
    image: fake
    expose:
    - "1001"
    healthcheck:
      test: ["CMD", "curl", "-f", "http://localhost"]
  1. Run:
    > docker compose -f a.yml -f b.yml config

Describe the results you received:

name: test
services:
  test:
    expose:
    - "1001"
    - "1001"
    healthcheck:
      test:
      - CMD
      - curl
      - -f
      - http://localhost
      - CMD
      - curl
      - -f
      - http://localhost
    image: fake
    networks:
      default: null
networks:
  default:
    name: test_default

docker compose generated an invalid docker compose file. exposes is expected to contain unique values and will error out. healthcheck.test is valid, but messed up.

Describe the results you expected:

name: test
services:
  test:
    expose:
    - "1001"
    healthcheck:
      test:
      - CMD
      - curl
      - -f
      - http://localhost
    image: fake
    networks:
      default: null
networks:
  default:
    name: test_default

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

Docker Compose v1 worked differently:

> docker-compose -f a.yml -f b.yml config
services:
  test:
    expose:
    - '1001'
    healthcheck:
      test:
      - CMD
      - curl
      - -f
      - http://localhost
    image: fake
version: '3.9'

The behavior of docker-compose (v1) isn't completely obvious. It appears to use different strategies for different arrays. It seems to be merging exposes and removing duplicates, but overwriting heathchecks.test.

For example, a.yml

services:
  test:
    image: fake
    expose:
    - "1001"
    healthcheck:
      test: ["CMD", "curl", "-f", "http://localhost"]

b.yml

services:
  test:
    image: fake
    expose:
    - "1002"
    healthcheck:
      test: ["CMD", "curls", "-f", "http://localhost"]

Returns:

services:
  test:
    expose:
    - '1001'
    - '1002'
    healthcheck:
      test:
      - CMD
      - curls
      - -f
      - http://localhost
    image: fake
version: '3.9'

Output of docker compose version:

Docker Compose version v2.7.0

Output of docker info:

Client:
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc., v0.8.2)
  compose: Docker Compose (Docker Inc., v2.7.0)
  extension: Manages Docker extensions (Docker Inc., v0.2.8)
  sbom: View the packaged-based Software Bill Of Materials (SBOM) for an image (Anchore Inc., 0.6.0)
  scan: Docker Scan (Docker Inc., v0.17.0)

Server:
 Containers: 17
  Running: 14
  Paused: 0
  Stopped: 3
 Images: 66
 Server Version: 20.10.17
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: true
  userxattr: false
 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.runc.v2 io.containerd.runtime.v1.linux runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 10c12954828e7c7c9b6e0ea9b0c02b01407d3ae1
 runc version: v1.1.2-0-ga916309
 init version: de40ad0
 Security Options:
  seccomp
   Profile: default
 Kernel Version: 5.10.102.1-microsoft-standard-WSL2
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 8
 Total Memory: 31.35GiB
 Name: docker-desktop
 ID: VZY6:J6XM:JUYX:J22T:KXTJ:KZAL:5THN:FHK3:QMT7:QDHZ:CTMJ:IYGF
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 HTTP Proxy: http.docker.internal:3128
 HTTPS Proxy: http.docker.internal:3128
 No Proxy: hubproxy.docker.internal
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  hubproxy.docker.internal:5000
  127.0.0.0/8
 Live Restore Enabled: false

WARNING: No blkio throttle.read_bps_device support
WARNING: No blkio throttle.write_bps_device support
WARNING: No blkio throttle.read_iops_device support
WARNING: No blkio throttle.write_iops_device support

Additional environment details:

EricSalemi commented 2 years ago

command is another array that could benefit from merging.

If I look at the documentation here, it says:

For single-value options like image, command or mem_limit, the new value replaces the old value.

However the doc about command here says the inverse:

The command can also be a list, in a manner similar to dockerfile.

For me this is a pretty useful scenario where I would extend the list of commands for a container via compose file merging, but there is no easy way to do this. Please let me know if this deserves a separate issue.

rltas commented 11 months ago

command is another array that could benefit from merging.

If I look at the documentation here, it says:

For single-value options like image, command or mem_limit, the new value replaces the old value.

However the doc about command here says the inverse:

The command can also be a list, in a manner similar to dockerfile.

For me this is a pretty useful scenario where I would extend the list of commands for a container via compose file merging, but there is no easy way to do this. Please let me know if this deserves a separate issue.

I'm just trying to merge command via compose.override.yaml for a Traefik container on a system where I need an additional config like this:

services:
  traefik:
    command:
      - --providers.file.filename=/etc/traefik/rules.yaml
    volumes:
      - ${PWD}/rules.yaml:/etc/traefik/rules.yaml:ro
    extra_hosts:
      - host.docker.internal:host-gateway

Turns out it doesn't work because command in compose.override.yaml just overwrites compose.yaml.

ndeloof commented 11 months ago

Shell commands ALWAYS are overridden, see https://github.com/compose-spec/compose-spec/blob/master/13-merge.md#shell-commands

Your use case is better addressed with an entrypoint + command to pass additional parameters

rltas commented 11 months ago

https://docs.docker.com/compose/multiple-compose-files/merge/#merging-rules

For single-value options like image, command or mem_limit, the new value replaces the old value.

I'm not a YAML expert, but

command:
 - --argument1
 - --argument2
 - --argument3

looks like a sequence/list to me.

ndeloof commented 11 months ago

better read the reference : https://docs.docker.com/compose/multiple-compose-files/merge/#reference-information

rltas commented 11 months ago

The reason a command in sequence form isn't treated like other sequences is that it's internally converted to exec syntax then I guess, wouldn't it make sense to explicity mention that?

ndeloof commented 11 months ago

as already noted, this is explicitly documented in reference content https://github.com/compose-spec/compose-spec/blob/master/13-merge.md#shell-commands

rltas commented 11 months ago

It may technically by explicit but not clear as it's conflicting with the sequence merge logic.